From 55da695710d0408fc314ffe5cc6d7a48cb71bc3b Mon Sep 17 00:00:00 2001 From: Raphael Long Date: Fri, 7 Aug 2020 12:34:33 -0500 Subject: [PATCH] feat: integrate limit to last (#145) * feat: integrate limit_to_last changes from #57 to async * fix: whitespace in docs * fix: whitespace in docs --- google/cloud/firestore_v1/async_collection.py | 34 ++--- google/cloud/firestore_v1/async_query.py | 60 +++++++-- google/cloud/firestore_v1/base_collection.py | 22 ++++ google/cloud/firestore_v1/base_query.py | 45 ++++++- google/cloud/firestore_v1/collection.py | 30 +++-- google/cloud/firestore_v1/query.py | 55 ++++++-- tests/unit/v1/test_async_collection.py | 32 +---- tests/unit/v1/test_async_query.py | 117 ++++++++++++++---- tests/unit/v1/test_base_collection.py | 14 +++ tests/unit/v1/test_collection.py | 23 +--- tests/unit/v1/test_query.py | 112 ++++++++++++++--- 11 files changed, 410 insertions(+), 134 deletions(-) diff --git a/google/cloud/firestore_v1/async_collection.py b/google/cloud/firestore_v1/async_collection.py index bd9aef5e5..2a37353fd 100644 --- a/google/cloud/firestore_v1/async_collection.py +++ b/google/cloud/firestore_v1/async_collection.py @@ -13,9 +13,6 @@ # limitations under the License. """Classes for representing collections for the Google Cloud Firestore API.""" -import warnings - - from google.cloud.firestore_v1.base_collection import ( BaseCollectionReference, _auto_id, @@ -130,17 +127,26 @@ async def list_documents( async for i in iterator: yield _item_to_document_ref(self, i) - async def get( - self, transaction=None - ) -> AsyncGenerator[async_document.DocumentSnapshot, Any]: - """Deprecated alias for :meth:`stream`.""" - warnings.warn( - "'Collection.get' is deprecated: please use 'Collection.stream' instead.", - DeprecationWarning, - stacklevel=2, - ) - async for d in self.stream(transaction=transaction): - yield d # pytype: disable=name-error + async def get(self, transaction=None) -> list: + """Read the documents in this collection. + + This sends a ``RunQuery`` RPC and returns a list of documents + returned in the stream of ``RunQueryResponse`` messages. + + Args: + transaction + (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]): + An existing transaction that this query will run in. + + If a ``transaction`` is used and it already has write operations + added, this method cannot be used (i.e. read-after-write is not + allowed). + + Returns: + list: The documents in this collection that match the query. + """ + query = self._query() + return await query.get(transaction=transaction) async def stream( self, transaction=None diff --git a/google/cloud/firestore_v1/async_query.py b/google/cloud/firestore_v1/async_query.py index f556c1206..3f89b04a8 100644 --- a/google/cloud/firestore_v1/async_query.py +++ b/google/cloud/firestore_v1/async_query.py @@ -18,12 +18,11 @@ a :class:`~google.cloud.firestore_v1.collection.Collection` and that can be a more common way to create a query than direct usage of the constructor. """ -import warnings - from google.cloud.firestore_v1.base_query import ( BaseQuery, _query_response_to_snapshot, _collection_group_query_response_to_snapshot, + _enum_from_direction, ) from google.cloud.firestore_v1 import _helpers @@ -94,6 +93,7 @@ def __init__( field_filters=(), orders=(), limit=None, + limit_to_last=False, offset=None, start_at=None, end_at=None, @@ -105,23 +105,51 @@ def __init__( field_filters=field_filters, orders=orders, limit=limit, + limit_to_last=limit_to_last, offset=offset, start_at=start_at, end_at=end_at, all_descendants=all_descendants, ) - async def get( - self, transaction=None - ) -> AsyncGenerator[async_document.DocumentSnapshot, None]: - """Deprecated alias for :meth:`stream`.""" - warnings.warn( - "'AsyncQuery.get' is deprecated: please use 'AsyncQuery.stream' instead.", - DeprecationWarning, - stacklevel=2, - ) - async for d in self.stream(transaction=transaction): - yield d + async def get(self, transaction=None) -> list: + """Read the documents in the collection that match this query. + + This sends a ``RunQuery`` RPC and returns a list of documents + returned in the stream of ``RunQueryResponse`` messages. + + Args: + transaction + (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]): + An existing transaction that this query will run in. + + If a ``transaction`` is used and it already has write operations + added, this method cannot be used (i.e. read-after-write is not + allowed). + + Returns: + list: The documents in the collection that match this query. + """ + is_limited_to_last = self._limit_to_last + + if self._limit_to_last: + # In order to fetch up to `self._limit` results from the end of the + # query flip the defined ordering on the query to start from the + # end, retrieving up to `self._limit` results from the backend. + for order in self._orders: + order.direction = _enum_from_direction( + self.DESCENDING + if order.direction == self.ASCENDING + else self.ASCENDING + ) + self._limit_to_last = False + + result = self.stream(transaction=transaction) + result = [d async for d in result] + if is_limited_to_last: + result = list(reversed(result)) + + return result async def stream( self, transaction=None @@ -152,6 +180,12 @@ async def stream( :class:`~google.cloud.firestore_v1.async_document.DocumentSnapshot`: The next document that fulfills the query. """ + if self._limit_to_last: + raise ValueError( + "Query results for queries that include limit_to_last() " + "constraints cannot be streamed. Use Query.get() instead." + ) + parent_path, expected_prefix = self._parent._parent_info() response_iterator = await self._client._firestore_api.run_query( request={ diff --git a/google/cloud/firestore_v1/base_collection.py b/google/cloud/firestore_v1/base_collection.py index 8ce40bd1b..0c2fe0e94 100644 --- a/google/cloud/firestore_v1/base_collection.py +++ b/google/cloud/firestore_v1/base_collection.py @@ -205,6 +205,10 @@ def order_by(self, field_path, **kwargs) -> NoReturn: def limit(self, count) -> NoReturn: """Create a limited query with this collection as parent. + .. note:: + `limit` and `limit_to_last` are mutually exclusive. + Setting `limit` will drop previously set `limit_to_last`. + See :meth:`~google.cloud.firestore_v1.query.Query.limit` for more information on this method. @@ -220,6 +224,24 @@ def limit(self, count) -> NoReturn: query = self._query() return query.limit(count) + def limit_to_last(self, count): + """Create a limited to last query with this collection as parent. + .. note:: + `limit` and `limit_to_last` are mutually exclusive. + Setting `limit_to_last` will drop previously set `limit`. + See + :meth:`~google.cloud.firestore_v1.query.Query.limit_to_last` + for more information on this method. + Args: + count (int): Maximum number of documents to return that + match the query. + Returns: + :class:`~google.cloud.firestore_v1.query.Query`: + A limited to last query. + """ + query = self._query() + return query.limit_to_last(count) + def offset(self, num_to_skip) -> NoReturn: """Skip to an offset in a query with this collection as parent. diff --git a/google/cloud/firestore_v1/base_query.py b/google/cloud/firestore_v1/base_query.py index 0522ac89a..7bc7d28cb 100644 --- a/google/cloud/firestore_v1/base_query.py +++ b/google/cloud/firestore_v1/base_query.py @@ -98,6 +98,8 @@ class BaseQuery(object): The "order by" entries to use in the query. limit (Optional[int]): The maximum number of documents the query is allowed to return. + limit_to_last (Optional[bool]): + Denotes whether a provided limit is applied to the end of the result set. offset (Optional[int]): The number of results to skip. start_at (Optional[Tuple[dict, bool]]): @@ -146,6 +148,7 @@ def __init__( field_filters=(), orders=(), limit=None, + limit_to_last=False, offset=None, start_at=None, end_at=None, @@ -156,6 +159,7 @@ def __init__( self._field_filters = field_filters self._orders = orders self._limit = limit + self._limit_to_last = limit_to_last self._offset = offset self._start_at = start_at self._end_at = end_at @@ -170,6 +174,7 @@ def __eq__(self, other): and self._field_filters == other._field_filters and self._orders == other._orders and self._limit == other._limit + and self._limit_to_last == other._limit_to_last and self._offset == other._offset and self._start_at == other._start_at and self._end_at == other._end_at @@ -224,6 +229,7 @@ def select(self, field_paths) -> "BaseQuery": field_filters=self._field_filters, orders=self._orders, limit=self._limit, + limit_to_last=self._limit_to_last, offset=self._offset, start_at=self._start_at, end_at=self._end_at, @@ -294,6 +300,7 @@ def where(self, field_path, op_string, value) -> "BaseQuery": orders=self._orders, limit=self._limit, offset=self._offset, + limit_to_last=self._limit_to_last, start_at=self._start_at, end_at=self._end_at, all_descendants=self._all_descendants, @@ -345,6 +352,7 @@ def order_by(self, field_path, direction=ASCENDING) -> "BaseQuery": field_filters=self._field_filters, orders=new_orders, limit=self._limit, + limit_to_last=self._limit_to_last, offset=self._offset, start_at=self._start_at, end_at=self._end_at, @@ -352,14 +360,43 @@ def order_by(self, field_path, direction=ASCENDING) -> "BaseQuery": ) def limit(self, count) -> "BaseQuery": - """Limit a query to return a fixed number of results. - - If the current query already has a limit set, this will overwrite it. + """Limit a query to return at most `count` matching results. + If the current query already has a `limit` set, this will override it. + .. note:: + `limit` and `limit_to_last` are mutually exclusive. + Setting `limit` will drop previously set `limit_to_last`. Args: count (int): Maximum number of documents to return that match the query. + Returns: + :class:`~google.cloud.firestore_v1.query.Query`: + A limited query. Acts as a copy of the current query, modified + with the newly added "limit" filter. + """ + return self.__class__( + self._parent, + projection=self._projection, + field_filters=self._field_filters, + orders=self._orders, + limit=count, + limit_to_last=False, + offset=self._offset, + start_at=self._start_at, + end_at=self._end_at, + all_descendants=self._all_descendants, + ) + def limit_to_last(self, count): + """Limit a query to return the last `count` matching results. + If the current query already has a `limit_to_last` + set, this will override it. + .. note:: + `limit` and `limit_to_last` are mutually exclusive. + Setting `limit_to_last` will drop previously set `limit`. + Args: + count (int): Maximum number of documents to return that match + the query. Returns: :class:`~google.cloud.firestore_v1.query.Query`: A limited query. Acts as a copy of the current query, modified @@ -371,6 +408,7 @@ def limit(self, count) -> "BaseQuery": field_filters=self._field_filters, orders=self._orders, limit=count, + limit_to_last=True, offset=self._offset, start_at=self._start_at, end_at=self._end_at, @@ -398,6 +436,7 @@ def offset(self, num_to_skip) -> "BaseQuery": field_filters=self._field_filters, orders=self._orders, limit=self._limit, + limit_to_last=self._limit_to_last, offset=num_to_skip, start_at=self._start_at, end_at=self._end_at, diff --git a/google/cloud/firestore_v1/collection.py b/google/cloud/firestore_v1/collection.py index 67144b0f7..43f2d8fc8 100644 --- a/google/cloud/firestore_v1/collection.py +++ b/google/cloud/firestore_v1/collection.py @@ -13,8 +13,6 @@ # limitations under the License. """Classes for representing collections for the Google Cloud Firestore API.""" -import warnings - from google.cloud.firestore_v1.base_collection import ( BaseCollectionReference, _auto_id, @@ -121,14 +119,26 @@ def list_documents(self, page_size=None) -> Generator[Any, Any, None]: ) return (_item_to_document_ref(self, i) for i in iterator) - def get(self, transaction=None) -> Generator[document.DocumentSnapshot, Any, None]: - """Deprecated alias for :meth:`stream`.""" - warnings.warn( - "'Collection.get' is deprecated: please use 'Collection.stream' instead.", - DeprecationWarning, - stacklevel=2, - ) - return self.stream(transaction=transaction) + def get(self, transaction=None) -> list: + """Read the documents in this collection. + + This sends a ``RunQuery`` RPC and returns a list of documents + returned in the stream of ``RunQueryResponse`` messages. + + Args: + transaction + (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]): + An existing transaction that this query will run in. + + If a ``transaction`` is used and it already has write operations + added, this method cannot be used (i.e. read-after-write is not + allowed). + + Returns: + list: The documents in this collection that match the query. + """ + query = query_mod.Query(self) + return query.get(transaction=transaction) def stream( self, transaction=None diff --git a/google/cloud/firestore_v1/query.py b/google/cloud/firestore_v1/query.py index 4523cc71b..9b0dc4462 100644 --- a/google/cloud/firestore_v1/query.py +++ b/google/cloud/firestore_v1/query.py @@ -18,12 +18,11 @@ a :class:`~google.cloud.firestore_v1.collection.Collection` and that can be a more common way to create a query than direct usage of the constructor. """ -import warnings - from google.cloud.firestore_v1.base_query import ( BaseQuery, _query_response_to_snapshot, _collection_group_query_response_to_snapshot, + _enum_from_direction, ) from google.cloud.firestore_v1 import _helpers @@ -95,6 +94,7 @@ def __init__( field_filters=(), orders=(), limit=None, + limit_to_last=False, offset=None, start_at=None, end_at=None, @@ -106,20 +106,49 @@ def __init__( field_filters=field_filters, orders=orders, limit=limit, + limit_to_last=limit_to_last, offset=offset, start_at=start_at, end_at=end_at, all_descendants=all_descendants, ) - def get(self, transaction=None) -> Generator[document.DocumentSnapshot, Any, None]: - """Deprecated alias for :meth:`stream`.""" - warnings.warn( - "'Query.get' is deprecated: please use 'Query.stream' instead.", - DeprecationWarning, - stacklevel=2, - ) - return self.stream(transaction=transaction) + def get(self, transaction=None) -> list: + """Read the documents in the collection that match this query. + + This sends a ``RunQuery`` RPC and returns a list of documents + returned in the stream of ``RunQueryResponse`` messages. + + Args: + transaction + (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]): + An existing transaction that this query will run in. + If a ``transaction`` is used and it already has write operations + added, this method cannot be used (i.e. read-after-write is not + allowed). + + Returns: + list: The documents in the collection that match this query. + """ + is_limited_to_last = self._limit_to_last + + if self._limit_to_last: + # In order to fetch up to `self._limit` results from the end of the + # query flip the defined ordering on the query to start from the + # end, retrieving up to `self._limit` results from the backend. + for order in self._orders: + order.direction = _enum_from_direction( + self.DESCENDING + if order.direction == self.ASCENDING + else self.ASCENDING + ) + self._limit_to_last = False + + result = self.stream(transaction=transaction) + if is_limited_to_last: + result = reversed(list(result)) + + return list(result) def stream( self, transaction=None @@ -150,6 +179,12 @@ def stream( :class:`~google.cloud.firestore_v1.document.DocumentSnapshot`: The next document that fulfills the query. """ + if self._limit_to_last: + raise ValueError( + "Query results for queries that include limit_to_last() " + "constraints cannot be streamed. Use Query.get() instead." + ) + parent_path, expected_prefix = self._parent._parent_info() response_iterator = self._client._firestore_api.run_query( request={ diff --git a/tests/unit/v1/test_async_collection.py b/tests/unit/v1/test_async_collection.py index 5649561e0..1b7587c73 100644 --- a/tests/unit/v1/test_async_collection.py +++ b/tests/unit/v1/test_async_collection.py @@ -249,47 +249,27 @@ async def test_list_documents_w_page_size(self): @mock.patch("google.cloud.firestore_v1.async_query.AsyncQuery", autospec=True) @pytest.mark.asyncio async def test_get(self, query_class): - import warnings - - query_class.return_value.stream.return_value = AsyncIter(range(3)) - collection = self._make_one("collection") - with warnings.catch_warnings(record=True) as warned: - get_response = collection.get() - - async for _ in get_response: - pass + get_response = await collection.get() query_class.assert_called_once_with(collection) query_instance = query_class.return_value - query_instance.stream.assert_called_once_with(transaction=None) - # Verify the deprecation - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + self.assertIs(get_response, query_instance.get.return_value) + query_instance.get.assert_called_once_with(transaction=None) @mock.patch("google.cloud.firestore_v1.async_query.AsyncQuery", autospec=True) @pytest.mark.asyncio async def test_get_with_transaction(self, query_class): - import warnings - - query_class.return_value.stream.return_value = AsyncIter(range(3)) - collection = self._make_one("collection") transaction = mock.sentinel.txn - with warnings.catch_warnings(record=True) as warned: - get_response = collection.get(transaction=transaction) - - async for _ in get_response: - pass + get_response = await collection.get(transaction=transaction) query_class.assert_called_once_with(collection) query_instance = query_class.return_value - query_instance.stream.assert_called_once_with(transaction=transaction) - # Verify the deprecation - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + self.assertIs(get_response, query_instance.get.return_value) + query_instance.get.assert_called_once_with(transaction=transaction) @mock.patch("google.cloud.firestore_v1.async_query.AsyncQuery", autospec=True) @pytest.mark.asyncio diff --git a/tests/unit/v1/test_async_query.py b/tests/unit/v1/test_async_query.py index be9c34358..14e41c278 100644 --- a/tests/unit/v1/test_async_query.py +++ b/tests/unit/v1/test_async_query.py @@ -56,36 +56,94 @@ def test_constructor(self): @pytest.mark.asyncio async def test_get(self): - import warnings + # Create a minimal fake GAPIC. + firestore_api = AsyncMock(spec=["run_query"]) - with mock.patch.object(self._get_target_class(), "stream") as stream_mock: - stream_mock.return_value = AsyncIter(range(3)) + # Attach the fake GAPIC to a real client. + client = _make_client() + client._firestore_api_internal = firestore_api - # Create a minimal fake GAPIC. - firestore_api = AsyncMock(spec=["run_query"]) + # Make a **real** collection reference as parent. + parent = client.collection("dee") - # Attach the fake GAPIC to a real client. - client = _make_client() - client._firestore_api_internal = firestore_api + # Add a dummy response to the minimal fake GAPIC. + _, expected_prefix = parent._parent_info() + name = "{}/sleep".format(expected_prefix) + data = {"snooze": 10} - # Make a **real** collection reference as parent. - parent = client.collection("dee") + response_pb = _make_query_response(name=name, data=data) - # Execute the query and check the response. - query = self._make_one(parent) + firestore_api.run_query.return_value = AsyncIter([response_pb]) - with warnings.catch_warnings(record=True) as warned: - get_response = query.get() - returned = [x async for x in get_response] + # Execute the query and check the response. + query = self._make_one(parent) + returned = await query.get() - # Verify that `get` merely wraps `stream`. - stream_mock.assert_called_once() - self.assertIsInstance(get_response, types.AsyncGeneratorType) - self.assertEqual(returned, list(stream_mock.return_value.items)) + self.assertIsInstance(returned, list) + self.assertEqual(len(returned), 1) - # Verify the deprecation. - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + snapshot = returned[0] + self.assertEqual(snapshot.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot.to_dict(), data) + + @pytest.mark.asyncio + async def test_get_limit_to_last(self): + from google.cloud import firestore + from google.cloud.firestore_v1.base_query import _enum_from_direction + + # Create a minimal fake GAPIC. + firestore_api = AsyncMock(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("dee") + + # Add a dummy response to the minimal fake GAPIC. + _, expected_prefix = parent._parent_info() + name = "{}/sleep".format(expected_prefix) + data = {"snooze": 10} + data2 = {"snooze": 20} + + response_pb = _make_query_response(name=name, data=data) + response_pb2 = _make_query_response(name=name, data=data2) + + firestore_api.run_query.return_value = AsyncIter([response_pb2, response_pb]) + + # Execute the query and check the response. + query = self._make_one(parent) + query = query.order_by( + u"snooze", direction=firestore.AsyncQuery.DESCENDING + ).limit_to_last(2) + returned = await query.get() + + self.assertIsInstance(returned, list) + self.assertEqual( + query._orders[0].direction, + _enum_from_direction(firestore.AsyncQuery.ASCENDING), + ) + self.assertEqual(len(returned), 2) + + snapshot = returned[0] + self.assertEqual(snapshot.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot.to_dict(), data) + + snapshot2 = returned[1] + self.assertEqual(snapshot2.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot2.to_dict(), data2) + + # Verify the mock call. + parent_path, _ = parent._parent_info() + firestore_api.run_query.assert_called_once_with( + request={ + "parent": parent_path, + "structured_query": query._to_protobuf(), + "transaction": None, + }, + metadata=client._rpc_metadata, + ) @pytest.mark.asyncio async def test_stream_simple(self): @@ -127,6 +185,21 @@ async def test_stream_simple(self): metadata=client._rpc_metadata, ) + @pytest.mark.asyncio + async def test_stream_with_limit_to_last(self): + # Attach the fake GAPIC to a real client. + client = _make_client() + # Make a **real** collection reference as parent. + parent = client.collection("dee") + # Execute the query and check the response. + query = self._make_one(parent) + query = query.limit_to_last(2) + + stream_response = query.stream() + + with self.assertRaises(ValueError): + [d async for d in stream_response] + @pytest.mark.asyncio async def test_stream_with_transaction(self): # Create a minimal fake GAPIC. diff --git a/tests/unit/v1/test_base_collection.py b/tests/unit/v1/test_base_collection.py index 870f95019..01c68483a 100644 --- a/tests/unit/v1/test_base_collection.py +++ b/tests/unit/v1/test_base_collection.py @@ -234,6 +234,20 @@ def test_limit(self, mock_query): mock_query.limit.assert_called_once_with(limit) self.assertEqual(query, mock_query.limit.return_value) + @mock.patch("google.cloud.firestore_v1.base_query.BaseQuery", autospec=True) + def test_limit_to_last(self, mock_query): + from google.cloud.firestore_v1.base_collection import BaseCollectionReference + + with mock.patch.object(BaseCollectionReference, "_query") as _query: + _query.return_value = mock_query + + collection = self._make_one("collection") + limit = 15 + query = collection.limit_to_last(limit) + + mock_query.limit_to_last.assert_called_once_with(limit) + self.assertEqual(query, mock_query.limit_to_last.return_value) + @mock.patch("google.cloud.firestore_v1.base_query.BaseQuery", autospec=True) def test_offset(self, mock_query): from google.cloud.firestore_v1.base_collection import BaseCollectionReference diff --git a/tests/unit/v1/test_collection.py b/tests/unit/v1/test_collection.py index 3833033f4..982cacdbc 100644 --- a/tests/unit/v1/test_collection.py +++ b/tests/unit/v1/test_collection.py @@ -239,38 +239,27 @@ def test_list_documents_w_page_size(self): @mock.patch("google.cloud.firestore_v1.query.Query", autospec=True) def test_get(self, query_class): - import warnings - collection = self._make_one("collection") - with warnings.catch_warnings(record=True) as warned: - get_response = collection.get() + get_response = collection.get() query_class.assert_called_once_with(collection) query_instance = query_class.return_value - self.assertIs(get_response, query_instance.stream.return_value) - query_instance.stream.assert_called_once_with(transaction=None) - # Verify the deprecation - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + self.assertIs(get_response, query_instance.get.return_value) + query_instance.get.assert_called_once_with(transaction=None) @mock.patch("google.cloud.firestore_v1.query.Query", autospec=True) def test_get_with_transaction(self, query_class): - import warnings collection = self._make_one("collection") transaction = mock.sentinel.txn - with warnings.catch_warnings(record=True) as warned: - get_response = collection.get(transaction=transaction) + get_response = collection.get(transaction=transaction) query_class.assert_called_once_with(collection) query_instance = query_class.return_value - self.assertIs(get_response, query_instance.stream.return_value) - query_instance.stream.assert_called_once_with(transaction=transaction) - # Verify the deprecation - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + self.assertIs(get_response, query_instance.get.return_value) + query_instance.get.assert_called_once_with(transaction=transaction) @mock.patch("google.cloud.firestore_v1.query.Query", autospec=True) def test_stream(self, query_class): diff --git a/tests/unit/v1/test_query.py b/tests/unit/v1/test_query.py index 40ea2bb16..3ad01d02c 100644 --- a/tests/unit/v1/test_query.py +++ b/tests/unit/v1/test_query.py @@ -44,32 +44,92 @@ def test_constructor(self): self.assertFalse(query._all_descendants) def test_get(self): - import warnings + # 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("dee") + + # Add a dummy response to the minimal fake GAPIC. + _, expected_prefix = parent._parent_info() + name = "{}/sleep".format(expected_prefix) + data = {"snooze": 10} + + response_pb = _make_query_response(name=name, data=data) + + firestore_api.run_query.return_value = iter([response_pb]) + + # Execute the query and check the response. + query = self._make_one(parent) + returned = query.get() + + self.assertIsInstance(returned, list) + self.assertEqual(len(returned), 1) + + snapshot = returned[0] + self.assertEqual(snapshot.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot.to_dict(), data) + + def test_get_limit_to_last(self): + from google.cloud import firestore + from google.cloud.firestore_v1.base_query import _enum_from_direction + + # 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 - with mock.patch.object(self._get_target_class(), "stream") as stream_mock: - # Create a minimal fake GAPIC. - firestore_api = mock.Mock(spec=["run_query"]) + # Make a **real** collection reference as parent. + parent = client.collection("dee") - # Attach the fake GAPIC to a real client. - client = _make_client() - client._firestore_api_internal = firestore_api + # Add a dummy response to the minimal fake GAPIC. + _, expected_prefix = parent._parent_info() + name = "{}/sleep".format(expected_prefix) + data = {"snooze": 10} + data2 = {"snooze": 20} - # Make a **real** collection reference as parent. - parent = client.collection("dee") + response_pb = _make_query_response(name=name, data=data) + response_pb2 = _make_query_response(name=name, data=data2) - # Execute the query and check the response. - query = self._make_one(parent) + firestore_api.run_query.return_value = iter([response_pb2, response_pb]) - with warnings.catch_warnings(record=True) as warned: - get_response = query.get() + # Execute the query and check the response. + query = self._make_one(parent) + query = query.order_by( + u"snooze", direction=firestore.Query.DESCENDING + ).limit_to_last(2) + returned = query.get() + + self.assertIsInstance(returned, list) + self.assertEqual( + query._orders[0].direction, _enum_from_direction(firestore.Query.ASCENDING) + ) + self.assertEqual(len(returned), 2) + + snapshot = returned[0] + self.assertEqual(snapshot.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot.to_dict(), data) - # Verify that `get` merely wraps `stream`. - stream_mock.assert_called_once() - self.assertEqual(get_response, stream_mock.return_value) + snapshot2 = returned[1] + self.assertEqual(snapshot2.reference._path, ("dee", "sleep")) + self.assertEqual(snapshot2.to_dict(), data2) - # Verify the deprecation. - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) + # Verify the mock call. + parent_path, _ = parent._parent_info() + firestore_api.run_query.assert_called_once_with( + request={ + "parent": parent_path, + "structured_query": query._to_protobuf(), + "transaction": None, + }, + metadata=client._rpc_metadata, + ) def test_stream_simple(self): # Create a minimal fake GAPIC. @@ -110,6 +170,20 @@ def test_stream_simple(self): metadata=client._rpc_metadata, ) + def test_stream_with_limit_to_last(self): + # Attach the fake GAPIC to a real client. + client = _make_client() + # Make a **real** collection reference as parent. + parent = client.collection("dee") + # Execute the query and check the response. + query = self._make_one(parent) + query = query.limit_to_last(2) + + stream_response = query.stream() + + with self.assertRaises(ValueError): + list(stream_response) + def test_stream_with_transaction(self): # Create a minimal fake GAPIC. firestore_api = mock.Mock(spec=["run_query"])