opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15'

Browse Source 
* stable-2.15:
  InitExperimental: Use com.google.inject instead of javax.inject
  AccountValidator: Use com.google.inject instead of javax.inject
  CreateRefControl: Use com.google.inject instead of javax.inject
  PutMessage: Use com.google.inject instead of javax.inject
  Fix deletion of meta branches through Delete Branch API

Change-Id: I1dc0c51853855ffb84af57ef2c03c0bf1ce000ca
This commit is contained in:
David Pursehouse
2017-11-13 16:59:50 +09:00
parent 7d35ff3fbe f851746c4b
commit 8732b066bc
7 changed files with 80 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/pgm/init/InitExperimental.java
View File

@@ -19,8 +19,8 @@ import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.Section;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Locale;
import javax.inject.Singleton;
@Singleton
class InitExperimental implements InitStep {

6
java/com/google/gerrit/server/change/PutMessage.java
View File

@@ -42,13 +42,13 @@ import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.List;
import java.util.TimeZone;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;

2
java/com/google/gerrit/server/git/validators/AccountValidator.java
View File

@@ -20,11 +20,11 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountConfig;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import javax.inject.Inject;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevWalk;

4
java/com/google/gerrit/server/project/CreateRefControl.java
View File

@@ -21,10 +21,10 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;

5
java/com/google/gerrit/server/project/DeleteRef.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.project;
import static java.lang.String.format;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.R_REFS;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
import static org.eclipse.jgit.transport.ReceiveCommand.Type.DELETE;
@@ -118,7 +119,7 @@ public class DeleteRef {
private void deleteSingleRef(Repository r) throws IOException, ResourceConflictException {
String ref = refsToDelete.get(0);
if (prefix != null && !ref.startsWith(prefix)) {
if (prefix != null && !ref.startsWith(R_REFS)) {
ref = prefix + ref;
}
RefUpdate.Result result;
@@ -186,7 +187,7 @@ public class DeleteRef {
? refsToDelete
: refsToDelete
.stream()
.map(ref -> ref.startsWith(prefix) ? ref : prefix + ref)
.map(ref -> ref.startsWith(R_REFS) ? ref : prefix + ref)
.collect(toList());
for (String ref : refs) {
batchUpdate.addCommand(createDeleteCommand(resource, r, ref));

77
javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
View File

@@ -25,45 +25,46 @@ import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.RefNames;
import org.junit.Before;
import org.junit.Test;
public class DeleteBranchIT extends AbstractDaemonTest {
private Branch.NameKey branch;
private Branch.NameKey testBranch;
@Before
public void setUp() throws Exception {
project = createProject(name("p"));
branch = new Branch.NameKey(project, "test");
branch().create(new BranchInput());
testBranch = new Branch.NameKey(project, "test");
branch(testBranch).create(new BranchInput());
}
@Test
public void deleteBranch_Forbidden() throws Exception {
setApiUser(user);
assertDeleteForbidden();
assertDeleteForbidden(testBranch);
}
@Test
public void deleteBranchByAdmin() throws Exception {
assertDeleteSucceeds();
assertDeleteSucceeds(testBranch);
}
@Test
public void deleteBranchByProjectOwner() throws Exception {
grantOwner();
setApiUser(user);
assertDeleteSucceeds();
assertDeleteSucceeds(testBranch);
}
@Test
public void deleteBranchByAdminForcePushBlocked() throws Exception {
blockForcePush();
assertDeleteSucceeds();
assertDeleteSucceeds(testBranch);
}
@Test
@@ -71,44 +72,57 @@ public class DeleteBranchIT extends AbstractDaemonTest {
grantOwner();
blockForcePush();
setApiUser(user);
assertDeleteForbidden();
assertDeleteForbidden(testBranch);
}
@Test
public void deleteBranchByUserWithForcePushPermission() throws Exception {
grantForcePush();
setApiUser(user);
assertDeleteSucceeds();
assertDeleteSucceeds(testBranch);
}
@Test
public void deleteBranchByUserWithDeletePermission() throws Exception {
grantDelete();
setApiUser(user);
assertDeleteSucceeds();
assertDeleteSucceeds(testBranch);
}
@Test
public void deleteBranchByRestWithoutRefsHeadsPrefix() throws Exception {
grantDelete();
String ref = branch.getShortName();
String ref = testBranch.getShortName();
assertThat(ref).doesNotMatch(R_HEADS);
assertDeleteByRestSucceeds(ref);
assertDeleteByRestSucceeds(testBranch, ref);
}
@Test
public void deleteBranchByRestWithEncodedFullName() throws Exception {
public void deleteBranchByRestWithFullName() throws Exception {
grantDelete();
assertDeleteByRestSucceeds(Url.encode(branch.get()));
assertDeleteByRestSucceeds(testBranch, testBranch.get());
}
@Test
public void deleteBranchByRestFailsWithUnencodedFullName() throws Exception {
grantDelete();
RestResponse r =
userRestSession.delete("/projects/" + project.get() + "/branches/" + branch.get());
userRestSession.delete("/projects/" + project.get() + "/branches/" + testBranch.get());
r.assertNotFound();
branch().get();
branch(testBranch).get();
}
@Test
public void deleteMetaBranch() throws Exception {
String metaRef = RefNames.REFS_META + "foo";
allow(metaRef, Permission.CREATE, REGISTERED_USERS);
allow(metaRef, Permission.PUSH, REGISTERED_USERS);
Branch.NameKey metaBranch = new Branch.NameKey(project, metaRef);
branch(metaBranch).create(new BranchInput());
grantDelete();
assertDeleteByRestSucceeds(metaBranch, metaRef);
}
private void blockForcePush() throws Exception {
@@ -127,31 +141,36 @@ public class DeleteBranchIT extends AbstractDaemonTest {
allow("refs/*", Permission.OWNER, REGISTERED_USERS);
}
private BranchApi branch() throws Exception {
private BranchApi branch(Branch.NameKey branch) throws Exception {
return gApi.projects().name(branch.getParentKey().get()).branch(branch.get());
}
private void assertDeleteByRestSucceeds(String ref) throws Exception {
RestResponse r = userRestSession.delete("/projects/" + project.get() + "/branches/" + ref);
private void assertDeleteByRestSucceeds(Branch.NameKey branch, String ref) throws Exception {
RestResponse r =
userRestSession.delete(
"/projects/"
+ IdString.fromDecoded(project.get()).encoded()
+ "/branches/"
+ IdString.fromDecoded(ref).encoded());
r.assertNoContent();
exception.expect(ResourceNotFoundException.class);
branch().get();
branch(branch).get();
}
private void assertDeleteSucceeds() throws Exception {
assertThat(branch().get().canDelete).isTrue();
String branchRev = branch().get().revision;
branch().delete();
private void assertDeleteSucceeds(Branch.NameKey branch) throws Exception {
assertThat(branch(branch).get().canDelete).isTrue();
String branchRev = branch(branch).get().revision;
branch(branch).delete();
eventRecorder.assertRefUpdatedEvents(
project.get(), branch.get(), null, branchRev, branchRev, null);
exception.expect(ResourceNotFoundException.class);
branch().get();
branch(branch).get();
}
private void assertDeleteForbidden() throws Exception {
assertThat(branch().get().canDelete).isNull();
private void assertDeleteForbidden(Branch.NameKey branch) throws Exception {
assertThat(branch(branch).get().canDelete).isNull();
exception.expect(AuthException.class);
exception.expectMessage("delete not permitted");
branch().delete();
branch(branch).delete();
}
}

31
javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
View File

@@ -15,14 +15,16 @@
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 com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.eclipse.jgit.lib.Constants.R_REFS;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.api.projects.ProjectApi;
@@ -31,6 +33,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.RefNames;
import java.util.HashMap;
import java.util.List;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -38,10 +41,12 @@ 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", "test-3");
ImmutableList.of("refs/heads/test-1", "refs/heads/test-2", "test-3", "refs/meta/foo");
@Before
public void setUp() throws Exception {
allow("refs/*", Permission.CREATE, REGISTERED_USERS);
allow("refs/*", Permission.PUSH, REGISTERED_USERS);
for (String name : BRANCHES) {
project().branch(name).create(new BranchInput());
}
@@ -54,7 +59,7 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
DeleteBranchesInput input = new DeleteBranchesInput();
input.branches = BRANCHES;
project().deleteBranches(input);
assertBranchesDeleted();
assertBranchesDeleted(BRANCHES);
assertRefUpdatedEvents(initialRevisions);
}
@@ -87,7 +92,7 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
.hasMessageThat()
.isEqualTo(errorMessageForBranches(ImmutableList.of("refs/heads/does-not-exist")));
}
assertBranchesDeleted();
assertBranchesDeleted(BRANCHES);
}
@Test
@@ -106,7 +111,7 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
.hasMessageThat()
.isEqualTo(errorMessageForBranches(ImmutableList.of("refs/heads/does-not-exist")));
}
assertBranchesDeleted();
assertBranchesDeleted(BRANCHES);
}
@Test
@@ -163,7 +168,7 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
}
private String prefixRef(String ref) {
return ref.startsWith(R_HEADS) ? ref : R_HEADS + ref;
return ref.startsWith(R_REFS) ? ref : R_HEADS + ref;
}
private ProjectApi project() throws Exception {
@@ -173,10 +178,18 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
private void assertBranches(List<String> branches) throws Exception {
List<String> expected = Lists.newArrayList("HEAD", RefNames.REFS_CONFIG, "refs/heads/master");
expected.addAll(branches.stream().map(b -> prefixRef(b)).collect(toList()));
assertRefNames(expected, project().branches().get());
try (Repository repo = repoManager.openRepository(project)) {
for (String branch : expected) {
assertThat(repo.exactRef(branch)).isNotNull();
}
}
}
private void assertBranchesDeleted() throws Exception {
assertBranches(ImmutableList.<String>of());
private void assertBranchesDeleted(List<String> branches) throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
for (String branch : branches) {
assertThat(repo.exactRef(branch)).isNull();
}
}
}
}