From 2dbbba027659322e60be248726c96c06e7a9e441 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Wed, 22 Sep 2021 18:03:36 -0400
Subject: [PATCH] fix: unbreak query orders w/ non-orderable operators (#453)

Closes #429.
---
 google/cloud/firestore_v1/base_query.py |  9 ++++++---
 tests/system/test_system.py             | 24 ++++++++++++++++++++++++
 tests/unit/v1/test_base_query.py        | 11 +++++++++++
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/google/cloud/firestore_v1/base_query.py b/google/cloud/firestore_v1/base_query.py
index 8502fdfb2..7a99e8dbb 100644
--- a/google/cloud/firestore_v1/base_query.py
+++ b/google/cloud/firestore_v1/base_query.py
@@ -704,9 +704,12 @@ def _normalize_orders(self) -> list:
             ]
             order_keys = [order.field.field_path for order in orders]
             for filter_ in self._field_filters:
-                field = filter_.field.field_path
-                if filter_.op in should_order and field not in order_keys:
-                    orders.append(self._make_order(field, "ASCENDING"))
+                # FieldFilter.Operator should not compare equal to
+                # UnaryFilter.Operator, but it does
+                if isinstance(filter_.op, StructuredQuery.FieldFilter.Operator):
+                    field = filter_.field.field_path
+                    if filter_.op in should_order and field not in order_keys:
+                        orders.append(self._make_order(field, "ASCENDING"))
             if not orders:
                 orders.append(self._make_order("__name__", "ASCENDING"))
             else:
diff --git a/tests/system/test_system.py b/tests/system/test_system.py
index 109029ced..cbff09ac9 100644
--- a/tests/system/test_system.py
+++ b/tests/system/test_system.py
@@ -1568,3 +1568,27 @@ def on_snapshot(docs, changes, read_time):
         raise AssertionError(
             "5 docs expected in snapshot method " + str(on_snapshot.last_doc_count)
         )
+
+
+def test_repro_429(client, cleanup):
+    # See: https://github.com/googleapis/python-firestore/issues/429
+    now = datetime.datetime.utcnow().replace(tzinfo=UTC)
+    collection = client.collection("repro-429" + UNIQUE_RESOURCE_ID)
+
+    document_ids = [f"doc-{doc_id:02d}" for doc_id in range(30)]
+    for document_id in document_ids:
+        data = {"now": now, "paymentId": None}
+        _, document = collection.add(data, document_id)
+        cleanup(document.delete)
+
+    query = collection.where("paymentId", "==", None).limit(10).order_by("__name__")
+
+    last_snapshot = None
+    for snapshot in query.stream():
+        print(f"id: {snapshot.id}")
+        last_snapshot = snapshot
+
+    query2 = query.start_after(last_snapshot)
+
+    for snapshot in query2.stream():
+        print(f"id: {snapshot.id}")
diff --git a/tests/unit/v1/test_base_query.py b/tests/unit/v1/test_base_query.py
index 7caa3799f..444ae1b47 100644
--- a/tests/unit/v1/test_base_query.py
+++ b/tests/unit/v1/test_base_query.py
@@ -726,6 +726,17 @@ def test__normalize_orders_wo_orders_w_snapshot_cursor_w_neq_where(self):
         ]
         self.assertEqual(query._normalize_orders(), expected)
 
+    def test__normalize_orders_wo_orders_w_snapshot_cursor_w_isnull_where(self):
+        values = {"a": 7, "b": "foo"}
+        docref = self._make_docref("here", "doc_id")
+        snapshot = self._make_snapshot(docref, values)
+        collection = self._make_collection("here")
+        query = self._make_one(collection).where("c", "==", None).end_at(snapshot)
+        expected = [
+            query._make_order("__name__", "ASCENDING"),
+        ]
+        self.assertEqual(query._normalize_orders(), expected)
+
     def test__normalize_orders_w_name_orders_w_none_cursor(self):
         collection = self._make_collection("here")
         query = (