From b22909adc06ed9bdc6a5930db7293a5d842d1d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Sun, 26 Apr 2020 17:36:36 +0200 Subject: [PATCH] CLean up workdir cache aoi --- .../util/CacheSupportingWorkdirProvider.java | 13 +--- .../util/CachingAllWorkdirProvider.java | 31 +++++---- .../repository/util/CreateWorkdirContext.java | 63 +++++++++++++++++++ .../util/NoneCachingWorkdirProvider.java | 10 +-- .../repository/util/SimpleWorkdirFactory.java | 38 +++++++---- .../scm/repository/util/WorkingCopy.java | 23 ++----- .../util/SimpleWorkdirFactoryTest.java | 11 ++-- 7 files changed, 125 insertions(+), 64 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/util/CreateWorkdirContext.java diff --git a/scm-core/src/main/java/sonia/scm/repository/util/CacheSupportingWorkdirProvider.java b/scm-core/src/main/java/sonia/scm/repository/util/CacheSupportingWorkdirProvider.java index e067d88f11..0a626b57d4 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/CacheSupportingWorkdirProvider.java +++ b/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 { - SimpleWorkdirFactory.ParentAndClone getWorkdir( - Repository scmRepository, - String requestedBranch, - C context, - SimpleWorkdirFactory.WorkdirInitializer initializer, - SimpleWorkdirFactory.WorkdirReclaimer reclaimer - ) throws IOException; + SimpleWorkdirFactory.ParentAndClone getWorkdir(CreateWorkdirContext context) throws Exception; - boolean cache(Repository repository, File target) throws IOException; + void contextClosed(CreateWorkdirContext createWorkdirContext, File workdir) throws Exception; } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/CachingAllWorkdirProvider.java b/scm-core/src/main/java/sonia/scm/repository/util/CachingAllWorkdirProvider.java index ebb9b7a7c6..75fcf24799 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/CachingAllWorkdirProvider.java +++ b/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 workdirs = new HashMap<>(); + private final Map workdirs = new ConcurrentHashMap<>(); private final WorkdirProvider workdirProvider; @@ -45,18 +44,17 @@ public class CachingAllWorkdirProvider implements CacheSupportingWorkdirProvider } @Override - public SimpleWorkdirFactory.ParentAndClone getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer initializer, SimpleWorkdirFactory.WorkdirReclaimer reclaimer) throws IOException { - String id = scmRepository.getId(); - if (workdirs.containsKey(id)) { - File existingWorkdir = workdirs.get(id); + public SimpleWorkdirFactory.ParentAndClone getWorkdir(CreateWorkdirContext 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 SimpleWorkdirFactory.ParentAndClone createNewWorkdir(SimpleWorkdirFactory.WorkdirInitializer 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); } } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/CreateWorkdirContext.java b/scm-core/src/main/java/sonia/scm/repository/util/CreateWorkdirContext.java new file mode 100644 index 0000000000..26314d2f1e --- /dev/null +++ b/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 { + private final Repository scmRepository; + private final String requestedBranch; + private final C context; + private final SimpleWorkdirFactory.WorkdirInitializer initializer; + private final SimpleWorkdirFactory.WorkdirReclaimer reclaimer; + + public CreateWorkdirContext(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer initializer, SimpleWorkdirFactory.WorkdirReclaimer 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 getInitializer() { + return initializer; + } + + public SimpleWorkdirFactory.WorkdirReclaimer getReclaimer() { + return reclaimer; + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/util/NoneCachingWorkdirProvider.java b/scm-core/src/main/java/sonia/scm/repository/util/NoneCachingWorkdirProvider.java index b46c0dcdf3..3856bb9cad 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/NoneCachingWorkdirProvider.java +++ b/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 SimpleWorkdirFactory.ParentAndClone getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer initializer, SimpleWorkdirFactory.WorkdirReclaimer reclaimer) throws IOException { - return initializer.initialize(workdirProvider.createNewWorkdir()); + public SimpleWorkdirFactory.ParentAndClone getWorkdir(CreateWorkdirContext 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); } } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java b/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java index 539dcac116..80f93a421e 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java @@ -34,7 +34,7 @@ import java.io.IOException; public abstract class SimpleWorkdirFactory implements WorkdirFactory { - 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 implements WorkdirFactory createWorkingCopy(C context, String initialBranch) { try { - ParentAndClone parentAndClone = workdirProvider.getWorkdir( + CreateWorkdirContext createWorkdirContext = new CreateWorkdirContext<>( getScmRepository(context), initialBranch, context, newFolder -> cloneRepository(context, newFolder, initialBranch), cachedFolder -> reclaimRepository(context, cachedFolder, initialBranch) ); - return new WorkingCopy(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 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 createWorkdirContext, ParentAndClone 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 { ParentAndClone initialize(File target) throws IOException; @@ -92,7 +104,7 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory implements WorkdirFactory implements AutoCloseable { - - private static final Logger LOG = LoggerFactory.getLogger(WorkingCopy.class); +public final class WorkingCopy implements AutoCloseable { private final File directory; private final W workingRepository; private final R centralRepository; - private final Consumer cleanupWorkdir; - private final Consumer cleanupCentral; + private final Runnable close; - public WorkingCopy(W workingRepository, R centralRepository, Consumer cleanupWorkdir, Consumer 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 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 { diff --git a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java b/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java index ce634ee6d2..5299907270 100644 --- a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java +++ b/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 SimpleWorkdirFactory.ParentAndClone getWorkdir(Repository scmRepository, String requestedBranch, C context, SimpleWorkdirFactory.WorkdirInitializer initializer, SimpleWorkdirFactory.WorkdirReclaimer reclaimer) throws IOException { + public SimpleWorkdirFactory.ParentAndClone getWorkdir(CreateWorkdirContext 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(configurableTestWorkdirProvider) {