Merge changes from topic "extension-registry-migration"
* changes:
AbstractPushForReview: Use ExtensionRegistry for CommitValidationListener
AccountIT: Use ExtensionRegistry for AccountActivationValidationListener
RevisionIT: Use ExtensionRegistry for PatchSetWebLink
Avoid passing lambda arguments to ExtensionRegistry.Registration#add
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index c0e968e..32c0fd1 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -28,6 +28,7 @@
import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.webui.FileHistoryWebLink;
+import com.google.gerrit.extensions.webui.PatchSetWebLink;
import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeETagComputation;
@@ -36,6 +37,7 @@
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.AccountActivationValidationListener;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.inject.Inject;
import com.google.inject.util.Providers;
@@ -60,8 +62,11 @@
private final DynamicSet<CommentAddedListener> commentAddedListeners;
private final DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners;
private final DynamicSet<FileHistoryWebLink> fileHistoryWebLinks;
+ private final DynamicSet<PatchSetWebLink> patchSetWebLinks;
private final DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
private final DynamicSet<GroupBackend> groupBackends;
+ private final DynamicSet<AccountActivationValidationListener>
+ accountActivationValidationListeners;
@Inject
ExtensionRegistry(
@@ -82,8 +87,10 @@
DynamicSet<CommentAddedListener> commentAddedListeners,
DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners,
DynamicSet<FileHistoryWebLink> fileHistoryWebLinks,
+ DynamicSet<PatchSetWebLink> patchSetWebLinks,
DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
- DynamicSet<GroupBackend> groupBackends) {
+ DynamicSet<GroupBackend> groupBackends,
+ DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -101,8 +108,10 @@
this.commentAddedListeners = commentAddedListeners;
this.refUpdatedListeners = refUpdatedListeners;
this.fileHistoryWebLinks = fileHistoryWebLinks;
+ this.patchSetWebLinks = patchSetWebLinks;
this.revisionCreatedListeners = revisionCreatedListeners;
this.groupBackends = groupBackends;
+ this.accountActivationValidationListeners = accountActivationValidationListeners;
}
public Registration newRegistration() {
@@ -185,6 +194,10 @@
return add(fileHistoryWebLinks, fileHistoryWebLink);
}
+ public Registration add(PatchSetWebLink patchSetWebLink) {
+ return add(patchSetWebLinks, patchSetWebLink);
+ }
+
public Registration add(RevisionCreatedListener revisionCreatedListener) {
return add(revisionCreatedListeners, revisionCreatedListener);
}
@@ -193,6 +206,11 @@
return add(groupBackends, groupBackend);
}
+ public Registration add(
+ AccountActivationValidationListener accountActivationValidationListener) {
+ return add(accountActivationValidationListeners, accountActivationValidationListener);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java b/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java
new file mode 100644
index 0000000..344b9b3
--- /dev/null
+++ b/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java
@@ -0,0 +1,30 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change.testing;
+
+import com.google.gerrit.server.change.ChangeETagComputation;
+
+public class TestChangeETagComputation {
+
+ public static ChangeETagComputation withETag(String etag) {
+ return (p, id) -> etag;
+ }
+
+ public static ChangeETagComputation withException(RuntimeException e) {
+ return (p, id) -> {
+ throw e;
+ };
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 1fdf3d6..6c61ae9 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -100,7 +100,6 @@
import com.google.gerrit.extensions.events.AccountIndexedListener;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -231,9 +230,6 @@
@Inject private AccountOperations accountOperations;
- @Inject
- private DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners;
-
@Inject protected GroupOperations groupOperations;
@After
@@ -598,27 +594,26 @@
accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
Account.Id deactivatableAccountId =
accountOperations.newAccount().preferredEmail("foo@deactivatable.com").create();
- RegistrationHandle registrationHandle =
- accountActivationValidationListeners.add(
- "gerrit",
- new AccountActivationValidationListener() {
- @Override
- public void validateActivation(AccountState account) throws ValidationException {
- String preferredEmail = account.account().preferredEmail();
- if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
- throw new ValidationException("not allowed to active account");
- }
- }
- @Override
- public void validateDeactivation(AccountState account) throws ValidationException {
- String preferredEmail = account.account().preferredEmail();
- if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
- throw new ValidationException("not allowed to deactive account");
- }
- }
- });
- try {
+ AccountActivationValidationListener listener =
+ new AccountActivationValidationListener() {
+ @Override
+ public void validateActivation(AccountState account) throws ValidationException {
+ String preferredEmail = account.account().preferredEmail();
+ if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
+ throw new ValidationException("not allowed to active account");
+ }
+ }
+
+ @Override
+ public void validateDeactivation(AccountState account) throws ValidationException {
+ String preferredEmail = account.account().preferredEmail();
+ if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
+ throw new ValidationException("not allowed to deactive account");
+ }
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
/* Test account that can be activated, but not deactivated */
// Deactivate account that is already inactive
ResourceConflictException thrown =
@@ -668,8 +663,6 @@
() -> gApi.accounts().id(deactivatableAccountId.get()).setActive(true));
assertThat(thrown).hasMessageThat().isEqualTo("not allowed to active account");
assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse();
- } finally {
- registrationHandle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 70d5e99..9767e08 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -161,6 +161,7 @@
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.testing.TestChangeETagComputation;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -2256,7 +2257,8 @@
PushOneCommit.Result r = createChange();
String oldETag = parseResource(r).getETag();
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> "foo")) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag("foo"))) {
assertThat(parseResource(r).getETag()).isNotEqualTo(oldETag);
}
@@ -2268,7 +2270,8 @@
PushOneCommit.Result r = createChange();
String oldETag = parseResource(r).getETag();
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> null)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag(null))) {
assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
}
}
@@ -2282,9 +2285,8 @@
extensionRegistry
.newRegistration()
.add(
- (p, id) -> {
- throw new StorageException("exception during test");
- })) {
+ TestChangeETagComputation.withException(
+ new StorageException("exception during test")))) {
assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index b7517a0..47eec0d 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -77,8 +77,6 @@
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -119,7 +117,6 @@
import org.junit.Test;
public class RevisionIT extends AbstractDaemonTest {
- @Inject private DynamicSet<PatchSetWebLink> patchSetLinks;
@Inject private GetRevisionActions getRevisionActions;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -1314,10 +1311,14 @@
@Test
public void commit() throws Exception {
WebLinkInfo expectedWebLinkInfo = new WebLinkInfo("foo", "imageUrl", "url");
- RegistrationHandle handle =
- patchSetLinks.add("gerrit", (projectName, commit) -> expectedWebLinkInfo);
-
- try {
+ PatchSetWebLink link =
+ new PatchSetWebLink() {
+ @Override
+ public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
+ return expectedWebLinkInfo;
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(link)) {
PushOneCommit.Result r = createChange();
RevCommit c = r.getCommit();
@@ -1339,8 +1340,6 @@
assertThat(webLinkInfo.imageUrl).isEqualTo(expectedWebLinkInfo.imageUrl);
assertThat(webLinkInfo.url).isEqualTo(expectedWebLinkInfo.url);
assertThat(webLinkInfo.target).isEqualTo(expectedWebLinkInfo.target);
- } finally {
- handle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index e8ab515..8afa0e0 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -50,6 +50,8 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
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.PushOneCommit;
@@ -82,8 +84,6 @@
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -150,12 +150,11 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private static String NEW_CHANGE_INDICATOR = " [NEW]";
private LabelType patchSetLock;
- @Inject private DynamicSet<CommitValidationListener> commitValidators;
-
@Before
public void setUpPatchSetLock() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -2206,24 +2205,16 @@
@GerritConfig(name = "receive.maxBatchCommits", value = "2")
@Test
public void maxBatchCommitsWithDefaultValidator() throws Exception {
- TestValidator validator = new TestValidator();
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(new TestValidator())) {
testMaxBatchCommits();
- } finally {
- handle.remove();
}
}
@GerritConfig(name = "receive.maxBatchCommits", value = "2")
@Test
public void maxBatchCommitsWithValidateAllCommitsValidator() throws Exception {
- TestValidator validator = new TestValidator(true);
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(new TestValidator())) {
testMaxBatchCommits();
- } finally {
- handle.remove();
}
}
@@ -2281,10 +2272,7 @@
public void skipValidation() throws Exception {
String master = "refs/heads/master";
TestValidator validator = new TestValidator();
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- RegistrationHandle handle2 = null;
-
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(validator)) {
// Validation listener is called on normal push
PushOneCommit push =
pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
@@ -2313,20 +2301,16 @@
// Validation listener that needs to validate all commits gets called even
// when the skip option is used.
TestValidator validator2 = new TestValidator(true);
- handle2 = commitValidators.add("test-validator-2", validator2);
- PushOneCommit push4 =
- pushFactory.create(admin.newIdent(), testRepo, "change2", "b.txt", "content");
- push4.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
- r = push4.to(master);
- r.assertOkStatus();
- // First listener was not called; its count remains the same.
- assertThat(validator.count()).isEqualTo(1);
- // Second listener was called.
- assertThat(validator2.count()).isEqualTo(1);
- } finally {
- handle.remove();
- if (handle2 != null) {
- handle2.remove();
+ try (Registration registration2 = extensionRegistry.newRegistration().add(validator2)) {
+ PushOneCommit push4 =
+ pushFactory.create(admin.newIdent(), testRepo, "change2", "b.txt", "content");
+ push4.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push4.to(master);
+ r.assertOkStatus();
+ // First listener was not called; its count remains the same.
+ assertThat(validator.count()).isEqualTo(1);
+ // Second listener was called.
+ assertThat(validator2.count()).isEqualTo(1);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index aeb5a4b..f2b8468 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -38,6 +38,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.RevisionJson;
+import com.google.gerrit.server.change.testing.TestChangeETagComputation;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
@@ -195,7 +196,8 @@
String change = createChange().getChangeId();
String oldETag = getETag(change);
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> "foo")) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag("foo"))) {
assertThat(getETag(change)).isNotEqualTo(oldETag);
}
@@ -207,7 +209,8 @@
String change = createChange().getChangeId();
String oldETag = getETag(change);
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> null)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag(null))) {
assertThat(getETag(change)).isEqualTo(oldETag);
}
}
@@ -221,9 +224,8 @@
extensionRegistry
.newRegistration()
.add(
- (p, id) -> {
- throw new StorageException("exception during test");
- })) {
+ TestChangeETagComputation.withException(
+ new StorageException("exception during test")))) {
assertThat(getETag(change)).isEqualTo(oldETag);
}
}
