fredo-dedup / JSONSchema.jl

JSON Schema validation package for Julia
Other
31 stars 12 forks source link

Make files read with JSON3.jl compatible #50

Closed jClugstor closed 1 year ago

jClugstor commented 1 year ago

I was trying to validate a file that was parsed using JSON3.jl, with a schema parsed using JSON3.jl, and ran in to some problems.

The first was that the references in the schema were not being resolved. Because the JSON3.jl objects and arrays are not mutable, the way references are resolved did not work.

Second, the use of haskey in the creation of the schema object / reference resolution and validation means that the keys must be strings. JSON3 tends to use Symbols for its keys.

Essentially, I just made it so that if the schema to turn in to a Schema object was from JSON3, recursively turn it in to a mutable dictionary with keys that are strings. Same thing for the JSON file to validate, if it's from JSON3, turn it in to a dictionary with string keys.

odow commented 1 year ago

Essentially, I just made it so that if the schema to turn in to a Schema object was from JSON3, recursively turn it in to a mutable dictionary with keys that are strings

Can't this be done in your user-code? What is the benefit of adding it to JSONSchema.jl?

I'd prefer that we kept this repo simple, which means supporting only JSON.jl and the Base Julia dictionaries and arrays.

jClugstor commented 1 year ago

The main benefit would be that a widely used (probably at least the second most widely used judging by answers on discourse and slack) JSON parser package would be compatible in a "plug and play" way with this package. So that users would be able to directly use a faster JSON parsing package without having to change the form of their parsed data manually.

I'll also reference https://github.com/fredo-dedup/JSONSchema.jl/pull/41, which was meant to add some support for JSON3.jl, but yes unlike this it didn't add a dependency.

If you would rather not add a dependency, I think there should at least be something in the readme that mentions what form any input Dicts should be in (i.e. strings for keys, and needs to be mutable). I can send a pull request along those lines if you would like.

odow commented 1 year ago

Maybe it's not so bad as a direct dependency. The only new transitive is StructTypes: https://juliahub.com/ui/Packages/JSON3/1p699/1.13.2?page=1

You can improve your implementation by using multiple dispatch. Perhaps something like:

import JSON3

_to_base_julia(x) = x

_to_base_julia(x::JSON3.Array) = _to_base_julia.(x)

function _to_base_julia(x::JSON3.Object)
      return Dict{String,Any}(string(k) => _to_base_julia(v) for (k, v) in x)
end

function validate(
      schema::Schema, 
      x::Union{JSON3.Array,JSON3.Object},
)
      return validate(schema, _to_base_julia(x))
end

function Schema(schema::JSON3.Object; kwargs...)
      return Schema(_to_base_julia(schema); kwargs...)
end

This will also need tests, etc.

jClugstor commented 1 year ago

Ok, so I've added in dispatches for the JSON3.Object and JSON3.Array types for Schema and validate. I've added the compat in the project file, and I added a test for JSON3 parsing. For the test, I used the same thing as the Draft 4/6 test, just changed the actual test to isnothing(validate(spec, test[:data])) == test[:valid] so that what's being tested will go through the dispatch for validate.

jClugstor commented 1 year ago

Oh and I forgot to ask: are there any other tests I should create?

odow commented 1 year ago

Thanks!