search

dwickern / swagger-play

Swagger API docs for Play Framework
Apache License 2.0
22 stars 4 forks source link

Fix determine host from request when not set via application config #8

Open diversit opened 3 years ago

diversit commented 3 years ago

In Swagger-Play 1.7.1 a change was made which only allowed the host to be set via the configuration with a default value of localhost:9000. This is problematic since that makes it difficult to get the correct host in the swagger.conf since the service might be available on different ports depending on the environment (development, accept/prod, in docker, in IDE, etc). With a wrong host the Swagger docs do not 'work' anymore.

This fixes the issue by changing the default to empty ("") and use the request.host in case the configured host is empty. So this still allowes setting the host to a fixed value, but now also allows to get host dynamically from the request.

dwickern commented 3 years ago

use the request.host in case the configured host is empty

Isn't that the default behavior when you don't set the host? https://swagger.io/specification/v2/

If the host is not included, the host serving the documentation is to be used (including the port).

diversit commented 3 years ago

use the request.host in case the configured host is empty

Isn't that the default behavior when you don't set the host? https://swagger.io/specification/v2/

That should be the default behaviour, but currently it is not. Now it defaults to localhost:9000 and can be overridden via swagger.api.host but this is cumbersome to configure for each environment. Easier to use the host serving the documentation, but still allow override it with a 'static' host. And that is exactly what this PR does.

If the host is not included, the host serving the documentation is to be used (including the port).

This this PR, this becomes indeed the default behaviour.

diversit commented 3 years ago

Mmm ... still using Java 8. That is obsolete ;-) Will fix it for now, but please consider updating to Java 11 since that's the current LTS. Version 17, coming next September, will be the next LTS.