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

Schema#ref does not apply custom configuration options #198

Closed
ahx opened this issue Nov 9, 2024 · 8 comments · Fixed by #208
Closed

Schema#ref does not apply custom configuration options #198

ahx opened this issue Nov 9, 2024 · 8 comments · Fixed by #208

Comments

@ahx
Copy link

ahx commented Nov 9, 2024

Hi. What I would like to do is to use insert_property_defaults: true for subschemas that are adressed via Schema#ref without globally changing JSONSchemer.configuration.
But currently that does not seem to be possible, because Schema#ref does not apply the custom local options, but only uses the global configuration and passing insert_property_defaults: true to Schema#validate is not supported.

Here is an example:

doc = JSONSchemer.openapi(contents, insert_property_defaults: true)
content_schema = doc.ref('/components/schemas/MySchema')
content_schema.validate(data) # Fails, because insert_property_defaults is not applied (false)
# Now this (hack) makes it apply the option, but also changes global configuration, which I don't want:
doc = JSONSchemer.openapi(contents,)
content_schema = doc.ref('/components/schemas/MySchema')
content_schema.configuration.insert_property_defaults = true
content_schema.validate(data) # Succeeds

puts JSONSchemer.configuration.insert_property_defaults # => true

If you have a suggestion for an API to solve this, I'd be happy to try to create a PR for that.

@ahx ahx changed the title Schema#ref does not use custom options configuration Schema#ref does not apply custom configuration options Nov 9, 2024
@ahx
Copy link
Author

ahx commented Nov 9, 2024

Ha! I've found a solution.

  configuration = JSONSchemer::Configuration.new(insert_property_defaults: true)
  doc = JSONSchemer.openapi(resolved, configuration:)
  content_schema = doc.ref('/components/schemas/MySchema')
  content_schema.validate(data) # Succeeds! 

Thanks! 🦆
Feel free to close this issue.

@ahx ahx closed this as completed Dec 6, 2024
@davishmcclurg
Copy link
Owner

Hi @ahx, sorry for the delay here. I'm going to reopen this because I think it's an issue that subschemas don't inherit some options. It seems like we need to merge (some of?) the initialize options back into configuration:

@configuration = configuration

so that they get passed to the ref schema:

:configuration => configuration,

Let me know if you want to work on a fix!

@davishmcclurg davishmcclurg reopened this Dec 15, 2024
@ahx
Copy link
Author

ahx commented Dec 16, 2024

Hey David, I agree and I would like to work on a fix.
What I would also like is support for being able to overwrite configuration per subschema, Like doc.ref('/components/schemas/MySchema', after_property_validation: myproc)

I'll try to work on this. If I don't get back here until the new year, feel free to poke me.

@tonobo
Copy link

tonobo commented Jan 3, 2025

Yep, also hit this case. The library defaults get inherited.

if root.before_property_validation.any?

it took me a moment to figure out that “root” is not “root”, but in the “parsed” loop below it saw the properties, then I came up with the issue.

@davishmcclurg
Copy link
Owner

@ahx have you had a chance to work on this? I've got another fix to merge/release and I'm hoping to include this as well.

@ahx
Copy link
Author

ahx commented Jan 20, 2025

My approach was to clone the base config and merge it with incoming parameters (see commit). And I wanted to merge the many arguments of Schema#new into one **options. I got a failing test to pass, but that resulted in other things breaking, which I could not get my head around just yet. So I have not accomplished much here. I bet you can find a solution much faster than me. So don't feel held back if you find something working.

davishmcclurg added a commit that referenced this issue Jan 21, 2025
This effectively merges `Schema.new` options with the provided
`Configuration` object (or the default global one) so that subschemas
validate using the same configuration as the parent schema.

`base_uri` and `meta_schema` are still instance variables because
they're modified by keywords (that's why they're `attr_accessor`).

`resolve_ref` still passes `ref_resolver` and `regexp_resolver` to
subschemas to support caching (because the `CachedResolver` instances
are created in `Schema`). They could maybe be cached in `Configuration`
instead, but that felt like a bigger change.

Fixes: #198
@davishmcclurg
Copy link
Owner

My approach was to clone the base config and merge it with incoming parameters (see commit). And I wanted to merge the many arguments of Schema#new into one **options.

Cool, I took a similar approach here: #208
If you have some time, do you mind testing it out to see if it fixes your original issue?

@davishmcclurg
Copy link
Owner

Fix released in 2.4.0

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 a pull request may close this issue.

3 participants