Merge pull request #353 from tseaver/refactor-bucket_scalar_properties

Refactor bucket scalar properties

gcloud/storage/_helpers.py

@@ -168,3 +168,17 @@ def __exit__(self, type, value, traceback):
         if type is None:
             if self._deferred:
                 self._wrapped._patch_properties(self._deferred)
+
+
+def _scalar_property(fieldname):
+    """Create a property descriptor around the :class:`_PropertyMixin` helpers.
+    """
+    def _getter(self):
+        """Scalar property getter."""
+        return self.properties[fieldname]
+
+    def _setter(self, value):
+        """Scalar property setter."""
+        self._patch_properties({fieldname: value})
+
+    return property(_getter, _setter)

gcloud/storage/bucket.py

@@ -3,6 +3,7 @@
 import os
 
 from gcloud.storage._helpers import _PropertyMixin
+from gcloud.storage._helpers import _scalar_property
 from gcloud.storage import exceptions
 from gcloud.storage.acl import BucketACL
 from gcloud.storage.acl import DefaultObjectACL
@@ -28,7 +29,7 @@ class Bucket(_PropertyMixin):
         'etag': 'etag',
         'id': 'id',
         'lifecycle': 'get_lifecycle()',
-        'location': 'get_location()',
+        'location': 'location',
         'logging': 'get_logging()',
         'metageneration': 'metageneration',
         'name': 'name',
@@ -37,7 +38,7 @@ class Bucket(_PropertyMixin):
         'selfLink': 'self_link',
         'storageClass': 'storage_class',
         'timeCreated': 'time_created',
-        'versioning': 'get_versioning()',
+        'versioning': 'versioning_enabled',
     }
     """Map field name -> accessor for fields w/ custom accessors."""
 
@@ -436,27 +437,14 @@ def update_lifecycle(self, rules):
         """
         self._patch_properties({'lifecycle': {'rule': rules}})
 
-    def get_location(self):
-        """Retrieve location configured for this bucket.
+    location = _scalar_property('location')
+    """Retrieve location configured for this bucket.
 
-        See: https://cloud.google.com/storage/docs/json_api/v1/buckets and
-        https://cloud.google.com/storage/docs/concepts-techniques#specifyinglocations
-
-        :rtype: string
-        :returns: The configured location.
-        """
-        return self.properties.get('location')
-
-    def set_location(self, location):
-        """Update location configured for this bucket.
-
-        See: https://cloud.google.com/storage/docs/json_api/v1/buckets and
-        https://cloud.google.com/storage/docs/concepts-techniques#specifyinglocations
+    See: https://cloud.google.com/storage/docs/json_api/v1/buckets and
+    https://cloud.google.com/storage/docs/concepts-techniques#specifyinglocations
 
-        :type location: string
-        :param location: The new configured location.
-        """
-        self._patch_properties({'location': location})
+    :rtype: string
+    """
 
     def get_logging(self):
         """Return info about access logging for this bucket.
@@ -556,7 +544,8 @@ def time_created(self):
         """
         return self.properties['timeCreated']
 
-    def get_versioning(self):
+    @property
+    def versioning_enabled(self):
         """Is versioning enabled for this bucket?
 
         See:  https://cloud.google.com/storage/docs/object-versioning for
@@ -568,21 +557,17 @@ def get_versioning(self):
         versioning = self.properties.get('versioning', {})
         return versioning.get('enabled', False)
 
-    def enable_versioning(self):
+    @versioning_enabled.setter
+    def versioning_enabled(self, value):
         """Enable versioning for this bucket.
 
         See:  https://cloud.google.com/storage/docs/object-versioning for
         details.
-        """
-        self._patch_properties({'versioning': {'enabled': True}})
 
-    def disable_versioning(self):
-        """Disable versioning for this bucket.
-
-        See:  https://cloud.google.com/storage/docs/object-versioning for
-        details.
+        :type value: convertible to bool
+        :param value: should versioning be anabled for the bucket?
         """
-        self._patch_properties({'versioning': {'enabled': False}})
+        self._patch_properties({'versioning': {'enabled': bool(value)}})
 
     def configure_website(self, main_page_suffix=None, not_found_page=None):
         """Configure website-related properties.

gcloud/storage/key.py

@@ -6,25 +6,12 @@
 from StringIO import StringIO
 
 from gcloud.storage._helpers import _PropertyMixin
+from gcloud.storage._helpers import _scalar_property
 from gcloud.storage.acl import ObjectACL
 from gcloud.storage.exceptions import StorageError
 from gcloud.storage.iterator import Iterator
 
 
-def _scalar_property(fieldname):
-    """Create a property descriptor around the :class:`_PropertyMixin` helpers.
-    """
-    def _getter(self):
-        """Scalar property getter."""
-        return self.properties[fieldname]
-
-    def _setter(self, value):
-        """Scalar property setter."""
-        self._patch_properties({fieldname: value})
-
-    return property(_getter, _setter)
-
-
 class Key(_PropertyMixin):
     """A wrapper around Cloud Storage's concept of an ``Object``."""
 

gcloud/storage/test__helpers.py

@@ -214,6 +214,34 @@ def test___exit___no_error_aggregates__patch_properties(self):
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
 
+class Test__scalar_property(unittest2.TestCase):
+
+    def _callFUT(self, fieldName):
+        from gcloud.storage._helpers import _scalar_property
+        return _scalar_property(fieldName)
+
+    def test_getter(self):
+
+        class Test(object):
+            def __init__(self, **kw):
+                self.properties = kw.copy()
+            do_re_mi = self._callFUT('solfege')
+
+        test = Test(solfege='Latido')
+        self.assertEqual(test.do_re_mi, 'Latido')
+
+    def test_setter(self):
+
+        class Test(object):
+            def _patch_properties(self, mapping):
+                self._patched = mapping.copy()
+            do_re_mi = self._callFUT('solfege')
+
+        test = Test()
+        test.do_re_mi = 'Latido'
+        self.assertEqual(test._patched, {'solfege': 'Latido'})
+
+
 class _Connection(object):
 
     def __init__(self, *responses):

gcloud/storage/test_bucket.py

@@ -562,31 +562,21 @@ def test_update_lifecycle(self):
         self.assertEqual(entries[0]['action']['type'], 'Delete')
         self.assertEqual(entries[0]['condition']['age'], 42)
 
-    def test_get_location_eager(self):
+    def test_location_getter(self):
         NAME = 'name'
         connection = _Connection()
         before = {'location': 'AS'}
         bucket = self._makeOne(connection, NAME, before)
-        self.assertEqual(bucket.get_location(), 'AS')
+        self.assertEqual(bucket.location, 'AS')
         kw = connection._requested
         self.assertEqual(len(kw), 0)
 
-    def test_get_location_lazy(self):
+    def test_location_setter(self):
         NAME = 'name'
         connection = _Connection({'location': 'AS'})
         bucket = self._makeOne(connection, NAME)
-        self.assertEqual(bucket.get_location(), 'AS')
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
-
-    def test_update_location(self):
-        NAME = 'name'
-        connection = _Connection({'location': 'AS'})
-        bucket = self._makeOne(connection, NAME)
-        bucket.set_location('AS')
-        self.assertEqual(bucket.get_location(), 'AS')
+        bucket.location = 'AS'
+        self.assertEqual(bucket.location, 'AS')
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
@@ -718,55 +708,39 @@ def test_time_created(self):
         bucket = self._makeOne(properties=properties)
         self.assertEqual(bucket.time_created, TIME_CREATED)
 
-    def test_get_versioning_eager(self):
+    def test_versioning_enabled_getter_missing(self):
         NAME = 'name'
-        before = {'bar': 'Bar', 'versioning': {'enabled': True}}
-        connection = _Connection()
-        bucket = self._makeOne(connection, NAME, before)
-        self.assertEqual(bucket.get_versioning(), True)
-        kw = connection._requested
-        self.assertEqual(len(kw), 0)
-
-    def test_get_versioning_lazy(self):
-        NAME = 'name'
-        after = {'bar': 'Bar', 'versioning': {'enabled': True}}
-        connection = _Connection(after)
+        connection = _Connection({})
         bucket = self._makeOne(connection, NAME)
-        self.assertEqual(bucket.get_versioning(), True)
+        self.assertEqual(bucket.versioning_enabled, False)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'GET')
         self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
         self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
 
-    def test_enable_versioning(self):
+    def test_versioning_enabled_getter(self):
         NAME = 'name'
-        before = {'versioning': {'enabled': False}}
-        after = {'versioning': {'enabled': True}}
-        connection = _Connection(after)
+        before = {'versioning': {'enabled': True}}
+        connection = _Connection()
         bucket = self._makeOne(connection, NAME, before)
-        self.assertFalse(bucket.get_versioning())
-        bucket.enable_versioning()
-        self.assertTrue(bucket.get_versioning())
+        self.assertEqual(bucket.versioning_enabled, True)
         kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['data'], {'versioning': {'enabled': True}})
-        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+        self.assertEqual(len(kw), 0)
 
-    def test_disable_versioning(self):
+    def test_versioning_enabled_setter(self):
         NAME = 'name'
-        before = {'versioning': {'enabled': True}}
-        after = {'versioning': {'enabled': False}}
+        before = {'versioning': {'enabled': False}}
+        after = {'versioning': {'enabled': True}}
         connection = _Connection(after)
         bucket = self._makeOne(connection, NAME, before)
-        self.assertTrue(bucket.get_versioning())
-        bucket.disable_versioning()
-        self.assertFalse(bucket.get_versioning())
+        self.assertFalse(bucket.versioning_enabled)
+        bucket.versioning_enabled = True
+        self.assertTrue(bucket.versioning_enabled)
         kw = connection._requested
         self.assertEqual(len(kw), 1)
         self.assertEqual(kw[0]['method'], 'PATCH')
-        self.assertEqual(kw[0]['data'], after)
+        self.assertEqual(kw[0]['data'], {'versioning': {'enabled': True}})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
     def test_configure_website_defaults(self):