Merge branch 'v2-staging' into asyncio-microgen-collection

google/cloud/firestore_v1/async_document.py

@@ -14,8 +14,6 @@
 
 """Classes for representing documents for the Google Cloud Firestore API."""
 
-import six
-
 from google.cloud.firestore_v1.base_document import (
     BaseDocumentReference,
     DocumentSnapshot,
@@ -310,7 +308,7 @@ async def get(self, field_paths=None, transaction=None):
                 :attr:`create_time` attributes will all be ``None`` and
                 its :attr:`exists` attribute will be ``False``.
         """
-        if isinstance(field_paths, six.string_types):
+        if isinstance(field_paths, str):
             raise ValueError("'field_paths' must be a sequence of paths, not a string.")
 
         if field_paths is not None:

google/cloud/firestore_v1/async_transaction.py

@@ -18,8 +18,6 @@
 import asyncio
 import random
 
-import six
-
 from google.cloud.firestore_v1.base_transaction import (
     _BaseTransactional,
     BaseTransaction,
@@ -272,7 +270,7 @@ async def __call__(self, transaction, *args, **kwargs):
         """
         self._reset()
 
-        for attempt in six.moves.xrange(transaction._max_attempts):
+        for attempt in range(transaction._max_attempts):
             result = await self._pre_commit(transaction, *args, **kwargs)
             succeeded = await self._maybe_commit(transaction)
             if succeeded:

google/cloud/firestore_v1/watch.py

@@ -70,7 +70,7 @@
 
 
 class WatchDocTree(object):
-    # TODO: Currently this uses a dict. Other implementations us an rbtree.
+    # TODO: Currently this uses a dict. Other implementations use a rbtree.
     # The performance of this implementation should be investigated and may
     # require modifying the underlying datastructure to a rbtree.
     def __init__(self):

tests/unit/v1/test_async_collection.py

@@ -18,7 +18,6 @@
 
 import mock
 from tests.unit.v1.test__helpers import AsyncMock
-import six
 
 
 class MockAsyncIter:
@@ -47,7 +46,7 @@ def _get_public_methods(klass):
             *(
                 (
                     name
-                    for name, value in six.iteritems(class_.__dict__)
+                    for name, value in class_.__dict__.items()
                     if (
                         not name.startswith("_")
                         and isinstance(value, types.FunctionType)
@@ -80,20 +79,6 @@ def test_constructor(self):
         expected_path = (collection_id1, document_id, collection_id2)
         self.assertEqual(collection._path, expected_path)
 
-    def test_constructor_invalid_path(self):
-        with self.assertRaises(ValueError):
-            self._make_one()
-        with self.assertRaises(ValueError):
-            self._make_one(99, "doc", "bad-collection-id")
-        with self.assertRaises(ValueError):
-            self._make_one("bad-document-ID", None, "sub-collection")
-        with self.assertRaises(ValueError):
-            self._make_one("Just", "A-Document")
-
-    def test_constructor_invalid_kwarg(self):
-        with self.assertRaises(TypeError):
-            self._make_one("Coh-lek-shun", donut=True)
-
     @pytest.mark.asyncio
     async def test_add_auto_assigned(self):
         from google.cloud.firestore_v1.types import document

tests/unit/v1/test_async_document.py

@@ -47,20 +47,6 @@ def test_constructor(self):
         )
         self.assertEqual(document.path, expected_path)
 
-    def test_constructor_invalid_path(self):
-        with self.assertRaises(ValueError):
-            self._make_one()
-        with self.assertRaises(ValueError):
-            self._make_one(None, "before", "bad-collection-id", "fifteen")
-        with self.assertRaises(ValueError):
-            self._make_one("bad-document-ID", None)
-        with self.assertRaises(ValueError):
-            self._make_one("Just", "A-Collection", "Sub")
-
-    def test_constructor_invalid_kwarg(self):
-        with self.assertRaises(TypeError):
-            self._make_one("Coh-lek-shun", "Dahk-yu-mehnt", burger=18.75)
-
     @staticmethod
     def _make_commit_repsonse(write_results=None):
         from google.cloud.firestore_v1.types import firestore

tests/unit/v1/test_async_query.py

@@ -21,6 +21,16 @@
 from tests.unit.v1.test_base_query import _make_credentials, _make_query_response
 
 
+class MockAsyncIter:
+    def __init__(self, count=3):
+        # count is arbitrary value
+        self.count = count
+
+    async def __aiter__(self, **_):
+        for i in range(self.count):
+            yield i
+
+
 class TestAsyncQuery(aiounittest.AsyncTestCase):
     @staticmethod
     def _get_target_class():
@@ -32,7 +42,7 @@ def _make_one(self, *args, **kwargs):
         klass = self._get_target_class()
         return klass(*args, **kwargs)
 
-    def test_constructor_defaults(self):
+    def test_constructor(self):
         query = self._make_one(mock.sentinel.parent)
         self.assertIs(query._parent, mock.sentinel.parent)
         self.assertIsNone(query._projection)
@@ -45,53 +55,37 @@ def test_constructor_defaults(self):
         self.assertFalse(query._all_descendants)
 
     @pytest.mark.asyncio
-    async def test_get_simple(self):
+    async def test_get(self):
         import warnings
 
-        # Create a minimal fake GAPIC.
-        firestore_api = mock.Mock(spec=["run_query"])
+        with mock.patch.object(self._get_target_class(), "stream") as stream_mock:
+            stream_mock.return_value = MockAsyncIter(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 = 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}
-        response_pb = _make_query_response(name=name, data=data)
-        firestore_api.run_query.return_value = iter([response_pb])
+            # Make a **real** collection reference as parent.
+            parent = client.collection("dee")
 
-        # Execute the query and check the response.
-        query = self._make_one(parent)
-
-        with warnings.catch_warnings(record=True) as warned:
-            get_response = query.get()
-            self.assertIsInstance(get_response, types.AsyncGeneratorType)
-            returned = [x async for x in get_response]
+            # Execute the query and check the response.
+            query = self._make_one(parent)
 
-        self.assertEqual(len(returned), 1)
-        snapshot = returned[0]
-        self.assertEqual(snapshot.reference._path, ("dee", "sleep"))
-        self.assertEqual(snapshot.to_dict(), data)
+            with warnings.catch_warnings(record=True) as warned:
+                get_response = query.get()
+                returned = [x async for x in get_response]
 
-        # 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,
-        )
+                # Verify that `get` merely wraps `stream`.
+                stream_mock.assert_called_once()
+                self.assertIsInstance(get_response, types.AsyncGeneratorType)
+                self.assertEqual(returned, list(range(stream_mock.return_value.count)))
 
-        # Verify the deprecation
-        self.assertEqual(len(warned), 1)
-        self.assertIs(warned[0].category, DeprecationWarning)
+            # Verify the deprecation.
+            self.assertEqual(len(warned), 1)
+            self.assertIs(warned[0].category, DeprecationWarning)
 
     @pytest.mark.asyncio
     async def test_stream_simple(self):

tests/unit/v1/test_base_collection.py

@@ -41,13 +41,19 @@ def test_constructor(self):
         expected_path = (collection_id1, document_id, collection_id2)
         self.assertEqual(collection._path, expected_path)
 
-    def test_constructor_invalid_path(self):
+    def test_constructor_invalid_path_empty(self):
         with self.assertRaises(ValueError):
             self._make_one()
+
+    def test_constructor_invalid_path_bad_collection_id(self):
         with self.assertRaises(ValueError):
             self._make_one(99, "doc", "bad-collection-id")
+
+    def test_constructor_invalid_path_bad_document_id(self):
         with self.assertRaises(ValueError):
             self._make_one("bad-document-ID", None, "sub-collection")
+
+    def test_constructor_invalid_path_bad_number_args(self):
         with self.assertRaises(ValueError):
             self._make_one("Just", "A-Document")
 

tests/unit/v1/test_base_document.py

@@ -47,13 +47,19 @@ def test_constructor(self):
         )
         self.assertEqual(document.path, expected_path)
 
-    def test_constructor_invalid_path(self):
+    def test_constructor_invalid_path_empty(self):
         with self.assertRaises(ValueError):
             self._make_one()
+
+    def test_constructor_invalid_path_bad_collection_id(self):
         with self.assertRaises(ValueError):
             self._make_one(None, "before", "bad-collection-id", "fifteen")
+
+    def test_constructor_invalid_path_bad_document_id(self):
         with self.assertRaises(ValueError):
             self._make_one("bad-document-ID", None)
+
+    def test_constructor_invalid_path_bad_number_args(self):
         with self.assertRaises(ValueError):
             self._make_one("Just", "A-Collection", "Sub")
 

tests/unit/v1/test_collection.py

@@ -68,20 +68,6 @@ def test_constructor(self):
         expected_path = (collection_id1, document_id, collection_id2)
         self.assertEqual(collection._path, expected_path)
 
-    def test_constructor_invalid_path(self):
-        with self.assertRaises(ValueError):
-            self._make_one()
-        with self.assertRaises(ValueError):
-            self._make_one(99, "doc", "bad-collection-id")
-        with self.assertRaises(ValueError):
-            self._make_one("bad-document-ID", None, "sub-collection")
-        with self.assertRaises(ValueError):
-            self._make_one("Just", "A-Document")
-
-    def test_constructor_invalid_kwarg(self):
-        with self.assertRaises(TypeError):
-            self._make_one("Coh-lek-shun", donut=True)
-
     def test_add_auto_assigned(self):
         from google.cloud.firestore_v1.types import document
         from google.cloud.firestore_v1.document import DocumentReference

tests/unit/v1/test_document.py

@@ -46,20 +46,6 @@ def test_constructor(self):
         )
         self.assertEqual(document.path, expected_path)
 
-    def test_constructor_invalid_path(self):
-        with self.assertRaises(ValueError):
-            self._make_one()
-        with self.assertRaises(ValueError):
-            self._make_one(None, "before", "bad-collection-id", "fifteen")
-        with self.assertRaises(ValueError):
-            self._make_one("bad-document-ID", None)
-        with self.assertRaises(ValueError):
-            self._make_one("Just", "A-Collection", "Sub")
-
-    def test_constructor_invalid_kwarg(self):
-        with self.assertRaises(TypeError):
-            self._make_one("Coh-lek-shun", "Dahk-yu-mehnt", burger=18.75)
-
     @staticmethod
     def _make_commit_repsonse(write_results=None):
         from google.cloud.firestore_v1.types import firestore

tests/unit/v1/test_query.py

@@ -31,7 +31,7 @@ def _make_one(self, *args, **kwargs):
         klass = self._get_target_class()
         return klass(*args, **kwargs)
 
-    def test_constructor_defaults(self):
+    def test_constructor(self):
         query = self._make_one(mock.sentinel.parent)
         self.assertIs(query._parent, mock.sentinel.parent)
         self.assertIsNone(query._projection)
@@ -43,53 +43,33 @@ def test_constructor_defaults(self):
         self.assertIsNone(query._end_at)
         self.assertFalse(query._all_descendants)
 
-    def test_get_simple(self):
+    def test_get(self):
         import warnings
 
-        # Create a minimal fake GAPIC.
-        firestore_api = mock.Mock(spec=["run_query"])
+        with mock.patch.object(self._get_target_class(), "stream") as stream_mock:
+            # 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
+            # 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")
+            # 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)
 
-        # Execute the query and check the response.
-        query = self._make_one(parent)
+            with warnings.catch_warnings(record=True) as warned:
+                get_response = query.get()
 
-        with warnings.catch_warnings(record=True) as warned:
-            get_response = query.get()
-
-        self.assertIsInstance(get_response, types.GeneratorType)
-        returned = list(get_response)
-        self.assertEqual(len(returned), 1)
-        snapshot = returned[0]
-        self.assertEqual(snapshot.reference._path, ("dee", "sleep"))
-        self.assertEqual(snapshot.to_dict(), data)
-
-        # 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,
-        )
+                # Verify that `get` merely wraps `stream`.
+                stream_mock.assert_called_once()
+                self.assertEqual(get_response, stream_mock.return_value)
 
-        # Verify the deprecation
-        self.assertEqual(len(warned), 1)
-        self.assertIs(warned[0].category, DeprecationWarning)
+            # Verify the deprecation.
+            self.assertEqual(len(warned), 1)
+            self.assertIs(warned[0].category, DeprecationWarning)
 
     def test_stream_simple(self):
         # Create a minimal fake GAPIC.