DeleteBranch{es}: Fix deletion of branch without refs/heads/ prefix
When creating the DeleteRef, the prefix should be provided.
Also update the tests to make sure it works. For DeleteBranches this
is done by amending the list of inputs to include a branch that does
not have the refs/heads prefix.
For DeleteBranch, which uses the API, we need to introduce another
test that explicitly uses the REST API. This means we can no longer
annotate it as @NoHttpd.
Bug: Issue 6591
Change-Id: I0d639d2cc261a51086a36a8fb1967b121b508f4f
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
index bf08ac9..bcc6766 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
@@ -14,11 +14,13 @@
package com.google.gerrit.acceptance.rest.project;
+import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static org.eclipse.jgit.lib.Constants.R_HEADS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -28,7 +30,6 @@
import org.junit.Before;
import org.junit.Test;
-@NoHttpd
public class DeleteBranchIT extends AbstractDaemonTest {
private Branch.NameKey branch;
@@ -86,6 +87,15 @@
assertDeleteSucceeds();
}
+ @Test
+ public void deleteBranchByRestWithoutRefsHeadsPrefix() throws Exception {
+ grantDelete();
+ String ref = branch.getShortName();
+ assertThat(ref).doesNotMatch(R_HEADS);
+ RestResponse r = userRestSession.delete("/projects/" + project.get() + "/branches/" + ref);
+ r.assertNoContent();
+ }
+
private void blockForcePush() throws Exception {
block(Permission.PUSH, ANONYMOUS_USERS, "refs/heads/*").setForce(true);
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
index 1ca6c15..dc18a58 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
@@ -16,6 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.rest.project.RefAssert.assertRefNames;
+import static java.util.stream.Collectors.toList;
+import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
@@ -37,7 +39,7 @@
@NoHttpd
public class DeleteBranchesIT extends AbstractDaemonTest {
private static final ImmutableList<String> BRANCHES =
- ImmutableList.of("refs/heads/test-1", "refs/heads/test-2", "refs/heads/test-3");
+ ImmutableList.of("refs/heads/test-1", "refs/heads/test-2", "test-3");
@Before
public void setUp() throws Exception {
@@ -138,7 +140,7 @@
for (String branch : branches) {
message
.append("Cannot delete ")
- .append(branch)
+ .append(prefixRef(branch))
.append(": it doesn't exist or you do not have permission ")
.append("to delete it\n");
}
@@ -156,17 +158,22 @@
private void assertRefUpdatedEvents(HashMap<String, RevCommit> revisions) throws Exception {
for (String branch : revisions.keySet()) {
RevCommit revision = revisions.get(branch);
- eventRecorder.assertRefUpdatedEvents(project.get(), branch, null, revision, revision, null);
+ eventRecorder.assertRefUpdatedEvents(
+ project.get(), prefixRef(branch), null, revision, revision, null);
}
}
+ private String prefixRef(String ref) {
+ return ref.startsWith(R_HEADS) ? ref : R_HEADS + ref;
+ }
+
private ProjectApi project() throws Exception {
return gApi.projects().name(project.get());
}
private void assertBranches(List<String> branches) throws Exception {
List<String> expected = Lists.newArrayList("HEAD", RefNames.REFS_CONFIG, "refs/heads/master");
- expected.addAll(branches);
+ expected.addAll(branches.stream().map(b -> prefixRef(b)).collect(toList()));
assertRefNames(expected, project().branches().get());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
index 4601bfe..049e2e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
+import static org.eclipse.jgit.lib.Constants.R_HEADS;
+
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -51,7 +53,7 @@
throw new ResourceConflictException("branch " + rsrc.getBranchKey() + " has open changes");
}
- deleteRefFactory.create(rsrc).ref(rsrc.getRef()).delete();
+ deleteRefFactory.create(rsrc).ref(rsrc.getRef()).prefix(R_HEADS).delete();
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
index f5e55b1..4c54423 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
+import static org.eclipse.jgit.lib.Constants.R_HEADS;
+
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
@@ -41,7 +43,7 @@
throw new BadRequestException("branches must be specified");
}
- deleteRefFactory.create(project).refs(input.branches).delete();
+ deleteRefFactory.create(project).refs(input.branches).prefix(R_HEADS).delete();
return Response.none();
}
}
