Preserve more context for header mergers that fail due to version val…

…idation.
samtools · Jun 5, 2023 · 0d625b7 · 0d625b7
1 parent 0f215ec
commit 0d625b7
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 18 deletions.
diff --git a/src/main/java/htsjdk/variant/vcf/VCFHeader.java b/src/main/java/htsjdk/variant/vcf/VCFHeader.java
@@ -662,8 +662,12 @@ public VCFHeader upgradeVersion(final VCFVersionUpgradePolicy policy) {
                     newHeader.addMetaDataLine(VCFHeader.makeHeaderVersionLine(VCFHeader.DEFAULT_VCF_VERSION));
                     return newHeader;
                 } else {
-                    logger.info("Header will be kept at original version: " + this.getVCFHeaderVersion()
-                        + VCFValidationFailure.createVersionTransitionErrorMessage(errors, this.getVCFHeaderVersion())
+                    logger.info("Header will be kept at original version: ",
+                            this.getVCFHeaderVersion(),
+                            VCFValidationFailure.createVersionTransitionErrorMessage(
+                                    errors,
+                                    "Upgrading to: " + this.getVCFHeaderVersion().getVersionString(),
+                                    VCFHeader.DEFAULT_VCF_VERSION)
                     );
                     return this;
                 }

diff --git a/src/main/java/htsjdk/variant/vcf/VCFHeaderMerger.java b/src/main/java/htsjdk/variant/vcf/VCFHeaderMerger.java
@@ -123,12 +123,22 @@ private static Set<VCFHeaderLine> makeMergedMetaDataSet(
             final SAMSequenceDictionary commonSequenceDictionary,
             final HeaderMergeConflictWarnings conflictWarner) {
 
-        if (conflictWarner.emitWarnings) {
-            //TODO: any header contains a line that fails version validation, then the merge should fail...just like
-            // a version upgrade would fail for that same header. We can't honor the fallback policy i.e., fallback to
-            // the old version) here because that would require knowing how to back-version the OTHER headers being merged
-            mergedMetaData.getVersionValidationFailures(newestVersion)
-                    .forEach(validationError -> conflictWarner.warn(validationError.getFailureMessage()));
+        final Collection<VCFValidationFailure<VCFHeaderLine>> vcfValidationFailures =
+                mergedMetaData.getVersionValidationFailures(newestVersion);
+        if (!vcfValidationFailures.isEmpty()) {
+            // if any header contains a line that fails version validation against the target version (the newest
+            // version amongst all headers in the merge group), the merge should fail just like a version upgrade
+            // would fail for that same header. We can't honor the fallback policy here, i.e., fallback to an older
+            // version, because that would require knowing how to back-version the headers in the merge group that
+            // already have the target version.
+            final String userMessage = VCFValidationFailure.createVersionTransitionErrorMessage(
+                    vcfValidationFailures,
+                    "merging multiple headers",
+                    newestVersion);
+            if (conflictWarner.emitWarnings) {
+                conflictWarner.warn(userMessage);
+            }
+            throw new TribbleException.VersionValidationFailure(userMessage);
         }
 
         final Set<VCFHeaderLine> mergedLines = VCFHeader.makeHeaderVersionLineSet(newestVersion);

diff --git a/src/main/java/htsjdk/variant/vcf/VCFValidationFailure.java b/src/main/java/htsjdk/variant/vcf/VCFValidationFailure.java
@@ -66,22 +66,27 @@ public VCFHeaderVersion getTargetVersion() {
     /**
      * Create a formatted error message for a collection of version transition errors.
      * @param errors the errors to format
-     * @param originalVersion the original version of the VCF object before the attempted transition
+     * @param contextMessage a String describing the context in which the upgrade failed (for error reporting)
+     * @param targetVersion the target version for which errors are reported
      * @param <T> the type of the {@link VCFValidationFailure}
      * @return formatted string
      */
     public static <T> String createVersionTransitionErrorMessage(
         final Collection<VCFValidationFailure<T>> errors,
-        final VCFHeaderVersion originalVersion
+        final String contextMessage,
+        final VCFHeaderVersion targetVersion
     ) {
         return String.format(
-            "Version transition from VCF version %s to %s failed with validation error(s):\n%s%s",
-            originalVersion.getVersionString(), VCFHeader.DEFAULT_VCF_VERSION.getVersionString(),
-            errors.stream()
-                .limit(5)
-                .map(VCFValidationFailure::getSourceMessage)
-                .collect(Collectors.joining("\n")),
-            errors.size() > 5 ? "\n+ " + (errors.size() - 5) + " additional error(s)" : ""
+            "Version transition for \"%s\" with target VCF header version \"%s\" failed with validation error(s):\n%s%s\n",
+                contextMessage,
+                targetVersion.getVersionString(),
+                errors.stream()
+                    .limit(5)
+                    .map(VCFValidationFailure::getSourceMessage)
+                    .collect(Collectors.joining("\n")),
+                errors.size() > 5 ?
+                        "\n+ " + (errors.size() - 5) + " additional error(s)" :
+                        ""
         );
     }
 

diff --git a/src/test/java/htsjdk/variant/vcf/VCFHeaderMergerUnitTest.java b/src/test/java/htsjdk/variant/vcf/VCFHeaderMergerUnitTest.java
@@ -107,7 +107,7 @@ public void testMergeInvalidVersions(final List<VCFHeaderVersion> headerVersions
         doHeaderMergeForVersions(headerVersions);
     }
 
-    @Test(expectedExceptions = TribbleException.class)
+    @Test(expectedExceptions = TribbleException.VersionValidationFailure.class)
     public void testMergeWithValidationFailure() {
         // test mixing header versions where the old version header has a line that fails validation
         // using the resulting (newer) version