Check preconditions when setting parent project through REST

Only update the parent project if the new parent project exists and if
setting this parent doesn't introduce a cycle in the line of projects.
Never set a parent project for the All-Projects root project.

Change-Id: I9caec6921d46f8205febdacb31f52bd6c21f28c8
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-10-17 13:49:32 +02:00
parent 0f07b86d75
commit 42f7bc3323
2 changed files with 82 additions and 7 deletions

View File

@@ -24,6 +24,8 @@ import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession; import com.google.gerrit.acceptance.RestSession;
import com.google.gerrit.acceptance.SshSession; import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gson.Gson; import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken; import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -42,6 +44,9 @@ public class SetParentIT extends AbstractDaemonTest {
@Inject @Inject
private AccountCreator accounts; private AccountCreator accounts;
@Inject
private AllProjectsNameProvider allProjects;
private RestSession adminSession; private RestSession adminSession;
private RestSession userSession; private RestSession userSession;
private SshSession sshSession; private SshSession sshSession;
@@ -98,6 +103,47 @@ public class SetParentIT extends AbstractDaemonTest {
r.consume(); r.consume();
} }
@Test
public void setParentForAllProjects_Conflict() throws IOException {
RestResponse r =
adminSession.put("/projects/" + allProjects.get() + "/parent",
new ParentInput(project));
assertEquals(HttpStatus.SC_CONFLICT, r.getStatusCode());
r.consume();
}
@Test
public void setInvalidParent_Conflict() throws IOException, JSchException {
RestResponse r =
adminSession.put("/projects/" + project + "/parent",
new ParentInput(project));
assertEquals(HttpStatus.SC_CONFLICT, r.getStatusCode());
r.consume();
String child = "child";
createProject(sshSession, child, new Project.NameKey(project), true);
r = adminSession.put("/projects/" + project + "/parent",
new ParentInput(child));
assertEquals(HttpStatus.SC_CONFLICT, r.getStatusCode());
r.consume();
String grandchild = "grandchild";
createProject(sshSession, grandchild, new Project.NameKey(child), true);
r = adminSession.put("/projects/" + project + "/parent",
new ParentInput(grandchild));
assertEquals(HttpStatus.SC_CONFLICT, r.getStatusCode());
r.consume();
}
@Test
public void setNonExistingParent_UnprocessibleEntity() throws IOException {
RestResponse r =
adminSession.put("/projects/" + project + "/parent",
new ParentInput("non-existing"));
assertEquals(HttpStatus.SC_UNPROCESSABLE_ENTITY, r.getStatusCode());
r.consume();
}
@SuppressWarnings("unused") @SuppressWarnings("unused")
private static class ParentInput { private static class ParentInput {
String parent; String parent;

View File

@@ -15,13 +15,16 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import com.google.common.base.Objects; import com.google.common.base.Objects;
import com.google.common.base.Predicate;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsName;
@@ -33,6 +36,8 @@ import com.google.inject.Inject;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.RepositoryNotFoundException;
import java.io.IOException;
class SetParent implements RestModifyView<ProjectResource, Input> { class SetParent implements RestModifyView<ProjectResource, Input> {
static class Input { static class Input {
@DefaultInput @DefaultInput
@@ -54,21 +59,45 @@ class SetParent implements RestModifyView<ProjectResource, Input> {
} }
@Override @Override
public String apply(ProjectResource resource, Input input) public String apply(final ProjectResource rsrc, Input input) throws AuthException,
throws AuthException, BadRequestException, ResourceConflictException, BadRequestException, ResourceConflictException,
Exception { ResourceNotFoundException, UnprocessableEntityException, IOException {
ProjectControl ctl = resource.getControl(); ProjectControl ctl = rsrc.getControl();
IdentifiedUser user = (IdentifiedUser) ctl.getCurrentUser(); IdentifiedUser user = (IdentifiedUser) ctl.getCurrentUser();
if (!user.getCapabilities().canAdministrateServer()) { if (!user.getCapabilities().canAdministrateServer()) {
throw new AuthException("not administrator"); throw new AuthException("not administrator");
} }
if (rsrc.getNameKey().equals(allProjects)) {
throw new ResourceConflictException("cannot set parent of "
+ allProjects.get());
}
input.parent = Strings.emptyToNull(input.parent);
if (input.parent != null) {
ProjectState parent = cache.get(new Project.NameKey(input.parent));
if (parent == null) {
throw new UnprocessableEntityException("parent project " + input.parent
+ " not found");
}
if (Iterables.tryFind(parent.tree(), new Predicate<ProjectState>() {
@Override
public boolean apply(ProjectState input) {
return input.getProject().getNameKey().equals(rsrc.getNameKey());
}
}).isPresent()) {
throw new ResourceConflictException("cycle exists between "
+ rsrc.getName() + " and " + parent.getProject().getName());
}
}
try { try {
MetaDataUpdate md = updateFactory.create(resource.getNameKey()); MetaDataUpdate md = updateFactory.create(rsrc.getNameKey());
try { try {
ProjectConfig config = ProjectConfig.read(md); ProjectConfig config = ProjectConfig.read(md);
Project project = config.getProject(); Project project = config.getProject();
project.setParentName(Strings.emptyToNull(input.parent)); project.setParentName(input.parent);
String msg = Strings.emptyToNull(input.commitMessage); String msg = Strings.emptyToNull(input.commitMessage);
if (msg == null) { if (msg == null) {
@@ -89,7 +118,7 @@ class SetParent implements RestModifyView<ProjectResource, Input> {
md.close(); md.close();
} }
} catch (RepositoryNotFoundException notFound) { } catch (RepositoryNotFoundException notFound) {
throw new ResourceNotFoundException(resource.getName()); throw new ResourceNotFoundException(rsrc.getName());
} catch (ConfigInvalidException e) { } catch (ConfigInvalidException e) {
throw new ResourceConflictException(String.format( throw new ResourceConflictException(String.format(
"invalid project.config: %s", e.getMessage())); "invalid project.config: %s", e.getMessage()));