Make KeyError from 'get_metadata' more flexible.

Allow subclasses to define a 'CUSTOM_METADATA_FIELDS' mapping which
maps field name -> accessor name, and use it to generate more meaningful
error message when raising KeyError for those fields.

Author: tseaver

gcloud/storage/_helpers.py

@@ -8,17 +8,17 @@ class _MetadataMixin(object):
     """Abstract mixin for cloud storage classes with associated metadata.
 
     Non-abstract subclasses should implement:
-      - METADATA_ACL_FIELDS
+      - CUSTOM_METADATA_FIELDS
       - connection
       - path
     """
 
-    METADATA_ACL_FIELDS = None
-    """Tuple of fields which pertain to metadata.
+    CUSTOM_METADATA_FIELDS = None
+    """Mapping of field name -> accessor for fields w/ custom accessors.
 
-    Expected to be set by subclasses. Fields in this tuple will cause
-    `get_metadata()` to raise a KeyError with a message to use get_acl()
-    methods.
+    Expected to be set by subclasses. Fields in this mapping will cause
+    `get_metadata()` to raise a KeyError with a message to use the relevant
+    accessor methods.
     """
 
     def __init__(self, name=None, metadata=None):
@@ -88,12 +88,13 @@ def get_metadata(self, field=None, default=None):
         :rtype: dict or anything
         :returns: All metadata or the value of the specific field.
 
-        :raises: :class:`KeyError` if the field is in METADATA_ACL_FIELDS.
+        :raises: :class:`KeyError` if the field is in CUSTOM_METADATA_FIELDS.
         """
         # We ignore 'acl' and related fields because they are meant to be
         # handled via 'get_acl()' and related methods.
-        if field in self.METADATA_ACL_FIELDS:
-            message = 'Use get_acl() or related methods instead.'
+        custom = self.CUSTOM_METADATA_FIELDS.get(field)
+        if custom is not None:
+            message = 'Use %s or related methods instead.' % custom
             raise KeyError((field, message))
 
         if not self.has_metadata(field=field):

gcloud/storage/bucket.py

@@ -21,8 +21,11 @@ class Bucket(_MetadataMixin):
     :param name: The name of the bucket.
     """
 
-    METADATA_ACL_FIELDS = ('acl', 'defaultObjectAcl')
-    """Tuple of metadata fields pertaining to bucket ACLs."""
+    CUSTOM_METADATA_FIELDS = {
+        'acl': 'get_acl',
+        'defaultObjectAcl': 'get_default_object_acl',
+    }
+    """Mapping of field name -> accessor for fields w/ custom accessors."""
 
     # ACL rules are lazily retrieved.
     _acl = _default_object_acl = None

gcloud/storage/key.py

@@ -13,8 +13,10 @@
 class Key(_MetadataMixin):
     """A wrapper around Cloud Storage's concept of an ``Object``."""
 
-    METADATA_ACL_FIELDS = ('acl',)
-    """Tuple of metadata fields pertaining to key ACLs."""
+    CUSTOM_METADATA_FIELDS = {
+        'acl': 'get_acl',
+    }
+    """Mapping of field name -> accessor for fields w/ custom accessors."""
 
     CHUNK_SIZE = 1024 * 1024  # 1 MB.
     """The size of a chunk of data whenever iterating (1 MB).

gcloud/storage/test__helpers.py

@@ -16,3 +16,23 @@ def test_abstract_properties(self):
                           lambda: metadata_object.connection)
         self.assertRaises(NotImplementedError,
                           lambda: metadata_object.path)
+
+    def test_get_metadata_w_custom_field(self):
+        class Derived(self._getTargetClass()):
+            CUSTOM_METADATA_FIELDS = {'foo': 'get_foo'}
+
+            @property
+            def connection(self):  # pragma: NO COVER
+                return None
+
+            @property
+            def path(self):  # pragma: NO COVER
+                return None
+
+        derived = Derived()
+        try:
+            derived.get_metadata('foo')
+        except KeyError as e:
+            self.assertTrue('get_foo' in str(e))
+        else:  # pragma: NO COVER
+            self.assert_('KeyError not raised')