More conditionally adding read_time to kwargs

Author: gkevinzheng

google/cloud/firestore_v1/base_client.py

@@ -448,8 +448,9 @@ def _prep_get_all(
             "documents": document_paths,
             "mask": mask,
             "transaction": _helpers.get_transaction_id(transaction),
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, reference_map, kwargs
@@ -477,8 +478,9 @@ def _prep_collections(
         """Shared setup for async/sync :meth:`collections`."""
         request = {
             "parent": "{}/documents".format(self._database_string),
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, kwargs

google/cloud/firestore_v1/base_collection.py

@@ -217,8 +217,9 @@ def _prep_list_documents(
             # include any fields. To save on data transfer, we can set a field_path mask
             # to include no fields
             "mask": {"field_paths": None},
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, kwargs

google/cloud/firestore_v1/base_document.py

@@ -307,8 +307,9 @@ def _prep_batch_get(
             "documents": [self._document_path],
             "mask": mask,
             "transaction": _helpers.get_transaction_id(transaction),
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, kwargs
@@ -335,8 +336,9 @@ def _prep_collections(
         request = {
             "parent": self._document_path,
             "page_size": page_size,
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, kwargs

google/cloud/firestore_v1/base_query.py

@@ -1448,8 +1448,9 @@ def _prep_get_partitions(
             "parent": parent_path,
             "structured_query": query._to_protobuf(),
             "partition_count": partition_count,
-            "read_time": read_time,
         }
+        if read_time is not None:
+            request["read_time"] = read_time
         kwargs = _helpers.make_retry_timeout_kwargs(retry, timeout)
 
         return request, kwargs

tests/unit/v1/test_client.py

@@ -307,11 +307,13 @@ def __iter__(self):
         assert collection.id == collection_id
 
     base_path = client._database_string + "/documents"
+    expected_request = {
+        "parent": base_path,
+    }
+    if read_time is not None:
+        expected_request["read_time"] = read_time
     firestore_api.list_collection_ids.assert_called_once_with(
-        request={
-            "parent": base_path,
-            "read_time": read_time,
-        },
+        request=expected_request,
         metadata=client._rpc_metadata,
         **kwargs,
     )
@@ -417,15 +419,17 @@ def _get_all_helper(
     mask = common.DocumentMask(field_paths=field_paths)
 
     kwargs.pop("transaction", None)
+    expected_request = {
+        "database": client._database_string,
+        "documents": doc_paths,
+        "mask": mask,
+        "transaction": txn_id,
+    }
+    if read_time is not None:
+        expected_request["read_time"] = read_time
 
     client._firestore_api.batch_get_documents.assert_called_once_with(
-        request={
-            "database": client._database_string,
-            "documents": doc_paths,
-            "mask": mask,
-            "transaction": txn_id,
-            "read_time": read_time,
-        },
+        request=expected_request,
         metadata=client._rpc_metadata,
         **kwargs,
     )
@@ -490,7 +494,6 @@ def test_client_get_all_unknown_result(database):
             "documents": doc_paths,
             "mask": None,
             "transaction": None,
-            "read_time": None,
         },
         metadata=client._rpc_metadata,
     )

tests/unit/v1/test_collection.py

@@ -318,15 +318,18 @@ def _next_page(self):
         assert document.id == document_id
 
     parent, _ = collection._parent_info()
+    expected_request = {
+        "parent": parent,
+        "collection_id": collection.id,
+        "page_size": page_size,
+        "show_missing": True,
+        "mask": {"field_paths": None},
+    }
+    if read_time is not None:
+        expected_request["read_time"] = read_time
+
     api_client.list_documents.assert_called_once_with(
-        request={
-            "parent": parent,
-            "collection_id": collection.id,
-            "page_size": page_size,
-            "show_missing": True,
-            "mask": {"field_paths": None},
-            "read_time": read_time,
-        },
+        request=expected_request,
         metadata=client._rpc_metadata,
         **kwargs,
     )

tests/unit/v1/test_cross_language.py

@@ -154,7 +154,6 @@ def test_get_testprotos(test_proto):
         "documents": [doc._document_path],
         "mask": None,
         "transaction": None,
-        "read_time": None,
     }
 
     firestore_api.batch_get_documents.assert_called_once_with(

tests/unit/v1/test_document.py

@@ -468,14 +468,17 @@ def WhichOneof(val):
     else:
         expected_transaction_id = None
 
+    expected_request = {
+        "database": client._database_string,
+        "documents": [document_reference._document_path],
+        "mask": mask,
+        "transaction": expected_transaction_id,
+    }
+    if read_time is not None:
+        expected_request["read_time"] = read_time
+
     firestore_api.batch_get_documents.assert_called_once_with(
-        request={
-            "database": client._database_string,
-            "documents": [document_reference._document_path],
-            "mask": mask,
-            "transaction": expected_transaction_id,
-            "read_time": read_time,
-        },
+        request=expected_request,
         metadata=client._rpc_metadata,
         **kwargs,
     )
@@ -566,13 +569,16 @@ def __iter__(self):
         assert isinstance(collection, CollectionReference)
         assert collection.parent == document
         assert collection.id == collection_id
+    
+    expected_result = {
+        "parent": document._document_path,
+        "page_size": page_size,
+    }
+    if read_time is not None:
+        expected_result["read_time"] = read_time
 
     api_client.list_collection_ids.assert_called_once_with(
-        request={
-            "parent": document._document_path,
-            "page_size": page_size,
-            "read_time": read_time,
-        },
+        request=expected_result,
         metadata=client._rpc_metadata,
         **kwargs,
     )

tests/unit/v1/test_query.py

@@ -970,13 +970,15 @@ def _collection_group_get_partitions_helper(
         parent,
         orders=(query._make_order("__name__", query.ASCENDING),),
     )
+    expected_request = {
+        "parent": parent_path,
+        "structured_query": partition_query._to_protobuf(),
+        "partition_count": 2,
+    }
+    if read_time is not None:
+        expected_request["read_time"] = read_time
     firestore_api.partition_query.assert_called_once_with(
-        request={
-            "parent": parent_path,
-            "structured_query": partition_query._to_protobuf(),
-            "partition_count": 2,
-            "read_time": read_time,
-        },
+        request=expected_request,
         metadata=client._rpc_metadata,
         **kwargs,
     )