jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 58 forks source link

'allOf' is ignored in the presence of 'not' #218

Closed blackwinter closed 4 years ago

blackwinter commented 4 years ago

First of all, great project! Really glad we can use it to our advantage :)

Steps to reproduce the behavior

When using not in combination with allOf, the constraints specified in the latter are ignored. Only after moving the not constraint to allOf does it validate as intended.

use lib '.';
use t::Helper;

my $missing = E('/required', '/allOf/0 Missing property.');

# Property `required` must be present.
my $schema = {type => 'object', allOf => [{required => ['required']}]}; 

my @tests = (
  [{foo => 1, required  => 2}, $schema],
  [{foo => 1, forbidden => 3}, $schema, $missing],
  [{foo => 1, required => 2, forbidden => 3}, $schema],
  [{foo => 1}, $schema, $missing]
);  

validate_ok @$_ foreach @tests;

# Property `forbidden` must not be present.
$schema->{not} = {required => ['forbidden']};
$tests[1]->[2] = $tests[2]->[2] = E('/', 'Should not match.');

# Test 4 fails: the fact that the required property
# is missing does not lead to a validation error.
validate_ok @$_ foreach @tests; 

# Move `not` constraint to `allOf`.
push(@{$schema->{allOf}}, {not => delete($schema->{not})});
$tests[1]->[2] = $tests[2]->[2] = E('/', '/allOf/1 Should not match.');
$tests[1]->[3] = $missing;

# Test 4 passes: now all constraints are enforced as intended.
validate_ok @$_ foreach @tests; 

done_testing;

Expected behavior

Both constraints should be enforced:

  1. The property required must be present.
  2. The property forbidden must not be present.

The assumption being that any subschema acts as an implict allOf.

Actual behavior

The first constraint is ignored: no validation error is reported for the missing property.

$ prove -l t/allof-with-not.t
t/allof-with-not.t .. 1/? 
#   Failed test 'errors: /required: /allOf/0 Missing property.'
#   at t/allof-with-not.t line 24.
#     Structures begin differing at:
#          $got->[0] = Does not exist
#     $expected->[0] = HASH(0x5597b85e9ed8)
# []
# Looks like you failed 1 test of 12.
t/allof-with-not.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/12 subtests 

Test Summary Report
-------------------
t/allof-with-not.t (Wstat: 256 Tests: 12 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=1, Tests=12,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.25 cusr  0.03 csys =  0.30 CPU)
Result: FAIL
jhthorsen commented 4 years ago

Thanks for the failing test! Made it a whole lot easier to fix the issue ❤️

blackwinter commented 4 years ago

Thank you for fixing it so quickly :) We've got a workaround for now, but gladly jump on the new version once it's released.