Skip to content

Commit e74972d

Browse files
committed
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).
1 parent e69c8fc commit e74972d

4 files changed

Lines changed: 26 additions & 6 deletions

File tree

gcloud/storage/bucket.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,8 @@ def save_acl(self, acl=None):
506506

507507
if dirty:
508508
result = self.connection.api_request(
509-
method='PATCH', path=self.path, data={'acl': list(acl)})
509+
method='PATCH', path=self.path, data={'acl': list(acl)},
510+
query_params={'projection': 'full'})
510511
self.acl.clear()
511512
for entry in result['acl']:
512513
self.acl.entity(self.acl.entity_from_dict(entry))
@@ -541,7 +542,8 @@ def clear_acl(self):
541542
At this point all the custom rules you created have been removed.
542543
"""
543544
self.connection.api_request(
544-
method='PATCH', path=self.path, data={'acl': []})
545+
method='PATCH', path=self.path, data={'acl': []},
546+
query_params={'projection': 'full'})
545547
self.acl.clear()
546548
self.acl.loaded = True
547549
return self
@@ -596,7 +598,8 @@ def save_default_object_acl(self, acl=None):
596598
if dirty:
597599
result = self.connection.api_request(
598600
method='PATCH', path=self.path,
599-
data={'defaultObjectAcl': list(acl)})
601+
data={'defaultObjectAcl': list(acl)},
602+
query_params={'projection': 'full'})
600603
doa = self.default_object_acl
601604
doa.clear()
602605
for entry in result['defaultObjectAcl']:
@@ -608,7 +611,8 @@ def clear_default_object_acl(self):
608611
"""Remove the Default Object ACL from this bucket."""
609612

610613
self.connection.api_request(
611-
method='PATCH', path=self.path, data={'defaultObjectAcl': []})
614+
method='PATCH', path=self.path, data={'defaultObjectAcl': []},
615+
query_params={'projection': 'full'})
612616
self.default_object_acl.clear()
613617
self.default_object_acl.loaded = True
614618
return self

gcloud/storage/key.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,8 @@ def save_acl(self, acl=None):
418418

419419
if dirty:
420420
result = self.connection.api_request(
421-
method='PATCH', path=self.path, data={'acl': list(acl)})
421+
method='PATCH', path=self.path, data={'acl': list(acl)},
422+
query_params={'projection': 'full'})
422423
self.acl.clear()
423424
for entry in result['acl']:
424425
self.acl.entity(self.acl.entity_from_dict(entry))
@@ -434,7 +435,8 @@ def clear_acl(self):
434435
rules with this method.
435436
"""
436437
self.connection.api_request(
437-
method='PATCH', path=self.path, data={'acl': []})
438+
method='PATCH', path=self.path, data={'acl': []},
439+
query_params={'projection': 'full'})
438440
self.acl.clear()
439441
self.acl.loaded = True
440442
return self

gcloud/storage/test_bucket.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,7 @@ def test_save_acl_existing_set_none_passed(self):
653653
self.assertEqual(kw[0]['method'], 'PATCH')
654654
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
655655
self.assertEqual(kw[0]['data'], {'acl': []})
656+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
656657

657658
def test_save_acl_existing_set_new_passed(self):
658659
NAME = 'name'
@@ -668,6 +669,7 @@ def test_save_acl_existing_set_new_passed(self):
668669
self.assertEqual(kw[0]['method'], 'PATCH')
669670
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
670671
self.assertEqual(kw[0]['data'], {'acl': new_acl})
672+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
671673

672674
def test_clear_acl(self):
673675
NAME = 'name'
@@ -684,6 +686,7 @@ def test_clear_acl(self):
684686
self.assertEqual(kw[0]['method'], 'PATCH')
685687
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
686688
self.assertEqual(kw[0]['data'], {'acl': []})
689+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
687690

688691
def test_reload_default_object_acl_eager_empty(self):
689692
from gcloud.storage.acl import DefaultObjectACL
@@ -772,6 +775,7 @@ def test_save_default_object_acl_existing_set_none_passed(self):
772775
self.assertEqual(kw[0]['method'], 'PATCH')
773776
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
774777
self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
778+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
775779

776780
def test_save_default_object_acl_existing_set_new_passed(self):
777781
NAME = 'name'
@@ -790,6 +794,7 @@ def test_save_default_object_acl_existing_set_new_passed(self):
790794
self.assertEqual(kw[0]['method'], 'PATCH')
791795
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
792796
self.assertEqual(kw[0]['data'], {'defaultObjectAcl': new_acl})
797+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
793798

794799
def test_clear_default_object_acl(self):
795800
NAME = 'name'
@@ -806,6 +811,7 @@ def test_clear_default_object_acl(self):
806811
self.assertEqual(kw[0]['method'], 'PATCH')
807812
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
808813
self.assertEqual(kw[0]['data'], {'defaultObjectAcl': []})
814+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
809815

810816
def test_make_public_defaults(self):
811817
from gcloud.storage.acl import _ACLEntity
@@ -823,6 +829,7 @@ def test_make_public_defaults(self):
823829
self.assertEqual(kw[0]['method'], 'PATCH')
824830
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
825831
self.assertEqual(kw[0]['data'], {'acl': after['acl']})
832+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
826833

827834
def test_make_public_w_future(self):
828835
from gcloud.storage.acl import _ACLEntity
@@ -842,9 +849,11 @@ def test_make_public_w_future(self):
842849
self.assertEqual(kw[0]['method'], 'PATCH')
843850
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
844851
self.assertEqual(kw[0]['data'], {'acl': permissive})
852+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
845853
self.assertEqual(kw[1]['method'], 'PATCH')
846854
self.assertEqual(kw[1]['path'], '/b/%s' % NAME)
847855
self.assertEqual(kw[1]['data'], {'defaultObjectAcl': permissive})
856+
self.assertEqual(kw[1]['query_params'], {'projection': 'full'})
848857

849858
def test_make_public_recursive(self):
850859
from gcloud.storage.acl import _ACLEntity
@@ -894,6 +903,7 @@ def get_items_from_response(self, response):
894903
self.assertEqual(kw[0]['method'], 'PATCH')
895904
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
896905
self.assertEqual(kw[0]['data'], {'acl': permissive})
906+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
897907
self.assertEqual(kw[1]['method'], 'GET')
898908
self.assertEqual(kw[1]['path'], '/b/%s/o' % NAME)
899909
self.assertEqual(kw[1]['query_params'], None)

gcloud/storage/test_key.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ def test_save_acl_existing_set_none_passed(self):
552552
self.assertEqual(kw[0]['method'], 'PATCH')
553553
self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
554554
self.assertEqual(kw[0]['data'], {'acl': []})
555+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
555556

556557
def test_save_acl_existing_set_new_passed(self):
557558
KEY = 'key'
@@ -568,6 +569,7 @@ def test_save_acl_existing_set_new_passed(self):
568569
self.assertEqual(kw[0]['method'], 'PATCH')
569570
self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
570571
self.assertEqual(kw[0]['data'], {'acl': new_acl})
572+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
571573

572574
def test_clear_acl(self):
573575
KEY = 'key'
@@ -583,6 +585,7 @@ def test_clear_acl(self):
583585
self.assertEqual(kw[0]['method'], 'PATCH')
584586
self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
585587
self.assertEqual(kw[0]['data'], {'acl': []})
588+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
586589

587590
def test_make_public(self):
588591
from gcloud.storage.acl import _ACLEntity
@@ -600,6 +603,7 @@ def test_make_public(self):
600603
self.assertEqual(kw[0]['method'], 'PATCH')
601604
self.assertEqual(kw[0]['path'], '/b/name/o/%s' % KEY)
602605
self.assertEqual(kw[0]['data'], {'acl': permissive})
606+
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
603607

604608

605609
class TestKeyIterator(unittest2.TestCase):

0 commit comments

Comments
 (0)