]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10154 No streaming of web services response by default
authorTeryk Bellahsene <teryk.bellahsene@sonarsource.com>
Fri, 29 Dec 2017 14:43:58 +0000 (15:43 +0100)
committerTeryk Bellahsene <teryk@users.noreply.github.com>
Wed, 3 Jan 2018 09:11:00 +0000 (10:11 +0100)
- 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

server/sonar-server/src/main/java/org/sonar/server/ws/CacheWriter.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java
server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java
server/sonar-server/src/test/java/org/sonar/server/ws/CacheWriterTest.java [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java
sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Response.java
sonar-plugin-api/src/main/java/org/sonar/api/utils/text/JsonWriter.java
sonar-plugin-api/src/test/java/org/sonar/api/utils/text/JsonWriterTest.java

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 (file)
index 0000000..87e0dfe
--- /dev/null
@@ -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();
+  }
+}
index 6e21a749df1a9c3e0c9e51270aa8f14422871e84..286ab98454ea1215bec9c7f2f6d70cbb5e9dc558 100644 (file)
  */
 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
index 4f9ee4b1c4b4dc4a56b57c70a68293ca8b03bc39..72e6340b3941aad21f01b34d6ddc994bd8eabf08 100644 (file)
@@ -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 (file)
index 0000000..a45547e
--- /dev/null
@@ -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");
+  }
+}
index d1b401edb4e7ac0a216fb2a53d7c7b1e1da4ffb3..84ee6f251787ded949cba174f67589a9f17d326a 100644 (file)
  */
 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 {
index d22bfed4207d9f22fc98b8aef0505173ac4889fa..c13a5fc6d5b00544fc15ec3de36a14015e4cb36f 100644 (file)
@@ -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();
index 376c98f04cf64d08cd1b5f9de480d7445bd35fda..15b3f5cfa2a508c117ed4c5eb76c4c2e623ad3ee 100644 (file)
@@ -46,7 +46,7 @@ import org.sonar.api.utils.DateUtils;
  *       .endObject()
  *   }
  * </pre>
- * 
+ *
  * <p>
  * By default, null objects are not serialized. To enable {@code null} serialization,
  * use {@link #setSerializeNulls(boolean)}.
index 96d3a4cb658065e6043c5f1b580205915d1e52a8..48567d928af29a89d19bc857c37f09c134511168 100644 (file)
@@ -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", "<hello \"world\">")
       .endObject().close();
     expect("{\"foo\":\"<hello \\\"world\\\">\"}");
@@ -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
   }