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

Fine-grained JSON document structure control #101

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

handrews
Copy link
Contributor

Fixes #99, enabling possibe OpenAPI 3.x support as an extension.

This factors the instantiation of list items and object values into their own methods to allow more flexible determination of whether a sub-structure is a JSON Schema or not. This will allow using jschon with formats like OpenAPI that embed schemas in complex pattern of locations, without the root object being a schema.

@marksparkza while I'd love to get this into a 0.10.3, I realize it potentially has more substantial implications by defining an extension interface. If this approach feels like it needs more work, I would be happy for this to be bumped to a future release if it means #100 can go out in an earlier 0.10.3 release, as that one is a bugfix that will have immediate impact.

@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (e6f71d0) 92.73% compared to head (416690d) 92.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   92.73%   92.75%   +0.01%     
==========================================
  Files          23       23              
  Lines        2052     2056       +4     
  Branches      435      435              
==========================================
+ Hits         1903     1907       +4     
  Misses         97       97              
  Partials       52       52              
Files Changed Coverage Δ
jschon/json.py 94.31% <100.00%> (+0.10%) ⬆️

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

@handrews
Copy link
Contributor Author

It would also be necessary to make JSONSchema.resolve_references() public or otherwise provide a way to trigger it as it currently is only called on root objects. With embedded schemas, there is essentially a local root for each embed. But this is not a document root, as they are part of the overall document (which can't have resolve_references() called on it for several reasons, including that OAS uses $ref in slightly different ways outside of schema objects.

This is done as part of PR #82, but could be split out to accompany this PR (which I guess would make it a 0.11.0 as it would be a public interface change, albeit a backwards compatible one)

@handrews
Copy link
Contributor Author

OK, playing with all of this more, I've essentially had to re-implement #82 and #87. And would probably also need #83 due to weirdness between OAS 3.1 schema $ids vs their source control locations.

So I think when you have time to look over all of this for a 0.11.0, it would be good to solve all of these related challenges. In the meantime, I'll work off of my fork and keep experimenting to see what works.

@handrews handrews changed the title 0.10.3 (mabye?): Fine-grained JSON document structure control 0.10.3 (or probably 0.11.0): Fine-grained JSON document structure control May 21, 2023
@handrews handrews changed the title 0.10.3 (or probably 0.11.0): Fine-grained JSON document structure control Fine-grained JSON document structure control Jun 22, 2023
@handrews
Copy link
Contributor Author

handrews commented Jul 1, 2023

My OpenAPI project no longer needs this specific change, although I still think something like this is probably worthwhile in the context of extensibility in general. If such extensibility is desired. I find myself using the JSON/JSONPointer/RelativeJSONPointer/JSONPatch system within jschon somewhat independently of the actual JSON Schema behavior, although the topic of referencing crosses over.

@handrews handrews marked this pull request as draft July 1, 2023 21:35
@handrews
Copy link
Contributor Author

Per comment in #108, I'm moving this back out of draft. The alternate way I have handled this in oascomply is more fragile and complex (starting with an all-OpenAPI document tree, then intercepting Catalog.load_schema() type errors and re-instantiating document subtrees as schemas). Having a way to build the document tree correctly in the first place will be much more robust.

@handrews handrews marked this pull request as ready for review July 18, 2023 16:25
This factors the instantiation of list items and object values
into their own methods to allow more flexible determination
of whether a sub-structure is a JSON Schema or not.  This will
allow using jschon with formats like OpenAPI that embed schemas
in complex pattern of locations, without the root object being
a schema.
Left off the previous commit by accident.
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.

jschon and OpenAPI support (surprisingly simple... probably)
1 participant