Merge "Fix styling issue on gr-fixed-panel"
diff --git a/Documentation/check_licenses_test.sh b/Documentation/check_licenses_test.sh
index a65a827..52e27f2 100755
--- a/Documentation/check_licenses_test.sh
+++ b/Documentation/check_licenses_test.sh
@@ -8,7 +8,7 @@
echo "FAIL: ${f}.txt out of date"
echo "to fix: "
echo ""
- echo " cp bazel-genfiles/Documentation/${f}.gen.txt Documentation/${f}.txt"
+ echo " cp bazel-bin/Documentation/${f}.gen.txt Documentation/${f}.txt"
echo ""
exit 1
fi
diff --git a/Documentation/linux-quickstart.txt b/Documentation/linux-quickstart.txt
index c2dcedb..0d8848e 100644
--- a/Documentation/linux-quickstart.txt
+++ b/Documentation/linux-quickstart.txt
@@ -29,10 +29,10 @@
. Download the desired Gerrit archive.
To view previous archives, see
-link:https://gerrit-releases.storage.googleapis.com/index.html[Gerrit Code Review: Releases]. The steps below install Gerrit 2.15.1:
+link:https://gerrit-releases.storage.googleapis.com/index.html[Gerrit Code Review: Releases]. The steps below install Gerrit 3.0.3:
....
-wget https://www.gerritcodereview.com/download/gerrit-2.15.1.war
+wget https://gerrit-releases.storage.googleapis.com/gerrit-3.0.3.war
....
NOTE: To build and install Gerrit from the source files, see
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/java/com/google/gerrit/extensions/common/AccountInfo.java b/java/com/google/gerrit/extensions/common/AccountInfo.java
index e949f07..d1bbe88 100644
--- a/java/com/google/gerrit/extensions/common/AccountInfo.java
+++ b/java/com/google/gerrit/extensions/common/AccountInfo.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.common;
+import com.google.common.base.MoreObjects;
import java.util.List;
import java.util.Objects;
@@ -55,6 +56,16 @@
}
@Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("id", _accountId)
+ .add("name", name)
+ .add("email", email)
+ .add("username", username)
+ .toString();
+ }
+
+ @Override
public int hashCode() {
return Objects.hash(
_accountId, name, email, secondaryEmails, username, avatars, _moreAccounts, status);
diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index ee2b672..5f7ea4e 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -24,6 +24,7 @@
import com.google.common.collect.SetMultimap;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.query.account.InternalAccountQuery;
@@ -34,8 +35,11 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.sql.Timestamp;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
+import org.eclipse.jgit.lib.PersonIdent;
/** Class to access accounts by email. */
@Singleton
@@ -117,6 +121,24 @@
return externalIds.byEmail(email).stream().map(ExternalId::accountId).collect(toImmutableSet());
}
+ public UserIdentity toUserIdentity(PersonIdent who) throws IOException {
+ UserIdentity u = new UserIdentity();
+ u.setName(who.getName());
+ u.setEmail(who.getEmailAddress());
+ u.setDate(new Timestamp(who.getWhen().getTime()));
+ u.setTimeZone(who.getTimeZoneOffset());
+
+ // If only one account has access to this email address, select it
+ // as the identity of the user.
+ //
+ Set<Account.Id> a = getAccountFor(u.getEmail());
+ if (a.size() == 1) {
+ u.setAccount(a.iterator().next());
+ }
+
+ return u;
+ }
+
private <T> T executeIndexQuery(Action<T> action) {
try {
return retryHelper.execute(
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index d50e740..b0477cd 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -298,7 +298,6 @@
Map<Change.Id, ChangeInfo> cache = Maps.newHashMapWithExpectedSize(in.size());
for (QueryResult<ChangeData> r : in) {
List<ChangeInfo> infos = toChangeInfos(r.entities(), cache);
- infos.forEach(c -> cache.put(Change.id(c._number), c));
if (!infos.isEmpty() && r.more()) {
infos.get(infos.size() - 1)._moreChanges = true;
}
@@ -415,15 +414,29 @@
List<ChangeData> changes, Map<Change.Id, ChangeInfo> cache) {
try (Timer0.Context ignored = metrics.toChangeInfosLatency.start()) {
List<ChangeInfo> changeInfos = new ArrayList<>(changes.size());
- for (ChangeData cd : changes) {
- ChangeInfo i = cache.get(cd.getId());
- if (i != null) {
- changeInfos.add(i);
+ for (int i = 0; i < changes.size(); i++) {
+ // We can only cache and re-use an entity if it's not the last in the list. The last entity
+ // may later get _moreChanges set. If it was cached or re-used, that setting would propagate
+ // to the original entity yielding wrong results.
+ // This problem has two sides where 'last in the list' has to be respected:
+ // (1) Caching
+ // (2) Reusing
+ boolean isCacheable = i != changes.size() - 1;
+ ChangeData cd = changes.get(i);
+ ChangeInfo info = cache.get(cd.getId());
+ if (info != null && isCacheable) {
+ changeInfos.add(info);
continue;
}
+
+ // Compute and cache if possible
try {
ensureLoaded(Collections.singleton(cd));
- changeInfos.add(format(cd, Optional.empty(), false, ChangeInfo::new));
+ info = format(cd, Optional.empty(), false, ChangeInfo::new);
+ changeInfos.add(info);
+ if (isCacheable) {
+ cache.put(Change.id(info._number), info);
+ }
} catch (RuntimeException e) {
logger.atWarning().withCause(e).log(
"Omitting corrupt change %s from results", cd.getId());
diff --git a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
index 569399b..a15b429 100644
--- a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
+++ b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
@@ -81,8 +81,7 @@
}
Query query = parser.parse(q);
try {
- // TODO(fishywang): Currently as we don't have much documentation, we just use MAX_VALUE here
- // and skipped paging. Maybe add paging later.
+ // We don't have much documentation, so we just use MAX_VALUE here and skip paging.
TopDocs results = searcher.search(query, Integer.MAX_VALUE);
ScoreDoc[] hits = results.scoreDocs;
long totalHits = results.totalHits;
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 4d5f158..0bf888f 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -68,7 +68,6 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -476,7 +475,7 @@
p.parents.add(parent.name());
}
- UserIdentity author = toUserIdentity(c.getAuthorIdent());
+ UserIdentity author = emails.toUserIdentity(c.getAuthorIdent());
if (author.getAccount() == null) {
p.author = new AccountAttribute();
p.author.email = author.getEmail();
@@ -504,26 +503,6 @@
return p;
}
- // TODO: The same method exists in PatchSetInfoFactory, find a common place
- // for it
- private UserIdentity toUserIdentity(PersonIdent who) throws IOException {
- UserIdentity u = new UserIdentity();
- u.setName(who.getName());
- u.setEmail(who.getEmailAddress());
- u.setDate(new Timestamp(who.getWhen().getTime()));
- u.setTimeZone(who.getTimeZoneOffset());
-
- // If only one account has access to this email address, select it
- // as the identity of the user.
- //
- Set<Account.Id> a = emails.getAccountFor(u.getEmail());
- if (a.size() == 1) {
- u.setAccount(a.iterator().next());
- }
-
- return u;
- }
-
public void addApprovals(
PatchSetAttribute p,
PatchSet.Id id,
diff --git a/java/com/google/gerrit/server/git/TracingHook.java b/java/com/google/gerrit/server/git/TracingHook.java
index 4191373..63d8bc6 100644
--- a/java/com/google/gerrit/server/git/TracingHook.java
+++ b/java/com/google/gerrit/server/git/TracingHook.java
@@ -16,9 +16,6 @@
import static com.google.common.base.Preconditions.checkState;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.LinkedListMultimap;
-import com.google.common.collect.ListMultimap;
import com.google.gerrit.server.logging.TraceContext;
import java.util.List;
import java.util.Optional;
@@ -79,21 +76,20 @@
return Optional.empty();
}
- ListMultimap<String, String> serverOptions = LinkedListMultimap.create();
- for (String option : serverOptionList) {
- int e = option.indexOf('=');
- if (e > 0) {
- serverOptions.put(option.substring(0, e), option.substring(e + 1));
- } else {
- serverOptions.put(option, "");
- }
+ Optional<String> traceOption =
+ serverOptionList.stream().filter(o -> o.startsWith("trace")).findAny();
+ if (!traceOption.isPresent()) {
+ return Optional.empty();
}
- List<String> traceValues = serverOptions.get("trace");
- if (!traceValues.isEmpty()) {
- return Optional.of(Iterables.getLast(traceValues));
+ int e = traceOption.get().indexOf('=');
+ if (e > 0) {
+ // trace option was specified with trace ID: "--trace=<trace-ID>"
+ return Optional.of(traceOption.get().substring(e + 1));
}
- return Optional.empty();
+ // trace option was specified without trace ID: "--trace",
+ // return an empty string so that a trace ID is generated
+ return Optional.of("");
}
}
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 4defded..7af204e 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -183,63 +183,66 @@
* @return string representation of this instance that is suitable for logging
*/
LazyArg<String> toStringForLoggingLazy() {
- return LazyArgs.lazy(
- () -> {
- // Append class name.
- String className = getClass().getSimpleName();
- if (className.startsWith("AutoValue_")) {
- className = className.substring(10);
- }
- ToStringHelper stringHelper = MoreObjects.toStringHelper(className);
+ // Don't use a lambda because different compilers generate different method names for lambdas,
+ // e.g. "lambda$myFunction$0" vs. just "lambda$0" in Eclipse. We need to identify the method
+ // by name to skip it and avoid infinite recursion.
+ return LazyArgs.lazy(this::toStringForLoggingImpl);
+ }
- // Append key-value pairs for field which are set.
- Method[] methods = Metadata.class.getDeclaredMethods();
- Arrays.sort(methods, Comparator.comparing(Method::getName));
- for (Method method : methods) {
- if (Modifier.isStatic(method.getModifiers())) {
- // skip static method
- continue;
- }
+ private String toStringForLoggingImpl() {
+ // Append class name.
+ String className = getClass().getSimpleName();
+ if (className.startsWith("AutoValue_")) {
+ className = className.substring(10);
+ }
+ ToStringHelper stringHelper = MoreObjects.toStringHelper(className);
- if (method.getName().matches("(lambda\\$)?toStringForLoggingLazy(\\$0)?")) {
- // skip toStringForLoggingLazy() and the lambda itself
- continue;
- }
+ // Append key-value pairs for field which are set.
+ Method[] methods = Metadata.class.getDeclaredMethods();
+ Arrays.sort(methods, Comparator.comparing(Method::getName));
+ for (Method method : methods) {
+ if (Modifier.isStatic(method.getModifiers())) {
+ // skip static method
+ continue;
+ }
- if (method.getReturnType().equals(Void.TYPE) || method.getParameterCount() > 0) {
- // skip method since it's not a getter
- continue;
- }
+ if (method.getName().equals("toStringForLoggingLazy")
+ || method.getName().equals("toStringForLoggingImpl")) {
+ // Don't call myself in infinite recursion.
+ continue;
+ }
- method.setAccessible(true);
+ if (method.getReturnType().equals(Void.TYPE) || method.getParameterCount() > 0) {
+ // skip method since it's not a getter
+ continue;
+ }
- Object returnValue;
- try {
- returnValue = method.invoke(this);
- } catch (IllegalArgumentException
- | IllegalAccessException
- | InvocationTargetException e) {
- // should never happen
- throw new IllegalStateException(e);
- }
+ method.setAccessible(true);
- if (returnValue instanceof Optional) {
- Optional<?> fieldValueOptional = (Optional<?>) returnValue;
- if (!fieldValueOptional.isPresent()) {
- // drop this key-value pair
- continue;
- }
+ Object returnValue;
+ try {
+ returnValue = method.invoke(this);
+ } catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) {
+ // should never happen
+ throw new IllegalStateException(e);
+ }
- // format as 'key=value' instead of 'key=Optional[value]'
- stringHelper.add(method.getName(), fieldValueOptional.get());
- } else {
- // not an Optional value, keep as is
- stringHelper.add(method.getName(), returnValue);
- }
- }
+ if (returnValue instanceof Optional) {
+ Optional<?> fieldValueOptional = (Optional<?>) returnValue;
+ if (!fieldValueOptional.isPresent()) {
+ // drop this key-value pair
+ continue;
+ }
- return stringHelper.toString();
- });
+ // format as 'key=value' instead of 'key=Optional[value]'
+ stringHelper.add(method.getName(), fieldValueOptional.get());
+ } else {
+ // not an Optional value, keep as is
+ stringHelper.add(method.getName(), returnValue);
+ }
+ }
+
+ return stringHelper.toString();
}
public static Metadata.Builder builder() {
diff --git a/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java b/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java
index c684da5..d3cadb2f 100644
--- a/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java
@@ -15,11 +15,9 @@
package com.google.gerrit.server.patch;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -27,12 +25,9 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.List;
-import java.util.Set;
import org.eclipse.jgit.errors.MissingObjectException;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -56,8 +51,8 @@
PatchSetInfo info = new PatchSetInfo(psi);
info.setSubject(src.getShortMessage());
info.setMessage(src.getFullMessage());
- info.setAuthor(toUserIdentity(src.getAuthorIdent()));
- info.setCommitter(toUserIdentity(src.getCommitterIdent()));
+ info.setAuthor(emails.toUserIdentity(src.getAuthorIdent()));
+ info.setCommitter(emails.toUserIdentity(src.getCommitterIdent()));
info.setCommitId(src);
return info;
}
@@ -85,25 +80,6 @@
}
}
- // TODO: The same method exists in EventFactory, find a common place for it
- private UserIdentity toUserIdentity(PersonIdent who) throws IOException {
- final UserIdentity u = new UserIdentity();
- u.setName(who.getName());
- u.setEmail(who.getEmailAddress());
- u.setDate(new Timestamp(who.getWhen().getTime()));
- u.setTimeZone(who.getTimeZoneOffset());
-
- // If only one account has access to this email address, select it
- // as the identity of the user.
- //
- Set<Account.Id> a = emails.getAccountFor(u.getEmail());
- if (a.size() == 1) {
- u.setAccount(a.iterator().next());
- }
-
- return u;
- }
-
private List<PatchSetInfo.ParentInfo> toParentInfos(RevCommit[] parents, RevWalk walk)
throws IOException, MissingObjectException {
List<PatchSetInfo.ParentInfo> pInfos = new ArrayList<>(parents.length);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
index 76166e1..92f914b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
@@ -44,8 +44,8 @@
QueryChanges queryChanges = queryChangesProvider.get();
- queryChanges.addQuery("is:open");
- queryChanges.addQuery("is:wip");
+ queryChanges.addQuery("is:open repo:" + project.get());
+ queryChanges.addQuery("is:wip repo:" + project.get());
List<List<ChangeInfo>> result =
(List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
@@ -58,4 +58,48 @@
assertThat(firstResultIds).containsExactly(numericId1, numericId2);
assertThat(result.get(1).get(0)._number).isEqualTo(numericId2);
}
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void moreChangesIndicatorDoesNotWronglyCopyToUnrelatedChanges() throws Exception {
+ String queryWithMoreChanges = "is:wip limit:1 repo:" + project.get();
+ String queryWithNoMoreChanges = "is:open limit:10 repo:" + project.get();
+ createChange().getChangeId();
+ String cId2 = createChange().getChangeId();
+ String cId3 = createChange().getChangeId();
+ gApi.changes().id(cId2).setWorkInProgress();
+ gApi.changes().id(cId3).setWorkInProgress();
+
+ // Run the capped query first
+ QueryChanges queryChanges = queryChangesProvider.get();
+ queryChanges.addQuery(queryWithMoreChanges);
+ queryChanges.addQuery(queryWithNoMoreChanges);
+ List<List<ChangeInfo>> result =
+ (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+ assertThat(result).hasSize(2);
+ assertThat(result.get(0)).hasSize(1);
+ assertThat(result.get(1)).hasSize(3);
+ // _moreChanges is set on the first response, but not on the second.
+ assertThat(result.get(0).get(0)._moreChanges).isTrue();
+ assertNoChangeHasMoreChangesSet(result.get(1));
+
+ // Run the capped query second
+ QueryChanges queryChanges2 = queryChangesProvider.get();
+ queryChanges2.addQuery(queryWithNoMoreChanges);
+ queryChanges2.addQuery(queryWithMoreChanges);
+ List<List<ChangeInfo>> result2 =
+ (List<List<ChangeInfo>>) queryChanges2.apply(TopLevelResource.INSTANCE).value();
+ assertThat(result2).hasSize(2);
+ assertThat(result2.get(0)).hasSize(3);
+ assertThat(result2.get(1)).hasSize(1);
+ // _moreChanges is set on the second response, but not on the first.
+ assertNoChangeHasMoreChangesSet(result2.get(0));
+ assertThat(result2.get(1).get(0)._moreChanges).isTrue();
+ }
+
+ private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
+ for (ChangeInfo info : results) {
+ assertThat(info._moreChanges).isNull();
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index c0083d2..154fabf 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;
@@ -67,8 +69,6 @@
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.LabelInfo;
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.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -118,7 +118,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 +130,11 @@
}
@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 +810,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 +866,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);
+ }
}
}
@@ -1228,9 +1227,8 @@
PushOneCommit.Result changeOtherBranch = createChange("refs/for/dev");
ChangeIndexedListener changeIndexedListener = mock(ChangeIndexedListener.class);
- RegistrationHandle registrationHandle =
- changeIndexedListeners.add("gerrit", changeIndexedListener);
- try {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedListener)) {
// submit a change, this should trigger asynchronous reindexing of the open changes on the
// same branch
approve(change1.getChangeId());
@@ -1257,8 +1255,6 @@
// open changes on other branches don't get reindexed
verify(changeIndexedListener, times(0))
.onChangeScheduledForIndexing(project.get(), changeOtherBranch.getChange().getId().get());
- } finally {
- registrationHandle.remove();
}
}
@@ -1438,11 +1434,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}");
diff --git a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
index a03d7f3..c841559 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
@@ -20,102 +20,91 @@
import com.google.common.base.Joiner;
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.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseSsh;
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.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Injector;
import java.util.List;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@NoHttpd
@UseSsh
public abstract class AbstractIndexTests extends AbstractDaemonTest {
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
-
- private ChangeIndexedCounter changeIndexedCounter;
- private RegistrationHandle changeIndexedCounterHandle;
+ @Inject private ExtensionRegistry extensionRegistry;
/** @param injector injector */
public void configureIndex(Injector injector) {}
- @Before
- public void addChangeIndexedCounter() {
- changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
- }
-
- @After
- public void removeChangeIndexedCounter() {
- if (changeIndexedCounterHandle != null) {
- changeIndexedCounterHandle.remove();
- }
- }
-
@Test
@GerritConfig(name = "index.autoReindexIfStale", value = "false")
public void indexChange() throws Exception {
- configureIndex(server.getTestInjector());
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ configureIndex(server.getTestInjector());
- PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
- String changeId = change.getChangeId();
- String changeLegacyId = change.getChange().getId().toString();
- ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
+ String changeId = change.getChangeId();
+ String changeLegacyId = change.getChange().getId().toString();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
- disableChangeIndexWrites();
- amendChange(changeId, "second test", "test2.txt", "test2");
+ disableChangeIndexWrites();
+ amendChange(changeId, "second test", "test2.txt", "test2");
- assertChangeQuery("message:second", change.getChange(), false);
- enableChangeIndexWrites();
+ assertChangeQuery("message:second", change.getChange(), false);
+ enableChangeIndexWrites();
- changeIndexedCounter.clear();
- String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
- adminSshSession.exec(cmd);
- adminSshSession.assertSuccess();
+ changeIndexedCounter.clear();
+ String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
+ adminSshSession.exec(cmd);
+ adminSshSession.assertSuccess();
- changeIndexedCounter.assertReindexOf(changeInfo, 1);
+ changeIndexedCounter.assertReindexOf(changeInfo, 1);
- assertChangeQuery("message:second", change.getChange(), true);
+ assertChangeQuery("message:second", change.getChange(), true);
+ }
}
@Test
@GerritConfig(name = "index.autoReindexIfStale", value = "false")
public void indexProject() throws Exception {
- configureIndex(server.getTestInjector());
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ configureIndex(server.getTestInjector());
- PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
- String changeId = change.getChangeId();
- ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
+ String changeId = change.getChangeId();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
- disableChangeIndexWrites();
- amendChange(changeId, "second test", "test2.txt", "test2");
+ disableChangeIndexWrites();
+ amendChange(changeId, "second test", "test2.txt", "test2");
- assertChangeQuery("message:second", change.getChange(), false);
- enableChangeIndexWrites();
+ assertChangeQuery("message:second", change.getChange(), false);
+ enableChangeIndexWrites();
- changeIndexedCounter.clear();
- String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
- adminSshSession.exec(cmd);
- adminSshSession.assertSuccess();
-
- boolean indexing = true;
- while (indexing) {
- String out = adminSshSession.exec("gerrit show-queue --wide");
+ changeIndexedCounter.clear();
+ String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
+ adminSshSession.exec(cmd);
adminSshSession.assertSuccess();
- indexing = out.contains("Index all changes of project " + project.get());
+
+ boolean indexing = true;
+ while (indexing) {
+ String out = adminSshSession.exec("gerrit show-queue --wide");
+ adminSshSession.assertSuccess();
+ indexing = out.contains("Index all changes of project " + project.get());
+ }
+
+ changeIndexedCounter.assertReindexOf(changeInfo, 1);
+
+ assertChangeQuery("message:second", change.getChange(), true);
}
-
- changeIndexedCounter.assertReindexOf(changeInfo, 1);
-
- assertChangeQuery("message:second", change.getChange(), true);
}
protected void assertChangeQuery(String q, ChangeData change, boolean assertTrue)
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
index 09e97b2..ae45d90 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
@@ -20,9 +20,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.UseSsh;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.logging.LoggingContext;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.PerformanceLogger;
@@ -33,66 +33,58 @@
import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.List;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@UseSsh
public class SshTraceIT extends AbstractDaemonTest {
- @Inject private DynamicSet<ProjectCreationValidationListener> projectCreationValidationListeners;
- @Inject private DynamicSet<PerformanceLogger> performanceLoggers;
-
- private TraceValidatingProjectCreationValidationListener projectCreationListener;
- private RegistrationHandle projectCreationListenerRegistrationHandle;
- private TestPerformanceLogger testPerformanceLogger;
- private RegistrationHandle performanceLoggerRegistrationHandle;
-
- @Before
- public void setup() {
- projectCreationListener = new TraceValidatingProjectCreationValidationListener();
- projectCreationListenerRegistrationHandle =
- projectCreationValidationListeners.add("gerrit", projectCreationListener);
- testPerformanceLogger = new TestPerformanceLogger();
- performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger);
- }
-
- @After
- public void cleanup() {
- projectCreationListenerRegistrationHandle.remove();
- performanceLoggerRegistrationHandle.remove();
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void sshCallWithoutTrace() throws Exception {
- adminSshSession.exec("gerrit create-project new1");
- adminSshSession.assertSuccess();
- assertThat(projectCreationListener.traceId).isNull();
- assertThat(projectCreationListener.foundTraceId).isFalse();
- assertThat(projectCreationListener.isLoggingForced).isFalse();
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project new1");
+ adminSshSession.assertSuccess();
+ assertThat(projectCreationListener.traceId).isNull();
+ assertThat(projectCreationListener.foundTraceId).isFalse();
+ assertThat(projectCreationListener.isLoggingForced).isFalse();
+ }
}
@Test
public void sshCallWithTrace() throws Exception {
- adminSshSession.exec("gerrit create-project --trace new2");
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project --trace new2");
- // The trace ID is written to stderr.
- adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
+ // The trace ID is written to stderr.
+ adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
- assertThat(projectCreationListener.traceId).isNotNull();
- assertThat(projectCreationListener.foundTraceId).isTrue();
- assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.traceId).isNotNull();
+ assertThat(projectCreationListener.foundTraceId).isTrue();
+ assertThat(projectCreationListener.isLoggingForced).isTrue();
+ }
}
@Test
public void sshCallWithTraceAndProvidedTraceId() throws Exception {
- adminSshSession.exec("gerrit create-project --trace --trace-id issue/123 new3");
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project --trace --trace-id issue/123 new3");
- // The trace ID is written to stderr.
- adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
+ // The trace ID is written to stderr.
+ adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
- assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
- assertThat(projectCreationListener.foundTraceId).isTrue();
- assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
+ assertThat(projectCreationListener.foundTraceId).isTrue();
+ assertThat(projectCreationListener.isLoggingForced).isTrue();
+ }
}
@Test
@@ -103,9 +95,13 @@
@Test
public void performanceLoggingForSshCall() throws Exception {
- adminSshSession.exec("gerrit create-project new5");
- adminSshSession.assertSuccess();
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testPerformanceLogger)) {
+ adminSshSession.exec("gerrit create-project new5");
+ adminSshSession.assertSuccess();
+ assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ }
}
private static class TraceValidatingProjectCreationValidationListener
diff --git a/polygerrit-ui/app/elements/gr-app-p2.js b/polygerrit-ui/app/elements/gr-app-p2.js
index 2163c02..5b6e316 100644
--- a/polygerrit-ui/app/elements/gr-app-p2.js
+++ b/polygerrit-ui/app/elements/gr-app-p2.js
@@ -17,6 +17,9 @@
(function() {
'use strict';
+ // Disable extra font load from paper-styles
+ window.polymerSkipLoadingFontRoboto = true;
+
Polymer({
is: 'gr-app-p2',
});
