From 7f4fe7cd785a64711cb38877e2d9b7413ad860ba Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 30 Oct 2014 18:05:09 -0400
Subject: [PATCH 01/14] Allow clearing an ACL.

---
 gcloud/storage/acl.py      | 4 ++++
 gcloud/storage/test_acl.py | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index 75e0577e9d8f..c23cf02e6564 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -168,6 +168,10 @@ class ACL(object):
     """Container class representing a list of access controls."""
 
     def __init__(self):
+        self.clear()
+
+    def clear(self):
+        """Remove all entities from the ACL."""
         self.entities = {}
 
     def __iter__(self):
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index 8a4f6e8f6da0..9515bcee892a 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -126,6 +126,15 @@ def test_ctor(self):
         self.assertEqual(acl.entities, {})
         self.assertEqual(list(acl.get_entities()), [])
 
+    def test_clear(self):
+        TYPE = 'type'
+        ID = 'id'
+        acl = self._makeOne()
+        acl.entity(TYPE, ID)
+        acl.clear()
+        self.assertEqual(acl.entities, {})
+        self.assertEqual(list(acl.get_entities()), [])
+
     def test___iter___empty(self):
         acl = self._makeOne()
         self.assertEqual(list(acl), [])

From 2d89c38853de029ba32fa45ba50a288c982249c7 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 30 Oct 2014 18:10:06 -0400
Subject: [PATCH 02/14] Add a 'loaded' flag to ACLs.

'reset()' clears it, along with entities.

'add_entity()' sets it.
---
 gcloud/storage/acl.py      |  8 ++++++++
 gcloud/storage/test_acl.py | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index c23cf02e6564..06897ab9c482 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -167,6 +167,8 @@ def revoke_owner(self):
 class ACL(object):
     """Container class representing a list of access controls."""
 
+    loaded = False
+
     def __init__(self):
         self.clear()
 
@@ -174,6 +176,11 @@ def clear(self):
         """Remove all entities from the ACL."""
         self.entities = {}
 
+    def reset(self):
+        """Remove all entities from the ACL, and clear the ``loaded`` flag."""
+        self.clear()
+        self.loaded = False
+
     def __iter__(self):
         for entity in self.entities.itervalues():
             for role in entity.get_roles():
@@ -246,6 +253,7 @@ def add_entity(self, entity):
         :param entity: The entity to add to this ACL.
         """
         self.entities[str(entity)] = entity
+        self.loaded = True
 
     def entity(self, entity_type, identifier=None):
         """Factory method for creating an Entity.
diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index 9515bcee892a..74f9cccd4ffb 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -125,6 +125,7 @@ def test_ctor(self):
         acl = self._makeOne()
         self.assertEqual(acl.entities, {})
         self.assertEqual(list(acl.get_entities()), [])
+        self.assertFalse(acl.loaded)
 
     def test_clear(self):
         TYPE = 'type'
@@ -132,6 +133,17 @@ def test_clear(self):
         acl = self._makeOne()
         acl.entity(TYPE, ID)
         acl.clear()
+        self.assertTrue(acl.loaded)
+        self.assertEqual(acl.entities, {})
+        self.assertEqual(list(acl.get_entities()), [])
+
+    def test_reset(self):
+        TYPE = 'type'
+        ID = 'id'
+        acl = self._makeOne()
+        acl.entity(TYPE, ID)
+        acl.reset()
+        self.assertFalse(acl.loaded)
         self.assertEqual(acl.entities, {})
         self.assertEqual(list(acl.get_entities()), [])
 
@@ -281,6 +293,7 @@ def test_add_entity_miss(self):
         entity.grant(ROLE)
         acl = self._makeOne()
         acl.add_entity(entity)
+        self.assertTrue(acl.loaded)
         self.assertEqual(list(acl),
                          [{'entity': 'type-id', 'role': ROLE}])
         self.assertEqual(list(acl.get_entities()), [entity])
@@ -296,6 +309,7 @@ def test_add_entity_hit(self):
         acl = self._makeOne()
         before = acl.entity(TYPE, ID)
         acl.add_entity(entity)
+        self.assertTrue(acl.loaded)
         self.assertFalse(acl.get_entity(KEY) is before)
         self.assertTrue(acl.get_entity(KEY) is entity)
         self.assertEqual(list(acl),
@@ -308,6 +322,7 @@ def test_entity_miss(self):
         ROLE = 'role'
         acl = self._makeOne()
         entity = acl.entity(TYPE, ID)
+        self.assertTrue(acl.loaded)
         entity.grant(ROLE)
         self.assertEqual(list(acl),
                          [{'entity': 'type-id', 'role': ROLE}])

From b524729e5cfcbf357b4e737a9cb2f2dbde4484d4 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 30 Oct 2014 18:12:26 -0400
Subject: [PATCH 03/14] Neaten.

---
 gcloud/storage/acl.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py
index 06897ab9c482..448ad1119c57 100644
--- a/gcloud/storage/acl.py
+++ b/gcloud/storage/acl.py
@@ -170,15 +170,15 @@ class ACL(object):
     loaded = False
 
     def __init__(self):
-        self.clear()
+        self.entities = {}
 
     def clear(self):
         """Remove all entities from the ACL."""
-        self.entities = {}
+        self.entities.clear()
 
     def reset(self):
         """Remove all entities from the ACL, and clear the ``loaded`` flag."""
-        self.clear()
+        self.entities.clear()
         self.loaded = False
 
     def __iter__(self):

From 6fe5372d9baa924fcc864152e16869450522dce3 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 30 Oct 2014 23:28:27 -0400
Subject: [PATCH 04/14] Use ACL-specific endpoints where feasible for buckets.

- Refactor lazy-handling of 'acl' and 'default_object_acl' using properties.

- Fetch 'ac' and 'defaultObjectAcl' via GET to specific endpoints.

- Continue to update 'acl' and 'defaultObjectAcl' via PATCH to the object's
  resource (we can't do incremental updates to ACEs).

Fixes #163.
---
 gcloud/storage/bucket.py      |  87 +++++++++----
 gcloud/storage/test_bucket.py | 223 ++++++++++++++++++----------------
 2 files changed, 179 insertions(+), 131 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 4d4ce3ac1962..00c8424c9567 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -19,15 +19,27 @@ class Bucket(object):
     :type name: string
     :param name: The name of the bucket.
     """
+    # ACL rules are lazily retrieved.
+    _acl = _default_object_acl = None
 
     def __init__(self, connection=None, name=None, metadata=None):
         self.connection = connection
         self.name = name
         self.metadata = metadata
 
-        # ACL rules are lazily retrieved.
-        self.acl = None
-        self.default_object_acl = None
+    @property
+    def acl(self):
+        """Create our ACL on demand."""
+        if self._acl is None:
+            self._acl = BucketACL(self)
+        return self._acl
+
+    @property
+    def default_object_acl(self):
+        """Create our defaultObjectACL on demand."""
+        if self._default_object_acl is None:
+            self._default_object_acl = DefaultObjectACL(self)
+        return self._default_object_acl
 
     @classmethod
     def from_dict(cls, bucket_dict, connection=None):
@@ -431,11 +443,15 @@ def reload_acl(self):
         :rtype: :class:`Bucket`
         :returns: The current bucket.
         """
-        self.acl = BucketACL(bucket=self)
+        self.acl.clear()
 
-        for entry in self.get_metadata('acl', []):
-            entity = self.acl.entity_from_dict(entry)
-            self.acl.add_entity(entity)
+        url_path = '%s/acl' % self.path
+        found = self.connection.api_request(method='GET', path=url_path)
+        for entry in found['items']:
+            self.acl.add_entity(self.acl.entity_from_dict(entry))
+
+        # Even if we fetch no entries, the ACL is still loaded.
+        self.acl.loaded = True
 
         return self
 
@@ -445,7 +461,7 @@ def get_acl(self):
         :rtype: :class:`gcloud.storage.acl.BucketACL`
         :returns: An ACL object for the current bucket.
         """
-        if not self.acl:
+        if not self.acl.loaded:
             self.reload_acl()
         return self.acl
 
@@ -487,12 +503,17 @@ def save_acl(self, acl=None):
         # both evaluate to False, but mean very different things.
         if acl is None:
             acl = self.acl
+            dirty = acl.loaded
+        else:
+            dirty = True
 
-        if acl is None:
-            return self
+        if dirty:
+            result = self.connection.api_request(
+                method='PATCH', path=self.path, data={'acl': list(acl)})
+            self.acl.clear()
+            for entry in result['acl']:
+                self.acl.entity(self.acl.entity_from_dict(entry))
 
-        self.patch_metadata({'acl': list(acl)})
-        self.reload_acl()
         return self
 
     def clear_acl(self):
@@ -522,7 +543,11 @@ def clear_acl(self):
 
         At this point all the custom rules you created have been removed.
         """
-        return self.save_acl(acl=[])
+        self.connection.api_request(
+            method='PATCH', path=self.path, data={'acl': []})
+        self.acl.clear()
+        self.acl.loaded = True
+        return self
 
     def reload_default_object_acl(self):
         """Reload the Default Object ACL rules for this bucket.
@@ -530,11 +555,16 @@ def reload_default_object_acl(self):
         :rtype: :class:`Bucket`
         :returns: The current bucket.
         """
-        self.default_object_acl = DefaultObjectACL(bucket=self)
+        doa = self.default_object_acl
+        doa.clear()
+
+        url_path = '%s/defaultObjectAcl' % self.path
+        found = self.connection.api_request(method='GET', path=url_path)
+        for entry in found['items']:
+            doa.add_entity(doa.entity_from_dict(entry))
 
-        for entry in self.get_metadata('defaultObjectAcl', []):
-            entity = self.default_object_acl.entity_from_dict(entry)
-            self.default_object_acl.add_entity(entity)
+        # Even if we fetch no entries, the ACL is still loaded.
+        doa.loaded = True
 
         return self
 
@@ -547,7 +577,7 @@ def get_default_object_acl(self):
         :rtype: :class:`gcloud.storage.acl.DefaultObjectACL`
         :returns: A DefaultObjectACL object for this bucket.
         """
-        if not self.default_object_acl:
+        if not self.default_object_acl.loaded:
             self.reload_default_object_acl()
         return self.default_object_acl
 
@@ -562,18 +592,29 @@ def save_default_object_acl(self, acl=None):
         """
         if acl is None:
             acl = self.default_object_acl
+            dirty = acl.loaded
+        else:
+            dirty = True
 
-        if acl is None:
-            return self
+        if dirty:
+            result = self.connection.api_request(
+                method='PATCH', path=self.path,
+                data={'defaultObjectAcl': list(acl)})
+            doa = self.default_object_acl
+            doa.clear()
+            for entry in result['defaultObjectAcl']:
+                doa.entity(doa.entity_from_dict(entry))
 
-        self.patch_metadata({'defaultObjectAcl': list(acl)})
-        self.reload_default_object_acl()
         return self
 
     def clear_default_object_acl(self):
         """Remove the Default Object ACL from this bucket."""
 
-        return self.save_default_object_acl(acl=[])
+        self.connection.api_request(
+            method='PATCH', path=self.path, data={'defaultObjectAcl': []})
+        self.default_object_acl.clear()
+        self.default_object_acl.loaded = True
+        return self
 
     def make_public(self, recursive=False, future=False):
         """Make a bucket public.
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index e7869ce0cab3..92d27cdaf9ef 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -17,8 +17,8 @@ def test_ctor_defaults(self):
         self.assertEqual(bucket.connection, None)
         self.assertEqual(bucket.name, None)
         self.assertEqual(bucket.metadata, None)
-        self.assertEqual(bucket.acl, None)
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertTrue(bucket._acl is None)
+        self.assertTrue(bucket._default_object_acl is None)
 
     def test_ctor_explicit(self):
         NAME = 'name'
@@ -28,8 +28,8 @@ def test_ctor_explicit(self):
         self.assertTrue(bucket.connection is connection)
         self.assertEqual(bucket.name, NAME)
         self.assertEqual(bucket.metadata, metadata)
-        self.assertEqual(bucket.acl, None)
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertTrue(bucket._acl is None)
+        self.assertTrue(bucket._default_object_acl is None)
 
     def test_from_dict_defaults(self):
         NAME = 'name'
@@ -39,8 +39,8 @@ def test_from_dict_defaults(self):
         self.assertEqual(bucket.connection, None)
         self.assertEqual(bucket.name, NAME)
         self.assertEqual(bucket.metadata, metadata)
-        self.assertEqual(bucket.acl, None)
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertTrue(bucket._acl is None)
+        self.assertTrue(bucket._default_object_acl is None)
 
     def test_from_dict_explicit(self):
         NAME = 'name'
@@ -51,8 +51,22 @@ def test_from_dict_explicit(self):
         self.assertTrue(bucket.connection is connection)
         self.assertEqual(bucket.name, NAME)
         self.assertEqual(bucket.metadata, metadata)
-        self.assertEqual(bucket.acl, None)
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertTrue(bucket._acl is None)
+        self.assertTrue(bucket._default_object_acl is None)
+
+    def test_acl_property(self):
+        from gcloud.storage.acl import BucketACL
+        bucket = self._makeOne()
+        acl = bucket.acl
+        self.assertTrue(isinstance(acl, BucketACL))
+        self.assertTrue(acl is bucket._acl)
+
+    def test_default_object_acl_property(self):
+        from gcloud.storage.acl import DefaultObjectACL
+        bucket = self._makeOne()
+        acl = bucket.default_object_acl
+        self.assertTrue(isinstance(acl, DefaultObjectACL))
+        self.assertTrue(acl is bucket._default_object_acl)
 
     def test___iter___empty(self):
         NAME = 'name'
@@ -551,28 +565,42 @@ def test_disable_website(self):
 
     def test_reload_acl_eager_empty(self):
         from gcloud.storage.acl import BucketACL
-        metadata = {'acl': []}
-        bucket = self._makeOne(metadata=metadata)
+        NAME = 'name'
+        ROLE = 'role'
+        connection = _Connection({'items': []})
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.entity('allUsers', ROLE)
         self.assertTrue(bucket.reload_acl() is bucket)
         self.assertTrue(isinstance(bucket.acl, BucketACL))
         self.assertEqual(list(bucket.acl), [])
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
 
     def test_reload_acl_eager_nonempty(self):
         from gcloud.storage.acl import BucketACL
+        NAME = 'name'
         ROLE = 'role'
-        metadata = {'acl': [{'entity': 'allUsers', 'role': ROLE}]}
-        bucket = self._makeOne(metadata=metadata)
+        connection = _Connection(
+            {'items': [{'entity': 'allUsers', 'role': ROLE}]})
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
         self.assertTrue(bucket.reload_acl() is bucket)
         self.assertTrue(isinstance(bucket.acl, BucketACL))
         self.assertEqual(list(bucket.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)
 
     def test_reload_acl_lazy(self):
         from gcloud.storage.acl import BucketACL
         NAME = 'name'
         ROLE = 'role'
-        after = {'acl': [{'entity': 'allUsers', 'role': ROLE}]}
-        connection = _Connection(after)
+        connection = _Connection(
+            {'items': [{'entity': 'allUsers', 'role': ROLE}]})
         bucket = self._makeOne(connection, NAME)
         self.assertTrue(bucket.reload_acl() is bucket)
         self.assertTrue(isinstance(bucket.acl, BucketACL))
@@ -581,26 +609,27 @@ def test_reload_acl_lazy(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
 
     def test_get_acl_lazy(self):
         from gcloud.storage.acl import BucketACL
-        metadata = {'acl': []}
-        connection = _Connection()
-        bucket = self._makeOne(metadata=metadata)
+        NAME = 'name'
+        connection = _Connection({'items': []})
+        bucket = self._makeOne(connection, NAME)
         acl = bucket.get_acl()
         self.assertTrue(acl is bucket.acl)
         self.assertTrue(isinstance(acl, BucketACL))
         self.assertEqual(list(bucket.acl), [])
         kw = connection._requested
-        self.assertEqual(len(kw), 0)
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/b/%s/acl' % NAME)
 
     def test_get_acl_eager(self):
-        from gcloud.storage.acl import BucketACL
         connection = _Connection()
         bucket = self._makeOne()
-        preset = bucket.acl = BucketACL(bucket)
+        preset = bucket.acl  # Ensure it is assigned
+        preset.loaded = True
         acl = bucket.get_acl()
         self.assertTrue(acl is preset)
         kw = connection._requested
@@ -616,25 +645,22 @@ def test_save_acl_none_set_none_passed(self):
     def test_save_acl_existing_set_none_passed(self):
         NAME = 'name'
         connection = _Connection({'foo': 'Foo', 'acl': []})
-        metadata = {'acl': []}
-        bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_acl()
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
         self.assertTrue(bucket.save_acl() is bucket)
         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]['data'], metadata)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'acl': []})
 
     def test_save_acl_existing_set_new_passed(self):
         NAME = 'name'
         ROLE = 'role'
         new_acl = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'foo': 'Foo', 'acl': new_acl})
-        metadata = {'acl': []}
-        bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_acl()
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
         self.assertTrue(bucket.save_acl(new_acl) is bucket)
         self.assertEqual(list(bucket.acl), new_acl)
         kw = connection._requested
@@ -642,16 +668,15 @@ def test_save_acl_existing_set_new_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_clear_acl(self):
         NAME = 'name'
         ROLE = 'role'
-        old_acl = [{'entity': 'allUsers', 'role': ROLE}]
-        connection = _Connection({'foo': 'Foo', 'acl': []})
-        metadata = {'acl': old_acl}
-        bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_acl()
+        connection = _Connection(
+            {'foo': 'Foo', 'acl': []},
+            )
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.entity('allUsers', ROLE)
         self.assertTrue(bucket.clear_acl() is bucket)
         self.assertEqual(list(bucket.acl), [])
         kw = connection._requested
@@ -659,60 +684,69 @@ def test_clear_acl(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_reload_default_object_acl_eager_empty(self):
-        from gcloud.storage.acl import BucketACL
-        metadata = {'defaultObjectAcl': []}
-        bucket = self._makeOne(metadata=metadata)
+        from gcloud.storage.acl import DefaultObjectACL
+        NAME = 'name'
+        after = {'items': []}
+        connection = _Connection(after)
+        bucket = self._makeOne(connection, NAME)
+        bucket.default_object_acl.loaded = True
         self.assertTrue(bucket.reload_default_object_acl() is bucket)
-        self.assertTrue(isinstance(bucket.default_object_acl, BucketACL))
+        self.assertTrue(
+            isinstance(bucket.default_object_acl, DefaultObjectACL))
         self.assertEqual(list(bucket.default_object_acl), [])
 
     def test_reload_default_object_acl_eager_nonempty(self):
-        from gcloud.storage.acl import BucketACL
+        from gcloud.storage.acl import DefaultObjectACL
+        NAME = 'name'
         ROLE = 'role'
-        metadata = {'defaultObjectAcl': [{'entity': 'allUsers', 'role': ROLE}]}
-        bucket = self._makeOne(metadata=metadata)
+        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
+        connection = _Connection(after)
+        bucket = self._makeOne(connection, NAME)
+        bucket.default_object_acl.entity('allUsers', 'OTHERROLE')
         self.assertTrue(bucket.reload_default_object_acl() is bucket)
-        self.assertTrue(isinstance(bucket.default_object_acl, BucketACL))
+        self.assertTrue(
+            isinstance(bucket.default_object_acl, DefaultObjectACL))
         self.assertEqual(list(bucket.default_object_acl),
                          [{'entity': 'allUsers', 'role': ROLE}])
 
     def test_reload_default_object_acl_lazy(self):
-        from gcloud.storage.acl import BucketACL
+        from gcloud.storage.acl import DefaultObjectACL
         NAME = 'name'
         ROLE = 'role'
-        after = {'defaultObjectAcl': [{'entity': 'allUsers', 'role': ROLE}]}
+        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
         connection = _Connection(after)
         bucket = self._makeOne(connection, NAME)
         self.assertTrue(bucket.reload_default_object_acl() is bucket)
-        self.assertTrue(isinstance(bucket.default_object_acl, BucketACL))
+        self.assertTrue(
+            isinstance(bucket.default_object_acl, DefaultObjectACL))
         self.assertEqual(list(bucket.default_object_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' % NAME)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['path'], '/b/%s/defaultObjectAcl' % NAME)
 
     def test_get_default_object_acl_lazy(self):
         from gcloud.storage.acl import BucketACL
-        metadata = {'defaultObjectAcl': []}
-        connection = _Connection()
-        bucket = self._makeOne(metadata=metadata)
+        NAME = 'name'
+        connection = _Connection({'items': []})
+        bucket = self._makeOne(connection, NAME)
         acl = bucket.get_default_object_acl()
         self.assertTrue(acl is bucket.default_object_acl)
         self.assertTrue(isinstance(acl, BucketACL))
         self.assertEqual(list(bucket.default_object_acl), [])
         kw = connection._requested
-        self.assertEqual(len(kw), 0)
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/b/%s/defaultObjectAcl' % NAME)
 
     def test_get_default_object_acl_eager(self):
-        from gcloud.storage.acl import BucketACL
         connection = _Connection()
         bucket = self._makeOne()
-        preset = bucket.default_object_acl = BucketACL(bucket)
+        preset = bucket.default_object_acl  # ensure it is assigned
+        preset.loaded = True
         acl = bucket.get_default_object_acl()
         self.assertTrue(acl is preset)
         kw = connection._requested
@@ -727,115 +761,90 @@ def test_save_default_object_acl_none_set_none_passed(self):
 
     def test_save_default_object_acl_existing_set_none_passed(self):
         NAME = 'name'
-        connection = _Connection({'foo': 'Foo', 'acl': []})
         connection = _Connection(
-            {'foo': 'Foo', 'acl': []},
             {'foo': 'Foo', 'acl': [], 'defaultObjectAcl': []},
         )
-        metadata = {'defaultObjectAcl': []}
-        bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_default_object_acl()
+        bucket = self._makeOne(connection, NAME)
+        bucket.default_object_acl.loaded = True
         self.assertTrue(bucket.save_default_object_acl() is bucket)
         kw = connection._requested
-        self.assertEqual(len(kw), 2)
+        self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['data'], metadata)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
-        self.assertEqual(kw[1]['method'], 'GET')
-        self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
 
     def test_save_default_object_acl_existing_set_new_passed(self):
         NAME = 'name'
         ROLE = 'role'
         new_acl = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection(
-            {'foo': 'Foo', 'acl': new_acl},
             {'foo': 'Foo', 'acl': new_acl, 'defaultObjectAcl': new_acl},
         )
         metadata = {'defaultObjectAcl': []}
         bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_default_object_acl()
+        bucket.default_object_acl.loaded = True
         self.assertTrue(bucket.save_default_object_acl(new_acl) is bucket)
         self.assertEqual(list(bucket.default_object_acl), new_acl)
         kw = connection._requested
-        self.assertEqual(len(kw), 2)
+        self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'defaultObjectAcl': new_acl})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
-        self.assertEqual(kw[1]['method'], 'GET')
-        self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
 
     def test_clear_default_object_acl(self):
         NAME = 'name'
         ROLE = 'role'
-        old_acl = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection(
-            {'foo': 'Foo', 'acl': []},
             {'foo': 'Foo', 'acl': [], 'defaultObjectAcl': []},
         )
-        metadata = {'defaultObjectAcl': old_acl}
-        bucket = self._makeOne(connection, NAME, metadata)
-        bucket.reload_default_object_acl()
+        bucket = self._makeOne(connection, NAME)
+        bucket.default_object_acl.entity('allUsers', ROLE)
         self.assertTrue(bucket.clear_default_object_acl() is bucket)
         self.assertEqual(list(bucket.default_object_acl), [])
         kw = connection._requested
-        self.assertEqual(len(kw), 2)
+        self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
-        self.assertEqual(kw[1]['method'], 'GET')
-        self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
 
     def test_make_public_defaults(self):
         from gcloud.storage.acl import _ACLEntity
         NAME = 'name'
-        before = {'acl': [], 'defaultObjectAcl': []}
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after = {'acl': permissive, 'defaultObjectAcl': []}
         connection = _Connection(after)
-        bucket = self._makeOne(connection, NAME, before)
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
         bucket.make_public()
-        self.assertEqual(bucket.metadata, after)
-        self.assertEqual(list(bucket.acl), after['acl'])
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertEqual(list(bucket.acl), permissive)
+        self.assertEqual(list(bucket.default_object_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]['data'], {'acl': after['acl']})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_make_public_w_future(self):
         from gcloud.storage.acl import _ACLEntity
         NAME = 'name'
-        before = {'acl': [], 'defaultObjectAcl': []}
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after1 = {'acl': permissive, 'defaultObjectAcl': []}
         after2 = {'acl': permissive, 'defaultObjectAcl': permissive}
         connection = _Connection(after1, after2)
-        bucket = self._makeOne(connection, NAME, before)
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
+        bucket.default_object_acl.loaded = True
         bucket.make_public(future=True)
-        self.assertEqual(bucket.metadata, after2)
-        self.assertEqual(list(bucket.acl), after2['acl'])
-        self.assertEqual(list(bucket.default_object_acl),
-                         after2['defaultObjectAcl'])
+        self.assertEqual(list(bucket.acl), permissive)
+        self.assertEqual(list(bucket.default_object_acl), permissive)
         kw = connection._requested
         self.assertEqual(len(kw), 2)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['data'], {'acl': after1['acl']})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'acl': permissive})
         self.assertEqual(kw[1]['method'], 'PATCH')
         self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[1]['data'], {'defaultObjectAcl':
-                                         after2['defaultObjectAcl']})
-        self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[1]['data'], {'defaultObjectAcl': permissive})
 
     def test_make_public_recursive(self):
         from gcloud.storage.acl import _ACLEntity
@@ -870,23 +879,21 @@ def get_items_from_response(self, response):
 
         NAME = 'name'
         KEY = 'key'
-        before = {'acl': [], 'defaultObjectAcl': []}
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after = {'acl': permissive, 'defaultObjectAcl': []}
         connection = _Connection(after, {'items': [{'name': KEY}]})
-        bucket = self._makeOne(connection, NAME, before)
+        bucket = self._makeOne(connection, NAME)
+        bucket.acl.loaded = True
         with _Monkey(MUT, _KeyIterator=_KeyIterator):
             bucket.make_public(recursive=True)
-        self.assertEqual(bucket.metadata, after)
-        self.assertEqual(list(bucket.acl), after['acl'])
-        self.assertEqual(bucket.default_object_acl, None)
+        self.assertEqual(list(bucket.acl), permissive)
+        self.assertEqual(list(bucket.default_object_acl), [])
         self.assertEqual(_saved, [(bucket, KEY, True)])
         kw = connection._requested
         self.assertEqual(len(kw), 2)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['data'], {'acl': after['acl']})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'acl': permissive})
         self.assertEqual(kw[1]['method'], 'GET')
         self.assertEqual(kw[1]['path'], '/b/%s/o' % NAME)
         self.assertEqual(kw[1]['query_params'], None)

From de2ad6631e121c8b15451a11c36ebbbea0ae492a Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Thu, 30 Oct 2014 23:52:39 -0400
Subject: [PATCH 05/14] Use ACL-specific endpoints where feasible for keys.

- Refactor lazy-handling of 'acl' using a property.

- Fetch 'acl' via GET to a specific endpoint.

- Continue to update 'acl' via PATCH to the object's resource (we can't do
  incremental updates to ACEs).

Fixes #151.
---
 gcloud/storage/key.py      | 45 ++++++++++++++------
 gcloud/storage/test_key.py | 87 +++++++++++++++++++++-----------------
 2 files changed, 80 insertions(+), 52 deletions(-)

diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index 175206227e3c..c287378f77ba 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -17,6 +17,8 @@ class Key(object):
 
     This must be a multiple of 256 KB per the API specification.
     """
+    # ACL rules are lazily retrieved.
+    _acl = None
 
     def __init__(self, bucket=None, name=None, metadata=None):
         """Key constructor.
@@ -35,8 +37,12 @@ def __init__(self, bucket=None, name=None, metadata=None):
         self.name = name
         self.metadata = metadata or {}
 
-        # Lazily get the ACL information.
-        self.acl = None
+    @property
+    def acl(self):
+        """Create our ACL on demand."""
+        if self._acl is None:
+            self._acl = ObjectACL(self)
+        return self._acl
 
     @classmethod
     def from_dict(cls, key_dict, bucket=None):
@@ -382,11 +388,15 @@ def reload_acl(self):
         :rtype: :class:`Key`
         :returns: The current key.
         """
-        self.acl = ObjectACL(key=self)
+        self.acl.clear()
+
+        url_path = '%s/acl' % self.path
+        found = self.connection.api_request(method='GET', path=url_path)
+        for entry in found['items']:
+            self.acl.add_entity(self.acl.entity_from_dict(entry))
 
-        for entry in self.get_metadata('acl', []):
-            entity = self.acl.entity_from_dict(entry)
-            self.acl.add_entity(entity)
+        # Even if we fetch no entries, the ACL is still loaded.
+        self.acl.loaded = True
 
         return self
 
@@ -396,7 +406,7 @@ def get_acl(self):
         :rtype: :class:`gcloud.storage.acl.ObjectACL`
         :returns: An ACL object for the current key.
         """
-        if not self.acl:
+        if not self.acl.loaded:
             self.reload_acl()
         return self.acl
 
@@ -407,16 +417,19 @@ def save_acl(self, acl=None):
         :param acl: The ACL object to save.  If left blank, this will
                     save the ACL set locally on the key.
         """
-        # We do things in this weird way because [] and None
-        # both evaluate to False, but mean very different things.
         if acl is None:
             acl = self.acl
+            dirty = acl.loaded
+        else:
+            dirty = True
 
-        if acl is None:
-            return self
+        if dirty:
+            result = self.connection.api_request(
+                method='PATCH', path=self.path, data={'acl': list(acl)})
+            self.acl.clear()
+            for entry in result['acl']:
+                self.acl.entity(self.acl.entity_from_dict(entry))
 
-        self.patch_metadata({'acl': list(acl)})
-        self.reload_acl()
         return self
 
     def clear_acl(self):
@@ -427,7 +440,11 @@ def clear_acl(self):
         have access to a key that you created even after you clear ACL
         rules with this method.
         """
-        return self.save_acl(acl=[])
+        self.connection.api_request(
+            method='PATCH', path=self.path, data={'acl': []})
+        self.acl.clear()
+        self.acl.loaded = True
+        return self
 
     def make_public(self):
         """Make this key public giving all users read access.
diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py
index 3a2ac3891ae0..571d277aeac2 100644
--- a/gcloud/storage/test_key.py
+++ b/gcloud/storage/test_key.py
@@ -16,7 +16,7 @@ def test_ctor_defaults(self):
         self.assertEqual(key.connection, None)
         self.assertEqual(key.name, None)
         self.assertEqual(key.metadata, {})
-        self.assertEqual(key.acl, None)
+        self.assertTrue(key._acl is None)
 
     def test_ctor_explicit(self):
         KEY = 'key'
@@ -28,7 +28,7 @@ def test_ctor_explicit(self):
         self.assertTrue(key.connection is connection)
         self.assertEqual(key.name, KEY)
         self.assertEqual(key.metadata, metadata)
-        self.assertEqual(key.acl, None)
+        self.assertTrue(key._acl is None)
 
     def test_from_dict_defaults(self):
         KEY = 'key'
@@ -39,7 +39,7 @@ def test_from_dict_defaults(self):
         self.assertEqual(key.connection, None)
         self.assertEqual(key.name, KEY)
         self.assertEqual(key.metadata, metadata)
-        self.assertEqual(key.acl, None)
+        self.assertTrue(key._acl is None)
 
     def test_from_dict_explicit(self):
         KEY = 'key'
@@ -52,7 +52,14 @@ def test_from_dict_explicit(self):
         self.assertTrue(key.connection is connection)
         self.assertEqual(key.name, KEY)
         self.assertEqual(key.metadata, metadata)
-        self.assertEqual(key.acl, None)
+        self.assertTrue(key._acl is None)
+
+    def test_acl_property(self):
+        from gcloud.storage.acl import ObjectACL
+        key = self._makeOne()
+        acl = key.acl
+        self.assertTrue(isinstance(acl, ObjectACL))
+        self.assertTrue(acl is key._acl)
 
     def test_path_no_bucket(self):
         key = self._makeOne()
@@ -461,27 +468,39 @@ def test_patch_metadata(self):
 
     def test_reload_acl_eager_empty(self):
         from gcloud.storage.acl import ObjectACL
-        metadata = {'acl': []}
-        key = self._makeOne(metadata=metadata)
+        KEY = 'key'
+        ROLE = 'role'
+        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
+        connection = _Connection(after)
+        bucket = _Bucket(connection)
+        key = self._makeOne(bucket, KEY)
+        key.acl.loaded = True
         self.assertTrue(key.reload_acl() is key)
         self.assertTrue(isinstance(key.acl, ObjectACL))
-        self.assertEqual(list(key.acl), [])
+        self.assertEqual(list(key.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' % KEY)
 
     def test_reload_acl_eager_nonempty(self):
         from gcloud.storage.acl import ObjectACL
+        KEY = 'key'
         ROLE = 'role'
-        metadata = {'acl': [{'entity': 'allUsers', 'role': ROLE}]}
-        key = self._makeOne(metadata=metadata)
+        after = {'items': []}
+        connection = _Connection(after)
+        bucket = _Bucket(connection)
+        key = self._makeOne(bucket, KEY)
+        key.acl.entity('allUsers', ROLE)
         self.assertTrue(key.reload_acl() is key)
         self.assertTrue(isinstance(key.acl, ObjectACL))
-        self.assertEqual(list(key.acl),
-                         [{'entity': 'allUsers', 'role': ROLE}])
+        self.assertEqual(list(key.acl), [])
 
     def test_reload_acl_lazy(self):
         from gcloud.storage.acl import ObjectACL
         KEY = 'key'
         ROLE = 'role'
-        after = {'acl': [{'entity': 'allUsers', 'role': ROLE}]}
+        after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
         connection = _Connection(after)
         bucket = _Bucket(connection)
         key = self._makeOne(bucket, KEY)
@@ -492,22 +511,23 @@ def test_reload_acl_lazy(self):
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['path'], '/b/name/o/%s/acl' % KEY)
 
     def test_get_acl_lazy(self):
         from gcloud.storage.acl import ObjectACL
-        metadata = {'acl': []}
-        key = self._makeOne(metadata=metadata)
+        KEY = 'key'
+        connection = _Connection({'items': []})
+        bucket = _Bucket(connection)
+        key = self._makeOne(bucket, KEY)
         acl = key.get_acl()
         self.assertTrue(acl is key.acl)
         self.assertTrue(isinstance(acl, ObjectACL))
         self.assertEqual(list(key.acl), [])
 
     def test_get_acl_eager(self):
-        from gcloud.storage.acl import ObjectACL
         key = self._makeOne()
-        preset = key.acl = ObjectACL(key)
+        preset = key.acl
+        preset.loaded = True
         acl = key.get_acl()
         self.assertTrue(acl is preset)
 
@@ -524,16 +544,14 @@ def test_save_acl_existing_set_none_passed(self):
         KEY = 'key'
         connection = _Connection({'foo': 'Foo', 'acl': []})
         bucket = _Bucket(connection)
-        metadata = {'acl': []}
-        key = self._makeOne(bucket, KEY, metadata)
-        key.reload_acl()
+        key = self._makeOne(bucket, KEY)
+        key.acl.loaded = True
         self.assertTrue(key.save_acl() is key)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['data'], metadata)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'acl': []})
 
     def test_save_acl_existing_set_new_passed(self):
         KEY = 'key'
@@ -541,9 +559,8 @@ def test_save_acl_existing_set_new_passed(self):
         new_acl = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'foo': 'Foo', 'acl': new_acl})
         bucket = _Bucket(connection)
-        metadata = {'acl': []}
-        key = self._makeOne(bucket, KEY, metadata)
-        key.reload_acl()
+        key = self._makeOne(bucket, KEY)
+        key.acl.entity('allUsers', 'other-role')
         self.assertTrue(key.save_acl(new_acl) is key)
         self.assertEqual(list(key.acl), new_acl)
         kw = connection._requested
@@ -551,17 +568,14 @@ def test_save_acl_existing_set_new_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_clear_acl(self):
         KEY = 'key'
         ROLE = 'role'
-        old_acl = [{'entity': 'allUsers', 'role': ROLE}]
         connection = _Connection({'foo': 'Foo', 'acl': []})
         bucket = _Bucket(connection)
-        metadata = {'acl': old_acl}
-        key = self._makeOne(bucket, KEY, metadata)
-        key.reload_acl()
+        key = self._makeOne(bucket, KEY)
+        key.acl.entity('allUsers', ROLE)
         self.assertTrue(key.clear_acl() is key)
         self.assertEqual(list(key.acl), [])
         kw = connection._requested
@@ -569,26 +583,23 @@ def test_clear_acl(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': []})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_make_public(self):
         from gcloud.storage.acl import _ACLEntity
         KEY = 'key'
-        before = {'acl': []}
         permissive = [{'entity': 'allUsers', 'role': _ACLEntity.READER_ROLE}]
         after = {'acl': permissive}
         connection = _Connection(after)
         bucket = _Bucket(connection)
-        key = self._makeOne(bucket, KEY, before)
+        key = self._makeOne(bucket, KEY)
+        key.acl.loaded = True
         key.make_public()
-        self.assertEqual(key.metadata, after)
-        self.assertEqual(list(key.acl), after['acl'])
+        self.assertEqual(list(key.acl), permissive)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['data'], {'acl': after['acl']})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(kw[0]['data'], {'acl': permissive})
 
 
 class Test__KeyIterator(unittest2.TestCase):

From b7d2feb49894ff240f50763343957650cf9d9b6b Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 11:15:38 -0400
Subject: [PATCH 06/14] Fix save/clear ACL operations.

Borked by dropping the 'projection' qs param to PATCH.  Docs spell
a default, but then note that overriding it w/ 'full' is required
(and we need 'full' anyway, because we expect to read the resulting
ACL back).
---
 gcloud/storage/bucket.py      | 12 ++++++++----
 gcloud/storage/key.py         |  6 ++++--
 gcloud/storage/test_bucket.py | 10 ++++++++++
 gcloud/storage/test_key.py    |  4 ++++
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 00c8424c9567..22b973a643c0 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -509,7 +509,8 @@ def save_acl(self, acl=None):
 
         if dirty:
             result = self.connection.api_request(
-                method='PATCH', path=self.path, data={'acl': list(acl)})
+                method='PATCH', path=self.path, data={'acl': list(acl)},
+                query_params={'projection': 'full'})
             self.acl.clear()
             for entry in result['acl']:
                 self.acl.entity(self.acl.entity_from_dict(entry))
@@ -544,7 +545,8 @@ def clear_acl(self):
         At this point all the custom rules you created have been removed.
         """
         self.connection.api_request(
-            method='PATCH', path=self.path, data={'acl': []})
+            method='PATCH', path=self.path, data={'acl': []},
+            query_params={'projection': 'full'})
         self.acl.clear()
         self.acl.loaded = True
         return self
@@ -599,7 +601,8 @@ def save_default_object_acl(self, acl=None):
         if dirty:
             result = self.connection.api_request(
                 method='PATCH', path=self.path,
-                data={'defaultObjectAcl': list(acl)})
+                data={'defaultObjectAcl': list(acl)},
+                query_params={'projection': 'full'})
             doa = self.default_object_acl
             doa.clear()
             for entry in result['defaultObjectAcl']:
@@ -611,7 +614,8 @@ def clear_default_object_acl(self):
         """Remove the Default Object ACL from this bucket."""
 
         self.connection.api_request(
-            method='PATCH', path=self.path, data={'defaultObjectAcl': []})
+            method='PATCH', path=self.path, data={'defaultObjectAcl': []},
+            query_params={'projection': 'full'})
         self.default_object_acl.clear()
         self.default_object_acl.loaded = True
         return self
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index c287378f77ba..b0fb372503f2 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -425,7 +425,8 @@ def save_acl(self, acl=None):
 
         if dirty:
             result = self.connection.api_request(
-                method='PATCH', path=self.path, data={'acl': list(acl)})
+                method='PATCH', path=self.path, data={'acl': list(acl)},
+                query_params={'projection': 'full'})
             self.acl.clear()
             for entry in result['acl']:
                 self.acl.entity(self.acl.entity_from_dict(entry))
@@ -441,7 +442,8 @@ def clear_acl(self):
         rules with this method.
         """
         self.connection.api_request(
-            method='PATCH', path=self.path, data={'acl': []})
+            method='PATCH', path=self.path, data={'acl': []},
+            query_params={'projection': 'full'})
         self.acl.clear()
         self.acl.loaded = True
         return self
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index 92d27cdaf9ef..d2753324130c 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -653,6 +653,7 @@ def test_save_acl_existing_set_none_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_save_acl_existing_set_new_passed(self):
         NAME = 'name'
@@ -668,6 +669,7 @@ def test_save_acl_existing_set_new_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_clear_acl(self):
         NAME = 'name'
@@ -684,6 +686,7 @@ def test_clear_acl(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_reload_default_object_acl_eager_empty(self):
         from gcloud.storage.acl import DefaultObjectACL
@@ -772,6 +775,7 @@ def test_save_default_object_acl_existing_set_none_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_save_default_object_acl_existing_set_new_passed(self):
         NAME = 'name'
@@ -790,6 +794,7 @@ def test_save_default_object_acl_existing_set_new_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'defaultObjectAcl': new_acl})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_clear_default_object_acl(self):
         NAME = 'name'
@@ -806,6 +811,7 @@ def test_clear_default_object_acl(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_make_public_defaults(self):
         from gcloud.storage.acl import _ACLEntity
@@ -823,6 +829,7 @@ def test_make_public_defaults(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': after['acl']})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_make_public_w_future(self):
         from gcloud.storage.acl import _ACLEntity
@@ -842,9 +849,11 @@ def test_make_public_w_future(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': permissive})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
         self.assertEqual(kw[1]['method'], 'PATCH')
         self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[1]['data'], {'defaultObjectAcl': permissive})
+        self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
 
     def test_make_public_recursive(self):
         from gcloud.storage.acl import _ACLEntity
@@ -894,6 +903,7 @@ def get_items_from_response(self, response):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['data'], {'acl': permissive})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
         self.assertEqual(kw[1]['method'], 'GET')
         self.assertEqual(kw[1]['path'], '/b/%s/o' % NAME)
         self.assertEqual(kw[1]['query_params'], None)
diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py
index 571d277aeac2..821e6a310532 100644
--- a/gcloud/storage/test_key.py
+++ b/gcloud/storage/test_key.py
@@ -552,6 +552,7 @@ def test_save_acl_existing_set_none_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_save_acl_existing_set_new_passed(self):
         KEY = 'key'
@@ -568,6 +569,7 @@ def test_save_acl_existing_set_new_passed(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': new_acl})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_clear_acl(self):
         KEY = 'key'
@@ -583,6 +585,7 @@ def test_clear_acl(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': []})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_make_public(self):
         from gcloud.storage.acl import _ACLEntity
@@ -600,6 +603,7 @@ def test_make_public(self):
         self.assertEqual(kw[0]['method'], 'PATCH')
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['data'], {'acl': permissive})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
 
 class Test__KeyIterator(unittest2.TestCase):

From cb234eeee5ae1dc00a863f485ac41e9b5b08566a Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 11:22:38 -0400
Subject: [PATCH 07/14] Accomodate 'stick' ACL entries when clearing bucket's
 ACL.

Back-end preserves some of them even after a successful PATCH with
data .
---
 gcloud/storage/bucket.py      |  6 +++++-
 gcloud/storage/test_bucket.py | 11 +++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 22b973a643c0..b6d5ce20194c 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -544,10 +544,14 @@ def clear_acl(self):
 
         At this point all the custom rules you created have been removed.
         """
-        self.connection.api_request(
+        found = self.connection.api_request(
             method='PATCH', path=self.path, data={'acl': []},
             query_params={'projection': 'full'})
         self.acl.clear()
+        # NOTE:  back-end makes some ACL entries sticky (they remain even
+        #        after the PATCH succeeds.
+        for entry in found['acl']:
+            self.acl.add_entity(self.acl.entity_from_dict(entry))
         self.acl.loaded = True
         return self
 
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index d2753324130c..00cd01b9de9b 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -673,14 +673,17 @@ def test_save_acl_existing_set_new_passed(self):
 
     def test_clear_acl(self):
         NAME = 'name'
-        ROLE = 'role'
+        ROLE1 = 'role1'
+        ROLE2 = 'role2'
+        STICKY = {'entity': 'allUsers', 'role': ROLE2}
         connection = _Connection(
-            {'foo': 'Foo', 'acl': []},
+            # Emulate back-end, which makes some entries "sticky".
+            {'foo': 'Foo', 'acl': [STICKY]},
             )
         bucket = self._makeOne(connection, NAME)
-        bucket.acl.entity('allUsers', ROLE)
+        bucket.acl.entity('allUsers', ROLE1)
         self.assertTrue(bucket.clear_acl() is bucket)
-        self.assertEqual(list(bucket.acl), [])
+        self.assertEqual(list(bucket.acl), [STICKY])
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')

From 907171062503c8b6c567d76449009c5274c10d74 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 11:42:22 -0400
Subject: [PATCH 08/14] Drop support for fetching ACLs through 'metadata'
 operations.

We have separate APIs for ACLs, and the 'full' option was needlessly
complex, maybe even expensive.
---
 gcloud/storage/bucket.py      | 14 ++++-----
 gcloud/storage/key.py         | 14 ++++-----
 gcloud/storage/test_bucket.py | 58 ++++++++++++++++-------------------
 gcloud/storage/test_key.py    | 39 +++++------------------
 4 files changed, 45 insertions(+), 80 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index b6d5ce20194c..57d587faa07a 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -325,17 +325,13 @@ def has_metadata(self, field=None):
         else:
             return True
 
-    def reload_metadata(self, full=False):
+    def reload_metadata(self):
         """Reload metadata from Cloud Storage.
 
-        :type full: bool
-        :param full: If True, loads all data (include ACL data).
-
         :rtype: :class:`Bucket`
         :returns: The bucket you just reloaded data for.
         """
-        projection = 'full' if full else 'noAcl'
-        query_params = {'projection': projection}
+        query_params = {'projection': 'noAcl'}
         self.metadata = self.connection.api_request(
             method='GET', path=self.path, query_params=query_params)
         return self
@@ -356,9 +352,11 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
+        if field in ('acl', 'defaultObjectAcl'):
+            return default
+
         if not self.has_metadata(field=field):
-            full = (field and field in ('acl', 'defaultObjectAcl'))
-            self.reload_metadata(full=full)
+            self.reload_metadata()
 
         if field:
             return self.metadata.get(field, default)
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index b0fb372503f2..0ee9f71c9d84 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -322,17 +322,13 @@ def has_metadata(self, field=None):
         else:
             return True
 
-    def reload_metadata(self, full=False):
+    def reload_metadata(self):
         """Reload metadata from Cloud Storage.
 
-        :type full: bool
-        :param full: If True, loads all data (include ACL data).
-
         :rtype: :class:`Key`
         :returns: The key you just reloaded data for.
         """
-        projection = 'full' if full else 'noAcl'
-        query_params = {'projection': projection}
+        query_params = {'projection': 'noAcl'}
         self.metadata = self.connection.api_request(
             method='GET', path=self.path, query_params=query_params)
         return self
@@ -353,9 +349,11 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
+        if field == 'acl':
+            return default
+
         if not self.has_metadata(field=field):
-            full = (field and field == 'acl')
-            self.reload_metadata(full=full)
+            self.reload_metadata()
 
         if field:
             return self.metadata.get(field, default)
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index 00cd01b9de9b..3046a54644b1 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -409,7 +409,7 @@ def test_has_metdata_hit(self):
         bucket = self._makeOne(metadata=metadata)
         self.assertTrue(bucket.has_metadata(KEY))
 
-    def test_reload_metadata_default(self):
+    def test_reload_metadata(self):
         NAME = 'name'
         before = {'foo': 'Foo'}
         after = {'bar': 'Bar'}
@@ -424,21 +424,6 @@ def test_reload_metadata_default(self):
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
 
-    def test_reload_metadata_explicit(self):
-        NAME = 'name'
-        before = {'foo': 'Foo'}
-        after = {'bar': 'Bar'}
-        connection = _Connection(after)
-        bucket = self._makeOne(connection, NAME, before)
-        found = bucket.reload_metadata(True)
-        self.assertTrue(found is bucket)
-        self.assertEqual(found.metadata, after)
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
-
     def test_get_metadata_none_set_none_passed(self):
         NAME = 'name'
         after = {'bar': 'Bar'}
@@ -453,34 +438,43 @@ def test_get_metadata_none_set_none_passed(self):
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
 
-    def test_get_metadata_none_set_acl_hit(self):
+    def test_get_metadata_acl_no_default(self):
         NAME = 'name'
-        after = {'bar': 'Bar', 'acl': []}
-        connection = _Connection(after)
+        connection = _Connection()
         bucket = self._makeOne(connection, NAME)
         found = bucket.get_metadata('acl')
-        self.assertEqual(found, [])
-        self.assertEqual(bucket.metadata, after)
+        self.assertEqual(found, None)
         kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(len(kw), 0)
+
+    def test_get_metadata_acl_w_default(self):
+        NAME = 'name'
+        connection = _Connection()
+        bucket = self._makeOne(connection, NAME)
+        default = object()
+        found = bucket.get_metadata('acl', default)
+        self.assertTrue(found is default)
+        kw = connection._requested
+        self.assertEqual(len(kw), 0)
+
+    def test_get_metadata_defaultObjectAcl_no_default(self):
+        NAME = 'name'
+        connection = _Connection()
+        bucket = self._makeOne(connection, NAME)
+        found = bucket.get_metadata('defaultObjectAcl')
+        self.assertEqual(found, None)
+        kw = connection._requested
+        self.assertEqual(len(kw), 0)
 
     def test_get_metadata_none_set_defaultObjectAcl_miss_clear_default(self):
         NAME = 'name'
-        after = {'bar': 'Bar'}
-        connection = _Connection(after)
+        connection = _Connection()
         bucket = self._makeOne(connection, NAME)
         default = object()
         found = bucket.get_metadata('defaultObjectAcl', default)
         self.assertTrue(found is default)
-        self.assertEqual(bucket.metadata, after)
         kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(len(kw), 0)
 
     def test_get_metadata_miss(self):
         NAME = 'name'
diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py
index 821e6a310532..ddefd2a8a7cd 100644
--- a/gcloud/storage/test_key.py
+++ b/gcloud/storage/test_key.py
@@ -349,7 +349,7 @@ def test_has_metdata_hit(self):
         key = self._makeOne(metadata=metadata)
         self.assertTrue(key.has_metadata(KEY))
 
-    def test_reload_metadata_default(self):
+    def test_reload_metadata(self):
         KEY = 'key'
         before = {'foo': 'Foo'}
         after = {'bar': 'Bar'}
@@ -365,22 +365,6 @@ def test_reload_metadata_default(self):
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
 
-    def test_reload_metadata_explicit(self):
-        KEY = 'key'
-        before = {'foo': 'Foo'}
-        after = {'bar': 'Bar'}
-        connection = _Connection(after)
-        bucket = _Bucket(connection)
-        key = self._makeOne(bucket, KEY, before)
-        found = key.reload_metadata(True)
-        self.assertTrue(found is key)
-        self.assertEqual(found.metadata, after)
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
-
     def test_get_metadata_none_set_none_passed(self):
         KEY = 'key'
         after = {'bar': 'Bar'}
@@ -396,22 +380,17 @@ def test_get_metadata_none_set_none_passed(self):
         self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
         self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
 
-    def test_get_metadata_none_set_acl_hit(self):
+    def test_get_metadata_acl_no_default(self):
         KEY = 'key'
-        after = {'bar': 'Bar', 'acl': []}
-        connection = _Connection(after)
+        connection = _Connection()
         bucket = _Bucket(connection)
         key = self._makeOne(bucket, KEY)
         found = key.get_metadata('acl')
-        self.assertEqual(found, [])
-        self.assertEqual(key.metadata, after)
+        self.assertEqual(found, None)
         kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(len(kw), 0)
 
-    def test_get_metadata_none_set_acl_miss_explicit_default(self):
+    def test_get_metadata_acl_w_default(self):
         KEY = 'key'
         after = {'bar': 'Bar'}
         connection = _Connection(after)
@@ -420,12 +399,8 @@ def test_get_metadata_none_set_acl_miss_explicit_default(self):
         default = object()
         found = key.get_metadata('acl', default)
         self.assertTrue(found is default)
-        self.assertEqual(key.metadata, after)
         kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(len(kw), 0)
 
     def test_get_metadata_miss(self):
         KEY = 'key'

From 4e5af92598711651cf8c3802d0a72e62494b7934 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 13:02:05 -0400
Subject: [PATCH 09/14] Add comments pointing to specific ACL-related methods.

Incorporates feedback from @dhermes.
---
 gcloud/storage/bucket.py | 2 ++
 gcloud/storage/key.py    | 1 +
 2 files changed, 3 insertions(+)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 57d587faa07a..d04abdb48da6 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -352,6 +352,8 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
+        # Use 'get_acl()' to retrieve the 'acl', and 'get_default_object_acl()'
+        # to retrieve the 'defaultObjectAcl'.
         if field in ('acl', 'defaultObjectAcl'):
             return default
 
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index 0ee9f71c9d84..c331f604b507 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -349,6 +349,7 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
+        # Use 'get_acl()' to retrieve the 'acl'.
         if field == 'acl':
             return default
 

From 9bbf6085d933a7521954d2a176dd7ddee1ff04ae Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 13:37:20 -0400
Subject: [PATCH 10/14] Ensure bucket's 'acl' is marked as loaded after
 'save_acl'.

Incorporates feedback from @dhermes.
---
 gcloud/storage/bucket.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index d04abdb48da6..faa7017cc752 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -514,6 +514,7 @@ def save_acl(self, acl=None):
             self.acl.clear()
             for entry in result['acl']:
                 self.acl.entity(self.acl.entity_from_dict(entry))
+            self.acl.loaded = True
 
         return self
 

From 6034e06c6e2fa346ff972d4e403ef8845542595b Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 13:44:24 -0400
Subject: [PATCH 11/14] Restore simplified 'clear_acl' after re-convergence.

Also 'clear_default_object_acl' for bucket.
---
 gcloud/storage/bucket.py | 18 +++---------------
 gcloud/storage/key.py    |  8 ++------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index faa7017cc752..091c0f97c1c7 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -545,16 +545,9 @@ def clear_acl(self):
 
         At this point all the custom rules you created have been removed.
         """
-        found = self.connection.api_request(
-            method='PATCH', path=self.path, data={'acl': []},
-            query_params={'projection': 'full'})
-        self.acl.clear()
         # NOTE:  back-end makes some ACL entries sticky (they remain even
         #        after the PATCH succeeds.
-        for entry in found['acl']:
-            self.acl.add_entity(self.acl.entity_from_dict(entry))
-        self.acl.loaded = True
-        return self
+        return self.save_acl([])
 
     def reload_default_object_acl(self):
         """Reload the Default Object ACL rules for this bucket.
@@ -612,18 +605,13 @@ def save_default_object_acl(self, acl=None):
             doa.clear()
             for entry in result['defaultObjectAcl']:
                 doa.entity(doa.entity_from_dict(entry))
+            doa.loaded = True
 
         return self
 
     def clear_default_object_acl(self):
         """Remove the Default Object ACL from this bucket."""
-
-        self.connection.api_request(
-            method='PATCH', path=self.path, data={'defaultObjectAcl': []},
-            query_params={'projection': 'full'})
-        self.default_object_acl.clear()
-        self.default_object_acl.loaded = True
-        return self
+        return self.save_default_object_acl([])
 
     def make_public(self, recursive=False, future=False):
         """Make a bucket public.
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index c331f604b507..33b72c36bd1e 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -429,6 +429,7 @@ def save_acl(self, acl=None):
             self.acl.clear()
             for entry in result['acl']:
                 self.acl.entity(self.acl.entity_from_dict(entry))
+            self.acl.loaded = True
 
         return self
 
@@ -440,12 +441,7 @@ def clear_acl(self):
         have access to a key that you created even after you clear ACL
         rules with this method.
         """
-        self.connection.api_request(
-            method='PATCH', path=self.path, data={'acl': []},
-            query_params={'projection': 'full'})
-        self.acl.clear()
-        self.acl.loaded = True
-        return self
+        return self.save_acl([])
 
     def make_public(self):
         """Make this key public giving all users read access.

From fe3edca02237d138a6740c8b7efa922bf55ee965 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 13:54:18 -0400
Subject: [PATCH 12/14] Make 'get_metadata' comments more developer-centric.

---
 gcloud/storage/bucket.py | 4 ++--
 gcloud/storage/key.py    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 091c0f97c1c7..39edab2adf82 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -352,8 +352,8 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
-        # Use 'get_acl()' to retrieve the 'acl', and 'get_default_object_acl()'
-        # to retrieve the 'defaultObjectAcl'.
+        # We ignore 'acl' / 'defaultObjectAcl' because they should be
+        # handled via 'get_acl()' / 'get_default_object_acl()'
         if field in ('acl', 'defaultObjectAcl'):
             return default
 
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index 33b72c36bd1e..2c2c9ab5cac1 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -349,7 +349,7 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
-        # Use 'get_acl()' to retrieve the 'acl'.
+        # We ignore 'acl' because it is meant to be handled via 'get_acl()'.
         if field == 'acl':
             return default
 

From 081689bf94de083324a1fceb19048c7781e1eb31 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 14:12:19 -0400
Subject: [PATCH 13/14] Clarify why 'reload_metadata' uses 'projection=noAcl'.

---
 gcloud/storage/bucket.py | 2 ++
 gcloud/storage/key.py    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 39edab2adf82..8346135766de 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -331,6 +331,8 @@ def reload_metadata(self):
         :rtype: :class:`Bucket`
         :returns: The bucket you just reloaded data for.
         """
+        # Pass only '?projection=noAcl' here because 'acl'/'defaultObjectAcl'
+        # are handled via 'get_acl()'/'get_default_object_acl()'
         query_params = {'projection': 'noAcl'}
         self.metadata = self.connection.api_request(
             method='GET', path=self.path, query_params=query_params)
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index 2c2c9ab5cac1..c51214d4b50d 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -328,6 +328,8 @@ def reload_metadata(self):
         :rtype: :class:`Key`
         :returns: The key you just reloaded data for.
         """
+        # Pass only '?projection=noAcl' here because 'acl' is handled via
+        # 'get_acl().
         query_params = {'projection': 'noAcl'}
         self.metadata = self.connection.api_request(
             method='GET', path=self.path, query_params=query_params)

From dcf7d276246ebeb083568303b28134356d93f8e6 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Fri, 31 Oct 2014 21:47:42 -0400
Subject: [PATCH 14/14] Raise KeyError from 'get_metadata' for ACL-related
 fields.

Error message names appropriate ACL-specific API for the field.
---
 gcloud/storage/bucket.py      |  9 +++++----
 gcloud/storage/key.py         |  2 +-
 gcloud/storage/test_bucket.py | 13 +++++--------
 gcloud/storage/test_key.py    |  6 ++----
 4 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index 8346135766de..7e7bd9640049 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -354,10 +354,11 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
         """
-        # We ignore 'acl' / 'defaultObjectAcl' because they should be
-        # handled via 'get_acl()' / 'get_default_object_acl()'
-        if field in ('acl', 'defaultObjectAcl'):
-            return default
+        if field == 'acl':
+            raise KeyError("Use 'get_acl()'")
+
+        if field == 'defaultObjectAcl':
+            raise KeyError("Use 'get_default_object_acl()'")
 
         if not self.has_metadata(field=field):
             self.reload_metadata()
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index c51214d4b50d..217f98ee29e3 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -353,7 +353,7 @@ def get_metadata(self, field=None, default=None):
         """
         # We ignore 'acl' because it is meant to be handled via 'get_acl()'.
         if field == 'acl':
-            return default
+            raise KeyError("Use 'get_acl()'")
 
         if not self.has_metadata(field=field):
             self.reload_metadata()
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index 3046a54644b1..6177de01b8ef 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -442,8 +442,7 @@ def test_get_metadata_acl_no_default(self):
         NAME = 'name'
         connection = _Connection()
         bucket = self._makeOne(connection, NAME)
-        found = bucket.get_metadata('acl')
-        self.assertEqual(found, None)
+        self.assertRaises(KeyError, bucket.get_metadata, 'acl')
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
@@ -452,8 +451,7 @@ def test_get_metadata_acl_w_default(self):
         connection = _Connection()
         bucket = self._makeOne(connection, NAME)
         default = object()
-        found = bucket.get_metadata('acl', default)
-        self.assertTrue(found is default)
+        self.assertRaises(KeyError, bucket.get_metadata, 'acl', default)
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
@@ -461,8 +459,7 @@ def test_get_metadata_defaultObjectAcl_no_default(self):
         NAME = 'name'
         connection = _Connection()
         bucket = self._makeOne(connection, NAME)
-        found = bucket.get_metadata('defaultObjectAcl')
-        self.assertEqual(found, None)
+        self.assertRaises(KeyError, bucket.get_metadata, 'defaultObjectAcl')
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
@@ -471,8 +468,8 @@ def test_get_metadata_none_set_defaultObjectAcl_miss_clear_default(self):
         connection = _Connection()
         bucket = self._makeOne(connection, NAME)
         default = object()
-        found = bucket.get_metadata('defaultObjectAcl', default)
-        self.assertTrue(found is default)
+        self.assertRaises(KeyError, bucket.get_metadata, 'defaultObjectAcl',
+                          default)
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py
index ddefd2a8a7cd..80ec615c3c5a 100644
--- a/gcloud/storage/test_key.py
+++ b/gcloud/storage/test_key.py
@@ -385,8 +385,7 @@ def test_get_metadata_acl_no_default(self):
         connection = _Connection()
         bucket = _Bucket(connection)
         key = self._makeOne(bucket, KEY)
-        found = key.get_metadata('acl')
-        self.assertEqual(found, None)
+        self.assertRaises(KeyError, key.get_metadata, 'acl')
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
@@ -397,8 +396,7 @@ def test_get_metadata_acl_w_default(self):
         bucket = _Bucket(connection)
         key = self._makeOne(bucket, KEY)
         default = object()
-        found = key.get_metadata('acl', default)
-        self.assertTrue(found is default)
+        self.assertRaises(KeyError, key.get_metadata, 'acl', default)
         kw = connection._requested
         self.assertEqual(len(kw), 0)