Extract the ser/des of HTTP payloads to utility class
This change refactors the Java class PullReplicationFilter and
it separates the serialisation and deserialisation processes
into a new Java class called PayloadSerDes. This refactoring not
only enables future developments to reuse the same code but also
enhances code clarity and maintains adherence to the Single
Responsibility Principle.
Bug: Issue 296854545
Change-Id: Ifeaf027163c1b8098eee601ce04ac764cf776957
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
index 5a4393c..d0e5c10 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
@@ -18,10 +18,8 @@
import static com.googlesource.gerrit.plugins.replication.pull.api.HttpServletOps.checkAcceptHeader;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
-import static javax.servlet.http.HttpServletResponse.SC_CREATED;
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import static javax.servlet.http.HttpServletResponse.SC_OK;
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
import com.google.common.flogger.FluentLogger;
@@ -38,28 +36,22 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.restapi.RestApiServlet;
-import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gson.Gson;
import com.google.gson.JsonParseException;
-import com.google.gson.stream.JsonReader;
import com.google.gson.stream.MalformedJsonException;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.google.inject.TypeLiteral;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.Input;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionsInput;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
-import java.io.BufferedReader;
-import java.io.EOFException;
+import com.googlesource.gerrit.plugins.replication.pull.api.util.PayloadSerDes;
import java.io.IOException;
-import java.io.PrintWriter;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Optional;
@@ -86,7 +78,6 @@
private UpdateHeadAction updateHEADAction;
private ProjectDeletionAction projectDeletionAction;
private ProjectCache projectCache;
- private Gson gson;
private String pluginName;
private final Provider<CurrentUser> currentUserProvider;
@@ -109,7 +100,6 @@
this.projectDeletionAction = projectDeletionAction;
this.projectCache = projectCache;
this.pluginName = pluginName;
- this.gson = OutputFormat.JSON.newGsonBuilder().create();
this.currentUserProvider = currentUserProvider;
}
@@ -126,13 +116,13 @@
try {
if (isFetchAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
- writeResponse(httpResponse, doFetch(httpRequest));
+ PayloadSerDes.writeResponse(httpResponse, doFetch(httpRequest));
} else if (isApplyObjectAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
- writeResponse(httpResponse, doApplyObject(httpRequest));
+ PayloadSerDes.writeResponse(httpResponse, doApplyObject(httpRequest));
} else if (isApplyObjectsAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
- writeResponse(httpResponse, doApplyObjects(httpRequest));
+ PayloadSerDes.writeResponse(httpResponse, doApplyObjects(httpRequest));
} else if (isInitProjectAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
if (!checkAcceptHeader(httpRequest, httpResponse)) {
@@ -141,10 +131,10 @@
doInitProject(httpRequest, httpResponse);
} else if (isUpdateHEADAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
- writeResponse(httpResponse, doUpdateHEAD(httpRequest));
+ PayloadSerDes.writeResponse(httpResponse, doUpdateHEAD(httpRequest));
} else if (isDeleteProjectAction(httpRequest)) {
failIfcurrentUserIsAnonymous();
- writeResponse(httpResponse, doDeleteProject(httpRequest));
+ PayloadSerDes.writeResponse(httpResponse, doDeleteProject(httpRequest));
} else {
chain.doFilter(request, response);
}
@@ -199,7 +189,7 @@
@SuppressWarnings("unchecked")
private Response<String> doApplyObject(HttpServletRequest httpRequest)
throws RestApiException, IOException, PermissionBackendException {
- RevisionInput input = readJson(httpRequest, TypeLiteral.get(RevisionInput.class));
+ RevisionInput input = PayloadSerDes.parseRevisionInput(httpRequest);
IdString id = getProjectName(httpRequest).get();
return (Response<String>) applyObjectAction.apply(parseProjectResource(id), input);
@@ -208,7 +198,7 @@
@SuppressWarnings("unchecked")
private Response<String> doApplyObjects(HttpServletRequest httpRequest)
throws RestApiException, IOException, PermissionBackendException {
- RevisionsInput input = readJson(httpRequest, TypeLiteral.get(RevisionsInput.class));
+ RevisionsInput input = PayloadSerDes.parseRevisionsInput(httpRequest);
IdString id = getProjectName(httpRequest).get();
return (Response<String>) applyObjectsAction.apply(parseProjectResource(id), input);
@@ -216,7 +206,7 @@
@SuppressWarnings("unchecked")
private Response<String> doUpdateHEAD(HttpServletRequest httpRequest) throws Exception {
- HeadInput input = readJson(httpRequest, TypeLiteral.get(HeadInput.class));
+ HeadInput input = PayloadSerDes.parseHeadInput(httpRequest);
IdString id = getProjectName(httpRequest).get();
return (Response<String>) updateHEADAction.apply(parseProjectResource(id), input);
@@ -233,7 +223,7 @@
@SuppressWarnings("unchecked")
private Response<Map<String, Object>> doFetch(HttpServletRequest httpRequest)
throws IOException, RestApiException, PermissionBackendException {
- Input input = readJson(httpRequest, TypeLiteral.get(Input.class));
+ Input input = PayloadSerDes.parseInput(httpRequest);
IdString id = getProjectName(httpRequest).get();
return (Response<Map<String, Object>>) fetchAction.apply(parseProjectResource(id), input);
@@ -247,50 +237,6 @@
return new ProjectResource(project.get(), currentUserProvider.get());
}
- private <T> void writeResponse(HttpServletResponse httpResponse, Response<T> response)
- throws IOException {
- String responseJson = gson.toJson(response);
- if (response.statusCode() == SC_OK || response.statusCode() == SC_CREATED) {
-
- httpResponse.setContentType("application/json");
- httpResponse.setStatus(response.statusCode());
- PrintWriter writer = httpResponse.getWriter();
- writer.print(new String(RestApiServlet.JSON_MAGIC));
- writer.print(responseJson);
- } else {
- httpResponse.sendError(response.statusCode(), responseJson);
- }
- }
-
- private <T> T readJson(HttpServletRequest httpRequest, TypeLiteral<T> typeLiteral)
- throws IOException, BadRequestException {
-
- try (BufferedReader br = httpRequest.getReader();
- JsonReader json = new JsonReader(br)) {
- try {
- json.setLenient(true);
-
- try {
- json.peek();
- } catch (EOFException e) {
- throw new BadRequestException("Expected JSON object", e);
- }
-
- return gson.fromJson(json, typeLiteral.getType());
- } finally {
- try {
- // Reader.close won't consume the rest of the input. Explicitly consume the request
- // body.
- br.skip(Long.MAX_VALUE);
- } catch (Exception e) {
- // ignore, e.g. trying to consume the rest of the input may fail if the request was
- // cancelled
- logger.atFine().withCause(e).log("Exception during the parsing of the request json");
- }
- }
- }
- }
-
/**
* Return project name from request URI. Request URI format:
* /a/projects/<project_name>/pull-replication~apply-object
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/util/PayloadSerDes.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/util/PayloadSerDes.java
new file mode 100644
index 0000000..414af37
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/util/PayloadSerDes.java
@@ -0,0 +1,106 @@
+// Copyright (C) 2023 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.replication.pull.api.util;
+
+import static javax.servlet.http.HttpServletResponse.SC_CREATED;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.api.projects.HeadInput;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.httpd.restapi.RestApiServlet;
+import com.google.gerrit.json.OutputFormat;
+import com.google.gson.Gson;
+import com.google.gson.stream.JsonReader;
+import com.google.inject.TypeLiteral;
+import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction;
+import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
+import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionsInput;
+import java.io.BufferedReader;
+import java.io.EOFException;
+import java.io.IOException;
+import java.io.PrintWriter;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public class PayloadSerDes {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final Gson gson = OutputFormat.JSON.newGsonBuilder().create();
+
+ public static RevisionInput parseRevisionInput(HttpServletRequest httpRequest)
+ throws BadRequestException, IOException {
+ return parse(httpRequest, TypeLiteral.get(RevisionInput.class));
+ }
+
+ public static RevisionsInput parseRevisionsInput(HttpServletRequest httpRequest)
+ throws BadRequestException, IOException {
+ return parse(httpRequest, TypeLiteral.get(RevisionsInput.class));
+ }
+
+ public static HeadInput parseHeadInput(HttpServletRequest httpRequest)
+ throws BadRequestException, IOException {
+ return parse(httpRequest, TypeLiteral.get(HeadInput.class));
+ }
+
+ public static FetchAction.Input parseInput(HttpServletRequest httpRequest)
+ throws BadRequestException, IOException {
+ return parse(httpRequest, TypeLiteral.get(FetchAction.Input.class));
+ }
+
+ public static <T> void writeResponse(HttpServletResponse httpResponse, Response<T> response)
+ throws IOException {
+ String responseJson = gson.toJson(response);
+ if (response.statusCode() == SC_OK || response.statusCode() == SC_CREATED) {
+
+ httpResponse.setContentType("application/json");
+ httpResponse.setStatus(response.statusCode());
+ PrintWriter writer = httpResponse.getWriter();
+ writer.print(new String(RestApiServlet.JSON_MAGIC));
+ writer.print(responseJson);
+ } else {
+ httpResponse.sendError(response.statusCode(), responseJson);
+ }
+ }
+
+ private static <T> T parse(HttpServletRequest httpRequest, TypeLiteral<T> typeLiteral)
+ throws IOException, BadRequestException {
+
+ try (BufferedReader br = httpRequest.getReader();
+ JsonReader json = new JsonReader(br)) {
+ try {
+ json.setLenient(true);
+
+ try {
+ json.peek();
+ } catch (EOFException e) {
+ throw new BadRequestException("Expected JSON object", e);
+ }
+
+ return gson.fromJson(json, typeLiteral.getType());
+ } finally {
+ try {
+ // Reader.close won't consume the rest of the input. Explicitly consume the request
+ // body.
+ br.skip(Long.MAX_VALUE);
+ } catch (Exception e) {
+ // ignore, e.g. trying to consume the rest of the input may fail if the request was
+ // cancelled
+ logger.atFine().withCause(e).log("Exception during the parsing of the request json");
+ }
+ }
+ }
+ }
+}
