Support reference objects in operation parameters #1749
Conversation
This requires us to pass down $root_schema and $path.
richvdh
force-pushed
the
rav/ref_objects_in_params
branch
from
04eda35 to
44db164
Compare
|
|
||
| Handles OpenAPI "Reference Objects". | ||
|
|
||
| Reference objects are JSON objects with a single property, `$ref` (and |
There was a problem hiding this comment.
It's actually a bit more complicated than that… Depending on the property it is used in, the behavior of conflicts could be undefined.
This table summarizes things.
There was a problem hiding this comment.
Right. I guess we are specifically targetting "reference objects" here, so it's correct that summary and description are preserved?
I guess we'd need to make this logic more generic if we wanted to use it in other places, but that could be a future change.
There was a problem hiding this comment.
Sure, what made me react here is more the comment in the resolve-refs partial that says it should be replaced by this one, but in its current state it's not possible.
There was a problem hiding this comment.
ah ok, maybe we can clarify that comment.
| {{/* | ||
| OpenAPI spec says that "summary" and "description" from the reference object override | ||
| the values from the referenced component. | ||
| */}} | ||
| {{ if isset $schema "summary" }} | ||
| {{ $ret = merge $ret (dict "summary" $schema.summary) }} | ||
| {{ end }} | ||
| {{ if isset $schema "description" }} | ||
| {{ $ret = merge $ret (dict "description" $schema.summary) }} | ||
| {{ end }} |
There was a problem hiding this comment.
If we want to be more strict with how we resolve refs, should we throw an error or log a warning if $schema and $ret have properties of the same name, other than those? That would allow to know sooner what is wrong if someone expects the ref to be overwritten by the schema, which we often do, but requires the use of allOf.
| "parameters" $parameters | ||
| "type" "header" | ||
| "caption" "header parameters" | ||
| "path" $path |
There was a problem hiding this comment.
path is expected to be the path to the file, but the one received by this partial is the path to the folder containing the file. So we need to have the filename in the path here somehow, without interfering with other partials that use resolve-refs and still expect the folder.
| @@ -0,0 +1,69 @@ | |||
| {{/* | |||
There was a problem hiding this comment.
We also need to update scripts/check-openapi-sources.py to support this, as it only resolves refs inside JSON schemas.
|
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 I don't know if we should add support for those here or in another PR. |
|
Having discussed this again with @zecakeh: As things currently stand, we implement $ref by calling Instead, maybe we should just call resolve-refs in the top-level template ( It's also worth noting that we currently use So, I'm going to abandon this approach, in favour of @zecakeh's solution over at #1751. |
According to https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#operation-object, the entries in
parameterslists 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