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 Unevaluated properties keyword. #534

Merged
merged 40 commits into from
Mar 18, 2022
Merged

Conversation

prashanthjos
Copy link
Member

This PR adds unevaluatedProperties keyword implementation to networknt's json-schema-validator. Please refer to the following discussion for more details on how this keyword might work.

json-schema-org/json-schema-spec#556

prashanthjos and others added 30 commits October 21, 2020 17:39
Getting latest changes from networknt
Fixing concurrency and compilation issues. (networknt#385)
@prashanthjos
Copy link
Member Author

@stevehu can you please have a look at this PR and give your suggestions.

@stevehu
Copy link
Contributor

stevehu commented Mar 18, 2022

@prashanthjos Thanks a lot for the PR. It was a good read for the spec :) I understand that the keyword is introduced in 2020-12 and we don't have this version created yet. I am going to find some time to add this new version. Meanwhile, I am wondering if you could add a test case to use the standard test suite? The test case can help us to keep the code stable if someone else wants to contribute to this keyword. Here is the link to the test https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft2020-12/unevaluatedProperties.json

BTW, you have the write access in this repo and you can create a feature branch instead of using your forked repo.

@stevehu stevehu merged commit 5007242 into networknt:master Mar 18, 2022
@prashanthjos
Copy link
Member Author

Sure @stevehu I will try to run the tests mentioned in the URL. Also please note that this unevaluatedProperties PR is only applicable for a boolean value for this keyword and not a schema.

@TJC TJC mentioned this pull request Apr 4, 2022
@TJC
Copy link

TJC commented Apr 4, 2022

Hi -- this PR seems to be the one to introduce the memory leak I reported in #546

If I check out the commit previously, my test runs fine. Then on this commit, it breaks.

@stevehu
Copy link
Contributor

stevehu commented Apr 4, 2022

@TJC Thanks a lot for identifying the breaking point. Let's review the changes in this PR again to see if we can find out the root cause. @prashanthjos I am wondering if you could help to review the code. If necessary, I can set up a zoom meeting to work together.

@TJC
Copy link

TJC commented Apr 5, 2022

Andreas helped me make a heap dump. Looking at the profiler now, it kinda looks like the AnyOfValidator.java has an ArrayList on line 80 which is not being cleared at the end of a validation, and just keeps accumulating more and more elements as more files get validated.

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

Successfully merging this pull request may close these issues.

4 participants