Making _PropertyBatch call _PropertyMixin.patch() on exit.

Author: dhermes

gcloud/storage/_helpers.py

@@ -138,25 +138,21 @@ def patch(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/test__helpers.py

@@ -53,7 +53,6 @@ def test_batch(self):
             derived._patch_properties({'foo': 'Foo'})
             derived._patch_properties({'bar': 'Baz'})
             derived._patch_properties({'foo': 'Qux'})
-        derived.patch()
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
@@ -97,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
@@ -129,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:
@@ -154,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)
@@ -165,7 +157,6 @@ def test___exit___no_error_aggregates__patch_properties(self):
             wrapped._patch_properties({'bar': 'Baz'})
             wrapped._patch_properties({'foo': 'Qux'})
             self.assertEqual(len(kw), 0)
-        wrapped.patch()
         # exited w/o error -> call to the real '_patch_properties'
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')