Apply changes to file-based websession cache from HA-plugin
The HA-plugin contains a modified version of the websession-flatfile
plugin. The version used in the HA-plugin was updated since it was
forked, but those changes were not applied to the websession-flatfile
plugin itself.
This change applies some minor changes made to the corresponding code
in the HA-plugin to the websession-flatfile plugin. This includes:
- Variable renaming
- `dir` -> `websessionsDir`
- `flatFileWebSessionCache` -> `cache`
- `cleanupInterval` -> `cleanupIntervalMillis`
- Removing `final` from parameters in `FlatFileWebSession`
- Using `SECONDS.toMillis(1)` inline as initial delay for cleanup tasks
instead of a class field (`INITIAL_DELAY_MS = 1000`)
- Using `path.toFile().exists()` instead of `Files.exists(path)`.
While minor changes, they bring the code closer to what is used in the
HA-plugin, making it easier to port future changes.
Change-Id: I38da3e9fc7dc2dcbada82b63d51acd595f7b93a8
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 dac8be6..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
@@ -45,13 +45,13 @@
@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) {
+ @RootRelative Provider<HttpServletRequest> request,
+ @RootRelative Provider<HttpServletResponse> response,
+ WebSessionManagerFactory managerFactory,
+ FlatFileWebSessionCache cache,
+ AuthConfig authConfig,
+ Provider<AnonymousUser> anonymousProvider,
+ RequestFactory identified) {
super(
request.get(),
response.get(),
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 fc6ee9b..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
@@ -75,12 +75,12 @@
}
}
- private final Path dir;
+ private final Path websessionsDir;
@Inject
- public FlatFileWebSessionCache(@WebSessionDir Path dir) throws IOException {
- this.dir = dir;
- Files.createDirectories(dir);
+ public FlatFileWebSessionCache(@WebSessionDir Path websessionsDir) throws IOException {
+ this.websessionsDir = websessionsDir;
+ Files.createDirectories(websessionsDir);
}
@Override
@@ -137,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;
@@ -146,7 +146,7 @@
@Override
public void invalidate(Object key) {
if (key instanceof String) {
- deleteFile(dir.resolve((String) key));
+ deleteFile(websessionsDir.resolve((String) key));
}
}
@@ -167,7 +167,7 @@
@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);
@@ -178,7 +178,7 @@
StandardCopyOption.ATOMIC_MOVE);
}
} catch (IOException e) {
- log.warn("Cannot put into cache " + dir, e);
+ log.warn("Cannot put into cache " + websessionsDir, e);
}
}
@@ -201,7 +201,7 @@
}
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();
@@ -210,12 +210,12 @@
"Entry "
+ path
+ " in cache "
- + dir
+ + 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;
@@ -225,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 426870b..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,6 +15,7 @@
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;
@@ -26,10 +27,10 @@
@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 final long cleanupIntervalMillis;
private ScheduledFuture<?> scheduledCleanupTask;
static class CleanupTask implements Runnable {
@@ -60,7 +61,7 @@
@CleanupInterval long cleanupInterval) {
this.queue = queue;
this.cleanupTaskProvider = cleanupTaskProvider;
- this.cleanupInterval = cleanupInterval;
+ this.cleanupIntervalMillis = cleanupInterval;
}
@Override
@@ -69,7 +70,10 @@
queue
.getDefaultQueue()
.scheduleAtFixedRate(
- cleanupTaskProvider.get(), INITIAL_DELAY_MS, cleanupInterval, MILLISECONDS);
+ cleanupTaskProvider.get(),
+ SECONDS.toMillis(1),
+ cleanupIntervalMillis,
+ MILLISECONDS);
}
@Override
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 c7a7fc6..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
@@ -47,48 +47,48 @@
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
- private FlatFileWebSessionCache flatFileWebSessionCache;
- private Path dir;
+ private FlatFileWebSessionCache cache;
+ private Path websessionDir;
@Before
public void createFlatFileWebSessionCache() throws Exception {
- dir = tempFolder.newFolder("websessions").toPath();
- flatFileWebSessionCache = new FlatFileWebSessionCache(dir);
+ websessionDir = tempFolder.newFolder("websessions").toPath();
+ cache = new FlatFileWebSessionCache(websessionDir);
}
@Test
public void asMapTest() throws Exception {
loadKeyToCacheDir(EMPTY_KEY);
- assertThat(flatFileWebSessionCache.asMap()).isEmpty();
+ assertThat(cache.asMap()).isEmpty();
loadKeyToCacheDir(INVALID_KEY);
- assertThat(flatFileWebSessionCache.asMap()).isEmpty();
+ assertThat(cache.asMap()).isEmpty();
loadKeyToCacheDir(EXISTING_KEY);
- assertThat(flatFileWebSessionCache.asMap()).containsKey(EXISTING_KEY);
+ assertThat(cache.asMap()).containsKey(EXISTING_KEY);
}
@Test
public void constructorCreateDir() throws IOException {
- assertThat(dir.toFile().delete()).isTrue();
- flatFileWebSessionCache = new FlatFileWebSessionCache(dir);
- assertThat(dir.toFile().exists()).isTrue();
+ assertThat(websessionDir.toFile().delete()).isTrue();
+ cache = new FlatFileWebSessionCache(websessionDir);
+ assertThat(websessionDir.toFile().exists()).isTrue();
}
@Test
public void cleanUpTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
try {
- long existingKeyExpireAt = flatFileWebSessionCache.getIfPresent(EXISTING_KEY).getExpiresAt();
+ long existingKeyExpireAt = cache.getIfPresent(EXISTING_KEY).getExpiresAt();
TimeMachine.useFixedClockAt(
Instant.ofEpochMilli(existingKeyExpireAt).minus(1, ChronoUnit.HOURS));
- flatFileWebSessionCache.cleanUp();
- assertThat(isDirEmpty(dir)).isFalse();
+ cache.cleanUp();
+ assertThat(isDirEmpty(websessionDir)).isFalse();
TimeMachine.useFixedClockAt(
Instant.ofEpochMilli(existingKeyExpireAt).plus(1, ChronoUnit.HOURS));
- flatFileWebSessionCache.cleanUp();
- assertThat(isDirEmpty(dir)).isTrue();
+ cache.cleanUp();
+ assertThat(isDirEmpty(websessionDir)).isTrue();
} finally {
TimeMachine.useSystemDefaultZoneClock();
}
@@ -97,43 +97,43 @@
@Test
public void cleanUpWithErrorsWhileListingFilesTest() throws Exception {
tempFolder.delete();
- flatFileWebSessionCache.cleanUp();
- assertThat(flatFileWebSessionCache.size()).isEqualTo(0);
+ cache.cleanUp();
+ assertThat(cache.size()).isEqualTo(0);
}
@Test
public void cleanUpWithErrorsWhileDeleteFileTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
try {
- dir.toFile().setWritable(false);
- flatFileWebSessionCache.cleanUp();
- assertThat(flatFileWebSessionCache.size()).isEqualTo(1);
+ websessionDir.toFile().setWritable(false);
+ cache.cleanUp();
+ assertThat(cache.size()).isEqualTo(1);
} finally {
- dir.toFile().setWritable(true);
+ websessionDir.toFile().setWritable(true);
}
}
@Test
public void getIfPresentEmptyKeyTest() throws Exception {
- assertThat(flatFileWebSessionCache.getIfPresent(EMPTY_KEY)).isNull();
+ assertThat(cache.getIfPresent(EMPTY_KEY)).isNull();
}
@Test
public void getIfPresentObjectNonStringTest() throws Exception {
- assertThat(flatFileWebSessionCache.getIfPresent(new Object())).isNull();
+ assertThat(cache.getIfPresent(new Object())).isNull();
}
@Test
public void getIfPresentInvalidKeyTest() throws Exception {
loadKeyToCacheDir(INVALID_KEY);
- Path path = dir.resolve(INVALID_KEY);
- assertThat(flatFileWebSessionCache.getIfPresent(path.toString())).isNull();
+ Path path = websessionDir.resolve(INVALID_KEY);
+ assertThat(cache.getIfPresent(path.toString())).isNull();
}
@Test
public void getIfPresentTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
- assertThat(flatFileWebSessionCache.getIfPresent(EXISTING_KEY)).isNotNull();
+ assertThat(cache.getIfPresent(EXISTING_KEY)).isNotNull();
}
@Test
@@ -142,8 +142,8 @@
loadKeyToCacheDir(INVALID_KEY);
loadKeyToCacheDir(EXISTING_KEY);
List<String> keys = ImmutableList.of(EMPTY_KEY, EXISTING_KEY);
- assertThat(flatFileWebSessionCache.getAllPresent(keys).size()).isEqualTo(1);
- assertThat(flatFileWebSessionCache.getAllPresent(keys)).containsKey(EXISTING_KEY);
+ assertThat(cache.getAllPresent(keys).size()).isEqualTo(1);
+ assertThat(cache.getAllPresent(keys)).containsKey(EXISTING_KEY);
}
@Test
@@ -154,10 +154,10 @@
return null;
}
}
- assertThat(flatFileWebSessionCache.get(EXISTING_KEY, new ValueLoader())).isNull();
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNull();
loadKeyToCacheDir(EXISTING_KEY);
- assertThat(flatFileWebSessionCache.get(EXISTING_KEY, new ValueLoader())).isNotNull();
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNotNull();
}
@Test(expected = ExecutionException.class)
@@ -168,91 +168,91 @@
throw new Exception();
}
}
- assertThat(flatFileWebSessionCache.get(EXISTING_KEY, new ValueLoader())).isNull();
+ assertThat(cache.get(EXISTING_KEY, new ValueLoader())).isNull();
}
@Test
public void invalidateAllCollectionTest() throws Exception {
int numberOfKeys = 15;
List<String> keys = loadKeysToCacheDir(numberOfKeys);
- assertThat(flatFileWebSessionCache.size()).isEqualTo(numberOfKeys);
- assertThat(isDirEmpty(dir)).isFalse();
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
+ assertThat(isDirEmpty(websessionDir)).isFalse();
- flatFileWebSessionCache.invalidateAll(keys);
- assertThat(flatFileWebSessionCache.size()).isEqualTo(0);
- assertThat(isDirEmpty(dir)).isTrue();
+ cache.invalidateAll(keys);
+ assertThat(cache.size()).isEqualTo(0);
+ assertThat(isDirEmpty(websessionDir)).isTrue();
}
@Test
public void invalidateAllTest() throws Exception {
int numberOfKeys = 5;
loadKeysToCacheDir(numberOfKeys);
- assertThat(flatFileWebSessionCache.size()).isEqualTo(numberOfKeys);
- assertThat(isDirEmpty(dir)).isFalse();
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
+ assertThat(isDirEmpty(websessionDir)).isFalse();
- flatFileWebSessionCache.invalidateAll();
- assertThat(flatFileWebSessionCache.size()).isEqualTo(0);
- assertThat(isDirEmpty(dir)).isTrue();
+ cache.invalidateAll();
+ assertThat(cache.size()).isEqualTo(0);
+ assertThat(isDirEmpty(websessionDir)).isTrue();
}
@Test
public void invalidateTest() throws Exception {
- Path fileToDelete = Files.createFile(dir.resolve(EXISTING_KEY));
+ Path fileToDelete = Files.createFile(websessionDir.resolve(EXISTING_KEY));
assertThat(Files.exists(fileToDelete)).isTrue();
- flatFileWebSessionCache.invalidate(EXISTING_KEY);
+ cache.invalidate(EXISTING_KEY);
assertThat(Files.exists(fileToDelete)).isFalse();
}
@Test
public void invalidateTestObjectNotString() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
- assertThat(flatFileWebSessionCache.size()).isEqualTo(1);
- flatFileWebSessionCache.invalidate(new Object());
- assertThat(flatFileWebSessionCache.size()).isEqualTo(1);
+ assertThat(cache.size()).isEqualTo(1);
+ cache.invalidate(new Object());
+ assertThat(cache.size()).isEqualTo(1);
}
@Test
public void putTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
- Val val = flatFileWebSessionCache.getIfPresent(EXISTING_KEY);
- flatFileWebSessionCache.put(NEW_KEY, val);
- assertThat(flatFileWebSessionCache.getIfPresent(NEW_KEY)).isNotNull();
+ Val val = cache.getIfPresent(EXISTING_KEY);
+ cache.put(NEW_KEY, val);
+ assertThat(cache.getIfPresent(NEW_KEY)).isNotNull();
}
@Test
public void putAllTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
- Val val = flatFileWebSessionCache.getIfPresent(EXISTING_KEY);
+ Val val = cache.getIfPresent(EXISTING_KEY);
Map<String, Val> sessions = ImmutableMap.of(NEW_KEY, val);
- flatFileWebSessionCache.putAll(sessions);
- assertThat(flatFileWebSessionCache.asMap()).containsKey(NEW_KEY);
+ cache.putAll(sessions);
+ assertThat(cache.asMap()).containsKey(NEW_KEY);
}
@Test
public void putWithErrorsTest() throws Exception {
loadKeyToCacheDir(EXISTING_KEY);
- Val val = flatFileWebSessionCache.getIfPresent(EXISTING_KEY);
+ Val val = cache.getIfPresent(EXISTING_KEY);
tempFolder.delete();
- flatFileWebSessionCache.put(NEW_KEY, val);
- assertThat(flatFileWebSessionCache.getIfPresent(NEW_KEY)).isNull();
+ cache.put(NEW_KEY, val);
+ assertThat(cache.getIfPresent(NEW_KEY)).isNull();
}
@Test
public void sizeTest() throws Exception {
int numberOfKeys = 10;
loadKeysToCacheDir(numberOfKeys);
- assertThat(flatFileWebSessionCache.size()).isEqualTo(numberOfKeys);
+ assertThat(cache.size()).isEqualTo(numberOfKeys);
}
@Test
public void statTest() throws Exception {
- assertThat(flatFileWebSessionCache.stats()).isNull();
+ assertThat(cache.stats()).isNull();
}
private List<String> loadKeysToCacheDir(int number) throws IOException {
List<String> keys = new ArrayList<>();
for (int i = 0; i < number; i++) {
- Path tmp = Files.createTempFile(dir, "cache", null);
+ Path tmp = Files.createTempFile(websessionDir, "cache", null);
keys.add(tmp.getFileName().toString());
}
return keys;
@@ -266,10 +266,10 @@
private Path loadKeyToCacheDir(String key) throws IOException {
if (key.equals(EMPTY_KEY)) {
- return Files.createFile(dir.resolve(EMPTY_KEY));
+ return Files.createFile(websessionDir.resolve(EMPTY_KEY));
}
try (InputStream in = loadFile(key)) {
- Path target = dir.resolve(key);
+ Path target = websessionDir.resolve(key);
Files.copy(in, target, StandardCopyOption.REPLACE_EXISTING);
return target;
}
