Core: Swagger ui overwrites html encoder response

[maechler]

I have recently updated api-platform/core from beta to rc2. Since that the swagger ui overwrites all our html encoder responses. I feel like there is something wrong with the condition here: https://github.com/api-platform/core/blob/master/src/Bridge/Symfony/Bundle/EventListener/SwaggerUiListener.php#L27-L28

If we request for instance an url like http://fancy-api.com/documents/1 and we have an html encoder registered the condition for exiting that function are not met:

if ('html' !== $request->getRequestFormat(null) 
    || (!$request->attributes->has('_api_resource_class') && !$request->attributes->has('_api_respond'))
    ) {
        return;
    }

reads:

if (false 
    || (!true && !false)
    ) {
        return;
    }

I think in the earlier version the swagger ui was only triggered on the route /apidoc. Maybe it would be possible to have something like this:

/doc --> swagger ui
/doc.jsonld --> hydra doc

What do you guys think?

[maechler]

I have just committed a workaround for our project: https://github.com/linked-swissbib/hydra-swissbib.ch/blob/develop/src/LinkedSwissbibBundle/Bridge/Symfony/Bundle/EventListener/SwaggerUiListener.php#L17

Although you might want to implement it a little different in the core, that exactly reflects the behaviour I suggested.

[Simperfit]

@maechler I think that it will be better to add a parameter true by default that allow you to disable this feature.

[maechler]

@Simperfit Thanks, but that is just a workaround anyways. I think the way it should be solved in the core needs to be discussed. I do not know what you want to make the interface look like.

[Simperfit]

@maechler IMO we should let it activate by default, it's something great to begin with, but I agree that it should not be mandatory to use it.

[Simperfit]

@maechler could you test this PR ?

[maechler]

@Simperfit Thanks for the pull request! I tested it, the deactivation of the swagger ui works perfectly.

but..with that implementation it is not possible to have an HTML encoder and the swagger ui at the same time. That is why our workaround only enables the swagger ui on the route /doc. It does not have to be like that, but I think there should be the possibility to have an HTML encoder and the swagger ui at the same time.

What do you think?

Maybe we could add an additional parameter swagger_ui_path with which it possible to restrict the ui to a certain path. Together with your parameter enable_swagger_ui I think we could cover all use cases.

We would for instance have a configuration like that:

api_platform:
    enable_swagger_ui: true
    swagger_ui_path: '/doc'

The default behaviour would be covered with an empty swagger_ui_path and if someone does not want to use swagger at all he can deactivate it with enable_swagger_ui.

[Simperfit]

That can be a great improvement.

Since you have another html encoder, what happens if you GET /doc.html ?

I'm 👍

@api-platform/core-team WDYT ?

[maechler]

@Simperfit
With our implementation the hydra documentation is being generated and then the html encoder gets triggered when we access /doc.html. I think that is actually the desired behaviour.

[maechler]

But that is just with our decorated DocumentationNormalizer where we support all supported formats for the documentation. I just checked the api-platform default behaviour:

a) 500 error: No resource class found., if html is a supported format
b) 404 error: Not found., if html is not supported

[dunglas]

The swagger_ui_path is not necessary. You can already register the SwaggerUI controller by yourself, you just need to add the following to app/config/routing.yml:

swagger_ui:
    path:       /swaggerui
    defaults:  { _controller: api_platform.swagger.action.ui }