Fix, correct object behavior of remove operation - closes #35

raphaelstolt · Aug 25, 2016 · dd419ab · dd419ab
1 parent c92286c
commit dd419ab
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 8 deletions.
diff --git a/src/Rs/Json/Patch/Operations/Remove.php b/src/Rs/Json/Patch/Operations/Remove.php
@@ -46,21 +46,21 @@ public function perform($targetDocument)
             return $targetDocument;
         }
 
-        $targetDocument = json_decode($targetDocument, true);
+        $targetDocument = json_decode($targetDocument);
         $this->remove($targetDocument, $this->getPointerParts());
 
         return json_encode($targetDocument, JSON_UNESCAPED_UNICODE);
     }
 
     /**
-     * @param array $json         The json_decode'd Json structure.
-     * @param array $pointerParts The parts of the fed pointer.
+     * @param array|object $json         The json_decode'd Json structure.
+     * @param array        $pointerParts The parts of the fed pointer.
      */
-    private function remove(array &$json, array $pointerParts)
+    private function remove(&$json, array $pointerParts)
     {
         $pointerPart = array_shift($pointerParts);
 
-        if (isset($json[$pointerPart])) {
+        if (is_array($json) && isset($json[$pointerPart])) {
             if (count($pointerParts) === 0) {
                 unset($json[$pointerPart]);
                 if (ctype_digit($pointerPart)) {
@@ -72,10 +72,23 @@ private function remove(array &$json, array $pointerParts)
                     $pointerParts
                 );
             }
+        } elseif (is_object($json) && isset($json->{$pointerPart})) {
+            if (count($pointerParts) === 0) {
+                unset($json->{$pointerPart});
+            } else {
+                $this->remove(
+                    $json->{$pointerPart},
+                    $pointerParts
+                );
+            }
         } elseif ($pointerPart === Pointer::LAST_ARRAY_ELEMENT_CHAR && is_array($json)) {
             unset($json[count($json) - 1]);
         } else {
-            unset($json[$pointerPart]);
+            if (is_object($json)) {
+                unset($json->{$pointerPart});
+            } else {
+                unset($json[$pointerPart]);
+            }
         }
     }
 }
diff --git a/tests/integration/Rs/Json/PatchRemoveTest.php b/tests/integration/Rs/Json/PatchRemoveTest.php
@@ -14,7 +14,7 @@ public function removeNullProvider()
             array(
                 '{"a":{"b":null}}', // target document
                 '[ {"op":"remove", "path":"/a/b"} ]', // patch document
-                '{"a":[]}' // expected doument
+                '{"a":{}}' // expected document
             ),
             array(
                 '{"a":null}',
@@ -105,6 +105,42 @@ public function shouldReturnTargetDocumentWhenPathPointerIsNonexistent()
         $patch = new Patch($targetDocument, $patchDocument);
         $patchedDocument = $patch->apply();
 
+        $this->assertJsonStringEqualsJsonString(
+            $expectedDocument,
+            $patchedDocument
+        );
+    }
+    /**
+     * @test
+     * @ticket 35 (https://github.com/raphaelstolt/php-jsonpatch/issues/35)
+     */
+    public function shouldPreserveEmptyObject()
+    {
+        $targetDocument = '{"foo":{"bar":{"baz": {}, "qux": "val"}}, "bar": {}}';
+        $patchDocument = '[{"op":"remove", "path":"/foo/bar/qux"}]';
+        $expectedDocument = '{"foo":{"bar":{"baz": {}}}, "bar": {}}';
+
+        $patch = new Patch($targetDocument, $patchDocument);
+        $patchedDocument = $patch->apply();
+
+        $this->assertJsonStringEqualsJsonString(
+            $expectedDocument,
+            $patchedDocument
+        );
+    }
+    /**
+     * @test
+     * @ticket 35 (https://github.com/raphaelstolt/php-jsonpatch/issues/35)
+     */
+    public function shouldPreserveEmptyObjectNumericObjectProperties()
+    {
+        $targetDocument = '{"foo":{"bar":{"baz": {}, "3": "val"}}, "bar": {}}';
+        $patchDocument = '[{"op":"remove", "path":"/foo/bar/3"}]';
+        $expectedDocument = '{"foo":{"bar":{"baz": {}}}, "bar": {}}';
+
+        $patch = new Patch($targetDocument, $patchDocument);
+        $patchedDocument = $patch->apply();
+
         $this->assertJsonStringEqualsJsonString(
             $expectedDocument,
             $patchedDocument

diff --git a/tests/unit/Rs/Json/Patch/Operations/RemoveTest.php b/tests/unit/Rs/Json/Patch/Operations/RemoveTest.php
@@ -34,6 +34,44 @@ public function shouldNotRemoveElementWhenPathDoesNotExist()
             $removeOperation->perform($targetJson)
         );
     }
+    /**
+     * @test
+     * @ticket 35 (https://github.com/raphaelstolt/php-jsonpatch/issues/35)
+     */
+    public function shouldPreserveEmptyObject()
+    {
+        $targetJson = '{"foo":{"bar":{"baz": {}, "qux": "val"}}, "bar": {}}';
+        $expectedJson = '{"foo":{"bar":{"baz": {}}}, "bar": {}}';
+
+        $operation = new \stdClass;
+        $operation->path = '/foo/bar/qux';
+
+        $removeOperation = new Remove($operation);
+
+        $this->assertJsonStringEqualsJsonString(
+            $expectedJson,
+            $removeOperation->perform($targetJson)
+        );
+    }
+    /**
+     * @test
+     * @ticket 35 (https://github.com/raphaelstolt/php-jsonpatch/issues/35)
+     */
+    public function shouldPreserveEmptyObjectNumericObjectProperties()
+    {
+        $targetJson = '{"foo":{"bar":{"baz": {}, "3": "val"}}, "bar": {}}';
+        $expectedJson = '{"foo":{"bar":{"baz": {}}}, "bar": {}}';
+
+        $operation = new \stdClass;
+        $operation->path = '/foo/bar/3';
+
+        $removeOperation = new Remove($operation);
+
+        $this->assertJsonStringEqualsJsonString(
+            $expectedJson,
+            $removeOperation->perform($targetJson)
+        );
+    }
     /**
      * @test
      * @dataProvider removeProvider
@@ -75,7 +113,7 @@ public function removeProvider()
             )),
             array(array(
                 'given-json' => '{"baz":{"boo":{"bar":"zoo"}}}',
-                'expected-json' => '{"baz":{"boo":[]}}',
+                'expected-json' => '{"baz":{"boo":{}}}',
                 'remove-operation' => (object) array('path' => '/baz/boo/bar'),
             )),
             array(array(