Revert "Refactor cleanup of file references"

3 files changed, 63 insertions, 58 deletions

diff --git a/config-proxy/pom.xml b/config-proxy/pom.xml
index bd1f907f87e..476f5f99b86 100644
--- a/config-proxy/pom.xml
+++ b/config-proxy/pom.xml
@@ -74,12 +74,6 @@
       <version>${project.version}</version>
       <scope>compile</scope>
     </dependency>
-    <dependency>
-      <groupId>com.yahoo.vespa</groupId>
-      <artifactId>testutil</artifactId>
-      <version>${project.version}</version>
-      <scope>test</scope>
-    </dependency>
   </dependencies>
   <build>
     <plugins>
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainer.java
index a3dd8d27c0f..3eafdf8f2b4 100644
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainer.java
+++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainer.java
@@ -1,17 +1,27 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.proxy.filedistribution;
 
+import com.yahoo.io.IOUtils;
+import java.util.logging.Level;
 import com.yahoo.vespa.filedistribution.FileDownloader;
-import com.yahoo.vespa.filedistribution.maintenance.FileDistributionCleanup;
+
 import java.io.File;
-import java.time.Clock;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Duration;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+import static java.nio.file.Files.readAttributes;
 
 /**
- * Deletes file references and url downloads on disk that have not been used for some time
+ * Deletes cached file references and url downloads that have not been used for some time
  *
  * @author hmusum
  */
@@ -21,29 +31,20 @@ class CachedFilesMaintainer implements Runnable {
 
     private static final File defaultUrlDownloadDir = UrlDownloadRpcServer.downloadDir;
     private static final File defaultFileReferencesDownloadDir = FileDownloader.defaultDownloadDirectory;
-    private static final Duration defaultDurationToKeepFiles = Duration.ofDays(20);
-    private static final int defaultKeepCount = 20;
+    private static final Duration defaultDurationToKeepFiles = Duration.ofDays(30);
 
     private final File urlDownloadDir;
     private final File fileReferencesDownloadDir;
     private final Duration durationToKeepFiles;
-    private final FileDistributionCleanup cleanup;
-    private final int keepCount;  // keep this many files no matter how old they are or when they were last accessed
 
     CachedFilesMaintainer() {
-        this(defaultFileReferencesDownloadDir, defaultUrlDownloadDir, defaultDurationToKeepFiles, Clock.systemUTC(), defaultKeepCount);
+        this(defaultFileReferencesDownloadDir, defaultUrlDownloadDir, defaultDurationToKeepFiles);
     }
 
-    CachedFilesMaintainer(File fileReferencesDownloadDir,
-                          File urlDownloadDir,
-                          Duration durationToKeepFiles,
-                          Clock clock,
-                          int keepCount) {
+    CachedFilesMaintainer(File fileReferencesDownloadDir, File urlDownloadDir, Duration durationToKeepFiles) {
         this.fileReferencesDownloadDir = fileReferencesDownloadDir;
         this.urlDownloadDir = urlDownloadDir;
         this.durationToKeepFiles = durationToKeepFiles;
-        this.cleanup = new FileDistributionCleanup(clock);
-        this.keepCount = keepCount;
     }
 
     @Override
@@ -57,7 +58,35 @@ class CachedFilesMaintainer implements Runnable {
     }
 
     private void deleteUnusedFiles(File directory) {
-        cleanup.deleteUnusedFileReferences(directory, durationToKeepFiles, keepCount, Set.of());
+        Instant deleteNotUsedSinceInstant = Instant.now().minus(durationToKeepFiles);
+        Set<String> filesOnDisk = new HashSet<>();
+        File[] files = directory.listFiles();
+        if (files != null)
+            filesOnDisk.addAll(Arrays.stream(files).map(File::getName).collect(Collectors.toSet()));
+        log.log(Level.FINE, () -> "Files on disk (in " + directory + "): " + filesOnDisk);
+
+        Set<String> filesToDelete = filesOnDisk
+                .stream()
+                .filter(fileReference -> isFileLastModifiedBefore(new File(directory, fileReference), deleteNotUsedSinceInstant))
+                .collect(Collectors.toSet());
+        if (filesToDelete.size() > 0) {
+            log.log(Level.INFO, "Files that can be deleted in " + directory + " (not used since " + deleteNotUsedSinceInstant + "): " + filesToDelete);
+            filesToDelete.forEach(fileReference -> {
+                File file = new File(directory, fileReference);
+                if (!IOUtils.recursiveDeleteDir(file))
+                    log.log(Level.WARNING, "Could not delete " + file.getAbsolutePath());
+            });
+        }
+    }
+
+    private boolean isFileLastModifiedBefore(File fileReference, Instant instant) {
+        BasicFileAttributes fileAttributes;
+        try {
+            fileAttributes = readAttributes(fileReference.toPath(), BasicFileAttributes.class);
+            return fileAttributes.lastModifiedTime().toInstant().isBefore(instant);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 
 }
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainerTest.java
index 835982be44c..a491a7b4fc4 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainerTest.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/CachedFilesMaintainerTest.java
@@ -2,17 +2,14 @@
 package com.yahoo.vespa.config.proxy.filedistribution;
 
 import com.yahoo.io.IOUtils;
-import com.yahoo.test.ManualClock;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+
 import java.io.File;
 import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.nio.file.Files;
-import java.nio.file.attribute.FileTime;
 import java.time.Duration;
-import java.util.stream.IntStream;
+import java.time.Instant;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -22,12 +19,9 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
  */
 public class CachedFilesMaintainerTest {
 
-    private static final int numberToAlwaysKeep = 2;
-
     private File cachedFileReferences;
     private File cachedDownloads;
     private CachedFilesMaintainer cachedFilesMaintainer;
-    private final ManualClock clock = new ManualClock();
 
     @TempDir
     public File tempFolder;
@@ -36,28 +30,28 @@ public class CachedFilesMaintainerTest {
     public void setup() throws IOException {
         cachedFileReferences = newFolder(tempFolder, "cachedFileReferences");
         cachedDownloads = newFolder(tempFolder, "cachedDownloads");
-        cachedFilesMaintainer = new CachedFilesMaintainer(cachedFileReferences,
-                                                          cachedDownloads,
-                                                          Duration.ofMinutes(2),
-                                                          clock,
-                                                          numberToAlwaysKeep);
+        cachedFilesMaintainer = new CachedFilesMaintainer(cachedFileReferences, cachedDownloads, Duration.ofMinutes(1));
     }
 
     @Test
-    void require_old_files_to_be_deleted() {
+    void require_old_files_to_be_deleted() throws IOException {
         runMaintainerAndAssertFiles(0, 0);
 
-        clock.advance(Duration.ofSeconds(55));
-        // Create file references and downloads
-        createFiles();
+        File fileReference = writeFile(cachedFileReferences, "fileReference");
+        File download = writeFile(cachedDownloads, "download");
+        runMaintainerAndAssertFiles(1, 1);
 
-        runMaintainerAndAssertFiles(4, 4);
+        updateLastModifiedTimeStamp(fileReference, Instant.now().minus(Duration.ofMinutes(10)));
+        runMaintainerAndAssertFiles(0, 1);
 
-        clock.advance(Duration.ofMinutes(1));
-        runMaintainerAndAssertFiles(3, 3);
+        updateLastModifiedTimeStamp(download, Instant.now().minus(Duration.ofMinutes(10)));
+        runMaintainerAndAssertFiles(0, 0);
+    }
 
-        clock.advance(Duration.ofMinutes(100));
-        runMaintainerAndAssertFiles(numberToAlwaysKeep, numberToAlwaysKeep);
+    private void updateLastModifiedTimeStamp(File file, Instant instant) {
+        if (!file.setLastModified(instant.toEpochMilli())) {
+            throw new RuntimeException("Could not set last modified timestamp for '" + file.getAbsolutePath() + "'");
+        }
     }
 
     private void runMaintainerAndAssertFiles(int fileReferenceCount, int downloadCount) {
@@ -71,10 +65,10 @@ public class CachedFilesMaintainerTest {
         assertEquals(downloadCount, downloads.length);
     }
 
-    private void writeFileAndSetLastAccessedTime(File directory, String filename) throws IOException {
+    private File writeFile(File directory, String filename) throws IOException {
         File file = new File(directory, filename);
         IOUtils.writeFile(file, filename, false);
-        Files.setAttribute(file.toPath(), "lastAccessTime", FileTime.from(clock.instant()));
+        return file;
     }
 
     private static File newFolder(File root, String... subDirs) throws IOException {
@@ -86,16 +80,4 @@ public class CachedFilesMaintainerTest {
         return result;
     }
 
-    private void createFiles() {
-        IntStream.of(0,1,2,3).forEach(i -> {
-            try {
-                writeFileAndSetLastAccessedTime(cachedFileReferences, "fileReference" + i);
-                writeFileAndSetLastAccessedTime(cachedDownloads, "download" + i);
-                clock.advance(Duration.ofMinutes(1));
-            } catch (IOException e) {
-                throw new UncheckedIOException(e);
-            }
-        });
-    }
-
 }