AbstractSubmit: Use ExtensionRegistry for OnSubmitValidationListener
Add the necessary support in ExtensionRegistry and update the tests
to use it.
Change-Id: I3a7a253baa736e10ea040eef5bedc02f8737b25e
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index 32c0fd1..f9116a1 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -34,6 +34,7 @@
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.OnSubmitValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.rules.SubmitRule;
@@ -67,6 +68,7 @@
private final DynamicSet<GroupBackend> groupBackends;
private final DynamicSet<AccountActivationValidationListener>
accountActivationValidationListeners;
+ private final DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
@Inject
ExtensionRegistry(
@@ -90,7 +92,8 @@
DynamicSet<PatchSetWebLink> patchSetWebLinks,
DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
DynamicSet<GroupBackend> groupBackends,
- DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners) {
+ DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
+ DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -112,6 +115,7 @@
this.revisionCreatedListeners = revisionCreatedListeners;
this.groupBackends = groupBackends;
this.accountActivationValidationListeners = accountActivationValidationListeners;
+ this.onSubmitValidationListeners = onSubmitValidationListeners;
}
public Registration newRegistration() {
@@ -211,6 +215,10 @@
return add(accountActivationValidationListeners, accountActivationValidationListener);
}
+ public Registration add(OnSubmitValidationListener onSubmitValidationListener) {
+ return add(onSubmitValidationListeners, onSubmitValidationListener);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index c0083d2..5c537c8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -42,6 +42,8 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
@@ -118,7 +120,6 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.RefSpec;
-import org.junit.After;
import org.junit.Test;
@NoHttpd
@@ -131,21 +132,12 @@
}
@Inject private ApprovalsUtil approvalsUtil;
- @Inject private DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
@Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private IdentifiedUser.GenericFactory userFactory;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private Submit submitHandler;
-
- private RegistrationHandle onSubmitValidatorHandle;
-
- @After
- public void removeOnSubmitValidator() {
- if (onSubmitValidatorHandle != null) {
- onSubmitValidatorHandle.remove();
- }
- }
+ @Inject private ExtensionRegistry extensionRegistry;
protected abstract SubmitType getSubmitType();
@@ -821,23 +813,28 @@
@Test
public void submitWithValidation() throws Throwable {
AtomicBoolean called = new AtomicBoolean(false);
- this.addOnSubmitValidationListener(
- args -> {
- called.set(true);
- HashSet<String> refs = Sets.newHashSet(args.getCommands().keySet());
- assertThat(refs).contains("refs/heads/master");
- refs.remove("refs/heads/master");
- if (!refs.isEmpty()) {
- // Some submit strategies need to insert new patchset.
- assertThat(refs).hasSize(1);
- assertThat(refs.iterator().next()).startsWith(RefNames.REFS_CHANGES);
+ OnSubmitValidationListener listener =
+ new OnSubmitValidationListener() {
+ @Override
+ public void preBranchUpdate(Arguments args) throws ValidationException {
+ called.set(true);
+ HashSet<String> refs = Sets.newHashSet(args.getCommands().keySet());
+ assertThat(refs).contains("refs/heads/master");
+ refs.remove("refs/heads/master");
+ if (!refs.isEmpty()) {
+ // Some submit strategies need to insert new patchset.
+ assertThat(refs).hasSize(1);
+ assertThat(refs.iterator().next()).startsWith(RefNames.REFS_CHANGES);
+ }
}
- });
+ };
- PushOneCommit.Result change = createChange();
- approve(change.getChangeId());
- submit(change.getChangeId());
- assertThat(called.get()).isTrue();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ PushOneCommit.Result change = createChange();
+ approve(change.getChangeId());
+ submit(change.getChangeId());
+ assertThat(called.get()).isTrue();
+ }
}
@Test
@@ -872,34 +869,39 @@
// Since there are 2 repos, first submit attempt will fail, the second will
// succeed.
List<String> projectsCalled = new ArrayList<>(4);
- this.addOnSubmitValidationListener(
- args -> {
- String master = "refs/heads/master";
- assertThat(args.getCommands()).containsKey(master);
- ReceiveCommand cmd = args.getCommands().get(master);
- ObjectId newMasterId = cmd.getNewId();
- try (Repository repo = repoManager.openRepository(args.getProject())) {
- assertThat(repo.exactRef(master).getObjectId()).isEqualTo(cmd.getOldId());
- assertThat(args.getRef(master)).hasValue(newMasterId);
- args.getRevWalk().parseBody(args.getRevWalk().parseCommit(newMasterId));
- } catch (IOException e) {
- throw new AssertionError("failed checking new ref value", e);
+ OnSubmitValidationListener listener =
+ new OnSubmitValidationListener() {
+ @Override
+ public void preBranchUpdate(Arguments args) throws ValidationException {
+ String master = "refs/heads/master";
+ assertThat(args.getCommands()).containsKey(master);
+ ReceiveCommand cmd = args.getCommands().get(master);
+ ObjectId newMasterId = cmd.getNewId();
+ try (Repository repo = repoManager.openRepository(args.getProject())) {
+ assertThat(repo.exactRef(master).getObjectId()).isEqualTo(cmd.getOldId());
+ assertThat(args.getRef(master)).hasValue(newMasterId);
+ args.getRevWalk().parseBody(args.getRevWalk().parseCommit(newMasterId));
+ } catch (IOException e) {
+ throw new AssertionError("failed checking new ref value", e);
+ }
+ projectsCalled.add(args.getProject().get());
+ if (projectsCalled.size() == 2) {
+ throw new ValidationException("time to fail");
+ }
}
- projectsCalled.add(args.getProject().get());
- if (projectsCalled.size() == 2) {
- throw new ValidationException("time to fail");
- }
- });
- submitWithConflict(change4.getChangeId(), "time to fail");
- assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get());
- for (PushOneCommit.Result change : changes) {
- change.assertChange(Change.Status.NEW, name(topic), admin);
- }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ submitWithConflict(change4.getChangeId(), "time to fail");
+ assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get());
+ for (PushOneCommit.Result change : changes) {
+ change.assertChange(Change.Status.NEW, name(topic), admin);
+ }
- submit(change4.getChangeId());
- assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get(), keyA.get(), keyB.get());
- for (PushOneCommit.Result change : changes) {
- change.assertChange(Change.Status.MERGED, name(topic), admin);
+ submit(change4.getChangeId());
+ assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get(), keyA.get(), keyB.get());
+ for (PushOneCommit.Result change : changes) {
+ change.assertChange(Change.Status.MERGED, name(topic), admin);
+ }
}
}
@@ -1438,11 +1440,6 @@
return getRemoteLog(project, "master");
}
- protected void addOnSubmitValidationListener(OnSubmitValidationListener listener) {
- assertThat(onSubmitValidatorHandle).isNull();
- onSubmitValidatorHandle = onSubmitValidationListeners.add("gerrit", listener);
- }
-
private String getLatestDiff(Repository repo) throws Throwable {
ObjectId oldTreeId = repo.resolve("HEAD~1^{tree}");
ObjectId newTreeId = repo.resolve("HEAD^{tree}");
