opendev/gerrit
Code Issues Proposed changes

RefOperationValidationIT: Use ExtensionRegistry for RefOperationValidationListener

Browse Source 
Add the necessary support in ExtensionRegistry and modify the tests
to use it.

Change-Id: I6150e1b4e1ed2e0f7ac3ae572a5833aacce478bc
This commit is contained in:
David Pursehouse
2019-10-10 15:22:48 +09:00
parent 05e51038ec
commit 76e22cfbcf
2 changed files with 23 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
java/com/google/gerrit/acceptance/ExtensionRegistry.java
View File

@@ -25,6 +25,7 @@ import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
@@ -44,6 +45,7 @@ public class ExtensionRegistry {
private final DynamicSet<ChangeETagComputation> changeETagComputations;
private final DynamicSet<ActionVisitor> actionVisitors;
private final DynamicMap<DownloadScheme> downloadSchemes;
private final DynamicSet<RefOperationValidationListener> refOperationValidationListeners;
@Inject
ExtensionRegistry(
@@ -56,7 +58,8 @@ public class ExtensionRegistry {
DynamicSet<ChangeMessageModifier> changeMessageModifiers,
DynamicSet<ChangeETagComputation> changeETagComputations,
DynamicSet<ActionVisitor> actionVisitors,
DynamicMap<DownloadScheme> downloadSchemes) {
DynamicMap<DownloadScheme> downloadSchemes,
DynamicSet<RefOperationValidationListener> refOperationValidationListeners) {
this.changeIndexedListeners = changeIndexedListeners;
this.commitValidationListeners = commitValidationListeners;
this.exceptionHooks = exceptionHooks;
@@ -67,6 +70,7 @@ public class ExtensionRegistry {
this.changeETagComputations = changeETagComputations;
this.actionVisitors = actionVisitors;
this.downloadSchemes = downloadSchemes;
this.refOperationValidationListeners = refOperationValidationListeners;
}
public Registration newRegistration() {
@@ -116,6 +120,10 @@ public class ExtensionRegistry {
return add(downloadSchemes, downloadScheme, exportName);
}
public Registration add(RefOperationValidationListener refOperationValidationListener) {
return add(refOperationValidationListeners, refOperationValidationListener);
}
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
RegistrationHandle registrationHandle = dynamicSet.add("gerrit", extension);
registrationHandles.add(registrationHandle);

31
javatests/com/google/gerrit/acceptance/git/RefOperationValidationIT.java
View File

@@ -26,12 +26,12 @@ import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE;
import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE_NONFASTFORWARD;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.events.RefReceivedEvent;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
@@ -50,18 +50,16 @@ import org.junit.Test;
public class RefOperationValidationIT extends AbstractDaemonTest {
private static final String TEST_REF = "refs/heads/protected";
@Inject DynamicSet<RefOperationValidationListener> validators;
@Inject private ProjectOperations projectOperations;
@Inject private ExtensionRegistry extensionRegistry;
private class TestRefValidator implements RefOperationValidationListener, AutoCloseable {
private static class TestRefValidator implements RefOperationValidationListener {
private final ReceiveCommand.Type rejectType;
private final String rejectRef;
private final RegistrationHandle handle;
public TestRefValidator(ReceiveCommand.Type rejectType) {
this.rejectType = rejectType;
this.rejectRef = TEST_REF;
this.handle = validators.add("test-" + rejectType.name(), this);
}
@Override
@@ -73,16 +71,15 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
}
return Collections.emptyList();
}
}
@Override
public void close() throws Exception {
handle.remove();
}
private Registration testValidator(ReceiveCommand.Type rejectType) {
return extensionRegistry.newRegistration().add(new TestRefValidator(rejectType));
}
@Test
public void rejectRefCreation() throws Exception {
try (TestRefValidator validator = new TestRefValidator(CREATE)) {
try (Registration registration = testValidator(CREATE)) {
RestApiException expected =
assertThrows(
RestApiException.class,
@@ -101,7 +98,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
@Test
public void rejectRefCreationByPush() throws Exception {
try (TestRefValidator validator = new TestRefValidator(CREATE)) {
try (Registration registration = testValidator(CREATE)) {
grant(Permission.PUSH);
PushOneCommit push1 =
pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
@@ -115,7 +112,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
@Test
public void rejectRefDeletion() throws Exception {
gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
try (TestRefValidator validator = new TestRefValidator(DELETE)) {
try (Registration registration = testValidator(DELETE)) {
RestApiException expected =
assertThrows(
RestApiException.class,
@@ -128,7 +125,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
public void rejectRefDeletionByPush() throws Exception {
gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
grant(Permission.DELETE);
try (TestRefValidator validator = new TestRefValidator(DELETE)) {
try (Registration registration = testValidator(DELETE)) {
PushResult result = deleteRef(testRepo, TEST_REF);
RemoteRefUpdate refUpdate = result.getRemoteUpdate(TEST_REF);
assertThat(refUpdate.getMessage()).contains(DELETE.name());
@@ -138,7 +135,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
@Test
public void rejectRefUpdateFastForward() throws Exception {
gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
try (TestRefValidator validator = new TestRefValidator(UPDATE)) {
try (Registration registration = testValidator(UPDATE)) {
grant(Permission.PUSH);
PushOneCommit push1 =
pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
@@ -150,7 +147,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
@Test
public void rejectRefUpdateNonFastForward() throws Exception {
gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
try (TestRefValidator validator = new TestRefValidator(UPDATE_NONFASTFORWARD)) {
try (Registration registration = testValidator(UPDATE_NONFASTFORWARD)) {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
grant(Permission.PUSH);
PushOneCommit push1 =
@@ -175,7 +172,7 @@ public class RefOperationValidationIT extends AbstractDaemonTest {
public void rejectRefUpdateNonFastForwardToExistingCommit() throws Exception {
gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
try (TestRefValidator validator = new TestRefValidator(UPDATE_NONFASTFORWARD)) {
try (Registration registration = testValidator(UPDATE_NONFASTFORWARD)) {
grant(Permission.PUSH);
PushOneCommit push1 =
pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");