From 794d1e03be57fdc0cde02bab79f8404c0eed7626 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 22 Oct 2018 17:45:15 +0200 Subject: [PATCH] Fix messed up printing of shed_tool_conf.xml file This drops xml_to_string from lib/tool_shed/util/xml_util.py and uses xml_to_string form lib/galaxy/utils/__init__.py. Also reduces plain string manipulation of xml structures. Uses xml.dom.minidom to make xml pretty if necessary. --- lib/galaxy/tools/data/__init__.py | 11 +-- lib/galaxy/util/__init__.py | 20 ++--- lib/tool_shed/capsule/capsule_manager.py | 8 +- .../galaxy_install/tools/data_manager.py | 30 +++---- .../tools/tool_panel_manager.py | 22 ++--- lib/tool_shed/metadata/metadata_generator.py | 2 +- lib/tool_shed/util/xml_util.py | 88 +++++-------------- 7 files changed, 65 insertions(+), 116 deletions(-) diff --git a/lib/galaxy/tools/data/__init__.py b/lib/galaxy/tools/data/__init__.py index 221914ff67a..6c05b60ccb1 100644 --- a/lib/galaxy/tools/data/__init__.py +++ b/lib/galaxy/tools/data/__init__.py @@ -15,6 +15,7 @@ import string import time from glob import glob from tempfile import NamedTemporaryFile +from xml.etree import ElementTree import requests @@ -185,12 +186,12 @@ class ToolDataTableManager(object): # add new elems out_elems.extend(new_elems) out_path_is_new = not os.path.exists(full_path) + + root = ElementTree.fromstring('\n') + for elem in out_elems: + root.append(elem) with RenamedTemporaryFile(full_path, mode='w') as out: - out.write('\n\n') - for elem in out_elems: - elem = util.xml_to_string(elem, pretty=True) - out.write(elem) - out.write('\n') + out.write(util.xml_to_string(root, pretty=True)) os.chmod(full_path, 0o644) if out_path_is_new: self.tool_data_path_files.update_files() diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 7c4aeb7de05..48f7094a833 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -22,6 +22,7 @@ import tempfile import threading import time import unicodedata +import xml.dom.minidom from datetime import datetime from hashlib import md5 from os.path import relpath @@ -235,19 +236,14 @@ def parse_xml_string(xml_string): def xml_to_string(elem, pretty=False): """Returns a string from an xml tree""" + if PY2: + xml_str = ElementTree.tostring(elem) + else: + xml_str = ElementTree.tostring(elem, encoding='unicode') if pretty: - elem = pretty_print_xml(elem) - try: - if PY2: - return ElementTree.tostring(elem) - else: - return ElementTree.tostring(elem, encoding='unicode') - except TypeError as e: - # we assume this is a comment - if hasattr(elem, 'text'): - return "\n" % (elem.text) - else: - raise e + pretty_string = xml.dom.minidom.parseString(xml_str).toprettyxml(indent=' ') + return "\n".join([line for line in pretty_string.split('\n') if not re.match(r'^[\s\\nb\']*$', line)]) + return xml_str def xml_element_compare(elem1, elem2): diff --git a/lib/tool_shed/capsule/capsule_manager.py b/lib/tool_shed/capsule/capsule_manager.py index 38cdd481901..d27d4e2f141 100644 --- a/lib/tool_shed/capsule/capsule_manager.py +++ b/lib/tool_shed/capsule/capsule_manager.py @@ -92,7 +92,7 @@ class ExportRepositoryManager(object): # Keep information about the export in a file named export_info.xml in the archive. sub_elements = self.generate_export_elem() export_elem = xml_util.create_element('export_info', attributes=None, sub_elements=sub_elements) - tmp_export_info = xml_util.create_and_write_tmp_file(export_elem, use_indent=True) + tmp_export_info = xml_util.create_and_write_tmp_file(export_elem) try: repositories_archive.add(tmp_export_info, arcname='export_info.xml') finally: @@ -102,7 +102,7 @@ class ExportRepositoryManager(object): exported_repository_root = xml_util.create_element('repositories') for exported_repository_elem in exported_repository_registry.exported_repository_elems: exported_repository_root.append(exported_repository_elem) - tmp_manifest = xml_util.create_and_write_tmp_file(exported_repository_root, use_indent=True) + tmp_manifest = xml_util.create_and_write_tmp_file(exported_repository_root) try: repositories_archive.add(tmp_manifest, arcname='manifest.xml') finally: @@ -165,7 +165,7 @@ class ExportRepositoryManager(object): if error_message: return None, error_message if altered: - tmp_filename = xml_util.create_and_write_tmp_file(root_elem, use_indent=True) + tmp_filename = xml_util.create_and_write_tmp_file(root_elem) shutil.move(tmp_filename, full_path) elif name == rt_util.TOOL_DEPENDENCY_DEFINITION_FILENAME: # Eliminate the toolshed, and changeset_revision attributes from all tags. @@ -173,7 +173,7 @@ class ExportRepositoryManager(object): if error_message: return None, error_message if altered: - tmp_filename = xml_util.create_and_write_tmp_file(root_elem, use_indent=True) + tmp_filename = xml_util.create_and_write_tmp_file(root_elem) shutil.move(tmp_filename, full_path) repository_archive.add(full_path, arcname=relative_path) repository_archive.close() diff --git a/lib/tool_shed/galaxy_install/tools/data_manager.py b/lib/tool_shed/galaxy_install/tools/data_manager.py index 6793a9afd2d..d27f4f6643d 100644 --- a/lib/tool_shed/galaxy_install/tools/data_manager.py +++ b/lib/tool_shed/galaxy_install/tools/data_manager.py @@ -1,8 +1,9 @@ import logging import os -import threading import time +from xml.etree import ElementTree +from galaxy.util import xml_to_string from galaxy.util.renamed_temporary_file import RenamedTemporaryFile from tool_shed.galaxy_install.tools import tool_panel_manager from tool_shed.util import xml_util @@ -29,21 +30,18 @@ class DataManagerHandler(object): of config_filename. """ data_managers_path = self.data_managers_path - lock = threading.Lock() - lock.acquire(True) + if data_managers_path: + root_str = '' % data_managers_path + else: + root_str = '' + root = ElementTree.fromstring(root_str) + for elem in config_elems: + root.append(elem) try: with RenamedTemporaryFile(config_filename, mode='w') as fh: - if data_managers_path is not None: - fh.write('\n\n ' % data_managers_path) - else: - fh.write('\n\n ') - for elem in config_elems: - fh.write(xml_util.xml_to_string(elem)) - fh.write('\n') - except Exception: - log.exception("Exception in DataManagerHandler.data_manager_config_elems_to_xml_file") - finally: - lock.release() + fh.write(xml_to_string(root)) + except Exception as e: + log.exception("Exception in DataManagerHandler.data_manager_config_elems_to_xml_file:\n %s", str(e)) def install_data_managers(self, shed_data_manager_conf_filename, metadata_dict, shed_config_dict, relative_install_dir, repository, repository_tools_tups): @@ -74,7 +72,7 @@ class DataManagerHandler(object): data_manager_id = elem.get('id', None) if data_manager_id is None: log.error("A data manager was defined that does not have an id and will not be installed:\n%s" % - xml_util.xml_to_string(elem)) + xml_to_string(elem)) continue data_manager_dict = metadata_dict['data_manager'].get('data_managers', {}).get(data_manager_id, None) if data_manager_dict is None: @@ -120,7 +118,7 @@ class DataManagerHandler(object): if data_manager: rval.append(data_manager) else: - log.warning("Encountered unexpected element '%s':\n%s" % (elem.tag, xml_util.xml_to_string(elem))) + log.warning("Encountered unexpected element '%s':\n%s" % (elem.tag, xml_to_string(elem))) config_elems.append(elem) data_manager_config_has_changes = True # Persist the altered shed_data_manager_config file. diff --git a/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py b/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py index 40f4899e2bd..47292749a29 100644 --- a/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py +++ b/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py @@ -1,7 +1,8 @@ import logging -import threading -from xml.etree import cElementTree as XmlET +from xml.etree import ElementTree as XmlET +from galaxy.util import xml_to_string +from galaxy.util.renamed_temporary_file import RenamedTemporaryFile from tool_shed.util import basic_util from tool_shed.util import common_util from tool_shed.util import repository_util @@ -79,19 +80,14 @@ class ToolPanelManager(object): Persist the current in-memory list of config_elems to a file named by the value of config_filename. """ - lock = threading.Lock() - lock.acquire(True) try: - fh = open(config_filename, 'w') - fh.write('\n\n' % str(tool_path)) + root = XmlET.fromstring('\n' % str(tool_path)) for elem in config_elems: - fh.write(xml_util.xml_to_string(elem, use_indent=True)) - fh.write('\n') - fh.close() - except Exception: - log.exception("Exception in ToolPanelManager.config_elems_to_xml_file") - finally: - lock.release() + root.append(elem) + with RenamedTemporaryFile(config_filename, mode='w') as fh: + fh.write(xml_to_string(root, pretty=True)) + except Exception as e: + log.exception("Exception in ToolPanelManager.config_elems_to_xml_file: \n %s", str(e)) def generate_tool_elem(self, tool_shed, repository_name, changeset_revision, owner, tool_file_path, tool, tool_section): diff --git a/lib/tool_shed/metadata/metadata_generator.py b/lib/tool_shed/metadata/metadata_generator.py index 66bce84a8de..6ab82b82761 100644 --- a/lib/tool_shed/metadata/metadata_generator.py +++ b/lib/tool_shed/metadata/metadata_generator.py @@ -886,7 +886,7 @@ class MetadataGenerator(object): # installation from proceeding. Reaching here implies a bug in the Tool Shed # framework. error_message = 'Installation encountered an invalid repository dependency definition:\n' - error_message += xml_util.xml_to_string(repository_elem, use_indent=True) + error_message += util.xml_to_string(repository_elem, pretty=True) log.error(error_message) return repository_dependency_tup, False, error_message if not toolshed: diff --git a/lib/tool_shed/util/xml_util.py b/lib/tool_shed/util/xml_util.py index 451d2e2f939..55e414b3fc7 100644 --- a/lib/tool_shed/util/xml_util.py +++ b/lib/tool_shed/util/xml_util.py @@ -2,10 +2,12 @@ import logging import os import sys import tempfile -import xml.etree.ElementTree from xml.etree import ElementTree as XmlET -from galaxy.util import listify +from galaxy.util import ( + listify, + xml_to_string +) log = logging.getLogger(__name__) using_python_27 = sys.version_info[:2] >= (2, 7) @@ -20,17 +22,16 @@ class Py27CommentedTreeBuilder(XmlET.TreeBuilder): self.end(XmlET.Comment) -def create_and_write_tmp_file(elems, use_indent=False): +def create_and_write_tmp_file(elems): tmp_str = '' for elem in listify(elems): - tmp_str += xml_to_string(elem, use_indent=use_indent) + tmp_str += xml_to_string(elem, pretty=True) fh = tempfile.NamedTemporaryFile('wb', prefix="tmp-toolshed-cawrf") tmp_filename = fh.name fh.close() - fh = open(tmp_filename, 'wb') - fh.write('\n') - fh.write(tmp_str) - fh.close() + with open(tmp_filename, 'w') as fh: + fh.write('\n') + fh.write(tmp_str) return tmp_filename @@ -78,68 +79,25 @@ def create_element(tag, attributes=None, sub_elements=None): return None -def indent(elem, level=0): - """ - Prints an XML tree with each node indented according to its depth. This method is used to print the - shed tool config (e.g., shed_tool_conf.xml from the in-memory list of config_elems because each config_elem - in the list may be a hierarchical structure that was not created using the parse_xml() method below, - and so will not be properly written with xml.etree.ElementTree.tostring() without manually indenting - the tree first. - """ - i = "\n" + level * " " - if len(elem): - if not elem.text or not elem.text.strip(): - elem.text = i + " " - if not elem.tail or not elem.tail.strip(): - elem.tail = i - for child in elem: - indent(child, level + 1) - if not child.tail or not child.tail.strip(): - child.tail = i - if not elem.tail or not elem.tail.strip(): - elem.tail = i - else: - if level and (not elem.tail or not elem.tail.strip()): - elem.tail = i - - def parse_xml(file_name): """Returns a parsed xml tree with comments intact.""" error_message = '' if not os.path.exists(file_name): return None, "File does not exist %s" % str(file_name) - fobj = open(file_name, 'r') - if using_python_27: - try: - tree = XmlET.parse(fobj, parser=XmlET.XMLParser(target=Py27CommentedTreeBuilder())) - except Exception as e: - fobj.close() - error_message = "Exception attempting to parse %s: %s" % (str(file_name), str(e)) - log.exception(error_message) - return None, error_message - else: - try: - tree = XmlET.parse(fobj) - except Exception as e: - fobj.close() - error_message = "Exception attempting to parse %s: %s" % (str(file_name), str(e)) - log.exception(error_message) - return None, error_message - fobj.close() - return tree, error_message - - -def xml_to_string(elem, encoding='utf-8', use_indent=False, level=0): - if elem is not None: - if use_indent: - # We were called from ToolPanelManager.config_elems_to_xml_file(), so - # set the level to 1 since level 0 is the tag set. - indent(elem, level=level) + with open(file_name, 'r') as fobj: if using_python_27: - xml_str = '%s\n' % xml.etree.ElementTree.tostring(elem, encoding=encoding, method="xml") + try: + tree = XmlET.parse(fobj, parser=XmlET.XMLParser(target=Py27CommentedTreeBuilder())) + except Exception as e: + error_message = "Exception attempting to parse %s: %s" % (str(file_name), str(e)) + log.exception(error_message) + return None, error_message else: - xml_str = '%s\n' % xml.etree.ElementTree.tostring(elem, encoding=encoding) - else: - xml_str = '' - return xml_str + try: + tree = XmlET.parse(fobj) + except Exception as e: + error_message = "Exception attempting to parse %s: %s" % (str(file_name), str(e)) + log.exception(error_message) + return None, error_message + return tree, error_message