Merge changes from topic "RestApiException-refactoring"

* changes:
  Make RestApiException(String, Exception) constructor protected
  Do not throw generic RestApiException to signal an internal server error
  ProjectsImpl: Use asRestApiException method to wrap exception as RestApiException
  Make RestApiException(String) constructor protected
  Submit: Do not throw generic RestApiException to signal an internal server error
  RestApiException: Make default constructor protected
  Remove unused default constructors from RestApiException classes
  PluginCollection: Pass id which was not found to ResourceNotFoundException
  RestApiServlet: Return error message when raw input is not supported
  Make error response for non-default dashboards consistent & include message
This commit is contained in:
David Pursehouse 2019-12-29 11:18:09 +00:00 committed by Gerrit Code Review
commit 494d93fd26
15 changed files with 28 additions and 31 deletions

View File

@ -45,7 +45,7 @@ public class PluginCollection implements ChildCollection<ConfigResource, PluginR
@Override @Override
public PluginResource parse(ConfigResource parent, IdString id) public PluginResource parse(ConfigResource parent, IdString id)
throws ResourceNotFoundException, Exception { throws ResourceNotFoundException, Exception {
throw new ResourceNotFoundException(); throw new ResourceNotFoundException(id);
} }
@Override @Override

View File

@ -18,10 +18,6 @@ package com.google.gerrit.extensions.restapi;
public class MethodNotAllowedException extends RestApiException { public class MethodNotAllowedException extends RestApiException {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
public MethodNotAllowedException() {
super();
}
/** @param msg error text for client describing why the method is not allowed. */ /** @param msg error text for client describing why the method is not allowed. */
public MethodNotAllowedException(String msg) { public MethodNotAllowedException(String msg) {
super(msg); super(msg);

View File

@ -18,8 +18,6 @@ package com.google.gerrit.extensions.restapi;
public class PreconditionFailedException extends RestApiException { public class PreconditionFailedException extends RestApiException {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
public PreconditionFailedException() {}
/** @param msg message to return to the client describing the error. */ /** @param msg message to return to the client describing the error. */
public PreconditionFailedException(String msg) { public PreconditionFailedException(String msg) {
super(msg); super(msg);

View File

@ -21,13 +21,17 @@ public class RestApiException extends Exception {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
private CacheControl caching = CacheControl.NONE; private CacheControl caching = CacheControl.NONE;
public RestApiException() {} public static RestApiException wrap(String msg, Exception e) {
return new RestApiException(msg, e);
}
public RestApiException(String msg) { protected RestApiException() {}
protected RestApiException(String msg) {
super(msg); super(msg);
} }
public RestApiException(String msg, Throwable cause) { protected RestApiException(String msg, Throwable cause) {
super(msg, cause); super(msg, cause);
} }

View File

@ -1237,7 +1237,7 @@ public class RestApiServlet extends HttpServlet {
return obj; return obj;
} }
} }
throw new MethodNotAllowedException(); throw new MethodNotAllowedException("raw input not supported");
} }
private Object parseString(String value, Type type) private Object parseString(String value, Type type)

View File

@ -32,7 +32,7 @@ public class ApiUtil {
public static RestApiException asRestApiException(String msg, Exception e) public static RestApiException asRestApiException(String msg, Exception e)
throws RuntimeException { throws RuntimeException {
Throwables.throwIfUnchecked(e); Throwables.throwIfUnchecked(e);
return e instanceof RestApiException ? (RestApiException) e : new RestApiException(msg, e); return e instanceof RestApiException ? (RestApiException) e : RestApiException.wrap(msg, e);
} }
private ApiUtil() {} private ApiUtil() {}

View File

@ -69,7 +69,11 @@ public class PluginApiImpl implements PluginApi {
@Override @Override
public void disable() throws RestApiException { public void disable() throws RestApiException {
disable.apply(resource, new Input()); try {
disable.apply(resource, new Input());
} catch (Exception e) {
throw asRestApiException("Cannot disable plugin", e);
}
} }
@Override @Override

View File

@ -156,7 +156,7 @@ class ProjectsImpl implements Projects {
.withStart(r.getStart()) .withStart(r.getStart())
.apply(); .apply();
} catch (StorageException e) { } catch (StorageException e) {
throw new RestApiException("Cannot query projects", e); throw asRestApiException("Cannot query projects", e);
} }
} }
} }

View File

@ -293,7 +293,7 @@ class ReceiveCommits {
} else if ((e instanceof ExecutionException) && (e.getCause() instanceof RestApiException)) { } else if ((e instanceof ExecutionException) && (e.getCause() instanceof RestApiException)) {
return (RestApiException) e.getCause(); return (RestApiException) e.getCause();
} }
return new RestApiException("Error inserting change/patchset", e); return RestApiException.wrap("Error inserting change/patchset", e);
} }
// ReceiveCommits has a lot of fields, sorry. Here and in the constructor they are split up // ReceiveCommits has a lot of fields, sorry. Here and in the constructor they are split up

View File

@ -45,12 +45,9 @@ public class DisablePlugin implements RestModifyView<PluginResource, Input> {
} }
@Override @Override
public Response<PluginInfo> apply(PluginResource resource, Input input) throws RestApiException { public Response<PluginInfo> apply(PluginResource resource, Input input)
try { throws RestApiException, PermissionBackendException {
permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER); permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
} catch (PermissionBackendException e) {
throw new RestApiException("Could not check permission", e);
}
loader.checkRemoteAdminEnabled(); loader.checkRemoteAdminEnabled();
String name = resource.getName(); String name = resource.getName();
if (mandatoryPluginsCollection.contains(name)) { if (mandatoryPluginsCollection.contains(name)) {

View File

@ -212,12 +212,10 @@ public class Submit
return Response.ok(new Output(change)); return Response.ok(new Output(change));
} }
String msg = throw new IllegalStateException(
String.format( String.format(
"change %s of project %s unexpectedly had status %s after submit attempt", "change %s of project %s unexpectedly had status %s after submit attempt",
updatedChange.getId(), updatedChange.getProject(), updatedChange.getStatus()); updatedChange.getId(), updatedChange.getProject(), updatedChange.getStatus()));
logger.atWarning().log(msg);
throw new RestApiException(msg);
} }
} }

View File

@ -17,7 +17,7 @@ package com.google.gerrit.server.restapi.project;
import com.google.gerrit.extensions.api.projects.DashboardInfo; import com.google.gerrit.extensions.api.projects.DashboardInfo;
import com.google.gerrit.extensions.api.projects.SetDashboardInput; import com.google.gerrit.extensions.api.projects.SetDashboardInput;
import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView; import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
@ -48,7 +48,7 @@ public class CreateDashboard
throws RestApiException, IOException, PermissionBackendException { throws RestApiException, IOException, PermissionBackendException {
parent.getProjectState().checkStatePermitsWrite(); parent.getProjectState().checkStatePermitsWrite();
if (!DashboardsCollection.isDefaultDashboard(id)) { if (!DashboardsCollection.isDefaultDashboard(id)) {
throw new ResourceNotFoundException(id); throw new MethodNotAllowedException("cannot create non-default dashboard");
} }
SetDefaultDashboard set = setDefault.get(); SetDefaultDashboard set = setDefault.get();
set.inherited = inherited; set.inherited = inherited;

View File

@ -43,6 +43,6 @@ public class DeleteDashboard implements RestModifyView<DashboardResource, SetDas
} }
// TODO: Implement delete of dashboards by API. // TODO: Implement delete of dashboards by API.
throw new MethodNotAllowedException(); throw new MethodNotAllowedException("cannot delete non-default dashboard");
} }
} }

View File

@ -40,7 +40,7 @@ public class SetDashboard implements RestModifyView<DashboardResource, SetDashbo
return defaultSetter.get().apply(resource, input); return defaultSetter.get().apply(resource, input);
} }
// TODO: Implement creation/update of dashboards by API. // TODO: Implement update of dashboards by API.
throw new MethodNotAllowedException(); throw new MethodNotAllowedException("cannot update non-default dashboard");
} }
} }

@ -1 +1 @@
Subproject commit 828d666bbb4aae1a2c348a12d7855ec5db3be46f Subproject commit 2933add62ecf2cbfc28cfe2cff81ff0e0eecc913