Allow scheduling archiving in the same way as deletion Currently it's possible to schedule the deletion of trash folders, but not the archiving of repositories, which operates on its own, non configurable, schedule. This creates confusions as to why there are two separate schedules for two very similar tasks. Align scheduling of repositories archiving to that of trash folder deletion. Bug: Issue 461332435 Change-Id: I27595912142693143b1352fee0e13b6321e385de
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java index 1c8d62c..b64337a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java
@@ -88,12 +88,14 @@ Arrays.asList(cfg.getStringList("protectedProject")).stream() .map(Pattern::compile) .collect(toList()); + + checkForUnsupportedScheduleConfigEntries(); this.schedule = ScheduleConfig.builder(gerritConfig, "plugin") .setSubsection(pluginName) - .setKeyInterval("deleteTrashFolderInterval") - .setKeyStartTime("deleteTrashFolderStartTime") - .setKeyJitter("deleteTrashFolderJitter") + .setKeyInterval("cleanupInterval") + .setKeyStartTime("cleanupStartTime") + .setKeyJitter("cleanupJitter") .buildSchedule(); this.trashFolderName = cfg.getString("trashFolderName", DEFAULT_TRASH_FOLDER_NAME); } @@ -170,6 +172,20 @@ } } + private void checkForUnsupportedScheduleConfigEntries() { + List<String> deprecatedConfigEntries = + List.of( + "deleteTrashFolderInterval", "deleteTrashFolderStartTime", "deleteTrashFolderJitter"); + for (String deprecatedConfigEntry : deprecatedConfigEntries) { + if (cfg.getString(deprecatedConfigEntry) != null) { + log.atSevere().log( + "Ignoring unsupported configuration value %s found in configuration. Check the docs for" + + " the available settings.", + deprecatedConfigEntry); + } + } + } + public Optional<ScheduleConfig.Schedule> getSchedule() { return schedule; }
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 bb9138e..934be3c 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,13 +15,16 @@ 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 static java.util.concurrent.TimeUnit.SECONDS; +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; @@ -34,6 +37,7 @@ 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; @@ -41,26 +45,34 @@ 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) { + 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(), - SECONDS.toMillis(1), - TimeUnit.DAYS.toMillis(1), - MILLISECONDS); + repositoryCleanupTaskProvider.get(), initialDelay, period, MILLISECONDS); } @Override @@ -70,6 +82,11 @@ scheduledCleanupTask = null; } } + + @VisibleForTesting + ScheduledFuture<?> getWorkerFuture() { + return scheduledCleanupTask; + } } class RepositoryCleanupTask implements Runnable {
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 74c7087..41eb509 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -133,27 +133,27 @@ By default empty string -Delete Trash Folder Scheduling +Delete & Archiving Trash Folder Scheduling ============= -Trash folder cleanup can be scheduled to run periodically. -If no schedule is configured, the cleanup runs periodically, once every day. +Trash folder cleanup and repository archiving can be scheduled to run periodically. +If no schedule is configured, the operations run periodically, once every day. The configuration has to be added to the `@PLUGIN@.config` file. -plugin.@PLUGIN@.deleteTrashFolderStartTime +plugin.@PLUGIN@.cleanupStartTime : The start time for running trash folders deletion. The [start time](/Documentation/config-gerrit.html#schedule-configuration-startTime) for running trash folders deletion. -plugin.@PLUGIN@.deleteTrashFolderInterval +plugin.@PLUGIN@.cleanupInterval : The interval between successive trash folder deletions. The [interval](/Documentation/config-gerrit.html#schedule-configuration-interval) for running trash folders deletion. -plugin.@PLUGIN@.deleteTrashFolderJitter +plugin.@PLUGIN@.cleanupJitter : A maximum random delay that will be added to the job’s scheduled start time. See the [jitter documentation](/Documentation/config-gerrit.html#schedule-configuration-jitter)
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 e94390c..190ef87 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
@@ -15,33 +15,32 @@ package com.googlesource.gerrit.plugins.deleteproject.fs; import static com.google.common.truth.Truth.assertThat; -import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.stream.Collectors.toList; import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; 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; import java.io.IOException; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; import java.util.List; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; +import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.stream.StreamSupport; import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; import org.junit.Before; import org.junit.Rule; @@ -54,40 +53,43 @@ @RunWith(MockitoJUnitRunner.class) public class ArchiveRepositoryRemoverTest { + private static final int INITIAL_DELAY_MIN = 1; + private static final int INTERVAL_MILLIS = 10; private static final long ARCHIVE_DURATION = 1; - private static final long CLEANUP_INTERVAL = TimeUnit.DAYS.toMillis(1); private static final int NUMBER_OF_REPOS = 10; private static final String PLUGIN_NAME = "delete-project"; - @Mock private ScheduledExecutorService executorMock; - @Mock private ScheduledFuture<?> scheduledFutureMock; @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; private Path archiveRepo; + private FakeScheduledExecutorService fakeScheduledExecutor; + private Config cfg; @Before public void setUp() throws Exception { - when(cleanupTaskProviderMock.get()).thenReturn(new RepositoryCleanupTask(null, null)); - when(workQueueMock.getDefaultQueue()).thenReturn(executorMock); - doReturn(scheduledFutureMock) - .when(executorMock) - .scheduleAtFixedRate( - isA(RepositoryCleanupTask.class), anyLong(), anyLong(), isA(TimeUnit.class)); - remover = new ArchiveRepositoryRemover(workQueueMock, cleanupTaskProviderMock); + cfg = new Config(); archiveRepo = tempFolder.newFolder("archive").toPath(); 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); } @Test public void cleanUpOverdueRepositoriesTest() throws IOException { setupArchiveFolder(); try { + // Move forward time to make repositories archiveable TimeMachine.useFixedClockAt( Instant.ofEpochMilli(Files.getLastModifiedTime(archiveRepo).toMillis()) .plusMillis(TimeUnit.DAYS.toMillis(ARCHIVE_DURATION) + 10)); @@ -106,21 +108,47 @@ } @Test - public void testRepositoryCleanupTaskIsScheduledOnStart() { + public void cleanUpOverdueRepositoriesRespectsScheduleTest() throws IOException { + assertDirectoryContents(archiveRepo, true); + setupArchiveFolder(); + + ZonedDateTime initialDateTime = + ZonedDateTime.now(ZoneId.systemDefault()).plusMinutes(INITIAL_DELAY_MIN); + String initialDateTimeFormatted = initialDateTime.format(DateTimeFormatter.ofPattern("HH:mm")); + setupArchiveFolderCleanupSchedule( + initialDateTimeFormatted, String.format("%d milliseconds", INTERVAL_MILLIS)); + + ArchiveRepositoryRemover remover = + new ArchiveRepositoryRemover(workQueueMock, cleanupTaskProviderMock, pluginCfg); + remover.start(); - verify(executorMock, times(1)) - .scheduleAtFixedRate( - isA(RepositoryCleanupTask.class), - eq(SECONDS.toMillis(1)), - eq(CLEANUP_INTERVAL), - eq(TimeUnit.MILLISECONDS)); + try { + assertDirectoryContents(archiveRepo, false); + + fakeScheduledExecutor.advance(TimeUnit.MINUTES.toMillis(INITIAL_DELAY_MIN / 2), MILLISECONDS); + // Repository are not archived at 1/2 time of the initial delay + assertDirectoryContents(archiveRepo, false); + // Move forward time to make repositories archiveable + TimeMachine.useFixedClockAt( + Instant.ofEpochMilli(Files.getLastModifiedTime(archiveRepo).toMillis()) + .plusMillis(TimeUnit.DAYS.toMillis(ARCHIVE_DURATION) + 10)); + // Repositories are archived at full time of the initial delay + fakeScheduledExecutor.advance( + TimeUnit.MINUTES.toMillis(INITIAL_DELAY_MIN) + INTERVAL_MILLIS + 20, MILLISECONDS); + + assertDirectoryContents(archiveRepo, true); + } finally { + TimeMachine.useSystemPctZoneClock(); + } } @Test - public void testRepositoryCleanupTaskIsCancelledOnStop() { + public void testRepositoryCleanupWorkerFutureIsNullOnStop() { remover.start(); + assertThat(remover.getWorkerFuture()).isNotNull(); + remover.stop(); - verify(scheduledFutureMock, times(1)).cancel(true); + assertThat(remover.getWorkerFuture()).isNull(); } private void setupArchiveFolder() throws IOException { @@ -150,4 +178,16 @@ } } } + + private void setupArchiveFolderCleanupSchedule(String startTime, String interval) { + cfg.setString("plugin", PLUGIN_NAME, "cleanupStartTime", startTime); + cfg.setString("plugin", PLUGIN_NAME, "cleanupInterval", interval); + Optional<ScheduleConfig.Schedule> schedule = + ScheduleConfig.builder(cfg, "plugin") + .setSubsection(PLUGIN_NAME) + .setKeyStartTime("cleanupStartTime") + .setKeyInterval("cleanupInterval") + .buildSchedule(); + when(pluginCfg.getSchedule()).thenReturn(schedule); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java index 1be602b..bf108aa 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java
@@ -126,7 +126,7 @@ trashFolders.start(); try (FileRepository repoToDelete = createRepositoryToDelete(REPOSITORY_TO_DELETE); - FileRepository repoToKeep = createRepository("anotherRepo.git")) { + FileRepository repoToKeep = createRepository("anotherRepo.git")) { // Repository is not deleted at 1/2 time of the initial delay fakeScheduledExecutor.advance(DEFAULT_INITIAL_DELAY_MILLIS / 2, TimeUnit.MILLISECONDS); assertThatRepositoryExists(repoToDelete); @@ -139,7 +139,7 @@ } try (FileRepository repoToDelete = createRepositoryToDelete(REPOSITORY_TO_DELETE); - FileRepository repoToKeep = createRepository("anotherRepoAgain.git")) { + FileRepository repoToKeep = createRepository("anotherRepoAgain.git")) { // Repository recreated assertThatRepositoryExists(repoToDelete); assertThatRepositoryExists(repoToKeep); @@ -201,13 +201,13 @@ } private void setupTrashFolderCleanupSchedule(String startTime, String interval) { - cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "deleteTrashFolderStartTime", startTime); - cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "deleteTrashFolderInterval", interval); + cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "cleanupStartTime", startTime); + cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "cleanupInterval", interval); Optional<ScheduleConfig.Schedule> schedule = ScheduleConfig.builder(cfg, "plugin") .setSubsection(DELETE_PROJECT_PLUGIN) - .setKeyStartTime("deleteTrashFolderStartTime") - .setKeyInterval("deleteTrashFolderInterval") + .setKeyStartTime("cleanupStartTime") + .setKeyInterval("cleanupInterval") .buildSchedule(); when(pluginCfg.getSchedule()).thenReturn(schedule); }