Versioned header line validation framework.

samtools · Feb 28, 2022 · 5a4a9a8 · 5a4a9a8
1 parent d645110
commit 5a4a9a8
Show file tree

Hide file tree

Showing 15 changed files with 92 additions and 86 deletions.
diff --git a/src/main/java/htsjdk/variant/vcf/VCFAltHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFAltHeaderLine.java
@@ -22,7 +22,7 @@ public VCFAltHeaderLine(final String line, final VCFHeaderVersion version) {
         // Honor the requested version to choose the parser, and let validateForVersion figure out
         // whether that version is valid for this line (for example, if this is called with a pre-4.0 version)
         super(VCFConstants.ALT_HEADER_KEY, VCFHeaderLineTranslator.parseLine(version, line, expectedTags));
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     public VCFAltHeaderLine(final String id, final String description) {
@@ -35,7 +35,7 @@ public VCFAltHeaderLine(final String id, final String description) {
     }
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         //TODO: Should we validate/constrain these to match the 4.3 spec constraints ?
         if (!vcfTargetVersion.isAtLeastAsRecentAs(VCFHeaderVersion.VCF4_0)) {
             final VCFValidationFailure<VCFHeaderLine> validationFailure = new VCFValidationFailure<>(
@@ -49,6 +49,6 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
             }
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 }
diff --git a/src/main/java/htsjdk/variant/vcf/VCFCompoundHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFCompoundHeaderLine.java
@@ -126,7 +126,7 @@ protected VCFCompoundHeaderLine(final String key, final Map<String, String> mapp
         final String countString = getGenericFieldValue(NUMBER_ATTRIBUTE);
         this.countType = decodeCountType(countString, vcfVersion);
         this.count = decodeCount(countString, this.countType);
-        validateForVersion(vcfVersion);
+        validateForVersionOrThrow(vcfVersion);
     }
 
     /**
@@ -174,7 +174,7 @@ public String getVersion() {
     }
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         // The VCF 4.3 spec does not phrase this restriction as one on the form of the ID value of
         // INFO/FORMAT lines but instead on the INFO/FORMAT fixed field key values (c.f. section 1.6.1).
         // However, the key values correspond to INFO/FORMAT header lines defining the attribute and its type,
@@ -194,7 +194,7 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
             }
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 
     /**

diff --git a/src/main/java/htsjdk/variant/vcf/VCFContigHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFContigHeaderLine.java
@@ -97,7 +97,7 @@ public VCFContigHeaderLine(final String line, final VCFHeaderVersion version, fi
         if (contigIndex < 0) {
             throw new TribbleException(String.format("The contig index (%d) is less than zero.", contigIndex));
         }
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     public VCFContigHeaderLine(final Map<String, String> mapping, final int contigIndex) {
@@ -186,7 +186,7 @@ public SAMSequenceRecord getSAMSequenceRecord() {
 	}
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         if (vcfTargetVersion.isAtLeastAsRecentAs(VCFHeaderVersion.VCF4_3)) {
              if (!VALID_CONTIG_ID_PATTERN.matcher(getID()).matches()) {
                 return Optional.of(new VCFValidationFailure<>(
@@ -196,7 +196,7 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
             }
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 
     public Integer getContigIndex() {

diff --git a/src/main/java/htsjdk/variant/vcf/VCFFilterHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFFilterHeaderLine.java
@@ -78,7 +78,7 @@ public VCFFilterHeaderLine(final String name) {
     public VCFFilterHeaderLine(final String line, final VCFHeaderVersion version) {
         super(VCFConstants.FILTER_HEADER_KEY, VCFHeaderLineTranslator.parseLine(version, line, requiredTagOrder));
         validate();
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     private void validate() {

diff --git a/src/main/java/htsjdk/variant/vcf/VCFFormatHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFFormatHeaderLine.java
@@ -57,7 +57,7 @@ public VCFFormatHeaderLine(String line, VCFHeaderVersion version) {
               VCFHeaderLineTranslator.parseLine(version, line, expectedTagOrder),
               version);
         validate();
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     /**

diff --git a/src/main/java/htsjdk/variant/vcf/VCFHeader.java b/src/main/java/htsjdk/variant/vcf/VCFHeader.java
@@ -161,7 +161,7 @@ public VCFHeader(final Set<VCFHeaderLine> metaData, final List<String> genotypeS
         // and the header version will be established using the last version line encountered
         mMetaData.addMetaDataLines(metaData);
         final VCFHeaderVersion vcfHeaderVersion = initializeHeaderVersion();
-        mMetaData.validateMetaDataLines(vcfHeaderVersion);
+        mMetaData.validateMetaDataLinesOrThrow(vcfHeaderVersion);
 
         checkForDeprecatedGenotypeLikelihoodsKey();
         if ( genotypeSampleNames.size() != new HashSet<>(genotypeSampleNames).size() )
@@ -625,9 +625,9 @@ private void validateVersionTransition(
 
             // the version moved forward, so validate ALL of the existing lines in the list to ensure
             // that the transition is valid
-            mMetaData.validateMetaDataLines(newVersion);
+            mMetaData.validateMetaDataLinesOrThrow(newVersion);
         } else {
-            newHeaderLine.validateForVersion(newVersion);
+            newHeaderLine.validateForVersionOrThrow(newVersion);
         }
     }
 

diff --git a/src/main/java/htsjdk/variant/vcf/VCFHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFHeaderLine.java
@@ -50,10 +50,13 @@ public class VCFHeaderLine implements Comparable, Serializable {
      * @param key     the key for this header line
      * @param value   the value for this header line
      */
-    public VCFHeaderLine(String key, String value) {
+    public VCFHeaderLine(final String key, final String value) {
+        final Optional<String> validationFailure = validateAttributeName(key, "header line key");
+        if (validationFailure.isPresent()) {
+            throw new TribbleException(validationFailure.get());
+        }
         mKey = key;
         mValue = value;
-        validate();
     }
 
     /**
@@ -86,15 +89,20 @@ public String getValue() {
     public String getID() { return null; }
 
     /**
-     * Validates this header line against {@code vcfTargetVersion}.
-     * Subclasses can override this to provide line type-specific version validation, and the
-     * overrides should also call super.getValidationFailure to allow each class in the class hierarchy
-     * to do class-level validation.
+     * Validates this header line against {@code vcfTargetVersion} and returns a {@link VCFValidationFailure}
+     * describing the reaon for the failure, if one exists. This method is used to report the reason for a
+     * version upgrade failure.
+     *
+     * Subclasses can override this to provide line type-specific version validation. Overrides should
+     * call super.validateForVersion to allow each class in the  hierarchy to do class-level validation.
      *
+     * @param vcfTargetVersion
      * @return Optional containing a {@link VCFValidationFailure} describing validation failure if this
      * line fails validation, otherwise Optional.empty().
      */
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
+        ValidationUtils.nonNull(vcfTargetVersion);
+
         // If this header line is itself a fileformat/version line,
         // make sure it doesn't clash with the requested vcfTargetVersion.
         if (VCFHeaderVersion.isFormatString(getKey())) {
@@ -124,33 +132,42 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
     }
 
     /**
-     * Validate that the header line conforms to {@code vcfTargetVersion.
-     * @param vcfTargetVersion
+     * Validate that the header line conforms to {@code vcfTargetVersion. throws if the line fails to
+     * validate for the target version.
+     *
+     * @param vcfTargetVersion the version agint which to validate the line
      * @throws {@link TribbleException.VersionValidationFailure} if this header line fails to conform
      */
-    public void validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
-        final Optional<VCFValidationFailure<VCFHeaderLine>> error = getValidationFailure(vcfTargetVersion);
-        if (error.isPresent()) {
-            throw new TribbleException.VersionValidationFailure(error.get().getSourceMessage());
+    public void validateForVersionOrThrow(final VCFHeaderVersion vcfTargetVersion) {
+        ValidationUtils.nonNull(vcfTargetVersion);
+        final Optional<VCFValidationFailure<VCFHeaderLine>> versionValidationFailure = validateForVersion(vcfTargetVersion);
+        if (versionValidationFailure.isPresent()) {
+            throw new TribbleException.VersionValidationFailure(versionValidationFailure.get().getSourceMessage());
         }
     }
 
     /**
-     * Validate a string that is to be used as a unique id or key field.
+     * Validate a string that is to be used as a unique id or key field, and return an Optional String describing
+     * the validation failure.
+     *
+     * @param targetString the string to validate
+     * @param targetContext the context for which the {@code targetString} is used. Used when reporting validation
+     *                      failures. May not be null.
+     * @return an Optional String containing an error message
      */
-    protected static void validateKeyOrID(final String keyString, final String sourceName) {
-        ValidationUtils.nonNull(sourceName);
-        if (keyString == null) {
-            throw new TribbleException(
-                    String.format("VCFHeaderLine: %s cannot be null or empty", sourceName));
-        }
-        if ( keyString.contains("<") || keyString.contains(">") ) {
-            throw new TribbleException(
-                    String.format("VCFHeaderLine: %s cannot contain angle brackets", sourceName));
-        }
-        if ( keyString.contains("=") ) {
-            throw new TribbleException(
-                    String.format("VCFHeaderLine: %s cannot contain an equals sign", sourceName));
+    protected static Optional<String> validateAttributeName(final String targetString, final String targetContext) {
+        ValidationUtils.nonNull(targetContext);
+
+        if (targetString == null) {
+            return Optional.of(String.format("VCFHeaderLine: %s is null", targetContext));
+        } else if (targetString.length() < 1) {
+            return Optional.of(String.format("VCFHeaderLine: %s has zero length", targetContext));
+        } else if ( targetString.contains("<") || targetString.contains(">") ) {
+            return Optional.of(String.format("VCFHeaderLine: angle brackets not allowed in \"%s\" value", targetContext));
+        } else if ( targetString.contains("=") ) {
+            return Optional.of(String.format("VCFHeaderLine: equals sign not allowed in %s value \"%s\"", targetContext, targetString));
+        } else {
+            return Optional.empty();
         }
     }
 
@@ -239,13 +256,6 @@ public static String toStringEncoding(Map<String, ? extends Object> keyValues) {
         return builder.toString();
     }
 
-    /**
-     * Validate the state of this header line. Require the key be valid as an "id".
-     */
-    private void validate() {
-        validateKeyOrID(mKey, "key");
-    }
-
     private static String escapeQuotes(final String value) {
         // java escaping in a string literal makes this harder to read than it should be
         // without string literal escaping and quoting the regex would be: replaceAll( ([^\])" , $1\" )

diff --git a/src/main/java/htsjdk/variant/vcf/VCFInfoHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFInfoHeaderLine.java
@@ -67,7 +67,7 @@ public VCFInfoHeaderLine(String line, VCFHeaderVersion version) {
               VCFHeaderLineTranslator.parseLine(version, line, expectedTagOrder),
               version
         );
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     /**

diff --git a/src/main/java/htsjdk/variant/vcf/VCFMetaDataLines.java b/src/main/java/htsjdk/variant/vcf/VCFMetaDataLines.java
@@ -171,10 +171,10 @@ public VCFHeaderLine findEquivalentHeaderLine(final VCFHeaderLine queryLine) {
      */
     //TODO: we need to tell users how to resolve the case where this fails due to version validation
     //i.e, use a custom upgrade tool
-    public void validateMetaDataLines(final VCFHeaderVersion targetVersion) {
+    public void validateMetaDataLinesOrThrow(final VCFHeaderVersion targetVersion) {
         mMetaData.values().forEach(headerLine -> {
             if (!VCFHeaderVersion.isFormatString(headerLine.getKey())) {
-                headerLine.validateForVersion(targetVersion);
+                headerLine.validateForVersionOrThrow(targetVersion);
             }
         });
     }
@@ -190,7 +190,7 @@ public void validateMetaDataLines(final VCFHeaderVersion targetVersion) {
     public Collection<VCFValidationFailure> getValidationErrors(final VCFHeaderVersion targetVersion) {
         return mMetaData.values().stream()
                 .filter(line -> !VCFHeaderVersion.isFormatString(line.getKey()))
-                .map(l -> l.getValidationFailure(targetVersion))
+                .map(l -> l.validateForVersion(targetVersion))
                 .filter(o -> o.isPresent())
                 .map(o -> o.get())
                 .collect(Collectors.toList());

diff --git a/src/main/java/htsjdk/variant/vcf/VCFMetaHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFMetaHeaderLine.java
@@ -15,15 +15,15 @@ public VCFMetaHeaderLine(final String line, final VCFHeaderVersion version) {
         // other tags. So let validateForVersion detect any version incompatibility, ie., if this is ever
         // called with a V3 version.
         super(VCFConstants.META_HEADER_KEY, new VCF4Parser().parseLine(line, expectedTagOrder));
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     public VCFMetaHeaderLine(final Map<String, String> mapping) {
         super(VCFConstants.META_HEADER_KEY, mapping);
     }
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         if (!vcfTargetVersion.isAtLeastAsRecentAs(VCFHeaderVersion.VCF4_3)) {
             return Optional.of(
                     new VCFValidationFailure<>(
@@ -35,7 +35,7 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
                 )));
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 
 }
diff --git a/src/main/java/htsjdk/variant/vcf/VCFPedigreeHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFPedigreeHeaderLine.java
@@ -21,15 +21,15 @@ public VCFPedigreeHeaderLine(String line, VCFHeaderVersion version) {
         // other tags. So let validateForVersion detect any version incompatibility, ie., if this is ever
         // called with a V3 version.
         super(VCFConstants.PEDIGREE_HEADER_KEY, new VCF4Parser().parseLine(line, expectedTagOrder));
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     public VCFPedigreeHeaderLine(final Map<String, String> mapping) {
         super(VCFConstants.PEDIGREE_HEADER_KEY, mapping);
     }
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         if (!vcfTargetVersion.isAtLeastAsRecentAs(VCFHeaderVersion.VCF4_3)) {
             // previous to VCFv4.3, the PEDIGREE line did not have an ID. Such lines are not modeled by this
             // class (since it is derived from VCFSimpleHeaderLine). Therefore instances of this class always
@@ -45,7 +45,7 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
             }
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 
 }
diff --git a/src/main/java/htsjdk/variant/vcf/VCFSampleHeaderLine.java b/src/main/java/htsjdk/variant/vcf/VCFSampleHeaderLine.java
@@ -15,15 +15,15 @@ public VCFSampleHeaderLine(String line, VCFHeaderVersion version) {
         // other tags. So let validateForVersion detect any version incompatibility, ie., if this is ever
         // called with a V3 version.
         super(VCFConstants.SAMPLE_HEADER_KEY, new VCF4Parser().parseLine(line, expectedTagOrder));
-        validateForVersion(version);
+        validateForVersionOrThrow(version);
     }
 
     public VCFSampleHeaderLine(final Map<String, String> mapping) {
         super(VCFConstants.SAMPLE_HEADER_KEY, mapping);
     }
 
     @Override
-    public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final VCFHeaderVersion vcfTargetVersion) {
+    public Optional<VCFValidationFailure<VCFHeaderLine>> validateForVersion(final VCFHeaderVersion vcfTargetVersion) {
         if (!vcfTargetVersion.isAtLeastAsRecentAs(VCFHeaderVersion.VCF4_0)) {
             final String message = String.format("%s header lines are not allowed in VCF version %s headers",
                 getKey(),
@@ -36,7 +36,7 @@ public Optional<VCFValidationFailure<VCFHeaderLine>> getValidationFailure(final
             }
         }
 
-        return super.getValidationFailure(vcfTargetVersion);
+        return super.validateForVersion(vcfTargetVersion);
     }
 
 }