Merge branch 'hotfix/25'

Close #25 Fixes #21
zfcampus · Nov 1, 2017 · 8b60321 · 8b60321
2 parents 09f5af9 + d036d06
commit 8b60321
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,15 +2,19 @@
 
 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
 
 - Nothing.
 
 ### 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

diff --git a/src/ConfigResource.php b/src/ConfigResource.php
@@ -110,25 +110,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
@@ -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
@@ -86,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 = [
@@ -97,8 +100,10 @@ public function testPatchListUpdatesFileWithMergedConfig()
         $this->assertEquals($patch, $response);
 
         $expected = [
+            'foo' => 'bar',
             'bar' => [
                 'baz' => 'UPDATED',
+                'bat' => 'bogus',
             ],
             'baz' => 'what you think',
         ];
@@ -402,4 +407,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);
+    }
 }