Refactor json handling for simplicity and maintainability
Move creation of the Gson serializer into a provider so that it
is only created in one place. This will make it easier to adjust
the parameters.
Use the Gson provided by this provider to serialize the objects
instead of the custom implementation in GsonParser.
Make GsonParser a Singleton that also uses the Gson provider to
deserialize the objects. Remove the redundant serialization code.
Change-Id: I185a2c13adf519ae4437afd7a3adf1c5d38f4cfe
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 033aaa7..fb49cf2 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
@@ -34,10 +34,13 @@
private static final long serialVersionUID = -1L;
private final ForwardedCacheEvictionHandler forwardedCacheEvictionHandler;
+ private final GsonParser gson;
@Inject
- CacheRestApiServlet(ForwardedCacheEvictionHandler forwardedCacheEvictionHandler) {
+ CacheRestApiServlet(
+ ForwardedCacheEvictionHandler forwardedCacheEvictionHandler, GsonParser gson) {
this.forwardedCacheEvictionHandler = forwardedCacheEvictionHandler;
+ this.gson = gson;
}
@Override
@@ -48,7 +51,7 @@
String cacheName = params.get(CACHENAME_INDEX);
String json = req.getReader().readLine();
forwardedCacheEvictionHandler.evict(
- CacheEntry.from(cacheName, GsonParser.fromJson(cacheName, json)));
+ CacheEntry.from(cacheName, gson.fromJson(cacheName, json)));
rsp.setStatus(SC_NO_CONTENT);
} catch (CacheNotFoundException e) {
log.error("Failed to process eviction request: {}", e.getMessage());
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java
index 2636df4..3da5081 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServlet.java
@@ -21,15 +21,10 @@
import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE;
import com.ericsson.gerrit.plugins.highavailability.forwarder.ForwardedEventHandler;
-import com.google.common.base.Supplier;
import com.google.common.io.CharStreams;
import com.google.common.net.MediaType;
import com.google.gerrit.server.events.Event;
-import com.google.gerrit.server.events.EventDeserializer;
-import com.google.gerrit.server.events.SupplierDeserializer;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -42,10 +37,12 @@
private static final long serialVersionUID = -1L;
private final ForwardedEventHandler forwardedEventHandler;
+ private final GsonProvider gson;
@Inject
- EventRestApiServlet(ForwardedEventHandler forwardedEventHandler) {
+ EventRestApiServlet(ForwardedEventHandler forwardedEventHandler, GsonProvider gson) {
this.forwardedEventHandler = forwardedEventHandler;
+ this.gson = gson;
}
@Override
@@ -67,13 +64,8 @@
}
}
- private static Event getEventFromRequest(HttpServletRequest req) throws IOException {
+ private Event getEventFromRequest(HttpServletRequest req) throws IOException {
String jsonEvent = CharStreams.toString(req.getReader());
- Gson gson =
- new GsonBuilder()
- .registerTypeAdapter(Event.class, new EventDeserializer())
- .registerTypeAdapter(Supplier.class, new SupplierDeserializer())
- .create();
- return gson.fromJson(jsonEvent, Event.class);
+ return gson.get().fromJson(jsonEvent, Event.class);
}
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
index 974f20b..8db2488 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParser.java
@@ -19,14 +19,19 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
-final class GsonParser {
+@Singleton
+class GsonParser {
+ private final Gson gson;
- private GsonParser() {}
+ @Inject
+ GsonParser(GsonProvider gson) {
+ this.gson = gson.get();
+ }
- static Object fromJson(String cacheName, String json) {
- Gson gson = new GsonBuilder().create();
+ public Object fromJson(String cacheName, String json) {
Object key;
// Need to add a case for 'adv_bases'
switch (cacheName) {
@@ -52,26 +57,4 @@
}
return key;
}
-
- static String toJson(String cacheName, Object key) {
- Gson gson = new GsonBuilder().create();
- String json;
- // Need to add a case for 'adv_bases'
- switch (cacheName) {
- case Constants.ACCOUNTS:
- json = gson.toJson(key, Account.Id.class);
- break;
- case Constants.GROUPS:
- json = gson.toJson(key, AccountGroup.Id.class);
- break;
- case Constants.GROUPS_BYINCLUDE:
- case Constants.GROUPS_MEMBERS:
- json = gson.toJson(key, AccountGroup.UUID.class);
- break;
- case Constants.PROJECT_LIST:
- default:
- json = gson.toJson(key);
- }
- return json;
- }
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java
new file mode 100644
index 0000000..e62cec2
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonProvider.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 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.rest;
+
+import com.google.common.base.Supplier;
+import com.google.gerrit.server.events.Event;
+import com.google.gerrit.server.events.EventDeserializer;
+import com.google.gerrit.server.events.SupplierDeserializer;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.inject.Provider;
+
+public class GsonProvider implements Provider<Gson> {
+
+ @Override
+ public Gson get() {
+ return new GsonBuilder()
+ .registerTypeAdapter(Event.class, new EventDeserializer())
+ .registerTypeAdapter(Supplier.class, new SupplierDeserializer())
+ .create();
+ }
+}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java
index dc59117..ef1b26d 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarder.java
@@ -49,17 +49,20 @@
private final String pluginRelativePath;
private final Configuration cfg;
private final Provider<Set<PeerInfo>> peerInfoProvider;
+ private final GsonProvider gson;
@Inject
RestForwarder(
HttpSession httpClient,
@PluginName String pluginName,
Configuration cfg,
- Provider<Set<PeerInfo>> peerInfoProvider) {
+ Provider<Set<PeerInfo>> peerInfoProvider,
+ GsonProvider gson) {
this.httpSession = httpClient;
this.pluginRelativePath = Joiner.on("/").join("plugins", pluginName);
this.cfg = cfg;
this.peerInfoProvider = peerInfoProvider;
+ this.gson = gson;
}
@Override
@@ -104,7 +107,7 @@
@Override
public boolean evict(final String cacheName, final Object key) {
- String json = GsonParser.toJson(cacheName, key);
+ String json = gson.get().toJson(key);
return execute(RequestMethod.POST, "invalidate cache " + cacheName, "cache", cacheName, json);
}
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 29f63fd..8b08d24 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
@@ -44,7 +44,9 @@
@Before
public void setUp() {
- servlet = new CacheRestApiServlet(forwardedCacheEvictionHandlerMock);
+ servlet =
+ new CacheRestApiServlet(
+ forwardedCacheEvictionHandlerMock, new GsonParser(new GsonProvider()));
}
@Test
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java
index e8747b7..835977f 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/EventRestApiServletTest.java
@@ -58,7 +58,7 @@
@Before
public void createEventsRestApiServlet() throws Exception {
- eventRestApiServlet = new EventRestApiServlet(forwardedEventHandlerMock);
+ eventRestApiServlet = new EventRestApiServlet(forwardedEventHandlerMock, new GsonProvider());
when(requestMock.getContentType()).thenReturn(MediaType.JSON_UTF_8.toString());
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
index b4f4dac..2f6ce7e 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/GsonParserTest.java
@@ -19,43 +19,47 @@
import com.ericsson.gerrit.plugins.highavailability.cache.Constants;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gson.Gson;
import org.junit.Test;
public class GsonParserTest {
private static final Object EMPTY_JSON = "{}";
+ private final Gson gson = new Gson();
+ private final GsonParser objectUnderTest = new GsonParser(new GsonProvider());
@Test
public void accountIDParse() {
Account.Id accountId = new Account.Id(1);
- String json = GsonParser.toJson(Constants.ACCOUNTS, accountId);
- assertThat(accountId).isEqualTo(GsonParser.fromJson(Constants.ACCOUNTS, json));
+ String json = gson.toJson(accountId);
+ assertThat(accountId).isEqualTo(objectUnderTest.fromJson(Constants.ACCOUNTS, json));
}
@Test
public void accountGroupIDParse() {
AccountGroup.Id accountGroupId = new AccountGroup.Id(1);
- String json = GsonParser.toJson(Constants.GROUPS, accountGroupId);
- assertThat(accountGroupId).isEqualTo(GsonParser.fromJson(Constants.GROUPS, json));
+ String json = gson.toJson(accountGroupId);
+ assertThat(accountGroupId).isEqualTo(objectUnderTest.fromJson(Constants.GROUPS, json));
}
@Test
public void accountGroupUUIDParse() {
AccountGroup.UUID accountGroupUuid = new AccountGroup.UUID("abc123");
- String json = GsonParser.toJson(Constants.GROUPS_BYINCLUDE, accountGroupUuid);
- assertThat(accountGroupUuid).isEqualTo(GsonParser.fromJson(Constants.GROUPS_BYINCLUDE, json));
+ String json = gson.toJson(accountGroupUuid);
+ assertThat(accountGroupUuid)
+ .isEqualTo(objectUnderTest.fromJson(Constants.GROUPS_BYINCLUDE, json));
}
@Test
public void stringParse() {
String key = "key";
- String json = GsonParser.toJson(Constants.PROJECTS, key);
- assertThat(key).isEqualTo(GsonParser.fromJson(Constants.PROJECTS, json));
+ String json = gson.toJson(key);
+ assertThat(key).isEqualTo(objectUnderTest.fromJson(Constants.PROJECTS, json));
}
@Test
public void noKeyParse() {
Object object = new Object();
- String json = GsonParser.toJson(Constants.PROJECT_LIST, object);
+ String json = gson.toJson(object);
assertThat(json).isEqualTo(EMPTY_JSON);
}
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java
index d516d09..55a3c2a 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/RestForwarderTest.java
@@ -33,7 +33,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.events.Event;
-import com.google.gson.GsonBuilder;
+import com.google.gson.Gson;
import com.google.inject.Provider;
import java.io.IOException;
import java.util.Set;
@@ -75,6 +75,9 @@
private static final String INDEX_GROUP_ENDPOINT =
Joiner.on("/").join(URL, PLUGINS, PLUGIN_NAME, "index/group", UUID);
+ private GsonProvider gsonProvider = new GsonProvider();
+ private Gson gson = gsonProvider.get();
+
// Event
private static Event event = new TestEvent();
private static final String EVENT_ENDPOINT =
@@ -94,7 +97,11 @@
when(peersMock.get()).thenReturn(ImmutableSet.of(new PeerInfo(URL)));
forwarder =
new RestForwarder(
- httpSessionMock, PLUGIN_NAME, configMock, peersMock); // TODO: Create provider
+ httpSessionMock,
+ PLUGIN_NAME,
+ configMock,
+ peersMock,
+ gsonProvider); // TODO: Create provider
}
@Test
@@ -199,7 +206,7 @@
@Test
public void testEvictProjectOK() throws Exception {
String key = PROJECT_NAME;
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
when(httpSessionMock.post(buildCacheEndpoint(Constants.PROJECTS), keyJson))
.thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG));
assertThat(forwarder.evict(Constants.PROJECTS, key)).isTrue();
@@ -208,7 +215,7 @@
@Test
public void testEvictAccountsOK() throws Exception {
Account.Id key = new Account.Id(123);
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
when(httpSessionMock.post(buildCacheEndpoint(Constants.ACCOUNTS), keyJson))
.thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG));
assertThat(forwarder.evict(Constants.ACCOUNTS, key)).isTrue();
@@ -217,7 +224,7 @@
@Test
public void testEvictGroupsOK() throws Exception {
AccountGroup.Id key = new AccountGroup.Id(123);
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
String endpoint = buildCacheEndpoint(Constants.GROUPS);
when(httpSessionMock.post(endpoint, keyJson)).thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG));
assertThat(forwarder.evict(Constants.GROUPS, key)).isTrue();
@@ -226,7 +233,7 @@
@Test
public void testEvictGroupsByIncludeOK() throws Exception {
AccountGroup.UUID key = new AccountGroup.UUID("90b3042d9094a37985f3f9281391dbbe9a5addad");
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
when(httpSessionMock.post(buildCacheEndpoint(Constants.GROUPS_BYINCLUDE), keyJson))
.thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG));
assertThat(forwarder.evict(Constants.GROUPS_BYINCLUDE, key)).isTrue();
@@ -235,7 +242,7 @@
@Test
public void testEvictGroupsMembersOK() throws Exception {
AccountGroup.UUID key = new AccountGroup.UUID("90b3042d9094a37985f3f9281391dbbe9a5addad");
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
when(httpSessionMock.post(buildCacheEndpoint(Constants.GROUPS_MEMBERS), keyJson))
.thenReturn(new HttpResult(SUCCESSFUL, EMPTY_MSG));
assertThat(forwarder.evict(Constants.GROUPS_MEMBERS, key)).isTrue();
@@ -244,7 +251,7 @@
@Test
public void testEvictCacheFailed() throws Exception {
String key = PROJECT_NAME;
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
when(httpSessionMock.post(buildCacheEndpoint(Constants.PROJECTS), keyJson))
.thenReturn(new HttpResult(FAILED, EMPTY_MSG));
assertThat(forwarder.evict(Constants.PROJECTS, key)).isFalse();
@@ -253,7 +260,7 @@
@Test
public void testEvictCacheThrowsException() throws Exception {
String key = PROJECT_NAME;
- String keyJson = new GsonBuilder().create().toJson(key);
+ String keyJson = gson.toJson(key);
doThrow(new IOException())
.when(httpSessionMock)
.post(buildCacheEndpoint(Constants.PROJECTS), keyJson);
