Firestore: don't omit originally-empty map values when processing timestamps. (#6050)

Closes #5944.

Author: tseaver

firestore/google/cloud/firestore_v1beta1/_helpers.py

@@ -851,6 +851,9 @@ def process_server_timestamp(document_data, split_on_dots=True):
         else:
             top_level_path = FieldPath.from_string(field_name)
         if isinstance(value, dict):
+            if len(value) == 0:
+                actual_data[field_name] = value
+                continue
             sub_transform_paths, sub_data, sub_field_paths = (
                 process_server_timestamp(value, False))
             for sub_transform_path in sub_transform_paths:

firestore/tests/unit/test__helpers.py

@@ -1377,6 +1377,27 @@ def test_field_updates(self):
         expected_data = {'a': {'b': data['a']['b']}}
         self.assertEqual(actual_data, expected_data)
 
+    def test_field_updates_w_empty_value(self):
+        import collections
+        from google.cloud.firestore_v1beta1 import _helpers
+        from google.cloud.firestore_v1beta1.constants import SERVER_TIMESTAMP
+
+        # "Cheat" and use OrderedDict-s so that iteritems() is deterministic.
+        data = collections.OrderedDict((
+            ('a', {'b': 10}),
+            ('c.d', {'e': SERVER_TIMESTAMP}),
+            ('f.g', SERVER_TIMESTAMP),
+            ('h', {}),
+        ))
+        transform_paths, actual_data, field_paths = self._call_fut(data)
+        self.assertEqual(
+            transform_paths,
+            [_helpers.FieldPath('c', 'd', 'e'),
+             _helpers.FieldPath('f', 'g')])
+
+        expected_data = {'a': {'b': data['a']['b']}, 'h': {}}
+        self.assertEqual(actual_data, expected_data)
+
 
 class Test_canonicalize_field_paths(unittest.TestCase):
 
@@ -1460,78 +1481,108 @@ def test_it(self):
 class Test_pbs_for_set(unittest.TestCase):
 
     @staticmethod
-    def _call_fut(document_path, document_data, option):
+    def _call_fut(document_path, document_data, merge=False, exists=None):
         from google.cloud.firestore_v1beta1._helpers import pbs_for_set
 
-        return pbs_for_set(document_path, document_data, option)
+        return pbs_for_set(
+            document_path, document_data, merge=merge, exists=exists)
 
-    def _helper(self, merge=False, do_transform=False, **write_kwargs):
-        from google.cloud.firestore_v1beta1.constants import SERVER_TIMESTAMP
-        from google.cloud.firestore_v1beta1.gapic import enums
-        from google.cloud.firestore_v1beta1.proto import common_pb2
+    @staticmethod
+    def _make_write_w_document(document_path, **data):
         from google.cloud.firestore_v1beta1.proto import document_pb2
         from google.cloud.firestore_v1beta1.proto import write_pb2
+        from google.cloud.firestore_v1beta1._helpers import encode_dict
 
-        document_path = _make_ref_string(
-            u'little', u'town', u'of', u'ham')
-        field_name1 = 'cheese'
-        value1 = 1.5
-        field_name2 = 'crackers'
-        value2 = True
-        field_name3 = 'butter'
+        return write_pb2.Write(
+            update=document_pb2.Document(
+                name=document_path,
+                fields=encode_dict(data),
+            ),
+        )
+
+    @staticmethod
+    def _make_write_w_transform(document_path, fields):
+        from google.cloud.firestore_v1beta1.proto import write_pb2
+        from google.cloud.firestore_v1beta1.gapic import enums
+
+        server_val = enums.DocumentTransform.FieldTransform.ServerValue
+        transforms = [
+            write_pb2.DocumentTransform.FieldTransform(
+                field_path=field, set_to_server_value=server_val.REQUEST_TIME)
+            for field in fields
+        ]
+
+        return write_pb2.Write(
+            transform=write_pb2.DocumentTransform(
+                document=document_path,
+                field_transforms=transforms,
+            ),
+        )
+
+    def _helper(self, merge=False, do_transform=False, exists=None,
+                empty_val=False):
+        from google.cloud.firestore_v1beta1.constants import SERVER_TIMESTAMP
+        from google.cloud.firestore_v1beta1.proto import common_pb2
 
+        document_path = _make_ref_string(u'little', u'town', u'of', u'ham')
         document_data = {
-            field_name1: value1,
-            field_name2: value2,
+            'cheese': 1.5,
+            'crackers': True,
         }
+
         if do_transform:
-            document_data[field_name3] = SERVER_TIMESTAMP
+            document_data['butter'] = SERVER_TIMESTAMP
 
-        write_pbs = self._call_fut(document_path, document_data, merge)
+        if empty_val:
+            document_data['mustard'] = {}
 
-        expected_update_pb = write_pb2.Write(
-            update=document_pb2.Document(
-                name=document_path,
-                fields={
-                    field_name1: _value_pb(double_value=value1),
-                    field_name2: _value_pb(boolean_value=value2),
-                },
-            ),
-            **write_kwargs
-        )
-        expected_pbs = [expected_update_pb]
+        write_pbs = self._call_fut(
+            document_path, document_data, merge, exists)
+
+        if empty_val:
+            update_pb = self._make_write_w_document(
+                document_path, cheese=1.5, crackers=True, mustard={},
+            )
+        else:
+            update_pb = self._make_write_w_document(
+                document_path, cheese=1.5, crackers=True,
+            )
+        expected_pbs = [update_pb]
 
         if merge:
-            field_paths = [field_name1, field_name2]
-            mask = common_pb2.DocumentMask(field_paths=sorted(field_paths))
-            expected_pbs[0].update_mask.CopyFrom(mask)
+            field_paths = sorted(['cheese', 'crackers'])
+            update_pb.update_mask.CopyFrom(
+                common_pb2.DocumentMask(field_paths=field_paths))
+
+        if exists is not None:
+            update_pb.current_document.CopyFrom(
+                common_pb2.Precondition(exists=exists))
 
         if do_transform:
-            server_val = enums.DocumentTransform.FieldTransform.ServerValue
-            expected_transform_pb = write_pb2.Write(
-                transform=write_pb2.DocumentTransform(
-                    document=document_path,
-                    field_transforms=[
-                        write_pb2.DocumentTransform.FieldTransform(
-                            field_path=field_name3,
-                            set_to_server_value=server_val.REQUEST_TIME,
-                        ),
-                    ],
-                ),
-            )
-            expected_pbs.append(expected_transform_pb)
+            expected_pbs.append(
+                self._make_write_w_transform(document_path, fields=['butter']))
 
         self.assertEqual(write_pbs, expected_pbs)
 
-    def test_without_option(self):
+    def test_without_merge(self):
         self._helper()
 
-    def test_with_merge_option(self):
+    def test_with_merge(self):
         self._helper(merge=True)
 
-    def test_update_and_transform(self):
+    def test_with_exists_false(self):
+        self._helper(exists=False)
+
+    def test_with_exists_true(self):
+        self._helper(exists=True)
+
+    def test_w_transform(self):
         self._helper(do_transform=True)
 
+    def test_w_transform_and_empty_value(self):
+        # Exercise #5944
+        self._helper(do_transform=True, empty_val=True)
+
 
 class Test_pbs_for_update(unittest.TestCase):