From 69eb0cfe129c4aa3c485805acd24672f22fc5486 Mon Sep 17 00:00:00 2001 From: mkherlakian Date: Wed, 20 Sep 2017 14:21:35 -0400 Subject: [PATCH 1/5] Modified hydration of ArraySerializeable to take existing data into account if getArrayCopy exists --- src/ArraySerializable.php | 4 ++ test/ArraySerializableTest.php | 38 ++++++++++++++++ .../ArraySerializableNoGetArrayCopy.php | 43 +++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 test/TestAsset/ArraySerializableNoGetArrayCopy.php diff --git a/src/ArraySerializable.php b/src/ArraySerializable.php index bf89df9..573d933 100644 --- a/src/ArraySerializable.php +++ b/src/ArraySerializable.php @@ -68,6 +68,10 @@ public function hydrate(array $data, $object) } if (is_callable([$object, 'exchangeArray'])) { + if(is_callable([$object, 'getArrayCopy'])) { + $original = $object->getArrayCopy($object); + $replacement = array_merge($original, $replacement); + } $object->exchangeArray($replacement); } elseif (is_callable([$object, 'populate'])) { $object->populate($replacement); diff --git a/test/ArraySerializableTest.php b/test/ArraySerializableTest.php index d401e01..939e409 100644 --- a/test/ArraySerializableTest.php +++ b/test/ArraySerializableTest.php @@ -14,6 +14,7 @@ use Zend\Hydrator\Exception\BadMethodCallException; use Zend\Hydrator\ArraySerializable; use ZendTest\Hydrator\TestAsset\ArraySerializable as ArraySerializableAsset; +use ZendTest\Hydrator\TestAsset\ArraySerializableNoGetArrayCopy as ArraySerializableAssetNoGetArrayCopy; /** * Unit tests for {@see ArraySerializable} @@ -93,4 +94,41 @@ public function testCanHydrateToArraySerializableObject() $this->assertSame($data, $object->getArrayCopy()); } + + /** + * Verifies that when an object already has properties, + * these properties are preserved when it's hydrated with new data + * existing properties should get overwritten + */ + public function testWillPreserveOriginalPropsAtHydration() + { + $original = new ArraySerializableAsset(); + + $data = [ + 'bar' => 'foo1' + ]; + + $expected = array_merge($original->getArrayCopy(), $data); + + $actual = $this->hydrator->hydrate($data, $original); + + $this->assertSame($expected, $actual->getArrayCopy()); + } + + /** + * To preserve backwards compatibility, if getArrayCopy() is not implemented + * by the to-be hydrated object, simply exchange the array + */ + public function testWillReplaceArrayIfNoGetArrayCopy() { + $original = new ArraySerializableAssetNoGetArrayCopy(); + + $data = [ + 'bar' => 'foo1' + ]; + + $expected = $data; + + $actual = $this->hydrator->hydrate($data, $original); + $this->assertSame($expected, $actual->getData()); + } } diff --git a/test/TestAsset/ArraySerializableNoGetArrayCopy.php b/test/TestAsset/ArraySerializableNoGetArrayCopy.php new file mode 100644 index 0000000..cb3cbf4 --- /dev/null +++ b/test/TestAsset/ArraySerializableNoGetArrayCopy.php @@ -0,0 +1,43 @@ +data = [ + "foo" => "bar", + "bar" => "foo", + "blubb" => "baz", + "quo" => "blubb" + ]; + } + + /** + * Exchange internal values from provided array + * + * @param array $array + * @return void + */ + public function exchangeArray(array $array) + { + $this->data = $array; + } + + /** + * Returns the internal data + */ + public function getData() { + return $this->data; + } +} From e7313bba492dda611e32d1f8206c11d15a8e28c8 Mon Sep 17 00:00:00 2001 From: mkherlakian Date: Wed, 20 Sep 2017 15:02:40 -0400 Subject: [PATCH 2/5] fixed CS issues --- src/ArraySerializable.php | 2 +- test/ArraySerializableTest.php | 3 ++- test/TestAsset/ArraySerializableNoGetArrayCopy.php | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ArraySerializable.php b/src/ArraySerializable.php index 573d933..ecff5a1 100644 --- a/src/ArraySerializable.php +++ b/src/ArraySerializable.php @@ -68,7 +68,7 @@ public function hydrate(array $data, $object) } if (is_callable([$object, 'exchangeArray'])) { - if(is_callable([$object, 'getArrayCopy'])) { + if (is_callable([$object, 'getArrayCopy'])) { $original = $object->getArrayCopy($object); $replacement = array_merge($original, $replacement); } diff --git a/test/ArraySerializableTest.php b/test/ArraySerializableTest.php index 939e409..a763551 100644 --- a/test/ArraySerializableTest.php +++ b/test/ArraySerializableTest.php @@ -119,7 +119,8 @@ public function testWillPreserveOriginalPropsAtHydration() * To preserve backwards compatibility, if getArrayCopy() is not implemented * by the to-be hydrated object, simply exchange the array */ - public function testWillReplaceArrayIfNoGetArrayCopy() { + public function testWillReplaceArrayIfNoGetArrayCopy() + { $original = new ArraySerializableAssetNoGetArrayCopy(); $data = [ diff --git a/test/TestAsset/ArraySerializableNoGetArrayCopy.php b/test/TestAsset/ArraySerializableNoGetArrayCopy.php index cb3cbf4..d240153 100644 --- a/test/TestAsset/ArraySerializableNoGetArrayCopy.php +++ b/test/TestAsset/ArraySerializableNoGetArrayCopy.php @@ -37,7 +37,8 @@ public function exchangeArray(array $array) /** * Returns the internal data */ - public function getData() { + public function getData() + { return $this->data; } } From 9457a25d6ae5e21a96779876467dff2327d8312d Mon Sep 17 00:00:00 2001 From: mkherlakian Date: Wed, 20 Sep 2017 15:29:11 -0400 Subject: [PATCH 3/5] changed merge for zend-stdlib's merge. Changed a use namespace reference --- src/ArraySerializable.php | 2 +- test/ArraySerializableTest.php | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ArraySerializable.php b/src/ArraySerializable.php index ecff5a1..9c0c64a 100644 --- a/src/ArraySerializable.php +++ b/src/ArraySerializable.php @@ -70,7 +70,7 @@ public function hydrate(array $data, $object) if (is_callable([$object, 'exchangeArray'])) { if (is_callable([$object, 'getArrayCopy'])) { $original = $object->getArrayCopy($object); - $replacement = array_merge($original, $replacement); + $replacement = \Zend\Stdlib\ArrayUtils::merge($original, $replacement); } $object->exchangeArray($replacement); } elseif (is_callable([$object, 'populate'])) { diff --git a/test/ArraySerializableTest.php b/test/ArraySerializableTest.php index a763551..dd37f17 100644 --- a/test/ArraySerializableTest.php +++ b/test/ArraySerializableTest.php @@ -14,7 +14,6 @@ use Zend\Hydrator\Exception\BadMethodCallException; use Zend\Hydrator\ArraySerializable; use ZendTest\Hydrator\TestAsset\ArraySerializable as ArraySerializableAsset; -use ZendTest\Hydrator\TestAsset\ArraySerializableNoGetArrayCopy as ArraySerializableAssetNoGetArrayCopy; /** * Unit tests for {@see ArraySerializable} @@ -121,10 +120,10 @@ public function testWillPreserveOriginalPropsAtHydration() */ public function testWillReplaceArrayIfNoGetArrayCopy() { - $original = new ArraySerializableAssetNoGetArrayCopy(); + $original = new \ZendTest\Hydrator\TestAsset\ArraySerializableNoGetArrayCopy(); $data = [ - 'bar' => 'foo1' + 'bar' => 'foo1' ]; $expected = $data; From cbc97e841aacd4b97bde35858d0df4719112975a Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 20 Sep 2017 15:50:16 -0500 Subject: [PATCH 4/5] Updates patch to follow framework conventions - Import all classes used - Return from conditionals in order to prevent if/elseif/else - Add comments for behavior needing explanations --- src/ArraySerializable.php | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/ArraySerializable.php b/src/ArraySerializable.php index 9c0c64a..b2cc97f 100644 --- a/src/ArraySerializable.php +++ b/src/ArraySerializable.php @@ -9,6 +9,8 @@ namespace Zend\Hydrator; +use Zend\Stdlib\ArrayUtils; + class ArraySerializable extends AbstractHydrator { /** @@ -68,18 +70,23 @@ public function hydrate(array $data, $object) } if (is_callable([$object, 'exchangeArray'])) { + // Ensure any previously populated values not in the replacement + // remain following population. if (is_callable([$object, 'getArrayCopy'])) { $original = $object->getArrayCopy($object); - $replacement = \Zend\Stdlib\ArrayUtils::merge($original, $replacement); + $replacement = ArrayUtils::merge($original, $replacement); } $object->exchangeArray($replacement); - } elseif (is_callable([$object, 'populate'])) { + return $object; + } + + if (is_callable([$object, 'populate'])) { $object->populate($replacement); - } else { - throw new Exception\BadMethodCallException( - sprintf('%s expects the provided object to implement exchangeArray() or populate()', __METHOD__) - ); + return $object; } - return $object; + + throw new Exception\BadMethodCallException( + sprintf('%s expects the provided object to implement exchangeArray() or populate()', __METHOD__) + ); } } From 2907ec9855ce021b3c0357f48567b3743d448884 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 20 Sep 2017 15:52:59 -0500 Subject: [PATCH 5/5] Adds CHANGELOG for #65 --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fec75c..ed9b1ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file, in reverse - Nothing. +### Changed + +- Nothing. + ### Deprecated - Nothing. @@ -18,7 +22,12 @@ All notable changes to this project will be documented in this file, in reverse ### Fixed -- Nothing. +- [#65](https://github.com/zendframework/zend-hydrator/pull/65) fixes the + hydration behavior of the `ArraySerializable` hydrator when using + `exchangeArray()`. Previously, the method would clear any existing values from + the instance, which is problematic when a partial update is provided as values + not in the update would disappear. The class now pulls the original values, + and recursively merges the replacement with those values. ## 2.2.2 - 2017-05-17