diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/Response.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/Response.java new file mode 100644 index 0000000000..b01146b5de --- /dev/null +++ b/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 { + @SuppressWarnings({"rawtypes"}) + private static final Response NONE = new None(); + + /** HTTP 200 OK: pointless wrapper for type safety. */ + public static Response ok(T value) { + return new Impl(200, value); + } + + /** HTTP 201 Created: typically used when a new resource is made. */ + public static Response created(T value) { + return new Impl(201, value); + } + + /** HTTP 204 No Content: typically used when the resource is deleted. */ + @SuppressWarnings("unchecked") + public static Response none() { + return NONE; + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + public static 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 extends Response { + 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 { + private None() { + } + + @Override + public int statusCode() { + return 204; + } + + public Object value() { + throw new UnsupportedOperationException(); + } + + @Override + public String toString() { + return "[204 No Content] None"; + } + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java index 4ea71219cf..573c5e7e40 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java +++ b/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() { @Override public void onSuccess(NativeString result) { - cb.onSuccess(result.asString()); + cb.onSuccess(result != null ? result.asString() : null); } @Override diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java index 4cbeb9a736..d3c7000431 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java +++ b/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())); } } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index ba168a05bc..8b9fb73163 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/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()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java index 74a5af9e24..cb56627582 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java +++ b/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 { } @Override - public Object apply(RevisionResource rsrc, Input in) throws AuthException, - BadRequestException, ResourceConflictException, Exception { + public Response 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 { 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)); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java index af9b84670d..6573204237 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java +++ b/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 { @Override public Object apply(DraftResource rsrc, Input input) throws OrmException { db.get().patchComments().delete(Collections.singleton(rsrc.getComment())); - return new Object(); + return Response.none(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java index c58f78d89d..9921388c58 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java +++ b/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 { } finally { db.rollback(); } - return new Object(); + return Response.none(); } private Iterable approvals(ReviewDb db, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java index 16c4e898be..3747f9f0b2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java +++ b/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 { }); db.changeMessages().insert(Collections.singleton(cmsg)); } - return Strings.nullToEmpty(newTopicName); + return Strings.isNullOrEmpty(newTopicName) + ? Response.none() + : newTopicName; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/InstallPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/InstallPlugin.java index 8f1280618e..cd64dfdae5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/InstallPlugin.java +++ b/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 { 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 { } @Override - public Object apply(TopLevelResource resource, Input input) - throws AuthException, BadRequestException, ResourceConflictException, - Exception { + public Response apply(TopLevelResource resource, + Input input) throws BadRequestException, IOException { try { InputStream in; if (input.raw != null) { @@ -91,7 +91,9 @@ class InstallPlugin implements RestModifyView { } 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 { } @Override - public Object apply(PluginResource resource, Input input) - throws AuthException, BadRequestException, ResourceConflictException, - Exception { - return new InstallPlugin(loader, resource.getName()) + public Response apply(PluginResource resource, + Input input) throws BadRequestException, IOException { + return new InstallPlugin(loader, resource.getName(), false) .apply(TopLevelResource.INSTANCE, input); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java index 1127a54baf..d461eb1deb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java +++ b/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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java index 6f82eca321..7fb298677a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java +++ b/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 { info.isDefault = true; return info; } - return new Object(); + return Response.none(); } finally { md.close(); } @@ -147,9 +148,9 @@ class SetDefaultDashboard implements RestModifyView { Exception { SetDefaultDashboard set = setDefault.get(); set.inherited = inherited; - return set.apply( + return Response.created(set.apply( DashboardResource.projectDefault(resource.getControl()), - input); + input)); } } }