opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'delete-refs' into stable-2.14

Browse Source 
* changes:
  DeleteTag{s}IT: Add tests for deleting tag without refs/tags prefix
  DeleteBranch{es}: Fix deletion of branch without refs/heads/ prefix
This commit is contained in:
Edwin Kempin
2017-06-29 06:03:29 +00:00
committed by Gerrit Code Review
parent ad3d6c1120 ea2d38ca9d
commit f714099f24
6 changed files with 55 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
View File

@@ -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 com.google.gerrit.reviewdb.client.Branch;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class DeleteBranchIT extends AbstractDaemonTest {
private Branch.NameKey branch;
@@ -86,6 +87,15 @@ public class DeleteBranchIT extends AbstractDaemonTest {
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);
}

15
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
View File

@@ -16,6 +16,8 @@ package com.google.gerrit.acceptance.rest.project;
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 @@ import org.junit.Test;
@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 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
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 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
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());
}

12
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java
View File

@@ -16,9 +16,10 @@ package com.google.gerrit.acceptance.rest.project;
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_TAGS;
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.TagApi;
import com.google.gerrit.extensions.api.projects.TagInput;
@@ -27,7 +28,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class DeleteTagIT extends AbstractDaemonTest {
private final String TAG = "refs/tags/test";
@@ -82,6 +82,14 @@ public class DeleteTagIT extends AbstractDaemonTest {
assertDeleteSucceeds();
}
@Test
public void deleteTagByRestWithoutRefsTagsPrefix() throws Exception {
grantDelete();
String ref = TAG.substring(R_TAGS.length());
RestResponse r = userRestSession.delete("/projects/" + project.get() + "/tags/" + ref);
r.assertNoContent();
}
private void blockForcePush() throws Exception {
block(Permission.PUSH, ANONYMOUS_USERS, "refs/tags/*").setForce(true);
}

22
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagsIT.java
View File

@@ -15,6 +15,8 @@
package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
@@ -36,7 +38,7 @@ import org.junit.Test;
@NoHttpd
public class DeleteTagsIT extends AbstractDaemonTest {
private static final ImmutableList<String> TAGS =
ImmutableList.of("refs/tags/test-1", "refs/tags/test-2", "refs/tags/test-3");
ImmutableList.of("refs/tags/test-1", "refs/tags/test-2", "refs/tags/test-3", "test-4");
@Before
public void setUp() throws Exception {
@@ -112,7 +114,7 @@ public class DeleteTagsIT extends AbstractDaemonTest {
for (String tag : tags) {
message
.append("Cannot delete ")
.append(tag)
.append(prefixRef(tag))
.append(": it doesn't exist or you do not have permission ")
.append("to delete it\n");
}
@@ -122,18 +124,24 @@ public class DeleteTagsIT extends AbstractDaemonTest {
private HashMap<String, RevCommit> initialRevisions(List<String> tags) throws Exception {
HashMap<String, RevCommit> result = new HashMap<>();
for (String tag : tags) {
result.put(tag, getRemoteHead(project, tag));
String ref = prefixRef(tag);
result.put(ref, getRemoteHead(project, ref));
}
return result;
}
private void assertRefUpdatedEvents(HashMap<String, RevCommit> revisions) throws Exception {
for (String tag : revisions.keySet()) {
RevCommit revision = revisions.get(tag);
eventRecorder.assertRefUpdatedEvents(project.get(), tag, null, revision, revision, null);
RevCommit revision = revisions.get(prefixRef(tag));
eventRecorder.assertRefUpdatedEvents(
project.get(), prefixRef(tag), null, revision, revision, null);
}
}
private String prefixRef(String ref) {
return ref.startsWith(R_TAGS) ? ref : R_TAGS + ref;
}
private ProjectApi project() throws Exception {
return gApi.projects().name(project.get());
}
@@ -141,7 +149,9 @@ public class DeleteTagsIT extends AbstractDaemonTest {
private void assertTags(List<String> expected) throws Exception {
List<TagInfo> actualTags = project().tags().get();
Iterable<String> actualNames = Iterables.transform(actualTags, b -> b.ref);
assertThat(actualNames).containsExactlyElementsIn(expected).inOrder();
assertThat(actualNames)
.containsExactlyElementsIn(expected.stream().map(t -> prefixRef(t)).collect(toList()))
.inOrder();
}
private void assertTagsDeleted() throws Exception {

4
gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
View File

@@ -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 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input> {
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();
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
View File

@@ -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 @@ public class DeleteBranches implements RestModifyView<ProjectResource, DeleteBra
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();
}
}