From 173d9ca4d773cea4827fc3f448766a9c6b2f1e36 Mon Sep 17 00:00:00 2001 From: Niels Braczek Date: Mon, 11 Oct 2021 16:11:06 +0200 Subject: [PATCH] Fix - Return previous value when setting a new value (consistency). Replaces #49 --- Tests/RegistryTest.php | 18 +-- composer.json | 79 +++++++------ src/Registry.php | 262 +++++++++++++++++------------------------ 3 files changed, 159 insertions(+), 200 deletions(-) diff --git a/Tests/RegistryTest.php b/Tests/RegistryTest.php index e461568b..23f7dbb8 100644 --- a/Tests/RegistryTest.php +++ b/Tests/RegistryTest.php @@ -64,18 +64,21 @@ public function testARegistryInstanceInstantiatedWithAStringOfDataIsCorrectlyMan // Check top level values $this->assertSame('bar', $a->get('foo')); $this->assertSame('bar', $a->def('foo')); - $this->assertSame('far', $a->set('foo', 'far')); + $this->assertSame('bar', $a->set('foo', 'far')); + $this->assertSame('far', $a->get('foo')); // Check nested values $this->assertSame('bar', $a->get('nested.foo')); $this->assertSame('bar', $a->def('nested.foo')); - $this->assertSame('far', $a->set('nested.foo', 'far')); + $this->assertSame('bar', $a->set('nested.foo', 'far')); + $this->assertSame('far', $a->get('nested.foo')); // Check adding a new nested object $a->set('new.nested', ['foo' => 'bar', 'goo' => 'car']); $this->assertSame('bar', $a->get('new.nested.foo')); $this->assertSame('bar', $a->def('new.nested.foo')); - $this->assertSame('far', $a->set('new.nested.foo', 'far')); + $this->assertSame('bar', $a->set('new.nested.foo', 'far')); + $this->assertSame('far', $a->get('new.nested.foo')); } /** @@ -491,15 +494,14 @@ public function testAValueIsStoredToTheRegistry() { $a = new Registry; - $this->assertSame( - 'testsetvalue1', + $this->assertNull( $a->set('foo', 'testsetvalue1'), - 'The current value should be returned when assigning a key for the first time.' + 'Null should be returned when assigning a key for the first time.' ); $this->assertSame( - 'testsetvalue2', + 'testsetvalue1', $a->set('foo', 'testsetvalue2'), - 'The new value should be returned when assigning a key multiple times.' + 'The previous value should be returned when assigning a key multiple times.' ); } diff --git a/composer.json b/composer.json index 2a93c6a7..d8234360 100644 --- a/composer.json +++ b/composer.json @@ -1,39 +1,44 @@ { - "name": "joomla/registry", - "type": "joomla-package", - "description": "Joomla Registry Package", - "keywords": ["joomla", "framework", "registry"], - "homepage": "https://github.com/joomla-framework/registry", - "license": "GPL-2.0-or-later", - "require": { - "php": "^7.2.5", - "joomla/utilities": "^1.4.1|^2.0" - }, - "require-dev": { - "joomla/coding-standards": "^3.0@dev", - "symfony/yaml": "^3.4|^4.4|^5.0", - "phpunit/phpunit": "^8.5|^9.0" - }, - "conflict": { - "symfony/yaml": "<3.4" - }, - "suggest": { - "symfony/yaml": "Install symfony/yaml if you require YAML support." - }, - "autoload": { - "psr-4": { - "Joomla\\Registry\\": "src/" - } - }, - "autoload-dev": { - "psr-4": { - "Joomla\\Registry\\Tests\\": "Tests/" - } - }, - "minimum-stability": "dev", - "extra": { - "branch-alias": { - "dev-2.0-dev": "2.0-dev" - } - } + "name": "joomla/registry", + "type": "joomla-package", + "description": "Joomla Registry Package", + "keywords": [ + "joomla", + "framework", + "registry" + ], + "homepage": "https://github.com/joomla-framework/registry", + "license": "GPL-2.0-or-later", + "require": { + "php": "^7.2.5", + "joomla/utilities": "^1.4.1|^2.0" + }, + "require-dev": { + "ext-json": "*", + "joomla/coding-standards": "^3.0@dev", + "symfony/yaml": "^3.4|^4.4|^5.0", + "phpunit/phpunit": "^8.5|^9.0" + }, + "conflict": { + "symfony/yaml": "<3.4" + }, + "suggest": { + "symfony/yaml": "Install symfony/yaml if you require YAML support." + }, + "autoload": { + "psr-4": { + "Joomla\\Registry\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "Joomla\\Registry\\Tests\\": "Tests/" + } + }, + "minimum-stability": "dev", + "extra": { + "branch-alias": { + "dev-2.0-dev": "2.0-dev" + } + } } diff --git a/src/Registry.php b/src/Registry.php index 272ec343..7ba5dc16 100644 --- a/src/Registry.php +++ b/src/Registry.php @@ -44,7 +44,7 @@ class Registry implements \JsonSerializable, \ArrayAccess, \IteratorAggregate, \ /** * Constructor * - * @param mixed $data The data to bind to the new Registry object. + * @param mixed $data The data to bind to the new Registry object. * * @since 1.0 */ @@ -54,16 +54,11 @@ public function __construct($data = null) $this->data = new \stdClass; // Optionally load supplied data. - if ($data instanceof self) - { + if ($data instanceof self) { $this->merge($data); - } - elseif (\is_array($data) || \is_object($data)) - { + } elseif (\is_array($data) || \is_object($data)) { $this->bindData($this->data, $data); - } - elseif (!empty($data) && \is_string($data)) - { + } elseif (!empty($data) && \is_string($data)) { $this->loadString($data); } } @@ -122,8 +117,8 @@ public function jsonSerialize() /** * Sets a default value if not already assigned. * - * @param string $key The name of the parameter. - * @param mixed $default An optional value for the parameter. + * @param string $key The name of the parameter. + * @param mixed $default An optional value for the parameter. * * @return mixed The value set, or the default if the value was not previously set (or null). * @@ -140,7 +135,7 @@ public function def($key, $default = '') /** * Check if a registry path exists. * - * @param string $path Registry path (e.g. joomla.content.showauthor) + * @param string $path Registry path (e.g. joomla.content.showauthor) * * @return boolean * @@ -149,8 +144,7 @@ public function def($key, $default = '') public function exists($path) { // Return default value if path is empty - if (empty($path)) - { + if (empty($path)) { return false; } @@ -162,18 +156,15 @@ public function exists($path) $found = false; // Traverse the registry to find the correct node for the result. - foreach ($nodes as $n) - { - if (\is_array($node) && isset($node[$n])) - { + foreach ($nodes as $n) { + if (\is_array($node) && isset($node[$n])) { $node = $node[$n]; $found = true; continue; } - if (!isset($node->$n)) - { + if (!isset($node->$n)) { return false; } @@ -187,8 +178,8 @@ public function exists($path) /** * Get a registry value. * - * @param string $path Registry path (e.g. joomla.content.showauthor) - * @param mixed $default Optional default value, returned if the internal value is null. + * @param string $path Registry path (e.g. joomla.content.showauthor) + * @param mixed $default Optional default value, returned if the internal value is null. * * @return mixed Value of entry or null * @@ -197,14 +188,13 @@ public function exists($path) public function get($path, $default = null) { // Return default value if path is empty - if (empty($path)) - { + if (empty($path)) { return $default; } - if (!strpos($path, $this->separator)) - { - return (isset($this->data->$path) && $this->data->$path !== null && $this->data->$path !== '') ? $this->data->$path : $default; + if (!strpos($path, $this->separator)) { + return (isset($this->data->$path) && $this->data->$path !== null && $this->data->$path !== '') + ? $this->data->$path : $default; } // Explode the registry path into an array @@ -215,18 +205,15 @@ public function get($path, $default = null) $found = false; // Traverse the registry to find the correct node for the result. - foreach ($nodes as $n) - { - if (\is_array($node) && isset($node[$n])) - { + foreach ($nodes as $n) { + if (\is_array($node) && isset($node[$n])) { $node = $node[$n]; $found = true; continue; } - if (!isset($node->$n)) - { + if (!isset($node->$n)) { return $default; } @@ -234,8 +221,7 @@ public function get($path, $default = null) $found = true; } - if (!$found || $node === null || $node === '') - { + if (!$found || $node === null || $node === '') { return $default; } @@ -260,9 +246,9 @@ public function getIterator() /** * Load an associative array of values into the default namespace * - * @param array $array Associative array of value to load - * @param boolean $flattened Load from a one-dimensional array - * @param string $separator The key separator + * @param array $array Associative array of value to load + * @param boolean $flattened Load from a one-dimensional array + * @param string $separator The key separator * * @return $this * @@ -270,15 +256,13 @@ public function getIterator() */ public function loadArray(array $array, $flattened = false, $separator = null) { - if (!$flattened) - { + if (!$flattened) { $this->bindData($this->data, $array); return $this; } - foreach ($array as $k => $v) - { + foreach ($array as $k => $v) { $this->set($k, $v, $separator); } @@ -288,7 +272,7 @@ public function loadArray(array $array, $flattened = false, $separator = null) /** * Load the public variables of the object into the default namespace. * - * @param object $object The object holding the publics to load + * @param object $object The object holding the publics to load * * @return $this * @@ -304,9 +288,9 @@ public function loadObject($object) /** * Load the contents of a file into the registry * - * @param string $file Path to file to load - * @param string $format Format of the file [optional: defaults to JSON] - * @param array $options Options used by the formatter + * @param string $file Path to file to load + * @param string $format Format of the file [optional: defaults to JSON] + * @param array $options Options used by the formatter * * @return $this * @@ -322,9 +306,9 @@ public function loadFile($file, $format = 'JSON', array $options = []) /** * Load a string into the registry * - * @param string $data String to load into the registry - * @param string $format Format of the string - * @param array $options Options used by the formatter + * @param string $data String to load into the registry + * @param string $format Format of the string + * @param array $options Options used by the formatter * * @return $this * @@ -336,8 +320,7 @@ public function loadString($data, $format = 'JSON', array $options = []) $obj = Factory::getFormat($format, $options)->stringToObject($data, $options); // If the data object has not yet been initialized, direct assign the object - if (!$this->initialized) - { + if (!$this->initialized) { $this->data = $obj; $this->initialized = true; @@ -352,8 +335,8 @@ public function loadString($data, $format = 'JSON', array $options = []) /** * Merge a Registry object into this one * - * @param Registry $source Source Registry object to merge. - * @param boolean $recursive True to support recursive merge the children values. + * @param Registry $source Source Registry object to merge. + * @param boolean $recursive True to support recursive merge the children values. * * @return $this * @@ -369,7 +352,7 @@ public function merge(Registry $source, $recursive = false) /** * Method to extract a sub-registry from path * - * @param string $path Registry path (e.g. joomla.content.showauthor) + * @param string $path Registry path (e.g. joomla.content.showauthor) * * @return Registry Registry object (empty if no data is present) * @@ -385,7 +368,7 @@ public function extract($path) /** * Checks whether an offset exists in the iterator. * - * @param mixed $offset The array offset. + * @param mixed $offset The array offset. * * @return boolean True if the offset exists, false otherwise. * @@ -399,7 +382,7 @@ public function offsetExists($offset) /** * Gets an offset in the iterator. * - * @param mixed $offset The array offset. + * @param mixed $offset The array offset. * * @return mixed The array value if it exists, null otherwise. * @@ -413,8 +396,8 @@ public function offsetGet($offset) /** * Sets an offset in the iterator. * - * @param mixed $offset The array offset. - * @param mixed $value The array value. + * @param mixed $offset The array offset. + * @param mixed $value The array value. * * @return void * @@ -428,7 +411,7 @@ public function offsetSet($offset, $value) /** * Unsets an offset in the iterator. * - * @param mixed $offset The array offset. + * @param mixed $offset The array offset. * * @return void * @@ -442,18 +425,17 @@ public function offsetUnset($offset) /** * Set a registry value. * - * @param string $path Registry Path (e.g. joomla.content.showauthor) - * @param mixed $value Value of entry - * @param string $separator The key separator + * @param string $path Registry Path (e.g. joomla.content.showauthor) + * @param mixed $value Value of entry + * @param string $separator The key separator * - * @return mixed The value of the that has been set. + * @return mixed The previous value of the entry that has been set. * * @since 1.0 */ public function set($path, $value, $separator = null) { - if (empty($separator)) - { + if (empty($separator)) { $separator = $this->separator; } @@ -464,8 +446,7 @@ public function set($path, $value, $separator = null) */ $nodes = array_values(array_filter(explode($separator, $path), 'strlen')); - if (!$nodes) - { + if (!$nodes) { return; } @@ -473,12 +454,9 @@ public function set($path, $value, $separator = null) $node = $this->data; // Traverse the registry to find the correct node for the result. - for ($i = 0, $n = \count($nodes) - 1; $i < $n; $i++) - { - if (\is_object($node)) - { - if (!isset($node->{$nodes[$i]}) && ($i !== $n)) - { + for ($i = 0, $n = \count($nodes) - 1; $i < $n; $i++) { + if (\is_object($node)) { + if (!isset($node->{$nodes[$i]}) && ($i !== $n)) { $node->{$nodes[$i]} = new \stdClass; } @@ -488,10 +466,8 @@ public function set($path, $value, $separator = null) continue; } - if (\is_array($node)) - { - if (($i !== $n) && !isset($node[$nodes[$i]])) - { + if (\is_array($node)) { + if (($i !== $n) && !isset($node[$nodes[$i]])) { $node[$nodes[$i]] = new \stdClass; } @@ -500,16 +476,17 @@ public function set($path, $value, $separator = null) } } - // Get the old value if exists so we can return it - switch (true) - { + // Get the old value if exists, so we can return it + switch (true) { case \is_object($node): - $result = $node->{$nodes[$i]} = $value; + $result = $node->{$nodes[$i]} ?? null; + $node->{$nodes[$i]} = $value; break; case \is_array($node): - $result = $node[$nodes[$i]] = $value; + $result = $node[$nodes[$i]] ?? null; + $node[$nodes[$i]] = $value; break; @@ -525,8 +502,8 @@ public function set($path, $value, $separator = null) /** * Append value to a path in registry * - * @param string $path Parent registry Path (e.g. joomla.content.showauthor) - * @param mixed $value Value of entry + * @param string $path Parent registry Path (e.g. joomla.content.showauthor) + * @param mixed $value Value of entry * * @return mixed The value of the that has been set. * @@ -543,29 +520,22 @@ public function append($path, $value) */ $nodes = array_values(array_filter(explode('.', $path), 'strlen')); - if ($nodes) - { + if ($nodes) { // Initialize the current node to be the registry root. $node = $this->data; // Traverse the registry to find the correct node for the result. // TODO Create a new private method from part of code below, as it is almost equal to 'set' method - for ($i = 0, $n = \count($nodes) - 1; $i <= $n; $i++) - { - if (\is_object($node)) - { - if (!isset($node->{$nodes[$i]}) && ($i !== $n)) - { + for ($i = 0, $n = \count($nodes) - 1; $i <= $n; $i++) { + if (\is_object($node)) { + if (!isset($node->{$nodes[$i]}) && ($i !== $n)) { $node->{$nodes[$i]} = new \stdClass; } // Pass the child as pointer in case it is an array $node = &$node->{$nodes[$i]}; - } - elseif (\is_array($node)) - { - if (($i !== $n) && !isset($node[$nodes[$i]])) - { + } elseif (\is_array($node)) { + if (($i !== $n) && !isset($node[$nodes[$i]])) { $node[$nodes[$i]] = new \stdClass; } @@ -574,8 +544,7 @@ public function append($path, $value) } } - if (!\is_array($node)) - { + if (!\is_array($node)) { // Convert the node to array to make append possible $node = get_object_vars($node); } @@ -590,7 +559,7 @@ public function append($path, $value) /** * Delete a registry value * - * @param string $path Registry Path (e.g. joomla.content.showauthor) + * @param string $path Registry Path (e.g. joomla.content.showauthor) * * @return mixed The value of the removed node or null if not set * @@ -599,9 +568,9 @@ public function append($path, $value) public function remove($path) { // Cheap optimisation to direct remove the node if there is no separator - if (!strpos($path, $this->separator)) - { - $result = (isset($this->data->$path) && $this->data->$path !== null && $this->data->$path !== '') ? $this->data->$path : null; + if (!strpos($path, $this->separator)) { + $result = (isset($this->data->$path) && $this->data->$path !== null && $this->data->$path !== '') + ? $this->data->$path : null; unset($this->data->$path); @@ -615,8 +584,7 @@ public function remove($path) */ $nodes = array_values(array_filter(explode($this->separator, $path), 'strlen')); - if (!$nodes) - { + if (!$nodes) { return; } @@ -625,12 +593,9 @@ public function remove($path) $parent = null; // Traverse the registry to find the correct node for the result. - for ($i = 0, $n = \count($nodes) - 1; $i < $n; $i++) - { - if (\is_object($node)) - { - if (!isset($node->{$nodes[$i]}) && ($i !== $n)) - { + for ($i = 0, $n = \count($nodes) - 1; $i < $n; $i++) { + if (\is_object($node)) { + if (!isset($node->{$nodes[$i]}) && ($i !== $n)) { continue; } @@ -640,10 +605,8 @@ public function remove($path) continue; } - if (\is_array($node)) - { - if (($i !== $n) && !isset($node[$nodes[$i]])) - { + if (\is_array($node)) { + if (($i !== $n) && !isset($node[$nodes[$i]])) { continue; } @@ -655,8 +618,7 @@ public function remove($path) } // Get the old value if exists so we can return it - switch (true) - { + switch (true) { case \is_object($node): $result = $node->{$nodes[$i]} ?? null; unset($parent->{$nodes[$i]}); @@ -687,7 +649,7 @@ public function remove($path) */ public function toArray() { - return (array) $this->asArray($this->data); + return (array)$this->asArray($this->data); } /** @@ -705,8 +667,8 @@ public function toObject() /** * Get a namespace in a given string format * - * @param string $format Format to return the string in - * @param array $options Parameters used by the formatter, see formatters for more info + * @param string $format Format to return the string in + * @param array $options Parameters used by the formatter, see formatters for more info * * @return string Namespace in string format * @@ -714,16 +676,17 @@ public function toObject() */ public function toString($format = 'JSON', array $options = []) { - return Factory::getFormat($format, $options)->objectToString($this->data, $options); + return Factory::getFormat($format, $options)->objectToString($this->data, $options) + ; } /** * Method to recursively bind data to a parent object. * - * @param object $parent The parent object on which to attach the data values. - * @param mixed $data An array or object of data to bind to the parent object. - * @param boolean $recursive True to support recursive bindData. - * @param boolean $allowNull True to allow null values. + * @param object $parent The parent object on which to attach the data values. + * @param mixed $data An array or object of data to bind to the parent object. + * @param boolean $recursive True to support recursive bindData. + * @param boolean $allowNull True to allow null values. * * @return void * @@ -735,19 +698,15 @@ protected function bindData($parent, $data, $recursive = true, $allowNull = true $this->initialized = true; // Ensure the input data is an array. - $data = \is_object($data) ? get_object_vars($data) : (array) $data; + $data = \is_object($data) ? get_object_vars($data) : (array)$data; - foreach ($data as $k => $v) - { - if (!$allowNull && !(($v !== null) && ($v !== ''))) - { + foreach ($data as $k => $v) { + if (!$allowNull && !(($v !== null) && ($v !== ''))) { continue; } - if ($recursive && ((\is_array($v) && ArrayHelper::isAssociative($v)) || \is_object($v))) - { - if (!isset($parent->$k)) - { + if ($recursive && ((\is_array($v) && ArrayHelper::isAssociative($v)) || \is_object($v))) { + if (!isset($parent->$k)) { $parent->$k = new \stdClass; } @@ -763,7 +722,7 @@ protected function bindData($parent, $data, $recursive = true, $allowNull = true /** * Method to recursively convert an object of data to an array. * - * @param object $data An object of data to return as an array. + * @param object $data An object of data to return as an array. * * @return array Array representation of the input object. * @@ -773,15 +732,12 @@ protected function asArray($data) { $array = []; - if (\is_object($data)) - { + if (\is_object($data)) { $data = get_object_vars($data); } - foreach ($data as $k => $v) - { - if (\is_object($v) || \is_array($v)) - { + foreach ($data as $k => $v) { + if (\is_object($v) || \is_array($v)) { $array[$k] = $this->asArray($v); continue; @@ -796,7 +752,7 @@ protected function asArray($data) /** * Dump to one dimension array. * - * @param string $separator The key separator. + * @param string $separator The key separator. * * @return string[] Dumped array. * @@ -806,8 +762,7 @@ public function flatten($separator = null) { $array = []; - if (empty($separator)) - { + if (empty($separator)) { $separator = $this->separator; } @@ -819,10 +774,10 @@ public function flatten($separator = null) /** * Method to recursively convert data to one dimension array. * - * @param string $separator The key separator. - * @param array|object $data Data source of this scope. - * @param array $array The result array, it is passed by reference. - * @param string $prefix Last level key prefix. + * @param string $separator The key separator. + * @param array|object $data Data source of this scope. + * @param array $array The result array, it is passed by reference. + * @param string $prefix Last level key prefix. * * @return void * @@ -830,19 +785,16 @@ public function flatten($separator = null) */ protected function toFlatten($separator = null, $data = null, array &$array = [], $prefix = '') { - $data = (array) $data; + $data = (array)$data; - if (empty($separator)) - { + if (empty($separator)) { $separator = $this->separator; } - foreach ($data as $k => $v) - { + foreach ($data as $k => $v) { $key = $prefix ? $prefix . $separator . $k : $k; - if (\is_object($v) || \is_array($v)) - { + if (\is_object($v) || \is_array($v)) { $this->toFlatten($separator, $v, $array, $key); continue;