handling of 'oneOf' rules less than optimal

[karenetheridge]

• JSON::Validator version: 3.11
• Perl version: not relevant

When more than one 'oneOf' rule matches, the error is not correct. The following script demonstrates the issue:

use strict;
use warnings;
use JSON::Validator;

my $schema = {
    type => 'object',
    properties => {},
    additionalProperties => {
        oneOf => [
            { type => 'integer', minimum => 1 },
            { type => 'integer', minimum => 2 },
            { type => 'integer', maximum => 3 },
        ],
    },
};

my $jv = JSON::Validator->new;
$jv->load_and_validate_schema($schema, { schema => 'http://json-schema.org/draft-07/schema#' });

my $data = { foo => 1 };
my @errors = $jv->validate($data, $schema);
use Data::Dumper;
print STDERR "### got errors: ", Dumper(\@errors);

# got message: '/oneOf/1 1 < minimum(2)'
# but it should be: 'oneOf Multiple rules matched: /oneOf/0, /oneof/2.'

In the _validate_one_of sub, I think we should be tracking all the rules that do match, and then if @matched != 1, the path to all the matching rules can be easily provided in the error message. As it is now, only the failing rules are tracked, and this is used to construct the error message, but that is not helpful here.

[jhthorsen]

I don't have the time to implement this, but I'm open to a PR.

Just note that it has to handle deep and nested cases in a readable way. The test case submitted is good enough to illustrate the problem, but not good enough to get accepted in a PR.

[karenetheridge]

Do you mean that this is not a good test, or that there should be more tests than this? What sort of other things should be tested? are there existing tests that I can copy from that are similar?

[jhthorsen]

I'm saying it need to look more like https://github.com/mojolicious/json-validator/blob/master/t/issue-103-one-of.t and not like https://github.com/mojolicious/json-validator/blob/master/t/jv-oneof.t to make sure that the code handles nested oneOf/allOf/... so it's not just pretty for the simple cases.

[karenetheridge]

ok! I'll have a go at providing a fix (but probably not today).

[jhthorsen]

I'm going to close this, since #184 will get merged.