Firestore: test document update w/ integer ids (#5895)

tseaver · web-flow · commit e551423216cb · 2018-09-20T16:22:40.000-04:00
Attempt to reproduce issue #5489: the new system tests both pass.
diff --git a/firestore/google/cloud/firestore_v1beta1/_helpers.py b/firestore/google/cloud/firestore_v1beta1/_helpers.py
@@ -833,15 +833,18 @@ def process_server_timestamp(document_data, split_on_dots=True):
             (for updates only).
 
     Returns:
-        Tuple[List[str, ...], Dict[str, Any]]: A two-tuple of
+        List[List[str, ...], Dict[str, Any]], List[List[str, ...]: A
+        three-tuple of:
 
         * A list of all transform paths that use the server timestamp sentinel
         * The remaining keys in ``document_data`` after removing the
           server timestamp sentinels
+        * A list of all field paths that do not use the server timestamp
+          sentinel
     """
-    field_paths = []
     transform_paths = []
     actual_data = {}
+    field_paths = []
     for field_name, value in six.iteritems(document_data):
         if split_on_dots:
             top_level_path = FieldPath(*field_name.split("."))
@@ -872,6 +875,26 @@ def process_server_timestamp(document_data, split_on_dots=True):
     return transform_paths, actual_data, field_paths
 
 
+def canonicalize_field_paths(field_paths):
+    """Converts non-simple field paths to quoted field paths
+
+    Args:
+        field_paths (Sequence[str]): A list of field paths
+
+    Returns:
+        Sequence[str]:
+            The same list of field paths except non-simple field names
+            in the `.` delimited field path have been converted
+            into quoted unicode field paths. Simple field paths match
+            the regex ^[_a-zA-Z][_a-zA-Z0-9]*$.  See `Document`_ page for
+            more information.
+
+    .. _Document: https://cloud.google.com/firestore/docs/reference/rpc/google.firestore.v1beta1#google.firestore.v1beta1.Document  # NOQA
+    """
+    field_paths = [path.to_api_repr() for path in field_paths]
+    return sorted(field_paths)  # for testing purposes
+
+
 def get_transform_pb(document_path, transform_paths):
     """Get a ``Write`` protobuf for performing a document transform.
 
@@ -946,26 +969,6 @@ def pbs_for_set(document_path, document_data, merge=False, exists=None):
     return write_pbs
 
 
-def canonicalize_field_paths(field_paths):
-    """Converts non-simple field paths to quoted field paths
-
-    Args:
-        field_paths (Sequence[str]): A list of field paths
-
-    Returns:
-        Sequence[str]:
-            The same list of field paths except non-simple field names
-            in the `.` delimited field path have been converted
-            into quoted unicode field paths. Simple field paths match
-            the regex ^[_a-zA-Z][_a-zA-Z0-9]*$.  See `Document`_ page for
-            more information.
-
-    .. _Document: https://cloud.google.com/firestore/docs/reference/rpc/google.firestore.v1beta1#google.firestore.v1beta1.Document  # NOQA
-    """
-    field_paths = [path.to_api_repr() for path in field_paths]
-    return sorted(field_paths)  # for testing purposes
-
-
 def pbs_for_update(client, document_path, field_updates, option):
     """Make ``Write`` protobufs for ``update()`` methods.
 
diff --git a/firestore/tests/system.py b/firestore/tests/system.py
@@ -248,6 +248,55 @@ def test_document_set_merge(client, cleanup):
     assert snapshot2.update_time == write_result2.update_time
 
 
+def test_document_set_w_int_field(client, cleanup):
+    document_id = 'set-int-key' + unique_resource_id('-')
+    document = client.document('i-did-it', document_id)
+    # Add to clean-up before API request (in case ``set()`` fails).
+    cleanup(document)
+
+    # 0. Make sure the document doesn't exist yet
+    snapshot = document.get()
+    assert not snapshot.exists
+
+    # 1. Use ``create()`` to create the document.
+    before = {'testing': '1'}
+    document.create(before)
+
+    # 2. Replace using ``set()``.
+    data = {'14': {'status': 'active'}}
+    document.set(data)
+
+    # 3. Verify replaced data.
+    snapshot1 = document.get()
+    assert snapshot1.to_dict() == data
+
+
+def test_document_update_w_int_field(client, cleanup):
+    # Attempt to reproduce #5489.
+    document_id = 'update-int-key' + unique_resource_id('-')
+    document = client.document('i-did-it', document_id)
+    # Add to clean-up before API request (in case ``set()`` fails).
+    cleanup(document)
+
+    # 0. Make sure the document doesn't exist yet
+    snapshot = document.get()
+    assert not snapshot.exists
+
+    # 1. Use ``create()`` to create the document.
+    before = {'testing': '1'}
+    document.create(before)
+
+    # 2. Add values using ``update()``.
+    data = {'14': {'status': 'active'}}
+    document.update(data)
+
+    # 3. Verify updated data.
+    expected = before.copy()
+    expected.update(data)
+    snapshot1 = document.get()
+    assert snapshot1.to_dict() == expected
+
+
 def test_update_document(client, cleanup):
     document_id = 'for-update' + unique_resource_id('-')
     document = client.document('made', document_id)
diff --git a/firestore/tests/unit/test__helpers.py b/firestore/tests/unit/test__helpers.py
@@ -122,7 +122,7 @@ def test_invalid_chars_in_constructor(self):
 
     def test_component(self):
         field_path = self._make_one('a..b')
-        self.assertEquals(field_path.parts, ('a..b',))
+        self.assertEqual(field_path.parts, ('a..b',))
 
     def test_constructor_iterable(self):
         field_path = self._make_one('a', 'b', 'c')
@@ -140,7 +140,7 @@ def test_to_api_repr_a(self):
     def test_to_api_repr_backtick(self):
         parts = '`'
         field_path = self._make_one(parts)
-        self.assertEqual(field_path.to_api_repr(), '`\``')
+        self.assertEqual(field_path.to_api_repr(), r'`\``')
 
     def test_to_api_repr_dot(self):
         parts = '.'
@@ -1378,6 +1378,41 @@ def test_field_updates(self):
         self.assertEqual(actual_data, expected_data)
 
 
+class Test_canonicalize_field_paths(unittest.TestCase):
+
+    @staticmethod
+    def _call_fut(field_paths):
+        from google.cloud.firestore_v1beta1 import _helpers
+
+        return _helpers.canonicalize_field_paths(field_paths)
+
+    def _test_helper(self, to_convert):
+        from google.cloud.firestore_v1beta1 import _helpers
+
+        paths = [
+            _helpers.FieldPath.from_string(path) for path in to_convert
+        ]
+        found = self._call_fut(paths)
+
+        self.assertEqual(found, sorted(to_convert.values()))
+
+    def test_w_native_strings(self):
+        to_convert = {
+            '0abc.deq': '`0abc`.deq',
+            'abc.654': 'abc.`654`',
+            '321.0deq._321': '`321`.`0deq`._321',
+        }
+        self._test_helper(to_convert)
+
+    def test_w_unicode(self):
+        to_convert = {
+            u'0abc.deq': '`0abc`.deq',
+            u'abc.654': 'abc.`654`',
+            u'321.0deq._321': '`321`.`0deq`._321',
+        }
+        self._test_helper(to_convert)
+
+
 class Test_get_transform_pb(unittest.TestCase):
 
     @staticmethod
@@ -1498,24 +1533,6 @@ def test_update_and_transform(self):
         self._helper(do_transform=True)
 
 
-class Test_canonicalize_field_paths(unittest.TestCase):
-
-    def test_canonicalize_field_paths(self):
-        from google.cloud.firestore_v1beta1 import _helpers
-
-        field_paths = ['0abc.deq', 'abc.654', '321.0deq._321',
-                       u'0abc.deq', u'abc.654', u'321.0deq._321']
-        field_paths = [
-            _helpers.FieldPath.from_string(path) for path in field_paths]
-        convert = _helpers.canonicalize_field_paths(field_paths)
-        self.assertListEqual(
-            convert,
-            sorted([
-                '`0abc`.deq', 'abc.`654`', '`321`.`0deq`._321',
-                '`0abc`.deq', 'abc.`654`', '`321`.`0deq`._321'])
-        )
-
-
 class Test_pbs_for_update(unittest.TestCase):
 
     @staticmethod
