Skip to content

Commit

Permalink
Handle nulls properly - fixes issue #377
Browse files Browse the repository at this point in the history
  • Loading branch information
@erayd
erayd committed Mar 10, 2017
1 parent 8af9398 commit c92dfae
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 28 deletions.
10 changes: 5 additions & 5 deletions src/JsonSchema/Constraints/UndefinedConstraint.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ protected function applyDefaultValues(&$value, $schema, $path)
foreach ($schema->properties as $currentProperty => $propertyDefinition) {
if (
!LooseTypeCheck::propertyExists($value, $currentProperty)
&& isset($propertyDefinition->default)
&& property_exists($propertyDefinition, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $propertyDefinition, $currentProperty, $schema)
) {
// assign default value
Expand All @@ -255,8 +255,8 @@ protected function applyDefaultValues(&$value, $schema, $path)
// $value is an array, and items are defined - treat as plain array
foreach ($schema->items as $currentItem => $itemDefinition) {
if (
!isset($value[$currentItem])
&& isset($itemDefinition->default)
!array_key_exists($currentItem, $value)
&& property_exists($itemDefinition, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $itemDefinition)) {
if (is_object($itemDefinition->default)) {
$value[$currentItem] = clone $itemDefinition->default;
Expand All @@ -267,8 +267,8 @@ protected function applyDefaultValues(&$value, $schema, $path)
$path->setFromDefault();
}
} elseif (
($value instanceof self || $value === null)
&& isset($schema->default)
$value instanceof self
&& property_exists($schema, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $schema)) {
// $value is a leaf, not a container - apply the default directly
$value = is_object($schema->default) ? clone $schema->default : $schema->default;
Expand Down
68 changes: 45 additions & 23 deletions tests/Constraints/DefaultPropertiesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,77 +19,84 @@ class DefaultPropertiesTest extends VeryBaseTestCase
public function getValidTests()
{
return array(
/*
// This test case was intended to check whether a default value can be applied for the
// entire object, however testing this case is impossible, because there is no way to
// distinguish between a deliberate top-level NULL and a top level that contains nothing.
// As such, the assumption is that a top-level NULL is deliberate, and should not be
// altered by replacing it with a default value.
array(// #0 default value for entire object
'',
'{"default":"valueOne"}',
'"valueOne"'
),
array(// #1 default value in an empty object
*/
array(// #0 default value in an empty object
'{}',
'{"properties":{"propertyOne":{"default":"valueOne"}}}',
'{"propertyOne":"valueOne"}'
),
array(// #2 default value for top-level property
array(// #1 default value for top-level property
'{"propertyOne":"valueOne"}',
'{"properties":{"propertyTwo":{"default":"valueTwo"}}}',
'{"propertyOne":"valueOne","propertyTwo":"valueTwo"}'
),
array(// #3 default value for sub-property
array(// #2 default value for sub-property
'{"propertyOne":{}}',
'{"properties":{"propertyOne":{"properties":{"propertyTwo":{"default":"valueTwo"}}}}}',
'{"propertyOne":{"propertyTwo":"valueTwo"}}'
),
array(// #4 default value for sub-property with sibling
array(// #3 default value for sub-property with sibling
'{"propertyOne":{"propertyTwo":"valueTwo"}}',
'{"properties":{"propertyOne":{"properties":{"propertyThree":{"default":"valueThree"}}}}}',
'{"propertyOne":{"propertyTwo":"valueTwo","propertyThree":"valueThree"}}'
),
array(// #5 default value for top-level property with type check
array(// #4 default value for top-level property with type check
'{"propertyOne":"valueOne"}',
'{"properties":{"propertyTwo":{"default":"valueTwo","type":"string"}}}',
'{"propertyOne":"valueOne","propertyTwo":"valueTwo"}'
),
array(// #6 default value for top-level property with v3 required check
array(// #5 default value for top-level property with v3 required check
'{"propertyOne":"valueOne"}',
'{"properties":{"propertyTwo":{"default":"valueTwo","required":"true"}}}',
'{"propertyOne":"valueOne","propertyTwo":"valueTwo"}'
),
array(// #7 default value for top-level property with v4 required check
array(// #6 default value for top-level property with v4 required check
'{"propertyOne":"valueOne"}',
'{"properties":{"propertyTwo":{"default":"valueTwo"}},"required":["propertyTwo"]}',
'{"propertyOne":"valueOne","propertyTwo":"valueTwo"}'
),
array(// #8 default value for an already set property
array(// #7 default value for an already set property
'{"propertyOne":"alreadySetValueOne"}',
'{"properties":{"propertyOne":{"default":"valueOne"}}}',
'{"propertyOne":"alreadySetValueOne"}'
),
array(// #9 default item value for an array
array(// #8 default item value for an array
'["valueOne"]',
'{"type":"array","items":[{},{"type":"string","default":"valueTwo"}]}',
'["valueOne","valueTwo"]'
),
array(// #10 default item value for an empty array
array(// #9 default item value for an empty array
'[]',
'{"type":"array","items":[{"type":"string","default":"valueOne"}]}',
'["valueOne"]'
),
array(// #11 property without a default available
array(// #10 property without a default available
'{"propertyOne":"alreadySetValueOne"}',
'{"properties":{"propertyOne":{"type":"string"}}}',
'{"propertyOne":"alreadySetValueOne"}'
),
array(// #12 default property value is an object
array(// #11 default property value is an object
'{"propertyOne":"valueOne"}',
'{"properties":{"propertyTwo":{"default":{}}}}',
'{"propertyOne":"valueOne","propertyTwo":{}}'
),
array(// #13 default item value is an object
array(// #12 default item value is an object
'[]',
'{"type":"array","items":[{"default":{}}]}',
'[{}]'
),
array(// #14 only set required values (draft-04)
array(// #13 only set required values (draft-04)
'{}',
'{
"properties": {
Expand All @@ -101,7 +108,7 @@ public function getValidTests()
'{"propertyTwo":"valueTwo"}',
Constraint::CHECK_MODE_ONLY_REQUIRED_DEFAULTS
),
array(// #15 only set required values (draft-03)
array(// #14 only set required values (draft-03)
'{}',
'{
"properties": {
Expand All @@ -112,21 +119,36 @@ public function getValidTests()
'{"propertyTwo":"valueTwo"}',
Constraint::CHECK_MODE_ONLY_REQUIRED_DEFAULTS
),
array(// #16 infinite recursion via $ref (object)
array(// #15 infinite recursion via $ref (object)
'{}',
'{"properties":{"propertyOne": {"$ref": "#","default": {}}}}',
'{"propertyOne":{}}'
),
array(// #17 infinite recursion via $ref (array)
array(// #16 infinite recursion via $ref (array)
'[]',
'{"items":[{"$ref":"#","default":[]}]}',
'[[]]'
),
array(// #18 default value for null
array(// #17 default top value does not overwrite defined null
'null',
'{"default":"valueOne"}',
'"valueOne"'
)
'null'
),
array(// #18 default property value does not overwrite defined null
'{"propertyOne":null}',
'{"properties":{"propertyOne":{"default":"valueOne"}}}',
'{"propertyOne":null}'
),
array(// #19 default value in an object is null
'{}',
'{"properties":{"propertyOne":{"default":null}}}',
'{"propertyOne":null}'
),
array(// #20 default value in an array is null
'[]',
'{"items":[{"default":null}]}',
'[null]'
),
);
}

Expand Down Expand Up @@ -180,16 +202,16 @@ public function testValidCasesUsingAssocWithoutTypeCast($input, $schema, $expect

public function testNoModificationViaReferences()
{
$input = json_decode('');
$schema = json_decode('{"default":{"propertyOne":"valueOne"}}');
$input = json_decode('{}');
$schema = json_decode('{"properties":{"propertyOne":{"default":"valueOne"}}}');

$validator = new Validator();
$validator->validate($input, $schema, Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_APPLY_DEFAULTS);

$this->assertEquals('{"propertyOne":"valueOne"}', json_encode($input));

$input->propertyOne = 'valueTwo';
$this->assertEquals('valueOne', $schema->default->propertyOne);
$this->assertEquals('valueOne', $schema->properties->propertyOne->default);
}

public function testLeaveBasicTypesAlone()
Expand Down

0 comments on commit c92dfae

Please sign in to comment.