search

litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.37k stars 363 forks source link

Bug: order of types in openapi spec is not consistent in json rendering #3646

Open atom-andrew opened 1 month ago

atom-andrew commented 1 month ago

Description

We are seeing the order of types change in openapi generation, which makes comparing golden versions of the openapi spec problematic. I think the specific problem we are seeing comes from https://github.com/litestar-org/litestar/blob/ffaf5616b19f6f0f4128209c8b49dbcb41568aa2/litestar/_openapi/schema_generation/schema.py#L160 where we use the set operation to uniquify the list of types. The order doesn't matter to the correctness of the openapi spec, so perhaps the responsibility for ensuring a determistic spec file could also come from the serializer, but either way it would be helpful if we could always render the same openapi spec the same way.

URL to code causing the issue

No response

MCVE

# Your MCVE code here

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

No response

Logs

No response

Litestar Version

2.9.1

Platform

[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

provinzkraut commented 1 month ago

Having this be consistent makes sense to me. There are 2 ways to fix this: Either sort the items after they have been deduplicated, or remove the usage of a set entirely. Sorting is easier, but not using a set to begin with gives us the opportunity to preserve order, which, while not necessary, could be a bit nicer to work with.

@atom-andrew Since you've already figured out what the issue is, would you be interested in submitting a PR with a fix? :)

atom-andrew commented 1 month ago

@provinzkraut I'm happy to submit a PR but it wasn't obvious to me where this would be tested. Can you give me a tip about where it would make sense to include a test for this? Thanks.