oneOf tests for success-caused failures

[RitoG09]

Added test cases for oneOf for now focusing on edge cases where validation fails due to multiple successful matches.
Related to issue: #120
Planning to add similar tests for not and contains after review.

[jdesrosiers]

Thanks. I think that covers the basics. I've converted this to a draft because we can't commit it until we have an implementation or it breaks the test automation.

[jdesrosiers]

This description doesn't match the test. I don't see any nesting.

[RitoG09]

I've updated the description and added a not with nested schema.

[jdesrosiers]

I see no difference between this test and the one before it. The not schemas are different, but there's nothing fundamentally different about what it's testing.

[RitoG09]

sorry for late reply I was travelling, yes this one and the previous one is fundamentally same except the not schema used here is required.

[jdesrosiers]

This one's not quite correct. The oneOf will fail in this example, which means not will pass and there should be no errors. You'd have to remove either "a" or "b" from the instance to get a failure.

My first thought was that I expected there to be a oneOf node in the output. But, then I realized that might not make sense in this case. Let's explore this case a bit more. Try writing out the error message and let's see if we can come up with something that's helpful and computable.

[RitoG09]

Yes, I got it. I should change the instance to { "a": 1 } which makes oneOf pass and not fail but I think the oneOf node in the output will be too abstract. not does not care about oneOf internal logic. It only cares that the subschema matched. Error message can be something like "The value matches a schema that it should not".

"errors": [
    {
      "messageId": "not-message",
      "instanceLocation": "#",
      "schemaLocations": ["#/not"],
      "alternatives": [
        [
          {
            "messageId": "required-success",
            "messageParams": { "property": "a" },
            "instanceLocation": "#",
            "schemaLocations": ["#/not/oneOf/0/required"]
          }
        ]
      ]
    }
  ]

[jdesrosiers]

Yes, I think I agree. If oneOf passes, it means one of the schemas passed and it's effectively a simple applicator in that case.

Error message can be something like "The value matches a schema that it should not".

We can't say "matches a schema". That's the kind of thing we're trying to avoid with this project. The reader shouldn't have to know anything about schemas to understand what's wrong with the data.

We can probably come up with a message for not that works ok, but I wonder if it would be better to provide a negated message for each keyword. For example, not-required would be something like The '{property}' property should not be present. It would probably make for a better experience, but it makes the handling even more complicated and we still don't really have a great solution for success messaging. Adding negation messaging might not be that simple.

[RitoG09]

Ok, How i am thinking about the negated message is ->

let keywords optionally define a negated form of their message at the localization level, rather than introducing a global success or failure mode flag.

In this case:
current format: The '{property}' property is required.
what we need: The '{property}' property should not be present

Then not wouldn’t need to understand the internal logic of required or oneOf it would just delegate to the inner keyword and request its negated form when available. If a keyword doesn’t define a negated message, we could fall back to the generic not message plus structured alternatives.

So we are avoiding: requiredErrorHandler(normalizedErrors, instance, localization, { negated: true })
(it introduces the boolean flag)
Rather we can do: getRequiredNegatedMessage(property)
(let localization decide)

I’m not sure if this fully avoids complexity, but it seems more localized than introducing parallel success or negated handlers everywhere.