From: Teryk Bellahsene Date: Fri, 29 Dec 2017 14:43:58 +0000 (+0100) Subject: SONAR-10154 No streaming of web services response by default X-Git-Tag: 7.0-RC1~71 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6a03f934fdd4d1f6700ff825d44e58a0d77c620d;p=sonarqube.git SONAR-10154 No streaming of web services response by default - most of the WS rely on protobuf with one encapsulating object. In effect, streaming is useless in this case - for WS relying on JsonWriter directly, if an exception was thrown when it has started writing, the WebServiceEngine error handling cannot work properly (clear response and format an error message with an appropriate HTTP response code) - a new factory method is added to JsonWriter to have a non streamable JsonWriter. The response is effectively written when the JsonWriter is closed --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/CacheWriter.java b/server/sonar-server/src/main/java/org/sonar/server/ws/CacheWriter.java new file mode 100644 index 00000000000..87e0dfe7a3d --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/CacheWriter.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.ws; + +import java.io.IOException; +import java.io.StringWriter; +import java.io.Writer; +import org.apache.commons.io.IOUtils; + +/** + * Writer that writes only when closing the resource + */ +class CacheWriter extends Writer { + private final StringWriter bufferWriter; + private final Writer outputWriter; + + CacheWriter(Writer outputWriter) { + this.bufferWriter = new StringWriter(); + this.outputWriter = outputWriter; + } + + @Override + public void write(char[] cbuf, int off, int len) { + bufferWriter.write(cbuf, off, len); + } + + @Override + public void flush() { + bufferWriter.flush(); + } + + @Override + public void close() throws IOException { + IOUtils.write(bufferWriter.toString(), outputWriter); + outputWriter.close(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java index 6e21a749df1..286ab98454e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java @@ -19,19 +19,20 @@ */ package org.sonar.server.ws; -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.sonarqube.ws.MediaTypes.JSON; -import static org.sonarqube.ws.MediaTypes.XML; - import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; +import java.nio.charset.StandardCharsets; import java.util.Collection; import javax.servlet.http.HttpServletResponse; import org.sonar.api.server.ws.Response; import org.sonar.api.utils.text.JsonWriter; import org.sonar.api.utils.text.XmlWriter; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.sonarqube.ws.MediaTypes.JSON; +import static org.sonarqube.ws.MediaTypes.XML; + public class ServletResponse implements Response { private final ServletStream stream; @@ -84,7 +85,7 @@ public class ServletResponse implements Response { @Override public JsonWriter newJsonWriter() { stream.setMediaType(JSON); - return JsonWriter.of(new OutputStreamWriter(stream.output(), UTF_8)); + return JsonWriter.of(new CacheWriter(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8))); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java index 4f9ee4b1c4b..72e6340b394 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java @@ -52,7 +52,7 @@ public class WsUtils { msg.writeTo(output); } else { response.stream().setMediaType(JSON); - try (JsonWriter writer = JsonWriter.of(new OutputStreamWriter(output, UTF_8))) { + try (JsonWriter writer = JsonWriter.of(new CacheWriter(new OutputStreamWriter(output, UTF_8)))) { ProtobufJsonFormat.write(msg, writer); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/CacheWriterTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/CacheWriterTest.java new file mode 100644 index 00000000000..a45547e259b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/CacheWriterTest.java @@ -0,0 +1,42 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.ws; + +import java.io.IOException; +import java.io.StringWriter; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class CacheWriterTest { + private StringWriter writer = new StringWriter(); + private CacheWriter underTest = new CacheWriter(writer); + + @Test + public void write_content_when_closing_resource() throws IOException { + underTest.write("content"); + assertThat(writer.toString()).isEmpty(); + + underTest.close(); + + assertThat(writer.toString()).isEqualTo("content"); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java index d1b401edb4e..84ee6f25178 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java @@ -19,6 +19,13 @@ */ package org.sonar.server.ws; +import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServletResponse; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -27,23 +34,15 @@ import static org.mockito.Mockito.when; import static org.sonarqube.ws.MediaTypes.JSON; import static org.sonarqube.ws.MediaTypes.XML; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServletResponse; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - public class ServletResponseTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - ServletOutputStream output = mock(ServletOutputStream.class); - - HttpServletResponse response = mock(HttpServletResponse.class); + private ServletOutputStream output = mock(ServletOutputStream.class); + private HttpServletResponse response = mock(HttpServletResponse.class); - ServletResponse underTest = new ServletResponse(response); + private ServletResponse underTest = new ServletResponse(response); @Before public void setUp() throws Exception { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Response.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Response.java index d22bfed4207..c13a5fc6d5b 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Response.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Response.java @@ -40,9 +40,13 @@ public interface Response { * By default value is set to 200. */ Stream setStatus(int httpStatus); + OutputStream output(); } + /** + * Non streamable {@link JsonWriter}. Response is written when resource is closed. + */ JsonWriter newJsonWriter(); XmlWriter newXmlWriter(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/text/JsonWriter.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/text/JsonWriter.java index 376c98f04cf..15b3f5cfa2a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/text/JsonWriter.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/text/JsonWriter.java @@ -46,7 +46,7 @@ import org.sonar.api.utils.DateUtils; * .endObject() * } * - * + * *

* By default, null objects are not serialized. To enable {@code null} serialization, * use {@link #setSerializeNulls(boolean)}. diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/text/JsonWriterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/text/JsonWriterTest.java index 96d3a4cb658..48567d928af 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/text/JsonWriterTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/text/JsonWriterTest.java @@ -41,37 +41,34 @@ public class JsonWriterTest { private static final String EMPTY_STRING = ""; @Rule - public ExpectedException thrown = ExpectedException.none(); + public ExpectedException expectedException = ExpectedException.none(); - private StringWriter json = new StringWriter(); - private JsonWriter writer = JsonWriter.of(json); + private StringWriter stringWriter = new StringWriter(); - private void expect(String s) { - assertThat(json.toString()).isEqualTo(s); - } + private JsonWriter underTest = JsonWriter.of(stringWriter); @Test public void empty_object() { - writer.beginObject().endObject().close(); + underTest.beginObject().endObject().close(); expect("{}"); } @Test public void empty_array() { - writer.beginArray().endArray().close(); + underTest.beginArray().endArray().close(); expect("[]"); } @Test public void stop_while_streaming() { - writer.beginObject().name("foo").value("bar"); + underTest.beginObject().name("foo").value("bar"); // endObject() and close() are missing expect("{\"foo\":\"bar\""); } @Test public void objects_and_arrays() { - writer.beginObject().name("issues") + underTest.beginObject().name("issues") .beginArray() .beginObject().prop("key", "ABC").endObject() .beginObject().prop("key", "DEF").endObject() @@ -82,14 +79,14 @@ public class JsonWriterTest { @Test public void array_values() { - writer.beginArray().values(Arrays.asList("foo", "bar", "baz")).endArray().close(); + underTest.beginArray().values(Arrays.asList("foo", "bar", "baz")).endArray().close(); expect("[\"foo\",\"bar\",\"baz\"]"); } @Test public void type_of_values() { Date date = DateUtils.parseDateTime("2010-05-18T15:50:45+0100"); - writer.beginObject() + underTest.beginObject() .prop("aBoolean", true) .prop("aInt", 123) .prop("aLong", 1000L) @@ -103,7 +100,7 @@ public class JsonWriterTest { @Test public void ignore_null_values_by_default() { - writer.beginObject() + underTest.beginObject() .prop("nullNumber", (Number) null) .prop("nullString", (String) null) .name("nullNumber").value((Number) null) @@ -116,8 +113,8 @@ public class JsonWriterTest { @Test public void serialize_null_values() { - writer.setSerializeNulls(true); - writer.beginObject() + underTest.setSerializeNulls(true); + underTest.beginObject() .prop("nullNumber", (Number) null) .prop("nullString", (String) null) .name("nullNumber").value((Number) null) @@ -130,7 +127,7 @@ public class JsonWriterTest { @Test public void serialize_empty_strings_by_default() { - writer.beginObject() + underTest.beginObject() .prop("emptyString", EMPTY_STRING) .name("emptyStringAsObject").valueObject(EMPTY_STRING) .endObject().close(); @@ -142,7 +139,7 @@ public class JsonWriterTest { @Test public void ignore_empty_strings_when_requested() { - writer.setSerializeEmptys(false) + underTest.setSerializeEmptys(false) .beginObject() .prop("emptyString", EMPTY_STRING) .name("emptyStringAsObject").valueObject(EMPTY_STRING) @@ -152,7 +149,7 @@ public class JsonWriterTest { @Test public void escape_values() { - writer.beginObject() + underTest.beginObject() .prop("foo", "") .endObject().close(); expect("{\"foo\":\"\"}"); @@ -160,7 +157,7 @@ public class JsonWriterTest { @Test public void valueObject() { - writer.beginObject() + underTest.beginObject() .name("aString").valueObject("stringValue") .name("aBoolean").valueObject(true) .name("aInt").valueObject(42) @@ -177,14 +174,14 @@ public class JsonWriterTest { @Test public void valueObject_recursive() { Map map = ImmutableMap.of("a", ImmutableMap.of("b", "c")); - writer.valueObject(map).close(); + underTest.valueObject(map).close(); expect("{\"a\":{\"b\":\"c\"}}"); } @Test public void valueObject_unsupported_type() { try { - writer.beginObject().valueObject(new StringWriter()).endObject().close(); + underTest.beginObject().valueObject(new StringWriter()).endObject().close(); fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("class org.sonar.api.utils.text.JsonWriter does not support encoding of type: class java.io.StringWriter"); @@ -193,26 +190,30 @@ public class JsonWriterTest { @Test public void fail_on_NaN_value() { - thrown.expect(WriterException.class); - writer.beginObject().prop("foo", Double.NaN).endObject().close(); + expectedException.expect(WriterException.class); + underTest.beginObject().prop("foo", Double.NaN).endObject().close(); } @Test public void fail_if_not_valid() { - thrown.expect(WriterException.class); - writer.beginObject().endArray().close(); + expectedException.expect(WriterException.class); + underTest.beginObject().endArray().close(); } @Test public void fail_to_begin_array() throws Exception { com.google.gson.stream.JsonWriter gson = mock(com.google.gson.stream.JsonWriter.class); when(gson.beginArray()).thenThrow(new IOException("the reason")); - thrown.expect(WriterException.class); - thrown.expectMessage("Fail to write JSON"); + expectedException.expect(WriterException.class); + expectedException.expectMessage("Fail to write JSON"); new JsonWriter(gson).beginArray(); } + private void expect(String s) { + assertThat(stringWriter.toString()).isEqualTo(s); + } + private enum ColorEnum { RED, GREEN }