diff --git a/src/Core/Variables/JSONVariableProvider.php b/src/Core/Variables/JSONVariableProvider.php index a2077467e..72d14906b 100644 --- a/src/Core/Variables/JSONVariableProvider.php +++ b/src/Core/Variables/JSONVariableProvider.php @@ -96,7 +96,7 @@ protected function load() } else { $source = $this->source; } - $this->variables = json_decode($source, defined('JSON_OBJECT_AS_ARRAY') ? JSON_OBJECT_AS_ARRAY : 1); + parent::setSource(json_decode($source, defined('JSON_OBJECT_AS_ARRAY') ? JSON_OBJECT_AS_ARRAY : 1)); $this->lastLoaded = time(); } } diff --git a/src/Core/Variables/StandardVariableProvider.php b/src/Core/Variables/StandardVariableProvider.php index 0fe193000..871065e6c 100644 --- a/src/Core/Variables/StandardVariableProvider.php +++ b/src/Core/Variables/StandardVariableProvider.php @@ -31,7 +31,7 @@ class StandardVariableProvider implements VariableProviderInterface */ public function __construct(array $variables = []) { - $this->variables = $variables; + $this->setSource($variables); } /** @@ -57,7 +57,14 @@ public function getScopeCopy($variables) */ public function setSource($source) { - $this->variables = $source; + // Rather than assign $this->variables = $source we iterate in order to make sure that + // the logic within add() which is capable of storing nested variables, is used. In other + // words: $source can contain dotted-path keys which become a nested array structure or + // become overrides for values on objects. + $this->variables = []; + foreach ($source as $key => $value) { + $this->add($key, $value); + } } /** @@ -90,7 +97,64 @@ public function getAll() */ public function add($identifier, $value) { - $this->variables[$identifier] = $value; + if (strpos($identifier, '.') === false) { + $this->variables[$identifier] = $value; + } else { + $parts = explode('.', $identifier); + $root = array_shift($parts); + if (!isset($this->variables[$root])) { + $this->variables[$root] = []; + } + $subject = &$this->variables[$root]; + $propertyName = array_pop($parts); + $iterated = [$root]; + + $this->assertSubjectIsArrayOrObject($subject, $iterated, $identifier); + + foreach ($parts as $part) { + $iterated[] = $part; + if (is_array($subject) || $subject instanceof \ArrayAccess || $subject instanceof \ArrayObject) { + if (!isset($subject[$part])) { + $subject[$part] = []; + } + $subject = &$subject[$part]; + } elseif (is_object($subject)) { + $subject = $this->extractSingleValue($subject, $part); + } else { + $subject = null; + } + + $this->assertSubjectIsArrayOrObject($subject, $iterated, $identifier); + } + + // Assign the value on the $subject that is now a reference (either to somewhere in $this->variables + // or itself an object that is by nature a reference). + if (is_array($subject) || $subject instanceof \ArrayAccess || $subject instanceof \ArrayObject) { + $subject[$propertyName] = $value; + } elseif (is_object($subject)) { + $setterMethodName = 'set' . ucfirst($propertyName); + if (method_exists($subject, $setterMethodName)) { + $subject->$setterMethodName($value); + } else { + $subject->$propertyName = $value; + } + } + } + } + + protected function assertSubjectIsArrayOrObject($subject, array $segmentsUntilSubject, $originalPathToSet) + { + if (!(is_array($subject) || is_object($subject))) { + throw new \UnexpectedValueException( + sprintf( + 'Variable in path "%s" is scalar and is not the last segment in the full path "%s". ' . + 'Refusing to coerce value of parent segment - cannot assign variable.', + implode('.', $segmentsUntilSubject), + $originalPathToSet + ), + 1546878798 + ); + } } /** @@ -310,7 +374,7 @@ protected function canExtractWithAccessor($subject, $propertyName, $accessor) * @param string $accessor * @return mixed */ - protected function extractWithAccessor($subject, $propertyName, $accessor) + protected function extractWithAccessor(&$subject, $propertyName, $accessor) { if ($accessor === self::ACCESSOR_ARRAY && is_array($subject) && array_key_exists($propertyName, $subject) || $subject instanceof \ArrayAccess && $subject->offsetExists($propertyName) diff --git a/tests/Unit/Core/Variables/StandardVariableProviderTest.php b/tests/Unit/Core/Variables/StandardVariableProviderTest.php index b71448533..a69aef148 100644 --- a/tests/Unit/Core/Variables/StandardVariableProviderTest.php +++ b/tests/Unit/Core/Variables/StandardVariableProviderTest.php @@ -254,7 +254,7 @@ public function getAccessorsForPathTestValues() * @param string $accessor * @param mixed $expected * @test - * @dataProvider getExtractRedectAccessorTestValues + * @dataProvider getExtractRedetectsAccessorTestValues */ public function testExtractRedetectsAccessorIfUnusableAccessorPassed($subject, $path, $accessor, $expected) { @@ -266,7 +266,7 @@ public function testExtractRedetectsAccessorIfUnusableAccessorPassed($subject, $ /** * @return array */ - public function getExtractRedectAccessorTestValues() + public function getExtractRedetectsAccessorTestValues() { return [ [['test' => 'test'], 'test', null, 'test'], @@ -276,4 +276,68 @@ public function getExtractRedectAccessorTestValues() [['test' => 'test'], 'test', StandardVariableProvider::ACCESSOR_ASSERTER, 'test'], ]; } + + /** + * @param array $variables + * @param string $path + * @param mixed $value + * @test + * @dataProvider getAddWithDottedPathTestValues + */ + public function testAddWithDottedPath(array $variables, $path, $value) + { + $subject = new StandardVariableProvider($variables); + if ($path !== null) { + $subject->add($path, $value); + $this->assertSame($value, $subject->getByPath($path)); + } else { + $this->assertSame($value, $subject->getSource()); + } + } + + /** + * @return array + */ + public function getAddWithDottedPathTestValues() + { + $user = new UserWithoutToString('testuser'); + return [ + 'Plain string assigned into blank variables array' => [[], 'new.array', 'mystring'], + 'Plain string assigned into blank variables array with multiple dimensions' => [[], 'new.array.sub', 'mystring'], + 'Array built from dotted paths in original array' => [['dotted.one' => 1, 'dotted.two' => 2], null, ['dotted' => ['one' => 1, 'two' => 2]]], + 'Plain string assigned into existing variable' => ['foo' => ['bar' => 'test'], 'foo.bar', 'new'], + 'Property value assigned on object via setter' => [['parent' => $user], 'parent.name', 'newname'], + 'Property value assigned on object via public property' => [['parent' => $user], 'parent.newProperty', 'newValue'], + ]; + } + + /** + * @param array $variables + * @param string $path + * @param mixed $value + * @test + * @dataProvider getAddWithDottedPathThrowsErrorIfSubjectIsScalarTestValues + */ + public function testAddWithDottedPathThrowsErrorIfSubjectIsScalar(array $variables, $path) + { + $this->setExpectedException(\UnexpectedValueException::class, null, 1546878798); + $subject = new StandardVariableProvider($variables); + if ($path !== null) { + $subject->add($path, 'foo'); + } + } + + /** + * @return array + */ + public function getAddWithDottedPathThrowsErrorIfSubjectIsScalarTestValues() + { + $user = new UserWithoutToString('testuser'); + return [ + 'Invalid property on object added after source' => [['user' => $user], 'user.doesnotexist.sub', 'value'], + 'Invalid property on object added in source' => [['user' => $user, 'user.doesnotexist.sub' => 'value'], null, null], + 'Scalar property on object used as array/object' => [['user' => $user, 'user.name.sub' => 'value'], null, null], + 'Scalar property on object used as array/object, additional dimension' => [['user' => $user, 'user.name.sub.moresub' => 'value'], null, null], + ]; + } } diff --git a/tests/Unit/ViewHelpers/Fixtures/UserWithoutToString.php b/tests/Unit/ViewHelpers/Fixtures/UserWithoutToString.php index a3f8ff939..26e331c01 100644 --- a/tests/Unit/ViewHelpers/Fixtures/UserWithoutToString.php +++ b/tests/Unit/ViewHelpers/Fixtures/UserWithoutToString.php @@ -25,6 +25,14 @@ public function __construct($name) $this->name = $name; } + /** + * @param string $name + */ + public function setName($name) + { + $this->name = $name; + } + /** * @return string */