Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webhooks with serializer's GET specification #1280

Closed
dontic opened this issue Aug 22, 2024 · 6 comments
Closed

Webhooks with serializer's GET specification #1280

dontic opened this issue Aug 22, 2024 · 6 comments

Comments

@dontic
Copy link

dontic commented Aug 22, 2024

Not really a bug but an improvement.

Here's my situation:
I have a specific serializer that I only use in a read-only GET endpoint. But I also have implemented a functionality that sends this data through a webhook when generated.
This serializer has another 3 nested serializers which also have read-only fields, the logic is quite complex.

As a simple example lets say I have these 2 serializers:

class CollarSerializer(serializers.Serializer):
    color = serializers.CharField()
    store = serializers.CharField()
    purchase_date = serializers.DateField()
    
    class Meta:
        fields = (
            "color",
            "store",
            "purchase_date",
        )
        read_only_fields = ("purchase_date",)


class DogSerializer(serializers.Serializer):
    name = serializers.CharField()
    age = serializers.IntegerField()
    collar = CollarSerializer()

    class Meta:
        fields = (
            "name",
            "age",
            "collar",
        )
        read_only_fields = ("collar",)

This is the schema I would have in my GET endpoint for a specific dog:

{
  "name": "string",
  "age": 0,
  "collar": {
    "color": "string",
    "store": "string",
    "purchase_date": "2024-08-22"
  }
}

And this is the data that I would also want to send to the webhook.

But the webhook implementation requires me to define the serializer in a "request" or POST "mode":

dog = OpenApiWebhook(
    name="Dog",
    decorator=extend_schema(
        summary="A dog object.",
        description="This webhook is used to send a dog object to a webhook as a POST request.",
        tags=["webhooks"],
        request=DogSerializer,
        responses={
            200: inline_serializer(
                name="Success",
                fields={
                    "status": int,
                    "message": str,
                },
            )
        },
    ),
)

This results in the webhook POST request body definition being:

{
  "name": "string",
  "age": 0
}

I get that I could define a different serializer or schema just for my webhook, in this case it would be simple, the problem is that in my specific use case it would be quite a hassle.
This would also mean code repetition in most cases.

Would there be a way to implement some context to give specifically to webhook endpoints so we can use the "read" or GET definitions of other serializers?

@tfranzel
Copy link
Owner

Hi, think I understood your post but I don't really see any problems yet.

  • Your API has GET /dogs/id which return a DogSerializer.
  • In case of the webhook, the other party will receive a out-of-band DogSerializer as payload and is expected to return a 200.

Not sure yet where exactly the problem is since this does what was asked for, right? Why would you need another DogSerializer? Isn't that one covering both usages as required?

Have you read the docstring for OpenApiWebhook?

Please note that this particular :func:`@extend_schema <.extend_schema>` instance operates
from the perspective of the webhook origin, which means that ``request`` specifies the
outgoing request.

@dontic
Copy link
Author

dontic commented Aug 29, 2024

Hi @tfranzel , I was aware of that documentation and I hope I've been understanding that correctly, because I believe it describes my problem precisely.

So, let's say I have an endpoint where the user can get some data, and also want to provide the user with a webhook that sends exactly that data when the data is created.

What I want to send via the serializer is exactly the same payload the user can get from the GET endpoint, which is the response schema of that serializer. But by having to set it up in the request portion of the webhook schema (which makes total sense, I'm not saying this is wrong), the user gets the request schema of the serializer, which is different from the response.

Let me set a (hopefully) more clear example. Let's say I have this set up:

from rest_framework import serializers
from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiExample
from drf_spectacular.types import OpenApiTypes

class NestedSerializer(serializers.Serializer):
    id = serializers.IntegerField(read_only=True)
    name = serializers.CharField()
    internal_code = serializers.CharField(write_only=True)

class ComplexSerializer(serializers.Serializer):
    title = serializers.CharField()
    description = serializers.CharField(required=False)
    created_at = serializers.DateTimeField(read_only=True)
    nested_data = NestedSerializer(many=True)

class MyAPIView(APIView):
    @extend_schema(
        request=ComplexSerializer,
        responses={200: ComplexSerializer}
    )
    def post(self, request):
        # Logic for POST
        pass

    @extend_schema(
        responses={200: ComplexSerializer}
    )
    def get(self, request):
        # Logic for GET
        pass

# Webhook definition
my_webhook= OpenApiWebhook(
    name="My Webhook",
    decorator=extend_schema(
        description="Webhook for new data",
        request=ComplexSerializer,  # This is where the problem occurs,
         responses={200: None},
    ),
)

These will be the results of the GET endpoint schema vs the webhook schema. As you can see there are differences because the serializer is different from a request perspective than from a response perspective:

{
  "GET_endpoint_response": {
    "title": "Example Title",
    "description": "This is an example description.",
    "created_at": "2024-08-29T10:30:00Z",
    "nested_data": [
      {
        "id": 1,
        "name": "Nested Item 1"
      },
      {
        "id": 2,
        "name": "Nested Item 2"
      }
    ]
  },
  
  "webhook_documentation_in_OpenAPI": {
    "title": "Example Title",
    "description": "This is an example description.",
    "nested_data": [
      {
        "name": "Nested Item 1",
        "internal_code": "CODE1"
      },
      {
        "name": "Nested Item 2",
        "internal_code": "CODE2"
      }
    ]
  },

  "actual_webhook_payload": {
    "title": "Example Title",
    "description": "This is an example description.",
    "created_at": "2024-08-29T10:30:00Z",
    "nested_data": [
      {
        "id": 1,
        "name": "Nested Item 1"
      },
      {
        "id": 2,
        "name": "Nested Item 2"
      }
    ]
  }
}

Here's a specific breakdown of the above:

  1. GET Endpoint Response:

    • Includes all fields except for write-only fields.
    • created_at (a read-only field) is included.
    • In nested_data, id (a read-only field) is included for each item.
    • internal_code (a write-only field) is not included in the nested items.
  2. Webhook Documentation in OpenAPI (as generated by drf-spectacular):

    • Does not include created_at because it's a read-only field.
    • In nested_data, id is missing because it's a read-only field.
    • internal_code is included in the nested items because it's treated as a regular field from the request perspective.
  3. What the user actually receives via the webhook:

    • Should be identical to the GET endpoint response.
    • Includes created_at and nested id fields.
    • Does not include internal_code.

So this is what I mean that, for me to properly document what the user receives in the webhook, I would need create another version of all the serializers, where I modify (or flip) the read only and write only fields just to be able to document this webhook properly.

Expected outcome:

What I want is to keep this request definition, as it makes total sense to define the webhook as a POST request, but that somehow drf-spectacular is aware that it needs to provide the "response" result of the serializer in the request body of the webhook POST definition. Maybe by providing a parameter such as use_serializer_response in the extend schema code.

Hopefully I've made a bit more sense this time. Please also let me know if this makes sense at all or if I'm completely oblivious to something important.

@tfranzel
Copy link
Owner

Thank you @dontic for the thorough explanation. I have not looked at this in a while so I had to refresh my memory.

Just from my memory I would have thought I had the same understanding as you before. However, it turns out that SwaggerUI at least sees this exactly inverted.

Also, if you look at the schema, you can see that we actually don't do that much in the webhook. We simply reuse the component from the view. Only then does SwaggerUI decide to present the Complex component example in "write-mode" for the outgoing request (where we expected "read-mode" instead).
The schema itself does not say anything about the "directionality" of the outgoing webhook request, so this is purely SwaggerUIs interpretation of it. As far as I know, we cannot simply coerce SwaggerUI to "flip" its interpretation. Maybe we just misunderstood how webhooks/callback are to be used in OpenAPI. 😄

I just checked and callbacks nested in operations do present in the same way.
The specification does not really clarify how to interpret it either: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#callback-object

webhooks:
  Foo:
    post:
      description: Foo
        request.
      summary: A complex object.
      tags:
      - webhooks
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Complex'
        required: true
      responses:
        '200':
          content:
            application/json:
              schema:
                type: string
          description: ''

As you pointed out, one might create a "mirror" copy of the serializer (flip read-only and write-only) and use that in the webhook instead of the regular component/serializer. If not realised as a feature, this would mean more boilerplate for you. Absolutely! But again I'm not sure yet if we are trying to shoehorn our misguided interpretation into the schema, to make SwaggerUI bend backwards. At least it feels wrong.

There is also a chance that SwaggerUI just went with the interpretation of the person who implemented the feature. Might be worth searching in their issue list for clues. Pretty sure someone else stumbled over that before.

Let me know what you think.

@dontic
Copy link
Author

dontic commented Aug 30, 2024

@tfranzel you are totally right. I failed to question and check if this is inherited from OpenAPI or SwaggerUI. Thanks for the thorough explanation.

Indeed, it seems that this issue was brought up to the OpenAPI people in:

Where they state they actually had a long thought on this issue when implementing webhooks. In the end, it seems that tools such as SwaggerUI are the ones responsible to interprete how the webhook payload is displayed.

For that reason, I opened an issue for SwaggerUI since I couldn't find anything describing the same exact behavior:

In the meantime the only decent workarounds I found:

  • User Redoc instead of Swagger UI with SpectacularRedocView
  • Create a different schema or serializer just for webhook payloads.

Feel free to close this issue as it's not relevant to drf-spectacular.

@tfranzel
Copy link
Owner

Thanks @dontic for doing the research and opening the issue.

Where they state they actually had a long thought on this issue when implementing webhooks. In the end, it seems that tools such as SwaggerUI are the ones responsible to interprete how the webhook payload is displayed.

Strongly disagree with that. The spec should say how this is to be understood. This is not some literature class where everybody can have a different interpretation. There needs to be a common understanding on what happens.

For normal requests/responses, nobody asks this question because it is so obvious how it is to be read. This is simply not the case for webhooks/callbacks, where people see this differently depending on where their mental model places the origin.

3 UIs - 2 interpretations.

  • SwaggerUI: webhook request as WRITE
  • Redoc: webhook request as READ
  • Stoplight Elements: webhook request as WRITE

I tend to agree with your perspective and Redoc for that matter.

I'll close this as it is an upstream issue on which we have no control here.

@dontic
Copy link
Author

dontic commented Aug 30, 2024

Thanks for expanding on that!

I also do think this should be clarified upstream, in my opinion it's similar to providing a dictionary and not providing the grammar rules on how to put the words together to form a sentence, so then everyone makes their own language in the end.

This motivates me to put a comment in that issue from OpenAPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants