Allow layering paths to include numeric indices

This PS makes updates to layering merge actions to allow for
numeric indices to work. jsonpath_* utility methods are used
instead for merging and replacing. Added unit tests to verify
that layering scenarios for replace, merge and delete work
with numeric indices.

Change-Id: Id6d592231cb90144bb5857bee48cecf9f6478692

deckhand/engine/layering.py

@@ -26,6 +26,7 @@
 from deckhand.engine import utils as engine_utils
 from deckhand import errors
 from deckhand import types
+from deckhand import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -46,7 +47,8 @@ class DocumentLayering(object):
         together into a fully rendered document.
     """
 
-    SUPPORTED_METHODS = ('merge', 'replace', 'delete')
+    _SUPPORTED_METHODS = (_MERGE_ACTION, _REPLACE_ACTION, _DELETE_ACTION) = (
+        'merge', 'replace', 'delete')
 
     def _replace_older_parent_with_younger_parent(self, child, parent,
                                                   all_children):
@@ -369,78 +371,75 @@ def _apply_action(self, action, child_data, overall_data):
 
         Supported actions include:
 
-            * `merge` - a "deep" merge that layers new and modified data onto
+            * ``merge`` - a "deep" merge that layers new and modified data onto
               existing data
-            * `replace` - overwrite data at the specified path and replace it
+            * ``replace`` - overwrite data at the specified path and replace it
               with the data given in this document
-            * `delete` - remove the data at the specified path
+            * ``delete`` - remove the data at the specified path
 
         :raises UnsupportedActionMethod: If the layering action isn't found
             among ``self.SUPPORTED_METHODS``.
         :raises MissingDocumentKey: If a layering action path isn't found
-            in both the parent and child documents being layered together.
+            in the child document.
         """
         method = action['method']
-        if method not in self.SUPPORTED_METHODS:
+        if method not in self._SUPPORTED_METHODS:
             raise errors.UnsupportedActionMethod(
                 action=action, document=child_data)
 
         # Use copy to prevent these data from being updated referentially.
         overall_data = copy.deepcopy(overall_data)
         child_data = copy.deepcopy(child_data)
-        rendered_data = overall_data
-
-        # Remove empty string paths and ensure that "data" is always present.
-        path = action['path'].split('.')
-        path = [p for p in path if p != '']
-        path.insert(0, 'data')
-        last_key = 'data' if not path[-1] else path[-1]
-
-        for attr in path:
-            if attr == path[-1]:
-                break
-            rendered_data = rendered_data.get(attr)
-            child_data = child_data.get(attr)
-
-        if method == 'delete':
-            # If the entire document is passed (i.e. the dict including
-            # metadata, data, schema, etc.) then reset data to an empty dict.
-            if last_key == 'data':
-                rendered_data['data'] = {}
-            elif last_key in rendered_data:
-                del rendered_data[last_key]
-            elif last_key not in rendered_data:
-                # If the key does not exist in `rendered_data`, this is a
-                # validation error.
-                raise errors.MissingDocumentKey(
-                    child=child_data, parent=rendered_data, key=last_key)
-        elif method == 'merge':
-            if last_key in rendered_data and last_key in child_data:
-                # If both entries are dictionaries, do a deep merge. Otherwise
-                # do a simple merge.
-                if (isinstance(rendered_data[last_key], dict)
-                    and isinstance(child_data[last_key], dict)):
-                    engine_utils.deep_merge(
-                        rendered_data[last_key], child_data[last_key])
-                else:
-                    rendered_data.setdefault(last_key, child_data[last_key])
-            elif last_key in child_data:
-                rendered_data.setdefault(last_key, child_data[last_key])
+
+        action_path = action['path']
+        if action_path.startswith('.data'):
+            action_path = action_path[5:]
+
+        if method == self._DELETE_ACTION:
+            if action_path == '.':
+                overall_data.data = {}
             else:
-                # If the key does not exist in the child document, this is a
-                # validation error.
+                from_child = utils.jsonpath_parse(overall_data.data,
+                                                  action_path)
+                if from_child is None:
+                    raise errors.MissingDocumentKey(
+                        child=child_data.data,
+                        parent=overall_data.data,
+                        key=action_path)
+
+                engine_utils.deep_delete(from_child, overall_data.data, None)
+
+        elif method == self._MERGE_ACTION:
+            from_parent = utils.jsonpath_parse(overall_data.data, action_path)
+            from_child = utils.jsonpath_parse(child_data.data, action_path)
+
+            if from_child is None:
                 raise errors.MissingDocumentKey(
-                    child=child_data, parent=rendered_data, key=last_key)
-        elif method == 'replace':
-            if last_key in rendered_data and last_key in child_data:
-                rendered_data[last_key] = child_data[last_key]
-            elif last_key in child_data:
-                rendered_data.setdefault(last_key, child_data[last_key])
-            elif last_key not in child_data:
-                # If the key does not exist in the child document, this is a
-                # validation error.
+                    child=child_data.data,
+                    parent=overall_data.data,
+                    key=action_path)
+
+            if (isinstance(from_parent, dict)
+                    and isinstance(from_child, dict)):
+                engine_utils.deep_merge(from_parent, from_child)
+
+            if from_parent is not None:
+                overall_data.data = utils.jsonpath_replace(
+                    overall_data.data, from_parent, action_path)
+            else:
+                overall_data.data = utils.jsonpath_replace(
+                    overall_data.data, from_child, action_path)
+        elif method == self._REPLACE_ACTION:
+            from_child = utils.jsonpath_parse(child_data.data, action_path)
+
+            if from_child is None:
                 raise errors.MissingDocumentKey(
-                    child=child_data, parent=rendered_data, key=last_key)
+                    child=child_data.data,
+                    parent=overall_data.data,
+                    key=action_path)
+
+            overall_data.data = utils.jsonpath_replace(
+                overall_data.data, from_child, action_path)
 
         return overall_data
 
@@ -466,10 +465,10 @@ def render(self):
             if doc.parent_selector:
                 parent_meta = self._parents.get((doc.schema, doc.name))
 
-                # Apply each action to the current document.
                 if parent_meta:
                     parent = self._documents_by_index[parent_meta]
                     rendered_data = parent
+                    # Apply each action to the current document.
                     for action in doc.actions:
                         LOG.debug('Applying action %s to document with '
                                   'name=%s, schema=%s, layer=%s.', action,

deckhand/engine/utils.py

@@ -34,3 +34,38 @@ def deep_merge(dct, merge_dct):
             deep_merge(dct[k], merge_dct[k])
         else:
             dct[k] = merge_dct[k]
+
+
+def deep_delete(target, value, parent):
+    """Recursively search for then delete ``target`` from ``parent``.
+
+    :param target: Target value to remove.
+    :param value: Current value in a list or dict to compare against
+        ``target`` and removed from ``parent`` given match.
+    :param parent: Tracks the parent data structure from which ``value``
+        is removed.
+    :type parent: list or dict
+    :returns: Whether ``target`` was found.
+    :rtype: bool
+    """
+
+    if value == target:
+        if isinstance(parent, list):
+            parent.remove(value)
+            return True
+        elif isinstance(parent, dict):
+            for k, v in parent.items():
+                if v == value:
+                    parent.pop(k)
+                    return True
+    elif isinstance(value, list):
+        for v in value:
+            found = deep_delete(target, v, value)
+            if found:
+                return True
+    elif isinstance(value, dict):
+        for v in value.values():
+            found = deep_delete(target, v, value)
+            if found:
+                return True
+    return False

deckhand/tests/unit/engine/test_document_layering.py

@@ -305,6 +305,153 @@ def test_layering_default_scenario(self):
         site_expected = {'a': {'x': 1, 'y': 2}, 'b': 4}
         self._test_layering(documents, site_expected)
 
+    def test_layering_default_scenario_merge_with_numeric_in_path(self):
+        # Check that 2 dicts are merged together for [0] index.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{"z": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'x': 1, 'y': 2, 'z': 3}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that 2 dicts are merged together for [0] index with [1] index
+        # data carried over.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}, {"z": 3}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'x': 1, 'y': 2}, {'z': 3}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that 2 dicts are merged together for [0] index with deep merge.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{"x": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'x': 3, 'y': 2}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that 2 dicts are merged together for [1] index.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": ["test", {"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{}, {"z": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": ".data.a[1]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': ["test", {'x': 1, 'y': 2, 'z': 3}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that merging works for an attribute within an index.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": ["test", {"x": {"b": 5}}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{}, {"x": {"a": 8}}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "merge", "path": ".data.a[1].x"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {"a": ["test", {"x": {"a": 8, "b": 5}}]}
+        self._test_layering(documents, site_expected)
+
+    def test_layering_default_scenario_replace_with_numeric_in_path(self):
+        # Check that replacing the first index works.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{"z": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "replace", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'z': 3}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that replacing the second index works.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1}, {"y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{}, {"y": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "replace", "path": ".data.a[1]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'x': 1}, {'y': 3}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that replacing an attribute within an index works.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{}, {"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {"a": [{}, {"y": 3}]}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "replace", "path": ".data.a[1].y"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{}, {'x': 1, 'y': 3}]}
+        self._test_layering(documents, site_expected)
+
+    def test_layering_default_scenario_delete_with_numeric_in_path(self):
+        # Check that removing the first index results in empty array.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "delete", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': []}
+        self._test_layering(documents, site_expected)
+
+        # Check that removing one index retains the other.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1}, {"y": 2}]}},
+            "_SITE_DATA_1_": {"data": {}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "delete", "path": ".data.a[0]"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{'y': 2}]}
+        self._test_layering(documents, site_expected)
+
+        # Check that removing an attribute within an index works.
+        mapping = {
+            "_GLOBAL_DATA_1_": {"data": {"a": [{"x": 1, "y": 2}]}},
+            "_SITE_DATA_1_": {"data": {}},
+            "_SITE_ACTIONS_1_": {
+                "actions": [{"method": "delete", "path": ".data.a[0].x"}]}
+        }
+        doc_factory = factories.DocumentFactory(2, [1, 1])
+        documents = doc_factory.gen_test(mapping, site_abstract=False)
+
+        site_expected = {'a': [{"y": 2}]}
+        self._test_layering(documents, site_expected)
+
     def test_layering_default_scenario_multi_parentselector(self):
         mapping = {
             "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}},