Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support reference objects in operation parameters #1749

Closed
wants to merge 5 commits into from
Closed

Support reference objects in operation parameters #1749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
477023b
Add a new `partial` for resolving object refs, properly
richvdh Mar 13, 2024
cc978c5
Resolve reference objects in parameters
richvdh Mar 13, 2024
44db164
changelog
richvdh Mar 13, 2024
62393f8
Clean up URI resolution
richvdh Mar 13, 2024
63638fb
Remove redundant $root_schema
richvdh Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/internal/newsfragments/1749.clarification
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templates: add support for reference objects in parameters.
8 changes: 8 additions & 0 deletions layouts/partials/json-schema/resolve-refs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
rather than a normal `dict` because with `dict` you can't replace key values:
https://discourse.gohugo.io/t/how-can-i-add-set-and-delete-keys-in-a-dictionary/29661

XXX: This partial should be phased out, and replaced with
`openapi/resolve-ref-object`. OpenAPI 3 doesn't really allow arbitrary JSON
references. As [swagger.io] says: "A common misconception is that `$ref` is
allowed anywhere in an OpenAPI specification file. Actually `$ref` is only
allowed in places where the OpenAPI 3.0 Specification explicitly states that
the value may be a reference."

[swagger.io]: https://swagger.io/docs/specification/using-ref/#allowed-places
*/}}

{{ $schema := .schema }}
Expand Down
30 changes: 17 additions & 13 deletions layouts/partials/openapi/render-parameters.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
Render the parameters of a given type, given:

* `parameters`: OpenAPI data specifying the parameters
* `type`: the type of parameters to render: "header, ""path", "query"
* `type`: the type of parameters to render: "header", "path", "query"
* `caption`: caption to use for the table
* `path`: the path where this definition was found, to enable us to resolve "$ref"

This template renders a single table containing parameters of the given type.

Expand All @@ -13,14 +14,18 @@
{{ $parameters := .parameters }}
{{ $type := .type }}
{{ $caption := .caption }}

{{ $parameters_of_type := where $parameters "in" $type }}

{{ with $parameters_of_type }}

{{/* build a dict mapping from name->parameter, which render-object-table expects */}}
{{ $param_dict := dict }}
{{ range $parameter := . }}
{{ $path := .path }}

{{/* build a dict mapping from name->parameter, which render-object-table expects */}}
{{ $param_dict := dict }}

{{ range $parameter := $parameters }}
{{/*
Per https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#operation-object:
the parameters can be reference objects; resolve them now.
*/}}
{{ $parameter = partial "openapi/resolve-ref-object" (dict "schema" $parameter "path" $path) }}
{{ if (eq $parameter.in $type) }}
{{/*
merge the schema at the same level as the rest of the other fields because that is
what `render-object-table` expects. Put the schema first so examples in it are
Expand All @@ -29,8 +34,7 @@
{{ $param := merge $parameter.schema $parameter }}
{{ $param_dict = merge $param_dict (dict $parameter.name $param )}}
{{ end }}

{{/* and render the parameters */}}
{{ partial "openapi/render-object-table" (dict "title" $caption "properties" $param_dict) }}

{{ end }}

{{/* and render the parameters */}}
{{ partial "openapi/render-object-table" (dict "title" $caption "properties" $param_dict) }}
24 changes: 19 additions & 5 deletions layouts/partials/openapi/render-request.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,24 @@ <h2>Request</h2>
{{ if $parameters }}
<h3>Request parameters</h3>

{{ partial "openapi/render-parameters" (dict "parameters" $parameters "type" "header" "caption" "header parameters") }}
{{ partial "openapi/render-parameters" (dict "parameters" $parameters "type" "path" "caption" "path parameters") }}
{{ partial "openapi/render-parameters" (dict "parameters" $parameters "type" "query" "caption" "query parameters") }}

{{ partial "openapi/render-parameters" (dict
"parameters" $parameters
"type" "header"
"caption" "header parameters"
"path" $path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

) }}
{{ partial "openapi/render-parameters" (dict
"parameters" $parameters
"type" "path"
"caption" "path parameters"
"path" $path
) }}
{{ partial "openapi/render-parameters" (dict
"parameters" $parameters
"type" "query"
"caption" "query parameters"
"path" $path
) }}
{{ end }}

{{ if $request_body }}
Expand All @@ -53,7 +67,7 @@ <h3>Request body</h3>
{{/*
Show the content types and description.
*/}}
{{ $mimes := slice }}
{{ $mimes := slice }}
{{ range $mime, $body := $request_body.content }}
{{ $mimes = $mimes | append $mime }}
{{ end }}
Expand Down
69 changes: 69 additions & 0 deletions layouts/partials/openapi/resolve-ref-object.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
{{/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to update scripts/check-openapi-sources.py to support this, as it only resolves refs inside JSON schemas.


Handles OpenAPI "Reference Objects".

Reference objects are JSON objects with a single property, `$ref` (and
Copy link
Contributor

@zecakeh zecakeh Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, maybe we can clarify that comment.

optionally a `summary` and `description`). This partial resolves the reference
and returns the expanded object.

The input parameter is a dict with the following keys:

* `schema`: A schema object to check for $ref properties.

* `path`: The path of the schema file containing the (potential) ref; used for resolving
relative references.

Ref: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#reference-object.

*/}}

{{ $schema := .schema }}
{{ $path := .path }}

{{ $ret := $schema }}

{{ $ref_value := index $schema "$ref"}}
{{ if $ref_value }}
{{/* Resolve the ref URI relative to the path of the schema file */}}
{{ $base_uri := urls.Parse $path }}
{{ $ref_uri := urls.Parse $ref_value }}
{{ $full_uri := $base_uri.ResolveReference $ref_uri }}

{{/* strip the extension, and the leading `/`, from the path */}}
{{ $full_path := strings.TrimPrefix "/" (replaceRE "\\.[^\\.]*$" "" $full_uri.Path) }}

{{ $pieces := split $full_path "/" }}
{{ $ret = index site.Data $pieces }}

{{ if $ref_uri.Fragment }}
{{/*
Per https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#relative-references-in-uris:

> If a URI contains a fragment identifier, then the fragment should
> be resolved per the fragment resolution mechanism of the
> referenced document. If the representation of the referenced
> document is JSON or YAML, then the fragment identifier SHOULD be
> interpreted as a JSON-Pointer as per RFC6901.

RFC6901, in a nutshell, says the pointer is a series of keys
separated by `/`. We strip off the leading `/` and use the
subsequent keys as indexes.
*/}}

{{ $keys := split (strings.TrimPrefix "/" $ref_uri.Fragment ) "/" }}
{{ $ret = index $ret $keys }}
{{ end }}

{{/*
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 }}
Comment on lines +57 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{{ end }}

{{ return $ret }}
Loading