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

Adding a default additionalProperties: false #2453

Closed
dblock opened this issue Jun 14, 2024 · 6 comments
Closed

Adding a default additionalProperties: false #2453

dblock opened this issue Jun 14, 2024 · 6 comments

Comments

@dblock
Copy link

dblock commented Jun 14, 2024

What version of Ajv you are you using?

8.13.0

What problem do you want to solve?

Coming from opensearch-project/opensearch-api-specification#338.

We are authoring an OpenAPI specification for OpenSearch in https://github.com/opensearch-project/opensearch-api-specification. For example, the GET / API returns server info. To verify that the specification is correct we author test stories. For example this story makes a GET / request to a docker instance of OpenSearch, and we use AJV to verify that the response matches the schema. We basically verify the schema from actual server response data, instead of the other way around.

A large category of bugs in the schema is that some fields are missing. For example, the distribution filed in the response from GET / was added in opensearch-project/opensearch-api-specification#336.

We'd like to have detected the missing field. Had the info schema had additionalProperties: false the schema validation would have failed with "data/version must NOT have additional properties". However adding that option means that our published schema cannot be future-proof for when new fields are added.

So far I am thinking of dynamically injecting additionalProperties: false but it feels messy.

What do you think is the correct solution to problem?

A way to generically implement the behavior of producing a validation error with any field that is returned in the data but is not referenced in the schema.

validate({ additionalProperties: false })

Some other approach?

Will you be able to implement it?

No promises, but sincerely appreciate everybody's work on this project! Million thanks.

@jasoniangreen
Copy link
Collaborator

To my knowledge the behaviour of things like that is decided by the JSON Schema spec and not something we would be open to changing, however I know it's possible to do some pretty amazing things with custom keywords. I can't be sure without delving much deeper into your use case, but maybe they can help you build some mechanism to achieve your goals.

See the docs on keywords - https://ajv.js.org/keywords.html

@dblock
Copy link
Author

dblock commented Jun 15, 2024

Thanks @jasoniangreen, I will look into this.

Possibly simpler, is there a way with AJV to extract the fields that are not referenced in the schema?

@jasoniangreen
Copy link
Collaborator

Thanks @jasoniangreen, I will look into this.

Possibly simpler, is there a way with AJV to extract the fields that are not referenced in the schema?

Not that I could see, but from what I remember about addKeyword, you will get access to all data and the schema so it should be possible to work it out and store the extra fields somewhere.

@jasoniangreen
Copy link
Collaborator

One more thought, I don't see what's messy about dynamically injecting additionalProperties: false. Sounds perfectly legit to have a wrapper functioning that can recursively add that prop and then you can maintain one schema for true validation and another for highlighting extra fields. It wouldn't be a lot of code at all.

Going to close as I think there is enough for OP to work with and I know for a fact that we won't want to change fundamental default behaviour or add new options for this.

@dblock
Copy link
Author

dblock commented Jun 17, 2024

@jasoniangreen Thank you so much!

@dblock
Copy link
Author

dblock commented Jun 17, 2024

Here's an implementation that adds ajv-errors and dynamically injects the following by walking the spec.

"additionalProperties": {
  "not": true,
  "errorMessage": "property is not defined in the spec"
}

opensearch-project/opensearch-api-specification#342

Feel free to leave comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants