Skip to content

Fixes allOf bug when using noAdditionalProperties option #535

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

Conversation

azrosen92
Copy link
Contributor

@azrosen92 azrosen92 commented May 16, 2025

Fixes bug when evaluating additional properties in allOf subschemas.

When using the noAdditionalProperties option, json-schema incorrectly evaluates allOf schemas by marking json data invalid before having evaluated it against all of the sub schemas within an allOf schema. In the logic implemented in the properties attribute class to handle the noAdditionalProperties option, json-schema correctly identifies any additional properties that are not defined in that schema. However, if the parent schema is allOf and one of the other sub shemas within that allOf does expect that attribute, then these additional properties may in fact be valid. Therefore, we must look at all of the errors from all sub schemas within that allOf in order to be able to properly determine whether there is an additional property that has not been defined in any of those sub schemas.

This PR does 2 things to accomplish this

  1. Adds a properties attribute to ValidationError allowing us to record the additional properties detected within the data
  2. Updates logic in the AllOfAttribute class to consider whether there are any additional properties that were detected across all sub schemas of the allOf.

Copy link

codecov bot commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.52%. Comparing base (728c17f) to head (178292e).
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #535   +/-   ##
=======================================
  Coverage   90.52%   90.52%           
=======================================
  Files          76       76           
  Lines        1562     1562           
=======================================
  Hits         1414     1414           
  Misses        148      148           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bastelfreak
Copy link
Member

Hey @azrosen92 , thanks for the test! Are you able to provide a fix for it?

@azrosen92
Copy link
Contributor Author

@bastelfreak just getting back from vacation – I'd like to try to work on a fix, I'll aim to have something in the next week or two!

@azrosen92 azrosen92 force-pushed the no-additional-properties-incompatible-with-all-of branch from 1fa33a0 to f18f13e Compare June 2, 2025 20:18
@azrosen92
Copy link
Contributor Author

@bastelfreak I think this is ready for a review. Please let me know if you have any questions/concerns!

@azrosen92 azrosen92 changed the title failing test Fixes allOf bug when using noAdditionalProperties option Jun 3, 2025
@azrosen92
Copy link
Contributor Author

@hoxworth – would love to get some eyes on this ASAP 🙇🏻‍♂️

@bastelfreak bastelfreak added the Bug label Jul 8, 2025
@bastelfreak
Copy link
Member

@azrosen92 thanks for the work!

@bastelfreak bastelfreak merged commit a92f016 into voxpupuli:master Jul 8, 2025
10 checks passed
@azrosen92 azrosen92 deleted the no-additional-properties-incompatible-with-all-of branch July 21, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants