Skip to content

Commit e54f282

Browse files
committed
Preserve more context for header mergers that fail due to version validation.
1 parent 73fc4d4 commit e54f282

File tree

4 files changed

+37
-18
lines changed

4 files changed

+37
-18
lines changed

src/main/java/htsjdk/variant/vcf/VCFHeader.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -662,8 +662,12 @@ public VCFHeader upgradeVersion(final VCFVersionUpgradePolicy policy) {
662662
newHeader.addMetaDataLine(VCFHeader.makeHeaderVersionLine(VCFHeader.DEFAULT_VCF_VERSION));
663663
return newHeader;
664664
} else {
665-
logger.info("Header will be kept at original version: " + this.getVCFHeaderVersion()
666-
+ VCFValidationFailure.createVersionTransitionErrorMessage(errors, this.getVCFHeaderVersion())
665+
logger.info("Header will be kept at original version: ",
666+
this.getVCFHeaderVersion(),
667+
VCFValidationFailure.createVersionTransitionErrorMessage(
668+
errors,
669+
"Upgrading to: " + this.getVCFHeaderVersion().getVersionString(),
670+
VCFHeader.DEFAULT_VCF_VERSION)
667671
);
668672
return this;
669673
}

src/main/java/htsjdk/variant/vcf/VCFHeaderMerger.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,22 @@ private static Set<VCFHeaderLine> makeMergedMetaDataSet(
123123
final SAMSequenceDictionary commonSequenceDictionary,
124124
final HeaderMergeConflictWarnings conflictWarner) {
125125

126-
if (conflictWarner.emitWarnings) {
127-
//TODO: any header contains a line that fails version validation, then the merge should fail...just like
128-
// a version upgrade would fail for that same header. We can't honor the fallback policy i.e., fallback to
129-
// the old version) here because that would require knowing how to back-version the OTHER headers being merged
130-
mergedMetaData.getVersionValidationFailures(newestVersion)
131-
.forEach(validationError -> conflictWarner.warn(validationError.getFailureMessage()));
126+
final Collection<VCFValidationFailure<VCFHeaderLine>> vcfValidationFailures =
127+
mergedMetaData.getVersionValidationFailures(newestVersion);
128+
if (!vcfValidationFailures.isEmpty()) {
129+
// if any header contains a line that fails version validation against the target version (the newest
130+
// version amongst all headers in the merge group), the merge should fail just like a version upgrade
131+
// would fail for that same header. We can't honor the fallback policy here, i.e., fallback to an older
132+
// version, because that would require knowing how to back-version the headers in the merge group that
133+
// already have the target version.
134+
final String userMessage = VCFValidationFailure.createVersionTransitionErrorMessage(
135+
vcfValidationFailures,
136+
"merging multiple headers",
137+
newestVersion);
138+
if (conflictWarner.emitWarnings) {
139+
conflictWarner.warn(userMessage);
140+
}
141+
throw new TribbleException.VersionValidationFailure(userMessage);
132142
}
133143

134144
final Set<VCFHeaderLine> mergedLines = VCFHeader.makeHeaderVersionLineSet(newestVersion);

src/main/java/htsjdk/variant/vcf/VCFValidationFailure.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,22 +66,27 @@ public VCFHeaderVersion getTargetVersion() {
6666
/**
6767
* Create a formatted error message for a collection of version transition errors.
6868
* @param errors the errors to format
69-
* @param originalVersion the original version of the VCF object before the attempted transition
69+
* @param contextMessage a String describing the context in which the upgrade failed (for error reporting)
70+
* @param targetVersion the target version for which errors are reported
7071
* @param <T> the type of the {@link VCFValidationFailure}
7172
* @return formatted string
7273
*/
7374
public static <T> String createVersionTransitionErrorMessage(
7475
final Collection<VCFValidationFailure<T>> errors,
75-
final VCFHeaderVersion originalVersion
76+
final String contextMessage,
77+
final VCFHeaderVersion targetVersion
7678
) {
7779
return String.format(
78-
"Version transition from VCF version %s to %s failed with validation error(s):\n%s%s",
79-
originalVersion.getVersionString(), VCFHeader.DEFAULT_VCF_VERSION.getVersionString(),
80-
errors.stream()
81-
.limit(5)
82-
.map(VCFValidationFailure::getSourceMessage)
83-
.collect(Collectors.joining("\n")),
84-
errors.size() > 5 ? "\n+ " + (errors.size() - 5) + " additional error(s)" : ""
80+
"Version transition for \"%s\" with target VCF header version \"%s\" failed with validation error(s):\n%s%s\n",
81+
contextMessage,
82+
targetVersion.getVersionString(),
83+
errors.stream()
84+
.limit(5)
85+
.map(VCFValidationFailure::getSourceMessage)
86+
.collect(Collectors.joining("\n")),
87+
errors.size() > 5 ?
88+
"\n+ " + (errors.size() - 5) + " additional error(s)" :
89+
""
8590
);
8691
}
8792

src/test/java/htsjdk/variant/vcf/VCFHeaderMergerUnitTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public void testMergeInvalidVersions(final List<VCFHeaderVersion> headerVersions
107107
doHeaderMergeForVersions(headerVersions);
108108
}
109109

110-
@Test(expectedExceptions = TribbleException.class)
110+
@Test(expectedExceptions = TribbleException.VersionValidationFailure.class)
111111
public void testMergeWithValidationFailure() {
112112
// test mixing header versions where the old version header has a line that fails validation
113113
// using the resulting (newer) version

0 commit comments

Comments
 (0)