Return optional from getFile

4 files changed, 47 insertions, 45 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
index da18c4e4fcc..6fe133958f5 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
@@ -24,12 +24,14 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Clock;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
+import static java.util.logging.Level.INFO;
 
 /**
  * Global file directory, holding files for file distribution for all deployed applications.
@@ -40,7 +42,6 @@ public class FileDirectory extends AbstractComponent {
     private static final Logger log = Logger.getLogger(FileDirectory.class.getName());
 
     private final Locks<FileReference> locks = new Locks<>(1, TimeUnit.MINUTES);
-
     private final File root;
 
     @Inject
@@ -67,7 +68,7 @@ public class FileDirectory extends AbstractComponent {
         }
     }
 
-    static private class Filter implements FilenameFilter {
+    private static class Filter implements FilenameFilter {
         @Override
         public boolean accept(File dir, String name) {
             return !".".equals(name) && !"..".equals(name) ;
@@ -78,17 +79,23 @@ public class FileDirectory extends AbstractComponent {
         return root.getAbsolutePath() + "/" + ref.value();
     }
 
-    public File getFile(FileReference reference) {
+    public Optional<File> getFile(FileReference reference) {
         ensureRootExist();
         File dir = new File(getPath(reference));
-        if (!dir.exists())
-            throw new IllegalArgumentException("File reference '" + reference.value() + "' with absolute path '" + dir.getAbsolutePath() + "' does not exist.");
-        if (!dir.isDirectory())
-            throw new IllegalArgumentException("File reference '" + reference.value() + "' with absolute path '" + dir.getAbsolutePath() + "' is not a directory.");
-        File [] files = dir.listFiles(new Filter());
-        if (files == null || files.length == 0)
-            throw new IllegalArgumentException("File reference '" + reference.value() + "' with absolute path '" + dir.getAbsolutePath() + " does not contain any files");
-        return files[0];
+        if (!dir.exists()) {
+            log.log(INFO, "File reference '" + reference.value() + "' ('" + dir.getAbsolutePath() + "') does not exist.");
+            return Optional.empty();
+        }
+        if (!dir.isDirectory()) {
+            log.log(INFO, "File reference '" + reference.value() + "' ('" + dir.getAbsolutePath() + ")' is not a directory.");
+            return Optional.empty();
+        }
+        File[] files = dir.listFiles(new Filter());
+        if (files == null || files.length == 0) {
+            log.log(INFO, "File reference '" + reference.value() + "' ('" + dir.getAbsolutePath() + "') does not contain any files");
+            return Optional.empty();
+        }
+        return Optional.of(files[0]);
     }
 
     public File getRoot() { return root; }
@@ -136,7 +143,7 @@ public class FileDirectory extends AbstractComponent {
     private void deleteDirRecursively(File dir) {
         log.log(Level.FINE, "Will delete dir " + dir);
         if ( ! IOUtils.recursiveDeleteDir(dir))
-            log.log(Level.INFO, "Failed to delete " + dir);
+            log.log(INFO, "Failed to delete " + dir);
     }
 
     // Check if we should add file, it might already exist
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 dcd2720ae3e..a26d87743f0 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
@@ -27,6 +27,7 @@ import java.nio.file.Path;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -42,7 +43,6 @@ import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType
 import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.gzip;
 import static com.yahoo.vespa.filedistribution.FileReferenceData.Type;
 import static com.yahoo.vespa.filedistribution.FileReferenceData.Type.compressed;
-import static com.yahoo.yolean.Exceptions.uncheck;
 
 public class FileServer {
 
@@ -108,26 +108,27 @@ public class FileServer {
     }
 
     private boolean hasFile(FileReference reference) {
-        try {
-            return fileDirectory.getFile(reference).exists();
-        } catch (IllegalArgumentException e) {
-            log.log(Level.FINE, () -> "Failed locating " + reference + ": " + e.getMessage());
-        }
+        Optional<File> file = fileDirectory.getFile(reference);
+        if (file.isPresent())
+            return file.get().exists();
+
+        log.log(Level.FINE, () -> "Failed locating " + reference);
         return false;
     }
 
     FileDirectory getRootDir() { return fileDirectory; }
 
     void startFileServing(FileReference reference, Receiver target, Set<CompressionType> acceptedCompressionTypes) {
-        File file = fileDirectory.getFile(reference);
-        if ( ! file.exists()) return;
+        Optional<File> file = fileDirectory. getFile(reference);
+        if (file.isEmpty()) return;
 
-        try (FileReferenceData fileData = fileReferenceData(reference, acceptedCompressionTypes, file)) {
-            log.log(Level.FINE, () -> "Start serving " + reference.value() + " with file '" + file.getAbsolutePath() + "'");
+        var absolutePath = file.get().getAbsolutePath();
+        try (FileReferenceData fileData = fileReferenceData(reference, acceptedCompressionTypes, file.get())) {
+            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 '" + file.getAbsolutePath() + "'");
+            log.log(Level.FINE, () -> "Done serving " + reference.value() + " with file '" + absolutePath + "'");
         } catch (IOException ioe) {
-            throw new UncheckedIOException("For " + reference.value() + ": failed reading file '" + file.getAbsolutePath() + "'" +
+            throw new UncheckedIOException("For " + reference.value() + ": failed reading file '" + absolutePath + "'" +
                                            " for sending to '" + target.toString() + "'. ", ioe);
         } catch (Exception e) {
             throw new RuntimeException("Failed serving " + reference.value() + " to '" + target + "': ", e);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
index 6ace2f5b0b8..c057bbedd5f 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -26,7 +26,6 @@ import com.yahoo.vespa.config.server.application.ApplicationSet;
 import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
 import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs;
-import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.config.server.filedistribution.FileDistributionFactory;
 import com.yahoo.vespa.config.server.http.InvalidApplicationException;
 import com.yahoo.vespa.config.server.http.UnknownVespaVersionException;
@@ -857,19 +856,14 @@ public class SessionRepository {
         log.log(Level.FINE, () -> "File reference for session id " + sessionId + ": " + fileReference);
         if (fileReference.isEmpty()) return;
 
-        File sessionDir;
-        FileDirectory fileDirectory = fileDistributionFactory.fileDirectory();
-        try {
-            sessionDir = fileDirectory.getFile(fileReference.get());
-        } catch (IllegalArgumentException e) {
-            // We cannot be guaranteed that the file reference exists (it could be that it has not
-            // been downloaded yet), and e.g. when bootstrapping we cannot throw an exception in that case
-            log.log(Level.FINE, () -> "File reference for session id " + sessionId + ": " + fileReference + " not found");
-            return;
-        }
+        Optional<File> sessionDir = fileDistributionFactory.fileDirectory().getFile(fileReference.get());
+        // We cannot be guaranteed that the file reference exists (it could be that it has not
+        // been downloaded yet), and e.g. when bootstrapping we cannot throw an exception in that case
+        if (sessionDir.isEmpty()) return;
+
         ApplicationId applicationId = sessionZKClient.readApplicationId();
         log.log(Level.FINE, () -> "Creating local session for tenant '" + tenantName + "' with session id " + sessionId);
-        createLocalSession(sessionDir, applicationId, sessionId);
+        createLocalSession(sessionDir.get(), applicationId, sessionId);
     }
 
     private Optional<Long> getActiveSessionId(ApplicationId applicationId) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java
index 649d382ddb6..040df208323 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDirectoryTest.java
@@ -37,9 +37,9 @@ public class FileDirectoryTest {
         FileReference foo = createFile("foo");
         FileReference bar = createFile("bar");
 
-        assertTrue(fileDirectory.getFile(foo).exists());
+        assertTrue(fileDirectory.getFile(foo).get().exists());
         assertEquals("ea315b7acac56246", foo.value());
-        assertTrue(fileDirectory.getFile(bar).exists());
+        assertTrue(fileDirectory.getFile(bar).get().exists());
         assertEquals("2b8e97f15c854e1d", bar.value());
     }
 
@@ -49,7 +49,7 @@ public class FileDirectoryTest {
         File subDirectory = new File(temporaryFolder.getRoot(), subdirName);
         createFileInSubDir(subDirectory, "foo", "some content");
         FileReference fileReference = fileDirectory.addFile(subDirectory);
-        File dir = fileDirectory.getFile(fileReference);
+        File dir = fileDirectory.getFile(fileReference).get();
         assertTrue(dir.exists());
         assertTrue(new File(dir, "foo").exists());
         assertFalse(new File(dir, "doesnotexist").exists());
@@ -58,7 +58,7 @@ public class FileDirectoryTest {
         // Change contents of a file, file reference value should change
         createFileInSubDir(subDirectory, "foo", "new content");
         FileReference fileReference2 = fileDirectory.addFile(subDirectory);
-        dir = fileDirectory.getFile(fileReference2);
+        dir = fileDirectory.getFile(fileReference2).get();
         assertTrue(new File(dir, "foo").exists());
         assertNotEquals(fileReference + " should not be equal to " + fileReference2, fileReference, fileReference2);
         assertEquals("e5d4b3fe5ee3ede3", fileReference2.value());
@@ -66,7 +66,7 @@ public class FileDirectoryTest {
         // Add a file, should be available and file reference should have another value
         createFileInSubDir(subDirectory, "bar", "some other content");
         FileReference fileReference3 = fileDirectory.addFile(subDirectory);
-        dir = fileDirectory.getFile(fileReference3);
+        dir = fileDirectory.getFile(fileReference3).get();
         assertTrue(new File(dir, "foo").exists());
         assertTrue(new File(dir, "bar").exists());
         assertEquals("894bced3fc9d199b", fileReference3.value());
@@ -78,7 +78,7 @@ public class FileDirectoryTest {
         File subDirectory = new File(temporaryFolder.getRoot(), subdirName);
         createFileInSubDir(subDirectory, "foo", "some content");
         FileReference fileReference = fileDirectory.addFile(subDirectory);
-        File dir = fileDirectory.getFile(fileReference);
+        File dir = fileDirectory.getFile(fileReference).get();
         assertTrue(dir.exists());
         File foo = new File(dir, "foo");
         assertTrue(foo.exists());
@@ -90,7 +90,7 @@ public class FileDirectoryTest {
         try { Thread.sleep(1000);} catch (InterruptedException e) {/*ignore */} // Needed since we have timestamp resolution of 1 second
         Files.delete(Paths.get(fileDirectory.getPath(fileReference)).resolve("subdir").resolve("foo"));
         fileReference = fileDirectory.addFile(subDirectory);
-        dir = fileDirectory.getFile(fileReference);
+        dir = fileDirectory.getFile(fileReference).get();
         File foo2 = new File(dir, "foo");
         assertTrue(dir.exists());
         assertTrue(foo2.exists());
@@ -107,7 +107,7 @@ public class FileDirectoryTest {
         File subDirectory = new File(temporaryFolder.getRoot(), subdirName);
         createFileInSubDir(subDirectory, "foo", "some content");
         FileReference fileReference = fileDirectory.addFile(subDirectory);
-        File dir = fileDirectory.getFile(fileReference);
+        File dir = fileDirectory.getFile(fileReference).get();
         assertTrue(dir.exists());
         File foo = new File(dir, "foo");
         assertTrue(foo.exists());
@@ -119,7 +119,7 @@ public class FileDirectoryTest {
         // Add a file that already exists, nothing should happen
         createFileInSubDir(subDirectory, "foo", "some content"); // same as before, nothing should happen
         FileReference fileReference3 = fileDirectory.addFile(subDirectory);
-        dir = fileDirectory.getFile(fileReference3);
+        dir = fileDirectory.getFile(fileReference3).get();
         assertTrue(new File(dir, "foo").exists());
         assertEquals("bebc5a1aee74223d", fileReference3.value()); // same hash