CLean up workdir cache aoi

scm-core/src/main/java/sonia/scm/repository/util/CacheSupportingWorkdirProvider.java

@@ -24,19 +24,10 @@
 
 package sonia.scm.repository.util;
 
-import sonia.scm.repository.Repository;
-
 import java.io.File;
-import java.io.IOException;
 
 public interface CacheSupportingWorkdirProvider {
-  <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(
-    Repository scmRepository,
-    String requestedBranch,
-    C context,
-    SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer,
-    SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer
-  ) throws IOException;
+  <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(CreateWorkdirContext<R, W, C> context) throws Exception;
 
-  boolean cache(Repository repository, File target) throws IOException;
+  void contextClosed(CreateWorkdirContext<?, ?, ?> createWorkdirContext, File workdir) throws Exception;
 }

scm-core/src/main/java/sonia/scm/repository/util/CachingAllWorkdirProvider.java

@@ -24,18 +24,17 @@
 
 package sonia.scm.repository.util;
 
-import sonia.scm.repository.Repository;
 import sonia.scm.util.IOUtil;
 
 import javax.inject.Inject;
 import java.io.File;
 import java.io.IOException;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class CachingAllWorkdirProvider implements CacheSupportingWorkdirProvider {
 
-  private final Map<String, File> workdirs = new HashMap<>();
+  private final Map<String, File> workdirs = new ConcurrentHashMap<>();
 
   private final WorkdirProvider workdirProvider;
 
@@ -45,18 +44,17 @@ public class CachingAllWorkdirProvider implements CacheSupportingWorkdirProvider
   }
 
   @Override
-  public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer, SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer) throws IOException {
-    String id = scmRepository.getId();
-    if (workdirs.containsKey(id)) {
-      File existingWorkdir = workdirs.get(id);
+  public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(CreateWorkdirContext<R, W, C> createWorkdirContext) throws IOException {
+    String id = createWorkdirContext.getScmRepository().getId();
+    File existingWorkdir = workdirs.remove(id);
+    if (existingWorkdir != null) {
       try {
-        return reclaimer.reclaim(existingWorkdir);
+        return createWorkdirContext.getReclaimer().reclaim(existingWorkdir);
       } catch (SimpleWorkdirFactory.ReclaimFailedException e) {
-        workdirs.remove(id);
-        IOUtil.delete(existingWorkdir, true);
+        deleteWorkdir(existingWorkdir);
       }
     }
-    return createNewWorkdir(initializer, id);
+    return createNewWorkdir(createWorkdirContext.getInitializer(), id);
   }
 
   public <R, W> SimpleWorkdirFactory.ParentAndClone<R, W> createNewWorkdir(SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer, String id) throws IOException {
@@ -66,7 +64,14 @@ public class CachingAllWorkdirProvider implements CacheSupportingWorkdirProvider
   }
 
   @Override
-  public boolean cache(Repository repository, File target) {
-    return true;
+  public void contextClosed(CreateWorkdirContext<?, ?, ?> createWorkdirContext, File workdir) throws IOException {
+    String id = createWorkdirContext.getScmRepository().getId();
+    if (workdirs.putIfAbsent(id, workdir) != null) {
+      deleteWorkdir(workdir);
+    }
+  }
+
+  private void deleteWorkdir(File existingWorkdir) throws IOException {
+    IOUtil.delete(existingWorkdir, true);
   }
 }

scm-core/src/main/java/sonia/scm/repository/util/CreateWorkdirContext.java

@@ -0,0 +1,63 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.repository.util;
+
+import sonia.scm.repository.Repository;
+
+public class CreateWorkdirContext<R, W, C> {
+  private final Repository scmRepository;
+  private final String requestedBranch;
+  private final C context;
+  private final SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer;
+  private final SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer;
+
+  public CreateWorkdirContext(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer, SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer) {
+    this.scmRepository = scmRepository;
+    this.requestedBranch = requestedBranch;
+    this.context = context;
+    this.initializer = initializer;
+    this.reclaimer = reclaimer;
+  }
+
+  public Repository getScmRepository() {
+    return scmRepository;
+  }
+
+  public String getRequestedBranch() {
+    return requestedBranch;
+  }
+
+  public C getContext() {
+    return context;
+  }
+
+  public SimpleWorkdirFactory.WorkdirInitializer<R, W> getInitializer() {
+    return initializer;
+  }
+
+  public SimpleWorkdirFactory.WorkdirReclaimer<R, W> getReclaimer() {
+    return reclaimer;
+  }
+}

scm-core/src/main/java/sonia/scm/repository/util/NoneCachingWorkdirProvider.java

@@ -24,7 +24,7 @@
 
 package sonia.scm.repository.util;
 
-import sonia.scm.repository.Repository;
+import sonia.scm.util.IOUtil;
 
 import javax.inject.Inject;
 import java.io.File;
@@ -40,12 +40,12 @@ public class NoneCachingWorkdirProvider implements CacheSupportingWorkdirProvide
   }
 
   @Override
-  public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer, SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer) throws IOException {
-    return initializer.initialize(workdirProvider.createNewWorkdir());
+  public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(CreateWorkdirContext<R, W, C> context) throws IOException {
+    return context.getInitializer().initialize(workdirProvider.createNewWorkdir());
   }
 
   @Override
-  public boolean cache(Repository repository, File target) {
-    return false;
+  public void contextClosed(CreateWorkdirContext<?, ?, ?> createWorkdirContext, File workdir) throws IOException {
+    IOUtil.delete(workdir, true);
   }
 }

scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java

@@ -34,7 +34,7 @@ import java.io.IOException;
 
 public abstract class SimpleWorkdirFactory<R, W, C> implements WorkdirFactory<R, W, C> {
 
-  private static final Logger logger = LoggerFactory.getLogger(SimpleWorkdirFactory.class);
+  private static final Logger LOG = LoggerFactory.getLogger(SimpleWorkdirFactory.class);
 
   private final CacheSupportingWorkdirProvider workdirProvider;
 
@@ -45,26 +45,38 @@ public abstract class SimpleWorkdirFactory<R, W, C> implements WorkdirFactory<R,
   @Override
   public WorkingCopy<R, W> createWorkingCopy(C context, String initialBranch) {
     try {
-      ParentAndClone<R, W> parentAndClone = workdirProvider.getWorkdir(
+      CreateWorkdirContext<R, W, C> createWorkdirContext = new CreateWorkdirContext<>(
         getScmRepository(context),
         initialBranch,
         context,
         newFolder -> cloneRepository(context, newFolder, initialBranch),
         cachedFolder -> reclaimRepository(context, cachedFolder, initialBranch)
       );
-      return new WorkingCopy<R, W>(parentAndClone.getClone(), parentAndClone.getParent(), this::closeWorkdir, this::closeCentral, parentAndClone.getDirectory()) {
-        @Override
-        public void delete() throws IOException {
-          if (!workdirProvider.cache(getScmRepository(context), getDirectory())) {
-            super.delete();
-          }
-        }
-      };
-    } catch (IOException e) {
+      ParentAndClone<R, W> parentAndClone = workdirProvider.getWorkdir(createWorkdirContext);
+      return new WorkingCopy<>(parentAndClone.getClone(), parentAndClone.getParent(), () -> this.close(createWorkdirContext, parentAndClone), parentAndClone.getDirectory());
+    } catch (Exception e) {
       throw new InternalRepositoryException(getScmRepository(context), "could not clone repository in temporary directory", e);
     }
   }
 
+  private void close(CreateWorkdirContext<R, W, C> createWorkdirContext, ParentAndClone<R, W> parentAndClone) {
+    try {
+      closeRepository(parentAndClone.getParent());
+    } catch (Exception e) {
+      LOG.warn("could not close central repository for {}", createWorkdirContext.getScmRepository(), e);
+    }
+    try {
+      closeWorkdir(parentAndClone.getClone());
+    } catch (Exception e) {
+      LOG.warn("could not close clone for {} in directory {}", createWorkdirContext.getScmRepository(), parentAndClone.getDirectory(), e);
+    }
+    try {
+      workdirProvider.contextClosed(createWorkdirContext, parentAndClone.getDirectory());
+    } catch (Exception e) {
+      LOG.warn("could not close context for {} with directory {}", createWorkdirContext.getScmRepository(), parentAndClone.getDirectory(), e);
+    }
+  }
+
   @FunctionalInterface
   public interface WorkdirInitializer<R, W> {
     ParentAndClone<R, W> initialize(File target) throws IOException;
@@ -92,7 +104,7 @@ public abstract class SimpleWorkdirFactory<R, W, C> implements WorkdirFactory<R,
     try {
       closeRepository(repository);
     } catch (Exception e) {
-      logger.warn("could not close temporary repository clone", e);
+      LOG.warn("could not close temporary repository clone", e);
     }
   }
 
@@ -100,7 +112,7 @@ public abstract class SimpleWorkdirFactory<R, W, C> implements WorkdirFactory<R,
     try {
       closeWorkdirInternal(repository);
     } catch (Exception e) {
-      logger.warn("could not close temporary repository clone", e);
+      LOG.warn("could not close temporary repository clone", e);
     }
   }
 

scm-core/src/main/java/sonia/scm/repository/util/WorkingCopy.java

@@ -24,30 +24,23 @@
 
 package sonia.scm.repository.util;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import sonia.scm.util.IOUtil;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.function.Consumer;
 
-public class WorkingCopy<R, W> implements AutoCloseable {
-
-  private static final Logger LOG = LoggerFactory.getLogger(WorkingCopy.class);
+public final class WorkingCopy<R, W> implements AutoCloseable {
 
   private final File directory;
   private final W workingRepository;
   private final R centralRepository;
-  private final Consumer<W> cleanupWorkdir;
-  private final Consumer<R> cleanupCentral;
+  private final Runnable close;
 
-  public WorkingCopy(W workingRepository, R centralRepository, Consumer<W> cleanupWorkdir, Consumer<R> cleanupCentral, File directory) {
+  public WorkingCopy(W workingRepository, R centralRepository, Runnable close, File directory) {
     this.directory = directory;
     this.workingRepository = workingRepository;
     this.centralRepository = centralRepository;
-    this.cleanupCentral = cleanupCentral;
-    this.cleanupWorkdir = cleanupWorkdir;
+    this.close = close;
   }
 
   public W getWorkingRepository() {
@@ -64,13 +57,7 @@ public class WorkingCopy<R, W> implements AutoCloseable {
 
   @Override
   public void close() {
-    try {
-      cleanupWorkdir.accept(workingRepository);
-      cleanupCentral.accept(centralRepository);
-      delete();
-    } catch (IOException e) {
-      LOG.warn("could not delete temporary workdir '{}'", directory, e);
-    }
+    close.run();
   }
 
   void delete() throws IOException {

scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java

@@ -30,6 +30,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import sonia.scm.repository.Repository;
+import sonia.scm.util.IOUtil;
 
 import java.io.Closeable;
 import java.io.File;
@@ -60,14 +61,16 @@ public class SimpleWorkdirFactoryTest {
     WorkdirProvider workdirProvider = new WorkdirProvider(temporaryFolder.newFolder());
     CacheSupportingWorkdirProvider configurableTestWorkdirProvider = new CacheSupportingWorkdirProvider() {
       @Override
-      public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer<R, W> initializer, SimpleWorkdirFactory.WorkdirReclaimer<R, W> reclaimer) throws IOException {
+      public <R, W, C> SimpleWorkdirFactory.ParentAndClone<R, W> getWorkdir(CreateWorkdirContext<R, W, C> context) throws IOException {
         workdir = workdirProvider.createNewWorkdir();
-        return initializer.initialize(workdir);
+        return context.getInitializer().initialize(workdir);
       }
 
       @Override
-      public boolean cache(Repository repository, File target) {
-        return workdirIsCached;
+      public void contextClosed(CreateWorkdirContext<?, ?, ?> createWorkdirContext, File workdir) throws Exception {
+        if (!workdirIsCached) {
+          IOUtil.delete(workdir);
+        }
       }
     };
     simpleWorkdirFactory = new SimpleWorkdirFactory<Closeable, Closeable, Context>(configurableTestWorkdirProvider) {