Skip to content

Issue with "not" in geojson schema #566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
beschoenen opened this issue Feb 26, 2019 · 4 comments
Closed

Issue with "not" in geojson schema #566

beschoenen opened this issue Feb 26, 2019 · 4 comments

Comments

@beschoenen
Copy link

beschoenen commented Feb 26, 2019

I'm trying to validate a simple object against the geojson schema but it fails on the "not" in the crs property.

This is the test I wrote, which currently fails.

public function testValidateWithNonExistedPropertyUsingNot()
{
    $schema = json_decode(file_get_contents('http://json.schemastore.org/geojson'), true);
    $data = json_decode('{"type": "FeatureCollection", "features": []}', true);

    $validator = new Validator();
    $validator->validate($data, $schema);

    $this->assertTrue($validator->isValid());
}

It looks like it has something do to with "not" on properties that are not required/present in the data.

@erayd
Copy link
Contributor

erayd commented Feb 26, 2019

@beschoenen It looks like you're passing an associative array as both the schema and the data. Don't do this; it's incorrect - just pass it a normally decoded object and don't muck with the types. JSON has no native assoc type; an array is not a valid schema.

@erayd
Copy link
Contributor

erayd commented Feb 26, 2019

@beschoenen Despite the issue with your example I mentioned above regarding passing assoc arrays, it does look like there's something funky going on regarding that crs property. I'm investigating...

erayd added a commit to erayd/json-schema that referenced this issue Feb 26, 2019
Validation tests for an undefined instance should be limited to those
tests which are actually applicable (e.g. required, default). Tests
which assume an instance previously attempted to validate an internal
default when the instance is undefined, rather than ignoring it, which
is incorrect behavior.

Closes jsonrainbow#566.
erayd added a commit to erayd/json-schema that referenced this issue Feb 26, 2019
Validation tests for an undefined instance should be limited to those
tests which are actually applicable (e.g. required, default). Tests
which assume an instance previously attempted to validate an internal
default when the instance is undefined, rather than ignoring it, which
is incorrect behavior.

Closes jsonrainbow#566.
erayd added a commit to erayd/json-schema that referenced this issue Feb 26, 2019
Validation tests for an undefined instance should be limited to those
tests which are actually applicable (e.g. required, default). Tests
which assume an instance previously attempted to validate an internal
default when the instance is undefined, rather than ignoring it, which
is incorrect behavior.

Closes jsonrainbow#566.
@erayd
Copy link
Contributor

erayd commented Feb 26, 2019

@beschoenen Thanks for reporting this - it should be fixed in #567. I'll backport that into the 5.x.x branch once it's merged.

The root cause of the bug was attempting to run some validation logic on undefined (but schema-constrained) properties, when that logic requires a defined instance in order to make sense, and should never have been running against an undefined value at all.

@beschoenen
Copy link
Author

Thank you for taking the time to research this, greatly appreciated.

@erayd erayd closed this as completed in #567 Apr 2, 2019
erayd added a commit to erayd/json-schema that referenced this issue Sep 25, 2019
Validation tests for an undefined instance should be limited to those
tests which are actually applicable (e.g. required, default). Tests
which assume an instance previously attempted to validate an internal
default when the instance is undefined, rather than ignoring it, which
is incorrect behavior.

Closes jsonrainbow#566.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants