All non-simple field names are converted into unicode (googleapis#4859)

chemelnucfin · web-flow · commit e9263014924b · 2018-02-26T15:02:16.000-08:00
* Firestore: Fix for quoting simple field names with integer beginnings

* Added comments

* Fix bug for multiple field names in field paths

* Escape all non simple paths

* Review Changes
diff --git a/firestore/google/cloud/firestore_v1beta1/_helpers.py b/firestore/google/cloud/firestore_v1beta1/_helpers.py
@@ -18,6 +18,7 @@
 import collections
 import contextlib
 import datetime
+import re
 import sys
 
 import google.gax
@@ -67,6 +68,7 @@
     grpc.StatusCode.ALREADY_EXISTS: exceptions.Conflict,
     grpc.StatusCode.NOT_FOUND: exceptions.NotFound,
 }
+_UNESCAPED_FIELD_NAME_RE = re.compile('^[_a-zA-Z][_a-zA-Z0-9]*$')
 
 
 class GeoPoint(object):
@@ -837,6 +839,38 @@ def pbs_for_set(document_path, document_data, option):
     return write_pbs
 
 
+def canonicalize_field_paths(field_paths):
+    """Converts simple field path with integer beginnings to quoted field path
+
+    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
+    """
+    canonical_strings = []
+    for field_path in field_paths:
+        escaped_names = []
+        field_names = field_path.split('.')
+        for field_name in field_names:
+            if re.match(_UNESCAPED_FIELD_NAME_RE, field_name):
+                escaped_name = field_name
+            else:
+                escaped_name = u"`{}`".format(
+                    field_name.replace('\\', '\\\\').replace('`', '``'))
+            escaped_names.append(escaped_name)
+        new_field_path = '.'.join(escaped_names)
+        canonical_strings.append(new_field_path)
+    return canonical_strings
+
+
 def pbs_for_update(client, document_path, field_updates, option):
     """Make ``Write`` protobufs for ``update()`` methods.
 
@@ -860,6 +894,7 @@ def pbs_for_update(client, document_path, field_updates, option):
 
     transform_paths, actual_updates = remove_server_timestamp(field_updates)
     update_values, field_paths = FieldPathHelper.to_field_paths(actual_updates)
+    field_paths = canonicalize_field_paths(field_paths)
 
     update_pb = write_pb2.Write(
         update=document_pb2.Document(
diff --git a/firestore/google/cloud/firestore_v1beta1/client.py b/firestore/google/cloud/firestore_v1beta1/client.py
@@ -434,7 +434,6 @@ class CreateIfMissingOption(WriteOption):
         create_if_missing (bool): Indicates if the document should be created
             if it doesn't already exist.
     """
-
     def __init__(self, create_if_missing):
         self._create_if_missing = create_if_missing
 
diff --git a/firestore/tests/system.py b/firestore/tests/system.py
@@ -194,6 +194,38 @@ def test_document_set(client, cleanup):
     assert exc_to_code(exc_info.value.cause) == StatusCode.FAILED_PRECONDITION
 
 
+def test_document_integer_field(client, cleanup):
+    document_id = 'for-set' + unique_resource_id('-')
+    document = client.document('i-did-it', document_id)
+    # Add to clean-up before API request (in case ``set()`` fails).
+    cleanup(document)
+
+    data1 = {
+        '1a': {
+            '2b': '3c',
+            'ab': '5e'},
+        '6f': {
+            '7g': '8h',
+            'cd': '0j'}
+    }
+    option1 = client.write_option(exists=False)
+    document.set(data1, option=option1)
+
+    data2 = {'1a.ab': '4d', '6f.7g': '9h'}
+    option2 = client.write_option(create_if_missing=True)
+    document.update(data2, option=option2)
+    snapshot = document.get()
+    expected = {
+        '1a': {
+            '2b': '3c',
+            'ab': '4d'},
+        '6f': {
+            '7g': '9h',
+            'cd': '0j'}
+    }
+    assert snapshot.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
@@ -1186,6 +1186,20 @@ 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']
+        convert = _helpers.canonicalize_field_paths(field_paths)
+        self.assertListEqual(
+            convert,
+            ['`0abc`.deq', 'abc.`654`', '`321`.`0deq`._321',
+             '`0abc`.deq', 'abc.`654`', '`321`.`0deq`._321']
+        )
+
+
 class Test_pbs_for_update(unittest.TestCase):
 
     @staticmethod
