Merge pull request #690 from dhermes/storage-remove-patch-properties-…

…blob

Moving Bucket._patch_properties (and patch) to _PropertyMixin.

gcloud/storage/_helpers.py

@@ -50,6 +50,7 @@ def __init__(self, name=None, properties=None):
         """
         self.name = name
         self._properties = {}
+        self._changes = set()
         if properties is not None:
             self._properties.update(properties)
 
@@ -105,43 +106,53 @@ def _patch_properties(self, properties):
         This method will only update the fields provided and will not
         touch the other fields.
 
-        It will also reload the properties locally based on the server's
-        response.
+        It **will not** reload the properties from the server. The behavior is
+        local only and syncing occurs via :meth:`patch`.
 
         :type properties: dict
         :param properties: The dictionary of values to update.
 
         :rtype: :class:`_PropertyMixin`
         :returns: The current object.
         """
+        self._changes.update(properties.keys())
+        self._properties.update(properties)
+        return self
+
+    def patch(self):
+        """Sends all changed properties in a PATCH request.
+
+        Updates the ``properties`` with the response from the backend.
+
+        :rtype: :class:`Bucket`
+        :returns: The current bucket.
+        """
         # Pass '?projection=full' here because 'PATCH' documented not
         # to work properly w/ 'noAcl'.
+        update_properties = dict((key, self._properties[key])
+                                 for key in self._changes)
         self._properties = self.connection.api_request(
-            method='PATCH', path=self.path, data=properties,
+            method='PATCH', path=self.path, data=update_properties,
             query_params={'projection': 'full'})
         return self
 
 
 class _PropertyBatch(object):
-    """Context manager: Batch updates to object's ``_patch_properties``
+    """Context manager: Batch updates to object.
 
     :type wrapped: class derived from :class:`_PropertyMixin`.
-    :param wrapped:  the instance whose property updates to defer/batch.
+    :param wrapped:  the instance whose property updates to batch.
     """
     def __init__(self, wrapped):
         self._wrapped = wrapped
-        self._deferred = {}
 
     def __enter__(self):
-        """Intercept / defer property updates."""
-        self._wrapped._patch_properties = self._deferred.update
+        """Do nothing method. Needed to be a context manager."""
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exc_type, value, traceback):
         """Patch deferred property updates if no error."""
-        del self._wrapped._patch_properties
-        if type is None:
-            if self._deferred:
-                self._wrapped._patch_properties(self._deferred)
+        if exc_type is None:
+            self._wrapped.patch()
 
 
 def _scalar_property(fieldname):

gcloud/storage/bucket.py

@@ -94,7 +94,6 @@ def __init__(self, connection=None, name=None, properties=None):
             name = properties.get('name')
         super(Bucket, self).__init__(name=name, properties=properties)
         self._connection = connection
-        self._changes = set()
 
     def __repr__(self):
         return '<Bucket: %s>' % self.name
@@ -765,40 +764,3 @@ def make_public(self, recursive=False, future=False):
             for blob in self:
                 blob.acl.all().grant_read()
                 blob.save_acl()
-
-    def _patch_properties(self, properties):
-        """Update particular fields of this object's properties.
-
-        This method will only update the fields provided and will not
-        touch the other fields.
-
-        It **will not** reload the properties from the server as is done
-        in :meth:`_PropertyMixin._patch_properties`. The behavior is
-        local only and syncing occurs via :meth:`patch`.
-
-        :type properties: dict
-        :param properties: The dictionary of values to update.
-
-        :rtype: :class:`Bucket`
-        :returns: The current bucket.
-        """
-        self._changes.update(properties.keys())
-        self._properties.update(properties)
-        return self
-
-    def patch(self):
-        """Sends all changed properties in a PATCH request.
-
-        Updates the ``properties`` with the response from the backend.
-
-        :rtype: :class:`Bucket`
-        :returns: The current bucket.
-        """
-        # Pass '?projection=full' here because 'PATCH' documented not
-        # to work properly w/ 'noAcl'.
-        update_properties = dict((key, self._properties[key])
-                                 for key in self._changes)
-        self._properties = self.connection.api_request(
-            method='PATCH', path=self.path, data=update_properties,
-            query_params={'projection': 'full'})
-        return self

gcloud/storage/test__helpers.py

@@ -87,6 +87,7 @@ def test__patch_properties(self):
         connection = _Connection({'foo': 'Foo'})
         derived = self._derivedClass(connection, '/path')()
         self.assertTrue(derived._patch_properties({'foo': 'Foo'}) is derived)
+        derived.patch()
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
@@ -95,7 +96,7 @@ def test__patch_properties(self):
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
 
-class TestPropertyBatch(unittest2.TestCase):
+class Test_PropertyBatch(unittest2.TestCase):
 
     def _getTargetClass(self):
         from gcloud.storage._helpers import _PropertyBatch
@@ -127,21 +128,11 @@ def test_ctor_does_not_intercept__patch_properties(self):
         self.assertEqual(before, after)
         self.assertTrue(batch._wrapped is wrapped)
 
-    def test_cm_intercepts_restores__patch_properties(self):
-        wrapped = self._makeWrapped()
-        before = wrapped._patch_properties
-        batch = self._makeOne(wrapped)
-        with batch:
-            # No deferred patching -> no call to the real '_patch_properties'
-            during = wrapped._patch_properties
-        after = wrapped._patch_properties
-        self.assertNotEqual(before, during)
-        self.assertEqual(before, after)
-
-    def test___exit___w_error_skips__patch_properties(self):
+    def test___exit___w_error_skips_patch(self):
         class Testing(Exception):
             pass
-        wrapped = self._makeWrapped()
+        connection = _Connection()
+        wrapped = self._makeWrapped(connection)
         batch = self._makeOne(wrapped)
         try:
             with batch:
@@ -152,7 +143,10 @@ class Testing(Exception):
         except Testing:
             pass
 
-    def test___exit___no_error_aggregates__patch_properties(self):
+        kw = connection._requested
+        self.assertEqual(len(kw), 0)
+
+    def test___exit___no_error_calls_patch(self):
         connection = _Connection({'foo': 'Foo'})
         wrapped = self._makeWrapped(connection, '/path')
         batch = self._makeOne(wrapped)

gcloud/storage/test_blob.py

@@ -667,6 +667,7 @@ def test_cache_control_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.cache_control = CACHE_CONTROL
+        blob.patch()
         self.assertEqual(blob.cache_control, CACHE_CONTROL)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -701,6 +702,7 @@ def test_content_disposition_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.content_disposition = CONTENT_DISPOSITION
+        blob.patch()
         self.assertEqual(blob.content_disposition, CONTENT_DISPOSITION)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -727,6 +729,7 @@ def test_content_encoding_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.content_encoding = CONTENT_ENCODING
+        blob.patch()
         self.assertEqual(blob.content_encoding, CONTENT_ENCODING)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -753,6 +756,7 @@ def test_content_language_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.content_language = CONTENT_LANGUAGE
+        blob.patch()
         self.assertEqual(blob.content_language, CONTENT_LANGUAGE)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -779,6 +783,7 @@ def test_content_type_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.content_type = CONTENT_TYPE
+        blob.patch()
         self.assertEqual(blob.content_type, CONTENT_TYPE)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -805,6 +810,7 @@ def test_crc32c_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.crc32c = CRC32C
+        blob.patch()
         self.assertEqual(blob.crc32c, CRC32C)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -858,6 +864,7 @@ def test_md5_hash_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.md5_hash = MD5_HASH
+        blob.patch()
         self.assertEqual(blob.md5_hash, MD5_HASH)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
@@ -893,6 +900,7 @@ def test_metadata_setter(self):
         bucket = _Bucket(connection)
         blob = self._makeOne(BLOB_NAME, bucket=bucket)
         blob.metadata = METADATA
+        blob.patch()
         self.assertEqual(blob.metadata, METADATA)
         kw = connection._requested
         self.assertEqual(len(kw), 1)