From 564da9da2424bd2ac82ae1c4de84a28a1ec27f90 Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Thu, 18 Apr 2019 12:24:51 -0700
Subject: [PATCH 01/18] Initial plumbing for collection group queries

---
 firestore/google/cloud/firestore_v1/client.py | 23 +++++++++++++++++++
 firestore/google/cloud/firestore_v1/query.py  |  5 +++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/firestore/google/cloud/firestore_v1/client.py b/firestore/google/cloud/firestore_v1/client.py
index 8c7c3f660807..49f7534fae5f 100644
--- a/firestore/google/cloud/firestore_v1/client.py
+++ b/firestore/google/cloud/firestore_v1/client.py
@@ -26,6 +26,7 @@
 from google.cloud.client import ClientWithProject
 
 from google.cloud.firestore_v1 import _helpers
+from google.cloud.firestore_v1 import query
 from google.cloud.firestore_v1 import types
 from google.cloud.firestore_v1.batch import WriteBatch
 from google.cloud.firestore_v1.collection import CollectionReference
@@ -179,6 +180,28 @@ def collection(self, *collection_path):
 
         return CollectionReference(*path, client=self)
 
+    def collection_group(self, collection_id):
+        """
+        Creates and returns a new Query that includes all documents in the
+        database that are contained in a collection or subcollection with the
+        given collection_id.
+   
+        .. code-block:: python
+        
+            >>> query = firestore.collection_group('mygroup')
+
+        @param {string} collectionId Identifies the collections to query over.
+        Every collection or subcollection with this ID as the last segment of its
+        path will be included. Cannot contain a slash.
+        @returns {Query} The created Query.
+        """    
+        if '/' in collection_id:
+            raise ValueError("Invalid collection_id " + collection_id + ". Collection IDs must not contain '/'.")
+        
+        collection = self.collection(collection_id)
+        return query.Query(collection, all_descendants=True)
+  
+
     def document(self, *document_path):
         """Get a reference to a document in a collection.
 
diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 6c6239989e8f..13db09e438be 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -128,6 +128,7 @@ def __init__(
         offset=None,
         start_at=None,
         end_at=None,
+        all_descendants=None,
     ):
         self._parent = parent
         self._projection = projection
@@ -137,6 +138,7 @@ def __init__(
         self._offset = offset
         self._start_at = start_at
         self._end_at = end_at
+        self._all_descendants=all_descendants
 
     def __eq__(self, other):
         if not isinstance(other, self.__class__):
@@ -679,7 +681,8 @@ def _to_protobuf(self):
             "select": projection,
             "from": [
                 query_pb2.StructuredQuery.CollectionSelector(
-                    collection_id=self._parent.id
+                    collection_id=self._parent.id,
+                    all_descendants=self._all_descendants,
                 )
             ],
             "where": self._filters_pb(),

From 84a368404497c9d8b03c39532d9447833597f779 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 18 Apr 2019 16:30:30 -0400
Subject: [PATCH 02/18] Don't assume direct children for collection group
 queries.

---
 firestore/google/cloud/firestore_v1/query.py | 40 ++++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 13db09e438be..6f1c33070321 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -742,9 +742,14 @@ def stream(self, transaction=None):
         )
 
         for response in response_iterator:
-            snapshot = _query_response_to_snapshot(
-                response, self._parent, expected_prefix
-            )
+            if self._all_descendants:
+                snapshot = _collection_group_query_response_to_snapshot(
+                    response, self._parent
+                )
+            else:
+                snapshot = _query_response_to_snapshot(
+                    response, self._parent, expected_prefix
+                )
             if snapshot is not None:
                 yield snapshot
 
@@ -971,3 +976,32 @@ def _query_response_to_snapshot(response_pb, collection, expected_prefix):
         update_time=response_pb.document.update_time,
     )
     return snapshot
+
+
+def _collection_group_query_response_to_snapshot(response_pb, collection):
+    """Parse a query response protobuf to a document snapshot.
+
+    Args:
+        response_pb (google.cloud.proto.firestore.v1.\
+            firestore_pb2.RunQueryResponse): A
+        collection (~.firestore_v1.collection.CollectionReference): A
+            reference to the collection that initiated the query.
+
+    Returns:
+        Optional[~.firestore.document.DocumentSnapshot]: A
+        snapshot of the data returned in the query. If ``response_pb.document``
+        is not set, the snapshot will be :data:`None`.
+    """
+    if not response_pb.HasField("document"):
+        return None
+    reference = collection._client.document(response_pb.document.name)
+    data = _helpers.decode_dict(response_pb.document.fields, collection._client)
+    snapshot = document.DocumentSnapshot(
+        reference,
+        data,
+        exists=True,
+        read_time=response_pb.read_time,
+        create_time=response_pb.document.create_time,
+        update_time=response_pb.document.update_time,
+    )
+    return snapshot

From bf36c6116eec80768d4a3c91d5033d22ea73cbb5 Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Fri, 19 Apr 2019 10:48:51 -0700
Subject: [PATCH 03/18] trim document path to DocumentReference

---
 firestore/google/cloud/firestore_v1/client.py | 7 +++++++
 firestore/tests/system.py                     | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/firestore/google/cloud/firestore_v1/client.py b/firestore/google/cloud/firestore_v1/client.py
index 49f7534fae5f..84e34fbb75d1 100644
--- a/firestore/google/cloud/firestore_v1/client.py
+++ b/firestore/google/cloud/firestore_v1/client.py
@@ -238,6 +238,13 @@ def document(self, *document_path):
         else:
             path = document_path
 
+        # DocumentReference takes a relative path. Strip the database string if present.
+        base_path = self._database_string
+        joined_path = _helpers.DOCUMENT_PATH_DELIMITER.join(path)
+        if joined_path.startswith(base_path):
+            joined_path = joined_path[len(base_path):]
+        path = joined_path.split(_helpers.DOCUMENT_PATH_DELIMITER)
+
         return DocumentReference(*path, client=self)
 
     @staticmethod
diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 75ae3fb2d4c6..97e69f51e252 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -797,6 +797,11 @@ def on_snapshot(docs, changes, read_time):
             "Expected the last document update to update born: " + str(on_snapshot.born)
         )
 
+def test_collection_group_query(client, cleanup):
+    db = client
+    group = db.collection_group('a')
+    for i in group.stream():
+        print(i)
 
 def test_watch_query(client, cleanup):
     db = client

From 796e59cb6bf381583ba25403af6a5b99c302209b Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Fri, 19 Apr 2019 13:04:35 -0700
Subject: [PATCH 04/18] add unit tests

---
 firestore/tests/unit/v1/test_client.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/firestore/tests/unit/v1/test_client.py b/firestore/tests/unit/v1/test_client.py
index 968d13487249..0c7829e87a69 100644
--- a/firestore/tests/unit/v1/test_client.py
+++ b/firestore/tests/unit/v1/test_client.py
@@ -130,6 +130,24 @@ def test_collection_factory_nested(self):
         self.assertIs(collection2._client, client)
         self.assertIsInstance(collection2, CollectionReference)
 
+    def test_collection_group(self):
+        from google.cloud.firestore_v1.collection import CollectionReference
+
+        client = self._make_default_one()
+        query = client.collection_group('collectionId').where('foo', '==', 'bar')
+
+        assert query._field_filters[0].field.field_path  == 'foo'
+        assert query._field_filters[0].value.string_value == 'bar'
+        assert query._field_filters[0].op == query._field_filters[0].EQUAL
+        assert query._parent.id == 'collectionId
+
+    def test_collection_group_no_slashes(self):
+        from google.cloud.firestore_v1.collection import CollectionReference
+
+        client = self._make_default_one()
+        with self.assertRaises(ValueError):
+            query = client.collection_group('foo/bar')
+
     def test_document_factory(self):
         from google.cloud.firestore_v1.document import DocumentReference
 

From c79aba40159670cbfab6fa38628445c692ecb3e2 Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Fri, 19 Apr 2019 13:49:49 -0700
Subject: [PATCH 05/18] ensure all_descendants is set after calling other query
 methods

---
 firestore/google/cloud/firestore_v1/query.py | 11 ++++++++++-
 firestore/tests/unit/v1/test_client.py       |  3 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 6f1c33070321..956d6e49aa80 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -111,6 +111,10 @@ class Query(object):
             any matching documents will be included in the result set.
             When the query is formed, the document values
             will be used in the order given by ``orders``.
+        all_descendants (Optional[bool]): When false, selects only collections
+            that are immediate children of the `parent` specified in the
+            containing `RunQueryRequest`. When true, selects all descendant
+            collections.
     """
 
     ASCENDING = "ASCENDING"
@@ -128,7 +132,7 @@ def __init__(
         offset=None,
         start_at=None,
         end_at=None,
-        all_descendants=None,
+        all_descendants=False,
     ):
         self._parent = parent
         self._projection = projection
@@ -205,6 +209,7 @@ def select(self, field_paths):
             offset=self._offset,
             start_at=self._start_at,
             end_at=self._end_at,
+            all_descendants=self._all_descendants,
         )
 
     def where(self, field_path, op_string, value):
@@ -272,6 +277,7 @@ def where(self, field_path, op_string, value):
             offset=self._offset,
             start_at=self._start_at,
             end_at=self._end_at,
+            all_descendants=self._all_descendants,
         )
 
     @staticmethod
@@ -323,6 +329,7 @@ def order_by(self, field_path, direction=ASCENDING):
             offset=self._offset,
             start_at=self._start_at,
             end_at=self._end_at,
+            all_descendants=self._all_descendants,
         )
 
     def limit(self, count):
@@ -348,6 +355,7 @@ def limit(self, count):
             offset=self._offset,
             start_at=self._start_at,
             end_at=self._end_at,
+            all_descendants=self._all_descendants,
         )
 
     def offset(self, num_to_skip):
@@ -374,6 +382,7 @@ def offset(self, num_to_skip):
             offset=num_to_skip,
             start_at=self._start_at,
             end_at=self._end_at,
+            all_descendants=self._all_descendants,
         )
 
     def _cursor_helper(self, document_fields, before, start):
diff --git a/firestore/tests/unit/v1/test_client.py b/firestore/tests/unit/v1/test_client.py
index 0c7829e87a69..c97f2cd49b04 100644
--- a/firestore/tests/unit/v1/test_client.py
+++ b/firestore/tests/unit/v1/test_client.py
@@ -136,10 +136,11 @@ def test_collection_group(self):
         client = self._make_default_one()
         query = client.collection_group('collectionId').where('foo', '==', 'bar')
 
+        assert query._all_descendants == True
         assert query._field_filters[0].field.field_path  == 'foo'
         assert query._field_filters[0].value.string_value == 'bar'
         assert query._field_filters[0].op == query._field_filters[0].EQUAL
-        assert query._parent.id == 'collectionId
+        assert query._parent.id == 'collectionId'
 
     def test_collection_group_no_slashes(self):
         from google.cloud.firestore_v1.collection import CollectionReference

From b606d889f673b077fb43373c856c14e144d13991 Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Fri, 19 Apr 2019 19:02:54 -0700
Subject: [PATCH 06/18] port test for node impl

---
 firestore/tests/system.py                     | 33 +++++++++++++++----
 .../_protocol/streaming_pull_manager.py       | 13 ++++----
 pubsub/tests/system.py                        |  1 -
 3 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 97e69f51e252..22ac41354e59 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -482,7 +482,6 @@ def test_collection_add(client, cleanup):
     assert set(collection2.list_documents()) == {document_ref3, document_ref4}
     assert set(collection3.list_documents()) == {document_ref5}
 
-
 def test_query_stream(client, cleanup):
     sub_collection = "child" + unique_resource_id("-")
     collection = client.collection("collek", "shun", sub_collection)
@@ -633,6 +632,32 @@ def test_query_unary(client, cleanup):
     assert len(data1) == 1
     assert math.isnan(data1[field_name])
 
+def test_collection_group_queries(client, cleanup):
+    collection_group = "child" + unique_resource_id("-")
+
+    doc_paths = [
+      'abc/123/' + collection_group + '/cg-doc1',
+      'abc/123/' + collection_group + '/cg-doc2',
+      collection_group + '/cg-doc3',
+      collection_group + '/cg-doc4',
+      'def/456/' + collection_group + '/cg-doc5',
+      collection_group + '/virtual-doc/nested-coll/not-cg-doc',
+      'x' + collection_group + '/not-cg-doc',
+      collection_group + 'x/not-cg-doc',
+      'abc/123/' + collection_group + 'x/not-cg-doc',
+      'abc/123/x' + collection_group + '/not-cg-doc',
+      'abc/' + collection_group
+    ];
+    
+    batch = client.batch()
+    for doc_path in doc_paths:
+        doc_ref = client.document(doc_path)
+        batch.set(doc_ref, {'x': 1});
+    
+    batch.commit()
+
+    results = [i for i in client.collection_group(collection_group).stream()]
+    assert [i.id for i in results] == ['cg-doc1', 'cg-doc2', 'cg-doc3', 'cg-doc4', 'cg-doc5']
 
 def test_get_all(client, cleanup):
     collection_name = "get-all" + unique_resource_id("-")
@@ -797,12 +822,6 @@ def on_snapshot(docs, changes, read_time):
             "Expected the last document update to update born: " + str(on_snapshot.born)
         )
 
-def test_collection_group_query(client, cleanup):
-    db = client
-    group = db.collection_group('a')
-    for i in group.stream():
-        print(i)
-
 def test_watch_query(client, cleanup):
     db = client
     doc_ref = db.collection(u"users").document(u"alovelace" + unique_resource_id())
diff --git a/pubsub/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py b/pubsub/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py
index acf514775779..d025fa71368f 100644
--- a/pubsub/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py
+++ b/pubsub/google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py
@@ -193,12 +193,10 @@ def load(self):
         if self._leaser is None:
             return 0
 
-        return max(
-            [
-                self._leaser.message_count / self._flow_control.max_messages,
-                self._leaser.bytes / self._flow_control.max_bytes,
-            ]
-        )
+        messages_percent = self._leaser.message_count / self._flow_control.max_messages
+        bytes_percent = self._leaser.bytes / self._flow_control.max_bytes
+        print(f"{messages_percent}, {bytes_percent}")
+        return max(messages_percent, bytes_percent)
 
     def add_close_callback(self, callback):
         """Schedules a callable when the manager closes.
@@ -210,10 +208,12 @@ def add_close_callback(self, callback):
 
     def maybe_pause_consumer(self):
         """Check the current load and pause the consumer if needed."""
+        print(self.load)
         if self.load >= 1.0:
             if self._consumer is not None and not self._consumer.is_paused:
                 _LOGGER.debug("Message backlog over load at %.2f, pausing.", self.load)
                 self._consumer.pause()
+                print('paused')
 
     def maybe_resume_consumer(self):
         """Check the current load and resume the consumer if needed."""
@@ -227,6 +227,7 @@ def maybe_resume_consumer(self):
             return
 
         if self.load < self.flow_control.resume_threshold:
+            print('resuming')
             self._consumer.resume()
         else:
             _LOGGER.debug("Did not resume, current load is %s", self.load)
diff --git a/pubsub/tests/system.py b/pubsub/tests/system.py
index e6001f8e7801..80349240fabb 100644
--- a/pubsub/tests/system.py
+++ b/pubsub/tests/system.py
@@ -65,7 +65,6 @@ def cleanup():
     for to_call, argument in registry:
         to_call(argument)
 
-
 def test_publish_messages(publisher, topic_path, cleanup):
     futures = []
     # Make sure the topic gets deleted.

From 10df4e9db919d038fd395de3512382abe458e9cb Mon Sep 17 00:00:00 2001
From: Chris Wilcox <crwilcox@google.com>
Date: Fri, 19 Apr 2019 20:29:24 -0700
Subject: [PATCH 07/18] port tests from node impl

---
 firestore/tests/system.py | 121 ++++++++++++++++++++++++++++++++------
 1 file changed, 104 insertions(+), 17 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 22ac41354e59..615a32209024 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -482,6 +482,7 @@ def test_collection_add(client, cleanup):
     assert set(collection2.list_documents()) == {document_ref3, document_ref4}
     assert set(collection3.list_documents()) == {document_ref5}
 
+
 def test_query_stream(client, cleanup):
     sub_collection = "child" + unique_resource_id("-")
     collection = client.collection("collek", "shun", sub_collection)
@@ -632,32 +633,117 @@ def test_query_unary(client, cleanup):
     assert len(data1) == 1
     assert math.isnan(data1[field_name])
 
+
 def test_collection_group_queries(client, cleanup):
-    collection_group = "child" + unique_resource_id("-")
+    collection_group = "col" + unique_resource_id("-")
 
     doc_paths = [
-      'abc/123/' + collection_group + '/cg-doc1',
-      'abc/123/' + collection_group + '/cg-doc2',
-      collection_group + '/cg-doc3',
-      collection_group + '/cg-doc4',
-      'def/456/' + collection_group + '/cg-doc5',
-      collection_group + '/virtual-doc/nested-coll/not-cg-doc',
-      'x' + collection_group + '/not-cg-doc',
-      collection_group + 'x/not-cg-doc',
-      'abc/123/' + collection_group + 'x/not-cg-doc',
-      'abc/123/x' + collection_group + '/not-cg-doc',
-      'abc/' + collection_group
-    ];
-    
+        "abc/123/" + collection_group + "/cg-doc1",
+        "abc/123/" + collection_group + "/cg-doc2",
+        collection_group + "/cg-doc3",
+        collection_group + "/cg-doc4",
+        "def/456/" + collection_group + "/cg-doc5",
+        collection_group + "/virtual-doc/nested-coll/not-cg-doc",
+        "x" + collection_group + "/not-cg-doc",
+        collection_group + "x/not-cg-doc",
+        "abc/123/" + collection_group + "x/not-cg-doc",
+        "abc/123/x" + collection_group + "/not-cg-doc",
+        "abc/" + collection_group,
+    ]
+
     batch = client.batch()
     for doc_path in doc_paths:
         doc_ref = client.document(doc_path)
-        batch.set(doc_ref, {'x': 1});
-    
+        batch.set(doc_ref, {"x": 1})
+
     batch.commit()
 
     results = [i for i in client.collection_group(collection_group).stream()]
-    assert [i.id for i in results] == ['cg-doc1', 'cg-doc2', 'cg-doc3', 'cg-doc4', 'cg-doc5']
+    assert [i.id for i in results] == [
+        "cg-doc1",
+        "cg-doc2",
+        "cg-doc3",
+        "cg-doc4",
+        "cg-doc5",
+    ]
+
+
+# def test_collection_group_queries_startat_endat(client, cleanup):
+#     collection_group = "col" + unique_resource_id("-")
+
+#     doc_paths = [
+#         "a/a/" + collection_group + "/cg-doc1",
+#         "a/b/a/b/" + collection_group + "/cg-doc2",
+#         "a/b/" + collection_group + "/cg-doc3",
+#         "a/b/c/d/" + collection_group + "/cg-doc4",
+#         "a/c/" + collection_group + "/cg-doc5",
+#         collection_group + "/cg-doc6",
+#         "a/b/nope/nope",
+#     ]
+
+#     batch = client.batch()
+#     for doc_path in doc_paths:
+#         doc_ref = client.document(doc_path)
+#         batch.set(doc_ref, {"x": doc_path})
+
+#     batch.commit()
+
+#     query_snapshot = (
+#         client.collection_group(collection_group)
+#         .order_by('documentId')
+#         .start_at("a/b")
+#         .end_at("a/b0")
+#         .stream()
+#     )
+#     assert [i.id for i in query_snapshot], ["cg-doc2", "cg-doc3", "cg-doc4"]
+
+#     query_snapshot = (
+#         firestore.collectionGroup(collectionGroup)
+#         .order_by('documentId')
+#         .start_after("a/b")
+#         .end_before("a/b/" + collection_group + "/cg-doc3")
+#         .stream()
+#     )
+#     assert [i.id for i in query_snapshot], ["cg-doc2"]
+
+
+# def test_collection_group_queries_filters(client, cleanup):
+#     collection_group = "col" + unique_resource_id("-")
+
+#     doc_paths = [
+#         "a/a/" + collection_group + "/cg-doc1",
+#         "a/b/a/b/" + collection_group + "/cg-doc2",
+#         "a/b/" + collection_group + "/cg-doc3",
+#         "a/b/c/d/" + collection_group + "/cg-doc4",
+#         "a/c/" + collection_group + "/cg-doc5",
+#         collection_group + "/cg-doc6",
+#         "a/b/nope/nope",
+#     ]
+
+#     batch = client.batch()
+
+#     for doc_path in doc_paths:
+#         doc_ref = client.document(doc_path)
+#         batch.set(doc_ref, {"x": 1})
+
+#     batch.commit()
+
+#     query_snapshot = (
+#         client.collection_group(collection_group)
+#         .where('documentId', ">=", "a/b")
+#         .where('documentId', "<=", "a/b0")
+#         .stream()
+#     )
+#     assert [i.id for i in query_snapshot], ["cg-doc2", "cg-doc3", "cg-doc4"]
+
+#     query_snapshot = (
+#         client.collection_group(collection_group)
+#         .where('documentId', ">", "a/b")
+#         .where('documentId', "<", "a/b/" + collection_group + "/cg-doc3")
+#         .stream()
+#     )
+#     assert [i.id for i in query_snapshot], ["cg-doc2"]
+
 
 def test_get_all(client, cleanup):
     collection_name = "get-all" + unique_resource_id("-")
@@ -822,6 +908,7 @@ def on_snapshot(docs, changes, read_time):
             "Expected the last document update to update born: " + str(on_snapshot.born)
         )
 
+
 def test_watch_query(client, cleanup):
     db = client
     doc_ref = db.collection(u"users").document(u"alovelace" + unique_resource_id())

From 89b97b4824b70913c8443df7010a083ecda20074 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 23 Apr 2019 14:49:59 -0400
Subject: [PATCH 08/18] Fix collection group test on Python 2.7.

Blacken.
---
 firestore/google/cloud/firestore_v1/client.py | 15 +++++++++------
 firestore/google/cloud/firestore_v1/query.py  |  5 ++---
 firestore/tests/unit/v1/test_client.py        | 10 +++++-----
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/client.py b/firestore/google/cloud/firestore_v1/client.py
index 84e34fbb75d1..aac9706c1d87 100644
--- a/firestore/google/cloud/firestore_v1/client.py
+++ b/firestore/google/cloud/firestore_v1/client.py
@@ -194,13 +194,16 @@ def collection_group(self, collection_id):
         Every collection or subcollection with this ID as the last segment of its
         path will be included. Cannot contain a slash.
         @returns {Query} The created Query.
-        """    
-        if '/' in collection_id:
-            raise ValueError("Invalid collection_id " + collection_id + ". Collection IDs must not contain '/'.")
-        
+        """
+        if "/" in collection_id:
+            raise ValueError(
+                "Invalid collection_id "
+                + collection_id
+                + ". Collection IDs must not contain '/'."
+            )
+
         collection = self.collection(collection_id)
         return query.Query(collection, all_descendants=True)
-  
 
     def document(self, *document_path):
         """Get a reference to a document in a collection.
@@ -242,7 +245,7 @@ def document(self, *document_path):
         base_path = self._database_string
         joined_path = _helpers.DOCUMENT_PATH_DELIMITER.join(path)
         if joined_path.startswith(base_path):
-            joined_path = joined_path[len(base_path):]
+            joined_path = joined_path[len(base_path) :]
         path = joined_path.split(_helpers.DOCUMENT_PATH_DELIMITER)
 
         return DocumentReference(*path, client=self)
diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 956d6e49aa80..2ee6c72fd39f 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -142,7 +142,7 @@ def __init__(
         self._offset = offset
         self._start_at = start_at
         self._end_at = end_at
-        self._all_descendants=all_descendants
+        self._all_descendants = all_descendants
 
     def __eq__(self, other):
         if not isinstance(other, self.__class__):
@@ -690,8 +690,7 @@ def _to_protobuf(self):
             "select": projection,
             "from": [
                 query_pb2.StructuredQuery.CollectionSelector(
-                    collection_id=self._parent.id,
-                    all_descendants=self._all_descendants,
+                    collection_id=self._parent.id, all_descendants=self._all_descendants
                 )
             ],
             "where": self._filters_pb(),
diff --git a/firestore/tests/unit/v1/test_client.py b/firestore/tests/unit/v1/test_client.py
index c97f2cd49b04..f73f948e2d24 100644
--- a/firestore/tests/unit/v1/test_client.py
+++ b/firestore/tests/unit/v1/test_client.py
@@ -134,20 +134,20 @@ def test_collection_group(self):
         from google.cloud.firestore_v1.collection import CollectionReference
 
         client = self._make_default_one()
-        query = client.collection_group('collectionId').where('foo', '==', 'bar')
+        query = client.collection_group("collectionId").where("foo", "==", u"bar")
 
         assert query._all_descendants == True
-        assert query._field_filters[0].field.field_path  == 'foo'
-        assert query._field_filters[0].value.string_value == 'bar'
+        assert query._field_filters[0].field.field_path == "foo"
+        assert query._field_filters[0].value.string_value == u"bar"
         assert query._field_filters[0].op == query._field_filters[0].EQUAL
-        assert query._parent.id == 'collectionId'
+        assert query._parent.id == "collectionId"
 
     def test_collection_group_no_slashes(self):
         from google.cloud.firestore_v1.collection import CollectionReference
 
         client = self._make_default_one()
         with self.assertRaises(ValueError):
-            query = client.collection_group('foo/bar')
+            query = client.collection_group("foo/bar")
 
     def test_document_factory(self):
         from google.cloud.firestore_v1.document import DocumentReference

From 4a643a2ab32359381e7779db9d0e779d9c71c323 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 23 Apr 2019 17:24:50 -0400
Subject: [PATCH 09/18] Use '_all_descendants' in 'Query.__eq__'.

---
 firestore/google/cloud/firestore_v1/query.py |  1 +
 firestore/tests/unit/v1/test_query.py        | 65 +++++++++++---------
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 2ee6c72fd39f..4eaa19bbe730 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -156,6 +156,7 @@ def __eq__(self, other):
             and self._offset == other._offset
             and self._start_at == other._start_at
             and self._end_at == other._end_at
+            and self._all_descendants == other._all_descendants
         )
 
     @property
diff --git a/firestore/tests/unit/v1/test_query.py b/firestore/tests/unit/v1/test_query.py
index c67c053c7765..71ec5bf6b87d 100644
--- a/firestore/tests/unit/v1/test_query.py
+++ b/firestore/tests/unit/v1/test_query.py
@@ -45,8 +45,11 @@ def test_constructor_defaults(self):
         self.assertIsNone(query._offset)
         self.assertIsNone(query._start_at)
         self.assertIsNone(query._end_at)
+        self.assertFalse(query._all_descendants)
 
-    def _make_one_all_fields(self, limit=9876, offset=12, skip_fields=(), parent=None):
+    def _make_one_all_fields(
+        self, limit=9876, offset=12, skip_fields=(), parent=None, all_descendants=True
+    ):
         kwargs = {
             "projection": mock.sentinel.projection,
             "field_filters": mock.sentinel.filters,
@@ -55,6 +58,7 @@ def _make_one_all_fields(self, limit=9876, offset=12, skip_fields=(), parent=Non
             "offset": offset,
             "start_at": mock.sentinel.start_at,
             "end_at": mock.sentinel.end_at,
+            "all_descendants": all_descendants,
         }
         for field in skip_fields:
             kwargs.pop(field)
@@ -74,6 +78,7 @@ def test_constructor_explicit(self):
         self.assertEqual(query._offset, offset)
         self.assertIs(query._start_at, mock.sentinel.start_at)
         self.assertIs(query._end_at, mock.sentinel.end_at)
+        self.assertTrue(query._all_descendants)
 
     def test__client_property(self):
         parent = mock.Mock(_client=mock.sentinel.client, spec=["_client"])
@@ -81,75 +86,79 @@ def test__client_property(self):
         self.assertIs(query._client, mock.sentinel.client)
 
     def test___eq___other_type(self):
-        client = self._make_one_all_fields()
+        query = self._make_one_all_fields()
         other = object()
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_parent(self):
         parent = mock.sentinel.parent
         other_parent = mock.sentinel.other_parent
-        client = self._make_one_all_fields(parent=parent)
+        query = self._make_one_all_fields(parent=parent)
         other = self._make_one_all_fields(parent=other_parent)
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_projection(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, skip_fields=("projection",))
-        client._projection = mock.sentinel.projection
+        query = self._make_one_all_fields(parent=parent, skip_fields=("projection",))
+        query._projection = mock.sentinel.projection
         other = self._make_one_all_fields(parent=parent, skip_fields=("projection",))
         other._projection = mock.sentinel.other_projection
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_field_filters(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(
-            parent=parent, skip_fields=("field_filters",)
-        )
-        client._field_filters = mock.sentinel.field_filters
+        query = self._make_one_all_fields(parent=parent, skip_fields=("field_filters",))
+        query._field_filters = mock.sentinel.field_filters
         other = self._make_one_all_fields(parent=parent, skip_fields=("field_filters",))
         other._field_filters = mock.sentinel.other_field_filters
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_orders(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, skip_fields=("orders",))
-        client._orders = mock.sentinel.orders
+        query = self._make_one_all_fields(parent=parent, skip_fields=("orders",))
+        query._orders = mock.sentinel.orders
         other = self._make_one_all_fields(parent=parent, skip_fields=("orders",))
         other._orders = mock.sentinel.other_orders
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_limit(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, limit=10)
+        query = self._make_one_all_fields(parent=parent, limit=10)
         other = self._make_one_all_fields(parent=parent, limit=20)
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_offset(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, offset=10)
+        query = self._make_one_all_fields(parent=parent, offset=10)
         other = self._make_one_all_fields(parent=parent, offset=20)
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_start_at(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, skip_fields=("start_at",))
-        client._start_at = mock.sentinel.start_at
+        query = self._make_one_all_fields(parent=parent, skip_fields=("start_at",))
+        query._start_at = mock.sentinel.start_at
         other = self._make_one_all_fields(parent=parent, skip_fields=("start_at",))
         other._start_at = mock.sentinel.other_start_at
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
 
     def test___eq___different_end_at(self):
         parent = mock.sentinel.parent
-        client = self._make_one_all_fields(parent=parent, skip_fields=("end_at",))
-        client._end_at = mock.sentinel.end_at
+        query = self._make_one_all_fields(parent=parent, skip_fields=("end_at",))
+        query._end_at = mock.sentinel.end_at
         other = self._make_one_all_fields(parent=parent, skip_fields=("end_at",))
         other._end_at = mock.sentinel.other_end_at
-        self.assertFalse(client == other)
+        self.assertFalse(query == other)
+
+    def test___eq___different_all_descendants(self):
+        parent = mock.sentinel.parent
+        query = self._make_one_all_fields(parent=parent, all_descendants=True)
+        other = self._make_one_all_fields(parent=parent, all_descendants=False)
+        self.assertFalse(query == other)
 
     def test___eq___hit(self):
-        client = self._make_one_all_fields()
+        query = self._make_one_all_fields()
         other = self._make_one_all_fields()
-        self.assertTrue(client == other)
+        self.assertTrue(query == other)
 
     def _compare_queries(self, query1, query2, attr_name):
         attrs1 = query1.__dict__.copy()

From e699f3306c6afbc645d25dbfaa6154ac7e9838d8 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 23 Apr 2019 17:25:22 -0400
Subject: [PATCH 10/18] Ensure '_all_descendants' propagates when narrowing
 query.

---
 firestore/google/cloud/firestore_v1/query.py |  1 +
 firestore/tests/unit/v1/test_query.py        | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/query.py b/firestore/google/cloud/firestore_v1/query.py
index 4eaa19bbe730..12141cc806b5 100644
--- a/firestore/google/cloud/firestore_v1/query.py
+++ b/firestore/google/cloud/firestore_v1/query.py
@@ -430,6 +430,7 @@ def _cursor_helper(self, document_fields, before, start):
             "orders": self._orders,
             "limit": self._limit,
             "offset": self._offset,
+            "all_descendants": self._all_descendants,
         }
         if start:
             query_kwargs["start_at"] = cursor_pair
diff --git a/firestore/tests/unit/v1/test_query.py b/firestore/tests/unit/v1/test_query.py
index 71ec5bf6b87d..ad8a0ceacf7d 100644
--- a/firestore/tests/unit/v1/test_query.py
+++ b/firestore/tests/unit/v1/test_query.py
@@ -190,7 +190,7 @@ def test_select_invalid_path(self):
             query.select(["*"])
 
     def test_select(self):
-        query1 = self._make_one_all_fields()
+        query1 = self._make_one_all_fields(all_descendants=True)
 
         field_paths2 = ["foo", "bar"]
         query2 = query1.select(field_paths2)
@@ -222,7 +222,9 @@ def test_where(self):
         from google.cloud.firestore_v1.proto import document_pb2
         from google.cloud.firestore_v1.proto import query_pb2
 
-        query = self._make_one_all_fields(skip_fields=("field_filters",))
+        query = self._make_one_all_fields(
+            skip_fields=("field_filters",), all_descendants=True
+        )
         new_query = query.where("power.level", ">", 9000)
 
         self.assertIsNot(query, new_query)
@@ -311,7 +313,9 @@ def test_order_by(self):
         from google.cloud.firestore_v1.gapic import enums
 
         klass = self._get_target_class()
-        query1 = self._make_one_all_fields(skip_fields=("orders",))
+        query1 = self._make_one_all_fields(
+            skip_fields=("orders",), all_descendants=True
+        )
 
         field_path2 = "a"
         query2 = query1.order_by(field_path2)
@@ -335,7 +339,7 @@ def test_order_by(self):
         self._compare_queries(query2, query3, "_orders")
 
     def test_limit(self):
-        query1 = self._make_one_all_fields()
+        query1 = self._make_one_all_fields(all_descendants=True)
 
         limit2 = 100
         query2 = query1.limit(limit2)
@@ -353,7 +357,7 @@ def test_limit(self):
         self._compare_queries(query2, query3, "_limit")
 
     def test_offset(self):
-        query1 = self._make_one_all_fields()
+        query1 = self._make_one_all_fields(all_descendants=True)
 
         offset2 = 23
         query2 = query1.offset(offset2)
@@ -391,6 +395,7 @@ def _make_snapshot(docref, values):
     def test__cursor_helper_w_dict(self):
         values = {"a": 7, "b": "foo"}
         query1 = self._make_one(mock.sentinel.parent)
+        query1._all_descendants = True
         query2 = query1._cursor_helper(values, True, True)
 
         self.assertIs(query2._parent, mock.sentinel.parent)
@@ -400,6 +405,7 @@ def test__cursor_helper_w_dict(self):
         self.assertIsNone(query2._limit)
         self.assertIsNone(query2._offset)
         self.assertIsNone(query2._end_at)
+        self.assertTrue(query2._all_descendants)
 
         cursor, before = query2._start_at
 
@@ -477,7 +483,9 @@ def test__cursor_helper_w_snapshot(self):
 
     def test_start_at(self):
         collection = self._make_collection("here")
-        query1 = self._make_one_all_fields(parent=collection, skip_fields=("orders",))
+        query1 = self._make_one_all_fields(
+            parent=collection, skip_fields=("orders",), all_descendants=True
+        )
         query2 = query1.order_by("hi")
 
         document_fields3 = {"hi": "mom"}

From 3d9b9018da37e3dd2dd1e18e88126eacc25a2ccb Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 23 Apr 2019 17:29:15 -0400
Subject: [PATCH 11/18] Refactor collection group system tests.

Skip the one for 'where', because it requires a custom index.
---
 firestore/tests/system.py | 162 ++++++++++++++++++++------------------
 1 file changed, 85 insertions(+), 77 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 615a32209024..eb731fbdfd39 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -658,91 +658,99 @@ def test_collection_group_queries(client, cleanup):
 
     batch.commit()
 
-    results = [i for i in client.collection_group(collection_group).stream()]
-    assert [i.id for i in results] == [
+    query = client.collection_group(collection_group)
+    snapshots = list(query.stream())
+    found = [snapshot.id for snapshot in snapshots]
+    expected = [
         "cg-doc1",
         "cg-doc2",
         "cg-doc3",
         "cg-doc4",
         "cg-doc5",
     ]
+    assert found == expected
 
 
-# def test_collection_group_queries_startat_endat(client, cleanup):
-#     collection_group = "col" + unique_resource_id("-")
-
-#     doc_paths = [
-#         "a/a/" + collection_group + "/cg-doc1",
-#         "a/b/a/b/" + collection_group + "/cg-doc2",
-#         "a/b/" + collection_group + "/cg-doc3",
-#         "a/b/c/d/" + collection_group + "/cg-doc4",
-#         "a/c/" + collection_group + "/cg-doc5",
-#         collection_group + "/cg-doc6",
-#         "a/b/nope/nope",
-#     ]
-
-#     batch = client.batch()
-#     for doc_path in doc_paths:
-#         doc_ref = client.document(doc_path)
-#         batch.set(doc_ref, {"x": doc_path})
-
-#     batch.commit()
-
-#     query_snapshot = (
-#         client.collection_group(collection_group)
-#         .order_by('documentId')
-#         .start_at("a/b")
-#         .end_at("a/b0")
-#         .stream()
-#     )
-#     assert [i.id for i in query_snapshot], ["cg-doc2", "cg-doc3", "cg-doc4"]
-
-#     query_snapshot = (
-#         firestore.collectionGroup(collectionGroup)
-#         .order_by('documentId')
-#         .start_after("a/b")
-#         .end_before("a/b/" + collection_group + "/cg-doc3")
-#         .stream()
-#     )
-#     assert [i.id for i in query_snapshot], ["cg-doc2"]
-
-
-# def test_collection_group_queries_filters(client, cleanup):
-#     collection_group = "col" + unique_resource_id("-")
-
-#     doc_paths = [
-#         "a/a/" + collection_group + "/cg-doc1",
-#         "a/b/a/b/" + collection_group + "/cg-doc2",
-#         "a/b/" + collection_group + "/cg-doc3",
-#         "a/b/c/d/" + collection_group + "/cg-doc4",
-#         "a/c/" + collection_group + "/cg-doc5",
-#         collection_group + "/cg-doc6",
-#         "a/b/nope/nope",
-#     ]
-
-#     batch = client.batch()
-
-#     for doc_path in doc_paths:
-#         doc_ref = client.document(doc_path)
-#         batch.set(doc_ref, {"x": 1})
-
-#     batch.commit()
-
-#     query_snapshot = (
-#         client.collection_group(collection_group)
-#         .where('documentId', ">=", "a/b")
-#         .where('documentId', "<=", "a/b0")
-#         .stream()
-#     )
-#     assert [i.id for i in query_snapshot], ["cg-doc2", "cg-doc3", "cg-doc4"]
-
-#     query_snapshot = (
-#         client.collection_group(collection_group)
-#         .where('documentId', ">", "a/b")
-#         .where('documentId', "<", "a/b/" + collection_group + "/cg-doc3")
-#         .stream()
-#     )
-#     assert [i.id for i in query_snapshot], ["cg-doc2"]
+def test_collection_group_queries_startat_endat(client, cleanup):
+    collection_group = "col" + unique_resource_id("-")
+
+    doc_paths = [
+        "a/a/" + collection_group + "/cg-doc1",
+        "a/b/a/b/" + collection_group + "/cg-doc2",
+        "a/b/" + collection_group + "/cg-doc3",
+        "a/b/c/d/" + collection_group + "/cg-doc4",
+        "a/c/" + collection_group + "/cg-doc5",
+        collection_group + "/cg-doc6",
+        "a/b/nope/nope",
+    ]
+
+    batch = client.batch()
+    for doc_path in doc_paths:
+        doc_ref = client.document(doc_path)
+        batch.set(doc_ref, {"x": doc_path})
+
+    batch.commit()
+
+    query = (
+        client.collection_group(collection_group)
+        .order_by("__name__")
+        .start_at([client.document("a/b")])
+        .end_at([client.document("a/b0")])
+    )
+    snapshots = list(query.stream())
+    found = set(snapshot.id for snapshot in snapshots)
+    assert found == set(["cg-doc2", "cg-doc3", "cg-doc4"])
+
+    query = (
+        client.collection_group(collection_group)
+        .order_by("__name__")
+        .start_after([client.document("a/b")])
+        .end_before([client.document("a/b/" + collection_group + "/cg-doc3")])
+    )
+    snapshots = list(query.stream())
+    found = set(snapshot.id for snapshot in snapshots)
+    assert found == set(["cg-doc2", "cg-doc4"])
+
+
+@pytest.mark.skip("where query across group requires custom index.")
+def test_collection_group_queries_filters(client, cleanup):
+    collection_group = "col" + unique_resource_id("-")
+
+    doc_paths = [
+        "a/a/" + collection_group + "/cg-doc1",
+        "a/b/a/b/" + collection_group + "/cg-doc2",
+        "a/b/" + collection_group + "/cg-doc3",
+        "a/b/c/d/" + collection_group + "/cg-doc4",
+        "a/c/" + collection_group + "/cg-doc5",
+        collection_group + "/cg-doc6",
+        "a/b/nope/nope",
+    ]
+
+    batch = client.batch()
+
+    for index, doc_path in enumerate(doc_paths):
+        doc_ref = client.document(doc_path)
+        batch.set(doc_ref, {"x": index})
+
+    batch.commit()
+
+    query = (
+        client.collection_group(collection_group)
+        .where("x", ">=", 1)
+        .where("x", "<=", len(doc_paths) - 1)
+    )
+    snapshots = list(query.stream())
+    found = set(snapshot.id for snapshot in snapshots)
+    assert found == set(["cg-doc2", "cg-doc3", "cg-doc4"])
+
+    query = (
+        client.collection_group(collection_group)
+        .where("x", ">", 0)
+        .where("x", "<", 2)
+    )
+    snapshots = list(query.stream())
+    found = set(snapshot.id for snapshot in snapshots)
+    assert found == set(["cg-doc2"])
 
 
 def test_get_all(client, cleanup):

From 115234c713fe6bb9e128957b73f29815c39ae645 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Mon, 29 Apr 2019 16:26:08 -0400
Subject: [PATCH 12/18] Match node test's collection group ID / expected
 output.

See:
https://github.com/googleapis/nodejs-firestore/pull/578/files#diff-6b8febc8d51ea01205628091b3611eacR1188
---
 firestore/tests/system.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index eb731fbdfd39..d0fbf192533f 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -635,7 +635,7 @@ def test_query_unary(client, cleanup):
 
 
 def test_collection_group_queries(client, cleanup):
-    collection_group = "col" + unique_resource_id("-")
+    collection_group = "b" + unique_resource_id("-")
 
     doc_paths = [
         "abc/123/" + collection_group + "/cg-doc1",
@@ -672,7 +672,7 @@ def test_collection_group_queries(client, cleanup):
 
 
 def test_collection_group_queries_startat_endat(client, cleanup):
-    collection_group = "col" + unique_resource_id("-")
+    collection_group = "b" + unique_resource_id("-")
 
     doc_paths = [
         "a/a/" + collection_group + "/cg-doc1",
@@ -709,12 +709,12 @@ def test_collection_group_queries_startat_endat(client, cleanup):
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)
-    assert found == set(["cg-doc2", "cg-doc4"])
+    assert found == set(["cg-doc2"])
 
 
 @pytest.mark.skip("where query across group requires custom index.")
 def test_collection_group_queries_filters(client, cleanup):
-    collection_group = "col" + unique_resource_id("-")
+    collection_group = "b" + unique_resource_id("-")
 
     doc_paths = [
         "a/a/" + collection_group + "/cg-doc1",

From 8925d62ceaadccc2d8dc29688d3712c11ecf29ea Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Mon, 29 Apr 2019 17:01:04 -0400
Subject: [PATCH 13/18] Match Node test for filter on '__name__'.

Note that this still doesn't pass, so remains skipped.
---
 firestore/tests/system.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index d0fbf192533f..01cfe52b12a8 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -736,8 +736,8 @@ def test_collection_group_queries_filters(client, cleanup):
 
     query = (
         client.collection_group(collection_group)
-        .where("x", ">=", 1)
-        .where("x", "<=", len(doc_paths) - 1)
+        .where("__name__", ">=", "a/b")
+        .where("__name__", "<=", "a/b0")
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)
@@ -745,8 +745,8 @@ def test_collection_group_queries_filters(client, cleanup):
 
     query = (
         client.collection_group(collection_group)
-        .where("x", ">", 0)
-        .where("x", "<", 2)
+        .where("__name__", ">", "a/b")
+        .where("__name__", "<", "a/b/{}/cg-doc3".format(collection_group))
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)

From 26891080329f2862e63cd13f91ddf560c5440ed8 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Mon, 29 Apr 2019 17:02:00 -0400
Subject: [PATCH 14/18] Blacken.

---
 firestore/tests/system.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 01cfe52b12a8..02185b8afad6 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -661,13 +661,7 @@ def test_collection_group_queries(client, cleanup):
     query = client.collection_group(collection_group)
     snapshots = list(query.stream())
     found = [snapshot.id for snapshot in snapshots]
-    expected = [
-        "cg-doc1",
-        "cg-doc2",
-        "cg-doc3",
-        "cg-doc4",
-        "cg-doc5",
-    ]
+    expected = ["cg-doc1", "cg-doc2", "cg-doc3", "cg-doc4", "cg-doc5"]
     assert found == expected
 
 

From 1223f3425042f571b2f0772735c0bbbb326cd8b4 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 30 Apr 2019 09:58:41 -0400
Subject: [PATCH 15/18] Fix / unskip systest for collection groups w/ filter on
 '__name__'.

---
 firestore/tests/system.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 02185b8afad6..559deca7d438 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -706,7 +706,6 @@ def test_collection_group_queries_startat_endat(client, cleanup):
     assert found == set(["cg-doc2"])
 
 
-@pytest.mark.skip("where query across group requires custom index.")
 def test_collection_group_queries_filters(client, cleanup):
     collection_group = "b" + unique_resource_id("-")
 
@@ -730,8 +729,8 @@ def test_collection_group_queries_filters(client, cleanup):
 
     query = (
         client.collection_group(collection_group)
-        .where("__name__", ">=", "a/b")
-        .where("__name__", "<=", "a/b0")
+        .where("__name__", ">=", client.document("a/b"))
+        .where("__name__", "<=", client.document("a/b0"))
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)
@@ -739,8 +738,8 @@ def test_collection_group_queries_filters(client, cleanup):
 
     query = (
         client.collection_group(collection_group)
-        .where("__name__", ">", "a/b")
-        .where("__name__", "<", "a/b/{}/cg-doc3".format(collection_group))
+        .where("__name__", ">", client.document("a/b"))
+        .where("__name__", "<", client.document("a/b/{}/cg-doc3".format(collection_group)))
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)

From 412aade0eb27020cdc514336195ec588ecd8fefa Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 30 Apr 2019 10:50:47 -0400
Subject: [PATCH 16/18] Blacken

---
 firestore/tests/system.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/firestore/tests/system.py b/firestore/tests/system.py
index 559deca7d438..a8e683629add 100644
--- a/firestore/tests/system.py
+++ b/firestore/tests/system.py
@@ -739,7 +739,9 @@ def test_collection_group_queries_filters(client, cleanup):
     query = (
         client.collection_group(collection_group)
         .where("__name__", ">", client.document("a/b"))
-        .where("__name__", "<", client.document("a/b/{}/cg-doc3".format(collection_group)))
+        .where(
+            "__name__", "<", client.document("a/b/{}/cg-doc3".format(collection_group))
+        )
     )
     snapshots = list(query.stream())
     found = set(snapshot.id for snapshot in snapshots)

From 7297f1025b38b4e17612f5656cfc1a9f1d1789a4 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 30 Apr 2019 11:51:49 -0400
Subject: [PATCH 17/18] 100% coverage.

---
 firestore/google/cloud/firestore_v1/client.py |  2 +-
 firestore/tests/unit/v1/test_client.py        | 15 +++-
 firestore/tests/unit/v1/test_query.py         | 81 +++++++++++++++++++
 3 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/client.py b/firestore/google/cloud/firestore_v1/client.py
index aac9706c1d87..de71f4d31c30 100644
--- a/firestore/google/cloud/firestore_v1/client.py
+++ b/firestore/google/cloud/firestore_v1/client.py
@@ -242,7 +242,7 @@ def document(self, *document_path):
             path = document_path
 
         # DocumentReference takes a relative path. Strip the database string if present.
-        base_path = self._database_string
+        base_path = self._database_string + "/documents/"
         joined_path = _helpers.DOCUMENT_PATH_DELIMITER.join(path)
         if joined_path.startswith(base_path):
             joined_path = joined_path[len(base_path) :]
diff --git a/firestore/tests/unit/v1/test_client.py b/firestore/tests/unit/v1/test_client.py
index f73f948e2d24..de14ba363606 100644
--- a/firestore/tests/unit/v1/test_client.py
+++ b/firestore/tests/unit/v1/test_client.py
@@ -167,7 +167,20 @@ def test_document_factory(self):
         self.assertIs(document2._client, client)
         self.assertIsInstance(document2, DocumentReference)
 
-    def test_document_factory_nested(self):
+    def test_document_factory_w_absolute_path(self):
+        from google.cloud.firestore_v1.document import DocumentReference
+
+        parts = ("rooms", "roomA")
+        client = self._make_default_one()
+        doc_path = "/".join(parts)
+        to_match = client.document(doc_path)
+        document1 = client.document(to_match._document_path)
+
+        self.assertEqual(document1._path, parts)
+        self.assertIs(document1._client, client)
+        self.assertIsInstance(document1, DocumentReference)
+
+    def test_document_factory_w_nested_path(self):
         from google.cloud.firestore_v1.document import DocumentReference
 
         client = self._make_default_one()
diff --git a/firestore/tests/unit/v1/test_query.py b/firestore/tests/unit/v1/test_query.py
index ad8a0ceacf7d..eada98cd192a 100644
--- a/firestore/tests/unit/v1/test_query.py
+++ b/firestore/tests/unit/v1/test_query.py
@@ -1287,6 +1287,47 @@ def test_stream_empty_after_first_response(self):
             metadata=client._rpc_metadata,
         )
 
+    def test_stream_w_collection_group(self):
+        # Create a minimal fake GAPIC.
+        firestore_api = mock.Mock(spec=["run_query"])
+
+        # Attach the fake GAPIC to a real client.
+        client = _make_client()
+        client._firestore_api_internal = firestore_api
+
+        # Make a **real** collection reference as parent.
+        parent = client.collection("charles")
+        other = client.collection("dora")
+
+        # Add two dummy responses to the minimal fake GAPIC.
+        _, other_prefix = other._parent_info()
+        name = "{}/bark".format(other_prefix)
+        data = {"lee": "hoop"}
+        response_pb1 = _make_query_response(name=name, data=data)
+        response_pb2 = _make_query_response()
+        firestore_api.run_query.return_value = iter([response_pb1, response_pb2])
+
+        # Execute the query and check the response.
+        query = self._make_one(parent)
+        query._all_descendants = True
+        get_response = query.stream()
+        self.assertIsInstance(get_response, types.GeneratorType)
+        returned = list(get_response)
+        self.assertEqual(len(returned), 1)
+        snapshot = returned[0]
+        to_match = other.document("bark")
+        self.assertEqual(snapshot.reference._document_path, to_match._document_path)
+        self.assertEqual(snapshot.to_dict(), data)
+
+        # Verify the mock call.
+        parent_path, _ = parent._parent_info()
+        firestore_api.run_query.assert_called_once_with(
+            parent_path,
+            query._to_protobuf(),
+            transaction=None,
+            metadata=client._rpc_metadata,
+        )
+
     @mock.patch("google.cloud.firestore_v1.query.Watch", autospec=True)
     def test_on_snapshot(self, watch):
         query = self._make_one(mock.sentinel.parent)
@@ -1554,6 +1595,46 @@ def test_response(self):
         self.assertEqual(snapshot.update_time, response_pb.document.update_time)
 
 
+class Test__collection_group_query_response_to_snapshot(unittest.TestCase):
+    @staticmethod
+    def _call_fut(response_pb, collection):
+        from google.cloud.firestore_v1.query import (
+            _collection_group_query_response_to_snapshot,
+        )
+
+        return _collection_group_query_response_to_snapshot(response_pb, collection)
+
+    def test_empty(self):
+        response_pb = _make_query_response()
+        snapshot = self._call_fut(response_pb, None)
+        self.assertIsNone(snapshot)
+
+    def test_after_offset(self):
+        skipped_results = 410
+        response_pb = _make_query_response(skipped_results=skipped_results)
+        snapshot = self._call_fut(response_pb, None)
+        self.assertIsNone(snapshot)
+
+    def test_response(self):
+        from google.cloud.firestore_v1.document import DocumentSnapshot
+
+        client = _make_client()
+        collection = client.collection("a", "b", "c")
+        other_collection = client.collection("a", "b", "d")
+        to_match = other_collection.document("gigantic")
+        data = {"a": 901, "b": True}
+        response_pb = _make_query_response(name=to_match._document_path, data=data)
+
+        snapshot = self._call_fut(response_pb, collection)
+        self.assertIsInstance(snapshot, DocumentSnapshot)
+        self.assertEqual(snapshot.reference._document_path, to_match._document_path)
+        self.assertEqual(snapshot.to_dict(), data)
+        self.assertTrue(snapshot.exists)
+        self.assertEqual(snapshot.read_time, response_pb.read_time)
+        self.assertEqual(snapshot.create_time, response_pb.document.create_time)
+        self.assertEqual(snapshot.update_time, response_pb.document.update_time)
+
+
 def _make_credentials():
     import google.auth.credentials
 

From 2ee7bbb265dbfcc41fb5fe35a8c18b593f8f76c8 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 30 Apr 2019 12:54:08 -0400
Subject: [PATCH 18/18] Lint

---
 firestore/google/cloud/firestore_v1/client.py | 4 ++--
 firestore/tests/unit/v1/test_client.py        | 8 ++------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/firestore/google/cloud/firestore_v1/client.py b/firestore/google/cloud/firestore_v1/client.py
index de71f4d31c30..06ca37e6d819 100644
--- a/firestore/google/cloud/firestore_v1/client.py
+++ b/firestore/google/cloud/firestore_v1/client.py
@@ -185,9 +185,9 @@ def collection_group(self, collection_id):
         Creates and returns a new Query that includes all documents in the
         database that are contained in a collection or subcollection with the
         given collection_id.
-   
+
         .. code-block:: python
-        
+
             >>> query = firestore.collection_group('mygroup')
 
         @param {string} collectionId Identifies the collections to query over.
diff --git a/firestore/tests/unit/v1/test_client.py b/firestore/tests/unit/v1/test_client.py
index de14ba363606..fb82b1f9d9bb 100644
--- a/firestore/tests/unit/v1/test_client.py
+++ b/firestore/tests/unit/v1/test_client.py
@@ -131,23 +131,19 @@ def test_collection_factory_nested(self):
         self.assertIsInstance(collection2, CollectionReference)
 
     def test_collection_group(self):
-        from google.cloud.firestore_v1.collection import CollectionReference
-
         client = self._make_default_one()
         query = client.collection_group("collectionId").where("foo", "==", u"bar")
 
-        assert query._all_descendants == True
+        assert query._all_descendants
         assert query._field_filters[0].field.field_path == "foo"
         assert query._field_filters[0].value.string_value == u"bar"
         assert query._field_filters[0].op == query._field_filters[0].EQUAL
         assert query._parent.id == "collectionId"
 
     def test_collection_group_no_slashes(self):
-        from google.cloud.firestore_v1.collection import CollectionReference
-
         client = self._make_default_one()
         with self.assertRaises(ValueError):
-            query = client.collection_group("foo/bar")
+            client.collection_group("foo/bar")
 
     def test_document_factory(self):
         from google.cloud.firestore_v1.document import DocumentReference