Merge pull request #52 from boto/plural-refs

Support plural references.

CHANGELOG.rst

@@ -4,6 +4,9 @@ Changelog
 Unreleased
 ----------
 
+* feature:Resources: Support plural references and nested JMESPath
+  queries for data members when building parameters and identifiers.
+  (`issue 52 <https://github.com/boto/boto3/pull/52>`__)
 * feature:Dependency: Update to JMESPath 0.6.1
 * feature:Resources: Update to the latest resource JSON format.
   (`issue 51 <https://github.com/boto/boto3/pull/51>`__)

boto3/resources/factory.py

@@ -21,7 +21,7 @@
 from .base import ResourceMeta, ServiceResource
 from .collection import CollectionFactory
 from .model import ResourceModel
-from .response import all_not_none, build_identifiers
+from .response import build_identifiers, ResourceHandler
 from ..exceptions import ResourceLoadException
 
 
@@ -176,7 +176,7 @@ def _load_has_relations(self, attrs, service_name, resource_name,
             # This is a dangling reference, i.e. we have all
             # the data we need to create the resource, so
             # this instance becomes an attribute on the class.
-            snake_cased = xform_name(reference.resource.type)
+            snake_cased = xform_name(reference.name)
             snake_cased = self._check_allowed_name(
                 attrs, snake_cased, 'reference', model.name)
             attrs[snake_cased] = self._create_reference(
@@ -299,24 +299,19 @@ def _create_reference(factory_self, name, snake_cased, reference,
         """
         Creates a new property on the resource to lazy-load a reference.
         """
+        # References are essentially an action with no request
+        # or response, so we can re-use the response handlers to
+        # build up resources from identifiers and data members.
+        handler = ResourceHandler('', factory_self, resource_defs,
+                                  service_model, reference.resource)
+
         def get_reference(self):
             # We need to lazy-evaluate the reference to handle circular
             # references between resources. We do this by loading the class
             # when first accessed.
             # First, though, we need to see if we have the required
             # identifiers to instantiate the resource reference.
-            identifiers = dict(build_identifiers(
-                reference.resource.identifiers, self))
-            resource = None
-            if all_not_none(identifiers.values()):
-                # Identifiers are present, so now we can create the resource
-                # instance using them.
-                resource_type = reference.resource.type
-                cls = factory_self.load_from_definition(
-                    service_name, name, resource_defs.get(resource_type),
-                    resource_defs, service_model)
-                resource = cls(**identifiers)
-            return resource
+            return handler(self, {}, {})
 
         get_reference.__name__ = str(snake_cased)
         get_reference.__doc__ = 'TODO'

boto3/resources/params.py

@@ -13,12 +13,41 @@
 
 import re
 
+import jmespath
 from botocore import xform_name
 
+from ..exceptions import ResourceLoadException
+
 
 INDEX_RE = re.compile('\[(.*)\]$')
 
 
+def get_data_member(parent, path):
+    """
+    Get a data member from a parent using a JMESPath search query,
+    loading the parent if required. If the parent cannot be loaded
+    and no data is present then an exception is raised.
+
+    :type parent: ServiceResource
+    :param parent: The resource instance to which contains data we
+                   are interested in.
+    :type path: string
+    :param path: The JMESPath expression to query
+    :raises ResourceLoadException: When no data is present and the
+                                   resource cannot be loaded.
+    :returns: The queried data or ``None``.
+    """
+    # Ensure the parent has its data loaded, if possible.
+    if parent.meta.data is None:
+        if hasattr(parent, 'load'):
+            parent.load()
+        else:
+            raise ResourceLoadException(
+                '{0} has no load method!'.format(parent.__class__.__name__))
+
+    return jmespath.search(path, parent.meta.data)
+
+
 def create_request_parameters(parent, request_model, params=None):
     """
     Handle request parameters that can be filled in from identifiers,
@@ -49,16 +78,9 @@ def create_request_parameters(parent, request_model, params=None):
             # Resource identifier, e.g. queue.url
             value = getattr(parent, xform_name(param.name))
         elif source == 'data':
-            # If this is a dataMember then it may incur a load
+            # If this is a data member then it may incur a load
             # action before returning the value.
-            # TODO: Use ``jmespath.search``
-            # Data members are accessed via a ``path``, which is
-            # a JMESPath query. JMESPath does not support attribute
-            # access on an object yet. Once it does, we should
-            # use it here. Until then, ``getattr`` works in most
-            # simple cases, but will fail if path is something
-            # like ``Items[0].id``.
-            value = getattr(parent, xform_name(param.path))
+            value = get_data_member(parent, param.path)
         elif source in ['string', 'integer', 'boolean']:
             # These are hard-coded values in the definition
             value = param.value

boto3/resources/response.py

@@ -14,6 +14,9 @@
 import jmespath
 from botocore import xform_name
 
+from ..exceptions import ResourceLoadException
+from .params import get_data_member
+
 
 def all_not_none(iterable):
     """
@@ -59,8 +62,9 @@ def build_identifiers(identifiers, parent, params=None, raw_response=None):
         elif source == 'identifier':
             value = getattr(parent, xform_name(identifier.name))
         elif source == 'data':
-            # TODO: This should be a JMESPath query
-            value = getattr(parent, xform_name(identifier.path))
+            # If this is a data member then it may incur a load
+            # action before returning the value.
+            value = get_data_member(parent, identifier.path)
         elif source == 'input':
             # This value is set by the user, so ignore it here
             continue
@@ -83,7 +87,7 @@ def build_empty_response(search_path, operation_name, service_model):
     :type search_path: string
     :param search_path: JMESPath expression to search in the response
     :type operation_name: string
-    :param operation_name: Name of the underlying service operation
+    :param operation_name: Name of the underlying service operation.
     :type service_model: :ref:`botocore.model.ServiceModel`
     :param service_model: The Botocore service model
     :rtype: dict, list, or None
@@ -169,12 +173,13 @@ class ResourceHandler(object):
     :type resource_model: :py:class:`~boto3.resources.model.ResponseResource`
     :param resource_model: Response resource model.
     :type operation_name: string
-    :param operation_name: Name of the underlying service operation
+    :param operation_name: Name of the underlying service operation, if it
+                           exists.
     :rtype: ServiceResource or list
     :return: New resource instance(s).
     """
     def __init__(self, search_path, factory, resource_defs, service_model,
-                 resource_model, operation_name):
+                 resource_model, operation_name=None):
         self.search_path = search_path
         self.factory = factory
         self.resource_defs = resource_defs
@@ -239,10 +244,16 @@ def __call__(self, parent, params, response):
             response = self.handle_response_item(resource_cls,
                 parent, identifiers, search_response)
         else:
-            # The response is should be empty, but that may mean an
-            # empty dict, list, or None.
-            response = build_empty_response(self.search_path,
-                self.operation_name, self.service_model)
+            # The response should be empty, but that may mean an
+            # empty dict, list, or None based on whether we make
+            # a remote service call and what shape it is expected
+            # to return.
+            response = None
+            if self.operation_name is not None:
+                # A remote service call was made, so try and determine
+                # its shape.
+                response = build_empty_response(self.search_path,
+                    self.operation_name, self.service_model)
 
         return response
 

tests/unit/resources/test_action.py

@@ -191,13 +191,16 @@ def test_batch_action_creates_parameters_from_items(self):
         client = mock.Mock()
 
         item1 = mock.Mock()
-        item1.meta = ResourceMeta('test', client=client)
-        item1.bucket_name = 'bucket'
-        item1.key = 'item1'
+        item1.meta = ResourceMeta('test', client=client, data={
+            'BucketName': 'bucket',
+            'Key': 'item1'
+        })
 
         item2 = mock.Mock()
-        item2.bucket_name = 'bucket'
-        item2.key = 'item2'
+        item2.meta = ResourceMeta('test', client=client, data={
+            'BucketName': 'bucket',
+            'Key': 'item2'
+        })
 
         collection = mock.Mock()
         collection.pages.return_value = [[item1, item2]]

tests/unit/resources/test_factory.py

@@ -432,12 +432,24 @@ def test_resource_loads_references(self):
                              'path': 'SubnetId'}
                         ]
                     }
+                },
+                'Vpcs': {
+                    'resource': {
+                        'type': 'Vpc',
+                        'identifiers': [
+                            {'target': 'Id', 'source': 'data',
+                             'path': 'Vpcs[].Id'}
+                        ]
+                    }
                 }
             }
         }
         defs = {
             'Subnet': {
                 'identifiers': [{'name': 'Id'}]
+            },
+            'Vpc': {
+                'identifiers': [{'name': 'Id'}]
             }
         }
         service_model = ServiceModel({
@@ -462,16 +474,32 @@ def test_resource_loads_references(self):
         # Load the resource with no data
         resource.meta.data = {}
 
-        self.assertTrue(hasattr(resource, 'subnet'),
-                        'Resource should have a subnet reference')
-        self.assertIsNone(resource.subnet,
-                          'Missing identifier, should return None')
+        self.assertTrue(
+            hasattr(resource, 'subnet'),
+            'Resource should have a subnet reference')
+        self.assertIsNone(
+            resource.subnet,
+            'Missing identifier, should return None')
+        self.assertIsNone(resource.vpcs)
 
         # Load the resource with data to instantiate a reference
-        resource.meta.data = {'SubnetId': 'abc123'}
+        resource.meta.data = {
+            'SubnetId': 'abc123',
+            'Vpcs': [
+                {'Id': 'vpc1'},
+                {'Id': 'vpc2'}
+            ]
+        }
+
         self.assertIsInstance(resource.subnet, ServiceResource)
         self.assertEqual(resource.subnet.id, 'abc123')
 
+        vpcs = resource.vpcs
+        self.assertIsInstance(vpcs, list)
+        self.assertEqual(len(vpcs), 2)
+        self.assertEqual(vpcs[0].id, 'vpc1')
+        self.assertEqual(vpcs[1].id, 'vpc2')
+
     @mock.patch('boto3.resources.model.Collection')
     def test_resource_loads_collections(self, mock_model):
         model = {

tests/unit/resources/test_params.py

@@ -11,6 +11,8 @@
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
 
+from boto3.exceptions import ResourceLoadException
+from boto3.resources.base import ResourceMeta, ServiceResource
 from boto3.resources.model import Request
 from boto3.resources.params import create_request_parameters, \
                                    build_param_structure
@@ -44,19 +46,68 @@ def test_service_action_params_data_member(self):
                 {
                     'target': 'WarehouseUrl',
                     'source': 'data',
-                    'path': 'some_member'
+                    'path': 'SomeMember'
                 }
             ]
         })
 
         parent = mock.Mock()
-        parent.some_member = 'w-url'
+        parent.meta = ResourceMeta('test', data={
+            'SomeMember': 'w-url'
+        })
 
         params = create_request_parameters(parent, request_model)
 
         self.assertEqual(params['WarehouseUrl'], 'w-url',
             'Parameter not set from resource property')
 
+    def test_service_action_params_data_member_missing(self):
+        request_model = Request({
+            'operation': 'GetFrobs',
+            'params': [
+                {
+                    'target': 'WarehouseUrl',
+                    'source': 'data',
+                    'path': 'SomeMember'
+                }
+            ]
+        })
+
+        parent = mock.Mock()
+
+        def load_data():
+            parent.meta.data = {
+                'SomeMember': 'w-url'
+            }
+
+        parent.load.side_effect = load_data
+        parent.meta = ResourceMeta('test')
+
+        params = create_request_parameters(parent, request_model)
+
+        parent.load.assert_called_with()
+        self.assertEqual(params['WarehouseUrl'], 'w-url',
+            'Parameter not set from resource property')
+
+    def test_service_action_params_data_member_missing_no_load(self):
+        request_model = Request({
+            'operation': 'GetFrobs',
+            'params': [
+                {
+                    'target': 'WarehouseUrl',
+                    'source': 'data',
+                    'path': 'SomeMember'
+                }
+            ]
+        })
+
+        # This mock has no ``load`` method.
+        parent = mock.Mock(spec=ServiceResource)
+        parent.meta = ResourceMeta('test', data=None)
+
+        with self.assertRaises(ResourceLoadException):
+            params = create_request_parameters(parent, request_model)
+
     def test_service_action_params_constants(self):
         request_model = Request({
             'operation': 'GetFrobs',
@@ -151,10 +202,14 @@ def test_service_action_params_reuse(self):
         })
 
         item1 = mock.Mock()
-        item1.key = 'item1'
+        item1.meta = ResourceMeta('test', data={
+            'Key': 'item1'
+        })
 
         item2 = mock.Mock()
-        item2.key = 'item2'
+        item2.meta = ResourceMeta('test', data={
+            'Key': 'item2'
+        })
 
         # Here we create params and then re-use it to build up a more
         # complex structure over multiple calls.

tests/unit/resources/test_response.py

@@ -83,7 +83,9 @@ def test_build_identifier_from_parent_data_member(self):
                        path='Member')]
 
         parent = mock.Mock()
-        parent.member = 'data-member'
+        parent.meta = ResourceMeta('test', data={
+            'Member': 'data-member'
+        })
         params = {}
         response = {
             'Container': {