From cbc16d3134bbb594a129467da51a066847fc8d7e Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 1 May 2015 11:48:05 -0400
Subject: [PATCH 1/7] Hoist 'save'/'reload'/'clear' implementations into base
 ACL class.

Use new 'connection', 'reload_path', and 'save_path' properties to make
the base class implementation generic.
---
 gcloud/storage/acl.py      | 165 +++++++++------------
 gcloud/storage/test_acl.py | 293 +++++++++++++------------------------
 2 files changed, 177 insertions(+), 281 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index a2eaf01b2f97..fe0c7ea11642 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -167,6 +167,7 @@ def revoke_owner(self):
 class ACL(object):
     """Container class representing a list of access controls."""
 
+    _URL_PATH_ELEM = 'acl'
     loaded = False
 
     def __init__(self):
@@ -345,50 +346,59 @@ def get_entities(self):
         self._ensure_loaded()
         return list(self.entities.values())
 
-    def reload(self):
-        """Reload the ACL data from Cloud Storage.
+    @property
+    def connection(self):
+        """Compute the connection for API requests for this ACL.
 
         This is a virtual method, expected to be implemented by subclasses.
 
-        :raises: :class:`NotImplementedError`
+        :raises: :class:`NotImplementedError` if ``_connection`` attribute
+                 is not set on the instance.
+
         """
+        # Allow override for testing
+        connection = getattr(self, '_connection', None)
+        if connection is not None:
+            return connection
         raise NotImplementedError
 
-    def save(self, acl=None):
-        """A method to be overridden by subclasses.
+    @property
+    def reload_path(self):
+        """Compute the path for GET API requests for this ACL.
 
-        :type acl: :class:`gcloud.storage.acl.ACL`, or a compatible list.
-        :param acl: The ACL object to save.  If left blank, this will save
-                    current entries.
+        This is a virtual method, expected to be implemented by subclasses.
 
-        :raises: NotImplementedError
+        :raises: :class:`NotImplementedError` if ``_reload_path`` attribute
+                 is not set on the instance.
         """
+        # Allow override for testing
+        path = getattr(self, '_reload_path', None)
+        if path is not None:
+            return path
         raise NotImplementedError
 
-    def clear(self):
-        """Remove all entities from the ACL."""
-        raise NotImplementedError
-
+    @property
+    def save_path(self):
+        """Compute the path for PATCH API requests for this ACL.
 
-class BucketACL(ACL):
-    """An ACL specifically for a bucket."""
+        This is a virtual method, expected to be implemented by subclasses.
 
-    _URL_PATH_ELEM = 'acl'
+        :raises: :class:`NotImplementedError` if ``_save_path`` attribute
+                 is not set on the instance.
 
-    def __init__(self, bucket):
         """
-        :type bucket: :class:`gcloud.storage.bucket.Bucket`
-        :param bucket: The bucket to which this ACL relates.
-        """
-        super(BucketACL, self).__init__()
-        self.bucket = bucket
+        # Allow override for testing
+        path = getattr(self, '_save_path', None)
+        if path is not None:
+            return path
+        raise NotImplementedError
 
     def reload(self):
         """Reload the ACL data from Cloud Storage."""
         self.entities.clear()
 
-        url_path = '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
-        found = self.bucket.connection.api_request(method='GET', path=url_path)
+        found = self.connection.api_request(method='GET',
+                                            path=self.reload_path)
         self.loaded = True
         for entry in found.get('items', ()):
             self.add_entity(self.entity_from_dict(entry))
@@ -396,23 +406,6 @@ def reload(self):
     def save(self, acl=None):
         """Save this ACL for the current bucket.
 
-        If called without arguments, this will save the entries
-        currently stored on this ACL::
-
-           >>> acl.save()
-
-        You can also provide a specific ACL to save instead of the one
-        currently set on the Bucket object::
-
-           >>> acl.save(acl=my_other_acl)
-
-        You can use this to set access controls to be consistent from
-        one bucket to another::
-
-          >>> bucket1 = storage.get_bucket(bucket1_name, connection=connection)
-          >>> bucket2 = storage.get_bucket(bucket2_name, connection=connection)
-          >>> bucket2.acl.save(bucket1.acl)
-
         :type acl: :class:`gcloud.storage.acl.ACL`, or a compatible list.
         :param acl: The ACL object to save.  If left blank, this will save
                     current entries.
@@ -424,8 +417,8 @@ def save(self, acl=None):
             save_to_backend = True
 
         if save_to_backend:
-            result = self.bucket.connection.api_request(
-                method='PATCH', path=self.bucket.path,
+            result = self.connection.api_request(
+                method='PATCH', path=self.save_path,
                 data={self._URL_PATH_ELEM: list(acl)},
                 query_params={'projection': 'full'})
             self.entities.clear()
@@ -440,22 +433,35 @@ def clear(self):
         will remove all the non-default rules.  In short, you'll still
         have access to a bucket that you created even after you clear
         ACL rules with this method.
+        """
+        self.save([])
 
-        For example, imagine that you granted access to this bucket to a
-        bunch of coworkers::
 
-          >>> acl.user('coworker1@example.org').grant_read()
-          >>> acl.user('coworker2@example.org').grant_read()
-          >>> acl.save()
+class BucketACL(ACL):
+    """An ACL specifically for a bucket."""
 
-        Now they work in another part of the company and you want to
-        'start fresh' on who has access::
+    def __init__(self, bucket):
+        """
+        :type bucket: :class:`gcloud.storage.bucket.Bucket`
+        :param bucket: The bucket to which this ACL relates.
+        """
+        super(BucketACL, self).__init__()
+        self.bucket = bucket
 
-          >>> acl.clear()
+    @property
+    def connection(self):
+        """Compute the connection for API requests for this ACL."""
+        return self.bucket.connection
 
-        At this point all the custom rules you created have been removed.
-        """
-        self.save([])
+    @property
+    def reload_path(self):
+        """Compute the path for GET API requests for this ACL."""
+        return '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
+
+    @property
+    def save_path(self):
+        """Compute the path for PATCH API requests for this ACL."""
+        return self.bucket.path
 
 
 class DefaultObjectACL(BucketACL):
@@ -475,44 +481,17 @@ def __init__(self, blob):
         super(ObjectACL, self).__init__()
         self.blob = blob
 
-    def reload(self):
-        """Reload the ACL data from Cloud Storage."""
-        self.entities.clear()
-
-        url_path = '%s/acl' % self.blob.path
-        found = self.blob.connection.api_request(method='GET', path=url_path)
-        self.loaded = True
-        for entry in found.get('items', ()):
-            self.add_entity(self.entity_from_dict(entry))
-
-    def save(self, acl=None):
-        """Save the ACL data for this blob.
-
-        :type acl: :class:`gcloud.storage.acl.ACL`
-        :param acl: The ACL object to save.  If left blank, this will
-                    save the entries set locally on the ACL.
-        """
-        if acl is None:
-            acl = self
-            save_to_backend = acl.loaded
-        else:
-            save_to_backend = True
+    @property
+    def connection(self):
+        """Compute the connection for API requests for this ACL."""
+        return self.blob.connection
 
-        if save_to_backend:
-            result = self.blob.connection.api_request(
-                method='PATCH', path=self.blob.path, data={'acl': list(acl)},
-                query_params={'projection': 'full'})
-            self.entities.clear()
-            for entry in result.get('acl', ()):
-                self.add_entity(self.entity_from_dict(entry))
-            self.loaded = True
-
-    def clear(self):
-        """Remove all ACL rules from the blob.
+    @property
+    def reload_path(self):
+        """Compute the path for GET API requests for this ACL."""
+        return '%s/acl' % self.blob.path
 
-        Note that this won't actually remove *ALL* the rules, but it
-        will remove all the non-default rules.  In short, you'll still
-        have access to a blob that you created even after you clear ACL
-        rules with this method.
-        """
-        self.save([])
+    @property
+    def save_path(self):
+        """Compute the path for PATCH API requests for this ACL."""
+        return self.blob.path
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index c998cac895c5..e6c273891fd5 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -505,42 +505,44 @@ def test_get_entities_nonempty(self):
         entity = acl.entity(TYPE, ID)
         self.assertEqual(acl.get_entities(), [entity])
 
-    def test_reload_raises_NotImplementedError(self):
+    def test_connection_wo_attr(self):
         acl = self._makeOne()
-        self.assertRaises(NotImplementedError, acl.reload)
+        self.assertRaises(NotImplementedError, lambda: acl.connection)
 
-    def test_save_raises_NotImplementedError(self):
+    def test_connection_w_attr(self):
         acl = self._makeOne()
-        self.assertRaises(NotImplementedError, acl.save)
+        connection = acl._connection = object()
+        self.assertTrue(acl.connection is connection)
 
-    def test_clear(self):
+    def test_reload_path_wo_attr(self):
         acl = self._makeOne()
-        self.assertRaises(NotImplementedError, acl.clear)
+        self.assertRaises(NotImplementedError, lambda: acl.reload_path)
 
+    def test_reload_path_w_attr(self):
+        acl = self._makeOne()
+        acl._reload_path = '/testing/acl'
+        self.assertEqual(acl.reload_path, '/testing/acl')
 
-class Test_BucketACL(unittest2.TestCase):
-
-    def _getTargetClass(self):
-        from gcloud.storage.acl import BucketACL
-        return BucketACL
+    def test_save_path_wo_attr(self):
+        acl = self._makeOne()
+        self.assertRaises(NotImplementedError, lambda: acl.save_path)
 
-    def _makeOne(self, *args, **kw):
-        return self._getTargetClass()(*args, **kw)
+    def test_save_path_w_attr(self):
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        self.assertEqual(acl.save_path, '/testing')
 
-    def test_ctor(self):
-        bucket = object()
-        acl = self._makeOne(bucket)
-        self.assertEqual(acl.entities, {})
-        self.assertFalse(acl.loaded)
-        self.assertTrue(acl.bucket is bucket)
+    def test_reload_wo_attrs_raises_NotImplementedError(self):
+        acl = self._makeOne()
+        self.assertRaises(NotImplementedError, acl.reload)
 
     def test_reload_eager_missing(self):
         # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
-        NAME = 'name'
         ROLE = 'role'
         connection = _Connection({})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         acl.reload()
@@ -548,14 +550,14 @@ def test_reload_eager_missing(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing/acl')
 
     def test_reload_eager_empty(self):
-        NAME = 'name'
         ROLE = 'role'
         connection = _Connection({'items': []})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         acl.reload()
@@ -563,68 +565,73 @@ def test_reload_eager_empty(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing/acl')
 
     def test_reload_eager_nonempty(self):
-        NAME = 'name'
         ROLE = 'role'
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.reload()
         self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing/acl')
 
     def test_reload_lazy(self):
-        NAME = 'name'
         ROLE = 'role'
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._reload_path = '/testing/acl'
         acl.reload()
         self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing/acl')
+
+    def test_save_wo_attrs_raises_NotImplementedError(self):
+        acl = self._makeOne()
+        acl.loaded = True
+        self.assertRaises(NotImplementedError, acl.save)
 
     def test_save_none_set_none_passed(self):
-        NAME = 'name'
         connection = _Connection()
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._save_path = '/testing'
         acl.save()
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
     def test_save_existing_missing_none_passed(self):
-        NAME = 'name'
         connection = _Connection({})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._save_path = '/testing'
         acl.loaded = True
         acl.save()
         self.assertEqual(list(acl), [])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing')
         self.assertEqual(kw[0]['data'], {'acl': []})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_save_no_arg(self):
-        NAME = 'name'
         ROLE = 'role'
         AFTER = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'acl': AFTER})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers').grant(ROLE)
         acl.save()
@@ -632,19 +639,19 @@ def test_save_no_arg(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing')
         self.assertEqual(kw[0]['data'], {'acl': AFTER})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_save_w_arg(self):
-        NAME = 'name'
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         new_acl = [{'entity': 'allUsers', 'role': ROLE1}]
         connection = _Connection({'acl': [STICKY] + new_acl})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._save_path = '/testing'
         acl.loaded = True
         acl.save(new_acl)
         entries = list(acl)
@@ -654,18 +661,22 @@ def test_save_w_arg(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing')
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
+    def test_clear_wo_attrs_raises_NotImplementedError(self):
+        acl = self._makeOne()
+        self.assertRaises(NotImplementedError, acl.clear)
+
     def test_clear(self):
-        NAME = 'name'
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         connection = _Connection({'acl': [STICKY]})
-        bucket = _Bucket(NAME, connection)
-        acl = self._makeOne(bucket)
+        acl = self._makeOne()
+        acl._connection = connection
+        acl._save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers', ROLE1)
         acl.clear()
@@ -673,172 +684,78 @@ def test_clear(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
+        self.assertEqual(kw[0]['path'], '/testing')
         self.assertEqual(kw[0]['data'], {'acl': []})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
 
-class Test_ObjectACL(unittest2.TestCase):
+class Test_BucketACL(unittest2.TestCase):
 
     def _getTargetClass(self):
-        from gcloud.storage.acl import ObjectACL
-        return ObjectACL
+        from gcloud.storage.acl import BucketACL
+        return BucketACL
 
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
     def test_ctor(self):
-        blob = object()
-        acl = self._makeOne(blob)
+        NAME = 'name'
+        connection = _Connection()
+        bucket = _Bucket(NAME, connection)
+        acl = self._makeOne(bucket)
         self.assertEqual(acl.entities, {})
         self.assertFalse(acl.loaded)
-        self.assertTrue(acl.blob is blob)
+        self.assertTrue(acl.bucket is bucket)
+        self.assertTrue(acl.connection is connection)
+        self.assertEqual(acl.reload_path, '/b/%s/acl' % NAME)
+        self.assertEqual(acl.save_path, '/b/%s' % NAME)
 
-    def test_reload_eager_missing(self):
-        # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        after = {}
-        connection = _Connection(after)
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.entity('allUsers', ROLE)
-        acl.reload()
-        self.assertEqual(list(acl), [])
 
-    def test_reload_eager_empty(self):
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        after = {'items': []}
-        connection = _Connection(after)
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.entity('allUsers', ROLE)
-        acl.reload()
-        self.assertEqual(list(acl), [])
+class Test_DefaultObjectACL(unittest2.TestCase):
 
-    def test_reload_eager_nonempty(self):
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
-        connection = _Connection(after)
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.reload()
-        self.assertEqual(list(acl), after['items'])
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s/acl' % BLOB_NAME)
+    def _getTargetClass(self):
+        from gcloud.storage.acl import DefaultObjectACL
+        return DefaultObjectACL
 
-    def test_reload_lazy(self):
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
-        connection = _Connection(after)
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.reload()
-        self.assertEqual(list(acl),
-                         [{'entity': 'allUsers', 'role': ROLE}])
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s/acl' % BLOB_NAME)
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
 
-    def test_save_none_set_none_passed(self):
+    def test_ctor(self):
         NAME = 'name'
         BLOB_NAME = 'blob-name'
         connection = _Connection()
         bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.save()
-        kw = connection._requested
-        self.assertEqual(len(kw), 0)
+        acl = self._makeOne(bucket)
+        self.assertEqual(acl.entities, {})
+        self.assertFalse(acl.loaded)
+        self.assertTrue(acl.bucket is bucket)
+        self.assertTrue(acl.connection is connection)
+        self.assertEqual(acl.reload_path, '/b/%s/defaultObjectAcl' % NAME)
+        self.assertEqual(acl.save_path, '/b/%s' % NAME)
 
-    def test_save_existing_missing_none_passed(self):
-        # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        connection = _Connection({'foo': 'Foo'})
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.save()
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % BLOB_NAME)
-        self.assertEqual(kw[0]['data'], {'acl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_save_existing_set_none_passed(self):
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        connection = _Connection({'foo': 'Foo', 'acl': []})
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.save()
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % BLOB_NAME)
-        self.assertEqual(kw[0]['data'], {'acl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+class Test_ObjectACL(unittest2.TestCase):
 
-    def test_save_existing_set_new_passed(self):
-        NAME = 'name'
-        BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        new_acl = [{'entity': 'allUsers', 'role': ROLE}]
-        connection = _Connection({'foo': 'Foo', 'acl': new_acl})
-        bucket = _Bucket(NAME, connection)
-        blob = _Blob(bucket, BLOB_NAME)
-        acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.entity('allUsers', 'other-role')
-        acl.save(new_acl)
-        self.assertEqual(list(acl), new_acl)
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % BLOB_NAME)
-        self.assertEqual(kw[0]['data'], {'acl': new_acl})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+    def _getTargetClass(self):
+        from gcloud.storage.acl import ObjectACL
+        return ObjectACL
 
-    def test_clear(self):
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
+    def test_ctor(self):
         NAME = 'name'
         BLOB_NAME = 'blob-name'
-        ROLE = 'role'
-        connection = _Connection({'foo': 'Foo', 'acl': []})
+        connection = _Connection()
         bucket = _Bucket(NAME, connection)
         blob = _Blob(bucket, BLOB_NAME)
         acl = self._makeOne(blob)
-        acl.loaded = True
-        acl.entity('allUsers', ROLE)
-        acl.clear()
-        self.assertEqual(list(acl), [])
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % BLOB_NAME)
-        self.assertEqual(kw[0]['data'], {'acl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(acl.entities, {})
+        self.assertFalse(acl.loaded)
+        self.assertTrue(acl.blob is blob)
+        self.assertTrue(acl.connection is connection)
+        self.assertEqual(acl.reload_path, '/b/%s/o/%s/acl' % (NAME, BLOB_NAME))
+        self.assertEqual(acl.save_path, '/b/%s/o/%s' % (NAME, BLOB_NAME))
 
 
 class _Blob(object):

From f34ca23d85df2582b56529a967a1a3f610e59466 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 1 May 2015 12:01:31 -0400
Subject: [PATCH 2/7] Remove testing fossils from old lazy / eager split.

---
 gcloud/storage/test_acl.py | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index e6c273891fd5..dd930a06a4c8 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -536,7 +536,7 @@ def test_reload_wo_attrs_raises_NotImplementedError(self):
         acl = self._makeOne()
         self.assertRaises(NotImplementedError, acl.reload)
 
-    def test_reload_eager_missing(self):
+    def test_reload_missing(self):
         # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
         ROLE = 'role'
         connection = _Connection({})
@@ -552,7 +552,7 @@ def test_reload_eager_missing(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_eager_empty(self):
+    def test_reload_empty_result_clears_local(self):
         ROLE = 'role'
         connection = _Connection({'items': []})
         acl = self._makeOne()
@@ -561,13 +561,14 @@ def test_reload_eager_empty(self):
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         acl.reload()
+        self.assertTrue(acl.loaded)
         self.assertEqual(list(acl), [])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_eager_nonempty(self):
+    def test_reload_nonempty_result(self):
         ROLE = 'role'
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
@@ -576,20 +577,7 @@ def test_reload_eager_nonempty(self):
         acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.reload()
-        self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/testing/acl')
-
-    def test_reload_lazy(self):
-        ROLE = 'role'
-        connection = _Connection(
-            {'items': [{'entity': 'allUsers', 'role': ROLE}]})
-        acl = self._makeOne()
-        acl._connection = connection
-        acl._reload_path = '/testing/acl'
-        acl.reload()
+        self.assertTrue(acl.loaded)
         self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
         kw = connection._requested
         self.assertEqual(len(kw), 1)

From 065233ddbe60220ec63a02ce2c435763df9222c8 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 1 May 2015 12:12:35 -0400
Subject: [PATCH 3/7] Allow passing explicit connection to 'ACL.{reload,save}'.

See #825.
---
 gcloud/storage/acl.py      |  34 ++++++---
 gcloud/storage/test_acl.py | 139 ++++++++++++++++++++++++++++++++++---
 2 files changed, 157 insertions(+), 16 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index fe0c7ea11642..5dddc69da918 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -393,23 +393,37 @@ def save_path(self):
             return path
         raise NotImplementedError
 
-    def reload(self):
-        """Reload the ACL data from Cloud Storage."""
+    def reload(self, connection=None):
+        """Reload the ACL data from Cloud Storage.
+
+        :type connection: :class:`gcloud.storage.connection.Connection` or None
+        :param connection: explicit connection to use for API request;
+                           defaults to instance property.
+        """
+        if connection is None:
+            connection = self.connection
+
         self.entities.clear()
 
-        found = self.connection.api_request(method='GET',
-                                            path=self.reload_path)
+        found = connection.api_request(method='GET', path=self.reload_path)
         self.loaded = True
         for entry in found.get('items', ()):
             self.add_entity(self.entity_from_dict(entry))
 
-    def save(self, acl=None):
+    def save(self, acl=None, connection=None):
         """Save this ACL for the current bucket.
 
         :type acl: :class:`gcloud.storage.acl.ACL`, or a compatible list.
         :param acl: The ACL object to save.  If left blank, this will save
                     current entries.
+
+        :type connection: :class:`gcloud.storage.connection.Connection` or None
+        :param connection: explicit connection to use for API request;
+                           defaults to instance property.
         """
+        if connection is None:
+            connection = self.connection
+
         if acl is None:
             acl = self
             save_to_backend = acl.loaded
@@ -417,7 +431,7 @@ def save(self, acl=None):
             save_to_backend = True
 
         if save_to_backend:
-            result = self.connection.api_request(
+            result = connection.api_request(
                 method='PATCH', path=self.save_path,
                 data={self._URL_PATH_ELEM: list(acl)},
                 query_params={'projection': 'full'})
@@ -426,15 +440,19 @@ def save(self, acl=None):
                 self.add_entity(self.entity_from_dict(entry))
             self.loaded = True
 
-    def clear(self):
+    def clear(self, connection=None):
         """Remove all ACL entries.
 
         Note that this won't actually remove *ALL* the rules, but it
         will remove all the non-default rules.  In short, you'll still
         have access to a bucket that you created even after you clear
         ACL rules with this method.
+
+        :type connection: :class:`gcloud.storage.connection.Connection` or None
+        :param connection: explicit connection to use for API request;
+                           defaults to instance property.
         """
-        self.save([])
+        self.save([], connection)
 
 
 class BucketACL(ACL):
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index dd930a06a4c8..a415eda6a67b 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -536,7 +536,7 @@ def test_reload_wo_attrs_raises_NotImplementedError(self):
         acl = self._makeOne()
         self.assertRaises(NotImplementedError, acl.reload)
 
-    def test_reload_missing(self):
+    def test_reload_missing_w_connection_attr(self):
         # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
         ROLE = 'role'
         connection = _Connection({})
@@ -552,7 +552,22 @@ def test_reload_missing(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_empty_result_clears_local(self):
+    def test_reload_missing_w_explicit_connection(self):
+        # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
+        ROLE = 'role'
+        connection = _Connection({})
+        acl = self._makeOne()
+        acl._reload_path = '/testing/acl'
+        acl.loaded = True
+        acl.entity('allUsers', ROLE)
+        acl.reload(connection=connection)
+        self.assertEqual(list(acl), [])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/testing/acl')
+
+    def test_reload_empty_result_clears_local_w_connection_attr(self):
         ROLE = 'role'
         connection = _Connection({'items': []})
         acl = self._makeOne()
@@ -568,7 +583,22 @@ def test_reload_empty_result_clears_local(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_nonempty_result(self):
+    def test_reload_empty_result_clears_local_w_explicit_connection(self):
+        ROLE = 'role'
+        connection = _Connection({'items': []})
+        acl = self._makeOne()
+        acl._reload_path = '/testing/acl'
+        acl.loaded = True
+        acl.entity('allUsers', ROLE)
+        acl.reload(connection=connection)
+        self.assertTrue(acl.loaded)
+        self.assertEqual(list(acl), [])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/testing/acl')
+
+    def test_reload_nonempty_result_w_connection_attr(self):
         ROLE = 'role'
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
@@ -584,12 +614,27 @@ def test_reload_nonempty_result(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
+    def test_reload_nonempty_result_w_explicit_connection(self):
+        ROLE = 'role'
+        connection = _Connection(
+            {'items': [{'entity': 'allUsers', 'role': ROLE}]})
+        acl = self._makeOne()
+        acl._reload_path = '/testing/acl'
+        acl.loaded = True
+        acl.reload(connection=connection)
+        self.assertTrue(acl.loaded)
+        self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/testing/acl')
+
     def test_save_wo_attrs_raises_NotImplementedError(self):
         acl = self._makeOne()
         acl.loaded = True
         self.assertRaises(NotImplementedError, acl.save)
 
-    def test_save_none_set_none_passed(self):
+    def test_save_none_set_none_passed_w_connection_attr(self):
         connection = _Connection()
         acl = self._makeOne()
         acl._connection = connection
@@ -598,7 +643,15 @@ def test_save_none_set_none_passed(self):
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
-    def test_save_existing_missing_none_passed(self):
+    def test_save_none_set_none_passed_w_explicit_connection(self):
+        connection = _Connection()
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        acl.save(connection=connection)
+        kw = connection._requested
+        self.assertEqual(len(kw), 0)
+
+    def test_save_existing_missing_none_passed_w_connection_attr(self):
         connection = _Connection({})
         acl = self._makeOne()
         acl._connection = connection
@@ -613,7 +666,21 @@ def test_save_existing_missing_none_passed(self):
         self.assertEqual(kw[0]['data'], {'acl': []})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_save_no_arg(self):
+    def test_save_existing_missing_none_passed_w_explicit_connection(self):
+        connection = _Connection({})
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        acl.loaded = True
+        acl.save(connection=connection)
+        self.assertEqual(list(acl), [])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/testing')
+        self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
+    def test_save_no_arg_w_connection_attr(self):
         ROLE = 'role'
         AFTER = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'acl': AFTER})
@@ -631,7 +698,24 @@ def test_save_no_arg(self):
         self.assertEqual(kw[0]['data'], {'acl': AFTER})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_save_w_arg(self):
+    def test_save_no_arg_w_explicit_connection(self):
+        ROLE = 'role'
+        AFTER = [{'entity': 'allUsers', 'role': ROLE}]
+        connection = _Connection({'acl': AFTER})
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        acl.loaded = True
+        acl.entity('allUsers').grant(ROLE)
+        acl.save(connection=connection)
+        self.assertEqual(list(acl), AFTER)
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/testing')
+        self.assertEqual(kw[0]['data'], {'acl': AFTER})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
+    def test_save_w_arg_w_connection_attr(self):
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
@@ -653,11 +737,32 @@ def test_save_w_arg(self):
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
+    def test_save_w_arg_w_explicit_connection(self):
+        ROLE1 = 'role1'
+        ROLE2 = 'role2'
+        STICKY = {'entity': 'allUsers', 'role': ROLE2}
+        new_acl = [{'entity': 'allUsers', 'role': ROLE1}]
+        connection = _Connection({'acl': [STICKY] + new_acl})
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        acl.loaded = True
+        acl.save(new_acl, connection)
+        entries = list(acl)
+        self.assertEqual(len(entries), 2)
+        self.assertTrue(STICKY in entries)
+        self.assertTrue(new_acl[0] in entries)
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/testing')
+        self.assertEqual(kw[0]['data'], {'acl': new_acl})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
     def test_clear_wo_attrs_raises_NotImplementedError(self):
         acl = self._makeOne()
         self.assertRaises(NotImplementedError, acl.clear)
 
-    def test_clear(self):
+    def test_clear_w_connection_attr(self):
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
@@ -676,6 +781,24 @@ def test_clear(self):
         self.assertEqual(kw[0]['data'], {'acl': []})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
+    def test_clear_w_explicit_connection(self):
+        ROLE1 = 'role1'
+        ROLE2 = 'role2'
+        STICKY = {'entity': 'allUsers', 'role': ROLE2}
+        connection = _Connection({'acl': [STICKY]})
+        acl = self._makeOne()
+        acl._save_path = '/testing'
+        acl.loaded = True
+        acl.entity('allUsers', ROLE1)
+        acl.clear(connection=connection)
+        self.assertEqual(list(acl), [STICKY])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/testing')
+        self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
 
 class Test_BucketACL(unittest2.TestCase):
 

From 0ed115b06d2ef7dcdda9549fb9c16f9f3d6a61bc Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 1 May 2015 16:15:21 -0400
Subject: [PATCH 4/7] pylint: unused variable.

---
 gcloud/storage/test_acl.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index a415eda6a67b..ff68271acee0 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -833,7 +833,6 @@ def _makeOne(self, *args, **kw):
 
     def test_ctor(self):
         NAME = 'name'
-        BLOB_NAME = 'blob-name'
         connection = _Connection()
         bucket = _Bucket(NAME, connection)
         acl = self._makeOne(bucket)

From a83fec2de8419c0d9ae46f7e01a04121eda4c98f Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Mon, 4 May 2015 17:25:44 -0400
Subject: [PATCH 5/7] Drop 'ACL.connection' property IFO '_require_connection.

Addresses:
https://github.com/GoogleCloudPlatform/gcloud-python/pull/853#discussion_r29611944
---
 gcloud/storage/acl.py         | 32 +++++-----------
 gcloud/storage/test_acl.py    | 72 +++++++++++++++++------------------
 gcloud/storage/test_blob.py   |  4 +-
 gcloud/storage/test_bucket.py | 12 ++++--
 4 files changed, 55 insertions(+), 65 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index 5dddc69da918..e240c89b7426 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -78,6 +78,8 @@
 when sending metadata for ACLs to the API.
 """
 
+from gcloud.storage._helpers import _require_connection
+
 
 class _ACLEntity(object):
     """Class representing a set of roles for an entity.
@@ -346,22 +348,6 @@ def get_entities(self):
         self._ensure_loaded()
         return list(self.entities.values())
 
-    @property
-    def connection(self):
-        """Compute the connection for API requests for this ACL.
-
-        This is a virtual method, expected to be implemented by subclasses.
-
-        :raises: :class:`NotImplementedError` if ``_connection`` attribute
-                 is not set on the instance.
-
-        """
-        # Allow override for testing
-        connection = getattr(self, '_connection', None)
-        if connection is not None:
-            return connection
-        raise NotImplementedError
-
     @property
     def reload_path(self):
         """Compute the path for GET API requests for this ACL.
@@ -400,12 +386,12 @@ def reload(self, connection=None):
         :param connection: explicit connection to use for API request;
                            defaults to instance property.
         """
-        if connection is None:
-            connection = self.connection
+        path = self.reload_path
+        connection = _require_connection(connection)
 
         self.entities.clear()
 
-        found = connection.api_request(method='GET', path=self.reload_path)
+        found = connection.api_request(method='GET', path=path)
         self.loaded = True
         for entry in found.get('items', ()):
             self.add_entity(self.entity_from_dict(entry))
@@ -421,9 +407,6 @@ def save(self, acl=None, connection=None):
         :param connection: explicit connection to use for API request;
                            defaults to instance property.
         """
-        if connection is None:
-            connection = self.connection
-
         if acl is None:
             acl = self
             save_to_backend = acl.loaded
@@ -431,8 +414,11 @@ def save(self, acl=None, connection=None):
             save_to_backend = True
 
         if save_to_backend:
+            path = self.save_path
+            connection = _require_connection(connection)
             result = connection.api_request(
-                method='PATCH', path=self.save_path,
+                method='PATCH',
+                path=path,
                 data={self._URL_PATH_ELEM: list(acl)},
                 query_params={'projection': 'full'})
             self.entities.clear()
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index ff68271acee0..3170cd67c3fd 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -505,15 +505,6 @@ def test_get_entities_nonempty(self):
         entity = acl.entity(TYPE, ID)
         self.assertEqual(acl.get_entities(), [entity])
 
-    def test_connection_wo_attr(self):
-        acl = self._makeOne()
-        self.assertRaises(NotImplementedError, lambda: acl.connection)
-
-    def test_connection_w_attr(self):
-        acl = self._makeOne()
-        connection = acl._connection = object()
-        self.assertTrue(acl.connection is connection)
-
     def test_reload_path_wo_attr(self):
         acl = self._makeOne()
         self.assertRaises(NotImplementedError, lambda: acl.reload_path)
@@ -532,20 +523,21 @@ def test_save_path_w_attr(self):
         acl._save_path = '/testing'
         self.assertEqual(acl.save_path, '/testing')
 
-    def test_reload_wo_attrs_raises_NotImplementedError(self):
+    def test_reload_wo_path_raises_NotImplementedError(self):
         acl = self._makeOne()
         self.assertRaises(NotImplementedError, acl.reload)
 
-    def test_reload_missing_w_connection_attr(self):
+    def test_reload_missing_w_implicit_connection(self):
         # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
+        from gcloud.storage._testing import _monkey_defaults
         ROLE = 'role'
         connection = _Connection({})
         acl = self._makeOne()
-        acl._connection = connection
         acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
-        acl.reload()
+        with _monkey_defaults(connection=connection):
+            acl.reload()
         self.assertEqual(list(acl), [])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -567,15 +559,16 @@ def test_reload_missing_w_explicit_connection(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_empty_result_clears_local_w_connection_attr(self):
+    def test_reload_empty_result_clears_local_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         ROLE = 'role'
         connection = _Connection({'items': []})
         acl = self._makeOne()
-        acl._connection = connection
         acl._reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
-        acl.reload()
+        with _monkey_defaults(connection=connection):
+            acl.reload()
         self.assertTrue(acl.loaded)
         self.assertEqual(list(acl), [])
         kw = connection._requested
@@ -598,15 +591,16 @@ def test_reload_empty_result_clears_local_w_explicit_connection(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_reload_nonempty_result_w_connection_attr(self):
+    def test_reload_nonempty_result_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         ROLE = 'role'
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
         acl = self._makeOne()
-        acl._connection = connection
         acl._reload_path = '/testing/acl'
         acl.loaded = True
-        acl.reload()
+        with _monkey_defaults(connection=connection):
+            acl.reload()
         self.assertTrue(acl.loaded)
         self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}])
         kw = connection._requested
@@ -629,17 +623,19 @@ def test_reload_nonempty_result_w_explicit_connection(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_save_wo_attrs_raises_NotImplementedError(self):
+    def test_save_wo_path_raises_NotImplementedError(self):
         acl = self._makeOne()
         acl.loaded = True
         self.assertRaises(NotImplementedError, acl.save)
 
-    def test_save_none_set_none_passed_w_connection_attr(self):
+    def test_save_none_set_none_passed_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         connection = _Connection()
         acl = self._makeOne()
         acl._connection = connection
         acl._save_path = '/testing'
-        acl.save()
+        with _monkey_defaults(connection=connection):
+            acl.save()
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
@@ -651,13 +647,14 @@ def test_save_none_set_none_passed_w_explicit_connection(self):
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
-    def test_save_existing_missing_none_passed_w_connection_attr(self):
+    def test_save_existing_missing_none_passed_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         connection = _Connection({})
         acl = self._makeOne()
-        acl._connection = connection
         acl._save_path = '/testing'
         acl.loaded = True
-        acl.save()
+        with _monkey_defaults(connection=connection):
+            acl.save()
         self.assertEqual(list(acl), [])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -680,16 +677,17 @@ def test_save_existing_missing_none_passed_w_explicit_connection(self):
         self.assertEqual(kw[0]['data'], {'acl': []})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_save_no_arg_w_connection_attr(self):
+    def test_save_no_arg_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         ROLE = 'role'
         AFTER = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'acl': AFTER})
         acl = self._makeOne()
-        acl._connection = connection
         acl._save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers').grant(ROLE)
-        acl.save()
+        with _monkey_defaults(connection=connection):
+            acl.save()
         self.assertEqual(list(acl), AFTER)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -715,17 +713,18 @@ def test_save_no_arg_w_explicit_connection(self):
         self.assertEqual(kw[0]['data'], {'acl': AFTER})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_save_w_arg_w_connection_attr(self):
+    def test_save_w_arg_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         new_acl = [{'entity': 'allUsers', 'role': ROLE1}]
         connection = _Connection({'acl': [STICKY] + new_acl})
         acl = self._makeOne()
-        acl._connection = connection
         acl._save_path = '/testing'
         acl.loaded = True
-        acl.save(new_acl)
+        with _monkey_defaults(connection=connection):
+            acl.save(new_acl)
         entries = list(acl)
         self.assertEqual(len(entries), 2)
         self.assertTrue(STICKY in entries)
@@ -758,21 +757,18 @@ def test_save_w_arg_w_explicit_connection(self):
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_clear_wo_attrs_raises_NotImplementedError(self):
-        acl = self._makeOne()
-        self.assertRaises(NotImplementedError, acl.clear)
-
-    def test_clear_w_connection_attr(self):
+    def test_clear_w_implicit_connection(self):
+        from gcloud.storage._testing import _monkey_defaults
         ROLE1 = 'role1'
         ROLE2 = 'role2'
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         connection = _Connection({'acl': [STICKY]})
         acl = self._makeOne()
-        acl._connection = connection
         acl._save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers', ROLE1)
-        acl.clear()
+        with _monkey_defaults(connection=connection):
+            acl.clear()
         self.assertEqual(list(acl), [STICKY])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
diff --git a/gcloud/storage/test_blob.py b/gcloud/storage/test_blob.py
index 07791862ba53..d5221f330b79 100644
--- a/gcloud/storage/test_blob.py
+++ b/gcloud/storage/test_blob.py
@@ -704,6 +704,7 @@ def test_upload_from_string_w_text(self):
 
     def test_make_public(self):
         from gcloud.storage.acl import _ACLEntity
+        from gcloud.storage._testing import _monkey_defaults
         BLOB_NAME = 'blob-name'
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after = {'acl': permissive}
@@ -711,7 +712,8 @@ def test_make_public(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.acl.loaded = True
-        blob.make_public()
+        with _monkey_defaults(connection=connection):
+            blob.make_public()
         self.assertEqual(list(blob.acl), permissive)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index 4f31859c7970..34cc2ec7b6b6 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -872,6 +872,7 @@ def test_disable_website(self):
 
     def test_make_public_defaults(self):
         from gcloud.storage.acl import _ACLEntity
+        from gcloud.storage._testing import _monkey_defaults
         NAME = 'name'
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after = {'acl': permissive, 'defaultObjectAcl': []}
@@ -879,7 +880,8 @@ def test_make_public_defaults(self):
         bucket = self._makeOne(NAME, connection)
         bucket.acl.loaded = True
         bucket.default_object_acl.loaded = True
-        bucket.make_public()
+        with _monkey_defaults(connection=connection):
+            bucket.make_public()
         self.assertEqual(list(bucket.acl), permissive)
         self.assertEqual(list(bucket.default_object_acl), [])
         kw = connection._requested
@@ -891,6 +893,7 @@ def test_make_public_defaults(self):
 
     def _make_public_w_future_helper(self, default_object_acl_loaded=True):
         from gcloud.storage.acl import _ACLEntity
+        from gcloud.storage._testing import _monkey_defaults
         NAME = 'name'
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after1 = {'acl': permissive, 'defaultObjectAcl': []}
@@ -906,7 +909,8 @@ def _make_public_w_future_helper(self, default_object_acl_loaded=True):
         bucket = self._makeOne(NAME, connection)
         bucket.acl.loaded = True
         bucket.default_object_acl.loaded = default_object_acl_loaded
-        bucket.make_public(future=True)
+        with _monkey_defaults(connection=connection):
+            bucket.make_public(future=True)
         self.assertEqual(list(bucket.acl), permissive)
         self.assertEqual(list(bucket.default_object_acl), permissive)
         kw = connection._requested
@@ -933,6 +937,7 @@ def test_make_public_w_future_reload_default(self):
     def test_make_public_recursive(self):
         from gcloud.storage.acl import _ACLEntity
         from gcloud.storage.bucket import _BlobIterator
+        from gcloud.storage._testing import _monkey_defaults
         _saved = []
 
         class _Blob(object):
@@ -969,7 +974,8 @@ def get_items_from_response(self, response):
         bucket.acl.loaded = True
         bucket.default_object_acl.loaded = True
         bucket._iterator_class = _Iterator
-        bucket.make_public(recursive=True)
+        with _monkey_defaults(connection=connection):
+            bucket.make_public(recursive=True)
         self.assertEqual(list(bucket.acl), permissive)
         self.assertEqual(list(bucket.default_object_acl), [])
         self.assertEqual(_saved, [(bucket, BLOB_NAME, True)])

From 324dea71f66203fcd5ada7eeb33c235728db77f1 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Mon, 4 May 2015 17:48:13 -0400
Subject: [PATCH 6/7] Rip out vestiges of 'ACL.connection' property.

---
 gcloud/storage/acl.py      | 10 ----------
 gcloud/storage/test_acl.py | 19 ++++---------------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index e240c89b7426..9afadddf282b 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -452,11 +452,6 @@ def __init__(self, bucket):
         super(BucketACL, self).__init__()
         self.bucket = bucket
 
-    @property
-    def connection(self):
-        """Compute the connection for API requests for this ACL."""
-        return self.bucket.connection
-
     @property
     def reload_path(self):
         """Compute the path for GET API requests for this ACL."""
@@ -485,11 +480,6 @@ def __init__(self, blob):
         super(ObjectACL, self).__init__()
         self.blob = blob
 
-    @property
-    def connection(self):
-        """Compute the connection for API requests for this ACL."""
-        return self.blob.connection
-
     @property
     def reload_path(self):
         """Compute the path for GET API requests for this ACL."""
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index 3170cd67c3fd..becf1b640bf1 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -807,13 +807,11 @@ def _makeOne(self, *args, **kw):
 
     def test_ctor(self):
         NAME = 'name'
-        connection = _Connection()
-        bucket = _Bucket(NAME, connection)
+        bucket = _Bucket(NAME)
         acl = self._makeOne(bucket)
         self.assertEqual(acl.entities, {})
         self.assertFalse(acl.loaded)
         self.assertTrue(acl.bucket is bucket)
-        self.assertTrue(acl.connection is connection)
         self.assertEqual(acl.reload_path, '/b/%s/acl' % NAME)
         self.assertEqual(acl.save_path, '/b/%s' % NAME)
 
@@ -829,13 +827,11 @@ def _makeOne(self, *args, **kw):
 
     def test_ctor(self):
         NAME = 'name'
-        connection = _Connection()
-        bucket = _Bucket(NAME, connection)
+        bucket = _Bucket(NAME)
         acl = self._makeOne(bucket)
         self.assertEqual(acl.entities, {})
         self.assertFalse(acl.loaded)
         self.assertTrue(acl.bucket is bucket)
-        self.assertTrue(acl.connection is connection)
         self.assertEqual(acl.reload_path, '/b/%s/defaultObjectAcl' % NAME)
         self.assertEqual(acl.save_path, '/b/%s' % NAME)
 
@@ -852,14 +848,12 @@ def _makeOne(self, *args, **kw):
     def test_ctor(self):
         NAME = 'name'
         BLOB_NAME = 'blob-name'
-        connection = _Connection()
-        bucket = _Bucket(NAME, connection)
+        bucket = _Bucket(NAME)
         blob = _Blob(bucket, BLOB_NAME)
         acl = self._makeOne(blob)
         self.assertEqual(acl.entities, {})
         self.assertFalse(acl.loaded)
         self.assertTrue(acl.blob is blob)
-        self.assertTrue(acl.connection is connection)
         self.assertEqual(acl.reload_path, '/b/%s/o/%s/acl' % (NAME, BLOB_NAME))
         self.assertEqual(acl.save_path, '/b/%s/o/%s' % (NAME, BLOB_NAME))
 
@@ -870,10 +864,6 @@ def __init__(self, bucket, blob):
         self.bucket = bucket
         self.blob = blob
 
-    @property
-    def connection(self):
-        return self.bucket.connection
-
     @property
     def path(self):
         return '%s/o/%s' % (self.bucket.path, self.blob)
@@ -881,9 +871,8 @@ def path(self):
 
 class _Bucket(object):
 
-    def __init__(self, name, connection):
+    def __init__(self, name):
         self.name = name
-        self.connection = connection
 
     @property
     def path(self):

From d4bc450aa764930964c8c6691710c4bc55bff036 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 5 May 2015 18:48:45 -0400
Subject: [PATCH 7/7] Remove 'pure virtualness' from
 ACL.{reload_path,save_path}.

Addresses:
https://github.com/GoogleCloudPlatform/gcloud-python/pull/853#discussion_r29627331
---
 gcloud/storage/acl.py      | 36 ++++-------------------
 gcloud/storage/test_acl.py | 59 +++++++++++---------------------------
 2 files changed, 21 insertions(+), 74 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index 9afadddf282b..84e3c7911876 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -172,6 +172,11 @@ class ACL(object):
     _URL_PATH_ELEM = 'acl'
     loaded = False
 
+    # Subclasses must override to provide these attributes (typically,
+    # as properties).
+    reload_path = None
+    save_path = None
+
     def __init__(self):
         self.entities = {}
 
@@ -348,37 +353,6 @@ def get_entities(self):
         self._ensure_loaded()
         return list(self.entities.values())
 
-    @property
-    def reload_path(self):
-        """Compute the path for GET API requests for this ACL.
-
-        This is a virtual method, expected to be implemented by subclasses.
-
-        :raises: :class:`NotImplementedError` if ``_reload_path`` attribute
-                 is not set on the instance.
-        """
-        # Allow override for testing
-        path = getattr(self, '_reload_path', None)
-        if path is not None:
-            return path
-        raise NotImplementedError
-
-    @property
-    def save_path(self):
-        """Compute the path for PATCH API requests for this ACL.
-
-        This is a virtual method, expected to be implemented by subclasses.
-
-        :raises: :class:`NotImplementedError` if ``_save_path`` attribute
-                 is not set on the instance.
-
-        """
-        # Allow override for testing
-        path = getattr(self, '_save_path', None)
-        if path is not None:
-            return path
-        raise NotImplementedError
-
     def reload(self, connection=None):
         """Reload the ACL data from Cloud Storage.
 
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index becf1b640bf1..7c0b9fb197e1 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -505,35 +505,13 @@ def test_get_entities_nonempty(self):
         entity = acl.entity(TYPE, ID)
         self.assertEqual(acl.get_entities(), [entity])
 
-    def test_reload_path_wo_attr(self):
-        acl = self._makeOne()
-        self.assertRaises(NotImplementedError, lambda: acl.reload_path)
-
-    def test_reload_path_w_attr(self):
-        acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
-        self.assertEqual(acl.reload_path, '/testing/acl')
-
-    def test_save_path_wo_attr(self):
-        acl = self._makeOne()
-        self.assertRaises(NotImplementedError, lambda: acl.save_path)
-
-    def test_save_path_w_attr(self):
-        acl = self._makeOne()
-        acl._save_path = '/testing'
-        self.assertEqual(acl.save_path, '/testing')
-
-    def test_reload_wo_path_raises_NotImplementedError(self):
-        acl = self._makeOne()
-        self.assertRaises(NotImplementedError, acl.reload)
-
     def test_reload_missing_w_implicit_connection(self):
         # https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
         from gcloud.storage._testing import _monkey_defaults
         ROLE = 'role'
         connection = _Connection({})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         with _monkey_defaults(connection=connection):
@@ -549,7 +527,7 @@ def test_reload_missing_w_explicit_connection(self):
         ROLE = 'role'
         connection = _Connection({})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         acl.reload(connection=connection)
@@ -564,7 +542,7 @@ def test_reload_empty_result_clears_local_w_implicit_connection(self):
         ROLE = 'role'
         connection = _Connection({'items': []})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         with _monkey_defaults(connection=connection):
@@ -580,7 +558,7 @@ def test_reload_empty_result_clears_local_w_explicit_connection(self):
         ROLE = 'role'
         connection = _Connection({'items': []})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         acl.entity('allUsers', ROLE)
         acl.reload(connection=connection)
@@ -597,7 +575,7 @@ def test_reload_nonempty_result_w_implicit_connection(self):
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         with _monkey_defaults(connection=connection):
             acl.reload()
@@ -613,7 +591,7 @@ def test_reload_nonempty_result_w_explicit_connection(self):
         connection = _Connection(
             {'items': [{'entity': 'allUsers', 'role': ROLE}]})
         acl = self._makeOne()
-        acl._reload_path = '/testing/acl'
+        acl.reload_path = '/testing/acl'
         acl.loaded = True
         acl.reload(connection=connection)
         self.assertTrue(acl.loaded)
@@ -623,17 +601,12 @@ def test_reload_nonempty_result_w_explicit_connection(self):
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/testing/acl')
 
-    def test_save_wo_path_raises_NotImplementedError(self):
-        acl = self._makeOne()
-        acl.loaded = True
-        self.assertRaises(NotImplementedError, acl.save)
-
     def test_save_none_set_none_passed_w_implicit_connection(self):
         from gcloud.storage._testing import _monkey_defaults
         connection = _Connection()
         acl = self._makeOne()
         acl._connection = connection
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         with _monkey_defaults(connection=connection):
             acl.save()
         kw = connection._requested
@@ -642,7 +615,7 @@ def test_save_none_set_none_passed_w_implicit_connection(self):
     def test_save_none_set_none_passed_w_explicit_connection(self):
         connection = _Connection()
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.save(connection=connection)
         kw = connection._requested
         self.assertEqual(len(kw), 0)
@@ -651,7 +624,7 @@ def test_save_existing_missing_none_passed_w_implicit_connection(self):
         from gcloud.storage._testing import _monkey_defaults
         connection = _Connection({})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         with _monkey_defaults(connection=connection):
             acl.save()
@@ -666,7 +639,7 @@ def test_save_existing_missing_none_passed_w_implicit_connection(self):
     def test_save_existing_missing_none_passed_w_explicit_connection(self):
         connection = _Connection({})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.save(connection=connection)
         self.assertEqual(list(acl), [])
@@ -683,7 +656,7 @@ def test_save_no_arg_w_implicit_connection(self):
         AFTER = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'acl': AFTER})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers').grant(ROLE)
         with _monkey_defaults(connection=connection):
@@ -701,7 +674,7 @@ def test_save_no_arg_w_explicit_connection(self):
         AFTER = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'acl': AFTER})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers').grant(ROLE)
         acl.save(connection=connection)
@@ -721,7 +694,7 @@ def test_save_w_arg_w_implicit_connection(self):
         new_acl = [{'entity': 'allUsers', 'role': ROLE1}]
         connection = _Connection({'acl': [STICKY] + new_acl})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         with _monkey_defaults(connection=connection):
             acl.save(new_acl)
@@ -743,7 +716,7 @@ def test_save_w_arg_w_explicit_connection(self):
         new_acl = [{'entity': 'allUsers', 'role': ROLE1}]
         connection = _Connection({'acl': [STICKY] + new_acl})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.save(new_acl, connection)
         entries = list(acl)
@@ -764,7 +737,7 @@ def test_clear_w_implicit_connection(self):
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         connection = _Connection({'acl': [STICKY]})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers', ROLE1)
         with _monkey_defaults(connection=connection):
@@ -783,7 +756,7 @@ def test_clear_w_explicit_connection(self):
         STICKY = {'entity': 'allUsers', 'role': ROLE2}
         connection = _Connection({'acl': [STICKY]})
         acl = self._makeOne()
-        acl._save_path = '/testing'
+        acl.save_path = '/testing'
         acl.loaded = True
         acl.entity('allUsers', ROLE1)
         acl.clear(connection=connection)