Use HTTP 201, 204 response codes in REST API

When creating a new entity on the server respond HTTP 201 Created
with the JSON representation of the entity in the body.

When deleting an existing entity, respond with HTTP 204 No Content,
telling the client there is no more content behind the resources
that was just deleted. This is a hint that a future GET is likely
to return 404 Not Found.

Change-Id: Ia7b3964267fcd55b4abcc49dcd6ba4c61f61fd5d

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/Response.java

@@ -0,0 +1,93 @@
+// Copyright (C) 2012 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.extensions.restapi;
+
+/** Special return value to mean specific HTTP status codes in a REST API. */
+public abstract class Response<T> {
+  @SuppressWarnings({"rawtypes"})
+  private static final Response NONE = new None();
+
+  /** HTTP 200 OK: pointless wrapper for type safety. */
+  public static <T> Response<T> ok(T value) {
+    return new Impl<T>(200, value);
+  }
+
+  /** HTTP 201 Created: typically used when a new resource is made. */
+  public static <T> Response<T> created(T value) {
+    return new Impl<T>(201, value);
+  }
+
+  /** HTTP 204 No Content: typically used when the resource is deleted. */
+  @SuppressWarnings("unchecked")
+  public static <T> Response<T> none() {
+    return NONE;
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  public static <T> T unwrap(T obj) {
+    while (obj instanceof Response) {
+      obj = (T) ((Response) obj).value();
+    }
+    return obj;
+  }
+
+  public abstract int statusCode();
+  public abstract T value();
+  public abstract String toString();
+
+  private static final class Impl<T> extends Response<T> {
+    private final int statusCode;
+    private final T value;
+
+    private Impl(int sc, T val) {
+      statusCode = sc;
+      value = val;
+    }
+
+    @Override
+    public int statusCode() {
+      return statusCode;
+    }
+
+    @Override
+    public T value() {
+      return value;
+    }
+
+    @Override
+    public String toString() {
+      return "[" + statusCode() + "] " + value();
+    }
+  }
+
+  private static final class None extends Response<Object> {
+    private None() {
+    }
+
+    @Override
+    public int statusCode() {
+      return 204;
+    }
+
+    public Object value() {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public String toString() {
+      return "[204 No Content] None";
+    }
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java

@@ -33,7 +33,7 @@ public final class NativeString extends JavaScriptObject {
     return new AsyncCallback<NativeString>() {
       @Override
       public void onSuccess(NativeString result) {
-        cb.onSuccess(result.asString());
+        cb.onSuccess(result != null ? result.asString() : null);
       }
 
       @Override

gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java

@@ -40,7 +40,8 @@ import com.google.gwt.user.client.rpc.StatusCodeException;
 /** Makes a REST API call to the server. */
 public class RestApi {
   private static final int SC_UNAVAILABLE = 2;
-  private static final int SC_TRANSPORT = 3;
+  private static final int SC_BAD_TRANSPORT = 3;
+  private static final int SC_BAD_RESPONSE = 4;
   private static final String JSON_TYPE = "application/json";
   private static final String TEXT_TYPE = "text/plain";
 
@@ -111,7 +112,33 @@ public class RestApi {
     @Override
     public void onResponseReceived(Request req, Response res) {
       int status = res.getStatusCode();
-      if (status != 200) {
+      if (status == Response.SC_NO_CONTENT) {
+        cb.onSuccess(null);
+        RpcStatus.INSTANCE.onRpcComplete();
+
+      } else if (200 <= status && status < 300) {
+        if (!isJsonBody(res)) {
+          RpcStatus.INSTANCE.onRpcComplete();
+          cb.onFailure(new StatusCodeException(SC_BAD_RESPONSE, "Expected "
+              + JSON_TYPE + "; received Content-Type: "
+              + res.getHeader("Content-Type")));
+          return;
+        }
+
+        T data;
+        try {
+          data = cast(parseJson(res));
+        } catch (JSONException e) {
+          RpcStatus.INSTANCE.onRpcComplete();
+          cb.onFailure(new StatusCodeException(SC_BAD_RESPONSE,
+              "Invalid JSON: " + e.getMessage()));
+          return;
+        }
+
+        cb.onSuccess(data);
+        RpcStatus.INSTANCE.onRpcComplete();
+
+      } else {
         String msg;
         if (isTextBody(res)) {
           msg = res.getText().trim();
@@ -133,29 +160,7 @@ public class RestApi {
 
         RpcStatus.INSTANCE.onRpcComplete();
         cb.onFailure(new StatusCodeException(status, msg));
-        return;
       }
-
-      if (!isJsonBody(res)) {
-        RpcStatus.INSTANCE.onRpcComplete();
-        cb.onFailure(new StatusCodeException(200, "Expected "
-            + JSON_TYPE + "; received Content-Type: "
-            + res.getHeader("Content-Type")));
-        return;
-      }
-
-      T data;
-      try {
-        data = cast(parseJson(res));
-      } catch (JSONException e) {
-        RpcStatus.INSTANCE.onRpcComplete();
-        cb.onFailure(new StatusCodeException(200,
-            "Invalid JSON: " + e.getMessage()));
-        return;
-      }
-
-      cb.onSuccess(data);
-      RpcStatus.INSTANCE.onRpcComplete();
     }
 
     @Override
@@ -166,7 +171,7 @@ public class RestApi {
             SC_UNAVAILABLE,
             RpcConstants.C.errorServerUnavailable()));
       } else {
-        cb.onFailure(new StatusCodeException(SC_TRANSPORT, err.getMessage()));
+        cb.onFailure(new StatusCodeException(SC_BAD_TRANSPORT, err.getMessage()));
       }
     }
   }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -43,6 +43,7 @@ import com.google.gerrit.extensions.restapi.AcceptsCreate;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.PutInput;
@@ -237,11 +238,20 @@ public class RestApiServlet extends HttpServlet {
         throw new ResourceNotFoundException();
       }
 
+      if (result instanceof Response) {
+        @SuppressWarnings("rawtypes")
+        Response r = (Response) result;
+        status = r.statusCode();
+      }
       res.setStatus(status);
-      if (result instanceof BinaryResult) {
-        replyBinaryResult(req, res, (BinaryResult) result);
-      } else {
-        replyJson(req, res, config, result);
+
+      if (result != Response.none()) {
+        result = Response.unwrap(result);
+        if (result instanceof BinaryResult) {
+          replyBinaryResult(req, res, (BinaryResult) result);
+        } else {
+          replyJson(req, res, config, result);
+        }
       }
     } catch (AuthException e) {
       replyError(res, SC_FORBIDDEN, e.getMessage());

gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java

@@ -18,6 +18,7 @@ import com.google.common.base.Strings;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
@@ -26,6 +27,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.change.PutDraft.Input;
 import com.google.gerrit.server.util.Url;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -45,8 +47,8 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
   }
 
   @Override
-  public Object apply(RevisionResource rsrc, Input in) throws AuthException,
-      BadRequestException, ResourceConflictException, Exception {
+  public Response<GetDraft.Comment> apply(RevisionResource rsrc, Input in)
+      throws AuthException, BadRequestException, ResourceConflictException, OrmException {
     if (Strings.isNullOrEmpty(in.path)) {
       throw new BadRequestException("path must be non-empty");
     } else if (in.message == null || in.message.trim().isEmpty()) {
@@ -66,6 +68,6 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
     c.setSide(in.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1);
     c.setMessage(in.message.trim());
     db.get().patchComments().insert(Collections.singleton(c));
-    return new GetDraft.Comment(c);
+    return Response.created(new GetDraft.Comment(c));
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.DeleteDraft.Input;
@@ -42,6 +43,6 @@ class DeleteDraft implements RestModifyView<DraftResource, Input> {
   @Override
   public Object apply(DraftResource rsrc, Input input) throws OrmException {
     db.get().patchComments().delete(Collections.singleton(rsrc.getComment()));
-    return new Object();
+    return Response.none();
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java

@@ -18,6 +18,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.Change;
@@ -71,7 +72,7 @@ class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
     } finally {
       db.rollback();
     }
-    return new Object();
+    return Response.none();
   }
 
   private Iterable<PatchSetApproval> approvals(ReviewDb db,

gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java

@@ -18,6 +18,7 @@ import com.google.common.base.Strings;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.Change;
@@ -102,6 +103,8 @@ class PutTopic implements RestModifyView<ChangeResource, Input> {
         });
       db.changeMessages().insert(Collections.singleton(cmsg));
     }
-    return Strings.nullToEmpty(newTopicName);
+    return Strings.isNullOrEmpty(newTopicName)
+        ? Response.none()
+        : newTopicName;
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/plugins/InstallPlugin.java

@@ -16,11 +16,10 @@ package com.google.gerrit.server.plugins;
 
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.PutInput;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.server.plugins.InstallPlugin.Input;
@@ -44,10 +43,12 @@ class InstallPlugin implements RestModifyView<TopLevelResource, Input> {
 
   private final PluginLoader loader;
   private final String name;
+  private final boolean created;
 
-  InstallPlugin(PluginLoader loader, String name) {
+  InstallPlugin(PluginLoader loader, String name, boolean created) {
     this.loader = loader;
     this.name = name;
+    this.created = created;
   }
 
   @Override
@@ -56,9 +57,8 @@ class InstallPlugin implements RestModifyView<TopLevelResource, Input> {
   }
 
   @Override
-  public Object apply(TopLevelResource resource, Input input)
-      throws AuthException, BadRequestException, ResourceConflictException,
-      Exception {
+  public Response<ListPlugins.PluginInfo> apply(TopLevelResource resource,
+      Input input) throws BadRequestException, IOException {
     try {
       InputStream in;
       if (input.raw != null) {
@@ -91,7 +91,9 @@ class InstallPlugin implements RestModifyView<TopLevelResource, Input> {
       }
       throw new BadRequestException(buf.toString());
     }
-    return new ListPlugins.PluginInfo(loader.get(name));
+
+    ListPlugins.PluginInfo info = new ListPlugins.PluginInfo(loader.get(name));
+    return created ? Response.created(info) : Response.ok(info);
   }
 
   @RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@@ -109,10 +111,9 @@ class InstallPlugin implements RestModifyView<TopLevelResource, Input> {
     }
 
     @Override
-    public Object apply(PluginResource resource, Input input)
-        throws AuthException, BadRequestException, ResourceConflictException,
-        Exception {
-      return new InstallPlugin(loader, resource.getName())
+    public Response<ListPlugins.PluginInfo> apply(PluginResource resource,
+        Input input) throws BadRequestException, IOException {
+      return new InstallPlugin(loader, resource.getName(), false)
         .apply(TopLevelResource.INSTANCE, input);
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java

@@ -59,7 +59,7 @@ public class PluginsCollection implements
   @Override
   public InstallPlugin create(TopLevelResource parent, String id)
       throws ResourceNotFoundException {
-    return new InstallPlugin(loader, Url.decode(id));
+    return new InstallPlugin(loader, Url.decode(id), true /* created */);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java

@@ -18,6 +18,7 @@ import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -112,7 +113,7 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, Input> {
           info.isDefault = true;
           return info;
         }
-        return new Object();
+        return Response.none();
       } finally {
         md.close();
       }
@@ -147,9 +148,9 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, Input> {
         Exception {
       SetDefaultDashboard set = setDefault.get();
       set.inherited = inherited;
-      return set.apply(
+      return Response.created(set.apply(
           DashboardResource.projectDefault(resource.getControl()),
-          input);
+          input));
     }
   }
 }