Set additionalProperties when [JsonExtensionData] is present #56767
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
captainsafia
added
area-mvc
3 tasks
captainsafia
force-pushed
the
safia/addl-props-extension-data
branch
from
0c19713 to
6227ab4
Compare
| // match the OpenAPI v3 interpretation here by checking to see if any properties on a type map to | ||
| // extension data and explicitly setting the `additionalProperties` keyword to an empty object. | ||
| // Since `[ExtensionData]` can only be applied on dictionaries with `object` or `JsonElement` values | ||
| // there is no need to encode a concrete schema for thm and the catch-all empty schema is sufficient. |
There was a problem hiding this comment.
Suggested change
| // there is no need to encode a concrete schema for thm and the catch-all empty schema is sufficient. | |
| // there is no need to encode a concrete schema for them and the catch-all empty schema is sufficient. |
| public string Name => name; | ||
|
|
||
| [JsonExtensionData] | ||
| public IDictionary<string, object> ExtensionData { get; set; } |
There was a problem hiding this comment.
Is it worth having a test for the JsonElement case also mentioned in the description? Possibly also a class that derives from something with the base having the extension property?
I don't think this is true. In the Schema Object section of the OpenAPI 3.0.3 spec it says:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reacting to feedback in #56318 (comment) and discussion in #56707.
The
additionalPropertieskeyword in a schema is used to indicate that a schema can match against properties explicitly defined in theschema.propertieslist and additional unspecified properties.The JSON Schema specification that STJ's JsonSchemaExporter targets assumes that
additionalPropertiesistrueby default for all schemas, meaning that a schema for a type will always be able to capture properties that are not explicitly defined in the properties list (see here).The OpenAPI specification (as of v3 that we target) assumes the opposite.
additionalPropertiesis assumed false and must be explicitly enabled in order to indicate that a schema can capture properties not explicitly defined. Furthermore, OpenAPI doesn't support a valuetruefor the schema to support catch-alls. It requires that you implement an empty object to indicate that additional property values can match any type.These two problems come together like Voltron in our handling of
[JsonExtensionData]attributes on property.JsonSchemaExporterassumes that additionalProperties is implicitly true and doesn't set it when it generates a schema. We have to patch this mismatch in ourTransformSchemaNodeimplementation to comply with what OpenAPI expects.Since
[JsonExtensionData]can only ever be applied onDictionary<string, object>andDictionary<string, JsonElement>, we don't have to worry about specifying concrete schemas for the values of additional properties. The empty object will do.