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

fix: Do not wrap an example list value in another list #1250

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

psamuels00
Copy link

The purpose of this PR is to allow a list of values (more than one) to be presented as the value of an Open API Example. In all the examples I found online, only a single value is supplied, which value is automatically turned into a list. Also, there are no unit tests for handling a list of values.

Background

In 0.22.0, a change was made, reflected by the following entry in the release notes:

Examples are now wrapped in pagination/lists when endpoint/serializer is many=True

Based on the following decorator...

@extend_schema(
    examples = [
        OpenApiExample(
            "name",
            value={"pet": "dog"}
        ),
    ]
)

...the Swagger example renders like this:

[
  {
    "pet": "dog"
  }
]

So far, so good.

The Problem

If a list is explicitly supplied for the example value, it is still wrapped in a list. Given the following...

@extend_schema(
    examples = [
        OpenApiExample(
            "name",
            value=[
                {"pet": "dog"}
                {"pet": "cat"}
            ],
        ),
    ]
)

...the Swagger example renders like this:

[
  [
    {
      "pet": "dog"
    },
    {
      "pet": "cat"
    }
  ]
]

The Expectation

I would expect the rendered example to not be nested in a list, for example:

[
  {
    "pet": "dog"
  },
  {
    "pet": "cat"
  }
]

The Solution

With the proposed changes, if a list is supplied as the value of an example, it is not wrapped in a new list.

@psamuels00 psamuels00 changed the title Do not wrap an example list value in another list fix: Do not wrap an example list value in another list May 31, 2024
@tfranzel
Copy link
Owner

tfranzel commented Sep 8, 2024

Hi @psamuels00, and first of all sorry for the super late reply. Haven't had much spare time lately.

I see your point there, but I am unsure whether we actually want that change. I think it makes definition of what we see as an example more ambiguous.

Examples are now wrapped in pagination/lists when endpoint/serializer is many=True

This meant that an example is usually ONE entity dict that is returned (e.g {"pet": "dog"}) for a retrieve (many=False) request. In case the serializer is in many=True mode, it is now either a list of those entities or the response is wrapped in the pagination structure.
Just showing {"pet": "dog"} as example in those instances turned out to be weird as it did not really represent what the endpoint would return and so we changed it by augmenting the example when necessary.

The thing is that you implicitly assume that nobody would want to have a list of entitites on retrieve and a list of list of entitites for list (many=True), which sounds reasonable but people do the craziest things. This would now potentially break some people.

Another issue: What does happen if you would give that example[{"pet": "dog"}{"pet": "cat"}] to a retrieve operation? This could indeed happen because we allow examples to be attached to the view class and all contained endpoints then use the example according to many=True/False, i.e. converted into a list or left untouched.

tldr: For drf-spectacular, examples were meant to be ONE atomic example. Sure, OpenAPI could be written differently, but in the context of DRF's many=True list and pagination mechanics it makes things a lot more complicated with only marginal benefit.

Let me know what you think, but I am still quiet confident this would create more problems than it solves.

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

Successfully merging this pull request may close these issues.

2 participants