Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handling of nulls when applying defaults #377

Closed
gtuccini opened this issue Mar 10, 2017 · 5 comments
Closed

Handling of nulls when applying defaults #377

gtuccini opened this issue Mar 10, 2017 · 5 comments

Comments

@gtuccini
Copy link

gtuccini commented Mar 10, 2017 •
edited
Loading

Currently null default values are not correctly applied. I think this happens because validateCommonProperties, in UndefinedConstraint.php, uses isset to check whether a default value has been defined and it cannot distinguish between a null default value and an undefined one.

// e.g.
if (!$this->getTypeCheck()->propertyExists($value, $i) && isset($propertyDefinition->default)) {

Moreover, when the subject is an array and default properties or items have been defined, non-null default values are applied even if the properties or the items have a null value. Again, I think this happens because the code uses isset to check whether the property or the item exists.

if (!isset($value[$i]) && isset($propertyDefinition->default)) {
// ...
if (!isset($value[$i]) && isset($itemDefinition->default)) {
@erayd
Copy link
Contributor

erayd commented Mar 10, 2017

@gtuccini Thanks for reporting this - will see if I can get this sorted over the weekend.

erayd added a commit to erayd/json-schema that referenced this issue Mar 10, 2017
@erayd
Handle nulls properly - fixes issue jsonrainbow#377
c92dfae
@erayd
Copy link
Contributor

erayd commented Mar 10, 2017

Fixed in c92dfae.

@erayd erayd mentioned this issue Mar 10, 2017
Merged
@gtuccini
Copy link
Author

That was fast! Thank you very much.

@erayd
Copy link
Contributor

erayd commented Mar 10, 2017

You're most welcome :-).

erayd added a commit to erayd/json-schema that referenced this issue Mar 17, 2017
@erayd
Handle nulls properly - fixes issue jsonrainbow#377
f63d938
bighappyface pushed a commit that referenced this issue Mar 21, 2017
@erayd @bighappyface
Fix infinite recursion on some schemas when setting defaults (#359) (#…
46d10ac 
…365)

* Don't try to fetch files that don't exist

Throws an exception when the ref can't be resolved to a useful file URI,
rather than waiting for something further down the line to fail after
the fact.

* Refactor defaults code to use LooseTypeCheck where appropriate

* Test for not treating non-containers like arrays

* Update comments

* Rename variable for clarity

* Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS

If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults
if they are marked as required.

* Workaround for $this scope issue on PHP-5.3

* Fix infinite recursion via $ref when applying defaults

* Add missing second test for array case

* Add test for setting a default value for null

* Also fix infinite recursion via $ref for array defaults

* Move nested closure into separate method

* $parentSchema will always be set when $name is, so don't check it

* Handle nulls properly - fixes issue #377
@erayd erayd mentioned this issue Mar 21, 2017
Merged
erayd added a commit to erayd/json-schema that referenced this issue Mar 21, 2017
@erayd
Fix infinite recursion on some schemas when setting defaults (jsonrai…
cabc3fa 
…nbow#359) (jsonrainbow#365)

* Don't try to fetch files that don't exist

Throws an exception when the ref can't be resolved to a useful file URI,
rather than waiting for something further down the line to fail after
the fact.

* Refactor defaults code to use LooseTypeCheck where appropriate

* Test for not treating non-containers like arrays

* Update comments

* Rename variable for clarity

* Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS

If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults
if they are marked as required.

* Workaround for $this scope issue on PHP-5.3

* Fix infinite recursion via $ref when applying defaults

* Add missing second test for array case

* Add test for setting a default value for null

* Also fix infinite recursion via $ref for array defaults

* Move nested closure into separate method

* $parentSchema will always be set when $name is, so don't check it

* Handle nulls properly - fixes issue jsonrainbow#377
bighappyface pushed a commit that referenced this issue Mar 22, 2017
@erayd @bighappyface
Backports for 5.2.0 (#368)
e3c9bcc 
* Add URI translation for retrieval & add local copies of spec schema

* Add use line for InvalidArgumentException & adjust scope (#372)

Fixes issue #371

* add quiet option (#382)

* add quiet option

* use verbose instead of quiet

* add quiet option

* always output dump-schema

* always output dump-schema-url

* fix typo and ws

* [BUGFIX] Add provided schema under a dummy / internal URI (fixes #376) (#378)

* Add provided schema under a dummy / internal URI (fixes #376)

In order to resolve internal $ref references within a user-provided
schema, SchemaStorage needs to know about the schema. As user-supplied
schemas do not have an associated URI, use a dummy / internal one instead.

* Remove dangling use

* Change URI to class constant on SchemaStorage

* Add option to disable validation of "format" constraint (#383)

* Add more unit tests (#366)

* Add test coverage for coercion API

* Complete test coverage for SchemaStorage

* Add test coverage for ObjectIterator

* Add exception test for JsonPointer

* MabeEnum\Enum appears to use singletons - add testing const

* Don't check this line for coverage

mbstring is on all test platforms, so this line will never be reached.

* Add test for TypeConstraint::validateTypeNameWording()

* Add test for exception on TypeConstraint::validateType()

* PHPunit doesn't like an explanation with its @codeCoverageIgnore...

* Add various tests for UriRetriever

* Add tests for FileGetContents

* Add tests for JsonSchema\Uri\Retrievers\Curl

* Add missing bad-syntax test file

* Restrict ignore to the exception line only

* Fix exception scope

* Allow the schema to be an associative array (#389)

* Allow the schema to be an associative array

Implements #388.

* Use json_decode(json_encode()) for array -> object cast

* Skip exception check on PHP versions < 5.5.0

* Skip test on HHVM, as it's happy to encode resources

* Enable FILTER_FLAG_EMAIL_UNICODE for email format if present (#398)

* Don't throw exceptions until after checking anyOf / oneOf (#394)

Fixes #393

* Fix infinite recursion on some schemas when setting defaults (#359) (#365)

* Don't try to fetch files that don't exist

Throws an exception when the ref can't be resolved to a useful file URI,
rather than waiting for something further down the line to fail after
the fact.

* Refactor defaults code to use LooseTypeCheck where appropriate

* Test for not treating non-containers like arrays

* Update comments

* Rename variable for clarity

* Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS

If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults
if they are marked as required.

* Workaround for $this scope issue on PHP-5.3

* Fix infinite recursion via $ref when applying defaults

* Add missing second test for array case

* Add test for setting a default value for null

* Also fix infinite recursion via $ref for array defaults

* Move nested closure into separate method

* $parentSchema will always be set when $name is, so don't check it

* Handle nulls properly - fixes issue #377

* Add option to also validate the schema (#357)

* Remove stale files from #357 (obviated by #362) (#400)

* Stop #386 sneaking in alongside another PR backport
erayd added a commit to erayd/json-schema that referenced this issue Mar 22, 2017
@erayd
Fix infinite recursion on some schemas when setting defaults (jsonrai…
ce1988f 
…nbow#359) (jsonrainbow#365)

* Don't try to fetch files that don't exist

Throws an exception when the ref can't be resolved to a useful file URI,
rather than waiting for something further down the line to fail after
the fact.

* Refactor defaults code to use LooseTypeCheck where appropriate

* Test for not treating non-containers like arrays

* Update comments

* Rename variable for clarity

* Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS

If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults
if they are marked as required.

* Workaround for $this scope issue on PHP-5.3

* Fix infinite recursion via $ref when applying defaults

* Add missing second test for array case

* Add test for setting a default value for null

* Also fix infinite recursion via $ref for array defaults

* Move nested closure into separate method

* $parentSchema will always be set when $name is, so don't check it

* Handle nulls properly - fixes issue jsonrainbow#377
@erayd
Copy link
Contributor

erayd commented Apr 11, 2017

@bighappyface Can this issue be closed? The fix is merged in both 6.0.0-dev and 5.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@erayd @bighappyface @gtuccini