oxidecomputer / typify

compiler from JSON Schema into idiomatic Rust types
Apache License 2.0
393 stars 57 forks source link

Boolean "required" field on object property leads to serialization failure #552

Closed halvko closed 4 months ago

halvko commented 5 months ago

I need to interact with an API specified using JSON Schema draft-03. cargo-typify errors during serialization of that API. The issue is the required: [bool] notation used on object properties in pre draft-04 versions of JSON Schema. The following is a minimal example where the serialization fails:

{
  "$schema": "http://json-schema.org/draft-03/schema#",
  "type": "object",
  "properties": {
    "name": {
      "type": "string",
      "required": true
    }
  }
}

This happens because schemars defines the required field as following:

pub struct RootSchema {
    //...
    #[serde(flatten)]
    pub schema: SchemaObject,
    //...
}
pub struct SchemaObject {
    //...
    #[serde(flatten, deserialize_with = "skip_if_default")]
    pub object: Option<Box<ObjectValidation>>,
    //...
}
pub struct ObjectValidation {
    //...
    pub required: Set<String>,
    //...
}

Since required have a wrong type, serde-json fails trying to deserializing the above example to an ObjectValidation.

To fix it for our use, I implemented parse-jsonschema to seperate the structure parsed into from the internal representation used, and correctly transforms the old required structure to the new.

I have used this in cargo-typify in our fork of typify (found here).

If you are interested in supporting the old required syntax in typify, I am ready to put in some time to make it ready for upstreaming, otherwise, feel free to close this issue.

Some notes on our current implementation:

I choose to create a separate structure from the one in schemars, since the structure in schemars seems to mainly be used for generating JSON Schema. It would be less fit for that purpose if it was modified to support serialization from any JSON Schema found in the wild.

The current implementation does not take the specified version of the JSON Schema into account. I have not checked the current specification to see if it disallows required fields in different contexts than the use in object schemas.

In our fork of typify the only crate which currently works is cargo-typify. I have not looked into why.

ahl commented 4 months ago

Thank you for this. I'd like to support multiple JSON schema versions, but in a "version-aware" way rather than a "permissive" way. I hope to write up a larger issue with my current thinking, but in general, I think I'd like to process each supported version with strict adherence to its spec, and dump that into an intermediate format. This would, for example, handle the incompatible changes to tuples in the 2020-12 spec. This may be taking on too much, but I've conflated this work with support for external references, dynamic references, as well as improved json path-based error messages.. so at a minimum I'd like to get my head around it all to sequence the work reasonably.