md5sum, sha1sum and friends). */
+ private static final Pattern BARE_HEX_HASH = Pattern.compile("^([0-9a-fA-F]{32,128})(?:[\\s*].*)?$");
+
+ /** The BSD layout, e.g. MD5 (file.txt) = d41d8cd9.... */
+ private static final Pattern BSD_HASH = Pattern.compile("^\\w+\\s*\\(.*\\)\\s*=\\s*([0-9a-fA-F]{32,128})$");
+
public enum Hash{
MD5, SHA1, SHA256, UNKNOWN
}
+ /**
+ * What to do with the ETag response header when no explicit hash
+ * URL is available.
+ *
+ * Some archives — notably files.wwpdb.org and
+ * files.rcsb.org — return the MD5 digest of the content as
+ * the bare ETag value, which lets us record a real checksum
+ * without a second request. Others (the EBI servers, for instance) return a
+ * <modification-time>-<size> form instead; because that
+ * always contains a -, it can never be mistaken for a hex digest.
+ *
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public enum ETagPolicy {
+ /** Never look at the ETag header. */
+ IGNORE,
+ /** Record the ETag as a checksum when it is a bare hex digest
+ * of a length matching one of the supported algorithms. */
+ USE_IF_HEX_DIGEST,
+ /** As {@link #USE_IF_HEX_DIGEST}, but log a warning when the header is
+ * missing or is not a usable digest. */
+ REQUIRE
+ }
+
/**
* Gets the file extension of a file, excluding '.'.
* If the file name has no extension the file name is returned.
@@ -95,44 +134,158 @@ public static void downloadFile(URL url, File destination) throws IOException {
int maxTries = 10;
int timeout = 60000; //60 sec
- File tempFile = Files.createTempFile(getFilePrefix(destination), "." + getFileExtension(destination)).toFile();
+ File tempFile = createTempFileFor(destination);
- // Took following recipe from stackoverflow:
- // http://stackoverflow.com/questions/921262/how-to-download-and-save-a-file-from-internet-using-java
- // It seems to be the most efficient way to transfer a file
- // See: http://docs.oracle.com/javase/7/docs/api/java/nio/channels/FileChannel.html
- ReadableByteChannel rbc = null;
- FileOutputStream fos = null;
- while (true) {
- try {
- URLConnection connection = prepareURLConnection(url.toString(), timeout);
- connection.connect();
- InputStream inputStream = connection.getInputStream();
-
- rbc = Channels.newChannel(inputStream);
- fos = new FileOutputStream(tempFile);
- fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE);
- break;
- } catch (SocketTimeoutException e) {
- if (++count == maxTries) throw e;
- } finally {
- if (rbc != null) {
- rbc.close();
- }
- if (fos != null) {
- fos.close();
+ try {
+ while (true) {
+ try {
+ URLConnection connection = prepareURLConnection(url.toString(), timeout);
+ connection.connect();
+ checkHttpStatus(connection);
+ try (InputStream inputStream = connection.getInputStream()) {
+ // Files.copy loops until end of stream. FileChannel.transferFrom(), used
+ // here previously, is not guaranteed to drain a socket-backed channel in
+ // a single call and could silently truncate a download.
+ Files.copy(inputStream, tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ }
+ break;
+ } catch (SocketTimeoutException e) {
+ if (++count == maxTries) throw e;
}
}
+
+ logger.debug("Copying temp file [{}] to final location [{}]", tempFile, destination);
+ Files.copy(tempFile.toPath(), destination.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ } finally {
+ // on every path, including failure: the temp file used to leak whenever
+ // the download threw.
+ deleteQuietly(tempFile);
}
+ }
+
+ /**
+ * Downloads a file and writes its validation metadata in a single pass over a
+ * single connection.
+ *
+ * This is preferable to calling {@link #createValidationFiles(URL, File, URL, Hash)}
+ * followed by {@link #downloadFile(URL, File)}: those open separate connections,
+ * so the Content-Length recorded in the .size file
+ * comes from a different response than the bytes actually written. If the
+ * resource changes between the two requests, the cache entry is left
+ * permanently failing validation.
+ *
+ * The content is streamed to a temporary file and only moved into place once
+ * the declared length and (where available) the checksum have been confirmed,
+ * so a failed download never leaves a partial file at destination.
+ *
+ * @param url the remote file to download
+ * @param destination the local file to download into. Its parent directory must exist.
+ * @param hashURL URL of a file containing the expected hash. May be null.
+ * @param hash the hashing algorithm matching hashURL. Ignored when
+ * hashURL is null.
+ * @param eTagPolicy what to do with an ETag response header when
+ * hashURL is null. May be null,
+ * which is treated as {@link ETagPolicy#IGNORE}.
+ * @throws HttpStatusException if the server answered with a non-2xx status
+ * @throws IOException if the transfer failed, or the transferred content did not
+ * match the length or checksum the server declared
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static void downloadFileWithValidation(URL url, File destination, URL hashURL, Hash hash,
+ ETagPolicy eTagPolicy) throws IOException {
+ int timeout = 60000; //60 sec
+ ETagPolicy policy = eTagPolicy == null ? ETagPolicy.IGNORE : eTagPolicy;
+
+ File tempFile = createTempFileFor(destination);
+ try {
+ URLConnection connection = prepareURLConnection(url.toString(), timeout);
+ connection.connect();
+ checkHttpStatus(connection);
- logger.debug("Copying temp file [{}] to final location [{}]", tempFile, destination);
- Files.copy(tempFile.toPath(), destination.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ long declaredSize = connection.getContentLengthLong();
+ String eTag = connection.getHeaderField("ETag");
- // delete the tmp file
- tempFile.delete();
+ // Only digest when we have something to compare against; hashing every
+ // download would cost CPU for no benefit.
+ Hash eTagHash = policy == ETagPolicy.IGNORE ? Hash.UNKNOWN : hashFromETag(eTag);
+ if (policy == ETagPolicy.REQUIRE && eTagHash == Hash.UNKNOWN) {
+ logger.warn("ETag [{}] of {} is not a usable hex digest; no checksum will be recorded.", eTag, url);
+ }
+
+ MessageDigest digest = eTagHash == Hash.UNKNOWN ? null : newDigest(eTagHash);
+ long written;
+ try (InputStream raw = connection.getInputStream();
+ InputStream in = digest == null ? raw : new DigestInputStream(raw, digest)) {
+ written = Files.copy(in, tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ }
+
+ if (declaredSize >= 0 && written != declaredSize) {
+ throw new IOException(String.format(
+ "Incomplete download of %s: got %d bytes but the server declared %d.",
+ url, written, declaredSize));
+ }
+
+ String actualDigest = digest == null ? null : toHex(digest.digest());
+ if (actualDigest != null && !actualDigest.equalsIgnoreCase(normalizeETag(eTag))) {
+ throw new IOException(String.format(
+ "Corrupt download of %s: %s of the content is %s but the server's ETag says %s.",
+ url, eTagHash, actualDigest, normalizeETag(eTag)));
+ }
+
+ moveIntoPlace(tempFile, destination);
+ // Sidecars are written only once the content is known good, so a failed
+ // download can never leave validation metadata describing a file that is
+ // not there.
+ writeSizeFile(destination, written);
+ if (hashURL != null) {
+ if (hash == null || hash == Hash.UNKNOWN) {
+ throw new IllegalArgumentException("Hash URL given but algorithm is unknown");
+ }
+ downloadFile(hashURL, hashFileFor(destination, hash));
+ } else if (actualDigest != null) {
+ writeHashFile(destination, eTagHash, actualDigest);
+ }
+ } finally {
+ deleteQuietly(tempFile);
+ }
}
-
+
+ /**
+ * Verifies that an HTTP connection returned a 2xx status. Connections using a
+ * non-HTTP protocol (file:, ftp:, ...) are left alone.
+ *
+ * Without this check a 404 error page is written into the cache as though it
+ * were the requested file — and because the .size sidecar is
+ * then taken from that same error response, {@link #validateFile(File)} would
+ * subsequently declare it valid.
+ *
+ * @param connection an already connected {@link URLConnection}
+ * @throws HttpStatusException if the status is outside the 2xx range
+ * @throws IOException if the status could not be read
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static void checkHttpStatus(URLConnection connection) throws IOException {
+ if (!(connection instanceof HttpURLConnection)) {
+ return;
+ }
+ HttpURLConnection http = (HttpURLConnection) connection;
+ int code = http.getResponseCode();
+ if (code >= 200 && code < 300) {
+ return;
+ }
+ if (code == 301 || code == 302 || code == 307 || code == 308) {
+ // The JDK follows redirects automatically, but never across protocols, so
+ // an http -> https redirect surfaces here and is worth naming explicitly.
+ logger.warn("{} returned redirect {} to [{}], which was not followed "
+ + "(the JDK does not follow redirects that change protocol).",
+ connection.getURL(), code, http.getHeaderField("Location"));
+ }
+ throw new HttpStatusException(code, connection.getURL().toString(), http.getResponseMessage());
+ }
+
/**
* Creates validation files beside a file to be downloaded.
* Whenever possible, for a file.ext file, it creates
@@ -146,9 +299,27 @@ public static void downloadFile(URL url, File destination) throws IOException {
* @param hash The Hashing algorithm. Ignored if hashURL is null.
*/
public static void createValidationFiles(URL url, File localDestination, URL hashURL, Hash hash){
+ createValidationFiles(url, localDestination, hashURL, hash, ETagPolicy.USE_IF_HEX_DIGEST);
+ }
+
+ /**
+ * Creates validation files beside a file to be downloaded, with explicit control
+ * over how the ETag response header is treated.
+ *
+ * @param url the remote file URL to download
+ * @param localDestination the local file to download into
+ * @param hashURL the URL of the hash file to download. Can be null.
+ * @param hash The Hashing algorithm. Ignored if hashURL is null.
+ * @param eTagPolicy how to treat the ETag header when hashURL
+ * is null. May be null, treated as {@link ETagPolicy#IGNORE}.
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static void createValidationFiles(URL url, File localDestination, URL hashURL, Hash hash,
+ ETagPolicy eTagPolicy){
try {
URLConnection resourceConnection = url.openConnection();
- createValidationFiles(resourceConnection, localDestination, hashURL, FileDownloadUtils.Hash.UNKNOWN);
+ createValidationFiles(resourceConnection, localDestination, hashURL, hash, eTagPolicy);
} catch (IOException e) {
logger.warn("could not open connection to resource file due to exception: {}", e.getMessage());
}
@@ -169,31 +340,246 @@ public static void createValidationFiles(URL url, File localDestination, URL has
* @since 7.0.0
*/
public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL, Hash hash){
+ createValidationFiles(resourceUrlConnection, localDestination, hashURL, hash, ETagPolicy.USE_IF_HEX_DIGEST);
+ }
+
+ /**
+ * Creates validation files beside a file to be downloaded, with explicit control
+ * over how the ETag response header is treated.
+ *
+ * Nothing is written when the connection reports a non-2xx status: previously an
+ * error page's Content-Length would be recorded as the expected
+ * size, so the cached error page then passed validation.
+ *
+ * @param resourceUrlConnection the remote file URLConnection to download
+ * @param localDestination the local file to download into
+ * @param hashURL the URL of the hash file to download. Can be null.
+ * @param hash The Hashing algorithm. Ignored if hashURL is null.
+ * @param eTagPolicy how to treat the ETag header when hashURL
+ * is null. May be null, treated as {@link ETagPolicy#IGNORE}.
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL,
+ Hash hash, ETagPolicy eTagPolicy){
+ try {
+ checkHttpStatus(resourceUrlConnection);
+ } catch (IOException e) {
+ logger.warn("Not writing validation metadata for {}: {}", resourceUrlConnection.getURL(), e.getMessage());
+ return;
+ }
+
long size = resourceUrlConnection.getContentLengthLong();
if(size == -1) {
logger.debug("Could not find expected file size for resource {}. Size validation metadata file won't be available for this download.", resourceUrlConnection.getURL());
} else {
logger.debug("Content-Length: {}", size);
- File sizeFile = new File(localDestination.getParentFile(), localDestination.getName() + SIZE_EXT);
- try (PrintStream sizePrintStream = new PrintStream(sizeFile)) {
- sizePrintStream.print(size);
- } catch (FileNotFoundException e) {
- logger.warn("Could not write size validation metadata file due to exception: {}", e.getMessage());
- }
+ writeSizeFile(localDestination, size);
}
-
- if(hashURL == null)
+
+ if(hashURL == null) {
+ ETagPolicy policy = eTagPolicy == null ? ETagPolicy.IGNORE : eTagPolicy;
+ if (policy != ETagPolicy.IGNORE) {
+ String eTag = resourceUrlConnection.getHeaderField("ETag");
+ Hash eTagHash = hashFromETag(eTag);
+ if (eTagHash == Hash.UNKNOWN) {
+ if (policy == ETagPolicy.REQUIRE) {
+ logger.warn("ETag [{}] of {} is not a usable hex digest; no checksum recorded.",
+ eTag, resourceUrlConnection.getURL());
+ }
+ } else {
+ writeHashFile(localDestination, eTagHash, normalizeETag(eTag));
+ }
+ }
return;
+ }
- if(hash == Hash.UNKNOWN)
+ if(hash == null || hash == Hash.UNKNOWN)
throw new IllegalArgumentException("Hash URL given but algorithm is unknown");
try {
- File hashFile = new File(localDestination.getParentFile(), String.format("%s%s_%s", localDestination.getName(), HASH_EXT, hash));
- downloadFile(hashURL, hashFile);
+ downloadFile(hashURL, hashFileFor(localDestination, hash));
} catch (IOException e) {
logger.warn("Could not write validation hash file due to exception: {}", e.getMessage());
}
}
+
+ /**
+ * Determines which hashing algorithm an ETag header value
+ * represents, based on the length of the hex digest it contains.
+ *
+ * Only a value consisting solely of hex characters is considered. The
+ * <time>-<size> form used by nginx and Apache always
+ * contains a - and therefore never matches.
+ *
+ * @param eTagHeaderValue the raw header value, possibly quoted or weak-prefixed.
+ * May be null.
+ * @return the matching algorithm, or {@link Hash#UNKNOWN} if the value is not a
+ * hex digest of a recognised length
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static Hash hashFromETag(String eTagHeaderValue) {
+ String value = normalizeETag(eTagHeaderValue);
+ if (value == null || !value.matches("[0-9a-fA-F]+")) {
+ return Hash.UNKNOWN;
+ }
+ switch (value.length()) {
+ case 32: return Hash.MD5;
+ case 40: return Hash.SHA1;
+ case 64: return Hash.SHA256;
+ default: return Hash.UNKNOWN;
+ }
+ }
+
+ /**
+ * Strips the weak-validator prefix and surrounding quotes from an
+ * ETag header value.
+ *
+ * @param eTagHeaderValue the raw header value. May be null.
+ * @return the bare value, or null if the input was null
+ * or blank
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static String normalizeETag(String eTagHeaderValue) {
+ if (eTagHeaderValue == null) {
+ return null;
+ }
+ String value = eTagHeaderValue.trim();
+ if (value.startsWith("W/")) {
+ value = value.substring(2).trim();
+ }
+ if (value.length() >= 2 && value.startsWith("\"") && value.endsWith("\"")) {
+ value = value.substring(1, value.length() - 1);
+ }
+ return value.isEmpty() ? null : value;
+ }
+
+ /**
+ * Writes a <name>.hash_<ALGORITHM> sidecar file
+ * containing the given digest as bare lowercase hex.
+ *
+ * @param localDestination the file the digest describes
+ * @param hash the hashing algorithm
+ * @param hexDigest the digest, in hex
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static void writeHashFile(File localDestination, Hash hash, String hexDigest) {
+ if (hash == null || hash == Hash.UNKNOWN || hexDigest == null) {
+ return;
+ }
+ File hashFile = hashFileFor(localDestination, hash);
+ try (PrintStream out = new PrintStream(hashFile, StandardCharsets.UTF_8.name())) {
+ out.println(hexDigest.toLowerCase());
+ } catch (IOException e) {
+ logger.warn("Could not write validation hash file due to exception: {}", e.getMessage());
+ }
+ }
+
+ /**
+ * Computes the digest of a file.
+ *
+ * @param file the file to digest
+ * @param hash the algorithm to use
+ * @return the digest as bare lowercase hex
+ * @throws IOException if the file could not be read
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static String computeHash(File file, Hash hash) throws IOException {
+ try (InputStream in = new BufferedInputStream(Files.newInputStream(file.toPath()), DIGEST_BUFFER_SIZE)) {
+ return computeHash(in, hash);
+ }
+ }
+
+ /**
+ * Computes the digest of a stream. The stream is read to its end but not closed.
+ *
+ * @param in the stream to digest
+ * @param hash the algorithm to use
+ * @return the digest as bare lowercase hex
+ * @throws IOException if the stream could not be read
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static String computeHash(InputStream in, Hash hash) throws IOException {
+ MessageDigest digest = newDigest(hash);
+ byte[] buffer = new byte[DIGEST_BUFFER_SIZE];
+ int read;
+ while ((read = in.read(buffer)) != -1) {
+ digest.update(buffer, 0, read);
+ }
+ return toHex(digest.digest());
+ }
+
+ /**
+ * Checks a file against an expected digest.
+ *
+ * @param file the file to check
+ * @param hash the algorithm to use
+ * @param expectedHex the expected digest, in hex; compared case-insensitively
+ * @return true if the digests match
+ * @throws IOException if the file could not be read
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static boolean verifyHash(File file, Hash hash, String expectedHex) throws IOException {
+ return expectedHex != null && expectedHex.trim().equalsIgnoreCase(computeHash(file, hash));
+ }
+
+ /**
+ * The JDK name of a hashing algorithm, which differs from the enum constant for
+ * the SHA variants.
+ *
+ * @param hash the algorithm
+ * @return the name to pass to {@link MessageDigest#getInstance(String)}
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ public static String getAlgorithmName(Hash hash) {
+ switch (hash) {
+ case MD5: return "MD5";
+ case SHA1: return "SHA-1";
+ case SHA256: return "SHA-256";
+ default: throw new IllegalArgumentException("Hashing algorithm not known: " + hash);
+ }
+ }
+
+ /**
+ * Reads the expected digest out of a .hash_XXXX sidecar file.
+ *
+ * The file may have been downloaded verbatim from a server, so several common
+ * layouts are accepted: a bare hex digest, the md5sum style
+ * <hex> <filename>, and the BSD style
+ * MD5 (<filename>) = <hex>.
+ *
+ * @param hashFile the sidecar file
+ * @return the digest in hex, or null if nothing recognisable was found
+ */
+ static String parseHashFile(File hashFile) {
+ try (Scanner scanner = new Scanner(hashFile, StandardCharsets.UTF_8.name())) {
+ while (scanner.hasNextLine()) {
+ String line = scanner.nextLine().trim();
+ if (line.isEmpty()) {
+ continue;
+ }
+ Matcher bare = BARE_HEX_HASH.matcher(line);
+ if (bare.matches()) {
+ return bare.group(1);
+ }
+ Matcher bsd = BSD_HASH.matcher(line);
+ if (bsd.matches()) {
+ return bsd.group(1);
+ }
+ return null; // first meaningful line was not a digest
+ }
+ } catch (IOException e) {
+ logger.warn("Could not read hash file [{}]: {}", hashFile, e.getMessage());
+ }
+ return null;
+ }
+
/**
* Validate a local file based on pre-existing metadata files for size and hash.
@@ -210,45 +596,154 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi
* @since 7.0.0
*/
public static boolean validateFile(File localFile) {
- File sizeFile = new File(localFile.getParentFile(), localFile.getName() + SIZE_EXT);
+ // getParentFile() is null for a bare relative name such as new File("x.cif"),
+ // which used to make this method throw a NullPointerException.
+ File parent = localFile.getAbsoluteFile().getParentFile();
+ if (parent == null) {
+ logger.debug("Cannot determine the parent directory of [{}]; nothing to validate against.", localFile);
+ return true;
+ }
+
+ File sizeFile = new File(parent, localFile.getName() + SIZE_EXT);
if(sizeFile.exists()) {
try (Scanner scanner = new Scanner(sizeFile)) {
- long expectedSize = scanner.nextLong();
- long actualSize = localFile.length();
- if (expectedSize != actualSize) {
- logger.warn("File [{}] size ({}) does not match expected size ({}).", localFile, actualSize, expectedSize);
- return false;
+ if (!scanner.hasNextLong()) {
+ // An empty or truncated .size file used to raise an unchecked
+ // NoSuchElementException that escaped the catch below.
+ logger.warn("Size metadata file [{}] is empty or malformed; skipping size validation.", sizeFile);
+ } else {
+ long expectedSize = scanner.nextLong();
+ long actualSize = localFile.length();
+ if (expectedSize != actualSize) {
+ logger.warn("File [{}] size ({}) does not match expected size ({}).", localFile, actualSize, expectedSize);
+ return false;
+ }
}
} catch (FileNotFoundException e) {
logger.warn("could not validate size of file [{}] because no size metadata file exists.", localFile);
}
}
- File[] hashFiles = localFile.getParentFile().listFiles(new FilenameFilter() {
- final String hashPattern = String.format("%s%s_(%s|%s|%s)", localFile.getName(), HASH_EXT, Hash.MD5, Hash.SHA1, Hash.SHA256);
+ File[] hashFiles = parent.listFiles(new FilenameFilter() {
+ final String hashPattern = String.format("%s%s_(%s|%s|%s)", Pattern.quote(localFile.getName()), HASH_EXT, Hash.MD5, Hash.SHA1, Hash.SHA256);
@Override
public boolean accept(File dir, String name) {
return name.matches(hashPattern);
}
});
- if(hashFiles.length > 0) {
- File hashFile = hashFiles[0];
+ // listFiles() returns null if the parent is not a directory or cannot be read.
+ if (hashFiles == null || hashFiles.length == 0) {
+ return true;
+ }
+
+ // Verify against every sidecar present, not only the first one found.
+ for (File hashFile : hashFiles) {
String name = hashFile.getName();
String algo = name.substring(name.lastIndexOf('_') + 1);
- switch (Hash.valueOf(algo)) {
- case MD5:
- case SHA1:
- case SHA256:
- throw new UnsupportedOperationException("Not yet implemented");
- case UNKNOWN:
- default: // No need. Already checked above
+ Hash hash;
+ try {
+ hash = Hash.valueOf(algo);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException("Hashing algorithm not known: " + algo, e);
+ }
+ if (hash == Hash.UNKNOWN) {
throw new IllegalArgumentException("Hashing algorithm not known: " + algo);
}
+
+ String expected = parseHashFile(hashFile);
+ if (expected == null) {
+ // A sidecar we cannot read should not condemn an otherwise good download.
+ logger.warn("Could not read a digest from [{}]; skipping {} validation of [{}].", hashFile, hash, localFile);
+ continue;
+ }
+ try {
+ if (!verifyHash(localFile, hash, expected)) {
+ logger.warn("File [{}] {} does not match the expected digest {}.", localFile, hash, expected);
+ return false;
+ }
+ } catch (IOException e) {
+ logger.warn("Could not compute the {} of [{}]: {}", hash, localFile, e.getMessage());
+ return false;
+ }
}
-
+
return true;
}
+ /**
+ * The <name>.hash_<ALGORITHM> sidecar file for a
+ * downloaded file.
+ */
+ private static File hashFileFor(File localDestination, Hash hash) {
+ return new File(localDestination.getAbsoluteFile().getParentFile(),
+ String.format("%s%s_%s", localDestination.getName(), HASH_EXT, hash));
+ }
+
+ /**
+ * Writes the <name>.size sidecar file.
+ */
+ private static void writeSizeFile(File localDestination, long size) {
+ File sizeFile = new File(localDestination.getAbsoluteFile().getParentFile(),
+ localDestination.getName() + SIZE_EXT);
+ try (PrintStream sizePrintStream = new PrintStream(sizeFile, StandardCharsets.UTF_8.name())) {
+ sizePrintStream.print(size);
+ } catch (IOException e) {
+ logger.warn("Could not write size validation metadata file due to exception: {}", e.getMessage());
+ }
+ }
+
+ private static MessageDigest newDigest(Hash hash) {
+ try {
+ return MessageDigest.getInstance(getAlgorithmName(hash));
+ } catch (NoSuchAlgorithmException e) {
+ // MD5, SHA-1 and SHA-256 are required of every Java platform.
+ throw new IllegalStateException("Required hashing algorithm is unavailable: " + hash, e);
+ }
+ }
+
+ private static String toHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder(bytes.length * 2);
+ for (byte b : bytes) {
+ sb.append(Character.forDigit((b >> 4) & 0xF, 16));
+ sb.append(Character.forDigit(b & 0xF, 16));
+ }
+ return sb.toString();
+ }
+
+ /**
+ * Creates a temp file whose name is derived from the destination.
+ * {@link Files#createTempFile} rejects prefixes shorter than 3 characters, so
+ * short names are padded.
+ */
+ private static File createTempFileFor(File destination) throws IOException {
+ String prefix = getFilePrefix(destination);
+ while (prefix.length() < 3) {
+ prefix = prefix + "_";
+ }
+ return Files.createTempFile(prefix, "." + getFileExtension(destination)).toFile();
+ }
+
+ private static void moveIntoPlace(File tempFile, File destination) throws IOException {
+ try {
+ Files.move(tempFile.toPath(), destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
+ } catch (AtomicMoveNotSupportedException e) {
+ // The temp directory is often on a different filesystem than the cache.
+ Files.copy(tempFile.toPath(), destination.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ }
+ }
+
+ private static void deleteQuietly(File file) {
+ if (file == null) {
+ return;
+ }
+ try {
+ Files.deleteIfExists(file.toPath());
+ } catch (IOException e) {
+ logger.debug("Could not delete temporary file [{}]: {}", file, e.getMessage());
+ }
+ }
+
/**
* Converts path to Unix convention and adds a terminating slash if it was
* omitted.
diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/HttpStatusException.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/HttpStatusException.java
new file mode 100644
index 0000000000..c74c6ffb05
--- /dev/null
+++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/HttpStatusException.java
@@ -0,0 +1,84 @@
+/**
+ * BioJava development code
+ *
+ * This code may be freely distributed and modified under the terms of the GNU
+ * Lesser General Public Licence. This should be distributed with the code. If
+ * you do not have a copy, see:
+ *
+ * http://www.gnu.org/copyleft/lesser.html
+ *
+ * Copyright for this code is held jointly by the individual authors. These
+ * should be listed in @author doc comments.
+ *
+ * For more information on the BioJava project and its aims, or to join the
+ * biojava-l mailing list, visit the home page at:
+ *
+ * http://www.biojava.org/
+ */
+package org.biojava.nbio.core.util;
+
+import java.io.IOException;
+
+/**
+ * Signals that an HTTP request completed but returned a status code outside the
+ * 2xx range.
+ *
+ * This exists so that callers can tell apart the two very different reasons a + * download can fail: + *
null
+ */
+ public HttpStatusException(int statusCode, String url, String responseMessage) {
+ super(String.format("HTTP %d%s for %s", statusCode,
+ responseMessage == null || responseMessage.isEmpty() ? "" : " " + responseMessage, url));
+ this.statusCode = statusCode;
+ this.url = url;
+ }
+
+ /**
+ * @return the HTTP status code returned by the server
+ */
+ public int getStatusCode() {
+ return statusCode;
+ }
+
+ /**
+ * @return the URL that was requested
+ */
+ public String getUrl() {
+ return url;
+ }
+
+ /**
+ * Whether the status indicates that the resource is simply not there, as
+ * opposed to a transport or server problem.
+ *
+ * @return true for HTTP 404 (Not Found) and 410 (Gone)
+ */
+ public boolean isNotFound() {
+ return statusCode == 404 || statusCode == 410;
+ }
+}
diff --git a/biojava-core/src/test/java/org/biojava/nbio/core/util/FileDownloadUtilsTest.java b/biojava-core/src/test/java/org/biojava/nbio/core/util/FileDownloadUtilsTest.java
index 201ad88e48..4a3cae7fb7 100644
--- a/biojava-core/src/test/java/org/biojava/nbio/core/util/FileDownloadUtilsTest.java
+++ b/biojava-core/src/test/java/org/biojava/nbio/core/util/FileDownloadUtilsTest.java
@@ -4,6 +4,7 @@
import static org.biojava.nbio.core.util.FileDownloadUtils.getFilePrefix;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -11,6 +12,7 @@
import java.io.IOException;
import java.io.PrintStream;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import org.junit.jupiter.api.Nested;
@@ -187,17 +189,248 @@ void testValidationFiles() throws IOException{
assertTrue(sizeFile.exists(), "couldn't create size file");
assertTrue(FileDownloadUtils.validateFile(destFile), "file not detected to be invalid although there is correct size validation file");
+ // files.wwpdb.org returns the content MD5 as the ETag, so the default
+ // ETag policy records a real checksum without a separate hash URL.
+ assertTrue(hashFile.exists(), "no hash file was derived from the ETag");
+ assertTrue(FileDownloadUtils.validateFile(destFile), "correctly downloaded file failed hash validation");
+
PrintStream temp2 = new PrintStream(hashFile);
- temp2.print("ABCD"); // some wrong hash value
+ temp2.print("ABCD"); // not a digest of any supported length
temp2.close();
- //This is not yet implemented. I am using this test for documentation purpose.
- assertThrows(UnsupportedOperationException.class,
- () -> FileDownloadUtils.validateFile(destFile),
+ // An unreadable sidecar must not condemn an otherwise good download.
+ assertTrue(FileDownloadUtils.validateFile(destFile),
+ "a malformed hash file should be ignored, not treated as a mismatch");
+
+ PrintStream temp3 = new PrintStream(hashFile);
+ temp3.print("00000000000000000000000000000000"); // well-formed but wrong MD5
+ temp3.close();
+ assertFalse(FileDownloadUtils.validateFile(destFile),
"file not detected to be invalid although hash value is wrong.");
-
+ System.out.println("Just ignore the previous warning. It is expected.");
+
destFile.delete();
sizeFile.delete();
hashFile.delete();
}
}
+
+ @Nested
+ class HttpStatus {
+
+ @Test
+ void notFoundThrowsAndLeavesNothingBehind() throws IOException {
+ // A path that is guaranteed absent from the wwPDB archive.
+ URL missing = new URL("https://files.wwpdb.org/pub/pdb/data/structures/divided/mmCIF/zz/zzzz.cif.gz");
+ File dest = new File(System.getProperty("java.io.tmpdir"), "bj-missing.cif.gz");
+ File sizeFile = new File(dest.getParentFile(), dest.getName() + ".size");
+ dest.delete();
+ sizeFile.delete();
+
+ HttpStatusException e = assertThrows(HttpStatusException.class,
+ () -> FileDownloadUtils.downloadFile(missing, dest));
+ assertEquals(404, e.getStatusCode());
+ assertTrue(e.isNotFound());
+ assertFalse(dest.exists(), "a 404 body must never be written to the destination");
+
+ // ... and no validation metadata may be recorded for it either, or the
+ // cached error page would later pass validation.
+ FileDownloadUtils.createValidationFiles(missing, dest, null, FileDownloadUtils.Hash.UNKNOWN);
+ assertFalse(sizeFile.exists(), "no size file should be written for a 404 response");
+ }
+ }
+
+ @Nested
+ class Hashing {
+
+ private File writeTemp(String name, byte[] content) throws IOException {
+ File f = new File(System.getProperty("java.io.tmpdir"), name);
+ Files.write(f.toPath(), content);
+ f.deleteOnExit();
+ return f;
+ }
+
+ @Test
+ void digestsOfEmptyFileMatchKnownValues() throws IOException {
+ File empty = writeTemp("bj-empty.bin", new byte[0]);
+ assertEquals("d41d8cd98f00b204e9800998ecf8427e",
+ FileDownloadUtils.computeHash(empty, FileDownloadUtils.Hash.MD5));
+ assertEquals("da39a3ee5e6b4b0d3255bfef95601890afd80709",
+ FileDownloadUtils.computeHash(empty, FileDownloadUtils.Hash.SHA1));
+ assertEquals("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+ FileDownloadUtils.computeHash(empty, FileDownloadUtils.Hash.SHA256));
+ }
+
+ @Test
+ void digestOfKnownContent() throws IOException {
+ File abc = writeTemp("bj-abc.bin", "abc".getBytes(StandardCharsets.UTF_8));
+ assertEquals("900150983cd24fb0d6963f7d28e17f72",
+ FileDownloadUtils.computeHash(abc, FileDownloadUtils.Hash.MD5));
+ assertTrue(FileDownloadUtils.verifyHash(abc, FileDownloadUtils.Hash.MD5,
+ "900150983CD24FB0D6963F7D28E17F72"), "comparison should be case-insensitive");
+ assertFalse(FileDownloadUtils.verifyHash(abc, FileDownloadUtils.Hash.MD5,
+ "00000000000000000000000000000000"));
+ }
+
+ @Test
+ void algorithmNames() {
+ assertEquals("MD5", FileDownloadUtils.getAlgorithmName(FileDownloadUtils.Hash.MD5));
+ assertEquals("SHA-1", FileDownloadUtils.getAlgorithmName(FileDownloadUtils.Hash.SHA1));
+ assertEquals("SHA-256", FileDownloadUtils.getAlgorithmName(FileDownloadUtils.Hash.SHA256));
+ assertThrows(IllegalArgumentException.class,
+ () -> FileDownloadUtils.getAlgorithmName(FileDownloadUtils.Hash.UNKNOWN));
+ }
+ }
+
+ @Nested
+ class HashFileParsing {
+
+ private static final String MD5 = "900150983cd24fb0d6963f7d28e17f72";
+
+ private String parse(String content) throws IOException {
+ File f = new File(System.getProperty("java.io.tmpdir"), "bj-hashfile.txt");
+ Files.write(f.toPath(), content.getBytes(StandardCharsets.UTF_8));
+ f.deleteOnExit();
+ return FileDownloadUtils.parseHashFile(f);
+ }
+
+ @Test
+ void bareHex() throws IOException {
+ assertEquals(MD5, parse(MD5));
+ assertEquals(MD5, parse(MD5 + "\n"));
+ }
+
+ @Test
+ void uppercaseHexIsKeptVerbatim() throws IOException {
+ assertEquals(MD5.toUpperCase(), parse(MD5.toUpperCase()));
+ }
+
+ @Test
+ void coreutilsLayouts() throws IOException {
+ assertEquals(MD5, parse(MD5 + " somefile.cif.gz\n"));
+ assertEquals(MD5, parse(MD5 + " *somefile.cif.gz\n"));
+ }
+
+ @Test
+ void bsdLayout() throws IOException {
+ assertEquals(MD5, parse("MD5 (somefile.cif.gz) = " + MD5 + "\n"));
+ }
+
+ @Test
+ void blankLeadingLinesAreSkipped() throws IOException {
+ assertEquals(MD5, parse("\n \n" + MD5 + "\n"));
+ }
+
+ @Test
+ void garbageYieldsNull() throws IOException {
+ assertNull(parse("not a hash at all\n"));
+ assertNull(parse("ABCD\n"));
+ assertNull(parse(""));
+ }
+ }
+
+ @Nested
+ class ETagParsing {
+
+ @Test
+ void wwpdbStyleMd5IsRecognised() {
+ assertEquals(FileDownloadUtils.Hash.MD5,
+ FileDownloadUtils.hashFromETag("\"f99fb9d964e1e1c22f2ea559ac5745cf\""));
+ assertEquals("f99fb9d964e1e1c22f2ea559ac5745cf",
+ FileDownloadUtils.normalizeETag("\"f99fb9d964e1e1c22f2ea559ac5745cf\""));
+ }
+
+ @Test
+ void sha1AndSha256LengthsAreRecognised() {
+ assertEquals(FileDownloadUtils.Hash.SHA1,
+ FileDownloadUtils.hashFromETag("da39a3ee5e6b4b0d3255bfef95601890afd80709"));
+ assertEquals(FileDownloadUtils.Hash.SHA256,
+ FileDownloadUtils.hashFromETag(
+ "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"));
+ }
+
+ @Test
+ void ebiStyleTimeSizeETagIsNotMistakenForADigest() {
+ // nginx and Apache emit
+ * The version is read from the file's header without parsing the domains, so this
+ * additionally parses the first few thousand lines of the same file. That is enough to
+ * notice a column change — which is what ECOD did at v294.1, unnoticed for months —
+ * without building the three million domains the whole file now holds.
+ */
@Test
public void testVersion() throws IOException {
EcodDatabase ecod3 = EcodFactory.getEcodDatabase("latest");
String version = ecod3.getVersion();
assertNotNull(version);
assertNotEquals("latest", version);
+ System.out.println("latest version of ECOD is "+version);
+
+ File domainsFile = new File(((EcodInstallation) ecod3).getCacheLocation(),
+ "ecod.latest.domains.txt");
+ assertTrue("No local copy of the domains file at "+domainsFile, domainsFile.exists());
+
+ EcodParser parser = new EcodParser(firstLines(domainsFile, 5000));
+ assertEquals(version, parser.getVersion());
+ assertFalse("No domains parsed from ECOD "+version
+ + "; the distribution format has probably changed",
+ parser.getDomains().isEmpty());
+ }
+
+ /**
+ * @return a reader over the first {@code maxLines} lines of the file
+ */
+ private static Reader firstLines(File f, int maxLines) throws IOException {
+ StringBuilder head = new StringBuilder();
+ try (BufferedReader in = new BufferedReader(new FileReader(f))) {
+ String line;
+ int n = 0;
+ while (n < maxLines && (line = in.readLine()) != null) {
+ head.append(line).append('\n');
+ n++;
+ }
+ }
+ return new StringReader(head.toString());
}
/**
diff --git a/biojava-structure/pom.xml b/biojava-structure/pom.xml
index f881200d48..647ab49c72 100644
--- a/biojava-structure/pom.xml
+++ b/biojava-structure/pom.xml
@@ -87,7 +87,7 @@
+ * Parallel primitive arrays are used rather than an array of index-distance objects because there are ~30
+ * neighbors per atom: for a large structure that would mean millions of small short-lived objects.
+ */
+ static class Neighbors {
+
+ /** The neighbor atom indices, ordered by increasing distance to the central atom */
+ final int[] indices;
+ /** The distances to the central atom, in increasing order and parallel to {@link #indices} */
+ final double[] dists;
+
+ private Neighbors(int[] indices, double[] dists) {
+ this.indices = indices;
+ this.dists = dists;
+ }
+
+ /**
+ * Creates a Neighbors from the first count elements of the given buffers, copying them to exact-size arrays
+ * and sorting them by increasing distance.
+ * @param indicesBuffer the neighbor indices, only the first count elements are used
+ * @param distsBuffer the neighbor distances, only the first count elements are used
+ * @param count the number of neighbors
+ * @return the sorted neighbors
+ */
+ static Neighbors createSorted(int[] indicesBuffer, double[] distsBuffer, int count) {
+ int[] indices = Arrays.copyOf(indicesBuffer, count);
+ double[] dists = Arrays.copyOf(distsBuffer, count);
+ // Sorting by closest to farthest away neighbors achieves faster runtimes when checking for occluded
+ // sphere sample points in calcSingleAsa. This follows the ideas exposed in
+ // Eisenhaber et al, J Comp Chemistry 1994 (https://onlinelibrary.wiley.com/doi/epdf/10.1002/jcc.540160303)
+ // This is essential for performance: it brings down the number of occlusion checks to
+ // an average of n_sphere_points/10 per atom, producing ~ x4 performance gain overall.
+ // An insertion sort is used because the arrays are small (~30 elements on average) and because it avoids
+ // both the boxing of a comparator-based sort and the object allocation an index-distance array would need.
+ for (int i = 1; i < count; i++) {
+ double dist = dists[i];
+ int index = indices[i];
+ int j = i - 1;
+ while (j >= 0 && dists[j] > dist) {
+ dists[j + 1] = dists[j];
+ indices[j + 1] = indices[j];
+ j--;
+ }
+ dists[j + 1] = dist;
+ indices[j + 1] = index;
+ }
+ return new Neighbors(indices, dists);
}
}
@@ -116,9 +159,16 @@ static class IndexAndDistance {
private final double[] radii;
private final double probe;
private final int nThreads;
- private Vector3d[] spherePoints;
+ /**
+ * The sphere points to sample, as a flat array of interleaved x,y,z coordinates (thus of size 3 x nSpherePoints).
+ * A flat array of primitives (rather than an array of Vector3d objects) is used for performance: it keeps the
+ * points contiguous in memory and avoids a pointer dereference per point in the innermost loop of
+ * {@link #calcSingleAsa(int)}.
+ */
+ private double[] spherePoints;
+ private int nSpherePoints;
private double cons;
- private IndexAndDistance[][] neighborIndices;
+ private Neighbors[] neighbors;
private boolean useSpatialHashingForNeighbors;
@@ -239,7 +289,8 @@ private void initSpherePoints(int nSpherePoints) {
logger.debug("Will use {} sphere points", nSpherePoints);
// initialising the sphere points to sample
- spherePoints = generateSpherePoints(nSpherePoints);
+ this.nSpherePoints = nSpherePoints;
+ this.spherePoints = generateSpherePoints(nSpherePoints);
cons = 4.0 * Math.PI / nSpherePoints;
}
@@ -285,10 +336,10 @@ public double[] calculateAsas() {
long start = System.currentTimeMillis();
if (useSpatialHashingForNeighbors) {
logger.debug("Will use spatial hashing to find neighbors");
- neighborIndices = findNeighborIndicesSpatialHashing();
+ neighbors = findNeighborIndicesSpatialHashing();
} else {
logger.debug("Will not use spatial hashing to find neighbors");
- neighborIndices = findNeighborIndices();
+ neighbors = findNeighborIndices();
}
long end = System.currentTimeMillis();
logger.debug("Took {} s to find neighbors", (end-start)/1000.0);
@@ -334,109 +385,126 @@ void setUseSpatialHashingForNeighbors(boolean useSpatialHashingForNeighbors) {
* Returns list of 3d coordinates of points on a unit sphere using the
* Golden Section Spiral algorithm.
* @param nSpherePoints the number of points to be used in generating the spherical dot-density
- * @return the array of points as Vector3d objects
+ * @return a flat array of interleaved x,y,z coordinates, of size 3 x nSpherePoints
*/
- private Vector3d[] generateSpherePoints(int nSpherePoints) {
- Vector3d[] points = new Vector3d[nSpherePoints];
+ private double[] generateSpherePoints(int nSpherePoints) {
+ double[] points = new double[3 * nSpherePoints];
double inc = Math.PI * (3.0 - Math.sqrt(5.0));
double offset = 2.0 / nSpherePoints;
for (int k=0;k
+ * Since 7.3.0 this reads only the file's header rather than parsing the whole
+ * file, so it no longer has the side effect of loading every domain.
* @return the ECOD version
* @throws IOException If an error occurs while downloading or parsing the file
*/
@Override
public String getVersion() throws IOException {
- ensureDomainsFileInstalled();
+ domainsFileLock.readLock().lock();
+ logger.trace("LOCK readlock");
+ try {
+ if( parsedVersion != null ) {
+ return parsedVersion;
+ }
+ } finally {
+ logger.trace("UNLOCK readlock");
+ domainsFileLock.readLock().unlock();
+ }
+
+ // The version is declared in the first few lines of the file, so read those rather
+ // than the millions of domain records behind them. The current release is 657 MB and
+ // holds nearly three million records; parsing it in full to answer this question
+ // costs over a gigabyte of heap and several seconds.
+ ensureDomainsFileDownloaded();
+
+ domainsFileLock.writeLock().lock();
+ logger.trace("LOCK writelock");
+ try {
+ if( parsedVersion == null ) {
+ parsedVersion = parseVersionOnly();
+ }
+ } finally {
+ logger.trace("UNLOCK writelock");
+ domainsFileLock.writeLock().unlock();
+ }
if( parsedVersion == null) {
return requestedVersion;
@@ -285,6 +326,30 @@ public String getVersion() throws IOException {
return parsedVersion;
}
+ /**
+ * Reads the version from the header of the local domains file without parsing the
+ * domains themselves.
+ * @return the version, or null if the header does not declare one
+ * @throws IOException if the file cannot be read
+ * @since 7.3.0
+ */
+ private String parseVersionOnly() throws IOException {
+ try( BufferedReader in = new BufferedReader(new FileReader(getDomainFile())) ) {
+ String line;
+ while( (line = in.readLine()) != null ) {
+ Matcher match = EcodParser.VERSION_RE.matcher(line);
+ if( match.matches() ) {
+ return match.group(1);
+ }
+ if( !line.startsWith("#") ) {
+ // past the header block; from v294.1 the column names are not commented
+ return null;
+ }
+ }
+ }
+ return null;
+ }
+
/**
* Get the top-level ECOD server URL. Defaults to "http://prodata.swmed.edu"
* @return the url to the ecod server
@@ -325,6 +390,24 @@ public void setCacheLocation(String cacheLocation) {
domainsFileLock.writeLock().unlock();
}
+ /**
+ * Ensures the domains file is present and current locally, without parsing it.
+ * @throws IOException in cases of file I/O, including failure to download a healthy file
+ * @since 7.3.0
+ */
+ private void ensureDomainsFileDownloaded() throws IOException {
+ domainsFileLock.writeLock().lock();
+ logger.trace("LOCK writelock");
+ try {
+ if( !domainsAvailable() ) {
+ downloadDomains();
+ }
+ } finally {
+ logger.trace("UNLOCK writelock");
+ domainsFileLock.writeLock().unlock();
+ }
+ }
+
/**
* Blocks until ECOD domains file has been downloaded and parsed.
*
@@ -549,6 +632,24 @@ Current version (1.4) contains the following columns:
v1.2 - added f-group identifiers to fasta file, domain description file. ECODf identifiers now used when available for F-group name.
Domain assemblies now represented by assembly uid in domain assembly status.
v1.4 - added seqid_range and headers (develop101)
+v1.6 - renamed column 4 from f_id to t_id and inserted unp_acc (UniProt accession) as
+ column 9, giving 16 columns (seen in develop291)
+
+From v294.1 the distribution was redesigned. The header comment changed from
+"#ECOD version develop291" to "# Version: v294.1", the column header row is no longer
+commented out, and the columns became:
+
+ uid ecod_domain_id manual_rep f_id pdb chain pdb_range seqid_range architecture_name
+ x_name h_name t_name f_name assembly_id domain_id_short range_count arch_manual
+ x_manual h_manual t_manual f_manual valid_structure ligand_binding
+
+v295 appends ligand_comp_ids and ligand_pdbnum, for 25 columns. Also note that
+manual_rep now holds True/False rather than MANUAL_REP/AUTO_NONREP, that assembly_id
+and domain_id_short are empty on every row, that f_name is empty rather than
+F_UNCLASSIFIED for unclassified domains, and that uid restarts from 0.
+
+Because the columns have been renamed, reordered and added to repeatedly, files that
+declare a column header are read by column name rather than by position.
*/
/** String for unclassified F-groups */
@@ -561,10 +662,28 @@ Current version (1.4) contains the following columns:
public static final String IS_REPRESENTATIVE = "MANUAL_REP";
/** Indicates not a manual representative */
public static final String NOT_REPRESENTATIVE = "AUTO_NONREP";
+ /**
+ * Matches the comment declaring the version, which has taken two forms:
+ * {@code #ECOD version develop291} up to develop292, and {@code # Version: v295}
+ * from v294.1 onwards.
+ * @since 7.3.0
+ */
+ static final Pattern VERSION_RE = Pattern.compile(
+ "^\\s*#\\s*(?:ECOD\\s+)?version\\s*:?\\s*(\\S+).*", Pattern.CASE_INSENSITIVE);
private List
+ * Every distribution since develop101 carries such a header. It is commented
+ * (
+ * The characters are taken relative to the end of the identifier rather
+ * than the start, so that both spellings of the same entry land in the same
+ * bucket:
+ * This is the failure that took the CATH downloader out when
+ *
+ * The test serves the responses from a local {@link HttpServer} rather than a real
+ * service. Pointing it at a third-party server that happens to redirect today would
+ * make the test fail on the day they stop, which is precisely the coupling that made
+ * the build unreliable in the first place.
+ *
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+public class TestChemCompRedirectNotCached {
+
+ private HttpServer server;
+ private String originalServerUrl;
+
+ @Before
+ public void setUp() {
+ originalServerUrl = DownloadChemCompProvider.serverBaseUrl;
+ }
+
+ @After
+ public void tearDown() {
+ if (server != null) {
+ server.stop(0);
+ }
+ // Static state: leaving either of these set would corrupt unrelated tests.
+ DownloadChemCompProvider.serverBaseUrl = originalServerUrl;
+ FlatFileCache.clear();
+ }
+
+ /**
+ * Starts a local server that answers every request with the given status and body.
+ *
+ * @return the base URL to point the provider at
+ */
+ private String startServer(int status, String location, String body) throws IOException {
+ server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+ server.createContext("/", exchange -> {
+ byte[] bytes = body.getBytes(StandardCharsets.UTF_8);
+ if (location != null) {
+ exchange.getResponseHeaders().add("Location", location);
+ }
+ exchange.sendResponseHeaders(status, bytes.length);
+ try (OutputStream out = exchange.getResponseBody()) {
+ out.write(bytes);
+ }
+ });
+ server.start();
+ return "http://127.0.0.1:" + server.getAddress().getPort() + "/";
+ }
+
+ private File cacheFileFor(String id) {
+ File file = new File(DownloadChemCompProvider.getLocalFileName(id));
+ file.delete();
+ FlatFileCache.clear();
+ return file;
+ }
+
+ /**
+ * The case that broke CATH: a redirect the JDK will not follow because it
+ * changes protocol. Its body must not end up on disk under the component's name.
+ */
+ @Test
+ public void redirectBodyIsNotCached() throws IOException {
+ File cached = cacheFileFor("ATP");
+ DownloadChemCompProvider.serverBaseUrl =
+ startServer(301, "https://example.invalid/ATP.cif", "Moved Permanently");
+
+ ChemComp cc = new DownloadChemCompProvider().getChemComp("ATP");
+
+ assertFalse("the body of a redirect must never be cached as a definition", cached.exists());
+ assertNull("nothing parseable was returned, so the component must be empty", cc.getName());
+ }
+
+ /**
+ * A 200 is still cached, so the guard has not simply disabled downloading.
+ *
+ * What is under test is the download path, not the CIF parser: the response is
+ * written to the cache before anything tries to parse it, so a parse failure on
+ * this deliberately minimal body says nothing about whether the guard behaved.
+ */
+ @Test
+ public void aValidResponseIsStillCached() throws IOException {
+ File cached = cacheFileFor("ATP");
+ DownloadChemCompProvider.serverBaseUrl = startServer(200, null,
+ "data_ATP\n#\n_chem_comp.id ATP\n_chem_comp.name \"ADENOSINE-5'-TRIPHOSPHATE\"\n#\n");
+
+ try {
+ new DownloadChemCompProvider().getChemComp("ATP");
+ } catch (RuntimeException parseFailure) {
+ // see the note above
+ }
+
+ assertTrue("a 200 response should still be cached", cached.exists());
+ cached.delete();
+ }
+
+ /** A server error must not be cached either. */
+ @Test
+ public void serverErrorBodyIsNotCached() throws IOException {
+ File cached = cacheFileFor("ATP");
+ DownloadChemCompProvider.serverBaseUrl =
+ startServer(503, null, "Service Unavailable");
+
+ new DownloadChemCompProvider().getChemComp("ATP");
+
+ assertFalse("the body of a 5xx must never be cached as a definition", cached.exists());
+ }
+}
diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java
new file mode 100644
index 0000000000..295806e915
--- /dev/null
+++ b/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java
@@ -0,0 +1,323 @@
+/*
+ * BioJava development code
+ *
+ * This code may be freely distributed and modified under the
+ * terms of the GNU Lesser General Public Licence. This should
+ * be distributed with the code. If you do not have a copy,
+ * see:
+ *
+ * http://www.gnu.org/copyleft/lesser.html
+ *
+ * Copyright for this code is held jointly by the individual
+ * authors. These should be listed in @author doc comments.
+ *
+ * For more information on the BioJava project and its aims,
+ * or to join the biojava-l mailing list, visit the home page
+ * at:
+ *
+ * http://www.biojava.org/
+ */
+package org.biojava.nbio.structure.ecod;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+
+import org.biojava.nbio.structure.ecod.EcodInstallation.EcodParser;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Checks that {@link EcodParser} reads every layout ECOD has distributed.
+ *
+ * The columns have been renamed, reordered and added to several times, and the version
+ * comment itself changed form at v294.1. Because the full distribution is 657 MB, none of
+ * that was covered by a test that could run in reasonable time, and a format change went
+ * unnoticed for months. These fixtures are taken verbatim from the real files, so the
+ * contract is pinned in milliseconds rather than by a download.
+ *
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+class EcodParserTest {
+
+ /** develop204, list format 1.5: 15 columns, commented header, quoted names. */
+ private static final String DEVELOP204 = String.join("\n",
+ "#/data/ecod/database_versions/v204/ecod.develop204.domains.txt",
+ "#ECOD version develop204",
+ "#Domain list version 1.5",
+ "#Grishin lab (http://prodata.swmed.edu/ecod)",
+ "#uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range"
+ + "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand",
+ "002137905\te6b4nA1\tAUTO_NONREP\t1.1.1\t6b4n\tA\tA:1-99\tA:1-99\tbeta barrels"
+ + "\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\""
+ + "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tCL,G53,NA");
+
+ /** develop291, list format 1.6: 16 columns, f_id renamed t_id, unp_acc inserted at 9. */
+ private static final String DEVELOP291 = String.join("\n",
+ "#/data/ecod/database_versions/v291/ecod.develop291.domains.txt",
+ "#ECOD version develop291",
+ "#Domain list version 1.6",
+ "#Grishin lab (http://prodata.swmed.edu/ecod)",
+ "#uid\tecod_domain_id\tmanual_rep\tt_id\tpdb\tchain\tpdb_range\tseqid_range\tunp_acc"
+ + "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand",
+ "000000267\te1udzA1\tMANUAL_REP\t1.1.1\t1udz\tA\tA:203-381\tA:4-182\tP12345"
+ + "\tbeta barrels\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\""
+ + "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tNO_LIGANDS_4A");
+
+ private static final String V295_COLUMNS =
+ "uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range"
+ + "\tarchitecture_name\tx_name\th_name\tt_name\tf_name\tassembly_id\tdomain_id_short"
+ + "\trange_count\tarch_manual\tx_manual\th_manual\tt_manual\tf_manual"
+ + "\tvalid_structure\tligand_binding\tligand_comp_ids\tligand_pdbnum";
+
+ /** v295: 25 columns, uncommented header, True/False, empty assembly_id, moved ligands. */
+ private static final String V295 = String.join("\n",
+ "# ECOD Domain List",
+ "# Version: v295",
+ "# Generated: 2026-06-24 22:47:42",
+ "# Ligand cutoff: 4.0 A (NO_LIGANDS_4A = no contact within cutoff)",
+ "#",
+ V295_COLUMNS,
+ "0\te2nmzA1\tTrue\t1.1.1.3\t2nmz\tA\tA:1-99\tA:1-99\tbeta barrels\tcradle loop barrel"
+ + "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue"
+ + "\tTrue\tTrue\tROC,SO4\tA:601,A:602,B:401",
+ // the last column is empty on four rows in five, so split() must keep it
+ "3\te2rspA1\tTrue\t1.1.1.3\t2rsp\tA\tA:1-124\tA:1-124\tbeta barrels\tcradle loop barrel"
+ + "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue"
+ + "\tTrue\tFalse\tNO_LIGANDS_4A\t",
+ // a domain classified from an AlphaFold model: no PDB entry, so no EcodDomain
+ "3163557\tP44140_F1_nD2\tFalse\t2004.1.1.123\t\t\t131-315\t131-315\talpha bundles"
+ + "\tsomething\tsomething else\ta third thing\t\t\t\t1\tFalse\tFalse\tFalse"
+ + "\tFalse\tTrue\tTrue\tFalse\tNO_LIGANDS_4A\t");
+
+ private static List#uid<tab>ecod_domain_id<tab>...) up to develop292 and
+ * uncommented (uid<tab>ecod_domain_id<tab>...) from v294.1
+ * onwards. Because names have also been changed between versions, lookups accept
+ * aliases and any name the file does not declare simply reads as absent.
+ *
+ * @author Amr ALHOSSARY
+ * @since 7.3.0
+ */
+ private static class ColumnLayout {
+ private final Mapcb for 1cbs.
+ * 1cbs and its extended form pdb_00001cbs both
+ * give cb. Taking them from the start would file the extended form
+ * under db instead. The extended PDB identifier format is expected
+ * to keep using this same hashing scheme.
+ *
+ * @param pdbId a PDB identifier, in either the short or the extended form
+ * @return the lowercase two-character directory name
+ * @since 7.3.0
+ */
+ public static String getMiddleHash(String pdbId) {
+ int offset = pdbId.length() - 3;
+ return pdbId.substring(offset, offset + 2).toLowerCase();
+ }
+
/**
* Get the last modified time of the file in given url by retrieveing the "Last-Modified" header.
* Note that this only works for http URLs
* @param url
* @return the last modified date or null if it couldn't be retrieved (in that case a warning will be logged)
+ * @since 7.3.0 made public so that other caching code can reuse it
*/
- private Date getLastModifiedTime(URL url) {
+ public static Date getLastModifiedTime(URL url) {
// see http://stackoverflow.com/questions/2416872/how-do-you-obtain-modified-date-from-a-remote-file-java
Date date = null;
@@ -629,14 +653,12 @@ private Date getLastModifiedTime(URL url) {
protected File getDir(String pdbId, boolean obsolete) {
File dir = null;
- int offset = pdbId.length() - 3;
+ String middle = getMiddleHash(pdbId);
if (obsolete) {
// obsolete is always split
- String middle = pdbId.substring(offset, offset + 2).toLowerCase();
dir = new File(obsoleteDirPath, middle);
} else {
- String middle = pdbId.substring(offset, offset + 2).toLowerCase();
dir = new File(splitDirPath, middle);
}
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java
index f5cc851fec..b1d327599e 100644
--- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java
+++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java
@@ -1406,7 +1406,7 @@ public void handleResolutionLine(String line, Pattern pR) {
try {
float res = Float.parseFloat(resString);
final float resInHeader = pdbHeader.getResolution();
- if (resInHeader!=PDBHeader.DEFAULT_RESOLUTION && resInHeader != res) {
+ if (resInHeader!=PDBHeader.DEFAULT_RESOLUTION && Math.abs(resInHeader - res) > 0.001) {
logger.warn("More than 1 resolution value present, will use last one {} and discard previous {} "
,resString, String.format("%4.2f",resInHeader));
}
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java
index 6bf8af90ef..e43565c827 100644
--- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java
+++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java
@@ -40,8 +40,27 @@ protected CifFile getInternal(Structure structure, Listdownload.cathdb.info moved to https, and the chem comp download had
+ * the same shape. A 4xx already failed safely, because
+ * getInputStream() throws for those; a redirect did not, because when
+ * the JDK declines to follow a 3xx it hands back the redirect's body instead, and
+ * that body is short but not empty.
+ *