Reload tools if a macro changes

We do this by recording any macros that are required to load a tool.  When
loading a tool we register the macro to be watched, and if the macro changes we
reload all corresponding tools.

lib/galaxy/jobs/__init__.py

@@ -156,7 +156,7 @@ def __init__(self, app):
         # Initialize the config
         job_config_file = self.app.config.job_config_file
         try:
-            tree = load(job_config_file)
+            tree, _ = load(job_config_file)
             self.__parse_job_conf_xml(tree)
         except IOError:
             log.warning('Job configuration "%s" does not exist, using legacy'

lib/galaxy/tools/__init__.py

@@ -720,6 +720,8 @@ def parse(self, tool_source, guid=None):
         # Determine if this tool can be used in workflows
         self.is_workflow_compatible = self.check_workflow_compatible(tool_source)
         self.__parse_trackster_conf(tool_source)
+        # Record macro paths so we can reload a tool if any of its macro has changes
+        self._macro_paths = tool_source._macro_paths
 
     def __parse_legacy_features(self, tool_source):
         self.code_namespace = dict()

lib/galaxy/tools/cache.py

@@ -16,6 +16,8 @@ def __init__(self):
         self._hash_by_tool_paths = {}
         self._tools_by_path = {}
         self._tool_paths_by_id = {}
+        self._macro_paths_by_id = {}
+        self._tool_ids_by_macro_paths = {}
         self._mod_time_by_path = {}
         self._new_tool_ids = set()
         self._removed_tool_ids = set()
@@ -54,6 +56,11 @@ def _should_cleanup(self, config_filename):
         if self._mod_time_by_path.get(config_filename) < new_mtime:
             if md5_hash_file(config_filename) != self._hash_by_tool_paths.get(config_filename):
                 return True
+        tool = self._tools_by_path[config_filename]
+        for macro_path in tool._macro_paths:
+            new_mtime = os.path.getmtime(macro_path)
+            if self._mod_time_by_path.get(macro_path) < new_mtime:
+                return True
         return False
 
     def get_tool(self, config_filename):
@@ -82,6 +89,16 @@ def cache_tool(self, config_filename, tool):
         self._tool_paths_by_id[tool_id] = config_filename
         self._tools_by_path[config_filename] = tool
         self._new_tool_ids.add(tool_id)
+        for macro_path in tool._macro_paths:
+            self._mod_time_by_path[macro_path] = os.path.getmtime(macro_path)
+            if tool_id not in self._macro_paths_by_id:
+                self._macro_paths_by_id[tool_id] = {macro_path}
+            else:
+                self._macro_paths_by_id[tool_id].add(macro_path)
+            if macro_path not in self._macro_paths_by_id:
+                self._tool_ids_by_macro_paths[macro_path] = {tool_id}
+            else:
+                self._tool_ids_by_macro_paths[macro_path].add(tool_id)
 
     def reset_status(self):
         """Reset self._new_tool_ids and self._removed_tool_ids once

lib/galaxy/tools/parser/factory.py

@@ -34,8 +34,8 @@ def get_tool_source(config_file=None, xml_tree=None, enable_beta_formats=True, t
 
     config_file = tool_location_fetcher.to_tool_path(config_file)
     if not enable_beta_formats:
-        tree = load_tool_xml(config_file)
-        return XmlToolSource(tree, source_path=config_file)
+        tree, macro_paths = load_tool_xml(config_file)
+        return XmlToolSource(tree, source_path=config_file, macro_paths=macro_paths)
 
     if config_file.endswith(".yml"):
         log.info("Loading tool from YAML - this is experimental - tool will not function in future.")
@@ -46,8 +46,8 @@ def get_tool_source(config_file=None, xml_tree=None, enable_beta_formats=True, t
         log.info("Loading CWL tool - this is experimental - tool likely will not function in future at least in same way.")
         return CwlToolSource(config_file)
     else:
-        tree = load_tool_xml(config_file)
-        return XmlToolSource(tree, source_path=config_file)
+        tree, macro_paths = load_tool_xml(config_file)
+        return XmlToolSource(tree, source_path=config_file, macro_paths=macro_paths)
 
 
 def ordered_load(stream):

lib/galaxy/tools/parser/xml.py

@@ -40,10 +40,11 @@ class XmlToolSource(ToolSource):
     """ Responsible for parsing a tool from classic Galaxy representation.
     """
 
-    def __init__(self, xml_tree, source_path=None):
+    def __init__(self, xml_tree, source_path=None, macro_paths=None):
         self.xml_tree = xml_tree
         self.root = xml_tree.getroot()
         self._source_path = source_path
+        self._macro_paths = macro_paths or []
         self.legacy_defaults = self.parse_profile() == "16.01"
 
     def parse_version(self):

lib/galaxy/tools/parser/yaml.py

@@ -21,6 +21,7 @@ class YamlToolSource(ToolSource):
     def __init__(self, root_dict, source_path=None):
         self.root_dict = root_dict
         self._source_path = source_path
+        self._macro_paths = []
 
     def parse_id(self):
         return self.root_dict.get("id")

lib/galaxy/tools/toolbox/base.py

@@ -73,9 +73,11 @@ def __init__(self, config_filenames, tool_root_dir, app):
         self.app = app
         if hasattr(self.app, 'watchers'):
             self._tool_watcher = self.app.watchers.tool_watcher
+            self._tool_config_watcher = self.app.watchers.tool_config_watcher
         else:
             # Toolbox is loaded but not used during toolshed tests
             self._tool_watcher = None
+            self._tool_config_watcher = None
         self._filter_factory = FilterFactory(self)
         self._tool_tag_manager = tool_tag_manager(app)
         self._init_tools_from_configs(config_filenames)
@@ -745,10 +747,13 @@ def load_tool(self, config_file, guid=None, repository_id=None, use_cached=False
             tool = self.create_tool(config_file=config_file, repository_id=repository_id, guid=guid, **kwds)
             if tool.tool_shed_repository or not guid:
                 self.add_tool_to_cache(tool, config_file)
-        if not tool.id.startswith("__") and self._tool_watcher:
+        if not tool.id.startswith("__"):
             # do not monitor special tools written to tmp directory - no reason
             # to monitor such a large directory.
-            self._tool_watcher.watch_file(config_file, tool.id)
+            if self._tool_watcher:
+                self._tool_watcher.watch_file(config_file, tool.id)
+            if self._tool_config_watcher:
+                [self._tool_config_watcher.watch_file(macro_path) for macro_path in tool._macro_paths]
         return tool
 
     def add_tool_to_cache(self, tool, config_file):

lib/galaxy/util/xml_macros.py

@@ -14,7 +14,7 @@ def load(path):
     tree = raw_tool_xml_tree(path)
     root = tree.getroot()
 
-    _import_macros(root, path)
+    macro_paths = _import_macros(root, path)
 
     # Collect tokens
     tokens = _macros_of_type(root, 'token', lambda el: el.text or '')
@@ -23,7 +23,7 @@ def load(path):
     macro_dict = _macros_of_type(root, 'xml', lambda el: XmlMacroDef(el))
     _expand_macros([root], macro_dict, tokens)
 
-    return tree
+    return tree, macro_paths
 
 
 def template_macro_params(root):
@@ -55,8 +55,9 @@ def _import_macros(root, path):
     tool_dir = os.path.dirname(path)
     macros_el = _macros_el(root)
     if macros_el is not None:
-        macro_els = _load_macros(macros_el, tool_dir)
+        macro_els, macro_paths = _load_macros(macros_el, tool_dir)
         _xml_set_children(macros_el, macro_els)
+        return macro_paths
 
 
 def _macros_el(root):
@@ -165,10 +166,11 @@ def _expand_yield_statements(macro_def, expand_el):
 def _load_macros(macros_el, tool_dir):
     macros = []
     # Import macros from external files.
-    macros.extend(_load_imported_macros(macros_el, tool_dir))
+    imported_macros, macro_paths = _load_imported_macros(macros_el, tool_dir)
+    macros.extend(imported_macros)
     # Load all directly defined macros.
     macros.extend(_load_embedded_macros(macros_el, tool_dir))
-    return macros
+    return macros, macro_paths
 
 
 def _load_embedded_macros(macros_el, tool_dir):
@@ -200,14 +202,17 @@ def _load_embedded_macros(macros_el, tool_dir):
 
 def _load_imported_macros(macros_el, tool_dir):
     macros = []
+    macro_paths = []
 
     for tool_relative_import_path in _imported_macro_paths_from_el(macros_el):
         import_path = \
             os.path.join(tool_dir, tool_relative_import_path)
-        file_macros = _load_macro_file(import_path, tool_dir)
+        macro_paths.append(import_path)
+        file_macros, current_macro_paths = _load_macro_file(import_path, tool_dir)
         macros.extend(file_macros)
+        macro_paths.extend(current_macro_paths)
 
-    return macros
+    return macros, macro_paths
 
 
 def _imported_macro_paths_from_el(macros_el):

scripts/validate_tools.sh

@@ -21,7 +21,7 @@ for p in "$@"; do
     echo $path
     PYTHONPATH=lib:$PYTHONPATH
     export PYTHONPATH
-    result=`python -c "import galaxy.tools.loader; import xml.etree; xml.etree.ElementTree.dump(galaxy.tools.loader.load_tool('$path').getroot())" | xmllint --nowarning --noout --schema "$xsd_path" - 2> "$err_tmp"`
+    result=`python -c "import galaxy.tools.loader; import xml.etree; xml.etree.ElementTree.dump(galaxy.tools.loader.load_tool('$path')[0].getroot())" | xmllint --nowarning --noout --schema "$xsd_path" - 2> "$err_tmp"`
     if [ $? -eq 0 ]
     then
         echo "ok $count";

test/unit/tools/test_tool_loader.py

@@ -1,12 +1,30 @@
 import os
 
 from shutil import rmtree
+from string import Template
 from tempfile import mkdtemp
 
 from galaxy.tools.loader import load_tool, template_macro_params
 from galaxy.util import parse_xml
 
 
+SIMPLE_TOOL_WITH_MACRO = """<tool id="tool_with_macro" name="macro_annotation" version="@WRAPPER_VERSION@">
+    <expand macro="inputs" />
+    <macros>
+        <import>external.xml</import>
+    </macros>
+</tool>"""
+
+SIMPLE_MACRO = Template("""
+<macros>
+    <token name="@WRAPPER_VERSION@">$tool_version</token>
+    <macro name="inputs">
+        <inputs/>
+    </macro>
+</macros>
+""")
+
+
 def test_loader():
 
     class TestToolDirectory(object):
@@ -23,11 +41,11 @@ def write(self, contents, name="tool.xml"):
             open(os.path.join(self.temp_directory, name), "w").write(contents)
 
         def load(self, name="tool.xml", preprocess=True):
+            path = os.path.join(self.temp_directory, name)
             if preprocess:
-                loader = load_tool
+                return load_tool(path)[0]
             else:
-                loader = parse_xml
-            return loader(os.path.join(self.temp_directory, name))
+                return parse_xml(path)
 
     # Test simple macro replacement.
     with TestToolDirectory() as tool_dir:
@@ -47,20 +65,9 @@ def load(self, name="tool.xml", preprocess=True):
 
     # Test importing macros from external files
     with TestToolDirectory() as tool_dir:
-        tool_dir.write('''
-<tool>
-    <expand macro="inputs" />
-    <macros>
-        <import>external.xml</import>
-    </macros>
-</tool>''')
+        tool_dir.write(SIMPLE_TOOL_WITH_MACRO)
 
-        tool_dir.write('''
-<macros>
-    <macro name="inputs">
-        <inputs />
-    </macro>
-</macros>''', name="external.xml")
+        tool_dir.write(SIMPLE_MACRO.substitute(tool_version="2.0"), name="external.xml")
         xml = tool_dir.load(preprocess=False)
         assert xml.find("inputs") is None
         xml = tool_dir.load(preprocess=True)

test/unit/tools/test_toolbox.py

@@ -1,6 +1,7 @@
 import json
 import os
 import string
+import time
 import unittest
 
 import routes
@@ -12,10 +13,13 @@
 from galaxy.model.tool_shed_install import mapping
 from galaxy.tools import ToolBox
 from galaxy.tools.cache import ToolCache
-from galaxy.tools.toolbox.watcher import get_tool_conf_watcher
 from galaxy.webapps.galaxy.config_watchers import ConfigWatchers
 
 from .test_toolbox_filters import mock_trans
+from .test_tool_loader import (
+    SIMPLE_MACRO,
+    SIMPLE_TOOL_WITH_MACRO
+)
 
 
 CONFIG_TEST_TOOL_VERSION_TEMPLATE = string.Template(
@@ -163,6 +167,40 @@ def test_load_file(self):
         assert toolbox.get_tool("test_tool") is not None
         assert toolbox.get_tool("not_a_test_tool") is None
 
+    def test_record_macros(self):
+        self._init_tool()
+        self._init_tool(filename="tool_with_macro.xml",
+                        tool_contents=SIMPLE_TOOL_WITH_MACRO,
+                        extra_file_contents=SIMPLE_MACRO.substitute(tool_version="2.0"),
+                        extra_file_path="external.xml")
+        self._add_config("""<toolbox><tool file="tool_with_macro.xml"/><tool file="tool.xml" /></toolbox>""")
+        toolbox = self.toolbox
+        tool = toolbox.get_tool("test_tool")
+        assert tool is not None
+        assert len(tool._macro_paths) == 0
+        tool = toolbox.get_tool("tool_with_macro")
+        assert tool is not None
+        assert len(tool._macro_paths) == 1
+
+    def test_tool_reload_when_macro_is_altered(self):
+        self._init_tool(filename="tool_with_macro.xml",
+                        tool_contents=SIMPLE_TOOL_WITH_MACRO,
+                        extra_file_contents=SIMPLE_MACRO.substitute(tool_version="2.0"),
+                        extra_file_path="external.xml")
+        self._add_config("""<toolbox><tool file="tool_with_macro.xml"/></toolbox>""")
+        toolbox = self.toolbox
+        tool = toolbox.get_tool("tool_with_macro")
+        assert tool is not None
+        assert len(tool._macro_paths) == 1
+        macro_path = tool._macro_paths[0]
+        time.sleep(1.5)
+        with open(macro_path, 'w') as macro_out:
+            macro_out.write(SIMPLE_MACRO.substitute(tool_version="3.0"))
+        time.sleep(1.5)
+        tool = self.app.toolbox.get_tool("tool_with_macro")
+
+        assert tool.version == "3.0"
+
     def test_enforce_tool_profile(self):
         self._init_tool(filename="old_tool.xml", version="1.0", profile="17.01", tool_id="test_old_tool_profile")
         self._init_tool(filename="new_tool.xml", version="2.0", profile="27.01", tool_id="test_new_tool_profile")
@@ -444,8 +482,9 @@ class SimplifiedToolBox(ToolBox):
 
     def __init__(self, test_case):
         app = test_case.app
+        app.watchers.tool_config_watcher.reload_callback = lambda: reload_callback(test_case)
         # Handle app/config stuff needed by toolbox but not by tools.
-        app.tool_cache = ToolCache()
+        app.tool_cache = ToolCache() if not hasattr(app, 'tool_cache') else app.tool_cache
         app.job_config.get_tool_resource_parameters = lambda tool_id: None
         app.config.update_integrated_tool_panel = True
         config_files = test_case.config_files
@@ -455,11 +494,11 @@ def __init__(self, test_case):
             tool_root_dir,
             app,
         )
-        self._tool_conf_watcher = get_tool_conf_watcher(dummy_callback)
 
     def handle_panel_update(self, section_dict):
         self.create_section(section_dict)
 
 
-def dummy_callback():
-    pass
+def reload_callback(test_case):
+    test_case.app.tool_cache.cleanup()
+    test_case.__toolbox = test_case.app.toolbox = SimplifiedToolBox(test_case)

test/unit/tools_support.py

@@ -75,12 +75,16 @@ def _init_tool(
         version="1.0",
         profile="16.01",
         tool_id="test_tool",
+        extra_file_contents=None,
+        extra_file_path=None,
     ):
         self._init_app_for_tools()
         self.tool_file = os.path.join(self.test_directory, filename)
         contents_template = string.Template(tool_contents)
         tool_contents = contents_template.safe_substitute(dict(version=version, profile=profile, tool_id=tool_id))
         self.__write_tool(tool_contents)
+        if extra_file_contents and extra_file_path:
+            self.__write_tool(extra_file_contents, path=os.path.join(self.test_directory, extra_file_path))
         return self.__setup_tool()
 
     def _init_app_for_tools(self):
@@ -99,8 +103,9 @@ def __setup_tool(self):
             self.tool.tool_action = self.tool_action
         return self.tool
 
-    def __write_tool(self, contents):
-        open(self.tool_file, "w").write(contents)
+    def __write_tool(self, contents, path=None):
+        path = path or self.tool_file
+        open(path, "w").write(contents)
 
 
 class MockContext(object):