gregsdennis / Manatee.Json

A fully object-oriented approach to JSON manipulation, validation, and serialization that focuses on modeling the JSON structure rather than mere string parsing and conversion.
MIT License
198 stars 32 forks source link

Resolving $refs with overridden "JsonSchemaOptions.Download" #269

Closed amiel92 closed 4 years ago

amiel92 commented 4 years ago

Hi, i tried overriding the automatic $ref resolution "JsonSchemaOptions.Download" to read the referenced schemas from the file system, like it is described on here: https://gregsdennis.github.io/Manatee.Json/usage/schema/references.html It seems however that not all schema $refs are used for validation and trigger a download.

How to Reproduce

Set up a project and include Manatee.Json

Create 3 json schema files:

Create a directory named "schemas" in your project directory Create the sub directories "1.0.0", "1.0.1" and "1.1.0"
Copy base.schema.json to the directory "1.0.0" Copy sub.schema.json to the directory "1.0.1" Copy instance.schema.json to the directory "1.1.0"

Use or adapt the following code and set the Breakpoint where it is commented

private readonly string schemaDirPath = $"{AppDomain.CurrentDomain.BaseDirectory}/schemas/";
private readonly string schemaUriPrefix = "http://localhost/";
private readonly string jsonData = "{}";  //the acutal json should not matter for this issue
private readonly JsonSerializer serializer;

JsonSchemaOptions.Download = uri =>
            {
                var localSchemaPath = uri.Replace(schemaUriPrefix, schemaDirPath); //Set Breakpoint
                var filePath = $"{localSchemaPath}.schema.json";
                return File.ReadAllText(filePath);
            };

var instanceSchemaFilePath = $"{Path.Combine(schemaDirPath, "schemas/example/1.1.0/instance")}.schema.json";
var schemaJson = JsonValue.Parse(File.ReadAllText(instanceSchemaFilePath));
var schema = serializer.Deserialize<JsonSchema>(schemaJson);
var validationResults = schema.Validate(jsonData);
var res = validationResults.IsValid;

Expected behavior instance.schema.json has 2 referenced schemas. Therefor "JsonSchemaOptions.Download" shoud also be triggerd for the second referenced schema "$ref": "http://localhost/schemas/1.0.1/sub". It is however only triggered for "$ref": "http://localhost/schemas/1.0.0/base".

OS and Versions

gregsdennis commented 4 years ago

Thanks for reporting this. ~Could you let me know which version of the library you're using, the runtime (e.g. .net core 2.1), and what OS you have?~

@amiel92, What runtime are you testing with? .Net Standard can't be run independently.

gregsdennis commented 4 years ago

Oh! No, this is the correct behavior for draft-7. Draft 7 and all drafts before declare that keywords alongside $ref are to be ignored. This means that your instance.schema.json schema is really invalid.

  1. $schema declares a draft where $ref-sibling keywords are ignored.
  2. $schema itself is $ref-sibling... instant paradox.

If we ignore that and assume that having $schema and $ref is invalid, it's still supposed to ignore the properties keyword. Since the reference to http://localhost/schemas/1.0.1/sub is within properties it properly never gets called.

However, if we instead use draft 2019-09, then $ref-sibling are allowed, and the validation should download both.

... but it's not, so there's a bug.

gregsdennis commented 4 years ago

OH! (yeah this is my default epiphany) The $refs are resolved as needed. In your test, the instance doesn't have the propertyAffectedFromSubSchema property, so that branch in the schema is never exercised, and the `http://localhost/schemas/1.0.1/sub schema isn't downloaded.

Updating the test instance to {"propertyAffectedFromSubSchema": {}} exercises this branch. Now the test should download only base for draft 7, and both base and sub for draft 2019-09.

Lastly, passing a string containing the JSON data is not quite right. That will cast the string to a JsonValue with a string value containing the data. You either want to parse the data:

var jsonData = JsonValue.Parse("{\"propertyAffectedFromSubSchema\": {}}")

or explicitly build it:

var jsonData = new JsonObject { ["propertyAffectedFromSubSchema"] = new JsonObject() };

So if we give it the correct value, it should validate both cases properly.

... but it's not for draft 7, so there's a bug.

gregsdennis commented 4 years ago

One last thing, the meta-schema IDs have # on the end. Yours don't.

Declared:
"$schema": "http://json-schema.org/draft-07/schema#"

Yours:
"$schema": "http://json-schema.org/draft-07/schema"

I had allowed for this in the schema registry, but not in the meta-schema validation. This has been fixed as well.

gregsdennis commented 4 years ago

@amiel92 please see v13.0.1 for your fix.

amiel92 commented 4 years ago

@gregsdennis Thanks a lot for responding this quickly. I updated to v13.0.1 and switched to draft 2019-09. Downloading seems to work now as expected.

I have a question regarding the following exception if you dont mind:

.....
Exception: Manatee.Json.Schema.SchemaLoadException: The given path does not contain a valid schema.
   at Manatee.Json.Schema.JsonSchemaRegistry.Get(String uri)
   at Manatee.Json.Schema.RefKeyword._ResolveReference(SchemaValidationContext context)
   at Manatee.Json.Schema.RefKeyword.Validate(SchemaValidationContext context)
   at Manatee.Json.Schema.JsonSchema.Validate(SchemaValidationContext context)
   at Manatee.Json.Schema.JsonSchema.Validate(JsonValue json, JsonSchemaOptions options)
.....

Say I change base.schema.json to:

{
    "$id": "http://localhost/schemas/1.0.0/base",
    "$schema": "https://json-schema.org/draft/2019-09/schema",
    "type": "object",
    "definitions": {
        "contextProp": {
            "type" : "object",
            "required": [
                "subProp"
            ],
            "properties": {
                "subProp": {
                    "type": "string"
                }
            }
        }
    },
    "required": [
        "contextProp"
    ],
    "properties": {
        "contextProp": {
            "$ref": "#/definitions/contextProp"
        }
    }
}

and the instance.schema.json to:

{
    "$id": "http://localhost/schemas/1.1.0/instance",
    "$schema": "https://json-schema.org/draft/2019-09/schema",
    "$ref": "http://localhost/schemas/1.0.0/base",
    "properties": {
        "propertyAffectedFromSubSchema": {
            "$ref": "http://localhost/schemas/1.0.1/sub"
        },
        "contextProp" : {
            "type" : "object"
        }
    }
}

the test.json looks like this:

{
  "propertyAffectedFromSubSchema" : {},
  "contextProp" : {
    "subProp" : {
      "type": "string"
    }
  }
}

If i use the schemas as described above, i will get the mentioned exception. I tried inserting "$id" to "contextProp" in the instance.schema.json:

{
    "$id": "http://localhost/schemas/1.1.0/instance",
    "$schema": "https://json-schema.org/draft/2019-09/schema",
    "$ref": "http://localhost/schemas/1.0.0/base",
    "properties": {
        "propertyAffectedFromSubSchema": {
            "$ref": "http://localhost/schemas/1.0.1/sub"
        },
        "contextProp" : {
            "$id" : "http://localhost/schemas/1.0.0/base",
            "type" : "object"
        }
    }
}

However only the sub schema gets downloaded by doing so. The base schema seems to be ignored. I am probably not using $id as intended. Since it is supposed to set the base uri for $ref if i understood it correctly. Is there a way to set the context?