From e18da77d4abe49c7e4c334ba4f136f1c6ea139e9 Mon Sep 17 00:00:00 2001 From: webimpress Date: Tue, 5 Sep 2017 12:56:07 +0100 Subject: [PATCH 1/4] `patchKey` merge configuration instead of override --- src/ConfigResource.php | 20 +----- test/ConfigResourceTest.php | 124 ++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 18 deletions(-) diff --git a/src/ConfigResource.php b/src/ConfigResource.php index 2799a0a..9311cca 100644 --- a/src/ConfigResource.php +++ b/src/ConfigResource.php @@ -107,25 +107,9 @@ public function patch($data, $tree = false) */ public function patchKey($key, $value) { - // Get local config file - $config = []; - if (file_exists($this->fileName)) { - $config = include $this->fileName; - if (! is_array($config)) { - $config = []; - } - } - $config = $this->replaceKey($key, $value, $config); + $this->patch([$key => $value]); - // Write to configuration file - $this->writer->toFile($this->fileName, $config); - $this->invalidateCache($this->fileName); - - // Reseed configuration - $this->config = $config; - - // Return written values - return $config; + return $this->config; } /** diff --git a/test/ConfigResourceTest.php b/test/ConfigResourceTest.php index a2703ee..8c4dd0d 100644 --- a/test/ConfigResourceTest.php +++ b/test/ConfigResourceTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use stdClass; use Zend\Config\Writer\PhpArray; +use Zend\Stdlib\ArrayUtils; use ZF\Configuration\ConfigResource; class ConfigResourceTest extends TestCase @@ -402,4 +403,127 @@ public function testDeleteNonexistentKeyShouldDoNothing() $test = include $this->file; $this->assertEquals($expected, $test); } + + public function patchKey() + { + return [ + 'scalar-top-level' => [ + 'top', + 'updated', + ['top' => 'updated'] + ], + + 'overwrite-hash' => [ + 'sub', + 'updated', + ['sub' => 'updated'], + ], + + 'nested-scalar' => [ + 'sub.level', + 'updated', + [ + 'sub' => [ + 'level' => 'updated', + ], + ], + ], + 'nested-list' => [ + 'sub.list', + ['three', 'four'], + [ + 'sub' => [ + 'list' => ['three', 'four'], + ], + ], + ], + 'nested-hash' => [ + 'sub.hash.two', + 'updated', + [ + 'sub' => [ + 'hash' => [ + 'two' => 'updated', + ], + ], + ], + ], + 'overwrite-nested-null' => [ + 'sub.null', + 'updated', + [ + 'sub' => [ + 'null' => 'updated', + ], + ], + ], + 'overwrite-nested-object' => [ + 'sub.object', + 'updated', + [ + 'sub' => [ + 'object' => 'updated', + ], + ], + ], + 'merge-nested' => [ + 'sub.hash', + [ + 'two' => 'two-updated', + 'three' => 'three-updated', + ], + [ + 'sub' => [ + 'hash' => [ + 'one' => 1, + 'two' => 'two-updated', + 'three' => 'three-updated', + ], + ], + ], + ], + 'add-new' => [ + 'sub', + ['new' => 'added'], + [ + 'sub' => [ + 'new' => 'added', + ], + ], + ], + ]; + } + + /** + * @dataProvider patchKey + * + * @param string $key + * @param mixed $value + * @param mixed $expected + */ + public function testPatchKey($key, $value, $expected) + { + $config = [ + 'top' => 'level', + 'sub' => [ + 'level' => 2, + 'list' => [ + 'one', + 'two', + ], + 'hash' => [ + 'one' => 1, + 'two' => 2, + ], + 'null' => null, + 'object' => stdClass::class, + ], + ]; + $writer = new PhpArray(); + $writer->toFile($this->file, $config); + + $updated = $this->configResource->patchKey($key, $value); + $expected = ArrayUtils::merge($config, $expected); + $this->assertEquals($expected, $updated); + } } From ce52c5bd0a2bfe1f329fce2719cb2f728978a6aa Mon Sep 17 00:00:00 2001 From: webimpress Date: Tue, 5 Sep 2017 12:56:20 +0100 Subject: [PATCH 2/4] Added missing param PHPDocs --- src/ConfigResource.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ConfigResource.php b/src/ConfigResource.php index 9311cca..876b34e 100644 --- a/src/ConfigResource.php +++ b/src/ConfigResource.php @@ -55,6 +55,7 @@ public function __construct(array $config, $fileName, ConfigWriter $writer) * Expects data to be in the form of key/value pairs * * @param array|stdClass|Traversable $data + * @param bool $tree * @return array */ public function patch($data, $tree = false) From dd3a3f1295832fdf31e68420b12eae5294a77d1b Mon Sep 17 00:00:00 2001 From: webimpress Date: Tue, 5 Sep 2017 17:58:55 +0100 Subject: [PATCH 3/4] Updated patch test with merge config Configuration is always read from the file, but in the test configuration was not stored in the file. Expected result was then not full, because it should contain current config with merged changes from patch. --- test/ConfigResourceTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/ConfigResourceTest.php b/test/ConfigResourceTest.php index 8c4dd0d..94e8bc4 100644 --- a/test/ConfigResourceTest.php +++ b/test/ConfigResourceTest.php @@ -87,6 +87,8 @@ public function testPatchListUpdatesFileWithMergedConfig() ], 'baz' => 'not what you think', ]; + $writer = new PhpArray(); + $writer->toFile($this->file, $config); $configResource = new ConfigResource($config, $this->file, $this->writer); $patch = [ @@ -98,8 +100,10 @@ public function testPatchListUpdatesFileWithMergedConfig() $this->assertEquals($patch, $response); $expected = [ + 'foo' => 'bar', 'bar' => [ 'baz' => 'UPDATED', + 'bat' => 'bogus', ], 'baz' => 'what you think', ]; From d036d061a9627f831dedee9665ce11cd94a214a7 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 1 Nov 2017 14:09:26 -0500 Subject: [PATCH 4/4] Adds CHANGELOG for #25 - Also removes empty stub for 1.2.2. --- CHANGELOG.md | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b9e4f0..c5323ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. -## 1.3.1 - TBD +## 1.3.1 - 2017-11-01 ### Added @@ -10,7 +10,11 @@ All notable changes to this project will be documented in this file, in reverse ### Changed -- Nothing. +- [#25](https://github.com/zfcampus/zf-configuration/pull/25) changes the + behavior of `ConfigResource::patchKey()` to do what it is advertised to do: + merge incoming configuration. Previously, it was overwriting configuration, + which could in some extreme instances lead to lost configuration. The behavior + is now correct. ### Deprecated @@ -47,24 +51,6 @@ All notable changes to this project will be documented in this file, in reverse - Nothing. -## 1.2.2 - TBD - -### Added - -- Nothing. - -### Deprecated - -- Nothing. - -### Removed - -- Nothing. - -### Fixed - -- Nothing. - ## 1.2.1 - 2016-08-13 ### Added