Merge pull request #852 from tseaver/825-storage_property_mixin-allow…

…_explicit_connection

#825: Allow passing explicit connection to '_PropertyMixin.{reload,patch}'.

gcloud/storage/_helpers.py

@@ -32,11 +32,6 @@ class _PropertyMixin(object):
       - path
     """
 
-    @property
-    def connection(self):
-        """Abstract getter for the connection to use."""
-        raise NotImplementedError
-
     @property
     def path(self):
         """Abstract getter for the object path."""
@@ -52,12 +47,19 @@ def __init__(self, name=None):
         self._properties = {}
         self._changes = set()
 
-    def reload(self):
-        """Reload properties from Cloud Storage."""
+    def reload(self, connection=None):
+        """Reload properties from Cloud Storage.
+
+        :type connection: :class:`gcloud.storage.connection.Connection`
+        :param connection: An explicit connection to use for the API request.
+                           If not passed, use the connection assigned to
+                           the object in its constructor.
+        """
+        connection = _require_connection(connection)
         # Pass only '?projection=noAcl' here because 'acl' and related
         # are handled via custom endpoints.
         query_params = {'projection': 'noAcl'}
-        api_response = self.connection.api_request(
+        api_response = connection.api_request(
             method='GET', path=self.path, query_params=query_params)
         self._set_properties(api_response)
 
@@ -89,16 +91,22 @@ def _set_properties(self, value):
         # If the values are reset, the changes must as well.
         self._changes = set()
 
-    def patch(self):
+    def patch(self, connection=None):
         """Sends all changed properties in a PATCH request.
 
         Updates the ``_properties`` with the response from the backend.
+
+        :type connection: :class:`gcloud.storage.connection.Connection`
+        :param connection: An explicit connection to use for the API request.
+                           If not passed, use the connection assigned to
+                           the object in its constructor.
         """
+        connection = _require_connection(connection)
         # Pass '?projection=full' here because 'PATCH' documented not
         # to work properly w/ 'noAcl'.
         update_properties = dict((key, self._properties[key])
                                  for key in self._changes)
-        api_response = self.connection.api_request(
+        api_response = connection.api_request(
             method='PATCH', path=self.path, data=update_properties,
             query_params={'projection': 'full'})
         self._set_properties(api_response)

gcloud/storage/api.py

@@ -160,7 +160,7 @@ def get_bucket(bucket_name, connection=None):
     """
     connection = _require_connection(connection)
     bucket = Bucket(bucket_name, connection=connection)
-    bucket.reload()
+    bucket.reload(connection=connection)
     return bucket
 
 

gcloud/storage/test__helpers.py

@@ -24,34 +24,46 @@ def _getTargetClass(self):
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
-    def _derivedClass(self, connection=None, path=None):
+    def _derivedClass(self, path=None):
 
         class Derived(self._getTargetClass()):
 
-            @property
-            def connection(self):
-                return connection
-
             @property
             def path(self):
                 return path
 
         return Derived
 
-    def test_connection_is_abstract(self):
-        mixin = self._makeOne()
-        self.assertRaises(NotImplementedError, lambda: mixin.connection)
+    def _monkey(self, connection):
+        from gcloud.storage._testing import _monkey_defaults
+        return _monkey_defaults(connection=connection)
 
     def test_path_is_abstract(self):
         mixin = self._makeOne()
         self.assertRaises(NotImplementedError, lambda: mixin.path)
 
-    def test_reload(self):
+    def test_reload_w_implicit_connection(self):
+        connection = _Connection({'foo': 'Foo'})
+        derived = self._derivedClass('/path')()
+        # Make sure changes is not a set, so we can observe a change.
+        derived._changes = object()
+        with self._monkey(connection):
+            derived.reload()
+        self.assertEqual(derived._properties, {'foo': 'Foo'})
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/path')
+        self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
+        # Make sure changes get reset by reload.
+        self.assertEqual(derived._changes, set())
+
+    def test_reload_w_explicit_connection(self):
         connection = _Connection({'foo': 'Foo'})
-        derived = self._derivedClass(connection, '/path')()
+        derived = self._derivedClass('/path')()
         # Make sure changes is not a set, so we can observe a change.
         derived._changes = object()
-        derived.reload()
+        derived.reload(connection)
         self.assertEqual(derived._properties, {'foo': 'Foo'})
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -66,15 +78,36 @@ def test__patch_property(self):
         derived._patch_property('foo', 'Foo')
         self.assertEqual(derived._properties, {'foo': 'Foo'})
 
-    def test_patch(self):
+    def test_patch_w_implicit_connection(self):
+        connection = _Connection({'foo': 'Foo'})
+        derived = self._derivedClass('/path')()
+        # Make sure changes is non-empty, so we can observe a change.
+        BAR = object()
+        BAZ = object()
+        derived._properties = {'bar': BAR, 'baz': BAZ}
+        derived._changes = set(['bar'])  # Ignore baz.
+        with self._monkey(connection):
+            derived.patch()
+        self.assertEqual(derived._properties, {'foo': 'Foo'})
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/path')
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        # Since changes does not include `baz`, we don't see it sent.
+        self.assertEqual(kw[0]['data'], {'bar': BAR})
+        # Make sure changes get reset by patch().
+        self.assertEqual(derived._changes, set())
+
+    def test_patch_w_explicit_connection(self):
         connection = _Connection({'foo': 'Foo'})
-        derived = self._derivedClass(connection, '/path')()
+        derived = self._derivedClass('/path')()
         # Make sure changes is non-empty, so we can observe a change.
         BAR = object()
         BAZ = object()
         derived._properties = {'bar': BAR, 'baz': BAZ}
         derived._changes = set(['bar'])  # Ignore baz.
-        derived.patch()
+        derived.patch(connection)
         self.assertEqual(derived._properties, {'foo': 'Foo'})
         kw = connection._requested
         self.assertEqual(len(kw), 1)