Update cache cleaner to HA-plugin version and add tests The HA-plugin implemented tests for the cache cleaner. These tests however relied on a refactored version of the cache cleaner that allowed to use a mocked cleanup task. To be able to make use of the tests and to be closer to the HA-plugin version of the cache cleaner, this change implements the refactoring and adds the tests. Change-Id: I72f65210d34049ae72baa094fa24bbe62f7db0f6
diff --git a/BUILD b/BUILD index bf3aa27..f00a805 100644 --- a/BUILD +++ b/BUILD
@@ -24,7 +24,17 @@ srcs = glob(["src/test/java/**/*.java"]), resources = glob(["src/test/resources/**/*"]), tags = ["websession-flatfile"], - deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [ + deps = [ + ":websession-flatfile__plugin_test_deps", + ], +) + +java_library( + name = "websession-flatfile__plugin_test_deps", + testonly = 1, + visibility = ["//visibility:public"], + exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [ ":websession-flatfile__plugin", + "@mockito//jar", ], )
diff --git a/WORKSPACE b/WORKSPACE new file mode 100644 index 0000000..78ac255 --- /dev/null +++ b/WORKSPACE
@@ -0,0 +1,5 @@ +workspace(name = "websession_flatfile") + +load("//:external_plugin_deps.bzl", "external_plugin_deps") + +external_plugin_deps()
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl new file mode 100644 index 0000000..1d8851b --- /dev/null +++ b/external_plugin_deps.bzl
@@ -0,0 +1,33 @@ +load("//tools/bzl:maven_jar.bzl", "maven_jar") + +def external_plugin_deps(): + maven_jar( + name = "mockito", + artifact = "org.mockito:mockito-core:2.27.0", + sha1 = "835fc3283b481f4758b8ef464cd560c649c08b00", + deps = [ + "@byte-buddy//jar", + "@byte-buddy-agent//jar", + "@objenesis//jar", + ], + ) + + BYTE_BUDDY_VERSION = "1.9.10" + + maven_jar( + name = "byte-buddy", + artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VERSION, + sha1 = "211a2b4d3df1eeef2a6cacf78d74a1f725e7a840", + ) + + maven_jar( + name = "byte-buddy-agent", + artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VERSION, + sha1 = "9674aba5ee793e54b864952b001166848da0f26b", + ) + + maven_jar( + name = "objenesis", + artifact = "org.objenesis:objenesis:2.6", + sha1 = "639033469776fd37c08358c6b92a4761feb2af4b", + )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleaner.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleaner.java index d5842a0..426870b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleaner.java +++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleaner.java
@@ -16,53 +16,67 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; +import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.server.git.WorkQueue; import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; +import java.util.concurrent.ScheduledFuture; -class FlatFileWebSessionCacheCleaner implements Runnable { +@Singleton +class FlatFileWebSessionCacheCleaner implements LifecycleListener { + private static final int INITIAL_DELAY_MS = 1000; + private final WorkQueue queue; + private final Provider<CleanupTask> cleanupTaskProvider; + private final long cleanupInterval; + private ScheduledFuture<?> scheduledCleanupTask; - static class CleanerLifecycle implements LifecycleListener { - private static final int INITIAL_DELAY_MS = 1000; - private final WorkQueue queue; - private final FlatFileWebSessionCacheCleaner cleaner; - private final long cleanupInterval; + static class CleanupTask implements Runnable { + private final FlatFileWebSessionCache flatFileWebSessionCache; + private final String pluginName; @Inject - CleanerLifecycle( - WorkQueue queue, - FlatFileWebSessionCacheCleaner cleaner, - @CleanupInterval long cleanupInterval) { - this.queue = queue; - this.cleaner = cleaner; - this.cleanupInterval = cleanupInterval; + CleanupTask(FlatFileWebSessionCache flatFileWebSessionCache, @PluginName String pluginName) { + this.flatFileWebSessionCache = flatFileWebSessionCache; + this.pluginName = pluginName; } @Override - public void start() { - queue - .getDefaultQueue() - .scheduleAtFixedRate(cleaner, INITIAL_DELAY_MS, cleanupInterval, MILLISECONDS); + public void run() { + flatFileWebSessionCache.cleanUp(); } @Override - public void stop() {} + public String toString() { + return String.format("[%s] Clean up expired file based websessions", pluginName); + } } - private FlatFileWebSessionCache flatFileWebSessionCache; - @Inject - FlatFileWebSessionCacheCleaner(FlatFileWebSessionCache flatFileWebSessionCache) { - this.flatFileWebSessionCache = flatFileWebSessionCache; + FlatFileWebSessionCacheCleaner( + WorkQueue queue, + Provider<CleanupTask> cleanupTaskProvider, + @CleanupInterval long cleanupInterval) { + this.queue = queue; + this.cleanupTaskProvider = cleanupTaskProvider; + this.cleanupInterval = cleanupInterval; } @Override - public void run() { - flatFileWebSessionCache.cleanUp(); + public void start() { + scheduledCleanupTask = + queue + .getDefaultQueue() + .scheduleAtFixedRate( + cleanupTaskProvider.get(), INITIAL_DELAY_MS, cleanupInterval, MILLISECONDS); } @Override - public String toString() { - return "FlatFile WebSession Cleaner"; + public void stop() { + if (scheduledCleanupTask != null) { + scheduledCleanupTask.cancel(true); + scheduledCleanupTask = null; + } } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/Module.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/Module.java index c98d0e9..4bc80b7 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/Module.java
@@ -24,7 +24,6 @@ import com.google.gerrit.server.config.SitePaths; import com.google.inject.Provides; import com.google.inject.Singleton; -import com.googlesource.gerrit.plugins.websession.flatfile.FlatFileWebSessionCacheCleaner.CleanerLifecycle; import java.nio.file.Path; import java.nio.file.Paths; @@ -34,7 +33,7 @@ @Override protected void configure() { - listener().to(CleanerLifecycle.class); + listener().to(FlatFileWebSessionCacheCleaner.class); } @Provides
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md index f893189..7e87215 100644 --- a/src/main/resources/Documentation/build.md +++ b/src/main/resources/Documentation/build.md
@@ -3,7 +3,15 @@ This @PLUGIN@ plugin is built with Bazel. -Clone (or link) this plugin to the `plugins` directory of Gerrit's source tree. +Clone (or link) this plugin to the `plugins` directory of Gerrit's +source tree. Put the external dependency Bazel build file into +the Gerrit /plugins directory, replacing the existing empty one. + +``` + cd gerrit/plugins + rm external_plugin_deps.bzl + ln -s @PLUGIN@/external_plugin_deps.bzl . +``` Then issue
diff --git a/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleanerTest.java b/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleanerTest.java new file mode 100644 index 0000000..5761bcc --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheCleanerTest.java
@@ -0,0 +1,104 @@ +// Copyright (C) 2017 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.websession.flatfile; + +import static com.google.common.truth.Truth.assertThat; +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.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.gerrit.server.git.WorkQueue; +import com.google.gerrit.server.git.WorkQueue.Executor; +import com.google.inject.Provider; +import com.googlesource.gerrit.plugins.websession.flatfile.FlatFileWebSessionCacheCleaner.CleanupTask; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class FlatFileWebSessionCacheCleanerTest { + + private static long CLEANUP_INTERVAL = 5000; + private static String SOME_PLUGIN_NAME = "somePluginName"; + + @Mock private Executor executorMock; + @Mock private ScheduledFuture<?> scheduledFutureMock; + @Mock private WorkQueue workQueueMock; + @Mock private Provider<CleanupTask> cleanupTaskProviderMock; + + private FlatFileWebSessionCacheCleaner cleaner; + + @Before + public void setUp() { + when(cleanupTaskProviderMock.get()).thenReturn(new CleanupTask(null, null)); + when(workQueueMock.getDefaultQueue()).thenReturn(executorMock); + doReturn(scheduledFutureMock) + .when(executorMock) + .scheduleAtFixedRate(isA(CleanupTask.class), anyLong(), anyLong(), isA(TimeUnit.class)); + cleaner = + new FlatFileWebSessionCacheCleaner( + workQueueMock, cleanupTaskProviderMock, CLEANUP_INTERVAL); + } + + @Test + public void testCleanupTaskRun() { + FlatFileWebSessionCache cacheMock = mock(FlatFileWebSessionCache.class); + CleanupTask task = new CleanupTask(cacheMock, null); + int numberOfRuns = 5; + for (int i = 0; i < numberOfRuns; i++) { + task.run(); + } + verify(cacheMock, times(numberOfRuns)).cleanUp(); + } + + @Test + public void testCleanupTaskToString() { + CleanupTask task = new CleanupTask(null, SOME_PLUGIN_NAME); + assertThat(task.toString()) + .isEqualTo(String.format("[%s] Clean up expired file based websessions", SOME_PLUGIN_NAME)); + } + + @Test + public void testCleanupTaskIsScheduledOnStart() { + cleaner.start(); + verify(executorMock, times(1)) + .scheduleAtFixedRate( + isA(CleanupTask.class), eq(1000l), eq(CLEANUP_INTERVAL), eq(TimeUnit.MILLISECONDS)); + } + + @Test + public void testCleanupTaskIsCancelledOnStop() { + cleaner.start(); + cleaner.stop(); + verify(scheduledFutureMock, times(1)).cancel(true); + } + + @Test + public void testCleanupTaskIsCancelledOnlyOnce() { + cleaner.start(); + cleaner.stop(); + cleaner.stop(); + verify(scheduledFutureMock, times(1)).cancel(true); + } +}