diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 70e64755..1c3177c4 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -551,6 +551,13 @@ class DocumentLayering(object): overall_data = copy.deepcopy(overall_data) child_data = copy.deepcopy(child_data) + # If None is used, then consider it as a placeholder and coerce the + # data into a dictionary. + if overall_data is None: + overall_data = {} + if child_data is None: + child_data = {} + action_path = action['path'] if action_path.startswith('.data'): action_path = action_path[5:] @@ -574,7 +581,7 @@ class DocumentLayering(object): 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_overall = utils.jsonpath_parse(overall_data.data, action_path) from_child = utils.jsonpath_parse(child_data.data, action_path) if from_child is None: @@ -587,13 +594,29 @@ class DocumentLayering(object): parent_name=overall_data.name, action=action) - if (isinstance(from_parent, dict) and - isinstance(from_child, dict)): - engine_utils.deep_merge(from_parent, from_child) + # If both the child and parent data are dictionaries, then + # traditional merging is possible using JSON path resolution. + # Otherwise, JSON path resolution is not possible, so the only + # way to perform layering is to prioritize the child data over + # that of the parent. This applies when the child data is a + # non-dict, the parent data is a non-dict, or both. + if all(isinstance(x, dict) for x in (from_overall, from_child)): + engine_utils.deep_merge(from_overall, from_child) + else: + LOG.info('Child data is type: %s for [%s, %s] %s. Parent data ' + 'is type: %s for [%s, %s] %s. Both must be ' + 'dictionaries for regular JSON path merging to work. ' + 'Because this is not the case, child data will be ' + 'prioritized over parent data for "merge" action.', + type(from_child), child_data.schema, child_data.layer, + child_data.name, type(from_overall), + overall_data.schema, overall_data.layer, + overall_data.name) + from_overall = from_child - if from_parent is not None: + if from_overall is not None: overall_data.data = utils.jsonpath_replace( - overall_data.data, from_parent, action_path) + overall_data.data, from_overall, action_path) else: overall_data.data = utils.jsonpath_replace( overall_data.data, from_child, action_path) diff --git a/deckhand/tests/unit/engine/test_document_layering.py b/deckhand/tests/unit/engine/test_document_layering.py index a4e68600..ce067f3e 100644 --- a/deckhand/tests/unit/engine/test_document_layering.py +++ b/deckhand/tests/unit/engine/test_document_layering.py @@ -610,6 +610,58 @@ class TestDocumentLayering2Layers(TestDocumentLayering): documents = doc_factory.gen_test(mapping, site_abstract=False) self._test_layering(documents, site_expected[idx]) + def test_layering_with_primitives(self): + """Validates that layering works with non-dictionaries or "primitives" + for short, which include integers, strings, etc. + """ + # Validate layering with strings. + mapping = { + "_GLOBAL_DATA_1_": {"data": "global"}, + "_SITE_DATA_1_": {"data": "site"}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + self._test_layering(documents, site_expected="site") + + # Validate layering with integers. + mapping = { + "_GLOBAL_DATA_1_": {"data": 2}, + "_SITE_DATA_1_": {"data": 1}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + self._test_layering(documents, site_expected=1) + + # Validate layering with mixed primitives. + mapping = { + "_GLOBAL_DATA_1_": {"data": False}, + "_SITE_DATA_1_": {"data": 'a'}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + self._test_layering(documents, site_expected='a') + + def test_layering_with_incompatible_types(self): + """Validates that layering works for incompatible types: that is, + merging a dictionary with a primitive is not possible. For this + case, the child data is simply selected over the parent data. + """ + mapping = { + "_GLOBAL_DATA_1_": {"data": {"foo": "bar"}}, + "_SITE_DATA_1_": {"data": "site"}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + doc_factory = factories.DocumentFactory(2, [1, 1]) + documents = doc_factory.gen_test(mapping, site_abstract=False) + self._test_layering(documents, site_expected="site") + class TestDocumentLayering2LayersAbstractConcrete(TestDocumentLayering): """The the 2-layer payload with site/global layers concrete. diff --git a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py index a7233d1c..6728385a 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py @@ -219,7 +219,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_1_": {"data": "placeholder"}, + "_SITE_DATA_1_": {"data": {}}, "_SITE_ACTIONS_1_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_1_": [{ @@ -275,7 +275,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_1_": {"data": "placeholder"}, + "_SITE_DATA_1_": {"data": {}}, "_SITE_ACTIONS_1_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_1_": [{ @@ -288,7 +288,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_2_": {"data": "placeholder"}, + "_SITE_DATA_2_": {"data": {}}, "_SITE_ACTIONS_2_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_2_": [{ @@ -349,7 +349,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_1_": {"data": "placeholder"}, + "_SITE_DATA_1_": {"data": {}}, "_SITE_ACTIONS_1_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_1_": [{ @@ -362,7 +362,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_2_": {"data": "placeholder"}, + "_SITE_DATA_2_": {"data": {}}, "_SITE_ACTIONS_2_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_2_": [{ @@ -423,7 +423,7 @@ class TestDocumentLayeringWithSubstitution( "path": "." } }], - "_SITE_DATA_1_": {"data": "placeholder"}, + "_SITE_DATA_1_": {"data": {}}, "_SITE_ACTIONS_1_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_1_": [ @@ -448,7 +448,7 @@ class TestDocumentLayeringWithSubstitution( } } ], - "_SITE_DATA_2_": {"data": "placeholder"}, + "_SITE_DATA_2_": {"data": {}}, "_SITE_ACTIONS_2_": { "actions": [{"method": "merge", "path": "."}]}, "_SITE_SUBSTITUTIONS_2_": [