Merge "Fix performance bug in logging: Avoid expensive stack trace/reflection"
diff --git a/.bazelversion b/.bazelversion
index 25939d3..3eefcb9 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-0.29.1
+1.0.0
diff --git a/WORKSPACE b/WORKSPACE
index 7b843b6..0d54bbe 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -611,18 +611,18 @@
sha1 = "18d4d07010c24405129a6dbb0e92057f8779fb9d",
)
-AUTO_VALUE_VERSION = "1.6.6"
+AUTO_VALUE_VERSION = "1.7"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "8831874cb2f4a9e1b196a2973f3cef695f5ce459",
+ sha1 = "fe8387764ed19460eda4f106849c664f51c07121",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "9947ae63d8ec42ea159283baf2e5b9c0ff100909",
+ sha1 = "5be124948ebdc7807df68207f35a0f23ce427f29",
)
declare_nongoogle_deps()
@@ -743,8 +743,8 @@
# Keep this version of Soy synchronized with the version used in Gitiles.
maven_jar(
name = "soy",
- artifact = "com.google.template:soy:2019-09-03",
- sha1 = "40781da0302b4b5d53006dc8bd5a432c7288d807",
+ artifact = "com.google.template:soy:2019-10-08",
+ sha1 = "4518bf8bac2dbbed684849bc209c39c4cb546237",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index 9b7e455..c0e968e 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -22,11 +22,14 @@
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.events.GroupIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
+import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
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.server.ExceptionHook;
+import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.validators.CommitValidationListener;
@@ -56,6 +59,9 @@
private final DynamicSet<RefOperationValidationListener> refOperationValidationListeners;
private final DynamicSet<CommentAddedListener> commentAddedListeners;
private final DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners;
+ private final DynamicSet<FileHistoryWebLink> fileHistoryWebLinks;
+ private final DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
+ private final DynamicSet<GroupBackend> groupBackends;
@Inject
ExtensionRegistry(
@@ -74,7 +80,10 @@
DynamicMap<DownloadScheme> downloadSchemes,
DynamicSet<RefOperationValidationListener> refOperationValidationListeners,
DynamicSet<CommentAddedListener> commentAddedListeners,
- DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners) {
+ DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners,
+ DynamicSet<FileHistoryWebLink> fileHistoryWebLinks,
+ DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
+ DynamicSet<GroupBackend> groupBackends) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -91,6 +100,9 @@
this.refOperationValidationListeners = refOperationValidationListeners;
this.commentAddedListeners = commentAddedListeners;
this.refUpdatedListeners = refUpdatedListeners;
+ this.fileHistoryWebLinks = fileHistoryWebLinks;
+ this.revisionCreatedListeners = revisionCreatedListeners;
+ this.groupBackends = groupBackends;
}
public Registration newRegistration() {
@@ -141,6 +153,10 @@
return add(changeMessageModifiers, changeMessageModifier);
}
+ public Registration add(ChangeMessageModifier changeMessageModifier, String exportName) {
+ return add(changeMessageModifiers, changeMessageModifier, exportName);
+ }
+
public Registration add(ChangeETagComputation changeETagComputation) {
return add(changeETagComputations, changeETagComputation);
}
@@ -165,8 +181,24 @@
return add(refUpdatedListeners, refUpdatedListener);
}
+ public Registration add(FileHistoryWebLink fileHistoryWebLink) {
+ return add(fileHistoryWebLinks, fileHistoryWebLink);
+ }
+
+ public Registration add(RevisionCreatedListener revisionCreatedListener) {
+ return add(revisionCreatedListeners, revisionCreatedListener);
+ }
+
+ public Registration add(GroupBackend groupBackend) {
+ return add(groupBackends, groupBackend);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
- RegistrationHandle registrationHandle = dynamicSet.add("gerrit", extension);
+ return add(dynamicSet, extension, "gerrit");
+ }
+
+ private <T> Registration add(DynamicSet<T> dynamicSet, T extension, String exportname) {
+ RegistrationHandle registrationHandle = dynamicSet.add(exportname, extension);
registrationHandles.add(registrationHandle);
return this;
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index e061b65..07bbade 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -3342,7 +3342,7 @@
}
logger.atFine().log(
- "Auto-closing %s changes with existing patch sets and %s with new patch sets",
+ "Auto-closing %d changes with existing patch sets and %d with new patch sets",
existingPatchSets, newPatchSets);
bu.execute();
} catch (IOException | StorageException | PermissionBackendException e) {
diff --git a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
index fc42474..d8ed29a 100644
--- a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
+++ b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
@@ -16,18 +16,16 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.UniversalGroupBackend;
import com.google.gerrit.server.group.testing.TestGroupBackend;
import com.google.inject.Inject;
import org.junit.Test;
public class TestGroupBackendTest extends AbstractDaemonTest {
- @Inject private DynamicSet<GroupBackend> groupBackends;
@Inject private UniversalGroupBackend universalGroupBackend;
+ @Inject private ExtensionRegistry extensionRegistry;
private final TestGroupBackend testGroupBackend = new TestGroupBackend();
private final AccountGroup.UUID testUUID = AccountGroup.uuid("testbackend:test");
@@ -39,11 +37,8 @@
@Test
public void universalGroupBackendHandlesTestGroup() throws Exception {
- RegistrationHandle registrationHandle = groupBackends.add("gerrit", testGroupBackend);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(testGroupBackend)) {
assertThat(universalGroupBackend.handles(testUUID)).isTrue();
- } finally {
- registrationHandle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index 16b7690..6f519f1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -19,6 +19,8 @@
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -29,9 +31,6 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
-import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.submit.CommitMergeStatus;
import com.google.inject.Inject;
import java.util.List;
@@ -40,8 +39,8 @@
import org.junit.Test;
public class SubmitByCherryPickIT extends AbstractSubmit {
- @Inject private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
@Override
protected SubmitType getSubmitType() {
@@ -89,15 +88,13 @@
@Test
public void changeMessageOnSubmit() throws Throwable {
PushOneCommit.Result change = createChange();
- RegistrationHandle handle =
- changeMessageModifiers.add(
- "gerrit",
- (newCommitMessage, original, mergeTip, destination) ->
- newCommitMessage + "Custom: " + destination.branch());
- try {
+ try (Registration registration =
+ extensionRegistry
+ .newRegistration()
+ .add(
+ (newCommitMessage, original, mergeTip, destination) ->
+ newCommitMessage + "Custom: " + destination.branch())) {
submit(change.getChangeId());
- } finally {
- handle.remove();
}
testRepo.git().fetch().setRemote("origin").call();
ChangeInfo info = get(change.getChangeId(), CURRENT_REVISION);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index 1808480..0cdc0e9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -20,6 +20,8 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -28,24 +30,20 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
-import java.util.ArrayDeque;
-import java.util.Deque;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase {
- @Inject private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Inject private DynamicItem<UrlFormatter> urlFormatter;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
@Override
protected SubmitType getSubmitType() {
@@ -99,7 +97,8 @@
ChangeMessageModifier modifier3 =
(msg, orig, tip, dest) -> msg + "Dest: " + dest.shortName() + "\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2, modifier3)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(modifier1).add(modifier2).add(modifier3)) {
ImmutableList<PushOneCommit.Result> changes = submitWithRebase(admin);
ChangeData cd1 = changes.get(0).getChange();
ChangeData cd2 = changes.get(1).getChange();
@@ -128,7 +127,8 @@
throw new IllegalStateException("boom");
};
ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(modifier1).add(modifier2)) {
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> submitWithRebase());
Throwable cause = Throwables.getRootCause(thrown);
@@ -141,7 +141,11 @@
public void changeMessageModifierReturningNullShortCircuits() throws Throwable {
ChangeMessageModifier modifier1 = (msg, orig, tip, dest) -> null;
ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try (Registration registration =
+ extensionRegistry
+ .newRegistration()
+ .add(modifier1, "modifier-1")
+ .add(modifier2, "modifier-2")) {
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> submitWithRebase());
Throwable cause = Throwables.getRootCause(thrown);
@@ -155,18 +159,6 @@
}
}
- private AutoCloseable installChangeMessageModifiers(ChangeMessageModifier... modifiers) {
- Deque<RegistrationHandle> handles = new ArrayDeque<>(modifiers.length);
- for (int i = 0; i < modifiers.length; i++) {
- handles.push(changeMessageModifiers.add("modifier-" + (i + 1), modifiers[i]));
- }
- return () -> {
- while (!handles.isEmpty()) {
- handles.pop().remove();
- }
- };
- }
-
private void assertLatestRevisionHasFooters(PushOneCommit.Result change) throws Throwable {
RevCommit c = getCurrentCommit(change);
assertThat(c.getFooterLines(FooterConstants.CHANGE_ID)).isNotEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
index 7590532..a3c1722 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
@@ -21,87 +21,84 @@
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.ChangeInfo;
-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.server.restapi.config.IndexChanges;
import com.google.inject.Inject;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
public class IndexChangesIT extends AbstractDaemonTest {
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private ProjectOperations projectOperations;
-
- private ChangeIndexedCounter changeIndexedCounter;
- private RegistrationHandle changeIndexedCounterHandle;
-
- @Before
- public void addChangeIndexedCounter() {
- changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
- }
-
- @After
- public void removeChangeIndexedCounter() {
- if (changeIndexedCounterHandle != null) {
- changeIndexedCounterHandle.remove();
- }
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void indexRequestFromNonAdminRejected() throws Exception {
- String changeId = createChange().getChangeId();
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = ImmutableSet.of(changeId);
- changeIndexedCounter.clear();
- userRestSession.post("/config/server/index.changes", in).assertForbidden();
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(0);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = ImmutableSet.of(changeId);
+ changeIndexedCounter.clear();
+ userRestSession.post("/config/server/index.changes", in).assertForbidden();
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(0);
+ }
}
@Test
public void indexVisibleChange() throws Exception {
- String changeId = createChange().getChangeId();
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = ImmutableSet.of(changeId);
- changeIndexedCounter.clear();
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = ImmutableSet.of(changeId);
+ changeIndexedCounter.clear();
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ }
}
@Test
public void indexNonVisibleChange() throws Exception {
- String changeId = createChange().getChangeId();
- ChangeInfo changeInfo = info(changeId);
- projectOperations
- .project(project)
- .forUpdate()
- .add(block(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
- .update();
- IndexChanges.Input in = new IndexChanges.Input();
- changeIndexedCounter.clear();
- in.changes = ImmutableSet.of(changeId);
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- assertThat(changeIndexedCounter.getCount(changeInfo)).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ ChangeInfo changeInfo = info(changeId);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
+ .update();
+ IndexChanges.Input in = new IndexChanges.Input();
+ changeIndexedCounter.clear();
+ in.changes = ImmutableSet.of(changeId);
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ assertThat(changeIndexedCounter.getCount(changeInfo)).isEqualTo(1);
+ }
}
@Test
public void indexMultipleChanges() throws Exception {
- ImmutableSet.Builder<String> changeIds = ImmutableSet.builder();
- for (int i = 0; i < 10; i++) {
- changeIds.add(createChange().getChangeId());
- }
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = changeIds.build();
- changeIndexedCounter.clear();
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- for (String changeId : in.changes) {
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ ImmutableSet.Builder<String> changeIds = ImmutableSet.builder();
+ for (int i = 0; i < 10; i++) {
+ changeIds.add(createChange().getChangeId());
+ }
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = changeIds.build();
+ changeIndexedCounter.clear();
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ for (String changeId : in.changes) {
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ }
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index bb043c2..c6c26c9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -22,6 +22,8 @@
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -40,8 +42,6 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
-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.ResourceNotFoundException;
@@ -72,9 +72,9 @@
private static final String LABEL_CODE_REVIEW = "Code-Review";
- @Inject private DynamicSet<FileHistoryWebLink> fileHistoryWebLinkDynamicSet;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private Project.NameKey newProjectName;
@@ -89,39 +89,39 @@
assertThat(inheritedName).isEqualTo(AllProjectsNameProvider.DEFAULT);
}
+ private Registration newFileHistoryWebLink() {
+ FileHistoryWebLink weblink =
+ new FileHistoryWebLink() {
+ @Override
+ public WebLinkInfo getFileHistoryWebLink(
+ String projectName, String revision, String fileName) {
+ return new WebLinkInfo(
+ "name", "imageURL", "http://view/" + projectName + "/" + fileName);
+ }
+ };
+ return extensionRegistry.newRegistration().add(weblink);
+ }
+
@Test
public void webLink() throws Exception {
- RegistrationHandle handle =
- fileHistoryWebLinkDynamicSet.add(
- "gerrit",
- (projectName, revision, fileName) ->
- new WebLinkInfo("name", "imageURL", "http://view/" + projectName + "/" + fileName));
- try {
+ try (Registration registration = newFileHistoryWebLink()) {
ProjectAccessInfo info = pApi().access();
assertThat(info.configWebLinks).hasSize(1);
assertThat(info.configWebLinks.get(0).url)
.isEqualTo("http://view/" + newProjectName + "/project.config");
- } finally {
- handle.remove();
}
}
@Test
public void webLinkNoRefsMetaConfig() throws Exception {
- RegistrationHandle handle =
- fileHistoryWebLinkDynamicSet.add(
- "gerrit",
- (projectName, revision, fileName) ->
- new WebLinkInfo("name", "imageURL", "http://view/" + projectName + "/" + fileName));
- try (Repository repo = repoManager.openRepository(newProjectName)) {
+ try (Repository repo = repoManager.openRepository(newProjectName);
+ Registration registration = newFileHistoryWebLink()) {
RefUpdate u = repo.updateRef(RefNames.REFS_CONFIG);
u.setForceUpdate(true);
assertThat(u.delete()).isEqualTo(Result.FORCED);
// This should not crash.
pApi().access();
- } finally {
- handle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
index 46fc689..8469fff 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
@@ -22,6 +22,8 @@
import static com.google.gerrit.server.project.testing.TestLabels.value;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -31,18 +33,15 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.events.CommentAddedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.inject.Inject;
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class CommentAddedEventIT extends AbstractDaemonTest {
- @Inject private DynamicSet<CommentAddedListener> source;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private final LabelType label =
label("CustomLabel", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
@@ -50,9 +49,6 @@
private final LabelType pLabel =
label("CustomLabel2", value(1, "Positive"), value(0, "No score"));
- private RegistrationHandle eventListenerRegistration;
- private CommentAddedListener.Event lastCommentAddedEvent;
-
@Before
public void setUp() throws Exception {
projectOperations
@@ -61,12 +57,6 @@
.add(allowLabel(label.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(-1, 1))
.add(allowLabel(pLabel.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(0, 1))
.update();
- eventListenerRegistration = source.add("gerrit", event -> lastCommentAddedEvent = event);
- }
-
- @After
- public void cleanup() {
- eventListenerRegistration.remove();
}
private void saveLabelConfig() throws Exception {
@@ -77,16 +67,30 @@
}
}
+ private static class TestListener implements CommentAddedListener {
+ private CommentAddedListener.Event lastCommentAddedEvent;
+
+ @Override
+ public void onCommentAdded(Event event) {
+ lastCommentAddedEvent = event;
+ }
+
+ public CommentAddedListener.Event getLastCommentAddedEvent() {
+ assertThat(lastCommentAddedEvent).isNotNull();
+ return lastCommentAddedEvent;
+ }
+ }
+
/* Need to lookup info for the label under test since there can be multiple
* labels defined. By default Gerrit already has a Code-Review label.
*/
- private ApprovalValues getApprovalValues(LabelType label) {
+ private ApprovalValues getApprovalValues(LabelType label, TestListener listener) {
ApprovalValues res = new ApprovalValues();
- ApprovalInfo info = lastCommentAddedEvent.getApprovals().get(label.getName());
+ ApprovalInfo info = listener.getLastCommentAddedEvent().getApprovals().get(label.getName());
if (info != null) {
res.value = info.value;
}
- info = lastCommentAddedEvent.getOldApprovals().get(label.getName());
+ info = listener.getLastCommentAddedEvent().getOldApprovals().get(label.getName());
if (info != null) {
res.oldValue = info.value;
}
@@ -97,15 +101,18 @@
public void newChangeWithVote() throws Exception {
saveLabelConfig();
- // push a new change with -1 vote
- PushOneCommit.Result r = createChange();
- ReviewInput reviewInput = new ReviewInput().label(label.getName(), (short) -1);
- revision(r).review(reviewInput);
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // push a new change with -1 vote
+ PushOneCommit.Result r = createChange();
+ ReviewInput reviewInput = new ReviewInput().label(label.getName(), (short) -1);
+ revision(r).review(reviewInput);
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ }
}
@Test
@@ -116,17 +123,19 @@
PushOneCommit.Result r = createChange();
ReviewInput reviewInput = new ReviewInput().message(label.getName());
revision(r).review(reviewInput);
-
- // push a new revision with +1 vote
- ChangeInfo c = info(r.getChangeId());
- r = amendChange(c.changeId);
- reviewInput = new ReviewInput().label(label.getName(), (short) 1);
- revision(r).review(reviewInput);
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 2: %s+1", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // push a new revision with +1 vote
+ ChangeInfo c = info(r.getChangeId());
+ r = amendChange(c.changeId);
+ reviewInput = new ReviewInput().label(label.getName(), (short) 1);
+ revision(r).review(reviewInput);
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 2: %s+1", label.getName()));
+ }
}
@Test
@@ -136,114 +145,120 @@
// push a change
PushOneCommit.Result r = createChange();
- // review with message only, do not apply votes
- ReviewInput reviewInput = new ReviewInput().message(label.getName());
- revision(r).review(reviewInput);
- // reply message only so vote is shown as 0
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isNull();
- assertThat(attr.value).isEqualTo(0);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // review with message only, do not apply votes
+ ReviewInput reviewInput = new ReviewInput().message(label.getName());
+ revision(r).review(reviewInput);
+ // reply message only so vote is shown as 0
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isNull();
+ assertThat(attr.value).isEqualTo(0);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
- // transition from un-voted to -1 vote
- reviewInput = new ReviewInput().label(label.getName(), -1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ // transition from un-voted to -1 vote
+ reviewInput = new ReviewInput().label(label.getName(), -1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
- // transition vote from -1 to 0
- reviewInput = new ReviewInput().label(label.getName(), 0);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(-1);
- assertThat(attr.value).isEqualTo(0);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: -%s", label.getName()));
+ // transition vote from -1 to 0
+ reviewInput = new ReviewInput().label(label.getName(), 0);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(-1);
+ assertThat(attr.value).isEqualTo(0);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: -%s", label.getName()));
- // transition vote from 0 to 1
- reviewInput = new ReviewInput().label(label.getName(), 1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s+1", label.getName()));
+ // transition vote from 0 to 1
+ reviewInput = new ReviewInput().label(label.getName(), 1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s+1", label.getName()));
- // transition vote from 1 to -1
- reviewInput = new ReviewInput().label(label.getName(), -1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(1);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ // transition vote from 1 to -1
+ reviewInput = new ReviewInput().label(label.getName(), -1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(1);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
- // review with message only, do not apply votes
- reviewInput = new ReviewInput().message(label.getName());
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isNull(); // no vote change so not included
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ // review with message only, do not apply votes
+ reviewInput = new ReviewInput().message(label.getName());
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isNull(); // no vote change so not included
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ }
}
@Test
public void reviewChange_MultipleVotes() throws Exception {
- saveLabelConfig();
- PushOneCommit.Result r = createChange();
- ReviewInput reviewInput = new ReviewInput().label(label.getName(), -1);
- reviewInput.message = label.getName();
- revision(r).review(reviewInput);
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ ReviewInput reviewInput = new ReviewInput().label(label.getName(), -1);
+ reviewInput.message = label.getName();
+ revision(r).review(reviewInput);
- ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
- LabelInfo q = c.labels.get(label.getName());
- assertThat(q.all).hasSize(1);
- ApprovalValues labelAttr = getApprovalValues(label);
- assertThat(labelAttr.oldValue).isEqualTo(0);
- assertThat(labelAttr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1\n\n%s", label.getName(), label.getName()));
+ ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
+ LabelInfo q = c.labels.get(label.getName());
+ assertThat(q.all).hasSize(1);
+ ApprovalValues labelAttr = getApprovalValues(label, listener);
+ assertThat(labelAttr.oldValue).isEqualTo(0);
+ assertThat(labelAttr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1\n\n%s", label.getName(), label.getName()));
- // there should be 3 approval labels (label, pLabel, and CRVV)
- assertThat(lastCommentAddedEvent.getApprovals()).hasSize(3);
+ // there should be 3 approval labels (label, pLabel, and CRVV)
+ assertThat(listener.getLastCommentAddedEvent().getApprovals()).hasSize(3);
- // check the approvals that were not voted on
- ApprovalValues pLabelAttr = getApprovalValues(pLabel);
- assertThat(pLabelAttr.oldValue).isNull();
- assertThat(pLabelAttr.value).isEqualTo(0);
+ // check the approvals that were not voted on
+ ApprovalValues pLabelAttr = getApprovalValues(pLabel, listener);
+ assertThat(pLabelAttr.oldValue).isNull();
+ assertThat(pLabelAttr.value).isEqualTo(0);
- LabelType crLabel = LabelType.withDefaultValues("Code-Review");
- ApprovalValues crlAttr = getApprovalValues(crLabel);
- assertThat(crlAttr.oldValue).isNull();
- assertThat(crlAttr.value).isEqualTo(0);
+ LabelType crLabel = LabelType.withDefaultValues("Code-Review");
+ ApprovalValues crlAttr = getApprovalValues(crLabel, listener);
+ assertThat(crlAttr.oldValue).isNull();
+ assertThat(crlAttr.value).isEqualTo(0);
- // update pLabel approval
- reviewInput = new ReviewInput().label(pLabel.getName(), 1);
- reviewInput.message = pLabel.getName();
- revision(r).review(reviewInput);
+ // update pLabel approval
+ reviewInput = new ReviewInput().label(pLabel.getName(), 1);
+ reviewInput.message = pLabel.getName();
+ revision(r).review(reviewInput);
- c = get(r.getChangeId(), DETAILED_LABELS);
- q = c.labels.get(label.getName());
- assertThat(q.all).hasSize(1);
- pLabelAttr = getApprovalValues(pLabel);
- assertThat(pLabelAttr.oldValue).isEqualTo(0);
- assertThat(pLabelAttr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s+1\n\n%s", pLabel.getName(), pLabel.getName()));
+ c = get(r.getChangeId(), DETAILED_LABELS);
+ q = c.labels.get(label.getName());
+ assertThat(q.all).hasSize(1);
+ pLabelAttr = getApprovalValues(pLabel, listener);
+ assertThat(pLabelAttr.oldValue).isEqualTo(0);
+ assertThat(pLabelAttr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s+1\n\n%s", pLabel.getName(), pLabel.getName()));
- // check the approvals that were not voted on
- labelAttr = getApprovalValues(label);
- assertThat(labelAttr.oldValue).isNull();
- assertThat(labelAttr.value).isEqualTo(-1);
+ // check the approvals that were not voted on
+ labelAttr = getApprovalValues(label, listener);
+ assertThat(labelAttr.oldValue).isNull();
+ assertThat(labelAttr.value).isEqualTo(-1);
- crlAttr = getApprovalValues(crLabel);
- assertThat(crlAttr.oldValue).isNull();
- assertThat(crlAttr.value).isEqualTo(0);
+ crlAttr = getApprovalValues(crLabel, listener);
+ assertThat(crlAttr.oldValue).isNull();
+ assertThat(crlAttr.value).isEqualTo(0);
+ }
}
private static class ApprovalValues {
diff --git a/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java b/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
index 5ff1c32..8744cfad 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
@@ -17,49 +17,48 @@
import static com.google.common.truth.Truth.assertThat;
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.NoHttpd;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.inject.Inject;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class EventPayloadIT extends AbstractDaemonTest {
- @Inject private DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
-
- private RegistrationHandle eventListenerRegistration;
- private RevisionCreatedListener.Event lastEvent;
-
- @Before
- public void setUp() throws Exception {
- eventListenerRegistration = revisionCreatedListeners.add("gerrit", event -> lastEvent = event);
- }
-
- @After
- public void cleanup() {
- eventListenerRegistration.remove();
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void defaultOptions() throws Exception {
- createChange();
-
- assertThat(lastEvent.getChange().submittable).isNotNull();
- assertThat(lastEvent.getRevision().files).isNotEmpty();
+ RevisionCreatedListener listener =
+ new RevisionCreatedListener() {
+ @Override
+ public void onRevisionCreated(Event event) {
+ assertThat(event.getChange().submittable).isNotNull();
+ assertThat(event.getRevision().files).isNotEmpty();
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ createChange();
+ }
}
@Test
@GerritConfig(name = "event.payload.listChangeOptions", value = "SKIP_MERGEABLE")
public void configuredOptions() throws Exception {
- createChange();
-
- assertThat(lastEvent.getChange().submittable).isNull();
- assertThat(lastEvent.getChange().mergeable).isNull();
- assertThat(lastEvent.getRevision().files).isNull();
- assertThat(lastEvent.getChange().subject).isNotEmpty();
+ RevisionCreatedListener listener =
+ new RevisionCreatedListener() {
+ @Override
+ public void onRevisionCreated(Event event) {
+ assertThat(event.getChange().submittable).isNull();
+ assertThat(event.getChange().mergeable).isNull();
+ assertThat(event.getRevision().files).isNull();
+ assertThat(event.getChange().subject).isNotEmpty();
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ createChange();
+ }
}
}
diff --git a/resources/com/google/gerrit/server/mime/mime-types.properties b/resources/com/google/gerrit/server/mime/mime-types.properties
index f12b39e..4229e4a 100644
--- a/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -31,6 +31,7 @@
cr = text/x-crystal
cs = text/x-csharp
csharp = text/x-csharp
+csproj = application/xml
css = text/css
cpp = text/x-c++src
cql = text/x-cassandra
@@ -242,6 +243,7 @@
vtl = text/velocity
webidl = text/x-webidl
wsdl = application/xml
+xaml = application/xml
xhtml = text/html
xml = application/xml
xsd = application/xml
