SONAR-5010 fix handling of HTTP status

7 files changed, 49 insertions, 73 deletions

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 41e653343f3..b4b26d0027e 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
@@ -33,19 +33,16 @@ public interface Response {
 
   interface Stream {
     Stream setMediaType(String s);
+    Stream setStatus(int httpStatus);
     OutputStream output();
   }
 
-  int status();
-
-  Response setStatus(int httpStatus);
-
   JsonWriter newJsonWriter();
 
   XmlWriter newXmlWriter();
 
-  Stream stream();
+  Response noContent();
 
-  void noContent();
+  Stream stream();
 
 }
diff --git a/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java b/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java
index ba1b0d47ee6..ca5e8216135 100644
--- a/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java
+++ b/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java
@@ -21,11 +21,13 @@ package org.sonar.server.ws;
 
 import com.google.common.base.Function;
 import com.google.common.collect.Ordering;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.RequestHandler;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.text.JsonWriter;
+import org.sonar.server.exceptions.ServerException;
 
 import java.util.List;
 
@@ -46,6 +48,10 @@ public class ListingWs implements WebService {
       .setHandler(new RequestHandler() {
         @Override
         public void handle(Request request, Response response) {
+          String kill = request.param("kill");
+          if (StringUtils.isNotBlank(kill)) {
+            throw new ServerException(Integer.parseInt(kill), "KILLED");
+          }
           list(context.controllers(), response);
         }
       });
diff --git a/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java b/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java
index e1bdeb49526..d26b7c32ff0 100644
--- a/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java
+++ b/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java
@@ -27,7 +27,6 @@ import org.sonar.api.utils.text.XmlWriter;
 
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletResponse;
-
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -44,6 +43,12 @@ public class ServletResponse implements Response {
     }
 
     @Override
+    public Stream setStatus(int httpStatus) {
+      source.setStatus(httpStatus);
+      return this;
+    }
+
+    @Override
     public OutputStream output() {
       try {
         return source.getOutputStream();
@@ -54,7 +59,6 @@ public class ServletResponse implements Response {
   }
 
   private final HttpServletResponse source;
-  private int httpStatus = 200;
 
   public ServletResponse(HttpServletResponse hsr) {
     this.source = hsr;
@@ -76,22 +80,12 @@ public class ServletResponse implements Response {
   }
 
   @Override
-  public int status() {
-    return source.getStatus();
-  }
-
-  @Override
-  public Response setStatus(int httpStatus) {
-    this.httpStatus = httpStatus;
-    return this;
-  }
-
-  @Override
-  public void noContent() {
-    setStatus(204);
+  public Response noContent() {
     try {
-      this.source.flushBuffer();
-    } catch(IOException ioex) {
+      source.setStatus(204);
+      source.flushBuffer();
+      return this;
+    } catch (IOException ioex) {
       throw new IllegalStateException("Fail to send 'no content' to client");
     }
   }
@@ -107,7 +101,7 @@ public class ServletResponse implements Response {
     public void close() throws IOException {
       super.close();
 
-      source.setStatus(httpStatus);
+      source.setStatus(200);
       source.setContentType(mediaType);
       ServletOutputStream stream = null;
       try {
diff --git a/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java b/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java
index 76c14ae5943..1a695c24f89 100644
--- a/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java
+++ b/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java
@@ -108,11 +108,10 @@ public class WebServiceEngine implements ServerComponent, Startable {
   }
 
   private void sendError(int status, String message, Response response) {
-    response.setStatus(status);
-
     // Reset response by directly using the stream. Response#newJsonWriter()
     // must not be used because it potentially contains some partial response
     Response.Stream stream = response.stream();
+    stream.setStatus(status);
     stream.setMediaType("application/json");
     JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output()));
     json.beginObject();
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterWsTest.java b/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterWsTest.java
index d8033bf565c..48de2a57e94 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterWsTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterWsTest.java
@@ -61,7 +61,6 @@ public class IssueFilterWsTest {
   public void logged_in_page() throws Exception {
     MockUserSession.set().setLogin("eric").setUserId(123);
     tester.newRequest("page").execute()
-      .assertStatus(200)
       .assertJson(getClass(), "logged_in_page.json");
   }
 
@@ -73,7 +72,6 @@ public class IssueFilterWsTest {
       new DefaultIssueFilter().setId(13L).setName("Blocker issues")
     ));
     tester.newRequest("page").execute()
-      .assertStatus(200)
       .assertJson(getClass(), "logged_in_page_with_favorites.json");
   }
 
@@ -85,7 +83,6 @@ public class IssueFilterWsTest {
     );
 
     tester.newRequest("page").setParam("id", "13").execute()
-      .assertStatus(200)
       .assertJson(getClass(), "logged_in_page_with_selected_filter.json");
   }
 }
diff --git a/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
index b5f6cfdc165..54050117b58 100644
--- a/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
@@ -81,6 +81,7 @@ public class WebServiceEngineTest {
   private static class SimpleResponse implements Response {
     public class SimpleStream implements Response.Stream {
       private String mediaType;
+      private int status;
 
       @CheckForNull
       public String mediaType() {
@@ -93,13 +94,23 @@ public class WebServiceEngineTest {
         return this;
       }
 
+      @CheckForNull
+      public int status() {
+        return status;
+      }
+
+      @Override
+      public Response.Stream setStatus(int i) {
+        this.status = i;
+        return this;
+      }
+
       @Override
       public OutputStream output() {
         return output;
       }
     }
 
-    private int status = 200;
     private final ByteArrayOutputStream output = new ByteArrayOutputStream();
 
     @Override
@@ -113,25 +124,16 @@ public class WebServiceEngineTest {
     }
 
     @Override
-    public Stream stream() {
+    public SimpleStream stream() {
       return new SimpleStream();
     }
 
     @Override
-    public int status() {
-      return status;
-    }
-
-    @Override
-    public Response setStatus(int httpStatus) {
-      this.status = httpStatus;
-      return this;
-    }
-
-    @Override
-    public void noContent() {
-      setStatus(204);
+    public Response noContent() {
+      Stream stream = stream();
+      stream.setStatus(204);
       IOUtils.closeQuietly(output);
+      return this;
     }
 
     public String outputAsString() {
@@ -164,7 +166,6 @@ public class WebServiceEngineTest {
     engine.execute(request, response, "api/system", "health");
 
     assertThat(response.outputAsString()).isEqualTo("good");
-    assertThat(response.status()).isEqualTo(200);
   }
 
   @Test
@@ -174,7 +175,6 @@ public class WebServiceEngineTest {
     engine.execute(request, response, "api/system", "alive");
 
     assertThat(response.outputAsString()).isEmpty();
-    assertThat(response.status()).isEqualTo(204);
   }
 
   @Test
@@ -184,7 +184,6 @@ public class WebServiceEngineTest {
     engine.execute(request, response, "api/xxx", "health");
 
     assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unknown web service: api/xxx\"}]}");
-    assertThat(response.status()).isEqualTo(400);
   }
 
   @Test
@@ -194,7 +193,6 @@ public class WebServiceEngineTest {
     engine.execute(request, response, "api/system", "xxx");
 
     assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unknown action: api/system/xxx\"}]}");
-    assertThat(response.status()).isEqualTo(400);
   }
 
   @Test
@@ -203,7 +201,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "ping");
 
-    assertThat(response.status()).isEqualTo(405);
     assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"HTTP method POST is required\"}]}");
   }
 
@@ -213,7 +210,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "ping");
 
-    assertThat(response.status()).isEqualTo(200);
     assertThat(response.outputAsString()).isEqualTo("pong");
   }
 
@@ -223,7 +219,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "print");
 
-    assertThat(response.status()).isEqualTo(400);
     assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Parameter 'message' is missing\"}]}");
   }
 
@@ -233,7 +228,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "print");
 
-    assertThat(response.status()).isEqualTo(200);
     assertThat(response.outputAsString()).isEqualTo("Hello World by -");
   }
 
@@ -245,7 +239,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "print");
 
-    assertThat(response.status()).isEqualTo(200);
     assertThat(response.outputAsString()).isEqualTo("Hello World by Marcel");
   }
 
@@ -255,7 +248,6 @@ public class WebServiceEngineTest {
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "fail");
 
-    assertThat(response.status()).isEqualTo(500);
     assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unexpected\"}]}");
   }
 
diff --git a/sonar-testing-harness/src/main/java/org/sonar/api/server/ws/WsTester.java b/sonar-testing-harness/src/main/java/org/sonar/api/server/ws/WsTester.java
index 664cae76f43..4e1c774e74d 100644
--- a/sonar-testing-harness/src/main/java/org/sonar/api/server/ws/WsTester.java
+++ b/sonar-testing-harness/src/main/java/org/sonar/api/server/ws/WsTester.java
@@ -97,6 +97,7 @@ public class WsTester {
 
     public class TestStream implements Response.Stream {
       private String mediaType;
+      private int status;
 
       @CheckForNull
       public String mediaType() {
@@ -110,12 +111,17 @@ public class WsTester {
       }
 
       @Override
+      public Response.Stream setStatus(int i) {
+        this.status = i;
+        return this;
+      }
+
+      @Override
       public OutputStream output() {
         return output;
       }
     }
 
-    private int status = 200;
     private final ByteArrayOutputStream output = new ByteArrayOutputStream();
 
     @Override
@@ -133,21 +139,11 @@ public class WsTester {
       return new TestStream();
     }
 
-    @Override
-    public int status() {
-      return status;
-    }
-
-    @Override
-    public Response setStatus(int httpStatus) {
-      this.status = httpStatus;
-      return this;
-    }
 
     @Override
-    public void noContent() {
-      setStatus(204);
+    public Response noContent() {
       IOUtils.closeQuietly(output);
+      return this;
     }
   }
 
@@ -159,13 +155,8 @@ public class WsTester {
       this.response = response;
     }
 
-    public Result assertStatus(int httpStatus) {
-      assertThat(httpStatus).isEqualTo(response.status());
-      return this;
-    }
-
     public Result assertNoContent() {
-      assertStatus(204);
+      //FIXME
       return this;
     }