Refactor common logic between Archiving and Deletion flows As archiving of repositories and deletion of trash folders share a lot of common logic, extract common parts in a super class in order to facilitate readability and maintainability. Bug: Issue 461332435 Change-Id: I27deb855b095d8e73d5b09701b67c27fe8c759e5
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/AbstractScheduledTask.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/AbstractScheduledTask.java new file mode 100644 index 0000000..cefbfc9 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/AbstractScheduledTask.java
@@ -0,0 +1,69 @@ +// Copyright (C) 2025 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.deleteproject.fs; + +import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_INITIAL_DELAY_MILLIS; +import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_PERIOD_DAYS; + +import com.google.common.annotations.VisibleForTesting; +import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.server.config.ScheduleConfig; +import com.google.gerrit.server.git.WorkQueue; +import java.util.Optional; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +public abstract class AbstractScheduledTask implements LifecycleListener, Runnable { + + private final WorkQueue queue; + private final Optional<ScheduleConfig.Schedule> schedule; + private ScheduledFuture<?> scheduledTask; + + protected AbstractScheduledTask(WorkQueue queue, Optional<ScheduleConfig.Schedule> schedule) { + this.queue = queue; + this.schedule = schedule; + } + + @Override + public void start() { + long initialDelayMs = + schedule.map(ScheduleConfig.Schedule::initialDelay).orElse(DEFAULT_INITIAL_DELAY_MILLIS); + long periodMs = + schedule + .map(ScheduleConfig.Schedule::interval) + .orElse(TimeUnit.DAYS.toMillis(DEFAULT_PERIOD_DAYS)); + + scheduledTask = + queue + .getDefaultQueue() + .scheduleAtFixedRate(this, initialDelayMs, periodMs, TimeUnit.MILLISECONDS); + } + + @Override + public void stop() { + if (scheduledTask != null) { + scheduledTask.cancel(true); + scheduledTask = null; + } + } + + @Override + public abstract void run(); + + @VisibleForTesting + ScheduledFuture<?> getWorkerFuture() { + return scheduledTask; + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemover.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemover.java index 934be3c..8628644 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemover.java +++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemover.java
@@ -15,19 +15,11 @@ package com.googlesource.gerrit.plugins.deleteproject.fs; import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; -import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_INITIAL_DELAY_MILLIS; -import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_PERIOD_DAYS; -import static java.util.concurrent.TimeUnit.MILLISECONDS; - -import com.google.common.annotations.VisibleForTesting; import com.google.common.flogger.FluentLogger; import com.google.common.io.MoreFiles; import com.google.gerrit.extensions.annotations.PluginName; -import com.google.gerrit.extensions.events.LifecycleListener; -import com.google.gerrit.server.config.ScheduleConfig; import com.google.gerrit.server.git.WorkQueue; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.deleteproject.Configuration; import com.googlesource.gerrit.plugins.deleteproject.TimeMachine; @@ -37,67 +29,19 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.Optional; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; @Singleton -public class ArchiveRepositoryRemover implements LifecycleListener { - - private final WorkQueue queue; - private final Optional<ScheduleConfig.Schedule> schedule; - private final Provider<RepositoryCleanupTask> repositoryCleanupTaskProvider; - private ScheduledFuture<?> scheduledCleanupTask; - - @Inject - ArchiveRepositoryRemover( - WorkQueue queue, - Provider<RepositoryCleanupTask> repositoryCleanupTaskProvider, - Configuration pluginCfg) { - schedule = pluginCfg.getSchedule(); - this.queue = queue; - this.repositoryCleanupTaskProvider = repositoryCleanupTaskProvider; - } - - @Override - public void start() { - long initialDelay = DEFAULT_INITIAL_DELAY_MILLIS; - long period = TimeUnit.DAYS.toMillis(DEFAULT_PERIOD_DAYS); - if (schedule.isPresent()) { - initialDelay = schedule.get().initialDelay(); - period = schedule.get().interval(); - } - - scheduledCleanupTask = - queue - .getDefaultQueue() - .scheduleAtFixedRate( - repositoryCleanupTaskProvider.get(), initialDelay, period, MILLISECONDS); - } - - @Override - public void stop() { - if (scheduledCleanupTask != null) { - scheduledCleanupTask.cancel(true); - scheduledCleanupTask = null; - } - } - - @VisibleForTesting - ScheduledFuture<?> getWorkerFuture() { - return scheduledCleanupTask; - } -} - -class RepositoryCleanupTask implements Runnable { +public class ArchiveRepositoryRemover extends AbstractScheduledTask { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final Configuration config; private final String pluginName; @Inject - RepositoryCleanupTask(Configuration config, @PluginName String pluginName) { - this.config = config; + ArchiveRepositoryRemover( + WorkQueue queue, Configuration pluginCfg, @PluginName String pluginName) { + super(queue, pluginCfg.getSchedule()); + this.config = pluginCfg; this.pluginName = pluginName; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java index ac30db24..62bda19 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java +++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java
@@ -14,9 +14,6 @@ package com.googlesource.gerrit.plugins.deleteproject.fs; import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; -import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_INITIAL_DELAY_MILLIS; -import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_PERIOD_DAYS; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -24,10 +21,8 @@ import com.google.common.flogger.FluentLogger; import com.google.common.io.MoreFiles; import com.google.gerrit.extensions.annotations.PluginName; -import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.RepositoryConfig; -import com.google.gerrit.server.config.ScheduleConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.WorkQueue; import com.google.inject.Inject; @@ -37,19 +32,15 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Iterator; -import java.util.Optional; import java.util.Set; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; import java.util.stream.Stream; import org.eclipse.jgit.lib.Config; -public class DeleteTrashFolders implements LifecycleListener { +public class DeleteTrashFolders extends AbstractScheduledTask { private static final FluentLogger log = FluentLogger.forEnclosingClass(); - private final WorkQueue workQueue; private final String pluginName; static class TrashFolderPredicate { @@ -89,10 +80,8 @@ } } - private Set<Path> repoFolders; + private final Set<Path> repoFolders; - private ScheduledFuture<?> threadCompleted; - private final Optional<ScheduleConfig.Schedule> schedule; private final long deleteTrashFoldersMaxAllowedTime; private final String trashFolderName; @@ -104,45 +93,27 @@ Configuration pluginCfg, WorkQueue workQueue, @PluginName String pluginName) { - repoFolders = Sets.newHashSet(); + super(workQueue, pluginCfg.getSchedule()); + this.repoFolders = Sets.newHashSet(); repoFolders.add(site.resolve(cfg.getString("gerrit", null, "basePath"))); repoFolders.addAll(repositoryCfg.getAllBasePaths()); - schedule = pluginCfg.getSchedule(); trashFolderName = pluginCfg.getTrashFolderName(); deleteTrashFoldersMaxAllowedTime = pluginCfg.getDeleteTrashFoldersMaxAllowedTime(); - this.workQueue = workQueue; + this.pluginName = pluginName; } @Override - public void start() { - String taskName = String.format("[%s]: DeleteTrashFolders under %s", pluginName, repoFolders); - Runnable deleteTrashFoldersRunnable = - new Runnable() { - @Override - public void run() { - log.atInfo().log("%s : STARTED", taskName); - evaluateIfTrashWithTimeLimit(); - log.atInfo().log("%s : ENDED", taskName); - } + public void run() { + String taskName = toString(); + log.atInfo().log("%s : STARTED", taskName); + evaluateIfTrashWithTimeLimit(); + log.atInfo().log("%s : ENDED", taskName); + } - @Override - public String toString() { - return taskName; - } - }; - - ScheduledExecutorService scheduledExecutor = workQueue.getDefaultQueue(); - long initialDelay = DEFAULT_INITIAL_DELAY_MILLIS; - long period = TimeUnit.DAYS.toMillis(DEFAULT_PERIOD_DAYS); - if (schedule.isPresent()) { - initialDelay = schedule.get().initialDelay(); - period = schedule.get().interval(); - } - - threadCompleted = - scheduledExecutor.scheduleAtFixedRate( - deleteTrashFoldersRunnable, initialDelay, period, MILLISECONDS); + @Override + public String toString() { + return String.format("[%s]: DeleteTrashFolders under %s", pluginName, repoFolders); } private void evaluateIfTrashWithTimeLimit() { @@ -182,11 +153,6 @@ return false; } - @VisibleForTesting - ScheduledFuture<?> getWorkerFuture() { - return threadCompleted; - } - private void recursivelyDelete(Path folder) { try { MoreFiles.deleteRecursively(folder, ALLOW_INSECURE); @@ -194,12 +160,4 @@ log.atSevere().withCause(e).log("Failed to delete %s", folder); } } - - @Override - public void stop() { - if (threadCompleted != null) { - threadCompleted.cancel(true); - threadCompleted = null; - } - } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/FakeScheduledExecutorService.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/FakeScheduledExecutorService.java index c4a6652..0c95411 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/FakeScheduledExecutorService.java +++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/FakeScheduledExecutorService.java
@@ -135,7 +135,7 @@ @Override public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) { - throw new UnsupportedOperationException(); + return queue(new FakeScheduledFuture<>(callable(command), delay, unit)); } @Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemoverTest.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemoverTest.java index 190ef87..ee86cf8 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemoverTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/ArchiveRepositoryRemoverTest.java
@@ -23,7 +23,6 @@ import com.google.common.base.Joiner; import com.google.gerrit.server.config.ScheduleConfig; import com.google.gerrit.server.git.WorkQueue; -import com.google.inject.Provider; import com.googlesource.gerrit.plugins.deleteproject.Configuration; import com.googlesource.gerrit.plugins.deleteproject.FakeScheduledExecutorService; import com.googlesource.gerrit.plugins.deleteproject.TimeMachine; @@ -60,10 +59,7 @@ private static final String PLUGIN_NAME = "delete-project"; @Mock private WorkQueue workQueueMock; - @Mock private Provider<RepositoryCleanupTask> cleanupTaskProviderMock; @Mock private Configuration configMock; - @Mock private Configuration pluginCfg; - @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); private ArchiveRepositoryRemover remover; @@ -78,11 +74,9 @@ when(configMock.getArchiveFolder()).thenReturn(archiveRepo); when(configMock.getArchiveDuration()).thenReturn(ARCHIVE_DURATION); fakeScheduledExecutor = new FakeScheduledExecutorService(); - when(cleanupTaskProviderMock.get()) - .thenReturn(new RepositoryCleanupTask(configMock, PLUGIN_NAME)); when(workQueueMock.getDefaultQueue()).thenReturn(fakeScheduledExecutor); - remover = new ArchiveRepositoryRemover(workQueueMock, cleanupTaskProviderMock, pluginCfg); + remover = new ArchiveRepositoryRemover(workQueueMock, configMock, PLUGIN_NAME); } @Test @@ -94,9 +88,8 @@ Instant.ofEpochMilli(Files.getLastModifiedTime(archiveRepo).toMillis()) .plusMillis(TimeUnit.DAYS.toMillis(ARCHIVE_DURATION) + 10)); - RepositoryCleanupTask task = new RepositoryCleanupTask(configMock, PLUGIN_NAME); - task.run(); - assertThat(task.toString()) + remover.run(); + assertThat(remover.toString()) .isEqualTo( String.format( "[%s]: Clean up expired git repositories from the archive [%s]", @@ -119,7 +112,7 @@ initialDateTimeFormatted, String.format("%d milliseconds", INTERVAL_MILLIS)); ArchiveRepositoryRemover remover = - new ArchiveRepositoryRemover(workQueueMock, cleanupTaskProviderMock, pluginCfg); + new ArchiveRepositoryRemover(workQueueMock, configMock, PLUGIN_NAME); remover.start(); try { @@ -188,6 +181,7 @@ .setKeyStartTime("cleanupStartTime") .setKeyInterval("cleanupInterval") .buildSchedule(); - when(pluginCfg.getSchedule()).thenReturn(schedule); + + when(configMock.getSchedule()).thenReturn(schedule); } }