Merge pull request #373 from tseaver/338-make_acl_lazy_load-remove_ge…

…t_acl

Fix #338: make 'acl' lazy load, remove 'get-acl()'

gcloud/storage/_helpers.py

@@ -138,15 +138,6 @@ def _get_property(self, field, default=None):
 
         return self.properties.get(field, default)
 
-    def get_acl(self):
-        """Get ACL as an object.
-
-        :returns: An ACL object for the current object.
-        """
-        if not self.acl.loaded:
-            self.acl.reload()
-        return self.acl
-
 
 class _PropertyBatch(object):
     """Context manager: Batch updates to object's ``_patch_properties``

gcloud/storage/acl.py

@@ -2,12 +2,12 @@
 
 :class:`gcloud.storage.bucket.Bucket` has a getting method that creates
 an ACL object under the hood, and you can interact with that using
-:func:`gcloud.storage.bucket.Bucket.get_acl`::
+:func:`gcloud.storage.bucket.Bucket.acl`::
 
   >>> from gcloud import storage
   >>> connection = storage.get_connection(project, email, key_path)
   >>> bucket = connection.get_bucket(bucket_name)
-  >>> acl = bucket.get_acl()
+  >>> acl = bucket.acl
 
 Adding and removing permissions can be done with the following methods
 (in increasing order of granularity):
@@ -172,12 +172,19 @@ class ACL(object):
     def __init__(self):
         self.entities = {}
 
+    def _ensure_loaded(self):
+        """Load if not already loaded."""
+        if not self.loaded:
+            self.reload()
+
     def reset(self):
         """Remove all entities from the ACL, and clear the ``loaded`` flag."""
         self.entities.clear()
         self.loaded = False
 
     def __iter__(self):
+        self._ensure_loaded()
+
         for entity in self.entities.itervalues():
             for role in entity.get_roles():
                 if role:
@@ -224,6 +231,7 @@ def has_entity(self, entity):
         :rtype: bool
         :returns: True of the entity exists in the ACL.
         """
+        self._ensure_loaded()
         return str(entity) in self.entities
 
     def get_entity(self, entity, default=None):
@@ -240,6 +248,7 @@ def get_entity(self, entity, default=None):
         :returns: The corresponding entity or the value provided
                   to ``default``.
         """
+        self._ensure_loaded()
         return self.entities.get(str(entity), default)
 
     def add_entity(self, entity):
@@ -248,8 +257,8 @@ def add_entity(self, entity):
         :type entity: :class:`_ACLEntity`
         :param entity: The entity to add to this ACL.
         """
+        self._ensure_loaded()
         self.entities[str(entity)] = entity
-        self.loaded = True
 
     def entity(self, entity_type, identifier=None):
         """Factory method for creating an Entity.
@@ -332,6 +341,7 @@ def get_entities(self):
         :rtype: list of :class:`_ACLEntity` objects
         :returns: A list of all Entity objects.
         """
+        self._ensure_loaded()
         return self.entities.values()
 
     def reload(self):
@@ -381,12 +391,10 @@ def reload(self):
 
         url_path = '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
         found = self.bucket.connection.api_request(method='GET', path=url_path)
+        self.loaded = True
         for entry in found['items']:
             self.add_entity(self.entity_from_dict(entry))
 
-        # Even if we fetch no entries, the ACL is still loaded.
-        self.loaded = True
-
         return self
 
     def save(self, acl=None):
@@ -407,7 +415,7 @@ def save(self, acl=None):
 
           >>> bucket1 = connection.get_bucket(bucket1_name)
           >>> bucket2 = connection.get_bucket(bucket2_name)
-          >>> bucket2.acl.save(bucket1.get_acl())
+          >>> 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
@@ -489,12 +497,10 @@ def reload(self):
 
         url_path = '%s/acl' % self.key.path
         found = self.key.connection.api_request(method='GET', path=url_path)
+        self.loaded = True
         for entry in found['items']:
             self.add_entity(self.entity_from_dict(entry))
 
-        # Even if we fetch no entries, the ACL is still loaded.
-        self.loaded = True
-
         return self
 
     def save(self, acl=None):

gcloud/storage/bucket.py

@@ -50,7 +50,7 @@ class Bucket(_PropertyMixin):
     _iterator_class = _KeyIterator
 
     CUSTOM_PROPERTY_ACCESSORS = {
-        'acl': 'get_acl()',
+        'acl': 'acl',
         'cors': 'get_cors()',
         'defaultObjectAcl': 'get_default_object_acl()',
         'etag': 'etag',
@@ -694,7 +694,7 @@ def make_public(self, recursive=False, future=False):
         :param future: If True, this will make all objects created in the
                        future public as well.
         """
-        self.get_acl().all().grant_read()
+        self.acl.all().grant_read()
         self.acl.save()
 
         if future:
@@ -704,5 +704,5 @@ def make_public(self, recursive=False, future=False):
 
         if recursive:
             for key in self:
-                key.get_acl().all().grant_read()
+                key.acl.all().grant_read()
                 key.save_acl()

gcloud/storage/key.py

@@ -16,7 +16,7 @@ class Key(_PropertyMixin):
     """A wrapper around Cloud Storage's concept of an ``Object``."""
 
     CUSTOM_PROPERTY_ACCESSORS = {
-        'acl': 'get_acl()',
+        'acl': 'acl',
         'cacheControl': 'cache_control',
         'contentDisposition': 'content_disposition',
         'contentEncoding': 'content_encoding',
@@ -383,7 +383,7 @@ def make_public(self):
 
         :returns: The current object.
         """
-        self.get_acl().all().grant_read()
+        self.acl.all().grant_read()
         self.acl.save()
         return self
 

gcloud/storage/test__helpers.py

@@ -117,25 +117,6 @@ def test__patch_properties(self):
         self.assertEqual(kw[0]['data'], {'foo': 'Foo'})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_get_acl_not_yet_loaded(self):
-        class ACL(object):
-            loaded = False
-
-            def reload(self):
-                self.loaded = True
-
-        mixin = self._makeOne()
-        acl = mixin.acl = ACL()
-        self.assertTrue(mixin.get_acl() is acl)
-        self.assertTrue(acl.loaded)
-
-    def test_get_acl_already_loaded(self):
-        class ACL(object):
-            loaded = True
-        mixin = self._makeOne()
-        acl = mixin.acl = ACL()
-        self.assertTrue(mixin.get_acl() is acl)  # no 'reload'
-
 
 class TestPropertyBatch(unittest2.TestCase):