Return optional of File instead of boolean

2 files changed, 27 insertions, 21 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
index a26d87743f0..e45c3a8e380 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
@@ -19,6 +19,7 @@ import com.yahoo.vespa.filedistribution.FileReferenceData;
 import com.yahoo.vespa.filedistribution.FileReferenceDownload;
 import com.yahoo.vespa.filedistribution.LazyFileReferenceData;
 import com.yahoo.vespa.filedistribution.LazyTemporaryStorageFileReferenceData;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -118,12 +119,9 @@ public class FileServer {
 
     FileDirectory getRootDir() { return fileDirectory; }
 
-    void startFileServing(FileReference reference, Receiver target, Set<CompressionType> acceptedCompressionTypes) {
-        Optional<File> file = fileDirectory. getFile(reference);
-        if (file.isEmpty()) return;
-
-        var absolutePath = file.get().getAbsolutePath();
-        try (FileReferenceData fileData = fileReferenceData(reference, acceptedCompressionTypes, file.get())) {
+    void startFileServing(FileReference reference, File file, Receiver target, Set<CompressionType> acceptedCompressionTypes) {
+        var absolutePath = file.getAbsolutePath();
+        try (FileReferenceData fileData = fileReferenceData(reference, acceptedCompressionTypes, file)) {
             log.log(Level.FINE, () -> "Start serving " + reference.value() + " with file '" + absolutePath + "'");
             target.receive(fileData, new ReplayStatus(0, "OK"));
             log.log(Level.FINE, () -> "Done serving " + reference.value() + " with file '" + absolutePath + "'");
@@ -178,10 +176,10 @@ public class FileServer {
 
         try {
             var fileReferenceDownload = new FileReferenceDownload(fileReference, client, downloadFromOtherSourceIfNotFound);
-            boolean fileExists = hasFileDownloadIfNeeded(fileReferenceDownload);
-            if ( ! fileExists) return NOT_FOUND;
+            var file = getFileDownloadIfNeeded(fileReferenceDownload);
+            if (file.isEmpty()) return NOT_FOUND;
 
-            startFileServing(fileReference, receiver, acceptedCompressionTypes);
+            startFileServing(fileReference, file.get(), receiver, acceptedCompressionTypes);
         } catch (Exception e) {
             log.warning("Failed serving file reference '" + fileReference + "', request from " + client + " failed with: " + e.getMessage());
             return TRANSFER_FAILED;
@@ -200,9 +198,11 @@ public class FileServer {
                                            acceptedCompressionTypes + ", compression types server can use: " + compressionTypes);
     }
 
-    boolean hasFileDownloadIfNeeded(FileReferenceDownload fileReferenceDownload) {
+    public Optional<File> getFileDownloadIfNeeded(FileReferenceDownload fileReferenceDownload) {
         FileReference fileReference = fileReferenceDownload.fileReference();
-        if (hasFile(fileReference)) return true;
+        Optional<File> file = fileDirectory.getFile(fileReference);
+        if (file.isPresent())
+            return file;
 
         if (fileReferenceDownload.downloadFromOtherSourceIfNotFound()) {
             log.log(Level.FINE, "File not found, downloading from another source");
@@ -211,13 +211,13 @@ public class FileServer {
             FileReferenceDownload newDownload = new FileReferenceDownload(fileReference,
                                                                           fileReferenceDownload.client(),
                                                                           false);
-            boolean fileExists = downloader.getFile(newDownload).isPresent();
-            if ( ! fileExists)
+            file = downloader.getFile(newDownload);
+            if (file.isEmpty())
                 log.log(Level.INFO, "Failed downloading '" + fileReferenceDownload + "'");
-            return fileExists;
+            return file;
         } else {
             log.log(Level.FINE, "File not found, will not download from another source");
-            return false;
+            return Optional.empty();
         }
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
index c17b68c6d12..373b39c8365 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
@@ -61,9 +61,9 @@ public class FileServerTest {
         String dir = "123";
         assertFalse(fileServer.hasFile(dir));
         FileReferenceDownload foo = new FileReferenceDownload(new FileReference(dir), "test");
-        assertFalse(fileServer.hasFileDownloadIfNeeded(foo));
+        assertFalse(fileServer.getFileDownloadIfNeeded(foo).isPresent());
         writeFile(dir);
-        assertTrue(fileServer.hasFileDownloadIfNeeded(foo));
+        assertTrue(fileServer.getFileDownloadIfNeeded(foo).isPresent());
     }
 
     @Test
@@ -79,7 +79,9 @@ public class FileServerTest {
         File dir = getFileServerRootDir();
         IOUtils.writeFile(dir + "/12y/f1", "dummy-data", true);
         CompletableFuture<byte []> content = new CompletableFuture<>();
-        fileServer.startFileServing(new FileReference("12y"), new FileReceiver(content), Set.of(gzip));
+        FileReference fileReference = new FileReference("12y");
+        var file = fileServer.getFileDownloadIfNeeded(new FileReferenceDownload(fileReference, "test"));
+        fileServer.startFileServing(fileReference, file.get(), new FileReceiver(content), Set.of(gzip));
         assertEquals(new String(content.get()), "dummy-data");
     }
 
@@ -90,7 +92,9 @@ public class FileServerTest {
         File dir = getFileServerRootDir();
         IOUtils.writeFile(dir + "/subdir/12z/f1", "dummy-data-2", true);
         CompletableFuture<byte []> content = new CompletableFuture<>();
-        fileServer.startFileServing(new FileReference("subdir"), new FileReceiver(content), Set.of(gzip, lz4));
+        FileReference fileReference = new FileReference("subdir");
+        var file = fileServer.getFileDownloadIfNeeded(new FileReferenceDownload(fileReference, "test"));
+        fileServer.startFileServing(fileReference, file.get(), new FileReceiver(content), Set.of(gzip, lz4));
 
         // Decompress with lz4 and check contents
         var compressor = new FileReferenceCompressor(FileReferenceData.Type.compressed, lz4);
@@ -139,14 +143,16 @@ public class FileServerTest {
         FailingFileReceiver fileReceiver = new FailingFileReceiver(content);
 
         // Should fail the first time, see FailingFileReceiver
+        FileReference reference = new FileReference("12y");
+        var file = fileServer.getFileDownloadIfNeeded(new FileReferenceDownload(reference, "test"));
         try {
-            fileServer.startFileServing(new FileReference("12y"), fileReceiver, Set.of(gzip));
+            fileServer.startFileServing(reference, file.get(), fileReceiver, Set.of(gzip));
             fail("Should have failed");
         } catch (RuntimeException e) {
             // expected
         }
 
-        fileServer.startFileServing(new FileReference("12y"), fileReceiver, Set.of(gzip));
+        fileServer.startFileServing(reference, file.get(), fileReceiver, Set.of(gzip));
         assertEquals(new String(content.get()), "dummy-data");
     }