federecio / dropwizard-swagger

a Dropwizard bundle that serves Swagger UI static content and loads Swagger endpoints.
Other
130 stars 186 forks source link

Problem with custom base URL #18

Closed derekcicerone-zz closed 9 years ago

derekcicerone-zz commented 9 years ago

We have all our Jersey APIs running under /api. When I add the Swagger module to DropWizard it seems to put /swagger under /api (so /api/swagger) but it still expects /api-docs to be at the root (vs /api/api-docs). I didn't see a good way to fix the "api-docs" URL so that its properly nested under /api.

federecio commented 9 years ago

Sorry for this late response. The situation you describe should not be a problem. What version of dropwizard-swagger are you using?

derekcicerone-zz commented 9 years ago

I'm using io.federecio:dropwizard-swagger:0.5.1 (via Gradle).

The code we are using to put all the REST APIs under /api looks like this:

environment.jersey().setUrlPattern("/api/*");
federecio commented 9 years ago

What server type are you using? simple or default? Can you please share your configuration file?

derekcicerone-zz commented 9 years ago

Is this the part you are interested in?

server:
  adminConnectors:
    - type: http
      port: 3081
  applicationConnectors:
    - type: https
      port: 3443
      keyStorePath: My_Keystore
      keyStorePassword: keystore
      validateCerts: false
    - type: http
      port: 3080
  requestLog:
    appenders: []
derekcicerone-zz commented 9 years ago

Looks like we use default.

federecio commented 9 years ago

Ok I see. It's a bug in 0.7.x that fact that you are not able to specify the path in the config file (see https://github.com/dropwizard/dropwizard/pull/665). Apparently this is going be available in 0.8+ as the pull request has been merged. So you are basically using a workaround by setting it programatically (more discussions here https://groups.google.com/forum/#!topic/dropwizard-user/ElrV5BqBcoo)

Let me see how to make it work in this case (default server with a context path other than the root).

If you were using simple server, it would work just fine with the /api path.

derekcicerone-zz commented 9 years ago

Are you sure the linked issues are related? We aren't setting a context path - we are only setting a URL pattern for the Jersey stuff so that it will all be under /api and won't potentially collide with static resources. It seems like Swagger needs 2 things: a web page/view for the UI and a resource for the REST APIs where it gets it's data. Currently the UI is being configured via Jersey so it goes under the Jersey path /api but the REST APIs are going under the normal server path which is /. There doesn't seem to be logic for handling a situation in which a non-default Jersey path has been set. I don't think it's necessarily a workaround to set it programmatically but I can definitely believe its a rarely used option.

The pull request you referenced doesn't appear to be merged yet (though it is tagged for inclusion in 0.8.0).

federecio commented 9 years ago

I pushed a potential fix for your problem. I took some time to fix it because I wanted to set up Selenium tests to make sure the UI works properly for the different combinations of server types and context path (either set as part of DropWizard configuration or programatically via environment.jersey().setUrlPattern(...)

I will deploy a snapshot shortly and will kindly ask you to try it out and see if your issue is solved.

federecio commented 9 years ago

Ok the snapshot is available under version 0.5.2-SNAPSHOT. Please take a look and let me know if it fixes your issue.

It takes a few hours to sync to Central from Sonatype OSS. If you can't get it from central it's available here: https://oss.sonatype.org/content/repositories/snapshots/io/federecio/dropwizard-swagger/0.5.2-SNAPSHOT/

derekcicerone-zz commented 9 years ago

Awesome, thank you! I'll take a look at the fix first thing tomorrow when I'm back at my computer).

derekcicerone-zz commented 9 years ago

Hmm, looks better - its hitting the right URL now. Unfortunately its still not working. When it calls https://localhost:3443/api/api-docs, it only returns:

{"apiVersion":"0.0","swaggerVersion":"1.2"}

This makes the JavaScript code throw an exception - it looks like there should be an "apis" property in there. I set a class-level @Api annotation on one of my resource classes so hitting https://localhost:3443/api/api-docs/sources works fine but for some reason it doesn't look like "sources" is being listed as an API.

Is there anything else I need to do besides adding annotations to a resource class to make this work? (I hooked up the code in my server class for onInitialize and onRun as well)

federecio commented 9 years ago

All your resources need to be annotated with @Api and your resource's methods with @ApiOperation and of course you need to register them with Jersey during the run() method of your Application class:

environment.jersey().register(new YourResource());

It seems you are missing some basic swagger stuff, if you share a sample project with me I could take a look.

derekcicerone-zz commented 9 years ago

Doh, sorry, I thought @ApiOperation was optional. Added that annotation and everything works great now - thanks!

federecio commented 9 years ago

Great!

Closing this issue. Thanks.

aaaaahaaaaa commented 9 years ago

The problem originally described by @derekcicerone seems to be an existing bug with 0.5.3. Swagger tries to load api-docs from the base url localhost:8080/api-docs instead of localhost:8080/api/api-docs after setting a custom base url with :

environment.jersey().setUrlPattern("/api/*");
yeraabh commented 9 years ago

I think there is still an issue in version 0.5.3 with simple server and a custom url pattern.

In getContextPath() of SwaggerConfiguration, contextPath is read from applicationContextPath()

if (serverFactory instanceof SimpleServerFactory) { applicationContextPath = ((SimpleServerFactory) serverFactory).getApplicationContextPath();