diff --git a/storage/google/cloud/storage/_helpers.py b/storage/google/cloud/storage/_helpers.py index 56a75c684f4c..3a4eb2e232a2 100644 --- a/storage/google/cloud/storage/_helpers.py +++ b/storage/google/cloud/storage/_helpers.py @@ -142,6 +142,8 @@ def patch(self, client=None): # to work properly w/ 'noAcl'. update_properties = {key: self._properties[key] for key in self._changes} + + # Make the API call. api_response = client._connection.api_request( method='PATCH', path=self.path, data=update_properties, query_params={'projection': 'full'}, _target_object=self) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 06550b09ffcb..076cda9c5d65 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -115,6 +115,7 @@ def __init__(self, client, name=None): self._client = client self._acl = BucketACL(self) self._default_object_acl = DefaultObjectACL(self) + self._label_removals = set() def __repr__(self): return '' % (self.name,) @@ -124,6 +125,15 @@ def client(self): """The client bound to this bucket.""" return self._client + def _set_properties(self, value): + """Set the properties for the current object. + + :type value: dict or :class:`google.cloud.storage.batch._FutureDict` + :param value: The properties to be set. + """ + self._label_removals.clear() + return super(Bucket, self)._set_properties(value) + def blob(self, blob_name, chunk_size=None, encryption_key=None): """Factory constructor for blob object. @@ -199,6 +209,27 @@ def create(self, client=None): data=properties, _target_object=self) self._set_properties(api_response) + def patch(self, client=None): + """Sends all changed properties in a PATCH request. + + Updates the ``_properties`` with the response from the backend. + + :type client: :class:`~google.cloud.storage.client.Client` or + ``NoneType`` + :param client: the client to use. If not passed, falls back to the + ``client`` stored on the current object. + """ + # Special case: For buckets, it is possible that labels are being + # removed; this requires special handling. + if self._label_removals: + self._changes.add('labels') + self._properties.setdefault('labels', {}) + for removed_label in self._label_removals: + self._properties['labels'][removed_label] = None + + # Call the superclass method. + return super(Bucket, self).patch(client=client) + @property def acl(self): """Create our ACL on demand.""" @@ -619,6 +650,15 @@ def labels(self, mapping): :type mapping: :class:`dict` :param mapping: Name-value pairs (string->string) labelling the bucket. """ + # If any labels have been expressly removed, we need to track this + # so that a future .patch() call can do the correct thing. + existing = set([k for k in self.labels.keys()]) + incoming = set([k for k in mapping.keys()]) + self._label_removals = self._label_removals.union( + existing.difference(incoming), + ) + + # Actually update the labels on the object. self._patch_property('labels', copy.deepcopy(mapping)) @property diff --git a/storage/tests/system.py b/storage/tests/system.py index bc8169c356b3..e51cfcaeccb2 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -127,7 +127,7 @@ def test_bucket_update_labels(self): new_labels = {'another-label': 'another-value'} bucket.labels = new_labels - bucket.update() + bucket.patch() self.assertEqual(bucket.labels, new_labels) bucket.labels = {} diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 0df94dc5db3d..aac11fe51c7b 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -710,6 +710,33 @@ def test_labels_setter(self): self.assertIsNot(bucket._properties['labels'], LABELS) self.assertIn('labels', bucket._changes) + def test_labels_setter_with_removal(self): + # Make sure the bucket labels look correct and follow the expected + # public structure. + bucket = self._make_one(name='name') + self.assertEqual(bucket.labels, {}) + bucket.labels = {'color': 'red', 'flavor': 'cherry'} + self.assertEqual(bucket.labels, {'color': 'red', 'flavor': 'cherry'}) + bucket.labels = {'color': 'red'} + self.assertEqual(bucket.labels, {'color': 'red'}) + + # Make sure that a patch call correctly removes the flavor label. + client = mock.NonCallableMock(spec=('_connection',)) + client._connection = mock.NonCallableMock(spec=('api_request',)) + bucket.patch(client=client) + client._connection.api_request.assert_called() + _, _, kwargs = client._connection.api_request.mock_calls[0] + self.assertEqual(len(kwargs['data']['labels']), 2) + self.assertEqual(kwargs['data']['labels']['color'], 'red') + self.assertIsNone(kwargs['data']['labels']['flavor']) + + # A second patch call should be a no-op for labels. + client._connection.api_request.reset_mock() + bucket.patch(client=client) + client._connection.api_request.assert_called() + _, _, kwargs = client._connection.api_request.mock_calls[0] + self.assertNotIn('labels', kwargs['data']) + def test_get_logging_w_prefix(self): NAME = 'name' LOG_BUCKET = 'logs'