Enhancement: Generate OpenAPI schema always in the same order about litestar HOT 16 CLOSED

This seeding is already being done though it is not exposed as a configurable value as seen here. I think it's a good idea to make this configurable as well.

If the examples being generated are still different, then it may be that the schemas are not being created in the same order each time which seems to be the case as @tuukkamustonen pointed out regarding the components.schemas. So, ensuring the schemas are generated in the same order for a given Litestar app should fix this as well.

from litestar.

@ccrvlh I'd say that's a different feature to this one so it'd be great if you could create an issue for that. I think it does follow a certain order which is GET, PUT, POST, DELETE etc. (I'm guessing the same order as seen here). However, this considers /:id/nested to have no relation to /:id or any of the other routes. The logic of this starts here.

The difficulty here is that by the time the route reaches to OpenAPI logic, we don't know whether this was defined as part of a controller or not. So, if that order is to be maintained then we'd need to figure out a way to tell let the OpenAPIPlugin figure out the order the paths should be in. This would have to be done at the time we parse the Controller to create the individual route handlers. Even then a single path would consist of all the GET, PUT, POST etc. in that path but it should be possible to define that they should come in the following order from your example: /, /:resource_id, /:resource_id/nested.

from litestar.

@guacs Thanks for looking into it. Maybe it's something more exotic, or maybe I'm just wrong. I'll investigate and ping back (a bit later).

from litestar.

The OpenAPI JSON spec is supposed for machines and not humans, so whatever order is fine as long as it's consistent 🤔

Alphabetical ordering would make sense, as it would keep the spec unchanged when you refactor and move controllers/handlers around... on the other hand, if you instead refactor by renaming models in-place, then the spec would change 😄

I guess I would just go with what is simplest and fastest, though I don't see exact issues with alphabetical ordering, either 🤷🏻‍♂️

from litestar.

@guacs

from litestar.

@guacs Were you planning to work on this?

I started wondering how the order of the elements could be random as since Python 3.7+ dicts retain insertion order and generating the schema should be deterministic?

But yeah, it does seem to generate some elements (in my long spec) in a bit different order each time.

from litestar.

@tuukkamustonen if you'd like to work on this, then feel free to do so.

I haven't looked into it yet, so I'm not sure just where the randomness is coming from.

from litestar.

I'm not sure whether this is what this issue is about, since it seems to be related to deterministic ordering (rather than defined ordering through the Controller), but it might be related, so I'll avoid opening a new one.

From a DX perspective I personally find very helpful to have ordered routes in the Controller, that follow a certain logic:

• Progressive nesting (GET / comes before GET /:id and before GET /:id/nested)
• Logical actions ordering (GET, POST, GET id, PATCH id, DELETE id)

Example:

class MyController(Controller):
    tags = ["..."]
    path = "/"
    dependencies = {...}

    @routes.get()
    async def get_many(self):
        ...

    @routes.post()
    async def create(self, data: ...):
        ...

    @routes.get("/{resource_id:uuid}")
    async def get(self, resource_id: UUID):
        ...

    @routes.patch("/{resource_id:uuid}")
    async def update(self, resource_id: UUID):
        ...

    @routes.delete("/{resource_id:uuid}")
    async def delete(self, resource_id: UUID):
        ...

    @routes.get("/{resource_id:uuid}/nested")
    async def get_nested(self, resource_id: UUID):
        ...

Currently the ordering of the route definition at the Controller is not respected by the docs, so I end up having a Swagger that looks like this:

- GET /:id/nested/:nest_id/another
- POST /:id/nested
- GET /
- DELETE /:id
- POST /

Which I personally find very confusing since:
(1) It doesn't seem to follow a pre-defined logic (couldn't find any pattern for that when looking at the docs)
(2) It doesn't respect the logic that was defined on the controller

Not sure what might be causing it, I think this is generated on _openapi/schema_generation/schema.py:SchemaCreator maybe? Will be happy to help with some guidance, since it seems this has been refactored not long ago here: #2805.

from litestar.

@guacs thank you, opening a new issue here

from litestar.

@tuukkamustonen I'm not able to reproduce the issue actually. I tried with the following and the schema generated is always the same:

from __future__ import annotations

from dataclasses import dataclass

from rich import print

from litestar import get, post
from litestar.openapi.config import OpenAPIConfig
from litestar.testing import create_test_client


@dataclass
class Foo:
    foo: int


@dataclass
class Bar:
    bar: str


@get("/foo")
async def get_foo() -> Foo:
    return Foo(9)


@post("/foo")
async def post_foo(foo: Foo) -> None:
    ...


@get("/bar")
async def get_bar() -> Bar:
    return Bar("bar")


with create_test_client([get_foo, get_bar, post_foo], openapi_config=OpenAPIConfig("foo", "voo", True)) as client:
    resp = client.get("/schema/openapi.json")

    print(resp.json())

from litestar.

Okay, I'm able to reproduce it in a really weird setup.

The random ordering occurs in components section of OpenAPI spec. Elements elsewhere (e.g. paths) seem to remain in same order.

It's related to use of Controllers, where the order of HTTP methods gets randomized by a use of set() in https://github.com/litestar-org/litestar/blob/main/litestar/utils/sequence.py#L22

Sets are not ordered, so that unique should return the handlers in whatever order, which leads to model registration in whatever order.

To reproduce, I had to separate the handlers into a separate file.

# handlers.py
from dataclasses import dataclass

from litestar import get, post, Controller
from pydantic import BaseModel


@dataclass
class Payload:
    foo: int


@dataclass
class Response(BaseModel):
    bar: str


class FooController(Controller):
    @post()
    async def post(self, data: Payload) -> None: ...

    @get()
    async def get(self) -> Response: ...


# app.py
import json

from litestar import Litestar

from handlers import FooController

app = Litestar(route_handlers=[FooController])


print(json.dumps(app.openapi_schema.to_schema()["components"], indent=4))

(If I dump it all into a single file, the issue will not reproduce.)

When I run that once, the first time, it outputs the schema components in Response, Payload order. Run it again, and the order is reverses. Run it again, and the order remains same as second time.

If you flush the .pyc cache (in __pycache__) and re-run it, the order is Response, Payload again. And then following invocations, with the cache have it reversed again.

I don't understand how exactly that bytecode caching impacts this, but it does, at least on my computer 🤯 Maybe the .pyc file is ordered somehow, and re-loading it up from there somehow leads to the set() always giving same ordering in this case. As only when running without cache the order is reversed. I don't know.

In any case, I believe that set messes it up. And there are other sets around, which might have an impact, too.

Does that sound plausible?

from litestar.

@tuukkamustonen thanks! I was able to reproduce it and I believe you're correct in that the use of unique is what's causing the difference in the order of the components and that this only happens when you use a Controller with multiple HTTP methods.

The use of unique to find the unique HTTP handlers results in route.route_handlers possibly having a different order of handlers each time which results in a different insertion order to route.route_handler_map. For creating the paths portion of the OpenaAPI schema, we iterate over the route_handler_map which in turn causes a difference in the order of insertion of the generated schema into the SchemaRegistry.

I was thinking that we force the schemas to be generated in alphabetical order so that we should be able to get the same OpenAPI schema every time regardless of the order the paths (and their individual HTTP methods) are processed. WDYT?

from litestar.

on the other hand, if you instead refactor by renaming models in-place, then the spec would change

This would happen regardless no? Since the names of the schemas are created based on the name of the model.

from litestar.

True, the order of the entries wouldn't change... but their names would (still less than the diff generated by changing the order).

from litestar.

@guacs I see your PR is merged, shall we close this issue?

from litestar.

Closed in #3172

from litestar.