Issue Creating Paths with Regex Expressions

[nkoterba]

Our Resource is annotated as:

@Path("api/survey/{packageId: [0-9]+}")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
/**
 * @resourcePath testMe
 * @resource testMe
 * @parentEndpointName testMe
 */
public class SurveyResource extends EndpointBase {

However, when the doclet runs, this is the output:

First, it doesn't look like @resourcePath, @resource, or @parentEndpointName is having any effect on the generated filename. Was expecting testMe.json.

Also, in looking at the service.json file, the path still includes the regex expression for the packageId property, instead of "/testMe.{format}":

{
  "swaggerVersion" : "1.2",
  "apiVersion" : "1-dev",
  "basePath" : "/app/apidocs",
  "apis" : [ {
    "path" : "/api_survey_packageId: [0-9]+.{format}"
  } ]
}

Having the regex expression in the path and filename causes issues with the Web UI API viewer b/c the browser can't resolve the file: https://localhost:8443/app/apidocs/api_survey_packageId:%20[0-9]+.json.

However, if I update the filename and paths to be testMe, all works as expected.

Does your doclet handle regex patterns in PathParam variables? Is there some option/JavaDoc element I can set to get this to work?

Thanks!

[conorroche]

regex patterns are not currently supported i will investigate a solution

[nkoterba]

@conorroche So I'm at a crossroads for our project. I really like this project and its ability to annotate using JavaDoc comments and JAX-RS annotations.

I really dislike the fact that swagger has its own set of annotations ON TOP of Javadoc. Granted it gives you more control and flexibility in documenting your API but at the end of the day, it's introducing a dependency in our source with no guarantee that Swagger will exist long-term, that it will maintain it's API, etc.

However, core Swagger does have a lot more exposure, issues, and project activity so issues/bugs/features I know will be implemented more.

For example, a path parameter in a Class' path variable that is required for method API calls seems like it was just fixed in Swagger-Core: https://github.com/swagger-api/swagger-core/issues/1085#issuecomment-103737392

but I bet it would take a while before this project may support it...

Which leaves me with three options: 1) Use Swagger-Core and the Swagger Annotations (ugh) 2) Fork this project and update/add/fix the things we need (could provide pull requests and help but definitely will take more time) 3) Go with an entirely different javadoc generator, e.g. http://fromage.github.io/jax-doclets/docs/0.10.0/html/#d0e259, that doesn't allow live testing of APIs (one of the perks of Swagger) and may have it's own issues but would also just generate documentation

I think I'm going to go with Option 2 above and see how much time it takes or how far I make it...otherwise will go with Option 1.

Sorry for the brain dump...just trying to weigh all the options to support our code base.

[nkoterba]

@conorroche Follow up ?...

So I know regex's won't work...however, shouldn't one of the JavaDoc annotations you list in the documentation override the Regex?

@Path("api/survey/{packageId: [0-9]+}")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
/**
 * @resourcePath testMe
 * @resource testMe
 * @parentEndpointName testMe
 */
public class SurveyResource extends EndpointBase {

I thought that @resourcePath, @resource, or @parentEndpointName would all override the jaxrs @Path annotation at least based on the documentation:

This sets the path for resources in the resource listing e.g. the service.json file. You should put this tag on either a) the resource class if using a single resource class per api resource, or b) one of the operation methods of each resource if you have endpoints from multiple resources in the same class file. NOTE if you have resource classes with empty paths or a path that is / then by default these classes will be give the resource path of /root, if you put @resourcePath on the class this will be used instead of /root. You can also use the doclet parameter -resourcePath to customize the resource path for root resources.

Am I mis-interpreting the documentation?

[conorroche]

i think the easiest is to have the doclet simply strip out the regex when generating the path so instead of api/survey/{packageId: [0-9]+} it would become api/survey/{packageId} which would then work ok, so the fix wont be too difficult. At the moment @resourcePath on the class is only used in the event that either a) the class has an empty @Path("") or @Path("/") eg. root path, or b) for methods that dont have their own tags, it does seem reasonable to be able to override @path

[conorroche]

@nkoterba i have committed a fix to handle regex path param expressions.

[conorroche]

@nkoterba ive been looking into the issue you mentioned in relation to @resourcePath, for me this is taking effect, e.g. if i add @resourcePath myTest then i end up with "path": "/myTest.{format}" in the resource listing (service.json) file and "resourcePath": "/project" in the myTest.json file which is what id expect. This does not impact the api path but that is no fixed in terms of regex, so other than clarify the readme i think that should cover it.

[nkoterba]

@conorroche Thanks for the super quick turnaround! Really appreciate it.

I actually forked and made my own fix/update even with a test fixture named Issue73 =P. I was going to do some more testing and submit a pull request, but I will instead try to use your updates since it's part of the "baseline".

I noticed your update and will check it out, including the tests.

I noticed you use char iteration to remove the regex patterns. I just used a regular expression to handle the same scenario:

// Fix for Issue #73 - Support Regex expressions in JAX-RS @Path annotation
// See: https://github.com/teamcarma/swagger-jaxrs-doclet/issues/73
path = path.replaceAll(":.*?}", "}");

My solution is definitely a little more "risky" on possible edge cases (though I would argue the JAX-RS wouldn't like the Path anyway) and perhaps not as performant but I thought it was an easy to understand solution.

Thanks also for the clarification on the @resourcePath in the documentation. I will check to see if indeed it works as I expected.

I did realize I was doing a big no-no by putting my JavaDoc AFTER my jaxrs annotations. Turns out JavaDoc doesn't like that and screws everything up. So I've now made sure that my code looks as follows:

/**
 * @resourcePath testMe
 * @resource testMe
 * @parentEndpointName testMe
 */
@Path("api/survey/{packageId: [0-9]+}")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public class SurveyResource extends EndpointBase {

[nkoterba]

@conorroche

I think your fix is now 'changing' the resourcePath's to be shorter than they should.

I ran my "Issue73" patch and here was the output of my files:

I then ran the current master and here was the output of my files:

Notice the reduction in number of files as well as the fact that api_workspaces.json filename no longer matches the apis' path property.

I think the issue is here: https://github.com/teamcarma/swagger-jaxrs-doclet/blob/58879ceb183da2072ce7e75204f7cdfd4b731a74/swagger-doclet/src/main/java/com/carma/swagger/doclet/parser/ParserHelper.java#L1611-1626.

Thus, I think the same behavior for resourcePath sanitization should be used as the one for apiPath, e.g. don't remove/replace the {bracketed} expressions, just remove the odd regex symbols from them.

I updated the ParserHelper to be this (and to handle spaces which I see you added to the tests):

    /**
     * Removes all occurrence of Regex expressions from a path
     * For example: /api/{workspaceId: [0-9]+} ==> /api/{workspaceId}
     * @param path The path to sanitize and remove regular expressions from
     * @return Sanitized path
     */
    public static String sanitizePath(String path) {
        // Fix for Issue #73 - Support Regex expressions in JAX-RS @Path annotation
        // See: https://github.com/teamcarma/swagger-jaxrs-doclet/issues/73
        return path.replaceAll("[: ]+.*?}", "}");
    }

I also updated the unit test to test multiple Regex expressions as well as more spacing options:

    /**
     * This tests the api path sanitization
     */
    public void testApiPath() {

        assertEquals("/api", ParserHelper.sanitizePath("/api"));
        assertEquals("/api/test", ParserHelper.sanitizePath("/api/test"));
        assertEquals("/api/test{id}", ParserHelper.sanitizePath("/api/test{id}"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id}"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id }"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id:}"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id : }"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id :}"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id:[0-9]+}"));
        assertEquals("/api/test/{id}", ParserHelper.sanitizePath("/api/test/{id: [0-9]+}"));

        assertEquals("/api/{workspace}/{id}", ParserHelper.sanitizePath("/api/{workspace: \\w+}/{id: " +
            "[0-9]+}"));
    }

I can submit a pull request if you want to accept this behavior, otherwise let me know your thoughts on the above.

[nkoterba]

@conorroche One final comment/question.

So I notice in CrossClassApiParser.java, in the constructor, you get the root and parse it:

    public CrossClassApiParser(DocletOptions options, ClassDoc classDoc, Collection<ClassDoc> classes, Map<Type, ClassDoc> subResourceClasses,
            Collection<ClassDoc> typeClasses, String swaggerVersion, String apiVersion, String basePath) {
        super();
        this.options = options;
        this.classDoc = classDoc;
        this.classes = classes;
        this.typeClasses = typeClasses;
        this.subResourceClasses = subResourceClasses;
        this.rootPath = firstNonNull(parsePath(classDoc, options), "");
        this.swaggerVersion = swaggerVersion;
        this.apiVersion = apiVersion;
        this.basePath = basePath;
        this.parentMethod = null;
    }

And this root later gets used in buildResourcePath, where I've copied your code but use my sanitizePath method:

    private String buildResourcePath(String classResourcePath, MethodDoc method) {
        String resourcePath = getRootPath();
        if (classResourcePath != null) {
            resourcePath = classResourcePath;
        }

        if (this.options.getResourceTags() != null) {
            for (String resourceTag : this.options.getResourceTags()) {
                Tag[] tags = method.tags(resourceTag);
                if (tags != null && tags.length > 0) {
                    resourcePath = tags[0].text();
                    resourcePath = resourcePath.toLowerCase();
                    resourcePath = resourcePath.trim().replace(" ", "_");
                    break;
                }
            }
        }

        // sanitize the path and ensure it starts with /
        if (resourcePath != null) {
            resourcePath = ParserHelper.sanitizePath(resourcePath);

            if (!resourcePath.startsWith("/")) {
                resourcePath = "/" + resourcePath;
            }
        }

        return resourcePath;
    }

But I also added sanitizePath method to parsePath in ParserHelper:

    public static String parsePath(com.sun.javadoc.ProgramElementDoc doc, DocletOptions options) {
        AnnotationParser p = new AnnotationParser(doc, options);
        String path = p.getAnnotationValue(JAX_RS_PATH, "value");
        if (path != null) {
            path = path.trim();
            if (path.endsWith("/")) {
                path = path.substring(0, path.length() - 1);
            }
            if (!path.isEmpty() && !path.startsWith("/")) {
                path = "/" + path;
            }

            return sanitizePath(path);
        }
        return null;
    }

So I guess the question/comment I'm posing here is: Should the root of the CrossClassApiParser be sanitized during constructor call?

Or should we assume that whenever it gets used later (like in buildResourcePath) it will be correctly sanitized.

My suggestion, is to add sanitizePath to the PathHelper.parsePath method so that anytime that method is used it will be sanitized...and then just call it again in CrossClassApiParser.buildResourcePath in case the non-root paths contain any regexs.

But I'm open to thoughts...

[conorroche]

hrmm have u tested in the swagger ui, as far as i can remember the resource path shouldnt be multiple directories but rather be like /res1 which then points to the res1.json file, i agree though that if that is the case you should have ended up with two files for the workspaces e.g. the devices one and the graph one

[nkoterba]

@conorroche

In the middle of some code updates, but the current Github master definitely "drops" files since it appears to create identical "resourcePaths" for different Resources, which results in one of the Resource.json files being overwritten.

[conorroche]

@nkoterba ive committed a fix that solves the dropped files issue, im still looking into the inheritance of paths issue u fixed on ur fork

[nkoterba]

@conorroche Thanks for the quick turnaround!

I looked through the commit files and like the fixes/updates. Keeping the "/" instead of a "_" seems to work great with the rest of the framework.

As for the inheritance of path issues, as mentioned in that issue (https://github.com/teamcarma/swagger-jaxrs-doclet/issues/74#issuecomment-105610676) it's also a general issue with Swagger core...actually surprised this hasn't surfaced sooner.

[conorroche]

issue now resolved so closing out