From 8ae8584c30dd2a0be4d01e0c741afc32f443f62b Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 4 Dec 2020 10:16:30 +0000 Subject: Compact the REST-API output JSON unconditionally The output JSON was initially compacted only when the Accept header was set to 'application/json', from the very initial Change-Id: I2014a2209. The ability to get a pretty-printed JSON is still available using the 'pp=1' query string, therefore it is best to always compress the JSON output by default. Bug: Issue 13781 Change-Id: I2f1cb91e95e10b27b328764834d58d0799e01ed4 (cherry picked from commit 9c09efbc48038c3b03d5f388f95c845ae8b409c1) --- .../gerrit/httpd/restapi/RestApiServlet.java | 23 ++----- .../gerrit/acceptance/rest/RestApiServletIT.java | 76 ++++++++++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 88c5106e67..a7c0e300fd 100644 --- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -1071,7 +1071,7 @@ public class RestApiServlet extends HttpServlet { TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE); buf.write(JSON_MAGIC); Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8)); - Gson gson = newGson(config, req); + Gson gson = newGson(config); if (result instanceof JsonElement) { gson.toJson((JsonElement) result, w); } else { @@ -1098,25 +1098,18 @@ public class RestApiServlet extends HttpServlet { req, res, asBinaryResult(buf).setContentType(JSON_TYPE).setCharacterEncoding(UTF_8)); } - private static Gson newGson( - ListMultimap config, @Nullable HttpServletRequest req) { + private static Gson newGson(ListMultimap config) { GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder(); - enablePrettyPrint(gb, config, req); + enablePrettyPrint(gb, config); enablePartialGetFields(gb, config); return gb.create(); } - private static void enablePrettyPrint( - GsonBuilder gb, ListMultimap config, @Nullable HttpServletRequest req) { - String pp = Iterables.getFirst(config.get("pp"), null); - if (pp == null) { - pp = Iterables.getFirst(config.get("prettyPrint"), null); - if (pp == null && req != null) { - pp = acceptsJson(req) ? "0" : "1"; - } - } + private static void enablePrettyPrint(GsonBuilder gb, ListMultimap config) { + String pp = + Iterables.getFirst(config.get("pp"), Iterables.getFirst(config.get("prettyPrint"), "0")); if ("1".equals(pp) || "true".equals(pp)) { gb.setPrettyPrinting(); } @@ -1573,10 +1566,6 @@ public class RestApiServlet extends HttpServlet { return CharMatcher.anyOf("<&").matchesAnyOf(text); } - private static boolean acceptsJson(HttpServletRequest req) { - return req != null && isType(JSON_TYPE, req.getHeader(HttpHeaders.ACCEPT)); - } - private static boolean acceptsGzip(HttpServletRequest req) { if (req != null) { String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING); diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java new file mode 100644 index 0000000000..bc454605ab --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java @@ -0,0 +1,76 @@ +// Copyright (C) 2020 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.google.gerrit.acceptance.rest; + +import static com.google.common.truth.Truth.assertThat; +import static org.apache.http.HttpStatus.SC_OK; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.httpd.restapi.RestApiServlet; +import java.io.IOException; +import java.util.regex.Pattern; +import org.apache.http.message.BasicHeader; +import org.junit.Test; + +public class RestApiServletIT extends AbstractDaemonTest { + private static String ANY_REST_API = "/accounts/self/capabilities"; + private static BasicHeader ACCEPT_STAR_HEADER = new BasicHeader("Accept", "*/*"); + private static Pattern ANY_SPACE = Pattern.compile("\\s"); + + @Test + public void restResponseBodyShouldBeCompactWithoutSpaces() throws Exception { + RestResponse response = adminRestSession.getWithHeader(ANY_REST_API, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + assertThat(contentWithoutMagicJson(response)).doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPPIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPrerryPrintIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPPIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 1))).containsMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPrettyPrintIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 1))) + .containsMatch(ANY_SPACE); + } + + private RestResponse prettyJsonRestResponse(String ppArgument, int ppValue) throws Exception { + RestResponse response = + adminRestSession.getWithHeader( + ANY_REST_API + "?" + ppArgument + "=" + ppValue, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + return response; + } + + private String contentWithoutMagicJson(RestResponse response) throws IOException { + return response.getEntityContent().substring(RestApiServlet.JSON_MAGIC.length); + } +} -- cgit v1.2.3