Add support for OpenAPI 3 #2302

antograssiot · 2018-11-02T14:32:45Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR	api-platform/docs#706

This PR aims to provide a way to export in a OpenAPI 3.0 compatible format.

This is still kind of a work in progress but I wanted to get early feedback on this.
Still tests and CS should be ok and good enough.
To ensure that the new normalizer is at least as tested as the v2, I've used the same tests and updated the expectations.

Here is some topics I'd like to discuss:

I made the choice to create a separate documentation normalizer, here are the main reasons: ease v2 removal maybe at some point, do no interfere with Api Gateway normalizer, allow to be marked as experimental. I'm inclined to change this...
This solution doesn't "transform" a swagger 2 compliant code to a version 3 compliant code. This implies that in the current situation, any overridden swagger context might not end up valid. Is it a big issue ?

Thank you before end for your time reviewing and discussing this.

TODO:

Check that the security parts of the generated files is ok
Add Links support
use query param to select version

antograssiot · 2018-11-02T17:19:17Z

src/Bridge/Symfony/Bundle/Command/SwaggerCommand.php

-        $data = $this->documentationNormalizer->normalize($documentation);
-        $content = $input->getOption('yaml') ? Yaml::dump($data, 6, 4, Yaml::DUMP_OBJECT_AS_MAP | Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE) : (json_encode($data, JSON_PRETTY_PRINT) ?: '');
+        $data = $this->$documentationNormalizer->normalize($documentation);
+        $content = $input->getOption('yaml') ? Yaml::dump($data, 10, 2, Yaml::DUMP_OBJECT_AS_MAP | Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE) : (json_encode($data, JSON_PRETTY_PRINT) ?: '');


I've increased the depth before inlining and reduced the indentation to 2 spaces as the latest version is even more nested

antograssiot · 2018-11-02T17:22:46Z

src/Serializer/AbstractDocumentationNormalizer.php

+
+declare(strict_types=1);
+
+namespace ApiPlatform\Core\Serializer;


Not sure if it belongs in this namespace

Could be left in Swagger or added in OpenApi.

src/OpenApi/Serializer/DocumentationNormalizer.php

src/Serializer/AbstractDocumentationNormalizer.php

src/Bridge/Symfony/Bundle/Command/SwaggerCommand.php

antograssiot · 2018-11-02T18:07:19Z

https://app.swaggerhub.com/apis/antograssiot/api-platform-fixtures/0.0.0
is the result

src/Bridge/Symfony/Bundle/Command/SwaggerCommand.php

src/OpenApi/Serializer/DocumentationNormalizer.php

dunglas · 2018-11-02T20:12:25Z

src/Serializer/AbstractDocumentationNormalizer.php

+
+declare(strict_types=1);
+
+namespace ApiPlatform\Core\Serializer;


Could be left in Swagger or added in OpenApi.

antograssiot · 2018-11-03T12:25:09Z

src/Bridge/Symfony/Bundle/Resources/config/swagger.xml

+            <argument>%api_platform.collection.pagination.items_per_page_parameter_name%</argument>
+            <argument>%api_platform.collection.pagination.client_enabled%</argument>
+            <argument>%api_platform.collection.pagination.enabled_parameter_name%</argument>
+            <tag name="serializer.normalizer" priority="18" />


@dunglas I think that this is requested so this normalizer became the default one too.
In understood from your comment in SwaggerCommand that you expect v3 to became the default, so I though it should change by default in swaggerUI too.

I can easily update all the behat swagger tests but I guess that you also expect this to still be tested (I would at least If some people still want to use v2) but I' don't know what would be the way to do that. it would need to change this priority in the test suite... ? or declare api_platform.openapi.normalizer.documentation as an alias of api_platform.swagger.normalizer.documentation ?
Anyway I don't really know if it is possible in a behat context and I wonder if setting this priority here is the good thing to do and if people expect SwagerUI to be updated automatically or if someone will want to stick to the old UI.

What do you think about allowing to choose the OpenAPI version through a query parameter? As done for API Gateway here: https://github.com/api-platform/core/blob/master/src/Documentation/Action/DocumentationAction.php#L66

Then it will be easy to test, and convenient for end users (they will be able to use OpenAPI v3 by default when supported, but still fallback to v2 for old tools).

For Swagger UI, we should to the same thing IMO (defaulting to v3, but allowing to switch to v2 by setting a query parameter).

WDYT?

looks like a good idea, I'll try to update soon

dunglas · 2018-11-05T10:45:39Z

@antograssiot do you think we can benefit from Links in this PR? It looks like a great new feature for API Platform.

antograssiot · 2018-11-05T22:36:41Z

Yeah adding Link seems doable, I'll try to work on that

antograssiot · 2018-11-11T11:18:35Z

@dunglas, I've ben working a bit more on this, not pushed yet and also waiting feedback on #2317 as it will be needed to... but I have a question on how should be handle the custom documentation.
Right now the swagger doc can be overridden using the swagger_context. As currently the OpenAPI normalizer doesn't transform a version 2 into a version 3, should I add the possibility to use a openapi_context attribute so people can override both documentation, should we keep using swagger_context only and then it's up to the developer to choose which version he want to support ?

dunglas · 2018-11-11T21:52:37Z

Introducing an openapi_context looks indeed better to me

antograssiot · 2018-12-18T20:19:53Z

@dunglas So I've rebased and put this branch up to date (implemented #2335 and #2344), I think that every tests should be passing, I'll re-read the whole PR tomorrow, I didn't took time to do this today but it should be ready for another set of review as discussed.

dunglas

Can you rebase one more time please?

meyerbaptiste

The swagger.yaml file seems to be the result of the api:swagger:export command. IMO it could be removed.

Add a OpenAPI 3.0 compatible normalizer Update the export command Update AbstractDocumentation namespace Increase new normalizer priority to make it the default in swaggerUI Rename namespace from OpenApi to OpenAPI Make OpenAPI 3 the default and alow to fallback to 2 Adding openapi_context attribute and cleaning

antograssiot force-pushed the openapi-3 branch from 124c6b2 to 2cfb0a3 Compare November 2, 2018 14:47

antograssiot commented Nov 2, 2018

View reviewed changes

antograssiot force-pushed the openapi-3 branch from 2cfb0a3 to be078cd Compare November 2, 2018 18:08

dunglas requested changes Nov 2, 2018

View reviewed changes

antograssiot force-pushed the openapi-3 branch 2 times, most recently from cc7be75 to 8a7ec7d Compare November 3, 2018 09:45

antograssiot commented Nov 3, 2018

View reviewed changes

antograssiot mentioned this pull request Nov 6, 2018

The swagger normalizer does not detect overridden operation format attribute api-platform/api-platform#886

Open

antograssiot force-pushed the openapi-3 branch from d3e434d to e230116 Compare November 13, 2018 04:36

antograssiot force-pushed the openapi-3 branch from e230116 to d6ddfc6 Compare December 18, 2018 20:15

dunglas approved these changes Dec 20, 2018

View reviewed changes

meyerbaptiste reviewed Dec 20, 2018

View reviewed changes

dunglas force-pushed the openapi-3 branch from d6ddfc6 to c7e7a64 Compare December 20, 2018 10:10

dunglas force-pushed the openapi-3 branch from c7e7a64 to fac1e8c Compare December 20, 2018 10:35

dunglas and others added 4 commits December 20, 2018 12:05

Remove useless swagger.yaml

39e02ae

Rename spec-version in spec_version for consistency with api_gateway

3004e65

Use a new openapi key in filter description

28a0b9a

Fix tests

4f3b992

dunglas merged commit aff44c9 into api-platform:master Dec 20, 2018

dunglas changed the title ~~[WIP] Add support for OpenAPI 3~~ Add support for OpenAPI 3 Dec 20, 2018

antograssiot deleted the openapi-3 branch December 20, 2018 19:56

This was referenced Dec 22, 2018

openApi 3.0.0 DocumentationNormalizer #2297

Closed

Change Swagger word in favor of OpenAPI #1164

Closed

dunglas mentioned this pull request Jan 8, 2019

Document OpenAPI v3 support api-platform/docs#706

Merged

shouze mentioned this pull request Apr 30, 2021

Update spec to OpenApi 3.1 #4069

Closed


		declare(strict_types=1);

		namespace ApiPlatform\Core\Serializer;

Uh oh!

Add support for OpenAPI 3 #2302

Add support for OpenAPI 3 #2302

Uh oh!

Conversation

antograssiot commented Nov 2, 2018 • edited by dunglas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antograssiot Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

antograssiot Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

dunglas Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antograssiot commented Nov 2, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dunglas Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

antograssiot Nov 3, 2018

Choose a reason for hiding this comment

Uh oh!

dunglas Nov 5, 2018

Choose a reason for hiding this comment

Uh oh!

antograssiot Nov 5, 2018

Choose a reason for hiding this comment

Uh oh!

dunglas commented Nov 5, 2018

Uh oh!

antograssiot commented Nov 5, 2018

Uh oh!

antograssiot commented Nov 11, 2018

Uh oh!

dunglas commented Nov 11, 2018

Uh oh!

antograssiot commented Dec 18, 2018

Uh oh!

dunglas left a comment

Choose a reason for hiding this comment

Uh oh!

meyerbaptiste left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

antograssiot commented Nov 2, 2018 •

edited by dunglas

Loading