Filter anyOf/oneOf object alternative results by the discriminator

[jdesrosiers]

Let's start with an example.

{
  "anyOf": [
    {
      "type": "object",
      "properties": {
        "type": { "const": "a" },
        "a": { "type": "string" }
      },
      "required": ["type", "a"]
    },
    {
      "type": "object",
      "properties": {
        "type": { "const": "b" },
        "b": { "type": "number" }
      },
      "required": ["type", "b"]
    }
  ]
}

If given {}, we don't know which alternative was intended, so we give the errors for both alternatives.

• Expected the value to match at least one alternative
  • Alternative 1
    • Missing required properties: "type" or "a"
  • Alternative 2
    • Missing required properties: "type" or "a"

If given { "type": "a" }, we know that the the user intended the first alternative and can collapse the message to the messages for that one schema.

• Missing required property: "a"

In this case, it's clear that "type" is the discriminator and if "type" matches the const keyword, that's the alternative that's intended. But, there are other more subtle discriminators. The property name can be anything, the schema of a discriminator can be anything, and the discriminator can be more than one property.

We'll define the discriminator as the set of properties defined by all object alternatives. If an object alternative passed all the discriminator property(s), then any other alternatives that don't match the discriminator can be filtered out.

As before if there is only one alternative left, it can be collapsed and the generic anyOf/oneOf message doesn't need to be returned.

Keep in mind that an anyOf/oneOf can be have both objects and non-objects and the discriminator can still be used to filter the object alternatives.

[justin212407]

We can add a discriminator-filtering pass to both anyOf and oneOf that will run after the existing type filter that we have right now. The discriminator could be a set of property names common to all object alternatives , we can then filter to keep only alternatives where the instance has no keyword failures at those discriminator properties. If exactly one alternative passes the discriminator, we will collapse it by returning only that alternative's errors without the generic anyOf/oneOf wrapper message.

[jdesrosiers]

we can then filter to keep only alternatives where the instance has no keyword failures at those discriminator properties

We can only filter if at least one object alternative matches the discriminator.

[justin212407]

So if i get this right the logic goes this way:
compute the discriminator properties, filter to alternatives with no failures at those properties, and if at least one passes, use the filtered set otherwise, fall back to showing all alternative.

Did i get that right?

[jdesrosiers]

I was thinking, I wonder if there's a better rule we can use that we can apply more generally. The following wouldn't have a discriminator, but we could still deduce which alternative is intended based on which properties match.

{
  "anyOf": [
    {
      "type": "object",
      "properties": {
        "type": { "const": "a" },
        "a": { "type": "string" }
      },
      "required": ["type", "a"]
    },
    {
      "type": "object",
      "properties": {
        "type": { "const": "b" },
        "b": { "type": "number" }
      },
      "required": ["type", "b"]
    },
    {
      "type": "object",
      "properties": {
        "c": { "type": "boolean" }
      },
      "required": ["c"]
    }
  ]
}

Given { "type": "a" }; "type" doesn't appear in alternative 3, so we can filter that out. "type" fails alternative 2 and there's another alternative where "type" passes, so we can filter out 2 as well. If we can generalize those eliminations, I think we can get the same kind of behavior as the discriminator approach and more.

Rule 1:
Filter out any alternatives where the instance properties don't have any intersection with the declared properties of the alternative.

Rule 2:
If an instance property passes validation for one alternative, filter out any alternatives that declare that same property and that property fails validation.

This might get more tricky when patternProperties, additionalProperties, or unevaluatedProperties are present and allow for properties that don't have a definite name.

{
  "anyOf": [
    {
      "type": "object",
      "properties": {
        "type": { "const": "a" },
        "a": { "type": "string" }
      },
      "required": ["type", "a"]
    },
    {
      "type": "object",
      "properties": {
        "c": { "type": "boolean" }
      },
      "required": ["c"],
      "additionalProperties": {
        "type": "string"
      }
    }
  ]
}

In this case, given { "type": "a" }, additionalProperties means all properties are defined and Rule 1 doesn't apply. Because "a" passes the additionalProperties keyword, Rule 2 doesn't filter out alternative 2. It think that's what we want.

I think it works as long as we're thoughtful about handling those keywords.

[justin212407]

So Rule 1 handles structural irrelevance and Rule 2 handles value-level clarity?
One thing I would like to confirm though, for Rule 1, should we treat patternProperties as also "declaring" a property if the property name matches the pattern?
For example, if alternative has patternProperties: { "type": { ... } } and the instance has "type", that should count as an overlap right then should we check pattern matches explicitly for Rule 1, or is it safe to treat any alternative with patternProperties as declaring all properties?

[jdesrosiers]

Yes, it should only consider a property declared if it matches the pattern.

"patternProperties": {
  "^t": { "type": "string" }
}

This should consider any property that starts with a "t" to be declared.

[animeshsahoo1]

I wanna add something to rule 2 if its not already intended, since we are talking about user intent I had a question what if the user by mistake wrote the instance slightly wrong, like for this schema:

{
  "anyOf": [
    {
      "type": "object",
      "properties": {
        "type": { "const": "a" },
        "x": { "type": "string" }
      },
      "required": ["type", "x"]
    },
    {
      "type": "object",
      "properties": {
        "type": {
          "allOf": [
            { "type": "string" },
            { "minLength": 2 }
          ]
        },
        "x": { "type": "number" }
      },
      "required": ["type", "x"]
    }
  ]
}

Instance: { "type": "a", "x": 42 }

now Rule 2 will pass the alternative 1 and filter out the 2nd one, but the person wrote x: number constraint which is correct for 2nd alternative so we cant rule the alternative out after checking just one of the properties, I think we should not filter out an alternative just because one property fails it - only filter it if no instance property passes validation in that alternative. let me know what you think about this or if it was what you intended from start.

[justin212407]

I think your suggestion does align better with user intent to keep any alternative where at least one instance property validates successfully.

So according to that proposal rule 2 becomes:

• For each alternative, check all instance properties against its declared properties
• If at least one instance property passes validation in that alternative, keep it
• Only eliminate an alternative if every instance property that appears in both the instance and the alternative's declared properties fails validation

[srivastava-diya]

Hey @jdesrosiers
I think we should stick to the explicit discriminator logic rather than relying on generalized rules. While Rules 1 and 2 seem good for simple schemas, they will quickly break down in practice due to the ambiguity of partial validation successes.
Since I have worked a bit on the discriminator implementation, I will shortly come up with some concrete examples that show exactly why these rules can unintentionally filter out the correct alternative or fail to filter out the wrong ones.

[animeshsahoo1]

I somewhat agree and disagree with @srivastava-diya. While I was finding some edge cases, I also felt that sometimes a user's intention and their code may not be the same, as seen in the example I gave above. Though with the extension to Rule 2 suggested above, I think we are handling a lot of cases, but we are not reducing noise in those cases. Having a discriminator guarantees noise reduction and also solves the hierarchical issue where one property is obviously more important than another. For example:

{
  "anyOf": [
    {
      "type": "object",
      "properties": {
        "type": { "const": "a" },
        "x": { "type": "string" }
      },
      "required": ["type", "x"]
    },
    {
      "type": "object",
      "properties": {
        "type": { "const": "b" },
        "x": { "minimum": 100 }
      },
      "required": ["type", "x"]
    }
  ]
}

for the Instance: { "type": "a", "x": 150 } alt1 passes the type property but fails the 2nd property but the 2nd alternative passes the "x" property. In most cases the user might have intended the first alternative but our rules cannot handle it.

why I disagree using a explicit discriminator is that its an openAPI specific term I only recently heard about it in openAPI docs but never came across it json schema specification and docs, this means we are relying on the schema author's knowledge to generate proper messages. perhaps we could use both discriminator and Rules but that would increase complexity.

I am just stating what I think about using discriminator vs rules so that when a decision is made everything is considered.

[jdesrosiers]

Great discussion! Thank you!

We're getting into the case where the discriminator consists of multiple properties.

now Rule 2 will pass the alternative 1 and filter out the 2nd one, but the person wrote x: number constraint which is correct for 2nd alternative so we cant rule the alternative out after checking just one of the properties

Rule 2 would have "type" filter out alternative 1, but it would also have "x" filter out alternative 2. The result would be that all alternatives are filtered out and we fall back to showing all alternatives, which would be the right thing in this case.

I think we should stick to the explicit discriminator logic rather than relying on generalized rules.

I think Rule 2 ends up being equivalent to the discriminator logic I originally described. Please share any examples if you think of a place where it breaks down. Rule 1 I think is useful regardless.

discriminator is [...] an openAPI specific term

While there is an OpenAPI keyword called discriminator, the concept we're aiming for is discriminated unions like in TypeScript.

[srivastava-diya]

I think Rule 2 ends up being equivalent to the discriminator logic I originally described. Please share any examples if you think of a place where it breaks down. Rule 1 I think is useful regardless.

@jdesrosiers since you mentioned that we can fall back to the generic message if we are left with 0 alternatives after property by property filtering, it makes some sense to me now.

But i still have a doubt, please have a look at this example, its somewhat similar to what we all have discussed above,

{
  "oneOf": [
    {
      "type": "object",
      "properties": {
        "foo": { "const": "bar" },
        "age": { "type": "number" }
      }
    },
    {
      "type": "object",
      "properties": {
        "foo": { "const": "baz" },
        "age": { "type": "string" } 
      }
    },
  ]
}

Instance:

 {
   "foo": "bar",
   "age": "25" 
 }

This is an exact example which would filter out all our alternatives and we will fall back to the generic error message [currently our code does this itself ] , BUT i feel it is clear here that the user intended alternative 1 so I worry that falling back here doesn't do justice to the user's obvious intent, showing all alternatives will ultimately create more noise for the user.

[animeshsahoo1]

@srivastava-diya I think identifying user intent in cases like these is difficult. We can’t really determine what the user is thinking based on our assumptions. Even in your example and the one I mentioned above, there does seem to be some kind of hierarchical behavior where one alternative is probably what the user intended, but we can’t assume that with certainty, the user might have simply made a mistake. Instead, I think it’s more important that we present the correct information rather than unintentionally showing something the user did not actually intend. Considering that one of the main goals of this repository is that users should not have to look at the schema to understand what they should write in the instance, I think our approach here is correct.

[jdesrosiers]

I agree with @animeshsahoo1. I don't think there's a clear intention in that case. Each alternative has one property passing and one failing. Maybe you can say that matching a const is a stronger indication than matching a string, but I don't think those kinds of distinctions are enough to filter on.

[srivastava-diya]

Agree with you all, it is better to proceed in the proposed direction rather than worrying too much about the user's intention.