Skip to content

Commit

Permalink
Merge pull request #682 from tseaver/652-handle_empty_acls
Browse files Browse the repository at this point in the history 
#652: Harden ACL 'save'/'reload' against missing element in server response.
  • Loading branch information
@tseaver
tseaver committed Feb 26, 2015
2 parents f0a1157 + 770f328 commit 51e286f
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 12 deletions.
8 changes: 4 additions & 4 deletions gcloud/storage/acl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ def reload(self):
url_path = '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
found = self.bucket.connection.api_request(method='GET', path=url_path)
self.loaded = True
for entry in found['items']:
for entry in found.get('items', ()):
self.add_entity(self.entity_from_dict(entry))

return self
Expand Down Expand Up @@ -450,7 +450,7 @@ def save(self, acl=None):
data={self._URL_PATH_ELEM: list(acl)},
query_params={'projection': 'full'})
self.entities.clear()
for entry in result[self._URL_PATH_ELEM]:
for entry in result.get(self._URL_PATH_ELEM, ()):
self.add_entity(self.entity_from_dict(entry))
self.loaded = True

Expand Down Expand Up @@ -512,7 +512,7 @@ def reload(self):
url_path = '%s/acl' % self.blob.path
found = self.blob.connection.api_request(method='GET', path=url_path)
self.loaded = True
for entry in found['items']:
for entry in found.get('items', ()):
self.add_entity(self.entity_from_dict(entry))

return self
Expand All @@ -535,7 +535,7 @@ def save(self, acl=None):
method='PATCH', path=self.blob.path, data={'acl': list(acl)},
query_params={'projection': 'full'})
self.entities.clear()
for entry in result['acl']:
for entry in result.get('acl', ()):
self.add_entity(self.entity_from_dict(entry))
self.loaded = True

Expand Down
79 changes: 71 additions & 8 deletions gcloud/storage/test_acl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,22 @@ def test_ctor(self):
self.assertFalse(acl.loaded)
self.assertTrue(acl.bucket is bucket)

def test_reload_eager_missing(self):
# https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
NAME = 'name'
ROLE = 'role'
connection = _Connection({})
bucket = _Bucket(connection, NAME)
acl = self._makeOne(bucket)
acl.loaded = True
acl.entity('allUsers', ROLE)
self.assertTrue(acl.reload() is acl)
self.assertEqual(list(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_eager_empty(self):
NAME = 'name'
ROLE = 'role'
Expand Down Expand Up @@ -589,6 +605,21 @@ def test_save_none_set_none_passed(self):
kw = connection._requested
self.assertEqual(len(kw), 0)

def test_save_existing_missing_none_passed(self):
NAME = 'name'
connection = _Connection({})
bucket = _Bucket(connection, NAME)
acl = self._makeOne(bucket)
acl.loaded = True
self.assertTrue(acl.save() is acl)
self.assertEqual(list(acl), [])
kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]['method'], 'PATCH')
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
self.assertEqual(kw[0]['data'], {'acl': []})
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})

def test_save_no_arg(self):
NAME = 'name'
ROLE = 'role'
Expand Down Expand Up @@ -665,24 +696,22 @@ def test_ctor(self):
self.assertFalse(acl.loaded)
self.assertTrue(acl.blob is blob)

def test_reload_eager_empty(self):
def test_reload_eager_missing(self):
# https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
NAME = 'name'
BLOB_NAME = 'blob-name'
ROLE = 'role'
after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
after = {}
connection = _Connection(after)
bucket = _Bucket(connection, NAME)
blob = _Blob(bucket, BLOB_NAME)
acl = self._makeOne(blob)
acl.loaded = True
acl.entity('allUsers', ROLE)
self.assertTrue(acl.reload() is acl)
self.assertEqual(list(acl), after['items'])
kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]['method'], 'GET')
self.assertEqual(kw[0]['path'], '/b/name/o/%s/acl' % BLOB_NAME)
self.assertEqual(list(acl), [])

def test_reload_eager_nonempty(self):
def test_reload_eager_empty(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
ROLE = 'role'
Expand All @@ -696,6 +725,23 @@ def test_reload_eager_nonempty(self):
self.assertTrue(acl.reload() is acl)
self.assertEqual(list(acl), [])

def test_reload_eager_nonempty(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
ROLE = 'role'
after = {'items': [{'entity': 'allUsers', 'role': ROLE}]}
connection = _Connection(after)
bucket = _Bucket(connection, NAME)
blob = _Blob(bucket, BLOB_NAME)
acl = self._makeOne(blob)
acl.loaded = True
self.assertTrue(acl.reload() is acl)
self.assertEqual(list(acl), after['items'])
kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]['method'], 'GET')
self.assertEqual(kw[0]['path'], '/b/name/o/%s/acl' % BLOB_NAME)

def test_reload_lazy(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
Expand Down Expand Up @@ -724,6 +770,23 @@ def test_save_none_set_none_passed(self):
kw = connection._requested
self.assertEqual(len(kw), 0)

def test_save_existing_missing_none_passed(self):
# https://github.com/GoogleCloudPlatform/gcloud-python/issues/652
NAME = 'name'
BLOB_NAME = 'blob-name'
connection = _Connection({'foo': 'Foo'})
bucket = _Bucket(connection, NAME)
blob = _Blob(bucket, BLOB_NAME)
acl = self._makeOne(blob)
acl.loaded = True
self.assertTrue(acl.save() is acl)
kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]['method'], 'PATCH')
self.assertEqual(kw[0]['path'], '/b/name/o/%s' % BLOB_NAME)
self.assertEqual(kw[0]['data'], {'acl': []})
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})

def test_save_existing_set_none_passed(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
Expand Down

0 comments on commit 51e286f

Please sign in to comment.