OneOf/required schema validation issue

[ikenne]

Given this schema:

{
....
"xyz": {
	"type": "object,
	"properties": {
		"A": {  "type": "string" }
		"B": {  "type": "integer" }
		"C": {  "type": "number"  }
	},
	"required": ["A"],
	"additionalProperties": false,
	"oneOf": [
            {
              "required": ["B"]
            },
			{
              "required": ["C"]
            }			
     ]
 }
... 
}

And this JSON:

 xyz : 
 {
	"A": "abc"
 }

I expected the validation to return an error because one of the required fields (B or C) is not present in the JSON. The idea is to have one of B or C.

However, this is not the case. The Validate method validates this JSON with no error. This is with version 9.9 of the library.

[gregsdennis]

Thanks for reporting this. I'll set up a test to try and replicate it.

I agree with your assessment. The JSON instance should fail.

[gregsdennis]

I've been able to replicate a problem, but it's not the same thing you're getting. For me it properly returns invalid, but for the wrong reason.

I'll upload my branch and provide a link here to make sure what I'm doing matches what you think it should.

[gregsdennis]

Here are the tests:

https://github.com/gregsdennis/Manatee.Json/blob/bug/167-one-of-failure/Manatee.Json.Tests/Schema/ClientTests.cs#L226

[ikenne]

The test looks good.

For me it properly returns invalid, but for the wrong reason.

What error are you getting? I assume the error that should be returned is a oneOf for the xyz schema not being satisfied.

I didn't get any error in my case; loaded a schema from a file (not exactly the one posted above but with same structure), and validated a JSON against the schema.

[gregsdennis]

For me, it was saying that both of the subschemas returned valid, which was also wrong. The bug I found was that required wasn't being evaluated unless properties, patternProperties, or additionalProperties was present.

Fixing this, both of the subschemas return invalid, and the oneOf properly states that they both failed.

[ikenne]

Look forward to the fix.

I had an issue while using "propertyNames" with "allOf"/"anyOf" but it might be due to this bug. I will give that a try when this fix is published. If it something different, could raise another bug/issue.

[gregsdennis]

Actually, if you can post an example of that, I can verify it works before publishing.

[ikenne]

Ok. A bit long, hopefully explains the point:
i) schema

{
   "fields": {
     "type": "object",
     "properties": {
   		"field1": {
   			"type": "string"
   		},
   		"field2": {
   			"type": "integer"
   		},
     } 
   },
   "xyzBaseFieldNames": {
      "enum": [
        "field1",
        "field2"
      ]
    },
    "worldwide": {
      "allOf": [
        {
          "$ref": "#/definitions/fields",
          "required": [
            "field1"
          ]
        },
        {
          "properties": {
            "A": {
              "type": "string"
            }
          },
          "properties": {
            "B": {
              "type": "integer"
            }
          },          
          "oneOf": [
            {
              "required": [
                "A"
              ]
            },
            {
              "required": [
                "B"
              ]
            }
          ]
        },
        {
          "propertyNames": {
            "anyOf": [
              {
                "$ref": "#/definitions/xyzBaseFieldNames"
              },
              {
                "enum": [
                  "A"
                ]
              },
              {
                "enum": [
                  "B"
                ]
              }
            ]
          }
        }
      ]
    },
    "xyz" : {
      "type": "object",
      "properties": {
      	"worldwide": {
      	  "$ref": "#/definitions/worldwide"
      	}
      
      },
      "additionalProperties": false        
    },
    ...
 }

ii) Should be valid (one of A or B)

  "xyz": {
    "worldwide": {
    	"field1": "abc",
         "A": "def",
    }
  }   

iii) Should be valid (one of A or B)

  "xyz": {
    "worldwide": {
    	"field1": "abc",
    	"field2": 20,
         "A": "def",
    }
  } 

iv) Should be invalid (only one of A or B )

 "xyz": {
    "worldwide": {
    	"field1": "abc",
         "A": "def",
         "B": 10,
    }
  }  

v) Should be invalid (field1 is missing)

  "xyz": {
    "worldwide": {
         "A": "def",
    }
  }

From my tests, the invalid scenarios were not caught with the validation.

[gregsdennis]

I think you may be missing a level in the schema. Are fields, xyzBaseFieldNames, etc. expected property names?

Also, I'm concerned that this part

        {
          "$ref": "#/definitions/fields",
          "required": [
            "field1"
          ]
        }

won't work as you expect. The spec requires that $ref should not be accompanied by other keywords. If other keywords are present, they should be ignored. To get what I think you want, you split it into two schema and combine them in an allOf schema:

        {
          "allOf": [
            { "$ref": "#/definitions/fields" },
            {
              "required": [
                "field1"
              ]
            }
          ]
        }

[ikenne]

fields, xyzBaseFieldNames define property names.
Having it this ways allows them to be reused in other object property definitions.
An example of use is here: json-schema-org/json-schema-org.github.io#77 (comment) Does the current version support propertyNames in this way?

Also the xyz JSON is not marked as invalid with a missing worldwide even with the allOf definition. This should not be the case.

Your modification for the $ref should be fine since it ensures "field1" is enforced as a required property.

[gregsdennis]

I'm having trouble constructing this schema due to the missing parts. Could you please post a full schema (or edit the above)? For instance, if fields is a property (listed under the properties keyword), what does {"$ref":"#/definitions/fields"} reference?

[ikenne]

Ok here is the whole correct schema and samples:
i) schema

{
  "$schema": "http://json-schema.org/schema#",
  "definitions": {
    "fields": {
      "type": "object",
      "properties": {
        "field1": {
          "type": "string"
        },
        "field2": {
          "type": "integer"
        }
      }
    },
    "xyzBaseFieldNames": {
      "enum": [ "field1", "field2" ]
    },
    "worldwide": {
      "allOf": [
        { "$ref": "#/definitions/fields"  },
        {"required": ["field1"]  },
        {
          "properties": {
            "A": { "type": "string" },
            "B": { "type": "integer" }
          },
          "oneOf": [
            {
              "required": ["A"]
            },
            {
              "required": ["B"]
            }
          ]
        },
        {
          "propertyNames": {
            "anyOf": [
              {
                "$ref": "#/definitions/xyzBaseFieldNames"
              },
              {
                "enum": ["A", "B"]
              }
            ]
          }
        }
      ]
    }
  },
  "type": "object",
  "properties": {
    "xyz": {
      "$ref": "#/definitions/worldwide"
    }
  },
  "additionalProperties": false,
  "required": ["xyz"]
}

ii) valid

{  
 "xyz": {
      "field1": "abc",  
      "field2": 1,
       "A": "def"
  }
}

iii) valid

{  
 "xyz": {
      "field1": "abc",  
       "A": "def",
  }
}

iv) invalid (only one of A or B)

{  
 "xyz": {
      "field1": "abc",  
      "field2": 1,
       "A": "def",
       "B": 3
  }
}

v) invalid (missing field1) i.e correct combination of field2 or A or B, without field1
E.g.

{  
 "xyz": {
       "A": "def",
  }
}

{  
 "xyz": {
       "field2": 1,
       "A": "def",
  }
}

The propertyNames in the allOf worldwide ensures no additional properties can be included.

I checked the schema and sample validation here: JSON schema validator

[gregsdennis]

Yes, all of these validate properly with the new version. I'll publish shortly. Thanks for your help.

[gregsdennis]

@ikenne FYI - the $ref alongside other keywords looks like it'll probably be allowed in draft-08. You can checkout the discussion here.