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.

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

Present OpenAPI additionalProperties field #1162

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use the `additionalProperties` field from the OpenAPI schemas to indicate that some JSON objects permit additional arbitrary key-value pairs.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* required
* enum
* anchor: a string suitable for using as an html anchor for this object (if `anchor_base` was set, and the object has a title)
* additionalProperties: either nil, a boolean, or an OpenAPI schema object (https://swagger.io/specification/#schema-object).

Note that the returned array may contain duplicate objects.

Expand Down Expand Up @@ -101,5 +102,5 @@
but with (for example) different examples will be considered different.
*/}}
{{ define "partials/clean-object" }}
{{ return (dict "title" .title "properties" .properties "required" .required "enum" .enum "anchor" .anchor) }}
{{ return (dict "title" .title "properties" .properties "required" .required "enum" .enum "anchor" .anchor "additionalProperties" .additionalProperties) }}
{{ end }}
31 changes: 31 additions & 0 deletions layouts/partials/openapi/render-object-table.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,21 @@
In some cases (such as response body specifications) this isn't used, and
instead properties have a `required` boolean attribute. We support this too.

* `additionalProperties`: one of

- `false` (no additional properties are permitted);
- `true` (additional properties pairs are permitted, with no constraints);
- an OpenAPI schema object [1] (additional properties are permitted, but
with constraints on `property_data`); or
- `nil` (not specified in schema, use `true` as a default value).

[1]: https://swagger.io/specification/#schema-object
*/}}

{{ $title := .title }}
{{ $properties := .properties}}
{{ $required := .required}}
{{ $additionalProperties := .additionalProperties }}
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

{{ if $properties }}

Expand Down Expand Up @@ -97,6 +107,27 @@

{{ end }}

{{/* If additionalProperties is true, do nothing.
If additionalProperties is nil, treat is as the default (true) and do nothing.
If additionalProperties is false, raise a build-time error:
"Everything is extensible in Matrix unless otherwise noted".
Otherwise, it should be an OpenAPI schema object. Include the description
of that schema object, if it exists. */}}
{{ if (and (ne $additionalProperties nil) (ne $additionalProperties true)) }}
{{ if (and (reflect.IsMap $additionalProperties)) -}}
Copy link
Member

Choose a reason for hiding this comment

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

redundant and here.

Comment on lines +116 to +117
Copy link
Member

Choose a reason for hiding this comment

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

can we restructure these if statements to reduce the nesting?

 {{ if (or (eq $additionalProperties nil) (eq $additionalProperties true)) }}
     {{/* do nothing */}}
  {{ else if (reflect.IsMap $additionalProperties) -}}
     ...
  {{ else if (not $additionalProperties) -}}
    ... 
  {{ end }}

or something.

{{ if (index $additionalProperties "description") -}}
<tr class="row-additional-properties">
<td colspan="3">
richvdh marked this conversation as resolved.
Show resolved Hide resolved
<em>Notes on additional properties:</em> {{ $additionalProperties.description }}
</td>
</tr>
{{ end }}
{{/* TODO: should we handle the case where additional properties are
permitted, but must follow an explicit schema? */}}
{{ else if (not $additionalProperties) -}}
{{ errorf "Unexpected additionalProperties=false for %s" $title }}
Copy link
Member

Choose a reason for hiding this comment

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

it's unclear to me that we should be raising an error in this case. Isn't it legitimate to specify that a given object may not have additional properties, even if that is currently unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷 There's some context here: #1162 (comment) ; I don't have any strong opinions here.

{{ end }}
{{ end }}
</table>

{{ end }}