Skip to content

Commit

Permalink
Fix save/clear ACL operations.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
tseaver committed Oct 31, 2014
1 parent e69c8fc commit e74972d
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 6 deletions.
12 changes: 8 additions & 4 deletions gcloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,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))
Expand Down Expand Up @@ -541,7 +542,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
Expand Down Expand Up @@ -596,7 +598,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']:
Expand All @@ -608,7 +611,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
Expand Down
6 changes: 4 additions & 2 deletions gcloud/storage/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,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))
Expand All @@ -434,7 +435,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
Expand Down
10 changes: 10 additions & 0 deletions gcloud/storage/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand All @@ -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
Expand Down Expand Up @@ -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'
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions gcloud/storage/test_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -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 TestKeyIterator(unittest2.TestCase):
Expand Down

0 comments on commit e74972d

Please sign in to comment.