davishmcclurg / json_schemer

JSON Schema validator. Supports drafts 4, 6, 7, 2019-09, 2020-12, OpenAPI 3.0, and OpenAPI 3.1.
MIT License
399 stars 64 forks source link

`JSONSchemer` is confused about string or symbol keys #166

Closed exterm closed 9 months ago

exterm commented 9 months ago

On version 2.1.1:

data = { "baz": 1 }
schema = JSONSchemer.schema({ "type": "object", "properties": { "foo": { "default": "bar" } } }, insert_property_defaults: true)
schema.validate(data)
data

# => {:baz=>1, "foo"=>"bar"}

IMO this should either result in only symbol keys or only string keys.

davishmcclurg commented 9 months ago

IMO this should either result in only symbol keys or only string keys.

How would you determine what type of key to use if there are no existing keys? In your example, if :baz wasn't present, would it still insert a symbol?

Another way I could imagine addressing this is something like insert_property_defaults: :symbol to specify which type of key you want.

exterm commented 9 months ago

How would you determine what type of key to use if there are no existing keys?

I think I'd prefer JSONSchemer to decide for one key type to use and convert the other type to it, always. Do you see any reasons not to do that?

davishmcclurg commented 9 months ago

Do you mean convert the keys in place? I think it would be confusing to modify the input hash like that (ie, changing all the key types) during validation.

exterm commented 9 months ago

Ah. I agree.

The problem here is that "Ruby representation of JSON" is so ambiguous. It would be great to have some kind of JSONData type that removed that inconsistency, but I believe that to be out of scope for this gem.

davishmcclurg commented 8 months ago

The problem here is that "Ruby representation of JSON" is so ambiguous. It would be great to have some kind of JSONData type that removed that inconsistency, but I believe that to be out of scope for this gem.

Yeah, I agree the ambiguity is annoying. I'd like to see it solved in Ruby itself, but it seems unlikely at this point. Somewhat related:

Would the insert_property_defaults: :symbol option be helpful in your case? I'm still considering adding it.

exterm commented 8 months ago

Yes I think it would improve the situation, although I'm no longer working on that code so can't play around with it to be sure.