Enhancement: Have OpenAPI Schema generation respect route handler ordering

[ccrvlh]

Summary

Opening a new issue as discussed here.

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
- PATCH /

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) it does seem to follow the ordering of the methods as defined here as shared by @guacs here
(2) It doesn't respect the logic that was defined on the controller (specifically the nesting).

Was having a quick look and it seems to be related to logic when registering the route here, and then the handler:method map on the HTTPRoute here.

It seems that by the time it reaches the plugin the nesting is already out of order - so it might make sense to handle when registering the routes maybe?

Anyways, if this is something of interest I'd help to contribute and take a deeper look into it.

---

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.

[guacs]

Btw, @ccrvlh, I was just looking into a bit on the swagger side of things and it seems they allow grouping based on tags. If this allows you to customize the order that's displayed then imo that would be better than us trying to create them in the order since I assume that would add quite a bit of complexity.

NOTE: I haven't really tested it out with the tags so I'm not sure if that will allow for the structure that you'd like.

[ccrvlh]

Yeah I usually use the tags to group "modules", my challenge is within the module.

I'll take a look to see if I can find something - my impression is that instead of iterating the controller's methods, the iteration is done in difference places based on path, HTTP method (both through the methods Enum and the decorators), and this is what makes the logic tricky.

I think it might related to this logic as well:

litestar/litestar/controller.py

Lines 203 to 210 in 3e5c179

	controller_names = set(dir(Controller))
	self_handlers = [
	getattr(self, name)
	for name in dir(self)
	if name not in controller_names and isinstance(getattr(self, name), BaseRouteHandler)
	]
	self_handlers.sort(key=attrgetter("handler_id"))
	for self_handler in self_handlers:

And if that's the case than it might not add too much complexity. I'll get back to you on this so we can at least have a better idea on the trade-offs.

[guacs]

And if that's the case than it might not add too much complexity. I'll get back to you on this so we can at least have a better idea on the trade-offs.

Awesome!

[ccrvlh]

Ran a couple of tests and could make it work in a sort of hacky way:

    # litestar/controller.py:Controller
    def get_route_handlers(self) -> list[BaseRouteHandler]:
        """Get a controller's route handlers and set the controller as the handlers' owner.

        Returns:
            A list containing a copy of the route handlers defined on the controller
        """
        route_handlers: list[BaseRouteHandler] = []
        controller_names = set(dir(Controller))
        self_handlers = []
        for name in dir(self):
            if name not in controller_names and isinstance(getattr(self, name), BaseRouteHandler):
                handler = getattr(self, name)
                if getattr(handler, '_fn', None):
                    handler_line = inspect.getsourcelines(handler._fn)[1]
                    self_handlers.append((handler_line, handler))

        sorted_list = sorted(self_handlers, key=lambda x: x[0])
        for _, self_handler in sorted_list:
            route_handler = deepcopy(self_handler)

The benefit is that it's a very simple change, and doesn't alter the logic on what is being iterated (path vs method vs handler) and when (controller, app, plugin, etc). The downside is that is somewhat hacky, as it depends on this getsourcelines (we'll need a fix to handle potential index errors on this handler_line). I'll spend a bit more time to see if I can find some other way.

This + putting patch before put and delete is exactly what I was looking for.

[guacs]

@ccrvlh does that result in the correct output in Swagger?

[ccrvlh]

Tested on a project with a couple of hundred routes, so can't tell for sure, and haven't done any proper testing (was literally just changing the installed litestar package from within my project), but it does seem to work.

I'm not sure whether dir respects the orders of the methods (i think __dict__ does so it also might be something to test), but when debugging I found that this self_handlers list is what is out of order, so fixing changing this ordering fixed changed the ordering on the Swagger schema.

[guacs]

@ccrvlh while I see the benefit of this feature, I'd like to wait just a bit to see if there are others who would like this as well before adding support for this.

Also, ideally, I think this ordering logic should happen within the OpenAPI plugin rather in the Controller to ensure that we don't end up coupling them together.

[ccrvlh]

Sure. I think It might be possible to work on the ordering through the included_routes of the plugin. My idea was to do this as upstream as possible, but it also makes sense for the plugin to responsible for it - I imagine there aren't other parts of the DX impacted by this. I'll make a couple of tests with a different approach, and we check whether this is something that makes sense for other users.