Merge pull request #18 from internalsystemerror/fix-merging-associati…

…ve-arrays fix: Mimic how postgres-document-store works, where documents are not merged recursively.
event-engine · Sep 30, 2020 · 7b53822 · 7b53822
2 parents 1e980c8 + 15d3034
commit 7b53822
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 31 deletions.
diff --git a/src/InMemoryDocumentStore.php b/src/InMemoryDocumentStore.php
@@ -205,7 +205,7 @@ public function updateDoc(string $collectionName, string $docId, array $docOrSub
         $this->assertDocExists($collectionName, $docId);
         $this->assertUniqueConstraints($collectionName, $docId, $docOrSubset);
 
-        $this->inMemoryConnection['documents'][$collectionName][$docId] = $this->arrayReplaceRecursiveAssocOnly(
+        $this->inMemoryConnection['documents'][$collectionName][$docId] = $this->arrayReplace(
             $this->inMemoryConnection['documents'][$collectionName][$docId],
             $docOrSubset
         );
@@ -550,7 +550,7 @@ private function assertMultiFieldUniqueConstraint(string $collectionName, string
 
         if($this->hasDoc($collectionName, $docId)) {
             $effectedDoc = $this->getDoc($collectionName, $docId);
-            $docOrSubset = $this->arrayReplaceRecursiveAssocOnly($effectedDoc, $docOrSubset);
+            $docOrSubset = $this->arrayReplace($effectedDoc, $docOrSubset);
         }
 
         $reader = new ArrayReader($docOrSubset);
@@ -672,32 +672,15 @@ private function sort(&$docs, OrderBy $orderBy)
      * @param array $array2
      * @return array
      */
-    private function arrayReplaceRecursiveAssocOnly(array $array1, array $array2): array
+    private function arrayReplace(array $array1, array $array2): array
     {
         foreach ($array2 as $key2 => $value2) {
-            $bothValuesArraysAndAtLeastOneAssoc = \is_array($value2) &&
-                isset($array1[$key2]) && \is_array($array1[$key2]) &&
-                !($this->isSequentialArray($value2) && $this->isSequentialArray($array1[$key2]));
-
-            if ($bothValuesArraysAndAtLeastOneAssoc) {
-                $array1[$key2] = $this->arrayReplaceRecursiveAssocOnly($array1[$key2], $value2);
-            } else {
-                $array1[$key2] = $value2;
-            }
+            $array1[$key2] = $value2;
         }
 
         return $array1;
     }
 
-    private function isSequentialArray(array $array): bool
-    {
-        if (\count($array) === 0) {
-            return true;
-        }
-
-        return \array_keys($array) === \range(0, \count($array) - 1);
-    }
-
     private function transformToPartialDoc(array $doc, PartialSelect $partialSelect): array
     {
         $partialDoc = [];

diff --git a/tests/InMemoryDocumentStoreTest.php b/tests/InMemoryDocumentStoreTest.php
@@ -92,9 +92,7 @@ public function it_updates_a_subset_of_a_doc()
         $doc = [
             'some' => [
                 'prop' => 'foo',
-                'other' => [
-                    'nested' => 42
-                ]
+                'other' => 'bar',
             ],
             'baz' => 'bat',
         ];
@@ -108,7 +106,7 @@ public function it_updates_a_subset_of_a_doc()
         ]);
 
         $filteredDocs = array_values(iterator_to_array($this->store->findDocs('test', new EqFilter('some.prop', 'fuzz'))));
-        $this->assertEquals(42, $filteredDocs[0]['some']['other']['nested']);
+        $this->assertArrayNotHasKey('other', $filteredDocs[0]['some']);
     }
 
     /**
@@ -424,6 +422,7 @@ public function it_ensures_unique_constraints_for_multiple_fields()
 
         $this->store->addDoc('test', '1', ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bat']]]);
         $this->store->addDoc('test', '2', ['some' => ['prop' => 'bar', 'other' => ['prop' => 'bat']]]);
+        $this->store->addDoc('test', '3', ['some' => ['prop' => 'bar']]);
 
         $this->expectExceptionMessageRegExp('/^Unique constraint violation/');
         $this->store->addDoc('test', '4', ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bat']]]);
@@ -443,7 +442,7 @@ public function it_ensures_unique_constraints_for_multiple_fields_for_update()
         $this->store->addDoc('test', '3', ['some' => ['prop' => 'bar']]);
 
         $this->expectExceptionMessageRegExp('/^Unique constraint violation/');
-        $this->store->updateDoc('test', '2', ['some' => ['prop' => 'foo']]);
+        $this->store->updateDoc('test', '2', ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bat']]]);
     }
 
     /**
@@ -574,7 +573,7 @@ public function it_deletes_many()
     /**
      * @test
      */
-    public function it_does_not_update_numeric_arrays_recursively()
+    public function it_does_not_update_arrays_recursively()
     {
         $this->store->addCollection('test');
 
@@ -604,13 +603,13 @@ public function it_does_not_update_numeric_arrays_recursively()
 
         $this->assertEquals(
             [
-                'a' => ['a' => 10, 'b' => 21, 'c' => 30],
+                'a' => ['b' => 21, 'c' => 30],
                 'b' => [10, 30],
                 'c' => [true],
                 'd' => [],
-                'e' => ['a' => 'b'],
-                'f' => [10, 20, 'x' => 10, 'y' => 20],
-                'g' => ['x' => 10, 'y' => 20, 30, 40],
+                'e' => [],
+                'f' => ['x' => 10, 'y' => 20],
+                'g' => [30, 40],
                 'h' => [11],
                 'i' => [22],
                 'j' => ['bar']