Skip to content

unevaluatedProperties and unevaluatedItems keywords are not implemented #276

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

Closed
ssilverman opened this issue Mar 21, 2020 · 18 comments
Closed

Comments

@ssilverman
Copy link

According to my testing, it doesn't appear that unevaluatedProperties from 2019-09 is implemented. If I'm correct, are there plans to add support?

@stevehu
Copy link
Contributor

stevehu commented Mar 21, 2020

@ssilverman You are correct that it is not implemented yet. We implemented most of the validators but there are still some pending. I am wondering if you could submit a PR for this validator. Thanks.

@ssilverman
Copy link
Author

Ah, I hadn't realized the 2019-09 support wasn't complete yet, that's why I filed this issue. I can't promise I have time at the moment, but I'm going to have a look at the code. Any implementation guidance for newcomers? Specifically, unevaluatedProperties needs to be evaluated after a few other properties are evaluated and collect info from them.

@stevehu
Copy link
Contributor

stevehu commented Mar 22, 2020

@ssilverman The library is a flexible plugin structure. Here are some of the points that I can think of.

  • Create a new validator
  • Register the validator to the ValidatorTypeCode
  • There are two ways to carry the context information across multiple validators. ValidationContext or ThreadLocal. There are examples in the library about how these two options work.

Let me know when you encounter any specific questions. Thanks.

@ssilverman
Copy link
Author

ssilverman commented Mar 22, 2020

I started here: ssilverman@35a8889

Does this look correct so far? What is versionCode in ValidatorTypeCode for? I used 15 because that's what ADDITIONAL_PROPERTIES did.

I think I'll need some guidance when adding "peering into" the in-place applicators (allOf, anyOf, oneOf, and not; and also $ref (may or may not point to an applicator, but may point to a properties-related value)) because they'll need some recursion, terminating at schemas.

@stevehu
Copy link
Contributor

stevehu commented Mar 22, 2020

You are on the right path. Here is the document talking about the version.

https://github.com/networknt/json-schema-validator/blob/master/doc/specversion.md

@ssilverman
Copy link
Author

I added the in-place applicator collection in the UnevaluatedProperties constructor, but I'm not 100% clear on how to go about adding $ref and in-place applicator results to validate.

See: https://github.com/ssilverman/json-schema-validator/commits/add-unevaluated-props

@stevehu
Copy link
Contributor

stevehu commented Mar 24, 2020

@ssilverman To be honest, I am far behind as I am still learning the spec. I am wondering if this is already published or not. The only document I can find is this issue. Let me know if you have any official links for us to learn.

I looked at your code and it looks pretty complicated. I am wondering if you could create a document in the /doc folder to write down your implementation ideas. In this way, it might be easier for other developers to understand what you are trying to archive and help you if needed. Thanks.

@ssilverman
Copy link
Author

ssilverman commented Mar 24, 2020

It's published in the 2019-09 draft (https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.3.2.4).

As for the complexity, I'm just following the existing code style for the other validators. "unevaluatedProperties" is similar to "additionalProperties", but it also has to look at any sibling "additionalProperties", "oneOf", "allOf", "anyOf", "not", and "ref" contents. How could I make it simpler? I don't yet know your code base that well.

@stevehu
Copy link
Contributor

stevehu commented Mar 25, 2020

@ssilverman Thanks for the link. It is not very clear what needs to be done by reading for that paragraph :) My understanding is that we need to iterate all the siblings. Do we need to drill down to the siblings? Or we just need to evaluate it at the same level? I don't think there is a simpler way to do so. @prashanthjos has created a collector to collect useful info during the validation and it could iterate all validators. I am wondering if it could help. #268

@gjvoosten
Copy link

Since the new keyword unevaluatedItems (see https://json-schema.org/draft/2019-09/release-notes.html#applicator-vocabulary) is also not implemented, maybe take these together (and change the title of this issue to unevaluatedProperties and unevaluatedItems keywords not implemented)?

@stevehu stevehu changed the title unevaluatedProperties doesn't appear to be implemented unevaluatedProperties and unevaluatedItems keywords are not implemented Aug 17, 2020
@stevehu
Copy link
Contributor

stevehu commented Aug 17, 2020

@gjvoosten Good idea. I have changed the title to cover both keywords. Thanks.

@Gaff
Copy link

Gaff commented Oct 13, 2020

@ssilverman Just looking at your branch. Maybe it would be better to add an interface, something like:

public boolean wouldHandleProperty(JsonNode node, String propertyName);

Then the UnevaluatedPropertiesValidator could call the other validators with wouldHandleProperty() to figure out which properties have been handled?

@ssilverman
Copy link
Author

@Gaff I ended up writing my own validator :)

@stevehu
Copy link
Contributor

stevehu commented Oct 23, 2020

@ssilverman I am assuming you add some dummy validators that doing nothing to map to these keywords.

@ssilverman
Copy link
Author

@stevehu do you mean in my validator?

@stevehu
Copy link
Contributor

stevehu commented Oct 23, 2020

@ssilverman I am sorry. I thought this thread is talking about the warning messages for the un-implemented keywords. Please ignore my message above. Thanks.

@Johnlon
Copy link

Johnlon commented Jul 9, 2021

Any PR?

@fdutton
Copy link
Contributor

fdutton commented May 22, 2023

Resolved in #716 and #749

@fdutton fdutton closed this as completed May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants