Generate components for OpenAPI schemas. #7124

gnuletik · 2020-01-08T15:20:43Z

As described in this issue, the OpenAPI specs generated by DRF may use a reference to existing models.

This PR add a components/schema property in the OpenAPI specs for serializers having a model property. The schema's name is the model's class name.
For serializers without model, the properties are inlined as before.

I added a sample test but I may add other ones if this PR is okay for you.

Edit: The schema references are also discussed here.

Thanks :)

carltongibson

Hi @gnuletik. Thanks for this — It's a nice start.

As per the comment, I've opened #7127 to refactor to allow a straight get_components() call, parallel to (and before) get_operation(). If you wanted to rebase and adjust on top of that, it would be cool.

carltongibson · 2020-01-08T20:28:54Z

rest_framework/schemas/openapi.py

+        RESPONSE = 1
+        BODY = 2
+
+    def _get_item_schema(self, serializer, schema_mode, method):


I don't see the benefit of this refactoring. It's less clear than the separate methods, and doesn't save a lot of duplication (what with the enum and the bit if...else on the schema_mode...)

You're right! I removed this refactoring.

carltongibson · 2020-01-08T20:29:56Z

rest_framework/schemas/openapi.py

+
+        # If the model has no model, then the serializer will be inlined
+        if not hasattr(serializer, 'Meta') or not hasattr(serializer.Meta, 'model'):
+            return None


Q: Do we need to allow for different components (with different fields) for requests and responses, at least?

Good question ! We could have have two components like ModelNameRequest and ModelNameResponse.
However, we may better keep one component and use the readOnly and writeOnly properties.

This is the generator's job to deduce that :

properties having writeOnly: true should not be in a Response

properties having readOnly: true should not be in Request

More information here : https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md

carltongibson · 2020-01-08T20:30:51Z

rest_framework/schemas/openapi.py

-        return operation
+        component_schema = self._get_component_schema(path, method)
+
+        return operation, component_schema


I see why you've done this, but I think it would be better to refactor to avoid the tuple return here. c.f. #7127.

Thanks ! I rebased my PR on your's !

carltongibson

Hi @gnuletik. Thanks for this. If you can now rebase on master again I'll give it a proper look.

gnuletik · 2020-01-24T16:26:12Z

Hi @carlfarrington, I rebased my branch on master.

carlfarrington · 2020-01-24T16:27:09Z

@carltongibson

gnuletik · 2020-01-30T17:42:23Z

Hey there,

After using this fork I realized an issue with the PR : using the model name as the component name could lead to a wrong schema.
For example, if you have an ItemDetailSerializer and ItemSerializer (with different fields) who use both the same Model, you may have a component with extra fields or missing fields.

As a workaround, the schema name is deducted from the serializer.
The current solution look for a Meta.schema_component_name attribute in the serializer and use it as the schema name if present.
If not, the schema name will be serializer.__class__.__name__ minus the "Serializer" string.

Let me know what you think of it :)
Thanks!

gnuletik · 2020-02-05T15:49:25Z

Do you know how serializers with FileField should be handled with components ?
As stated here, the properties in the request and response may be different.

It seems that request properties should have format: binary and responses should omit format.
Does it mean that we should create one component for the Request and one for the Response?

There is an example in the OpenAPI 3.0.2 spec : https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#considerations-for-file-uploads but the schemas is inlined. I can't find any example with a component referenced.

carltongibson · 2020-02-05T18:40:35Z

...using the model name as the component name could lead to a wrong schema.

Yes. I think in general it's a bit more complex than you have here. (I'm thinking on it but...)

gnuletik · 2020-02-11T16:42:33Z

...using the model name as the component name could lead to a wrong schema.

Yes. I think in general it's a bit more complex than you have here. (I'm thinking on it but...)

What's wrong with the actual implementation ?
The serializer's name is now used instead of the model's name.

carltongibson · 2020-02-11T17:51:50Z

Hi @gnuletik. I think we need to allow for request and response components. Take just read-only and write-only fields — they lead to different components.

But folks also want to be able to use different serializers for different methods and such.

The default implementation just needs to handle core cases, but the API needs to be correct so that folks can customize easily.

Make sense?

gnuletik · 2020-02-12T10:12:42Z

Hi @carltongibson,
Thanks for the feedback!

I think we need to allow for request and response components. Take just read-only and write-only fields — they lead to different components.

One point of component is to keep the same component for Request / Response.
For read-only and write-only fields, those are handled with the writeOnly and readOnly field of the spec : https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#fixed-fields-20

But folks also want to be able to use different serializers for different methods and such.

There is no regression in this PR that would change how the view's serializer is retrieved.
Someone is free to use different serializers for different methods.

The default implementation just needs to handle core cases, but the API needs to be correct so that folks can customize easily.

Totally agree with you! That's the point of this PR : Handle core cases and allow folks to customize it easily.

carltongibson · 2020-02-12T18:51:51Z

One point of component is to keep the same component for Request / Response.
For read-only and write-only fields, those are handled with the writeOnly and readOnly field of the spec : https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#fixed-fields-20

Yes. OK.I need to have a closer look at the details but I think this isn't far off then. Super.

carltongibson

Right. 🙂 This is good.

But I'm not seeing why we don't always just use the serialiser to create a component, regardless of whether there's a model or not. (I don't have to be using ModelSerializer.)

See the inline comments. They should make sense.

The existing tests will need adjusting, and we should document the get_components() and get_component_name() methods, plus the __init__() arg component_name.

But I don't think we need to do anything more complicated that that.

carltongibson · 2020-02-12T19:15:18Z

rest_framework/schemas/openapi.py

        paths = {}
        _, view_endpoints = self._get_paths_and_endpoints(None if public else request)
        for path, method, view in view_endpoints:
            if not self.has_view_permissions(path, method, view):
                continue

            operation = view.schema.get_operation(path, method)
+            component = view.schema.get_components(path, method)


name this components with an s just in case folks want to return multiple components.

carltongibson · 2020-02-12T19:15:48Z

rest_framework/schemas/openapi.py

+            component = view.schema.get_components(path, method)
+
+            if component is not None:
+                components_schemas.update(component)


Drop the guard. Let's have get_components always return a dict.

carltongibson · 2020-02-12T19:16:36Z

rest_framework/schemas/openapi.py

@@ -101,6 +112,34 @@ def get_operation(self, path, method):

        return operation

+    def _get_serializer_component_name(self, serializer):


Rename this just get_component_name().

carltongibson · 2020-02-12T19:16:52Z

rest_framework/schemas/openapi.py

@@ -101,6 +112,34 @@ def get_operation(self, path, method):

        return operation

+    def _get_serializer_component_name(self, serializer):
+        if not hasattr(serializer, 'Meta'):
+            return None


carltongibson · 2020-02-12T19:18:51Z

rest_framework/schemas/openapi.py

+            return None
+
+        if hasattr(serializer.Meta, 'schema_component_name'):
+            return serializer.Meta.schema_component_name


Drop this. Instead have AutoSchema take an __init__() kwarg, component_name. If that's passed use it. Otherwise generate a name as per below.

Usage would be:

class MyView(APIView): schema = AutoSchema(component_name="Ulysses")

If folks need more complex usage, they can override the method.

carltongibson · 2020-02-12T19:19:15Z

rest_framework/schemas/openapi.py

+        component_name = serializer.__class__.__name__
+        # We remove the "serializer" string from the class name.
+        pattern = re.compile("serializer", re.IGNORECASE)
+        return pattern.sub("", component_name)


These three lines are probably fine for a default implementation.

carltongibson · 2020-02-12T19:21:58Z

rest_framework/schemas/openapi.py

+        serializer = self._get_serializer(path, method)
+
+        if not isinstance(serializer, serializers.Serializer):
+            return None


Let's always return a dict. {}

carltongibson · 2020-02-12T19:22:18Z

rest_framework/schemas/openapi.py

+        component_name = self._get_serializer_component_name(serializer)
+
+        if component_name is None:
+            return None


carltongibson · 2020-02-12T19:24:39Z

rest_framework/schemas/openapi.py

@@ -491,6 +530,10 @@ def _get_serializer(self, path, method):
                          .format(view.__class__.__name__, method, path))
            return None

+    def _get_reference(self, serializer):
+        component_name = self._get_serializer_component_name(serializer)
+        return {'$ref': '#/components/schemas/{}'.format(component_name)}


I think inline this function.

carltongibson · 2020-02-12T19:25:02Z

rest_framework/schemas/openapi.py

+            # If possible, the serializer should use a reference
+            item_schema = self._get_reference(serializer)
+        else:
+            # There is no model, we'll map the serializer's fields


Then we can drop this block.

gnuletik · 2020-02-13T15:39:23Z

Thanks for your review and your time @carltongibson ! I updated the PR.

I'll also be glad to write some user-documentation about it (inside docs/api-guide/schemas.md).
I was thinking about:

Adding an introduction about components
Links to the OpenAPI Specs
Component name customization with AutoSchema(component_name="Ulysses")
Example of overriding get_component_name and get_components

What do you think about it ? Should I make a separate PR ?

carltongibson · 2020-02-13T15:42:14Z

Hi @gnuletik.

Should I make a separate PR ?

No, add the docs changes here. (They're integral.)

Thanks for the effort! 🥇

rest_framework/schemas/openapi.py

carltongibson

Hi @gnuletik.

Thank you for this. It is really nice. Just a couple of comments, a rebase needed.

Good work! 🥇

carltongibson · 2020-03-02T15:45:55Z

docs/api-guide/schemas.md

+=======
+### Components
+
+Since DRF 3.12, Schema uses the [OpenAPI Components](openapi-components). This method define components in the schema and [referenced them](openapi-reference) inside request and response objects. The component's name is deduced from the Serializer's name.


defines and references.

By default, the component's name...

carltongibson · 2020-03-02T15:47:59Z

docs/api-guide/schemas.md

+
+Using OpenAPI's components provides the following advantages:
+* The schema is more readable and lightweight.
+* If you use the schema to generate a SDK (using [openapi-generator](openapi-generator) or [swagger-codegen](swagger-codegen)). The generator can name your SDK's models.


an SDK ("es-dee-kay")

martyzz1 · 2020-03-13T13:25:25Z

I came looking to see if anybody else was looking into this as I was thinking of doing it myself - if nobody had done it already... So great work!!

I have a question, and figured this might be a better place to ask it?
Why base the component/schema on the underlying Model - rather than the Serializer? (which was the route I was going to take as I thought it might be a more comprehensive solution)

gnuletik · 2020-03-13T15:01:17Z

Thanks @martyzz1 !
At first, we used the Model name as a base for the component/schema.
Then, we changed it and we are now using the Serializer's name :)

See the get_component_name method:
https://github.com/encode/django-rest-framework/pull/7124/files#diff-16b3da8ab434dab83a9fcf30f1322643R161

martyzz1 · 2020-05-20T12:33:40Z

Thanks @gnuletik.

carltongibson self-assigned this Jan 8, 2020

carltongibson added the Schema Generation label Jan 8, 2020

carltongibson mentioned this pull request Jan 8, 2020

OpenAPI endpoint enumeration refactoring. #7127

Merged

carltongibson reviewed Jan 8, 2020

View reviewed changes

gnuletik force-pushed the openapi-reference-components branch from 69c6355 to 83f0810 Compare January 12, 2020 08:34

carltongibson reviewed Jan 21, 2020

View reviewed changes

gnuletik force-pushed the openapi-reference-components branch from f7de465 to 8e65ae9 Compare January 24, 2020 14:50

carltongibson force-pushed the openapi-reference-components branch from f4d826d to 568452c Compare February 12, 2020 18:58

carltongibson changed the title ~~Add schema references for OpenAPI generation, see #6984~~ Generate components for OpenAPI schemas. Feb 12, 2020

carltongibson reviewed Feb 12, 2020

View reviewed changes

This was referenced Feb 12, 2020

OpenAPI3 proposal: Components #7096

Closed

OpenAPI3 proposal for enhancements/fixes #7089

Closed

carltongibson added this to the 3.12 Release milestone Feb 12, 2020

gnuletik commented Feb 13, 2020

View reviewed changes

rest_framework/schemas/openapi.py Show resolved Hide resolved

gnuletik force-pushed the openapi-reference-components branch 2 times, most recently from d295b9f to d6fbc32 Compare February 28, 2020 18:03

carltongibson reviewed Mar 2, 2020

View reviewed changes

Implement OpenAPI Components

7c8edbc

gnuletik force-pushed the openapi-reference-components branch from d6fbc32 to 7c8edbc Compare March 2, 2020 17:37

carltongibson approved these changes Mar 2, 2020

View reviewed changes

carltongibson merged commit 8aa8be7 into encode:master Mar 2, 2020

tfranzel mentioned this pull request Mar 29, 2020

OpenAPI3 proposal for enhancements/fixes #7088

Closed

6 tasks

pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020

Implement OpenAPI Components (encode#7124)

2c15f52

sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022

Implement OpenAPI Components (encode#7124)

c9aff1e

		@@ -101,6 +112,34 @@ def get_operation(self, path, method):

		return operation

		def _get_serializer_component_name(self, serializer):

Generate components for OpenAPI schemas. #7124

Generate components for OpenAPI schemas. #7124

Conversation

gnuletik commented Jan 8, 2020 • edited by carltongibson Loading

carltongibson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson left a comment

Choose a reason for hiding this comment

gnuletik commented Jan 24, 2020

carlfarrington commented Jan 24, 2020

gnuletik commented Jan 30, 2020

gnuletik commented Feb 5, 2020 • edited Loading

carltongibson commented Feb 5, 2020

gnuletik commented Feb 11, 2020

carltongibson commented Feb 11, 2020

gnuletik commented Feb 12, 2020

carltongibson commented Feb 12, 2020

carltongibson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnuletik commented Feb 13, 2020

carltongibson commented Feb 13, 2020

carltongibson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martyzz1 commented Mar 13, 2020

gnuletik commented Mar 13, 2020

martyzz1 commented May 20, 2020

gnuletik commented Jan 8, 2020 •

edited by carltongibson

Loading

gnuletik commented Feb 5, 2020 •

edited

Loading