From b8e69a4b022aaa6b5f5ecfbd31de70e32358ce85 Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Mon, 21 Feb 2022 07:35:59 +0800 Subject: [PATCH 1/8] Two utility methods to validate the downloaded file Currently, we validate the file size only. We could validate the content using any hashing function later. --- .../nbio/core/util/FileDownloadUtils.java | 61 +++++++++++++++++++ .../nbio/structure/ecod/EcodFactory.java | 3 + .../nbio/structure/ecod/EcodInstallation.java | 5 +- .../nbio/structure/io/LocalPDBDirectory.java | 8 +++ .../io/sifts/SiftsMappingProvider.java | 5 ++ .../nbio/structure/scop/ScopInstallation.java | 11 ++-- 6 files changed, 88 insertions(+), 5 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index e3b678ce8d..51e0718382 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -23,9 +23,11 @@ import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.PrintStream; import java.net.HttpURLConnection; import java.net.SocketTimeoutException; import java.net.URL; @@ -39,6 +41,7 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; +import java.util.Scanner; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -161,6 +164,64 @@ public static void downloadFile(URL url, File destination) throws IOException { tempFile.delete(); } + + public static void createValidationFiles(URL url, File localDestination, URL hashURL){ + try { + URLConnection resourceConnection = url.openConnection(); + createValidationFiles(resourceConnection, localDestination, hashURL); + } catch (IOException e) { + logger.warn("could not open connection to resource file due to exception", e); + } + } + public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL){ + long size = resourceUrlConnection.getContentLengthLong(); + if(size != -1) { + System.out.println("Content-Length: " + size); + File sizeFile = new File(localDestination.getParentFile(), localDestination.getName()+".size"); + try (PrintStream sizePrintStream = new PrintStream(sizeFile)) { + sizePrintStream.print(size); + sizePrintStream.close(); + } catch (FileNotFoundException e) { + logger.warn("could not write validation size file due to exception", e); + } + } + + if(hashURL == null) + return; + + try { + File hashFile = new File(localDestination.getParentFile(), localDestination.getName()+".hash"); + downloadFile(hashURL, hashFile); + } catch (IOException e) { + logger.warn("could not write validation hash file due to exception", e); + } + } + + public static boolean validateFile(File localFile) { + File sizeFile = new File(localFile.getParentFile(), localFile.getName()+".size"); + if(sizeFile.exists()) { + Scanner scanner = null; + try { + scanner = new Scanner(sizeFile); + long expectedSize = scanner.nextLong(); + if (expectedSize != localFile.length()) { + logger.warn("File size does not match expected"); + return false; + } + } catch (FileNotFoundException e) { + logger.warn("could not validate size of file ["+ localFile+ "] due to exception", e); + } finally { + scanner.close(); + } + } + + File hashFile = new File(localFile.getParentFile(), localFile.getName()+".hash"); + if(hashFile.exists()) { + throw new UnsupportedOperationException("Not yet implemented"); + } + + return true; + } /** * Converts path to Unix convention and adds a terminating slash if it was diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java index 89791c59e1..0f8f7b6654 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java @@ -89,6 +89,9 @@ public static EcodDatabase getEcodDatabase(String version) { } } catch (IOException e) { // For parsing errors, just use the requested version + // What about corrupted downloading errors?? Amr + logger.warn("Cound not get Ecod version, or file is corrupted", e); + return null; } } logger.trace("Releasing EcodFactory lock after getting version "+version); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java index 004e2634ac..e89dab092a 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java @@ -369,7 +369,7 @@ private boolean domainsAvailable() { try { File f = getDomainFile(); - if (!f.exists() || f.length() <= 0 ) + if (! (f.exists() && FileDownloadUtils.validateFile(f))) return false; // Re-download old copies of "latest" @@ -406,7 +406,10 @@ private void downloadDomains() throws IOException { File localFile = getDomainFile(); logger.info("Downloading {} to: {}",domainsURL, localFile); + FileDownloadUtils.createValidationFiles(domainsURL, localFile, null); FileDownloadUtils.downloadFile(domainsURL, localFile); + if(! FileDownloadUtils.validateFile(localFile)) + throw new IOException("Downloaded file invalid: "+ localFile); } catch (MalformedURLException e) { logger.error("Malformed url: "+ url + DOMAINS_PATH + getDomainFilename(),e); } finally { diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java index 0051180acb..4b4cc000fd 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java @@ -373,6 +373,9 @@ protected InputStream getInputStream(PdbId pdbId) throws IOException{ throw new IOException("Structure "+pdbId+" not found and unable to download."); } + if(! FileDownloadUtils.validateFile(file)) + throw new IOException("Downloaded file invalid: "+file); + InputStreamProvider isp = new InputStreamProvider(); InputStream inputStream = isp.getInputStream(file); @@ -395,6 +398,8 @@ public void prefetchStructure(String pdbId) throws IOException { if(!file.exists()) { throw new IOException("Structure "+pdbId+" not found and unable to download."); } + if(! FileDownloadUtils.validateFile(file)) + throw new IOException("Downloaded file invalid: "+file); } /** @@ -576,7 +581,10 @@ private File downloadStructure(PdbId pdbId, String pathOnServer, boolean obsolet logger.info("Fetching " + ftp); logger.info("Writing to "+ realFile); + FileDownloadUtils.createValidationFiles(url, realFile, null); FileDownloadUtils.downloadFile(url, realFile); + if(! FileDownloadUtils.validateFile(realFile)) + throw new IOException("Downloaded file invalid: "+realFile); return realFile; } diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java index ed697b55fd..3701c87e29 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java @@ -87,10 +87,15 @@ public static List getSiftsMapping(String pdbId) throws IOException if ( ! dest.exists()){ String u = String.format(fileLoc,pdbId); URL url = new URL(u); + logger.debug("Downloading SIFTS file {} validation metadata.",url); + FileDownloadUtils.createValidationFiles(url, dest, null); logger.debug("Downloading SIFTS file {} to {}",url,dest); FileDownloadUtils.downloadFile(url, dest); } + if(! FileDownloadUtils.validateFile(dest)) + throw new IOException("Downloaded file invalid: "+dest); + InputStreamProvider prov = new InputStreamProvider(); InputStream is = prov.getInputStream(dest); SiftsXMLParser parser = new SiftsXMLParser(); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java index 252ccfabfb..bcf4f32306 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java @@ -741,7 +741,10 @@ protected void downloadComFile() throws FileNotFoundException, IOException{ protected void downloadFileFromRemote(URL remoteURL, File localFile) throws IOException{ logger.info("Downloading " + remoteURL + " to: " + localFile); + FileDownloadUtils.createValidationFiles(remoteURL, localFile, null); FileDownloadUtils.downloadFile(remoteURL, localFile); + if(! FileDownloadUtils.validateFile(localFile)) + throw new IOException("Downloaded file invalid: "+localFile); } private boolean claFileAvailable(){ @@ -749,14 +752,14 @@ private boolean claFileAvailable(){ File f = new File(fileName); - return f.exists() && f.length()>0; + return f.exists() && FileDownloadUtils.validateFile(f); } private boolean desFileAvailable(){ String fileName = getDesFilename(); File f = new File(fileName); - return f.exists() && f.length()>0; + return f.exists() && FileDownloadUtils.validateFile(f); } private boolean hieFileAvailable(){ @@ -764,7 +767,7 @@ private boolean hieFileAvailable(){ File f = new File(fileName); - return f.exists() && f.length()>0; + return f.exists() && FileDownloadUtils.validateFile(f); } private boolean comFileAvailable(){ @@ -772,7 +775,7 @@ private boolean comFileAvailable(){ File f = new File(fileName); - return f.exists() && f.length()>0; + return f.exists() && FileDownloadUtils.validateFile(f); } protected String getClaFilename(){ From aed24fad9664484780dce00fb3f073921e34ae36 Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Mon, 21 Feb 2022 18:09:29 +0800 Subject: [PATCH 2/8] JavaDock --- .../nbio/core/util/FileDownloadUtils.java | 38 +++++++++++++++++++ .../nbio/structure/ecod/EcodInstallation.java | 4 +- .../nbio/structure/io/LocalPDBDirectory.java | 8 ++-- .../nbio/structure/scop/ScopInstallation.java | 6 +++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index 51e0718382..0371f1d86a 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -165,6 +165,17 @@ public static void downloadFile(URL url, File destination) throws IOException { } + /** + * Creates validation files beside a file to be downloaded.
+ * Whenever possible, for a file.ext file, it creates + * file.ext.size and file.hash for in the same + * folder where file.ext exists. + * If the file connection size could not be deduced from the URL, no size file is created. + * If hashURL is null, no hash file is created. + * @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. + */ public static void createValidationFiles(URL url, File localDestination, URL hashURL){ try { URLConnection resourceConnection = url.openConnection(); @@ -173,6 +184,19 @@ public static void createValidationFiles(URL url, File localDestination, URL has logger.warn("could not open connection to resource file due to exception", e); } } + /** + * Creates validation files beside a file to be downloaded.
+ * Whenever possible, for a file.ext file, it creates + * file.ext.size and file.hash for in the same + * folder where file.ext exists. + * If the file connection size could not be deduced from the resourceUrlConnection + * {@link URLConnection}, no size file is created. + * If hashURL is null, no hash file is created. + * @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. + * @since 6.0.6 + */ public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL){ long size = resourceUrlConnection.getContentLengthLong(); if(size != -1) { @@ -197,6 +221,20 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi } } + /** + * Validate a local file based on pre-existing metadata files for size and hash.
+ * If the passed in localFile parameter is a file named file.ext, the function searches in the same folder for: + *
    + *
  • file.ext.size: if found, it compares the size stored in it to the length of localFile (in bytes).
    • + *
  • file.ext.hash: if found, it compares the size stored in it to the hash code of localFile.
    • + *
+ * If any of these comparisons fail, the function returns false. otherwise it returns true. + *

+ * This function does not implement hash code verification yet. + * @param localFile The file to validate + * @return false if any of the size or hash code metadata files exists but its contents does not match the expected value in the file, true' otherwise. + * @since 6.0.6 + */ public static boolean validateFile(File localFile) { File sizeFile = new File(localFile.getParentFile(), localFile.getName()+".size"); if(sizeFile.exists()) { diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java index e89dab092a..74f49f8dd5 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java @@ -395,8 +395,8 @@ private boolean domainsAvailable() { } /** - * Downloads the domains file, overwriting any existing file - * @throws IOException + * Downloads the domains file +/- its validation metadata, overwriting any existing file + * @throws IOException in cases of file I/O, including failure to download a healthy (non-corrupted) file. */ private void downloadDomains() throws IOException { domainsFileLock.writeLock().lock(); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java index 4b4cc000fd..dce7e9853d 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java @@ -362,7 +362,7 @@ public Structure getStructureById(PdbId pdbId) throws IOException { * for direct parsing. * @param pdbId * @return - * @throws IOException + * @throws IOException in cases of file I/O, including failure to download a healthy (non-corrupted) file. */ protected InputStream getInputStream(PdbId pdbId) throws IOException{ @@ -388,7 +388,7 @@ protected InputStream getInputStream(PdbId pdbId) throws IOException{ * * Used to pre-fetch large numbers of structures. * @param pdbId - * @throws IOException + * @throws IOException in cases of file I/O, including failure to download a healthy (non-corrupted) file. */ public void prefetchStructure(String pdbId) throws IOException { @@ -530,14 +530,14 @@ protected File downloadStructure(PdbId pdbId) throws IOException { } /** - * Download a file from the ftp server, replacing any existing files if needed + * Download a file from the ftp server +/- its validation metadata, replacing any existing files if needed * @param pdbId PDB ID * @param pathOnServer Path on the FTP server, e.g. data/structures/divided/pdb * @param obsolete Whether or not file should be saved to the obsolete location locally * @param existingFile if not null and checkServerFileDate is true, the last modified date of the * server file and this file will be compared to decide whether to download or not * @return - * @throws IOException + * @throws IOException in cases of file I/O, including failure to download a healthy (non-corrupted) file. */ private File downloadStructure(PdbId pdbId, String pathOnServer, boolean obsolete, File existingFile) throws IOException{ diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java index bcf4f32306..e46dbca9f1 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java @@ -739,6 +739,12 @@ protected void downloadComFile() throws FileNotFoundException, IOException{ throw new IOException("Unable to download SCOP .com file",exception); } + /** + * Downloads the SCOP installation file +/- its validation metadata files. + * @param remoteURL The remote file to download + * @param localFile the local file to download to + * @throws IOException in cases of file I/O, including failure to download a healthy (non-corrupted) file. + */ protected void downloadFileFromRemote(URL remoteURL, File localFile) throws IOException{ logger.info("Downloading " + remoteURL + " to: " + localFile); FileDownloadUtils.createValidationFiles(remoteURL, localFile, null); From f59de95988bde2e1e6cb5fd3042d1f8ee3175ebc Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Tue, 22 Feb 2022 09:08:10 +0800 Subject: [PATCH 3/8] more verbose --- .../java/org/biojava/nbio/core/util/FileDownloadUtils.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index 0371f1d86a..4bc0c4afb8 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -199,14 +199,16 @@ public static void createValidationFiles(URL url, File localDestination, URL has */ public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL){ long size = resourceUrlConnection.getContentLengthLong(); - if(size != -1) { + if(size == -1) { + logger.warn("could not find expected file size for resource {}.", resourceUrlConnection.getURL()); + } else { System.out.println("Content-Length: " + size); File sizeFile = new File(localDestination.getParentFile(), localDestination.getName()+".size"); try (PrintStream sizePrintStream = new PrintStream(sizeFile)) { sizePrintStream.print(size); sizePrintStream.close(); } catch (FileNotFoundException e) { - logger.warn("could not write validation size file due to exception", e); + logger.warn("could not write size validation file due to exception", e); } } From a26adde89e07cc2cd9e931a00e489922b6c7cb6d Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Tue, 22 Feb 2022 10:42:13 +0800 Subject: [PATCH 4/8] Unit testing --- .../nbio/core/util/FileDownloadUtilsTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) 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 374001ec5a..761d06d097 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,12 +4,15 @@ 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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.PrintStream; +import java.net.URL; import java.nio.file.Files; import org.junit.jupiter.api.Nested; @@ -190,4 +193,50 @@ void deleteFolderTree() throws IOException{ assertFalse(toDelete.exists()); } } + + @Nested + class CreateValidationFiles{ + + @Test + void testValidationFiles() throws IOException{ + URL sourceUrl = new URL("https://ftp.wwpdb.org/pub/pdb/data/structures/divided/mmCIF/45/145d.cif.gz"); + File destFile = new File(System.getProperty("java.io.tmpdir"), "145d.cif.gz"); + File sizeFile = new File(destFile.getParentFile(), destFile.getName()+".size"); + File hashFile = new File(destFile.getParentFile(), destFile.getName()+".hash"); + System.out.println(destFile.getAbsolutePath()); + destFile.delete(); + sizeFile.delete(); + hashFile.delete(); + assertFalse(destFile.exists(), "couldn't delete dest file"); + assertFalse(sizeFile.exists(), "couldn't delete size file"); + assertFalse(hashFile.exists(), "couldn't delete hash file"); + + FileDownloadUtils.downloadFile(sourceUrl, destFile); + assertTrue(destFile.exists(), "couldn't create dest file"); + + assertTrue(FileDownloadUtils.validateFile(destFile), "file not detected to be invalid although there are no validation files"); + + PrintStream temp1 = new PrintStream(sizeFile); + temp1.print(15); // some wrong size value + temp1.close(); + assertFalse(FileDownloadUtils.validateFile(destFile), "file not detected to be invalid although size value is wrong."); + System.out.println("Just ignore the previous warning. It is expected."); + + FileDownloadUtils.createValidationFiles(sourceUrl, destFile, null); + 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"); + + PrintStream temp2 = new PrintStream(hashFile); + temp2.print("ABCD"); // some wrong hash value + temp2.close(); + //This is not yet implemented. I am using this test for documentation purpose. + assertThrows(UnsupportedOperationException.class, + () -> FileDownloadUtils.validateFile(destFile), + "file not detected to be invalid although size value is wrong."); + + destFile.delete(); + sizeFile.delete(); + hashFile.delete(); + } + } } From 06cde1b621156af3a6a768c4cad005b65864a478 Mon Sep 17 00:00:00 2001 From: Amr Date: Fri, 23 Dec 2022 17:03:07 -0500 Subject: [PATCH 5/8] Minor corrections Typos, wording, and version correction --- .../java/org/biojava/nbio/core/util/FileDownloadUtils.java | 6 +++--- .../org/biojava/nbio/core/util/FileDownloadUtilsTest.java | 4 ++-- .../java/org/biojava/nbio/structure/ecod/EcodFactory.java | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index 4bc0c4afb8..e5894982cf 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -195,7 +195,7 @@ public static void createValidationFiles(URL url, File localDestination, URL has * @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. - * @since 6.0.6 + * @since 6.1.1 */ public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL){ long size = resourceUrlConnection.getContentLengthLong(); @@ -234,8 +234,8 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi *

* This function does not implement hash code verification yet. * @param localFile The file to validate - * @return false if any of the size or hash code metadata files exists but its contents does not match the expected value in the file, true' otherwise. - * @since 6.0.6 + * @return false if any of the size or hash code metadata files exists but its contents does not match the expected value in the file, true otherwise. + * @since 6.1.1 */ public static boolean validateFile(File localFile) { File sizeFile = new File(localFile.getParentFile(), localFile.getName()+".size"); 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 761d06d097..7bf0f679ab 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 @@ -214,7 +214,7 @@ void testValidationFiles() throws IOException{ FileDownloadUtils.downloadFile(sourceUrl, destFile); assertTrue(destFile.exists(), "couldn't create dest file"); - assertTrue(FileDownloadUtils.validateFile(destFile), "file not detected to be invalid although there are no validation files"); + assertTrue(FileDownloadUtils.validateFile(destFile), "file detected to be invalid although there are no validation files"); PrintStream temp1 = new PrintStream(sizeFile); temp1.print(15); // some wrong size value @@ -232,7 +232,7 @@ void testValidationFiles() throws IOException{ //This is not yet implemented. I am using this test for documentation purpose. assertThrows(UnsupportedOperationException.class, () -> FileDownloadUtils.validateFile(destFile), - "file not detected to be invalid although size value is wrong."); + "file not detected to be invalid although hash value is wrong."); destFile.delete(); sizeFile.delete(); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java index 0f8f7b6654..998d8ab4a5 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodFactory.java @@ -89,8 +89,8 @@ public static EcodDatabase getEcodDatabase(String version) { } } catch (IOException e) { // For parsing errors, just use the requested version - // What about corrupted downloading errors?? Amr - logger.warn("Cound not get Ecod version, or file is corrupted", e); + // TODO What about corrupted downloading errors?? Amr + logger.warn("Could not get Ecod version, or file is corrupted", e); return null; } } From 630bb64fcf03354cd5e89a391c3ac654610b0dc9 Mon Sep 17 00:00:00 2001 From: Amr Date: Sat, 24 Dec 2022 08:52:56 -0500 Subject: [PATCH 6/8] Addressing reviewer's comments --- .../nbio/core/util/FileDownloadUtils.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index e5894982cf..f07a5849fc 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -48,6 +48,8 @@ public class FileDownloadUtils { + private static final String SIZE_EXT = ".size"; + private static final String HASH_EXT = ".hash"; private static final Logger logger = LoggerFactory.getLogger(FileDownloadUtils.class); /** @@ -157,7 +159,7 @@ public static void downloadFile(URL url, File destination) throws IOException { } } - logger.debug("Copying temp file {} to final location {}", tempFile, destination); + logger.debug("Copying temp file [{}] to final location [{}]", tempFile, destination); copy(tempFile, destination); // delete the tmp file @@ -181,7 +183,7 @@ public static void createValidationFiles(URL url, File localDestination, URL has URLConnection resourceConnection = url.openConnection(); createValidationFiles(resourceConnection, localDestination, hashURL); } catch (IOException e) { - logger.warn("could not open connection to resource file due to exception", e); + logger.warn("could not open connection to resource file due to exception:\n\t", e.getMessage()); } } /** @@ -202,13 +204,13 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi if(size == -1) { logger.warn("could not find expected file size for resource {}.", resourceUrlConnection.getURL()); } else { - System.out.println("Content-Length: " + size); - File sizeFile = new File(localDestination.getParentFile(), localDestination.getName()+".size"); + logger.debug("Content-Length: " + size); + File sizeFile = new File(localDestination.getParentFile(), localDestination.getName() + SIZE_EXT); try (PrintStream sizePrintStream = new PrintStream(sizeFile)) { sizePrintStream.print(size); sizePrintStream.close(); } catch (FileNotFoundException e) { - logger.warn("could not write size validation file due to exception", e); + logger.warn("could not write size validation file due to exception:\n\t", e.getMessage()); } } @@ -216,10 +218,10 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi return; try { - File hashFile = new File(localDestination.getParentFile(), localDestination.getName()+".hash"); + File hashFile = new File(localDestination.getParentFile(), localDestination.getName() + HASH_EXT); downloadFile(hashURL, hashFile); } catch (IOException e) { - logger.warn("could not write validation hash file due to exception", e); + logger.warn("could not write validation hash file due to exception:\n\t{}", e.getMessage()); } } @@ -238,24 +240,25 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi * @since 6.1.1 */ public static boolean validateFile(File localFile) { - File sizeFile = new File(localFile.getParentFile(), localFile.getName()+".size"); + File sizeFile = new File(localFile.getParentFile(), localFile.getName() + SIZE_EXT); if(sizeFile.exists()) { Scanner scanner = null; try { scanner = new Scanner(sizeFile); long expectedSize = scanner.nextLong(); - if (expectedSize != localFile.length()) { - logger.warn("File size does not match expected"); + long actualLSize = localFile.length(); + if (expectedSize != actualLSize) { + logger.warn("File [{}] size ({}) does not match expected size ({}).", localFile, actualLSize, expectedSize); return false; } } catch (FileNotFoundException e) { - logger.warn("could not validate size of file ["+ localFile+ "] due to exception", e); + logger.warn("could not validate size of file [{}] because no size metadata file exists.", localFile); } finally { scanner.close(); } } - File hashFile = new File(localFile.getParentFile(), localFile.getName()+".hash"); + File hashFile = new File(localFile.getParentFile(), localFile.getName() + HASH_EXT); if(hashFile.exists()) { throw new UnsupportedOperationException("Not yet implemented"); } From 3c7b04fab2de76b00d5259490ca36a1d06689d13 Mon Sep 17 00:00:00 2001 From: Amr Date: Sat, 24 Dec 2022 10:03:56 -0500 Subject: [PATCH 7/8] Adding placeholder for several common hashing algorithms The expected hashing algorithms are MD5, SHA1, and SHA256 --- .../nbio/core/util/FileDownloadUtils.java | 53 ++++++++++++++----- .../nbio/core/util/FileDownloadUtilsTest.java | 4 +- .../nbio/structure/ecod/EcodInstallation.java | 2 +- .../nbio/structure/io/LocalPDBDirectory.java | 2 +- .../io/sifts/SiftsMappingProvider.java | 2 +- .../nbio/structure/scop/ScopInstallation.java | 2 +- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index f07a5849fc..2c65813eb4 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -25,6 +25,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; @@ -52,6 +53,10 @@ public class FileDownloadUtils { private static final String HASH_EXT = ".hash"; private static final Logger logger = LoggerFactory.getLogger(FileDownloadUtils.class); + public enum Hash{ + MD5, SHA1, SHA256, UNKNOWN + } + /** * Copy the content of file src to dst TODO since java 1.7 this is provided * in java.nio.file.Files @@ -177,11 +182,12 @@ public static void downloadFile(URL url, File destination) throws IOException { * @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. */ - public static void createValidationFiles(URL url, File localDestination, URL hashURL){ + public static void createValidationFiles(URL url, File localDestination, URL hashURL, Hash hash){ try { URLConnection resourceConnection = url.openConnection(); - createValidationFiles(resourceConnection, localDestination, hashURL); + createValidationFiles(resourceConnection, localDestination, hashURL, FileDownloadUtils.Hash.UNKNOWN); } catch (IOException e) { logger.warn("could not open connection to resource file due to exception:\n\t", e.getMessage()); } @@ -189,17 +195,19 @@ public static void createValidationFiles(URL url, File localDestination, URL has /** * Creates validation files beside a file to be downloaded.
* Whenever possible, for a file.ext file, it creates - * file.ext.size and file.hash for in the same - * folder where file.ext exists. + * file.ext.size and file.hash_XXXX in the same + * folder where file.ext exists (XXXX may be DM5, SHA1, or SHA256). * If the file connection size could not be deduced from the resourceUrlConnection * {@link URLConnection}, no size file is created. - * If hashURL is null, no hash file is created. + * If hashURL is null, no hash file is created.
+ * N.B. None of the hashing algorithms is implemented (yet), because we did not need any of them yet. * @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. * @since 6.1.1 */ - public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL){ + public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL, Hash hash){ long size = resourceUrlConnection.getContentLengthLong(); if(size == -1) { logger.warn("could not find expected file size for resource {}.", resourceUrlConnection.getURL()); @@ -217,8 +225,10 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi if(hashURL == null) return; + if(hash == Hash.UNKNOWN) + throw new IllegalArgumentException("Hash URL given but algorithm is unknown"); try { - File hashFile = new File(localDestination.getParentFile(), localDestination.getName() + HASH_EXT); + File hashFile = new File(localDestination.getParentFile(), String.format("%s%s_%s", localDestination.getName(), HASH_EXT, hash)); downloadFile(hashURL, hashFile); } catch (IOException e) { logger.warn("could not write validation hash file due to exception:\n\t{}", e.getMessage()); @@ -229,12 +239,12 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi * Validate a local file based on pre-existing metadata files for size and hash.
* If the passed in localFile parameter is a file named file.ext, the function searches in the same folder for: *

    - *
  • file.ext.size: if found, it compares the size stored in it to the length of localFile (in bytes).
    • - *
  • file.ext.hash: if found, it compares the size stored in it to the hash code of localFile.
    • + *
  • file.ext.size: If found, it compares the size stored in it to the length of localFile (in bytes).
    • + *
  • file.ext.hash_XXXX (where XXXX is DM5, SHA1, or SHA256): If found, it compares the size stored in it to the hash code of localFile.
    • *
* If any of these comparisons fail, the function returns false. otherwise it returns true. *

- * This function does not implement hash code verification yet. + * N.B. None of the 3 common verification hashing algorithms are implement yet. * @param localFile The file to validate * @return false if any of the size or hash code metadata files exists but its contents does not match the expected value in the file, true otherwise. * @since 6.1.1 @@ -258,9 +268,26 @@ public static boolean validateFile(File localFile) { } } - File hashFile = new File(localFile.getParentFile(), localFile.getName() + HASH_EXT); - if(hashFile.exists()) { - throw new UnsupportedOperationException("Not yet implemented"); + File[] hashFiles = localFile.getParentFile().listFiles(new FilenameFilter() { + String hashPattern = String.format("%s%s_(%s|%s|%s)", 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]; + 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 + throw new IllegalArgumentException("Hashing algorithm not known: " + algo); + } } return true; 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 7bf0f679ab..9bf0a68231 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 @@ -202,7 +202,7 @@ void testValidationFiles() throws IOException{ URL sourceUrl = new URL("https://ftp.wwpdb.org/pub/pdb/data/structures/divided/mmCIF/45/145d.cif.gz"); File destFile = new File(System.getProperty("java.io.tmpdir"), "145d.cif.gz"); File sizeFile = new File(destFile.getParentFile(), destFile.getName()+".size"); - File hashFile = new File(destFile.getParentFile(), destFile.getName()+".hash"); + File hashFile = new File(destFile.getParentFile(), destFile.getName()+".hash_MD5"); System.out.println(destFile.getAbsolutePath()); destFile.delete(); sizeFile.delete(); @@ -222,7 +222,7 @@ void testValidationFiles() throws IOException{ assertFalse(FileDownloadUtils.validateFile(destFile), "file not detected to be invalid although size value is wrong."); System.out.println("Just ignore the previous warning. It is expected."); - FileDownloadUtils.createValidationFiles(sourceUrl, destFile, null); + FileDownloadUtils.createValidationFiles(sourceUrl, destFile, null, FileDownloadUtils.Hash.UNKNOWN); 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"); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java index 74f49f8dd5..7a39423077 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java @@ -406,7 +406,7 @@ private void downloadDomains() throws IOException { File localFile = getDomainFile(); logger.info("Downloading {} to: {}",domainsURL, localFile); - FileDownloadUtils.createValidationFiles(domainsURL, localFile, null); + FileDownloadUtils.createValidationFiles(domainsURL, localFile, null, FileDownloadUtils.Hash.UNKNOWN); FileDownloadUtils.downloadFile(domainsURL, localFile); if(! FileDownloadUtils.validateFile(localFile)) throw new IOException("Downloaded file invalid: "+ localFile); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java index dce7e9853d..87163b5ea7 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/LocalPDBDirectory.java @@ -581,7 +581,7 @@ private File downloadStructure(PdbId pdbId, String pathOnServer, boolean obsolet logger.info("Fetching " + ftp); logger.info("Writing to "+ realFile); - FileDownloadUtils.createValidationFiles(url, realFile, null); + FileDownloadUtils.createValidationFiles(url, realFile, null, FileDownloadUtils.Hash.UNKNOWN); FileDownloadUtils.downloadFile(url, realFile); if(! FileDownloadUtils.validateFile(realFile)) throw new IOException("Downloaded file invalid: "+realFile); diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java index 3701c87e29..f386140cdf 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/sifts/SiftsMappingProvider.java @@ -88,7 +88,7 @@ public static List getSiftsMapping(String pdbId) throws IOException String u = String.format(fileLoc,pdbId); URL url = new URL(u); logger.debug("Downloading SIFTS file {} validation metadata.",url); - FileDownloadUtils.createValidationFiles(url, dest, null); + FileDownloadUtils.createValidationFiles(url, dest, null, FileDownloadUtils.Hash.UNKNOWN); logger.debug("Downloading SIFTS file {} to {}",url,dest); FileDownloadUtils.downloadFile(url, dest); } diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java index e46dbca9f1..3a54bce234 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/scop/ScopInstallation.java @@ -747,7 +747,7 @@ protected void downloadComFile() throws FileNotFoundException, IOException{ */ protected void downloadFileFromRemote(URL remoteURL, File localFile) throws IOException{ logger.info("Downloading " + remoteURL + " to: " + localFile); - FileDownloadUtils.createValidationFiles(remoteURL, localFile, null); + FileDownloadUtils.createValidationFiles(remoteURL, localFile, null, FileDownloadUtils.Hash.UNKNOWN); FileDownloadUtils.downloadFile(remoteURL, localFile); if(! FileDownloadUtils.validateFile(localFile)) throw new IOException("Downloaded file invalid: "+localFile); From 07e65b22f1579a912532a3e757e275c05da71b26 Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Wed, 25 Jan 2023 10:24:13 -0500 Subject: [PATCH 8/8] Addressing reviewer's comments --- .../org/biojava/nbio/core/util/FileDownloadUtils.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java index 2c65813eb4..7b9c1f0cc6 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/util/FileDownloadUtils.java @@ -189,7 +189,7 @@ public static void createValidationFiles(URL url, File localDestination, URL has URLConnection resourceConnection = url.openConnection(); createValidationFiles(resourceConnection, localDestination, hashURL, FileDownloadUtils.Hash.UNKNOWN); } catch (IOException e) { - logger.warn("could not open connection to resource file due to exception:\n\t", e.getMessage()); + logger.warn("could not open connection to resource file due to exception: {}", e.getMessage()); } } /** @@ -205,7 +205,7 @@ public static void createValidationFiles(URL url, File localDestination, URL has * @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. - * @since 6.1.1 + * @since 7.0.0 */ public static void createValidationFiles(URLConnection resourceUrlConnection, File localDestination, URL hashURL, Hash hash){ long size = resourceUrlConnection.getContentLengthLong(); @@ -218,7 +218,7 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi sizePrintStream.print(size); sizePrintStream.close(); } catch (FileNotFoundException e) { - logger.warn("could not write size validation file due to exception:\n\t", e.getMessage()); + logger.warn("could not write size validation file due to exception: {}", e.getMessage()); } } @@ -231,7 +231,7 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi File hashFile = new File(localDestination.getParentFile(), String.format("%s%s_%s", localDestination.getName(), HASH_EXT, hash)); downloadFile(hashURL, hashFile); } catch (IOException e) { - logger.warn("could not write validation hash file due to exception:\n\t{}", e.getMessage()); + logger.warn("could not write validation hash file due to exception: {}", e.getMessage()); } } @@ -247,7 +247,7 @@ public static void createValidationFiles(URLConnection resourceUrlConnection, Fi * N.B. None of the 3 common verification hashing algorithms are implement yet. * @param localFile The file to validate * @return false if any of the size or hash code metadata files exists but its contents does not match the expected value in the file, true otherwise. - * @since 6.1.1 + * @since 7.0.0 */ public static boolean validateFile(File localFile) { File sizeFile = new File(localFile.getParentFile(), localFile.getName() + SIZE_EXT);