From d66c6db2142ffa764128969302eae286cd83b314 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 1 May 2015 10:40:59 -0400 Subject: [PATCH 1/2] Allow passing explicit connection to '_PropertyMixin.{reload,patch}'. See #825. --- gcloud/storage/_helpers.py | 25 ++++++++++++++++++----- gcloud/storage/test__helpers.py | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py index f0f28867231c..9bc5a60ff56d 100644 --- a/gcloud/storage/_helpers.py +++ b/gcloud/storage/_helpers.py @@ -52,12 +52,20 @@ def __init__(self, name=None): self._properties = {} self._changes = set() - def reload(self): - """Reload properties from Cloud Storage.""" + def reload(self, connection=None): + """Reload properties from Cloud Storage. + + :type connection: :class:`gcloud.storage.connection.Connection` + :param connection: An explicit connection to use for the API request. + If not passed, use the connection assigned to + the object in its constructor. + """ + if connection is None: + connection = self.connection # Pass only '?projection=noAcl' here because 'acl' and related # are handled via custom endpoints. query_params = {'projection': 'noAcl'} - api_response = self.connection.api_request( + api_response = connection.api_request( method='GET', path=self.path, query_params=query_params) self._set_properties(api_response) @@ -89,16 +97,23 @@ def _set_properties(self, value): # If the values are reset, the changes must as well. self._changes = set() - def patch(self): + def patch(self, connection=None): """Sends all changed properties in a PATCH request. Updates the ``_properties`` with the response from the backend. + + :type connection: :class:`gcloud.storage.connection.Connection` + :param connection: An explicit connection to use for the API request. + If not passed, use the connection assigned to + the object in its constructor. """ + if connection is None: + connection = self.connection # 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) - api_response = self.connection.api_request( + api_response = connection.api_request( method='PATCH', path=self.path, data=update_properties, query_params={'projection': 'full'}) self._set_properties(api_response) diff --git a/gcloud/storage/test__helpers.py b/gcloud/storage/test__helpers.py index d8f608ab71d1..1dd015a00ef0 100644 --- a/gcloud/storage/test__helpers.py +++ b/gcloud/storage/test__helpers.py @@ -61,6 +61,21 @@ def test_reload(self): # Make sure changes get reset by reload. self.assertEqual(derived._changes, set()) + def test_reload_w_explicit_connection(self): + connection = _Connection({'foo': 'Foo'}) + derived = self._derivedClass(None, '/path')() + # Make sure changes is not a set, so we can observe a change. + derived._changes = object() + derived.reload(connection) + self.assertEqual(derived._properties, {'foo': 'Foo'}) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]['method'], 'GET') + self.assertEqual(kw[0]['path'], '/path') + self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'}) + # Make sure changes get reset by reload. + self.assertEqual(derived._changes, set()) + def test__patch_property(self): derived = self._derivedClass()() derived._patch_property('foo', 'Foo') @@ -86,6 +101,26 @@ def test_patch(self): # Make sure changes get reset by patch(). self.assertEqual(derived._changes, set()) + def test_patch_w_explicit_connection(self): + connection = _Connection({'foo': 'Foo'}) + derived = self._derivedClass(None, '/path')() + # Make sure changes is non-empty, so we can observe a change. + BAR = object() + BAZ = object() + derived._properties = {'bar': BAR, 'baz': BAZ} + derived._changes = set(['bar']) # Ignore baz. + derived.patch(connection) + self.assertEqual(derived._properties, {'foo': 'Foo'}) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]['method'], 'PATCH') + self.assertEqual(kw[0]['path'], '/path') + self.assertEqual(kw[0]['query_params'], {'projection': 'full'}) + # Since changes does not include `baz`, we don't see it sent. + self.assertEqual(kw[0]['data'], {'bar': BAR}) + # Make sure changes get reset by patch(). + self.assertEqual(derived._changes, set()) + class Test__scalar_property(unittest2.TestCase): From b44d3686d3dbf778c3242cc53b3a99343c65e0cf Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 4 May 2015 16:32:22 -0400 Subject: [PATCH 2/2] Remove 'storage._helpers._PropertyMixin.connection' property. Use implicit default instead. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/852#discussion_r29611410 --- gcloud/storage/_helpers.py | 11 ++--------- gcloud/storage/api.py | 2 +- gcloud/storage/test__helpers.py | 30 ++++++++++++++---------------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py index 9bc5a60ff56d..e6b528b01a12 100644 --- a/gcloud/storage/_helpers.py +++ b/gcloud/storage/_helpers.py @@ -32,11 +32,6 @@ class _PropertyMixin(object): - path """ - @property - def connection(self): - """Abstract getter for the connection to use.""" - raise NotImplementedError - @property def path(self): """Abstract getter for the object path.""" @@ -60,8 +55,7 @@ def reload(self, connection=None): If not passed, use the connection assigned to the object in its constructor. """ - if connection is None: - connection = self.connection + connection = _require_connection(connection) # Pass only '?projection=noAcl' here because 'acl' and related # are handled via custom endpoints. query_params = {'projection': 'noAcl'} @@ -107,8 +101,7 @@ def patch(self, connection=None): If not passed, use the connection assigned to the object in its constructor. """ - if connection is None: - connection = self.connection + connection = _require_connection(connection) # Pass '?projection=full' here because 'PATCH' documented not # to work properly w/ 'noAcl'. update_properties = dict((key, self._properties[key]) diff --git a/gcloud/storage/api.py b/gcloud/storage/api.py index 0e73ab9a75d3..b25a5149ca87 100644 --- a/gcloud/storage/api.py +++ b/gcloud/storage/api.py @@ -160,7 +160,7 @@ def get_bucket(bucket_name, connection=None): """ connection = _require_connection(connection) bucket = Bucket(bucket_name, connection=connection) - bucket.reload() + bucket.reload(connection=connection) return bucket diff --git a/gcloud/storage/test__helpers.py b/gcloud/storage/test__helpers.py index 1dd015a00ef0..fabf23d4718b 100644 --- a/gcloud/storage/test__helpers.py +++ b/gcloud/storage/test__helpers.py @@ -24,34 +24,31 @@ def _getTargetClass(self): def _makeOne(self, *args, **kw): return self._getTargetClass()(*args, **kw) - def _derivedClass(self, connection=None, path=None): + def _derivedClass(self, path=None): class Derived(self._getTargetClass()): - @property - def connection(self): - return connection - @property def path(self): return path return Derived - def test_connection_is_abstract(self): - mixin = self._makeOne() - self.assertRaises(NotImplementedError, lambda: mixin.connection) + def _monkey(self, connection): + from gcloud.storage._testing import _monkey_defaults + return _monkey_defaults(connection=connection) def test_path_is_abstract(self): mixin = self._makeOne() self.assertRaises(NotImplementedError, lambda: mixin.path) - def test_reload(self): + def test_reload_w_implicit_connection(self): connection = _Connection({'foo': 'Foo'}) - derived = self._derivedClass(connection, '/path')() + derived = self._derivedClass('/path')() # Make sure changes is not a set, so we can observe a change. derived._changes = object() - derived.reload() + with self._monkey(connection): + derived.reload() self.assertEqual(derived._properties, {'foo': 'Foo'}) kw = connection._requested self.assertEqual(len(kw), 1) @@ -63,7 +60,7 @@ def test_reload(self): def test_reload_w_explicit_connection(self): connection = _Connection({'foo': 'Foo'}) - derived = self._derivedClass(None, '/path')() + derived = self._derivedClass('/path')() # Make sure changes is not a set, so we can observe a change. derived._changes = object() derived.reload(connection) @@ -81,15 +78,16 @@ def test__patch_property(self): derived._patch_property('foo', 'Foo') self.assertEqual(derived._properties, {'foo': 'Foo'}) - def test_patch(self): + def test_patch_w_implicit_connection(self): connection = _Connection({'foo': 'Foo'}) - derived = self._derivedClass(connection, '/path')() + derived = self._derivedClass('/path')() # Make sure changes is non-empty, so we can observe a change. BAR = object() BAZ = object() derived._properties = {'bar': BAR, 'baz': BAZ} derived._changes = set(['bar']) # Ignore baz. - derived.patch() + with self._monkey(connection): + derived.patch() self.assertEqual(derived._properties, {'foo': 'Foo'}) kw = connection._requested self.assertEqual(len(kw), 1) @@ -103,7 +101,7 @@ def test_patch(self): def test_patch_w_explicit_connection(self): connection = _Connection({'foo': 'Foo'}) - derived = self._derivedClass(None, '/path')() + derived = self._derivedClass('/path')() # Make sure changes is non-empty, so we can observe a change. BAR = object() BAZ = object()