Repository: googleapis/python-firestore

Issue:
Firestore: coll_ref.list_documents() retrieves complete documents first, then returns doc_refs?
The current implementation of CollectionReference.list_documents() seems to be seriously flawed.

Reading the API documentation at https://googleapis.dev/python/firestore/latest/collection.html it returns a sequence of doc_refs, which sounds like a good way to get (or count) all document references in a collection, without retrieving any unneeded data.
[Note: this is a bit like what a keys_only() query would do in Datastore mode, which doesn't really seem to have an equivalent select().stream() in native mode -> actually there is an equivalent, it's just not well documented: select([]) corresponds to select(["\_\_name\_\_"])]

So you would expect list_documents() to only retrieve doc_refs from Firestore in the back-end as well.

But what seems to happen in practice is that all document data are retrieved completely (with all fields) by the API, and then _item_to_document_ref() returns a doc_ref based on the document.

So this is actually the worst possible combination: you loose a lot of time/bandwidth retrieving all the document data and then simply discard it.

I'm not sure how this is implemented in other languages, but either list_documents() should continue returning only doc_refs but then only actually retrieve those doc_refs in the back-end. Or it if that's not technically possible for some reason, it should return actual docs, possibly with selectable field_paths like query().

Thanks,

Mike.

Discussion:

Patch:
diff --git a/google/cloud/firestore_v1/base_collection.py b/google/cloud/firestore_v1/base_collection.py
index 956c4b4..1557057 100644
--- a/google/cloud/firestore_v1/base_collection.py
+++ b/google/cloud/firestore_v1/base_collection.py
@@ -183,6 +183,10 @@ class BaseCollectionReference(object):
             "collection_id": self.id,
             "page_size": page_size,
             "show_missing": True,
+            # list_documents returns an iterator of document references, which do not
+            # include any fields. To save on data transfer, we can set a field_path mask
+            # to include no fields
+            "mask": {"field_paths": None},
         }
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 


Test patch:
diff --git a/tests/unit/v1/test_async_collection.py b/tests/unit/v1/test_async_collection.py
index 4a2f30d..866fbb0 100644
--- a/tests/unit/v1/test_async_collection.py
+++ b/tests/unit/v1/test_async_collection.py
@@ -252,6 +252,7 @@ class TestAsyncCollectionReference(aiounittest.AsyncTestCase):
                 "collection_id": collection.id,
                 "page_size": page_size,
                 "show_missing": True,
+                "mask": {"field_paths": None},
             },
             metadata=client._rpc_metadata,
             **kwargs,
diff --git a/tests/unit/v1/test_collection.py b/tests/unit/v1/test_collection.py
index b75dfdf..3e6b1d7 100644
--- a/tests/unit/v1/test_collection.py
+++ b/tests/unit/v1/test_collection.py
@@ -245,6 +245,7 @@ class TestCollectionReference(unittest.TestCase):
                 "collection_id": collection.id,
                 "page_size": page_size,
                 "show_missing": True,
+                "mask": {"field_paths": None},
             },
             metadata=client._rpc_metadata,
             **kwargs,