Refactor EvictCache.evict method to accept only one argument
This change is required to be able to refactor and remove duplication
between the classes handling forwarded tasks (EvictCache and
DispatchEvent). Having only one argument will allow to create a generic
base class where common things will be done, such as setting/resetting
the context.
Change-Id: I9c303e02bfcb4da67f336e09332f71596188967e
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntry.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntry.java
new file mode 100644
index 0000000..ed9898f
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntry.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.forwarder;
+
+import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
+
+/** Represents a cache entry to evict */
+public class CacheEntry {
+ private final String pluginName;
+ private final String cacheName;
+ private final Object key;
+
+ /**
+ * Cache entry
+ *
+ * @param pluginName the plugin name to which the cache belongs, or "gerrit" for a Gerrit core
+ * cache
+ * @param cacheName the name of the cache to evict the entry from
+ * @param key the key identifying the entry in the cache
+ */
+ public CacheEntry(String pluginName, String cacheName, Object key) {
+ this.pluginName = pluginName;
+ this.cacheName = cacheName;
+ this.key = key;
+ }
+
+ public String getPluginName() {
+ return pluginName;
+ }
+
+ public String getCacheName() {
+ return cacheName;
+ }
+
+ public Object getKey() {
+ return key;
+ }
+
+ /**
+ * Build a CacheEntry from the specified cache and key
+ *
+ * @param cache String representing the cache, e.g. my_plugin.my_cache
+ * @param key The Object representing the key
+ * @return the CacheEntry
+ */
+ public static CacheEntry from(String cache, Object key) {
+ int dot = cache.indexOf('.');
+ if (dot > 0) {
+ return new CacheEntry(cache.substring(0, dot), cache.substring(dot + 1), key);
+ }
+ return new CacheEntry(Constants.GERRIT, cache, key);
+ }
+}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCache.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCache.java
index c0ffd23..54b4307 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCache.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCache.java
@@ -42,26 +42,23 @@
* Evict an entry from the cache of the local node, eviction will not be forwarded to the other
* node.
*
- * @param pluginName the plugin name to which the cache belongs, or "gerrit" for a Gerrit core
- * cache
- * @param cacheName the name of the cache to evict the entry from
- * @param key the key identifying the entry to evict
+ * @param cacheEntry the entry to evict
* @throws CacheNotFoundException if cache does not exist
*/
- public void evict(String pluginName, String cacheName, Object key) throws CacheNotFoundException {
- Cache<?, ?> cache = cacheMap.get(pluginName, cacheName);
+ public void evict(CacheEntry entry) throws CacheNotFoundException {
+ Cache<?, ?> cache = cacheMap.get(entry.getPluginName(), entry.getCacheName());
if (cache == null) {
- throw new CacheNotFoundException(pluginName, cacheName);
+ throw new CacheNotFoundException(entry.getPluginName(), entry.getCacheName());
}
try {
Context.setForwardedEvent(true);
- if (Constants.PROJECT_LIST.equals(cacheName)) {
+ if (Constants.PROJECT_LIST.equals(entry.getCacheName())) {
// One key is holding the list of projects
cache.invalidateAll();
- logger.debug("Invalidated cache {}", cacheName);
+ logger.debug("Invalidated cache {}", entry.getCacheName());
} else {
- cache.invalidate(key);
- logger.debug("Invalidated cache {}[{}]", cacheName, key);
+ cache.invalidate(entry.getKey());
+ logger.debug("Invalidated cache {}[{}]", entry.getCacheName(), entry.getKey());
}
} finally {
Context.unsetForwardedEvent();
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
index 3d83de7..9392392 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServlet.java
@@ -17,10 +17,9 @@
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
-import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
+import com.ericsson.gerrit.plugins.highavailability.forwarder.CacheEntry;
import com.ericsson.gerrit.plugins.highavailability.forwarder.CacheNotFoundException;
import com.ericsson.gerrit.plugins.highavailability.forwarder.EvictCache;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -53,9 +52,7 @@
List<String> params = Splitter.on('/').splitToList(req.getPathInfo());
String cacheName = params.get(CACHENAME_INDEX);
String json = req.getReader().readLine();
- Object key = GsonParser.fromJson(cacheName, json);
- CacheParameters cacheKey = getCacheParameters(cacheName);
- evictCache.evict(cacheKey.pluginName, cacheKey.cacheName, key);
+ evictCache.evict(CacheEntry.from(cacheName, GsonParser.fromJson(cacheName, json)));
rsp.setStatus(SC_NO_CONTENT);
} catch (CacheNotFoundException e) {
logger.error("Failed to process eviction request: {}", e.getMessage());
@@ -66,26 +63,6 @@
}
}
- @VisibleForTesting
- public static class CacheParameters {
- public final String pluginName;
- public final String cacheName;
-
- public CacheParameters(String pluginName, String cacheName) {
- this.pluginName = pluginName;
- this.cacheName = cacheName;
- }
- }
-
- @VisibleForTesting
- public static CacheParameters getCacheParameters(String cache) {
- int dot = cache.indexOf('.');
- if (dot > 0) {
- return new CacheParameters(cache.substring(0, dot), cache.substring(dot + 1));
- }
- return new CacheParameters(Constants.GERRIT, cache);
- }
-
private static void sendError(HttpServletResponse rsp, int statusCode, String message) {
try {
rsp.sendError(statusCode, message);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntryTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntryTest.java
new file mode 100644
index 0000000..5cdcf5a
--- /dev/null
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/CacheEntryTest.java
@@ -0,0 +1,36 @@
+// Copyright (C) 2018 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.ericsson.gerrit.plugins.highavailability.forwarder;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
+import org.junit.Test;
+
+public class CacheEntryTest {
+
+ @Test
+ public void cacheEntry() throws Exception {
+ CacheEntry entry = CacheEntry.from("accounts_by_name", "someKey");
+ assertThat(entry.getPluginName()).isEqualTo(Constants.GERRIT);
+ assertThat(entry.getCacheName()).isEqualTo("accounts_by_name");
+ assertThat(entry.getKey()).isEqualTo("someKey");
+
+ entry = CacheEntry.from("my_plugin.my_cache", "someOtherKey");
+ assertThat(entry.getPluginName()).isEqualTo("my_plugin");
+ assertThat(entry.getCacheName()).isEqualTo("my_cache");
+ assertThat(entry.getKey()).isEqualTo("someOtherKey");
+ }
+}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCacheTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCacheTest.java
index bab6473..46e7132 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCacheTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/EvictCacheTest.java
@@ -47,35 +47,36 @@
@Test
public void shouldThrowAnExceptionWhenCacheNotFound() throws Exception {
- String pluginName = "somePlugin";
- String cacheName = "unexistingCache";
+ CacheEntry entry = new CacheEntry("somePlugin", "unexistingCache", null);
exception.expect(CacheNotFoundException.class);
- exception.expectMessage(String.format("cache %s.%s not found", pluginName, cacheName));
- evictCache.evict(pluginName, cacheName, null);
+ exception.expectMessage(
+ String.format("cache %s.%s not found", entry.getPluginName(), entry.getCacheName()));
+ evictCache.evict(entry);
}
@Test
public void testSuccessfulCacheEviction() throws Exception {
- Account.Id key = new Account.Id(123);
- doReturn(cacheMock).when(cacheMapMock).get(Constants.GERRIT, Constants.ACCOUNTS);
+ CacheEntry entry = new CacheEntry(Constants.GERRIT, Constants.ACCOUNTS, new Account.Id(123));
+ doReturn(cacheMock).when(cacheMapMock).get(entry.getPluginName(), entry.getCacheName());
- evictCache.evict(Constants.GERRIT, Constants.ACCOUNTS, key);
- verify(cacheMock).invalidate(key);
+ evictCache.evict(entry);
+ verify(cacheMock).invalidate(entry.getKey());
}
@Test
public void testSuccessfulProjectListCacheEviction() throws Exception {
- doReturn(cacheMock).when(cacheMapMock).get(Constants.GERRIT, Constants.PROJECT_LIST);
+ CacheEntry entry = new CacheEntry(Constants.GERRIT, Constants.PROJECT_LIST, null);
+ doReturn(cacheMock).when(cacheMapMock).get(entry.getPluginName(), entry.getCacheName());
- evictCache.evict(Constants.GERRIT, Constants.PROJECT_LIST, null);
+ evictCache.evict(entry);
verify(cacheMock).invalidateAll();
}
@Test
public void shouldSetAndUnsetForwardedContext() throws Exception {
- Account.Id key = new Account.Id(456);
- doReturn(cacheMock).when(cacheMapMock).get(Constants.GERRIT, Constants.ACCOUNTS);
+ CacheEntry entry = new CacheEntry(Constants.GERRIT, Constants.ACCOUNTS, new Account.Id(456));
+ doReturn(cacheMock).when(cacheMapMock).get(entry.getPluginName(), entry.getCacheName());
//this doAnswer is to allow to assert that context is set to forwarded
//while cache eviction is called.
@@ -86,19 +87,19 @@
return null;
})
.when(cacheMock)
- .invalidate(key);
+ .invalidate(entry.getKey());
assertThat(Context.isForwardedEvent()).isFalse();
- evictCache.evict(Constants.GERRIT, Constants.ACCOUNTS, key);
+ evictCache.evict(entry);
assertThat(Context.isForwardedEvent()).isFalse();
- verify(cacheMock).invalidate(key);
+ verify(cacheMock).invalidate(entry.getKey());
}
@Test
public void shouldSetAndUnsetForwardedContextEvenIfExceptionIsThrown() throws Exception {
- Account.Id key = new Account.Id(789);
- doReturn(cacheMock).when(cacheMapMock).get(Constants.GERRIT, Constants.ACCOUNTS);
+ CacheEntry entry = new CacheEntry(Constants.GERRIT, Constants.ACCOUNTS, new Account.Id(789));
+ doReturn(cacheMock).when(cacheMapMock).get(entry.getPluginName(), entry.getCacheName());
doAnswer(
(Answer<Void>)
@@ -107,15 +108,15 @@
throw new RuntimeException();
})
.when(cacheMock)
- .invalidate(key);
+ .invalidate(entry.getKey());
assertThat(Context.isForwardedEvent()).isFalse();
try {
- evictCache.evict(Constants.GERRIT, Constants.ACCOUNTS, key);
+ evictCache.evict(entry);
} catch (RuntimeException e) {
}
assertThat(Context.isForwardedEvent()).isFalse();
- verify(cacheMock).invalidate(key);
+ verify(cacheMock).invalidate(entry.getKey());
}
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
index 782cb81..52ba116 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/CacheRestApiServletTest.java
@@ -14,11 +14,9 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder.rest;
-import static com.google.common.truth.Truth.assertThat;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -26,7 +24,6 @@
import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
import com.ericsson.gerrit.plugins.highavailability.forwarder.CacheNotFoundException;
import com.ericsson.gerrit.plugins.highavailability.forwarder.EvictCache;
-import com.ericsson.gerrit.plugins.highavailability.forwarder.rest.CacheRestApiServlet.CacheParameters;
import java.io.BufferedReader;
import java.io.IOException;
import javax.servlet.http.HttpServletRequest;
@@ -107,7 +104,7 @@
String cacheName = "nonexistingCache";
configureMocksFor(pluginName, cacheName);
CacheNotFoundException e = new CacheNotFoundException(pluginName, cacheName);
- doThrow(e).when(evictCacheMock).evict(eq(pluginName), eq(cacheName), any());
+ doThrow(e).when(evictCacheMock).evict(any());
servlet.doPost(requestMock, responseMock);
verify(responseMock).sendError(SC_BAD_REQUEST, e.getMessage());
}
@@ -121,17 +118,6 @@
verify(responseMock).sendError(SC_BAD_REQUEST, errorMessage);
}
- @Test
- public void cacheParameters() throws Exception {
- CacheParameters key = CacheRestApiServlet.getCacheParameters("accounts_by_name");
- assertThat(key.pluginName).isEqualTo(Constants.GERRIT);
- assertThat(key.cacheName).isEqualTo("accounts_by_name");
-
- key = CacheRestApiServlet.getCacheParameters("my_plugin.my_cache");
- assertThat(key.pluginName).isEqualTo("my_plugin");
- assertThat(key.cacheName).isEqualTo("my_cache");
- }
-
private void verifyResponseIsOK() throws Exception {
servlet.doPost(requestMock, responseMock);
verify(responseMock).setStatus(SC_NO_CONTENT);
