Remove unused Response.InternalServerError and Response.traceId

Response.InternalServerError was the only implementation of
Response#value() that threw an exception. Hence the throw clause from
Response#value() can be removed and callers can handle exceptions
specifically now.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I083ea41990378058ad46d73bc7b4748801dab2da
This commit is contained in:
Edwin Kempin
2019-10-22 15:35:49 +02:00
committed by David Pursehouse
parent 3e634628bb
commit 87a5125118
10 changed files with 31 additions and 101 deletions

View File

@@ -14,10 +14,6 @@
package com.google.gerrit.extensions.restapi;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.gerrit.common.Nullable;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
/** Special return value to mean specific HTTP status codes in a REST API. */
@@ -66,48 +62,24 @@ public abstract class Response<T> {
return new Redirect(location);
}
/**
* HTTP 500 Internal Server Error: failure due to an unexpected exception.
*
* <p>Can be returned from REST endpoints, instead of throwing the exception, if additional
* properties (e.g. a traceId) should be set on the response.
*
* @param cause the exception that caused the request to fail, must not be a {@link
* RestApiException} because such an exception would result in a 4XX response code
*/
public static <T> InternalServerError<T> internalServerError(Exception cause) {
return new InternalServerError<>(cause);
}
/** Arbitrary status code with wrapped result. */
public static <T> Response<T> withStatusCode(int statusCode, T value) {
return new Impl<>(statusCode, value);
}
@SuppressWarnings({"unchecked", "rawtypes"})
public static <T> T unwrap(T obj) throws Exception {
public static <T> T unwrap(T obj) {
while (obj instanceof Response) {
obj = (T) ((Response) obj).value();
}
return obj;
}
private String traceId;
public Response<T> traceId(@Nullable String traceId) {
this.traceId = traceId;
return this;
}
public Optional<String> traceId() {
return Optional.ofNullable(traceId);
}
public abstract boolean isNone();
public abstract int statusCode();
public abstract T value() throws Exception;
public abstract T value();
public abstract CacheControl caching();
@@ -297,57 +269,4 @@ public abstract class Response<T> {
return String.format("[202 Accepted] %s", location);
}
}
public static final class InternalServerError<T> extends Response<T> {
private final Exception cause;
private InternalServerError(Exception cause) {
checkArgument(!(cause instanceof RestApiException), "cause must not be a RestApiException");
this.cause = cause;
}
@Override
public boolean isNone() {
return false;
}
@Override
public int statusCode() {
return 500;
}
@Override
public T value() throws Exception {
throw cause();
}
@Override
public CacheControl caching() {
return CacheControl.NONE;
}
@Override
public Response<T> caching(CacheControl c) {
throw new UnsupportedOperationException();
}
public Exception cause() {
return cause;
}
@Override
public int hashCode() {
return cause.hashCode();
}
@Override
public boolean equals(Object o) {
return o instanceof InternalServerError && ((InternalServerError<?>) o).cause.equals(cause);
}
@Override
public String toString() {
return String.format("[500 Internal Server Error] %s", cause.getClass());
}
}
}

View File

@@ -562,9 +562,6 @@ public class RestApiServlet extends HttpServlet {
throw new ResourceNotFoundException();
}
traceId = response.traceId();
traceId.ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
if (response instanceof Response.Redirect) {
CacheHeaders.setNotCacheable(res);
String location = ((Response.Redirect) response).location();
@@ -577,12 +574,6 @@ public class RestApiServlet extends HttpServlet {
res.setHeader(HttpHeaders.LOCATION, ((Response.Accepted) response).location());
logger.atFinest().log("REST call succeeded: %d", response.statusCode());
return;
} else if (response instanceof Response.InternalServerError) {
// Rethrow the exception to have exactly the same error handling as if the REST endpoint
// would have thrown the exception directly, instead of returning
// Response.InternalServerError.
Exception cause = ((Response.InternalServerError<?>) response).cause();
throw cause;
}
status = response.statusCode();

View File

@@ -98,7 +98,8 @@ public class Stars implements ChildCollection<AccountResource, AccountResource.S
@Override
@SuppressWarnings("unchecked")
public Response<List<ChangeInfo>> apply(AccountResource rsrc) throws Exception {
public Response<List<ChangeInfo>> apply(AccountResource rsrc)
throws RestApiException, PermissionBackendException {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to list stars of another account");
}

View File

@@ -269,7 +269,8 @@ public class Rebase
}
@Override
public Response<ChangeInfo> apply(ChangeResource rsrc, RebaseInput input) throws Exception {
public Response<ChangeInfo> apply(ChangeResource rsrc, RebaseInput input)
throws RestApiException, UpdateException, IOException, PermissionBackendException {
PatchSet ps = psUtil.current(rsrc.getNotes());
if (ps == null) {
throw new ResourceConflictException("current revision is missing");

View File

@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.RevertSubmissionInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.ChangeUtil;
@@ -33,10 +34,14 @@ import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ContributorAgreementsChecker;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -44,6 +49,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.apache.commons.lang.RandomStringUtils;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class RevertSubmission
@@ -81,7 +87,8 @@ public class RevertSubmission
@Override
public Response<RevertSubmissionInfo> apply(ChangeResource changeResource, RevertInput input)
throws Exception {
throws RestApiException, NoSuchChangeException, IOException, UpdateException,
PermissionBackendException, NoSuchProjectException, ConfigInvalidException {
if (!changeResource.getChange().isMerged()) {
throw new ResourceConflictException(
@@ -117,7 +124,9 @@ public class RevertSubmission
}
private RevertSubmissionInfo revertSubmission(
List<ChangeData> changeDatas, RevertInput input, String submissionId) throws Exception {
List<ChangeData> changeDatas, RevertInput input, String submissionId)
throws RestApiException, NoSuchChangeException, IOException, UpdateException,
PermissionBackendException, NoSuchProjectException, ConfigInvalidException {
List<ChangeInfo> results;
results = new ArrayList<>();
if (input.topic == null) {

View File

@@ -28,6 +28,7 @@ import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -222,7 +223,8 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
@Override
public Response<AccountInfo> apply(GroupResource resource, IdString id, Input input)
throws Exception {
throws RestApiException, NotInternalGroupException, IOException, ConfigInvalidException,
PermissionBackendException {
AddMembers.Input in = new AddMembers.Input();
in._oneMember = id.get();
try {

View File

@@ -19,12 +19,15 @@ import com.google.gerrit.extensions.api.projects.SetDashboardInput;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardResource;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import org.kohsuke.args4j.Option;
@Singleton
@@ -42,7 +45,7 @@ public class CreateDashboard
@Override
public Response<DashboardInfo> apply(ProjectResource parent, IdString id, SetDashboardInput input)
throws Exception {
throws RestApiException, IOException, PermissionBackendException {
parent.getProjectState().checkStatePermitsWrite();
if (!DashboardsCollection.isDefaultDashboard(id)) {
throw new ResourceNotFoundException(id);

View File

@@ -24,9 +24,11 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.DashboardResource;
import com.google.gerrit.server.project.ProjectCache;
@@ -34,6 +36,7 @@ import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.kohsuke.args4j.Option;
@@ -67,7 +70,7 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, SetDashbo
@Override
public Response<DashboardInfo> apply(DashboardResource rsrc, SetDashboardInput input)
throws Exception {
throws RestApiException, IOException, PermissionBackendException {
if (input == null) {
input = new SetDashboardInput(); // Delete would set input to null.
}

View File

@@ -119,7 +119,8 @@ final class CreateGroupCommand extends SshCommand {
}
}
private GroupResource createGroup() throws Exception {
private GroupResource createGroup()
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
GroupInput input = new GroupInput();
input.description = groupDescription;
input.visibleToAll = visibleToAll;

View File

@@ -197,7 +197,7 @@ final class ShowCaches extends SshCommand {
stdout.flush();
}
private Collection<CacheInfo> getCaches() throws Exception {
private Collection<CacheInfo> getCaches() {
@SuppressWarnings("unchecked")
Map<String, CacheInfo> caches =
(Map<String, CacheInfo>) listCaches.apply(new ConfigResource()).value();