From 196e00019123ed134a7a40684c97d7545c68de75 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 10 May 2019 18:04:08 +0100 Subject: [PATCH] Fix upload of gzipped VCF files which were mistakenly sniffed as `vcf_bgzip`. Fix Fix https://github.com/galaxyproject/galaxy/issues/4892 . --- lib/galaxy/datatypes/tabular.py | 20 +++++++- lib/galaxy/datatypes/test/vcf_gzipped.vcf.gz | Bin 0 -> 392 bytes test/integration/test_datatype_upload.py | 46 ++++++++++++------- test/unit/datatypes/test_vcf.py | 25 +++++----- 4 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 lib/galaxy/datatypes/test/vcf_gzipped.vcf.gz diff --git a/lib/galaxy/datatypes/tabular.py b/lib/galaxy/datatypes/tabular.py index da06227416a..2eabeb7e732 100644 --- a/lib/galaxy/datatypes/tabular.py +++ b/lib/galaxy/datatypes/tabular.py @@ -4,6 +4,7 @@ Tabular datatype from __future__ import absolute_import import abc +import binascii import csv import logging import os @@ -691,11 +692,11 @@ class BaseVcf(Tabular): MetadataElement(name="viz_filter_cols", desc="Score column for visualization", default=[5], param=metadata.ColumnParameter, optional=True, multiple=True, visible=False) MetadataElement(name="sample_names", default=[], desc="Sample names", readonly=True, visible=False, optional=True, no_value=[]) - def sniff_prefix(self, file_prefix): + def _sniff(self, fname_or_file_prefix): # Because this sniffer is run on compressed files that might be BGZF (due to the VcfGz subclass), we should # handle unicode decode errors. This should ultimately be done in get_headers(), but guess_ext() currently # relies on get_headers() raising this exception. - headers = get_headers(file_prefix, '\n', count=1) + headers = get_headers(fname_or_file_prefix, '\n', count=1) return headers[0][0].startswith("##fileformat=VCF") def display_peek(self, dataset): @@ -745,14 +746,29 @@ class BaseVcf(Tabular): class Vcf(BaseVcf): file_ext = 'vcf' + def sniff_prefix(self, file_prefix): + return self._sniff(file_prefix) + class VcfGz(BaseVcf, binary.Binary): + # This class name is a misnomer, should be VcfBgzip file_ext = 'vcf_bgzip' compressed = True compressed_format = "gzip" MetadataElement(name="tabix_index", desc="Vcf Index File", param=metadata.FileParameter, file_ext="tbi", readonly=True, no_value=None, visible=False, optional=True) + def sniff(self, filename): + if not self._sniff(filename): + return False + # Check that the file is compressed with bgzip (not gzip), i.e. the + # compressed format is BGZF, as explained in + # http://samtools.github.io/hts-specs/SAMv1.pdf + with open(filename, 'rb') as fh: + fh.seek(-28, 2) + last28 = fh.read() + return binascii.hexlify(last28) == b'1f8b08040000000000ff0600424302001b0003000000000000000000' + def set_meta(self, dataset, **kwd): super(BaseVcf, self).set_meta(dataset, **kwd) """ Creates the index for the VCF file. """ diff --git a/lib/galaxy/datatypes/test/vcf_gzipped.vcf.gz b/lib/galaxy/datatypes/test/vcf_gzipped.vcf.gz new file mode 100644 index 0000000000000000000000000000000000000000..67e5ea92790e4a8c8443b5bb56eba0340cc1fcbb GIT binary patch literal 392 zcmV;30eAi%iwFoCOVwNe12HakV`cz_lS^-cKoo`7*Iz+oYwQeykF+!)Ae2PKBJCb^ zRFd+@KyCf^MPX>Q-mqZ-=brDJnTss%q7Sx9c^nq{yJ;WQKJCb|?fG*f(4SmOcY@yH zGhQ>U(_V1On7%4_lOQCJ|K^4?PP!!>T mQU>IH(q9U`n}X6T=fAeXLv8y*sD@rF>HGjLfQpOS1ONc}8?^}l literal 0 HcmV?d00001 diff --git a/test/integration/test_datatype_upload.py b/test/integration/test_datatype_upload.py index b743cea5075..4c7c742c534 100644 --- a/test/integration/test_datatype_upload.py +++ b/test/integration/test_datatype_upload.py @@ -6,12 +6,17 @@ import pytest from base import integration_util # noqa: I100,I202 from galaxy.datatypes.registry import Registry # noqa: I201 +from galaxy.util.checkers import ( + is_bz2, + is_gzip, + is_zip +) from galaxy.util.hash_util import md5_hash_file from .test_upload_configuration_options import BaseUploadContentConfigurationInstance SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) TEST_FILE_DIR = '%s/../../lib/galaxy/datatypes/test' % SCRIPT_DIRECTORY -TEST_DATA = collections.namedtuple('UploadDatatypesData', 'path datatype uploadable') +TestData = collections.namedtuple('UploadDatatypesData', 'path datatype uploadable') GALAXY_ROOT = os.path.abspath('%s/../../' % SCRIPT_DIRECTORY) DATATYPES_CONFIG = os.path.join(GALAXY_ROOT, 'config/datatypes_conf.xml.sample') PARENT_SNIFFER_MAP = {'fastqsolexa': 'fastq'} @@ -26,14 +31,12 @@ def find_datatype(registry, filename): raise Exception("Couldn't guess datatype for file '%s'" % filename) -def collect_test_data(): - registry = Registry() - registry.load_datatypes(root_dir=GALAXY_ROOT, config=DATATYPES_CONFIG) +def collect_test_data(registry): test_files = os.listdir(TEST_FILE_DIR) files = [os.path.join(TEST_FILE_DIR, f) for f in test_files] datatypes = [find_datatype(registry, f) for f in test_files] uploadable = [datatype.file_ext in registry.upload_file_formats for datatype in datatypes] - test_data_description = [TEST_DATA(*items) for items in zip(files, datatypes, uploadable)] + test_data_description = [TestData(*items) for items in zip(files, datatypes, uploadable)] return {os.path.basename(data.path): data for data in test_data_description} @@ -51,11 +54,18 @@ def temp_file(): yield fh -TEST_CASES = collect_test_data() +registry = Registry() +registry.load_datatypes(root_dir=GALAXY_ROOT, config=DATATYPES_CONFIG) +TEST_CASES = collect_test_data(registry) @pytest.mark.parametrize('test_data', TEST_CASES.values(), ids=list(TEST_CASES.keys())) def test_upload_datatype_auto(instance, test_data, temp_file): + is_compressed = False + for is_method in (is_bz2, is_gzip, is_zip): + is_compressed = is_method(test_data.path) + if is_compressed: + break with open(test_data.path, 'rb') as content: if hasattr(test_data.datatype, 'sniff') or 'false' in test_data.path: file_type = 'auto' @@ -75,16 +85,20 @@ def test_upload_datatype_auto(instance, test_data, temp_file): # state should be OK assert dataset['state'] == 'ok' # Check that correct datatype has been detected + file_ext = dataset['file_ext'] if 'false' in test_data.path: # datasets with false in their name are not of a specific datatype - assert dataset['file_ext'] != PARENT_SNIFFER_MAP.get(expected_file_ext, expected_file_ext) + assert file_ext != PARENT_SNIFFER_MAP.get(expected_file_ext, expected_file_ext) else: - assert dataset['file_ext'] == PARENT_SNIFFER_MAP.get(expected_file_ext, expected_file_ext) - # download file and verify it hasn't been manipulated - temp_file.write(instance.dataset_populator.get_history_dataset_content(history_id=instance.history_id, - dataset=dataset, - type='bytes', - assert_ok=False, - raw=True)) - temp_file.flush() - assert md5_hash_file(test_data.path) == md5_hash_file(temp_file.name) + assert file_ext == PARENT_SNIFFER_MAP.get(expected_file_ext, expected_file_ext) + datatype = registry.datatypes_by_extension[file_ext] + datatype_compressed = getattr(datatype, "compressed", False) + if not is_compressed or datatype_compressed: + # download file and verify it hasn't been manipulated + temp_file.write(instance.dataset_populator.get_history_dataset_content(history_id=instance.history_id, + dataset=dataset, + type='bytes', + assert_ok=False, + raw=True)) + temp_file.flush() + assert md5_hash_file(test_data.path) == md5_hash_file(temp_file.name) diff --git a/test/unit/datatypes/test_vcf.py b/test/unit/datatypes/test_vcf.py index 66537272baf..f068591f229 100644 --- a/test/unit/datatypes/test_vcf.py +++ b/test/unit/datatypes/test_vcf.py @@ -11,19 +11,22 @@ from .util import ( def test_vcf_sniff(): - vcf = Vcf() - vcf_gz = VcfGz() - with get_input_files('1.vcf_bgzip', '1.vcf') as input_files: - compressed, uncompressed = input_files - assert vcf_gz.sniff(compressed) is True - assert vcf_gz.sniff(uncompressed) is False - assert vcf.sniff(compressed) is False - assert vcf.sniff(uncompressed) is True + vcf_datatype = Vcf() + vcf_bgzip_datatype = VcfGz() + with get_input_files('1.vcf_bgzip', '1.vcf', 'vcf_gzipped.vcf.gz') as input_files: + vcf_bgzip, uncompressed, compressed = input_files + assert vcf_bgzip_datatype.sniff(vcf_bgzip) is True + assert vcf_bgzip_datatype.sniff(uncompressed) is False + assert vcf_bgzip_datatype.sniff(compressed) is False + assert vcf_datatype.sniff(vcf_bgzip) is False + assert vcf_datatype.sniff(uncompressed) is True + # Cannot sniff the vcf.gz file as vcf directly, works only when + # auto_decompress is enabled -def test_vcf_gz_set_meta(): - vcf_gz = VcfGz() +def test_vcf_bgzip_set_meta(): + vcf_bgzip_datatype = VcfGz() with get_input_files('1.vcf_bgzip') as input_files, get_dataset(input_files[0], index_attr='tabix_index') as dataset: - vcf_gz.set_meta(dataset) + vcf_bgzip_datatype.set_meta(dataset) f = pysam.VariantFile(dataset.file_name, index_filename=dataset.metadata.tabix_index.file_name) assert isinstance(f.index, pysam.libcbcf.TabixIndex) is True