Skip to content

Commit e79dfde

Browse files
authored
Remove bogus error checking of query response stream. (googleapis#7206)
Closes googleapis#6924.
1 parent 5f2999a commit e79dfde

File tree

2 files changed

+18
-57
lines changed

2 files changed

+18
-57
lines changed

firestore/google/cloud/firestore_v1beta1/query.py

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@
6363
"come from fields set in ``order_by()``."
6464
)
6565
_MISMATCH_CURSOR_W_ORDER_BY = "The cursor {!r} does not match the order fields {!r}."
66-
_EMPTY_DOC_TEMPLATE = (
67-
"Unexpected server response. All responses other than the first must "
68-
"contain a document. The response at index {} was\n{}."
69-
)
7066

7167

7268
class Query(object):
@@ -725,12 +721,6 @@ def get(self, transaction=None):
725721
Yields:
726722
~.firestore_v1beta1.document.DocumentSnapshot: The next
727723
document that fulfills the query.
728-
729-
Raises:
730-
ValueError: If the first response in the stream is empty, but
731-
then more responses follow.
732-
ValueError: If a response other than the first does not contain
733-
a document.
734724
"""
735725
parent_path, expected_prefix = self._parent._parent_info()
736726
response_iterator = self._client._firestore_api.run_query(
@@ -740,24 +730,11 @@ def get(self, transaction=None):
740730
metadata=self._client._rpc_metadata,
741731
)
742732

743-
empty_stream = False
744-
for index, response_pb in enumerate(response_iterator):
745-
if empty_stream:
746-
raise ValueError(
747-
"First response in stream was empty",
748-
"Received second response",
749-
response_pb,
750-
)
751-
752-
snapshot, skipped_results = _query_response_to_snapshot(
753-
response_pb, self._parent, expected_prefix
733+
for response in response_iterator:
734+
snapshot = _query_response_to_snapshot(
735+
response, self._parent, expected_prefix
754736
)
755-
if snapshot is None:
756-
if index != 0:
757-
msg = _EMPTY_DOC_TEMPLATE.format(index, response_pb)
758-
raise ValueError(msg)
759-
empty_stream = skipped_results == 0
760-
else:
737+
if snapshot is not None:
761738
yield snapshot
762739

763740
def on_snapshot(self, callback):
@@ -964,13 +941,12 @@ def _query_response_to_snapshot(response_pb, collection, expected_prefix):
964941
directly from ``collection`` via :meth:`_parent_info`.
965942
966943
Returns:
967-
Tuple[Optional[~.firestore.document.DocumentSnapshot], int]: A
968-
snapshot of the data returned in the query and the number of skipped
969-
results. If ``response_pb.document`` is not set, the snapshot will be
970-
:data:`None`.
944+
Optional[~.firestore.document.DocumentSnapshot]: A
945+
snapshot of the data returned in the query. If ``response_pb.document``
946+
is not set, the snapshot will be :data:`None`.
971947
"""
972948
if not response_pb.HasField("document"):
973-
return None, response_pb.skipped_results
949+
return None
974950

975951
document_id = _helpers.get_doc_id(response_pb.document, expected_prefix)
976952
reference = collection.document(document_id)
@@ -983,4 +959,4 @@ def _query_response_to_snapshot(response_pb, collection, expected_prefix):
983959
create_time=response_pb.document.create_time,
984960
update_time=response_pb.document.update_time,
985961
)
986-
return snapshot, response_pb.skipped_results
962+
return snapshot

firestore/tests/unit/test_query.py

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,13 +1137,7 @@ def test_get_second_response_in_empty_stream(self):
11371137

11381138
get_response = query.get()
11391139
self.assertIsInstance(get_response, types.GeneratorType)
1140-
with self.assertRaises(ValueError) as exc_info:
1141-
list(get_response)
1142-
1143-
exc_args = exc_info.exception.args
1144-
self.assertEqual(len(exc_args), 3)
1145-
self.assertIs(exc_args[2], empty_response2)
1146-
self.assertIsNot(empty_response1, empty_response2)
1140+
self.assertEqual(list(get_response), [])
11471141

11481142
# Verify the mock call.
11491143
parent_path, _ = parent._parent_info()
@@ -1193,8 +1187,6 @@ def test_get_with_skipped_results(self):
11931187
)
11941188

11951189
def test_get_empty_after_first_response(self):
1196-
from google.cloud.firestore_v1beta1.query import _EMPTY_DOC_TEMPLATE
1197-
11981190
# Create a minimal fake GAPIC.
11991191
firestore_api = mock.Mock(spec=["run_query"])
12001192

@@ -1217,13 +1209,11 @@ def test_get_empty_after_first_response(self):
12171209
query = self._make_one(parent)
12181210
get_response = query.get()
12191211
self.assertIsInstance(get_response, types.GeneratorType)
1220-
with self.assertRaises(ValueError) as exc_info:
1221-
list(get_response)
1222-
1223-
exc_args = exc_info.exception.args
1224-
self.assertEqual(len(exc_args), 1)
1225-
msg = _EMPTY_DOC_TEMPLATE.format(1, response_pb2)
1226-
self.assertEqual(exc_args[0], msg)
1212+
returned = list(get_response)
1213+
self.assertEqual(len(returned), 1)
1214+
snapshot = returned[0]
1215+
self.assertEqual(snapshot.reference._path, ("charles", "bark"))
1216+
self.assertEqual(snapshot.to_dict(), data)
12271217

12281218
# Verify the mock call.
12291219
parent_path, _ = parent._parent_info()
@@ -1468,16 +1458,14 @@ def _call_fut(response_pb, collection, expected_prefix):
14681458

14691459
def test_empty(self):
14701460
response_pb = _make_query_response()
1471-
snapshot, skipped_results = self._call_fut(response_pb, None, None)
1461+
snapshot = self._call_fut(response_pb, None, None)
14721462
self.assertIsNone(snapshot)
1473-
self.assertEqual(skipped_results, 0)
14741463

14751464
def test_after_offset(self):
14761465
skipped_results = 410
14771466
response_pb = _make_query_response(skipped_results=skipped_results)
1478-
snapshot, skipped_results = self._call_fut(response_pb, None, None)
1467+
snapshot = self._call_fut(response_pb, None, None)
14791468
self.assertIsNone(snapshot)
1480-
self.assertEqual(skipped_results, skipped_results)
14811469

14821470
def test_response(self):
14831471
from google.cloud.firestore_v1beta1.document import DocumentSnapshot
@@ -1492,10 +1480,7 @@ def test_response(self):
14921480
data = {"a": 901, "b": True}
14931481
response_pb = _make_query_response(name=name, data=data)
14941482

1495-
snapshot, skipped_results = self._call_fut(
1496-
response_pb, collection, expected_prefix
1497-
)
1498-
self.assertEqual(skipped_results, 0)
1483+
snapshot = self._call_fut(response_pb, collection, expected_prefix)
14991484
self.assertIsInstance(snapshot, DocumentSnapshot)
15001485
expected_path = collection._path + (doc_id,)
15011486
self.assertEqual(snapshot.reference._path, expected_path)

0 commit comments

Comments
 (0)