From b3bec00b0d7e93f09a6eb90a9659dd0d35e2e00a Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 30 Nov 2018 18:09:10 -0500 Subject: [PATCH 1/2] For queries ordered on '__name__', expand field values to full paths. Closes #6793. --- .../google/cloud/firestore_v1beta1/query.py | 14 +++++++-- firestore/tests/unit/test_query.py | 30 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/firestore/google/cloud/firestore_v1beta1/query.py b/firestore/google/cloud/firestore_v1beta1/query.py index 6860f45578be..c594f1591fa7 100644 --- a/firestore/google/cloud/firestore_v1beta1/query.py +++ b/firestore/google/cloud/firestore_v1beta1/query.py @@ -563,8 +563,7 @@ def _normalize_projection(projection): return projection - @staticmethod - def _normalize_cursor(cursor, orders): + def _normalize_cursor(self, cursor, orders): """Helper: convert cursor to a list of values based on orders.""" if cursor is None: return @@ -598,6 +597,17 @@ def _normalize_cursor(cursor, orders): msg = _INVALID_CURSOR_TRANSFORM raise ValueError(msg) + try: + name_index = order_keys.index("__name__") + except ValueError: + pass + else: + name = document_fields[name_index] + if "/" not in name: + document_fields[name_index] = "{}/{}/{}".format( + self._client._database_string, "/".join(self._parent._path), name + ) + return document_fields, before def _to_protobuf(self): diff --git a/firestore/tests/unit/test_query.py b/firestore/tests/unit/test_query.py index 2a71f3ec7391..4dd15240fd36 100644 --- a/firestore/tests/unit/test_query.py +++ b/firestore/tests/unit/test_query.py @@ -603,6 +603,36 @@ def test__normalize_cursor_as_dict_hit(self): self.assertEqual(query._normalize_cursor(cursor, query._orders), ([1], True)) + def test__normalize_cursor_w___name___w_slash(self): + db_string = "projects/my-project/database/(default)" + client = mock.Mock(spec=["_database_string"]) + client._database_string = db_string + parent = mock.Mock(spec=["_path", "_client"]) + parent._client = client + parent._path = ["C"] + query = self._make_one(parent).order_by("__name__", "ASCENDING") + expected = "{}/C/b".format(db_string) + cursor = ([expected], True) + + self.assertEqual( + query._normalize_cursor(cursor, query._orders), ([expected], True) + ) + + def test__normalize_cursor_w___name___wo_slash(self): + db_string = "projects/my-project/database/(default)" + client = mock.Mock(spec=["_database_string"]) + client._database_string = db_string + parent = mock.Mock(spec=["_path", "_client"]) + parent._client = client + parent._path = ["C"] + query = self._make_one(parent).order_by("__name__", "ASCENDING") + cursor = (["b"], True) + expected = "{}/C/b".format(db_string) + + self.assertEqual( + query._normalize_cursor(cursor, query._orders), ([expected], True) + ) + def test__to_protobuf_all_fields(self): from google.protobuf import wrappers_pb2 from google.cloud.firestore_v1beta1.gapic import enums From 0d7ce9efcc59c4cffbd738e36ab05a8393001ffa Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 3 Dec 2018 18:29:22 -0500 Subject: [PATCH 2/2] Tighten implementation of '_normalize_cursor'. --- .../google/cloud/firestore_v1beta1/query.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/firestore/google/cloud/firestore_v1beta1/query.py b/firestore/google/cloud/firestore_v1beta1/query.py index c594f1591fa7..e170e6405aeb 100644 --- a/firestore/google/cloud/firestore_v1beta1/query.py +++ b/firestore/google/cloud/firestore_v1beta1/query.py @@ -592,20 +592,17 @@ def _normalize_cursor(self, cursor, orders): raise ValueError(msg) _transform_bases = (transforms.Sentinel, transforms._ValueList) - for field in document_fields: + + for index, key_field in enumerate(zip(order_keys, document_fields)): + key, field = key_field + if isinstance(field, _transform_bases): msg = _INVALID_CURSOR_TRANSFORM raise ValueError(msg) - try: - name_index = order_keys.index("__name__") - except ValueError: - pass - else: - name = document_fields[name_index] - if "/" not in name: - document_fields[name_index] = "{}/{}/{}".format( - self._client._database_string, "/".join(self._parent._path), name + if key == "__name__" and "/" not in field: + document_fields[index] = "{}/{}/{}".format( + self._client._database_string, "/".join(self._parent._path), field ) return document_fields, before