Review Changes

Author: chemelnucfin

firestore/google/cloud/firestore_v1beta1/_helpers.py

@@ -823,29 +823,29 @@ def process_server_timestamp(document_data, split_on_dots=True):
     transform_paths = []
     actual_data = {}
     for field_name, value in six.iteritems(document_data):
+        if split_on_dots:
+            top_level_path = FieldPath(*field_name.split("."))
+        else:
+            top_level_path = FieldPath.from_string(field_name)
         if isinstance(value, dict):
             sub_transform_paths, sub_data, sub_field_paths = (
                 process_server_timestamp(value, False))
-            for sub_path in sub_transform_paths:
-                field_path = FieldPath.from_string(field_name)
-                field_path.parts = field_path.parts + sub_path.parts
-                transform_paths.extend([field_path])
+            for sub_transform_path in sub_transform_paths:
+                transform_path = FieldPath.from_string(field_name)
+                transform_path.parts = transform_path.parts + sub_transform_path.parts
+                transform_paths.extend([transform_path])
             if sub_data:
                 # Only add a key to ``actual_data`` if there is data.
-
                 actual_data[field_name] = sub_data
                 for sub_field_path in sub_field_paths:
                     field_path = FieldPath.from_string(field_name)
                     field_path.parts = field_path.parts + sub_field_path.parts
                     field_paths.append(field_path)
         elif value is constants.SERVER_TIMESTAMP:
-            if split_on_dots:
-                transform_paths.append(FieldPath(*field_name.split(".")))
-            else:
-                transform_paths.append(FieldPath.from_string(field_name))
+            transform_paths.append(top_level_path)
         else:
             actual_data[field_name] = value
-            field_paths.append(FieldPath(field_name))
+            field_paths.append(top_level_path)
     if not transform_paths:
         actual_data = document_data
     return transform_paths, actual_data, field_paths
@@ -887,9 +887,9 @@ def pbs_for_set(document_path, document_data, merge=False, exists=None):
         document_path (str): A fully-qualified document path.
         document_data (dict): Property names and values to use for
             replacing a document.
-        option (optional[~.firestore_v1beta1.client.WriteOption]): A
-           write option to make assertions / preconditions on the server
-           state of the document before applying changes.
+        merge (bool): Whether to merge the fields or replace them
+        exists (bool): If set, a precondition to indicate whether the
+            document should exist or not. Used for create.
 
     Returns:
         List[google.cloud.firestore_v1beta1.types.Write]: One

firestore/google/cloud/firestore_v1beta1/client.py

@@ -39,8 +39,7 @@
 DEFAULT_DATABASE = '(default)'
 """str: The default database used in a :class:`~.firestore.client.Client`."""
 _BAD_OPTION_ERR = (
-    'Exactly one of ``last_update_time``, ``exists`` '
-    'must be provided.')
+    'Exactly one of ``last_update_time`` or ``exists`` must be provided.')
 _BAD_DOC_TEMPLATE = (
     'Document {!r} appeared in response but was not present among references')
 _ACTIVE_TXN = 'There is already an active transaction.'

firestore/tests/system.py

@@ -143,7 +143,7 @@ def test_document_set(client, cleanup):
     snapshot = document.get()
     assert snapshot.to_dict() is None
 
-    # 1. Use ``create()`` to create the document (using an option).
+    # 1. Use ``create()`` to create the document.
     data1 = {'foo': 88}
     write_result1 = document.create(data1)
     snapshot1 = document.get()
@@ -152,7 +152,7 @@ def test_document_set(client, cleanup):
     assert snapshot1.create_time == snapshot1.update_time
     assert snapshot1.update_time == write_result1.update_time
 
-    # 2. Call ``set()`` again to overwrite (no option).
+    # 2. Call ``set()`` again to overwrite.
     data2 = {'bar': None}
     write_result2 = document.set(data2)
     snapshot2 = document.get()
@@ -203,7 +203,7 @@ def test_document_set_merge(client, cleanup):
     snapshot = document.get()
     assert not snapshot.exists
 
-    # 1. Use ``set()`` to create the document (using an option).
+    # 1. Use ``create()`` to create the document.
     data1 = {'name': 'Sam',
              'address': {'city': 'SF',
                          'state': 'CA'}}

firestore/tests/unit/test_cross_language.py

@@ -27,7 +27,7 @@ class TestCrossLanguage(unittest.TestCase):
 
     def test_cross_language(self):
         filenames = sorted(glob.glob('tests/unit/testdata/*.textproto'))
-        count = 0
+        failed = 0
         descs = []
         for test_filename in filenames:
             bytes = open(test_filename, 'r').read()
@@ -38,14 +38,14 @@ def test_cross_language(self):
                 os.path.splitext(os.path.basename(test_filename))[0])
             try:
                 self.run_write_test(test_proto, desc)
-            except (AssertionError, Exception) as error:
-                count += 1
+            except Exception as error:
+                failed += 1
                 # print(desc, test_proto)  # for debugging
                 # print(error.args[0])  # for debugging
                 descs.append(desc)
         # for desc in descs:  # for debugging
             # print(desc)  # for debugging
-        # print(str(count) + "/" + str(len(filenames)))  # for debugging
+        # print(str(failed) + "/" + str(len(filenames)))  # for debugging
 
     def run_write_test(self, test_proto, desc):
         from google.cloud.firestore_v1beta1.proto import firestore_pb2
@@ -78,10 +78,10 @@ def run_write_test(self, test_proto, desc):
             client, doc = self.setup(firestore_api, tp)
             data = convert_data(json.loads(tp.json_data))
             if tp.HasField("option"):
-                option = True
+                merge = True
             else:
-                option = False
-            call = functools.partial(doc.set, data, option)
+                merge = False
+            call = functools.partial(doc.set, data, merge)
         elif kind == "update":
             tp = test_proto.update
             client, doc = self.setup(firestore_api, tp)