From d121d3e8e2ad98f52dc4c20f71b84a4d940c95d8 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Sat, 21 Apr 2018 11:37:10 +0200 Subject: Fix directory checked for already downloaded files --- .../FileDistributionRpcServer.java | 1 + .../vespa/filedistribution/FileDownloader.java | 25 +++++++++++----------- .../vespa/filedistribution/FileDownloaderTest.java | 4 +++- 3 files changed, 16 insertions(+), 14 deletions(-) (limited to 'filedistribution') diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java index d82a806b79e..bf9201e6cda 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java @@ -103,6 +103,7 @@ public class FileDistributionRpcServer { @SuppressWarnings({"UnusedDeclaration"}) public final void setFileReferencesToDownload(Request req) { + log.log(LogLevel.DEBUG, () -> "Received method call '" + req.methodName() + "' with parameters : " + req.parameters()); Arrays.stream(req.parameters().get(0).asStringArray()) .map(FileReference::new) .forEach(fileReference -> downloader.downloadIfNeeded(new FileReferenceDownload(fileReference))); diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java index ad08a4467f5..462dc1d4700 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java @@ -63,17 +63,16 @@ public class FileDownloader { private Future> getFutureFile(FileReferenceDownload fileReferenceDownload) { FileReference fileReference = fileReferenceDownload.fileReference(); Objects.requireNonNull(fileReference, "file reference cannot be null"); - File directory = new File(downloadDirectory, fileReference.value()); - log.log(LogLevel.DEBUG, () -> "Checking if there is a file in '" + directory.getAbsolutePath() + "' "); - - Optional file = getFileFromFileSystem(fileReference, directory); + log.log(LogLevel.DEBUG, () -> "Checking if file reference '" + fileReference.value() + "' exists in '" + + downloadDirectory.getAbsolutePath() + "' "); + Optional file = getFileFromFileSystem(fileReference, downloadDirectory); if (file.isPresent()) { SettableFuture> future = SettableFuture.create(); future.set(file); return future; } else { log.log(LogLevel.DEBUG, () -> "File reference '" + fileReference.value() + "' not found in " + - directory.getAbsolutePath() + ", starting download"); + downloadDirectory.getAbsolutePath() + ", starting download"); return download(fileReferenceDownload); } } @@ -91,14 +90,15 @@ public class FileDownloader { } private Optional getFileFromFileSystem(FileReference fileReference, File directory) { - File[] files = directory.listFiles(); + File[] files = new File(directory, fileReference.value()).listFiles(); if (directory.exists() && directory.isDirectory() && files != null && files.length > 0) { File file = files[0]; if (!file.exists()) { - throw new RuntimeException("File with reference '" + fileReference.value() + "' does not exist"); + throw new RuntimeException("File reference '" + fileReference.value() + "' does not exist"); } else if (!file.canRead()) { - throw new RuntimeException("File with reference '" + fileReference.value() + "'exists, but unable to read it"); + throw new RuntimeException("File reference '" + fileReference.value() + "'exists, but unable to read it"); } else { + log.log(LogLevel.DEBUG, () -> "File reference '" + fileReference.value() + "' found: " + file.getAbsolutePath()); fileReferenceDownloader.setDownloadStatus(fileReference, 1.0); return Optional.of(file); } @@ -108,12 +108,10 @@ public class FileDownloader { private boolean alreadyDownloaded(FileReference fileReference) { try { - if (getFileFromFileSystem(fileReference, downloadDirectory).isPresent()) - return true; + return (getFileFromFileSystem(fileReference, downloadDirectory).isPresent()); } catch (RuntimeException e) { - /* ignored */ + return false; } - return false; } public boolean downloadIfNeeded(FileReferenceDownload fileReferenceDownload) { @@ -121,11 +119,12 @@ public class FileDownloader { download(fileReferenceDownload); return true; } else { + log.log(LogLevel.DEBUG, () -> "Download not needed, " + fileReferenceDownload.fileReference() + " already downloaded" ); return false; } } - synchronized Future> download(FileReferenceDownload fileReferenceDownload) { + private synchronized Future> download(FileReferenceDownload fileReferenceDownload) { FileReference fileReference = fileReferenceDownload.fileReference(); Future> inProgress = fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReferenceDownload)); if (inProgress != null) { diff --git a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java index 2a40d27831c..cab3a490a13 100644 --- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java +++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java @@ -208,7 +208,9 @@ public class FileDownloaderTest { } private void writeFileReference(File dir, String fileReferenceString, String fileName) throws IOException { - File file = new File(new File(dir, fileReferenceString), fileName); + File fileReferenceDir = new File(dir, fileReferenceString); + fileReferenceDir.mkdir(); + File file = new File(fileReferenceDir, fileName); IOUtils.writeFile(file, "content", false); } -- cgit v1.2.3