Return proper HTTP status codes when project creation fails

So far all errors during project creation resulted in 500 Internal
Server Error. Now the proper HTTP status codes are returned.

Change-Id: Id6d7a5394c9be6656abca652ef3b9025b7df5c0a
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2015-04-07 16:19:17 +02:00
parent 3c6960b699
commit 076ad18921
5 changed files with 68 additions and 46 deletions

View File

@@ -65,7 +65,20 @@ public class RestSession extends HttpSession {
}
public RestResponse put(String endPoint, Object content) throws IOException {
return putWithHeader(endPoint, null, content);
}
public RestResponse putWithHeader(String endPoint, Header header)
throws IOException {
return putWithHeader(endPoint, header, null);
}
public RestResponse putWithHeader(String endPoint, Header header,
Object content) throws IOException {
HttpPut put = new HttpPut(url + "/a" + endPoint);
if (header != null) {
put.addHeader(header);
}
if (content != null) {
put.addHeader(new BasicHeader("Content-Type", "application/json"));
put.setEntity(new StringEntity(

View File

@@ -21,8 +21,10 @@ import static org.junit.Assert.fail;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.net.HttpHeaders;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
@@ -31,6 +33,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -38,6 +41,7 @@ import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectState;
import org.apache.http.HttpStatus;
import org.apache.http.message.BasicHeader;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
@@ -64,6 +68,29 @@ public class CreateProjectIT extends AbstractDaemonTest {
assertHead(newProjectName, "refs/heads/master");
}
@Test
public void testCreateProjectHttpWhenProjectAlreadyExists_Conflict()
throws Exception {
RestResponse r = adminSession.put("/projects/" + allProjects.get());
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_CONFLICT);
}
@Test
public void testCreateProjectHttpWhenProjectAlreadyExists_PreconditionFailed()
throws Exception {
RestResponse r = adminSession.putWithHeader("/projects/" + allProjects.get(),
new BasicHeader(HttpHeaders.IF_NONE_MATCH, "*"));
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_PRECONDITION_FAILED);
}
@Test
@UseLocalDisk
public void testCreateProjectHttpWithUnreasonableName_BadRequest()
throws Exception {
RestResponse r = adminSession.put("/projects/" + Url.encode(name("invalid/../name")));
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST);
}
@Test
public void testCreateProjectHttpWithNameMismatch_BadRequest() throws Exception {
ProjectInput in = new ProjectInput();
@@ -72,6 +99,15 @@ public class CreateProjectIT extends AbstractDaemonTest {
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST);
}
@Test
public void testCreateProjectHttpWithInvalidRefName_BadRequest()
throws Exception {
ProjectInput in = new ProjectInput();
in.branches = Collections.singletonList(name("invalid ref name"));
RestResponse r = adminSession.put("/projects/" + name("newProject"), in);
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST);
}
@Test
public void testCreateProject() throws Exception {
String newProjectName = name("newProject");

View File

@@ -1,29 +0,0 @@
// Copyright (C) 2011 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.common.errors;
/** Error indicating failed to create new project. */
public class ProjectCreationFailedException extends Exception {
private static final long serialVersionUID = 1L;
public ProjectCreationFailedException(final String message) {
this(message, null);
}
public ProjectCreationFailedException(final String message,
final Throwable why) {
super(message, why);
}
}

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.api.projects;
import static com.google.gerrit.server.account.CapabilityUtils.checkRequiresCapability;
import com.google.gerrit.common.errors.ProjectCreationFailedException;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.ChildProjectApi;
@@ -44,6 +43,8 @@ import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.errors.ConfigInvalidException;
import java.io.IOException;
import java.util.List;
@@ -149,7 +150,7 @@ public class ProjectApiImpl implements ProjectApi {
createProjectFactory.get().create(name)
.apply(TopLevelResource.INSTANCE, in);
return projectApi.create(projects.parse(name));
} catch (ProjectCreationFailedException | IOException e) {
} catch (IOException | ConfigInvalidException e) {
throw new RestApiException("Cannot create project: " + e.getMessage(), e);
}
}

View File

@@ -24,7 +24,6 @@ import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.ProjectCreationFailedException;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -143,10 +142,10 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
}
@Override
public Response<ProjectInfo> apply(TopLevelResource resource, ProjectInput input)
throws BadRequestException, UnprocessableEntityException,
ResourceConflictException, ProjectCreationFailedException,
ResourceNotFoundException, IOException {
public Response<ProjectInfo> apply(TopLevelResource resource,
ProjectInput input) throws BadRequestException,
UnprocessableEntityException, ResourceConflictException,
ResourceNotFoundException, IOException, ConfigInvalidException {
if (input == null) {
input = new ProjectInput();
}
@@ -222,7 +221,9 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
return Response.created(json.format(p));
}
public Project createProject(CreateProjectArgs args) throws ProjectCreationFailedException {
public Project createProject(CreateProjectArgs args)
throws BadRequestException, ResourceConflictException, IOException,
ConfigInvalidException {
final Project.NameKey nameKey = args.getProject();
try {
final String head =
@@ -265,18 +266,18 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
repo.close();
}
} catch (RepositoryCaseMismatchException e) {
throw new ProjectCreationFailedException("Cannot create " + nameKey.get()
throw new ResourceConflictException("Cannot create " + nameKey.get()
+ " because the name is already occupied by another project."
+ " The other project has the same name, only spelled in a"
+ " different case.", e);
+ " different case.");
} catch (RepositoryNotFoundException badName) {
throw new ProjectCreationFailedException("Cannot create " + nameKey, badName);
throw new BadRequestException("invalid project name: " + nameKey);
} catch (IllegalStateException err) {
try {
Repository repo = repoManager.openRepository(nameKey);
try {
if (repo.getObjectDatabase().exists()) {
throw new ProjectCreationFailedException("project \"" + nameKey + "\" exists");
throw new ResourceConflictException("project \"" + nameKey + "\" exists");
}
throw err;
} finally {
@@ -285,12 +286,12 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
} catch (IOException ioErr) {
String msg = "Cannot create " + nameKey;
log.error(msg, err);
throw new ProjectCreationFailedException(msg, ioErr);
throw ioErr;
}
} catch (Exception e) {
} catch (ConfigInvalidException e) {
String msg = "Cannot create " + nameKey;
log.error(msg, e);
throw new ProjectCreationFailedException(msg, e);
throw e;
}
}
@@ -341,7 +342,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
}
private List<String> normalizeBranchNames(List<String> branches)
throws ProjectCreationFailedException {
throws BadRequestException {
if (branches == null || branches.isEmpty()) {
return Collections.singletonList(Constants.R_HEADS + Constants.MASTER);
}
@@ -355,7 +356,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
branch = Constants.R_HEADS + branch;
}
if (!Repository.isValidRefName(branch)) {
throw new ProjectCreationFailedException(String.format(
throw new BadRequestException(String.format(
"Branch \"%s\" is not a valid name.", branch));
}
if (!normalizedBranches.contains(branch)) {