Merge "Add undefined check throughout app"
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/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/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/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 059a8fd..b9eb518 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -113,7 +113,7 @@
}
.subHeader {
flex-wrap: wrap;
- margin: 0 var(--default-horizontal-margin) .75em;
+ padding: 0 var(--default-horizontal-margin) .75em;
}
.subHeader > div {
margin-top: .25em;
diff --git a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.html b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.html
index 47acd0c..226092f 100644
--- a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.html
+++ b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.html
@@ -22,6 +22,7 @@
<template>
<style include="shared-styles">
:host {
+ box-sizing: border-box;
display: block;
min-height: var(--header-height);
position: relative;
