Merge branch stable-2.15 into master
* origin/stable-2.15:
Use java.util.concurrent.ScheduledThreadPoolExecutor for tests
Document that cache cleaning intervals < 1h are not supported
Apply changes to file-based websession cache from HA-plugin
Update cache cleaner to HA-plugin version and add tests
Add additional tests for websession cash from HA-plugin
Refactor test key assignment in cache tests
Use junit to provide a temporary directory for testing
Simplify cache-directory creation
Adapt system time manipulation to method used in HA-plugin
Reformat code with google-java-format 1.7
Fix getIfPresentObjectNonStringTest
Change-Id: Ibca0bd25bb008bbf9f3a635398d517dfba4b0efe
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/CleanupInterval.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/CleanupInterval.java
index 9874881..56c64fd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/CleanupInterval.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/CleanupInterval.java
@@ -17,10 +17,8 @@
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import com.google.inject.BindingAnnotation;
-
import java.lang.annotation.Retention;
@Retention(RUNTIME)
@BindingAnnotation
-public @interface CleanupInterval {
-}
+public @interface CleanupInterval {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSession.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSession.java
index e9a2b70..813e392 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSession.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSession.java
@@ -27,7 +27,6 @@
import com.google.inject.Provider;
import com.google.inject.servlet.RequestScoped;
import com.google.inject.servlet.ServletScopes;
-
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -39,18 +38,26 @@
protected void configure() {
bindScope(RequestScoped.class, ServletScopes.REQUEST);
DynamicItem.bind(binder(), WebSession.class)
- .to(FlatFileWebSession.class).in(RequestScoped.class);
+ .to(FlatFileWebSession.class)
+ .in(RequestScoped.class);
}
}
@Inject
- FlatFileWebSession(@RootRelative final Provider<HttpServletRequest> request,
- @RootRelative final Provider<HttpServletResponse> response,
- final WebSessionManagerFactory managerFactory,
- final FlatFileWebSessionCache cache, final AuthConfig authConfig,
- final Provider<AnonymousUser> anonymousProvider,
- final RequestFactory identified) {
- super(request.get(), response.get(), managerFactory.create(cache),
- authConfig, anonymousProvider, identified);
+ FlatFileWebSession(
+ @RootRelative Provider<HttpServletRequest> request,
+ @RootRelative Provider<HttpServletResponse> response,
+ WebSessionManagerFactory managerFactory,
+ FlatFileWebSessionCache cache,
+ AuthConfig authConfig,
+ Provider<AnonymousUser> anonymousProvider,
+ RequestFactory identified) {
+ super(
+ request.get(),
+ response.get(),
+ managerFactory.create(cache),
+ authConfig,
+ anonymousProvider,
+ identified);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCache.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCache.java
index a240aee..0d29720 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCache.java
@@ -22,10 +22,6 @@
import com.google.gerrit.httpd.WebSessionManager.Val;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
@@ -35,6 +31,9 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -44,26 +43,44 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@Singleton
-public class FlatFileWebSessionCache implements
- Cache<String, WebSessionManager.Val> {
- private static final Logger log = LoggerFactory
- .getLogger(FlatFileWebSessionCache.class);
+public class FlatFileWebSessionCache implements Cache<String, WebSessionManager.Val> {
+ private static final Logger log = LoggerFactory.getLogger(FlatFileWebSessionCache.class);
- private final Path dir;
+ /** Provides static methods to set the system clock for testing purposes only. */
+ static class TimeMachine {
+ private static Clock clock = Clock.systemDefaultZone();
+
+ private TimeMachine() {
+ throw new IllegalAccessError("Utility class. Not meant to be instantiated.");
+ }
+
+ static Instant now() {
+ return Instant.now(getClock());
+ }
+
+ static void useFixedClockAt(Instant instant) {
+ clock = Clock.fixed(instant, ZoneId.systemDefault());
+ }
+
+ static void useSystemDefaultZoneClock() {
+ clock = Clock.systemDefaultZone();
+ }
+
+ private static Clock getClock() {
+ return clock;
+ }
+ }
+
+ private final Path websessionsDir;
@Inject
- public FlatFileWebSessionCache(@WebSessionDir Path dir) {
- this.dir = dir;
- if (Files.notExists(dir)) {
- log.info(dir + " not found. Creating it.");
- try {
- Files.createDirectory(dir);
- } catch (IOException e) {
- log.error("Unable to create directory " + dir, e);
- }
- }
+ public FlatFileWebSessionCache(@WebSessionDir Path websessionsDir) throws IOException {
+ this.websessionsDir = websessionsDir;
+ Files.createDirectories(websessionsDir);
}
@Override
@@ -82,16 +99,17 @@
public void cleanUp() {
for (Path path : listFiles()) {
Val val = readFile(path);
- long expires = val.getExpiresAt();
- if (expires < System.currentTimeMillis()) {
- deleteFile(path);
+ if (val != null) {
+ Instant expires = Instant.ofEpochMilli(val.getExpiresAt());
+ if (expires.isBefore(TimeMachine.now())) {
+ deleteFile(path);
+ }
}
}
}
@Override
- public Val get(String key, Callable<? extends Val> valueLoader)
- throws ExecutionException {
+ public Val get(String key, Callable<? extends Val> valueLoader) throws ExecutionException {
Val value = getIfPresent(key);
if (value == null) {
try {
@@ -119,7 +137,7 @@
@Nullable
public Val getIfPresent(Object key) {
if (key instanceof String) {
- Path path = dir.resolve((String) key);
+ Path path = websessionsDir.resolve((String) key);
return readFile(path);
}
return null;
@@ -128,7 +146,7 @@
@Override
public void invalidate(Object key) {
if (key instanceof String) {
- deleteFile(dir.resolve((String) key));
+ deleteFile(websessionsDir.resolve((String) key));
}
}
@@ -149,17 +167,18 @@
@Override
public void put(String key, Val value) {
try {
- Path tempFile =
- Files.createTempFile(dir, UUID.randomUUID().toString(), null);
+ Path tempFile = Files.createTempFile(websessionsDir, UUID.randomUUID().toString(), null);
try (OutputStream fileStream = Files.newOutputStream(tempFile);
ObjectOutputStream objStream = new ObjectOutputStream(fileStream)) {
objStream.writeObject(value);
- Files.move(tempFile, tempFile.resolveSibling(key),
+ Files.move(
+ tempFile,
+ tempFile.resolveSibling(key),
StandardCopyOption.REPLACE_EXISTING,
StandardCopyOption.ATOMIC_MOVE);
}
} catch (IOException e) {
- log.warn("Cannot put into cache " + dir, e);
+ log.warn("Cannot put into cache " + websessionsDir, e);
}
}
@@ -182,16 +201,21 @@
}
private Val readFile(Path path) {
- if (Files.exists(path)) {
+ if (path.toFile().exists()) {
try (InputStream fileStream = Files.newInputStream(path);
ObjectInputStream objStream = new ObjectInputStream(fileStream)) {
return (Val) objStream.readObject();
} catch (ClassNotFoundException e) {
- log.warn("Entry " + path + " in cache " + dir + " has an incompatible "
- + "class and can't be deserialized. Invalidating entry.");
+ log.warn(
+ "Entry "
+ + path
+ + " in cache "
+ + websessionsDir
+ + " has an incompatible "
+ + "class and can't be deserialized. Invalidating entry.");
invalidate(path.getFileName().toString());
} catch (IOException e) {
- log.warn("Cannot read cache " + dir, e);
+ log.warn("Cannot read cache " + websessionsDir, e);
}
}
return null;
@@ -201,18 +225,18 @@
try {
Files.deleteIfExists(path);
} catch (IOException e) {
- log.error("Error trying to delete " + path + " from " + dir, e);
+ log.error("Error trying to delete " + path + " from " + websessionsDir, e);
}
}
private List<Path> listFiles() {
List<Path> files = new ArrayList<>();
- try (DirectoryStream<Path> dirStream = Files.newDirectoryStream(dir)) {
+ try (DirectoryStream<Path> dirStream = Files.newDirectoryStream(websessionsDir)) {
for (Path path : dirStream) {
files.add(path);
}
} catch (IOException e) {
- log.error("Cannot list files in cache " + dir, e);
+ log.error("Cannot list files in cache " + websessionsDir, e);
}
return files;
}
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 c6c89a6..5c88f92 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
@@ -15,54 +15,72 @@
package com.googlesource.gerrit.plugins.websession.flatfile;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+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 {
- 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;
+ private final WorkQueue queue;
+ private final Provider<CleanupTask> cleanupTaskProvider;
+ private final long cleanupIntervalMillis;
+ private ScheduledFuture<?> scheduledCleanupTask;
+
+ 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.cleanupIntervalMillis = cleanupInterval;
}
@Override
- public void run() {
- flatFileWebSessionCache.cleanUp();
+ public void start() {
+ scheduledCleanupTask =
+ queue
+ .getDefaultQueue()
+ .scheduleAtFixedRate(
+ cleanupTaskProvider.get(),
+ SECONDS.toMillis(1),
+ cleanupIntervalMillis,
+ 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 a5ea0c3..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,9 +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;
@@ -36,16 +33,16 @@
@Override
protected void configure() {
- listener().to(CleanerLifecycle.class);
+ listener().to(FlatFileWebSessionCacheCleaner.class);
}
@Provides
@Singleton
@WebSessionDir
- Path getWebSessionDir(SitePaths site, PluginConfigFactory cfg,
- @PluginName String pluginName) {
- return Paths.get(cfg.getFromGerritConfig(pluginName).getString("directory",
- site.site_path + "/websessions"));
+ Path getWebSessionDir(SitePaths site, PluginConfigFactory cfg, @PluginName String pluginName) {
+ return Paths.get(
+ cfg.getFromGerritConfig(pluginName)
+ .getString("directory", site.site_path + "/websessions"));
}
@Provides
@@ -53,9 +50,7 @@
@CleanupInterval
Long getCleanupInterval(PluginConfigFactory cfg, @PluginName String pluginName) {
String fromConfig =
- Strings.nullToEmpty(cfg.getFromGerritConfig(pluginName).getString(
- "cleanupInterval"));
- return HOURS.toMillis(ConfigUtil.getTimeUnit(fromConfig,
- DEFAULT_CLEANUP_INTERVAL, HOURS));
+ Strings.nullToEmpty(cfg.getFromGerritConfig(pluginName).getString("cleanupInterval"));
+ return HOURS.toMillis(ConfigUtil.getTimeUnit(fromConfig, DEFAULT_CLEANUP_INTERVAL, HOURS));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/WebSessionDir.java b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/WebSessionDir.java
index 02288bf..eb01bb1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/WebSessionDir.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/websession/flatfile/WebSessionDir.java
@@ -17,10 +17,8 @@
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import com.google.inject.BindingAnnotation;
-
import java.lang.annotation.Retention;
@Retention(RUNTIME)
@BindingAnnotation
-@interface WebSessionDir {
-}
+@interface WebSessionDir {}
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/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index cc5e71f..e0224e5 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -34,8 +34,6 @@
Values should use common time unit suffixes to express their setting:
-* s, sec, second, seconds
-* m, min, minute, minutes
* h, hr, hour, hours
* d, day, days
* w, week, weeks (`1 week` is treated as `7 days`)
@@ -44,6 +42,8 @@
If a time unit suffix is not specified, `hours` is assumed.
+Time intervals smaller than one hour are not supported.
+
If 'cleanupInterval' is not present in the configuration, the
cleanup operation is triggered every 24 hours.
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..de32dcd
--- /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.inject.Provider;
+import com.googlesource.gerrit.plugins.websession.flatfile.FlatFileWebSessionCacheCleaner.CleanupTask;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+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 ScheduledThreadPoolExecutor 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);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheTest.java b/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheTest.java
index 038135a..22401d1 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/websession/flatfile/FlatFileWebSessionCacheTest.java
@@ -16,104 +16,134 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.httpd.WebSessionManager.Val;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-
+import com.googlesource.gerrit.plugins.websession.flatfile.FlatFileWebSessionCache.TimeMachine;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.DirectoryStream;
-import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.nio.file.SimpleFileVisitor;
import java.nio.file.StandardCopyOption;
-import java.nio.file.attribute.BasicFileAttributes;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
public class FlatFileWebSessionCacheTest {
- private static final int DEFAULT_KEYS_SIZE = 10;
+ private static final String EXISTING_KEY = "aSceprtBc02YaMY573T5jfW64ZudJfPbDq";
+ private static final String EMPTY_KEY = "aOc2prqlZRpSO3LpauGO5efCLs1L9r9KkG";
+ private static final String INVALID_KEY = "aOFdpHriBM6dN055M13PjDdTZagl5r5aSG";
+ private static final String NEW_KEY = "abcde12345";
- private FlatFileWebSessionCache flatFileWebSessionCache;
- private Path dir;
- private String key;
- private String existingKey;
+ @Rule public TemporaryFolder tempFolder = new TemporaryFolder();
+
+ private FlatFileWebSessionCache cache;
+ private Path websessionDir;
@Before
public void createFlatFileWebSessionCache() throws Exception {
- dir = Files.createTempDirectory("websessions");
- key = "aOc2prqlZRpSO3LpauGO5efCLs1L9r9KkG";
- existingKey = "aSceprtBc02YaMY573T5jfW64ZudJfPbDq";
- flatFileWebSessionCache = new FlatFileWebSessionCache(dir);
- }
-
- @After
- public void tearDown() throws Exception {
- if (isDirEmpty(dir)) {
- Files.deleteIfExists(dir);
- } else {
- emptyAndDelete(dir);
- }
+ websessionDir = tempFolder.newFolder("websessions").toPath();
+ cache = new FlatFileWebSessionCache(websessionDir);
}
@Test
public void asMapTest() throws Exception {
- Files.createFile(dir.resolve(key));
- assertThat(flatFileWebSessionCache.asMap()).isEmpty();
+ loadKeyToCacheDir(EMPTY_KEY);
+ assertThat(cache.asMap()).isEmpty();
- loadExistingKeyToCacheDir();
- assertThat(flatFileWebSessionCache.asMap()).containsKey(existingKey);
+ loadKeyToCacheDir(INVALID_KEY);
+ assertThat(cache.asMap()).isEmpty();
+
+ loadKeyToCacheDir(EXISTING_KEY);
+ assertThat(cache.asMap()).containsKey(EXISTING_KEY);
}
@Test
public void constructorCreateDir() throws IOException {
- Path testDir = Paths.get("tmp");
- flatFileWebSessionCache = new FlatFileWebSessionCache(testDir);
- assertThat(Files.exists(testDir)).isTrue();
- Files.deleteIfExists(testDir);
+ assertThat(websessionDir.toFile().delete()).isTrue();
+ cache = new FlatFileWebSessionCache(websessionDir);
+ assertThat(websessionDir.toFile().exists()).isTrue();
}
@Test
public void cleanUpTest() throws Exception {
- loadExistingKeyToCacheDir();
- flatFileWebSessionCache.cleanUp();
- assertThat(isDirEmpty(dir)).isTrue();
+ loadKeyToCacheDir(EXISTING_KEY);
+ try {
+ long existingKeyExpireAt = cache.getIfPresent(EXISTING_KEY).getExpiresAt();
+ TimeMachine.useFixedClockAt(
+ Instant.ofEpochMilli(existingKeyExpireAt).minus(1, ChronoUnit.HOURS));
+ cache.cleanUp();
+ assertThat(isDirEmpty(websessionDir)).isFalse();
+
+ TimeMachine.useFixedClockAt(
+ Instant.ofEpochMilli(existingKeyExpireAt).plus(1, ChronoUnit.HOURS));
+ cache.cleanUp();
+ assertThat(isDirEmpty(websessionDir)).isTrue();
+ } finally {
+ TimeMachine.useSystemDefaultZoneClock();
+ }
}
@Test
- public void getAllPresentTest() throws Exception {
- Files.createFile(dir.resolve(key));
- loadExistingKeyToCacheDir();
- List<String> keys = Arrays.asList(new String[] {key, existingKey});
- assertThat(flatFileWebSessionCache.getAllPresent(keys))
- .containsKey(existingKey);
+ public void cleanUpWithErrorsWhileListingFilesTest() throws Exception {
+ tempFolder.delete();
+ cache.cleanUp();
+ assertThat(cache.size()).isEqualTo(0);
}
@Test
- public void getIfPresentKeyDoesNotExistTest() throws Exception {
- assertThat(flatFileWebSessionCache.getIfPresent(key)).isNull();
+ public void cleanUpWithErrorsWhileDeleteFileTest() throws Exception {
+ loadKeyToCacheDir(EXISTING_KEY);
+ try {
+ websessionDir.toFile().setWritable(false);
+ cache.cleanUp();
+ assertThat(cache.size()).isEqualTo(1);
+ } finally {
+ websessionDir.toFile().setWritable(true);
+ }
+ }
+
+ @Test
+ public void getIfPresentEmptyKeyTest() throws Exception {
+ assertThat(cache.getIfPresent(EMPTY_KEY)).isNull();
}
@Test
public void getIfPresentObjectNonStringTest() throws Exception {
- Path path = dir.resolve(key);
- assertThat(flatFileWebSessionCache.getIfPresent(path)).isNull();
+ assertThat(cache.getIfPresent(new Object())).isNull();
+ }
+
+ @Test
+ public void getIfPresentInvalidKeyTest() throws Exception {
+ loadKeyToCacheDir(INVALID_KEY);
+ Path path = websessionDir.resolve(INVALID_KEY);
+ assertThat(cache.getIfPresent(path.toString())).isNull();
}
@Test
public void getIfPresentTest() throws Exception {
- loadExistingKeyToCacheDir();
- assertThat(flatFileWebSessionCache.getIfPresent(existingKey)).isNotNull();
+ loadKeyToCacheDir(EXISTING_KEY);
+ assertThat(cache.getIfPresent(EXISTING_KEY)).isNotNull();
+ }
+
+ @Test
+ public void getAllPresentTest() throws Exception {
+ loadKeyToCacheDir(EMPTY_KEY);
+ loadKeyToCacheDir(INVALID_KEY);
+ loadKeyToCacheDir(EXISTING_KEY);
+ List<String> keys = ImmutableList.of(EMPTY_KEY, EXISTING_KEY);
+ assertThat(cache.getAllPresent(keys).size()).isEqualTo(1);
+ assertThat(cache.getAllPresent(keys)).containsKey(EXISTING_KEY);
}
@Test
@@ -124,12 +154,10 @@
return null;
}
}
- assertThat(flatFileWebSessionCache.get(existingKey, new ValueLoader()))
- .isNull();
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNull();
- loadExistingKeyToCacheDir();
- assertThat(flatFileWebSessionCache.get(existingKey, new ValueLoader()))
- .isNotNull();
+ loadKeyToCacheDir(EXISTING_KEY);
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNotNull();
}
@Test(expected = ExecutionException.class)
@@ -140,107 +168,111 @@
throw new Exception();
}
}
- assertThat(flatFileWebSessionCache.get(existingKey, new ValueLoader()))
- .isNull();
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNull();
}
@Test
public void invalidateAllCollectionTest() throws Exception {
- List<String> keys = createKeysCollection();
- flatFileWebSessionCache.invalidateAll(keys);
- assertThat(isDirEmpty(dir)).isTrue();
+ int numberOfKeys = 15;
+ List<String> keys = loadKeysToCacheDir(numberOfKeys);
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
+ assertThat(isDirEmpty(websessionDir)).isFalse();
+
+ cache.invalidateAll(keys);
+ assertThat(cache.size()).isEqualTo(0);
+ assertThat(isDirEmpty(websessionDir)).isTrue();
}
@Test
public void invalidateAllTest() throws Exception {
- createKeysCollection();
- flatFileWebSessionCache.invalidateAll();
- assertThat(isDirEmpty(dir)).isTrue();
+ int numberOfKeys = 5;
+ loadKeysToCacheDir(numberOfKeys);
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
+ assertThat(isDirEmpty(websessionDir)).isFalse();
+
+ cache.invalidateAll();
+ assertThat(cache.size()).isEqualTo(0);
+ assertThat(isDirEmpty(websessionDir)).isTrue();
}
@Test
public void invalidateTest() throws Exception {
- Path fileToDelete = Files.createFile(dir.resolve(key));
+ Path fileToDelete = Files.createFile(websessionDir.resolve(EXISTING_KEY));
assertThat(Files.exists(fileToDelete)).isTrue();
- flatFileWebSessionCache.invalidate(key);
+ cache.invalidate(EXISTING_KEY);
assertThat(Files.exists(fileToDelete)).isFalse();
}
@Test
public void invalidateTestObjectNotString() throws Exception {
- createKeysCollection();
- assertThat(flatFileWebSessionCache.size()).isEqualTo(DEFAULT_KEYS_SIZE);
- flatFileWebSessionCache.invalidate(new Object());
- assertThat(flatFileWebSessionCache.size()).isEqualTo(DEFAULT_KEYS_SIZE);
+ loadKeyToCacheDir(EXISTING_KEY);
+ assertThat(cache.size()).isEqualTo(1);
+ cache.invalidate(new Object());
+ assertThat(cache.size()).isEqualTo(1);
}
@Test
public void putTest() throws Exception {
- loadExistingKeyToCacheDir();
- Val val = flatFileWebSessionCache.getIfPresent(existingKey);
- String newKey = "abcde12345";
- flatFileWebSessionCache.put(newKey, val);
- assertThat(flatFileWebSessionCache.getIfPresent(newKey)).isNotNull();
+ loadKeyToCacheDir(EXISTING_KEY);
+ Val val = cache.getIfPresent(EXISTING_KEY);
+ cache.put(NEW_KEY, val);
+ assertThat(cache.getIfPresent(NEW_KEY)).isNotNull();
}
@Test
public void putAllTest() throws Exception {
- loadExistingKeyToCacheDir();
- Val val = flatFileWebSessionCache.getIfPresent(existingKey);
- String newKey = "abcde12345";
- Map<String, Val> sessions = ImmutableMap.of(newKey, val);
- flatFileWebSessionCache.putAll(sessions);
- assertThat(flatFileWebSessionCache.asMap()).containsKey(newKey);
+ loadKeyToCacheDir(EXISTING_KEY);
+ Val val = cache.getIfPresent(EXISTING_KEY);
+ Map<String, Val> sessions = ImmutableMap.of(NEW_KEY, val);
+ cache.putAll(sessions);
+ assertThat(cache.asMap()).containsKey(NEW_KEY);
+ }
+
+ @Test
+ public void putWithErrorsTest() throws Exception {
+ loadKeyToCacheDir(EXISTING_KEY);
+ Val val = cache.getIfPresent(EXISTING_KEY);
+ tempFolder.delete();
+ cache.put(NEW_KEY, val);
+ assertThat(cache.getIfPresent(NEW_KEY)).isNull();
}
@Test
public void sizeTest() throws Exception {
- createKeysCollection();
- assertThat(flatFileWebSessionCache.size()).isEqualTo(DEFAULT_KEYS_SIZE);
+ int numberOfKeys = 10;
+ loadKeysToCacheDir(numberOfKeys);
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
}
@Test
public void statTest() throws Exception {
- assertThat(flatFileWebSessionCache.stats()).isNull();
+ assertThat(cache.stats()).isNull();
}
- private List<String> createKeysCollection() throws IOException {
+ private List<String> loadKeysToCacheDir(int number) throws IOException {
List<String> keys = new ArrayList<>();
- for (int i = 0; i < DEFAULT_KEYS_SIZE; i++) {
- Path tmp = Files.createTempFile(dir, "cache", null);
+ for (int i = 0; i < number; i++) {
+ Path tmp = Files.createTempFile(websessionDir, "cache", null);
keys.add(tmp.getFileName().toString());
}
return keys;
}
- private void emptyAndDelete(Path dir) throws IOException {
- Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {
- @Override
- public FileVisitResult postVisitDirectory(Path dir, IOException exc)
- throws IOException {
- Files.delete(dir);
- return FileVisitResult.CONTINUE;
- }
-
- @Override
- public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
- throws IOException {
- Files.delete(file);
- return FileVisitResult.CONTINUE;
- }
- });
- }
-
private boolean isDirEmpty(final Path dir) throws IOException {
try (DirectoryStream<Path> dirStream = Files.newDirectoryStream(dir)) {
return !dirStream.iterator().hasNext();
}
}
- private void loadExistingKeyToCacheDir() throws IOException {
- InputStream in = loadFile(existingKey);
- Path target = dir.resolve(existingKey);
- Files.copy(in, target, StandardCopyOption.REPLACE_EXISTING);
+ private Path loadKeyToCacheDir(String key) throws IOException {
+ if (key.equals(EMPTY_KEY)) {
+ return Files.createFile(websessionDir.resolve(EMPTY_KEY));
+ }
+ try (InputStream in = loadFile(key)) {
+ Path target = websessionDir.resolve(key);
+ Files.copy(in, target, StandardCopyOption.REPLACE_EXISTING);
+ return target;
+ }
}
private InputStream loadFile(String file) {
diff --git a/src/test/resources/aOFdpHriBM6dN055M13PjDdTZagl5r5aSG b/src/test/resources/aOFdpHriBM6dN055M13PjDdTZagl5r5aSG
new file mode 100644
index 0000000..96c6933
--- /dev/null
+++ b/src/test/resources/aOFdpHriBM6dN055M13PjDdTZagl5r5aSG
Binary files differ
