Fix handling of empty sets in Firestore updates (#1316)

For context see: * googleapis/google-cloud-python#5944 * https://github.com/googleapis/google-cloud-common/pull/264 In PHP we can differentiate between Firestore maps and arrays based on whether the PHP array is associative or not. That difference gets considerably trickier when considering empty arrays. To create an empty map in PHP's Firestore client, you must explicitly provide `(object) []`.
googleapis · Oct 2, 2018 · 0c2c132 · 0c2c132
1 parent 4f45b0c
commit 0c2c132
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 8 deletions.
diff --git a/src/WriteBatch.php b/src/WriteBatch.php
@@ -688,8 +688,10 @@ private function filterFields(array $fields, array $inputPaths = [])
                         ? $inArray
                         : !$this->isAssoc($value);
 
+                    $isEmptyBeforeFiltering = empty($fields[$key]);
+
                     $fields[$key] = $filterFn($value, $currentPath, $localInArray);
-                    if (empty($fields[$key])) {
+                    if (!$isEmptyBeforeFiltering && empty($fields[$key])) {
                         unset($fields[$key]);
                     }
                 } else {

diff --git a/tests/Conformance/proto/firestore-test-suite.binproto b/tests/Conformance/proto/firestore-test-suite.binproto
diff --git a/tests/Unit/ConformanceTest.php b/tests/Unit/ConformanceTest.php
@@ -169,7 +169,7 @@ public function testUpdatePaths($test)
             foreach ($test['fieldPaths'] as $key => $val) {
                 $fields[] = [
                     'path' => new FieldPath($val['field']),
-                    'value' => $this->injectSentinel(json_decode($test['jsonValues'][$key], true))
+                    'value' => $this->injectSentinel($this->decodeJson($test['jsonValues'][$key], true))
                 ];
             }
 
@@ -249,7 +249,11 @@ public function testQuery($test)
                         $query = $query->where(
                             $path,
                             $clause['where']['op'],
-                            $this->injectWhere($this->injectSentinel(json_decode($clause['where']['jsonValue'], true)))
+                            $this->injectWhere(
+                                $this->injectSentinel(
+                                    $this->decodeJson($clause['where']['jsonValue'])
+                                )
+                            )
                         );
                         break;
 
@@ -279,7 +283,7 @@ public function testQuery($test)
                         $values = [];
                         if (isset($clause[$name]['jsonValues'])) {
                             foreach ($clause[$name]['jsonValues'] as $value) {
-                                $values[] = $this->injectSentinel(json_decode($value, true));
+                                $values[] = $this->injectSentinel($this->decodeJson($value));
                             }
                         }
 
@@ -299,7 +303,7 @@ public function testQuery($test)
                                 $ref->reveal(),
                                 $mapper,
                                 [],
-                                json_decode($clause[$name]['docSnapshot']['jsonData'], true),
+                                $this->decodeJson($clause[$name]['docSnapshot']['jsonData']),
                                 true
                             );
                         }
@@ -377,12 +381,16 @@ private function formatOptions(array $test)
 
     private function generateFields($data)
     {
-        $fields = json_decode($data, true);
+        $fields = $this->decodeJson($data, false);
         return $this->injectSentinels($fields);
     }
 
-    private function injectSentinels(array $fields)
+    private function injectSentinels($fields)
     {
+        if (!is_array($fields)) {
+            return $fields;
+        }
+
         foreach ($fields as $name => &$value) {
             $value = $this->injectSentinel($value);
         }
@@ -392,7 +400,7 @@ private function injectSentinels(array $fields)
 
     private function injectSentinel($value)
     {
-        if (is_array($value)) {
+        if (is_array($value) && !empty($value)) {
             if (in_array(array_values($value)[0], ['ArrayUnion', 'ArrayRemove'], true)) {
                 $type = lcfirst(array_shift($value));
                 return FieldValue::$type($this->injectSentinels($value));
@@ -409,6 +417,10 @@ private function injectSentinel($value)
             return FieldValue::serverTimestamp();
         }
 
+        if ($value === 'EMPTY_MAP') {
+            return (object) [];
+        }
+
         return $value;
     }
 
@@ -421,6 +433,18 @@ private function injectWhere($value)
         return $value;
     }
 
+    private function decodeJson($json, $returnEmptyMapAsObject = false)
+    {
+        if ($json === '{}') {
+            return $returnEmptyMapAsObject
+                ? (object) []
+                : [];
+        }
+        $json = str_replace('{}', '"EMPTY_MAP"', $json);
+
+        return json_decode($json, true);
+    }
+
     private function setupCases($suite, array $types, array $excludes)
     {
         if (self::$cases) {