Drop support for fetching ACLs through 'metadata' operations.

We have separate APIs for ACLs, and the 'full' option was needlessly
complex, maybe even expensive.

Author: tseaver

gcloud/storage/bucket.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:`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
@@ -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 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)

gcloud/storage/key.py

@@ -315,17 +315,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
@@ -346,9 +342,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)

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'

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'