From 574d9ad1f5c1eaf2418dbd2befe61ada35d416f8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 25 Apr 2019 18:41:48 +0200 Subject: [PATCH 1/2] Fix nested macro/token expansion on python 3 I noticed that in multiqc @ESCAPE_IDENTIFIER@ wasn't being replaced. It's surprising that this didn't cause any issues on python 2, but might have to do with the iteration order changes. --- lib/galaxy/util/xml_macros.py | 21 ++++++++++++++++++++- test/unit/tools/test_parsing.py | 26 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py index f03b8dac8b5..540b7a8cfbc 100644 --- a/lib/galaxy/util/xml_macros.py +++ b/lib/galaxy/util/xml_macros.py @@ -19,6 +19,7 @@ def load_with_references(path): # Collect tokens tokens = _macros_of_type(root, 'token', lambda el: el.text or '') + tokens = expand_nested_tokens(tokens) # Expand xml macros macro_dict = _macros_of_type(root, 'xml', lambda el: XmlMacroDef(el)) @@ -82,8 +83,26 @@ def _macros_of_type(root, type, el_func): return macro_dict +def expand_nested_tokens(tokens, restarts=10): + token_copy = tokens.copy() + token_changed = False + for token_name in token_copy.keys(): + for current_token_name, current_token_value in token_copy.items(): + if token_name in current_token_value: + current_token_value = current_token_value.replace(token_name, token_copy[token_name]) + tokens[current_token_name] = current_token_value + # We changed a token, so we need to restart + token_changed = True + if token_changed: + if restarts > 0: + expand_nested_tokens(tokens, restarts=restarts - 1) + else: + raise Exception("Tokens are nested too deep") + return tokens + + def _expand_tokens(elements, tokens): - if not tokens or not elements: + if not tokens or elements is None: return for element in elements: diff --git a/test/unit/tools/test_parsing.py b/test/unit/tools/test_parsing.py index 1734dadbca3..0f58c7504ef 100644 --- a/test/unit/tools/test_parsing.py +++ b/test/unit/tools/test_parsing.py @@ -41,6 +41,26 @@ TOOL_XML_1 = """ """ +TOOL_WITH_TOKEN = r""" + + + + + + + + +@NESTED_TOKEN@ + + +""" + TOOL_YAML_1 = """ name: "Bowtie Mapper" class: GalaxyTool @@ -253,6 +273,12 @@ class XmlLoaderTestCase(BaseLoaderTestCase): def test_refresh_option(self): assert self._tool_source.parse_refresh() is False + def test_nested_token(self): + tool_source = self._get_tool_source(source_contents=TOOL_WITH_TOKEN) + command = tool_source.parse_command() + assert command + assert '@' not in command + class YamlLoaderTestCase(BaseLoaderTestCase): source_file_name = "bwa.yml" From fc2bff63b320dc2a54458d1a78bc8a41a3f60879 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Sat, 27 Apr 2019 00:00:01 +0100 Subject: [PATCH 2/2] Remove recursion from ``expand_nested_tokens()`` Also add test for recursive nested token. --- lib/galaxy/util/xml_macros.py | 30 +++++++++++------------------- test/unit/tools/test_parsing.py | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py index 540b7a8cfbc..b02796080e1 100644 --- a/lib/galaxy/util/xml_macros.py +++ b/lib/galaxy/util/xml_macros.py @@ -83,21 +83,13 @@ def _macros_of_type(root, type, el_func): return macro_dict -def expand_nested_tokens(tokens, restarts=10): - token_copy = tokens.copy() - token_changed = False - for token_name in token_copy.keys(): - for current_token_name, current_token_value in token_copy.items(): +def expand_nested_tokens(tokens): + for token_name in tokens.keys(): + for current_token_name, current_token_value in tokens.items(): if token_name in current_token_value: - current_token_value = current_token_value.replace(token_name, token_copy[token_name]) - tokens[current_token_name] = current_token_value - # We changed a token, so we need to restart - token_changed = True - if token_changed: - if restarts > 0: - expand_nested_tokens(tokens, restarts=restarts - 1) - else: - raise Exception("Tokens are nested too deep") + if token_name == current_token_name: + raise Exception("Token '%s' cannot contain itself" % token_name) + tokens[current_token_name] = current_token_value.replace(token_name, tokens[token_name]) return tokens @@ -122,11 +114,11 @@ def _expand_tokens_for_el(element, tokens): _expand_tokens(list(element), tokens) -def _expand_tokens_str(str, tokens): +def _expand_tokens_str(s, tokens): for key, value in tokens.items(): - if str.find(key) > -1: - str = str.replace(key, value) - return str + if key in s: + s = s.replace(key, value) + return s def _expand_macros(elements, macros, tokens): @@ -158,7 +150,7 @@ def _expand_macro(element, expand_el, macros, tokens): # HACK for elementtree, newer implementations (etree/lxml) won't # require this parent_map data structure but elementtree does not - # track parents or recongnize .find('..'). + # track parents or recognize .find('..'). # TODO fix this now that we're not using elementtree parent_map = dict((c, p) for p in element.iter() for c in p) _xml_replace(expand_el, expanded_elements, parent_map) diff --git a/test/unit/tools/test_parsing.py b/test/unit/tools/test_parsing.py index 0f58c7504ef..1ca7308d27a 100644 --- a/test/unit/tools/test_parsing.py +++ b/test/unit/tools/test_parsing.py @@ -61,6 +61,22 @@ TOOL_WITH_TOKEN = r""" """ +TOOL_WITH_RECURSIVE_TOKEN = r""" + + + + + + +@NESTED_TOKEN@ + + +""" + TOOL_YAML_1 = """ name: "Bowtie Mapper" class: GalaxyTool @@ -279,6 +295,10 @@ class XmlLoaderTestCase(BaseLoaderTestCase): assert command assert '@' not in command + def test_recursive_token(self): + with self.assertRaises(Exception): + self._get_tool_source(source_contents=TOOL_WITH_RECURSIVE_TOKEN) + class YamlLoaderTestCase(BaseLoaderTestCase): source_file_name = "bwa.yml"