matrix-org / matrix-spec

The Matrix protocol specification
Apache License 2.0
171 stars 91 forks source link

Support reference objects in operation parameters #1749

Closed richvdh closed 3 months ago

richvdh commented 3 months ago

According to https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#operation-object, the entries in parameters lists can be reference objects. We don't currently support that in our templates, so let's fix that.

Preview: https://pr1749--matrix-spec-previews.netlify.app

zecakeh commented 3 months ago

For the refactoring I made in #1751, we will also need to support reference objects in the Responses Object, and the Media Type Object for examples.

I don't know if we should add support for those here or in another PR.

richvdh commented 3 months ago

Having discussed this again with @zecakeh:

As things currently stand, we implement $ref by calling resolve-refs, which recursively expands $ref without concern about where the $ref actually is. The problem with that is then it's unclear whose job it is to call resolve-refs. I was hoping to resolve that by moving towards a model where we only expand $ref in places where we know it is valid. But I'm now wondering if that is actually worthwhile.

Instead, maybe we should just call resolve-refs in the top-level template (layouts/shortcodes/http-api.html, etc), and rely on the validator to actually enforce the rules. This is also what happens in scripts/dump-openapi.py.

It's also worth noting that we currently use $ref in a place where OpenAPI doesn't permit it (in the bodies of examples), and, assuming we want to keep that power (I think we do) we probably need the recursive resolve-refs for that.

So, I'm going to abandon this approach, in favour of @zecakeh's solution over at #1751.