Merge "javadoc for ProjectCacheImpl"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 1d787cc..82e15d9 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -914,6 +914,12 @@
+
If direct updates are made to `All-Users`, this cache should be flushed.
+cache `"approvals"`::
++
+Cache entries contain approvals for a given patch set. This includes
+approvals granted on this patch set as well as approvals copied from
+earlier patch sets.
+
cache `"adv_bases"`::
+
Used only for push over smart HTTP when branch level access controls
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 9d3446e..f5346c1 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -268,6 +268,46 @@
If true, any score for the label is copied forward when a new patch
set is uploaded. Defaults to false.
+[[label_copyCondition]]
+=== `label.Label-Name.copyCondition`
+
+If set, Gerrit matches patch set approvals against the provided query
+string. If the query matches, the approval is copied from one patch set
+to the next. The query language is the same as for
+link:user-search.html[other queries].
+
+This logic is triggered whenever a new patch set is uploaded.
+
+Gerrit currently supports the following predicates:
+
+==== change-kind:{REWORK,TRIVIAL_REBASE,MERGE_FIRST_PARENT_UPDATE,NO_CODE_CHANGE,NO_CHANGE}
+
+Matches if the diff between two patch sets was of a certain change kind.
+
+==== is:{MIN,MAX,ANY}
+
+Matches votes that are equal to the minimal or maximal voting range. Or any votes.
+
+==== approverin:`groupUUID`
+
+Matches votes granted by a user who is a member of `groupUUID`.
+
+==== uploaderin:`groupUUID`
+
+Matches votes where the new patch set was uploaded by a member of `groupUUID`.
+
+==== has:unchanged-files
+
+Matches when the new patch-set includes the same files as the old patch-set.
+
+Only 'unchanged-files' is supported for 'has'.
+
+==== Example
+
+----
+copyCondition = is:MIN OR -change-kind:REWORK OR uploaderin:dead...beef
+----
+
[[label_copyMinScore]]
=== `label.Label-Name.copyMinScore`
diff --git a/Documentation/intro-gerrit-walkthrough-github.txt b/Documentation/intro-gerrit-walkthrough-github.txt
index 8f3ff88..173f709 100644
--- a/Documentation/intro-gerrit-walkthrough-github.txt
+++ b/Documentation/intro-gerrit-walkthrough-github.txt
@@ -25,7 +25,7 @@
Here’s how getting code reviewed and submitted with Gerrit is different from
doing the same with GitHub:
-* You need the add a commit-msg hook script when you clone a repo for the first
+* You need to add a commit-msg hook script when you clone a repo for the first
time using a snippet you can find e.g. https://gerrit-review.googlesource.com/admin/repos/gerrit[here,role=external,window=_blank];
* Your review will be on a single commit instead of a branch. You use
`git commit --amend` to modify a code change.
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index df83f1a..eb38434 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3964,6 +3964,8 @@
|`copy_any_score`|`false` if not set|
Whether link:config-labels.html#label_copyAnyScore[copyAnyScore] is set on the
label.
+|`copy_condition`|optional|
+See link:config-labels.html#label_copyCondition[copyCondition].
|`copy_min_score`|`false` if not set|
Whether link:config-labels.html#label_copyMinScore[copyMinScore] is set on the
label.
@@ -4034,6 +4036,10 @@
|`copy_any_score`|optional|
Whether link:config-labels.html#label_copyAnyScore[copyAnyScore] is set on the
label.
+|`copy_condition`|optional|
+See link:config-labels.html#label_copyCondition[copyCondition].
+|`unset_copy_condition`|optional|
+If true, clears the value stored in `copy_condition`.
|`copy_min_score`|optional|
Whether link:config-labels.html#label_copyMinScore[copyMinScore] is set on the
label.
diff --git a/e2e-tests/src/test/resources/hooks/commit-msg b/e2e-tests/src/test/resources/hooks/commit-msg
deleted file mode 100644
index b05a671..0000000
--- a/e2e-tests/src/test/resources/hooks/commit-msg
+++ /dev/null
@@ -1,43 +0,0 @@
-#!/bin/sh
-#
-# Part of Gerrit Code Review (https://www.gerritcodereview.com/)
-#
-# Copyright (C) 2009 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-# avoid [[ which is not POSIX sh.
-if test "$#" != 1 ; then
- echo "$0 requires an argument."
- exit 1
-fi
-
-if test ! -f "$1" ; then
- echo "file does not exist: $1"
- exit 1
-fi
-
-if test ! -s "$1" ; then
- echo "file is empty: $1"
- exit 1
-fi
-
-# $RANDOM will be undefined if not using bash, so don't use set -u
-random=$( (whoami ; hostname ; date; cat $1 ; echo $RANDOM) | git hash-object --stdin)
-dest="$1.tmp.${random}"
-
-# Avoid the --in-place option which only appeared in Git 2.8
-# Avoid the --if-exists option which only appeared in Git 2.15
-cat "$1" \
-| git -c trailer.ifexists=doNothing interpret-trailers --trailer "Change-Id: I${random}" > "${dest}" \
-&& mv "${dest}" "$1"
diff --git a/e2e-tests/src/test/resources/hooks/commit-msg b/e2e-tests/src/test/resources/hooks/commit-msg
new file mode 120000
index 0000000..6066256
--- /dev/null
+++ b/e2e-tests/src/test/resources/hooks/commit-msg
@@ -0,0 +1 @@
+../../../../../resources/com/google/gerrit/server/tools/root/hooks/commit-msg
\ No newline at end of file
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index 67e26ec..a613588 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -30,7 +30,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.query.change.ChangeData;
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeKindCreator.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeKindCreator.java
new file mode 100644
index 0000000..cb987da
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeKindCreator.java
@@ -0,0 +1,348 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static org.eclipse.jgit.lib.Constants.HEAD;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.RevisionApi;
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.inject.Inject;
+import java.util.List;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/** Helper to create changes of a certain {@link ChangeKind}. */
+public class ChangeKindCreator {
+ private GerritApi gApi;
+ private PushOneCommit.Factory pushFactory;
+ private RequestScopeOperations requestScopeOperations;
+ private ProjectOperations projectOperations;
+
+ @Inject
+ private ChangeKindCreator(
+ GerritApi gApi,
+ PushOneCommit.Factory pushFactory,
+ RequestScopeOperations requestScopeOperations,
+ ProjectOperations projectOperations) {
+ this.gApi = gApi;
+ this.pushFactory = pushFactory;
+ this.requestScopeOperations = requestScopeOperations;
+ this.projectOperations = projectOperations;
+ }
+
+ /** Creates a change with the given {@link ChangeKind} and returns the change id. */
+ public String createChange(
+ ChangeKind kind, TestRepository<InMemoryRepository> testRepo, TestAccount user)
+ throws Exception {
+ switch (kind) {
+ case NO_CODE_CHANGE:
+ case REWORK:
+ case TRIVIAL_REBASE:
+ case NO_CHANGE:
+ return createChange(testRepo, user).getChangeId();
+ case MERGE_FIRST_PARENT_UPDATE:
+ return createChangeForMergeCommit(testRepo, user);
+ default:
+ throw new IllegalStateException("unexpected change kind: " + kind);
+ }
+ }
+
+ /** Updates a change with the given {@link ChangeKind}. */
+ public void updateChange(
+ String changeId,
+ ChangeKind changeKind,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ switch (changeKind) {
+ case NO_CODE_CHANGE:
+ noCodeChange(changeId, testRepo, user, project);
+ return;
+ case REWORK:
+ rework(changeId, testRepo, user, project);
+ return;
+ case TRIVIAL_REBASE:
+ trivialRebase(changeId, testRepo, user, project);
+ return;
+ case MERGE_FIRST_PARENT_UPDATE:
+ updateFirstParent(changeId, testRepo, user);
+ return;
+ case NO_CHANGE:
+ noChange(changeId, testRepo, user, project);
+ return;
+ default:
+ assertWithMessage("unexpected change kind: " + changeKind).fail();
+ }
+ }
+
+ /**
+ * Creates a cherry pick of the provided change with the given {@link ChangeKind} and returns the
+ * change id.
+ */
+ public String cherryPick(
+ String changeId,
+ ChangeKind changeKind,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ switch (changeKind) {
+ case REWORK:
+ case TRIVIAL_REBASE:
+ break;
+ case NO_CODE_CHANGE:
+ case NO_CHANGE:
+ case MERGE_FIRST_PARENT_UPDATE:
+ default:
+ assertWithMessage("unexpected change kind: " + changeKind).fail();
+ }
+
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ user.newIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT,
+ "other.txt",
+ "new content " + System.nanoTime())
+ .to("refs/for/master");
+ r.assertOkStatus();
+ vote(user, r.getChangeId(), 2, 1);
+ merge(r);
+
+ String subject =
+ ChangeKind.TRIVIAL_REBASE.equals(changeKind)
+ ? PushOneCommit.SUBJECT
+ : "Reworked change " + System.nanoTime();
+ CherryPickInput in = new CherryPickInput();
+ in.destination = "master";
+ in.message = String.format("%s\n\nChange-Id: %s", subject, changeId);
+ ChangeInfo c = gApi.changes().id(changeId).current().cherryPick(in).get();
+ return c.changeId;
+ }
+
+ /** Creates a change that is a merge {@link ChangeKind} and returns the change id. */
+ public String createChangeForMergeCommit(
+ TestRepository<InMemoryRepository> testRepo, TestAccount user) throws Exception {
+ ObjectId initial = testRepo.getRepository().exactRef(HEAD).getLeaf().getObjectId();
+
+ PushOneCommit.Result parent1 = createChange("parent 1", "p1.txt", "content 1", testRepo, user);
+
+ testRepo.reset(initial);
+ PushOneCommit.Result parent2 = createChange("parent 2", "p2.txt", "content 2", testRepo, user);
+
+ testRepo.reset(parent1.getCommit());
+
+ PushOneCommit merge = pushFactory.create(user.newIdent(), testRepo);
+ merge.setParents(ImmutableList.of(parent1.getCommit(), parent2.getCommit()));
+ PushOneCommit.Result result = merge.to("refs/for/master");
+ result.assertOkStatus();
+ return result.getChangeId();
+ }
+
+ /** Update the first parent of a merge. */
+ public void updateFirstParent(
+ String changeId, TestRepository<InMemoryRepository> testRepo, TestAccount user)
+ throws Exception {
+ ChangeInfo c = detailedChange(changeId);
+ List<CommitInfo> parents = c.revisions.get(c.currentRevision).commit.parents;
+ String parent1 = parents.get(0).commit;
+ String parent2 = parents.get(1).commit;
+ RevCommit commitParent2 = testRepo.getRevWalk().parseCommit(ObjectId.fromString(parent2));
+
+ testRepo.reset(parent1);
+ PushOneCommit.Result newParent1 =
+ createChange("new parent 1", "p1-1.txt", "content 1-1", testRepo, user);
+
+ PushOneCommit merge = pushFactory.create(user.newIdent(), testRepo, changeId);
+ merge.setParents(ImmutableList.of(newParent1.getCommit(), commitParent2));
+ PushOneCommit.Result result = merge.to("refs/for/master");
+ result.assertOkStatus();
+
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.MERGE_FIRST_PARENT_UPDATE);
+ }
+
+ /** Update the second parent of a merge. */
+ public void updateSecondParent(
+ String changeId, TestRepository<InMemoryRepository> testRepo, TestAccount user)
+ throws Exception {
+ ChangeInfo c = detailedChange(changeId);
+ List<CommitInfo> parents = c.revisions.get(c.currentRevision).commit.parents;
+ String parent1 = parents.get(0).commit;
+ String parent2 = parents.get(1).commit;
+ RevCommit commitParent1 = testRepo.getRevWalk().parseCommit(ObjectId.fromString(parent1));
+
+ testRepo.reset(parent2);
+ PushOneCommit.Result newParent2 =
+ createChange("new parent 2", "p2-2.txt", "content 2-2", testRepo, user);
+
+ PushOneCommit merge = pushFactory.create(user.newIdent(), testRepo, changeId);
+ merge.setParents(ImmutableList.of(commitParent1, newParent2.getCommit()));
+ PushOneCommit.Result result = merge.to("refs/for/master");
+ result.assertOkStatus();
+
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.REWORK);
+ }
+
+ private void noCodeChange(
+ String changeId,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ TestRepository<?>.CommitBuilder commitBuilder =
+ testRepo.amendRef("HEAD").insertChangeId(changeId.substring(1));
+ commitBuilder
+ .message("New subject " + System.nanoTime())
+ .author(user.newIdent())
+ .committer(new PersonIdent(user.newIdent(), testRepo.getDate()));
+ commitBuilder.create();
+ GitUtil.pushHead(testRepo, "refs/for/master", false);
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.NO_CODE_CHANGE);
+ }
+
+ private void noChange(
+ String changeId,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ String commitMessage = change.revisions.get(change.currentRevision).commit.message;
+
+ TestRepository<?>.CommitBuilder commitBuilder =
+ testRepo.amendRef("HEAD").insertChangeId(changeId.substring(1));
+ commitBuilder
+ .message(commitMessage)
+ .author(user.newIdent())
+ .committer(new PersonIdent(user.newIdent(), testRepo.getDate()));
+ commitBuilder.create();
+ GitUtil.pushHead(testRepo, "refs/for/master", false);
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.NO_CHANGE);
+ }
+
+ private void rework(
+ String changeId,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ PushOneCommit push =
+ pushFactory.create(
+ user.newIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT,
+ PushOneCommit.FILE_NAME,
+ "new content " + System.nanoTime(),
+ changeId);
+ push.to("refs/for/master").assertOkStatus();
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.REWORK);
+ }
+
+ private void trivialRebase(
+ String changeId,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user,
+ Project.NameKey project)
+ throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+ PushOneCommit push =
+ pushFactory.create(
+ user.newIdent(),
+ testRepo,
+ "Other Change",
+ "a" + System.nanoTime() + ".txt",
+ PushOneCommit.FILE_CONTENT);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ ReviewInput in = new ReviewInput().label(LabelId.CODE_REVIEW, 2).label(LabelId.VERIFIED, 1);
+ revision.review(in);
+ revision.submit();
+
+ gApi.changes().id(changeId).current().rebase();
+ assertThat(getChangeKind(changeId)).isEqualTo(ChangeKind.TRIVIAL_REBASE);
+ }
+
+ private ChangeKind getChangeKind(String changeId) throws Exception {
+ ChangeInfo c = gApi.changes().id(changeId).get(ListChangesOption.CURRENT_REVISION);
+ return c.revisions.get(c.currentRevision).kind;
+ }
+
+ private PushOneCommit.Result createChange(
+ TestRepository<InMemoryRepository> testRepo, TestAccount user) throws Exception {
+ PushOneCommit push = pushFactory.create(user.newIdent(), testRepo);
+ PushOneCommit.Result result = push.to("refs/for/master");
+ result.assertOkStatus();
+ return result;
+ }
+
+ private ChangeInfo detailedChange(String changeId) throws Exception {
+ return gApi.changes()
+ .id(changeId)
+ .get(
+ ListChangesOption.DETAILED_LABELS,
+ ListChangesOption.CURRENT_REVISION,
+ ListChangesOption.CURRENT_COMMIT);
+ }
+
+ private PushOneCommit.Result createChange(
+ String subject,
+ String fileName,
+ String content,
+ TestRepository<InMemoryRepository> testRepo,
+ TestAccount user)
+ throws Exception {
+ PushOneCommit push = pushFactory.create(user.newIdent(), testRepo, subject, fileName, content);
+ return push.to("refs/for/master");
+ }
+
+ private void vote(TestAccount user, String changeId, int codeReviewVote, int verifiedVote)
+ throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ ReviewInput in =
+ new ReviewInput()
+ .label(LabelId.CODE_REVIEW, codeReviewVote)
+ .label(LabelId.VERIFIED, verifiedVote);
+ gApi.changes().id(changeId).current().review(in);
+ }
+
+ private void merge(PushOneCommit.Result r) throws Exception {
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ }
+}
diff --git a/java/com/google/gerrit/entities/LabelType.java b/java/com/google/gerrit/entities/LabelType.java
index 9649642..d254752 100644
--- a/java/com/google/gerrit/entities/LabelType.java
+++ b/java/com/google/gerrit/entities/LabelType.java
@@ -24,6 +24,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
@AutoValue
public abstract class LabelType {
@@ -128,6 +129,8 @@
public abstract boolean isCanOverride();
+ public abstract Optional<String> getCopyCondition();
+
@Nullable
public abstract ImmutableList<String> getRefPatterns();
@@ -239,6 +242,8 @@
public abstract Builder setCopyAnyScore(boolean copyAnyScore);
+ public abstract Builder setCopyCondition(@Nullable String copyCondition);
+
public abstract Builder setCopyMinScore(boolean copyMinScore);
public abstract Builder setCopyMaxScore(boolean copyMaxScore);
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
index 9a6d086..6f733d6 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
@@ -26,6 +26,7 @@
public List<String> branches;
public Boolean canOverride;
public Boolean copyAnyScore;
+ public String copyCondition;
public Boolean copyMinScore;
public Boolean copyMaxScore;
public Boolean copyAllScoresIfListOfFilesDidNotChange;
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
index 87cae86..38b76c1 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
@@ -25,6 +25,8 @@
public List<String> branches;
public Boolean canOverride;
public Boolean copyAnyScore;
+ public String copyCondition;
+ public Boolean unsetCopyCondition;
public Boolean copyMinScore;
public Boolean copyMaxScore;
public Boolean copyAllScoresIfListOfFilesDidNotChange;
diff --git a/java/com/google/gerrit/index/query/IndexPredicate.java b/java/com/google/gerrit/index/query/IndexPredicate.java
index aac6682..7bbe70b 100644
--- a/java/com/google/gerrit/index/query/IndexPredicate.java
+++ b/java/com/google/gerrit/index/query/IndexPredicate.java
@@ -14,11 +14,30 @@
package com.google.gerrit.index.query;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.common.primitives.Ints;
+import com.google.common.primitives.Longs;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.FieldType;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.StreamSupport;
/** Predicate that is mapped to a field in the index. */
-public abstract class IndexPredicate<I> extends OperatorPredicate<I> {
+public abstract class IndexPredicate<I> extends OperatorPredicate<I> implements Matchable<I> {
+ /**
+ * Text segmentation to be applied to both the query string and the indexed field for full-text
+ * queries. This is inspired by http://unicode.org/reports/tr29/ which is what Lucene uses, but
+ * complexity was reduced to the bare minimum at the cost of small discrepancies to the Unicode
+ * spec.
+ */
+ private static final Splitter FULL_TEXT_SPLITTER = Splitter.on(CharMatcher.anyOf(" ,.-\\/_\n"));
+
private final FieldDef<I, ?> def;
protected IndexPredicate(FieldDef<I, ?> def, String value) {
@@ -38,4 +57,70 @@
public FieldType<?> getType() {
return def.getType();
}
+
+ /**
+ * This method matches documents without calling an index subsystem. For primitive fields (e.g.
+ * integer, long) , the matching logic is consistent across this method and all known index
+ * implementations. For text fields (i.e. prefix and full-text) the semantics vary between this
+ * implementation and known index implementations:
+ * <li>Prefix: Lucene as well as {@link #match(I)} matches terms as true prefixes (prefix:foo ->
+ * `foo bar` matches, but `baz foo bar` does not match). The index implementation at Google
+ * tokenizes both the query and the indexed text and matches tokens individually (prefix:fo ba
+ * -> `baz foo bar` matches).
+ * <li>Full text: Lucene uses a {@code PhraseQuery} to search for terms in full text fields
+ * in-order. The index implementation at Google as well as {@link #match(I)} tokenizes both
+ * the query and the indexed text and matches tokens individually.
+ *
+ * @return true if the predicate matches the provided {@link I}.
+ */
+ @Override
+ public boolean match(I doc) {
+ if (getField().isRepeatable()) {
+ Iterable<Object> values = (Iterable<Object>) getField().get(doc);
+ for (Object v : values) {
+ if (matchesSingleObject(v)) {
+ return true;
+ }
+ }
+ return false;
+ } else {
+ return matchesSingleObject(getField().get(doc));
+ }
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+
+ private boolean matchesSingleObject(Object fieldValueFromObject) {
+ String fieldTypeName = getField().getType().getName();
+ if (fieldTypeName.equals(FieldType.INTEGER.getName())) {
+ return Objects.equals(fieldValueFromObject, Ints.tryParse(value));
+ } else if (fieldTypeName.equals(FieldType.EXACT.getName())) {
+ return Objects.equals(fieldValueFromObject, value);
+ } else if (fieldTypeName.equals(FieldType.LONG.getName())) {
+ return Objects.equals(fieldValueFromObject, Longs.tryParse(value));
+ } else if (fieldTypeName.equals(FieldType.PREFIX.getName())) {
+ return String.valueOf(fieldValueFromObject).startsWith(value);
+ } else if (fieldTypeName.equals(FieldType.FULL_TEXT.getName())) {
+ Set<String> tokenizedField = tokenizeString(String.valueOf(fieldValueFromObject));
+ Set<String> tokenizedValue = tokenizeString(value);
+ return !Sets.intersection(tokenizedField, tokenizedValue).isEmpty();
+ } else if (fieldTypeName.equals(FieldType.STORED_ONLY.getName())) {
+ throw new IllegalStateException("can't filter for storedOnly field " + getField().getName());
+ } else if (fieldTypeName.equals(FieldType.TIMESTAMP.getName())) {
+ throw new IllegalStateException("timestamp queries must be handled in subclasses");
+ } else if (fieldTypeName.equals(FieldType.INTEGER_RANGE.getName())) {
+ throw new IllegalStateException("integer range queries must be handled in subclasses");
+ } else {
+ throw new IllegalStateException("unrecognized field " + fieldTypeName);
+ }
+ }
+
+ private static ImmutableSet<String> tokenizeString(String value) {
+ return StreamSupport.stream(FULL_TEXT_SPLITTER.split(value.toLowerCase()).spliterator(), false)
+ .filter(s -> !s.trim().isEmpty())
+ .collect(toImmutableSet());
+ }
}
diff --git a/java/com/google/gerrit/index/query/IntegerRangePredicate.java b/java/com/google/gerrit/index/query/IntegerRangePredicate.java
index 6780867..850c4a5 100644
--- a/java/com/google/gerrit/index/query/IntegerRangePredicate.java
+++ b/java/com/google/gerrit/index/query/IntegerRangePredicate.java
@@ -31,6 +31,7 @@
protected abstract Integer getValueInt(T object);
+ @Override
public boolean match(T object) {
Integer valueInt = getValueInt(object);
if (valueInt == null) {
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 2cfc49f..30de2f5 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.ServiceUserClassifierImpl;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
+import com.google.gerrit.server.approval.ApprovalCacheImpl;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.cache.h2.H2CacheModule;
import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
@@ -79,6 +80,7 @@
import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.restapi.group.GroupModule;
@@ -171,6 +173,7 @@
modules.add(new GroupModule());
modules.add(new NoteDbModule());
modules.add(AccountCacheImpl.module());
+ modules.add(ApprovalCacheImpl.module());
modules.add(DefaultPreferencesCacheImpl.module());
modules.add(GroupCacheImpl.module());
modules.add(GroupIncludeCacheImpl.module());
@@ -181,6 +184,7 @@
modules.add(ServiceUserClassifierImpl.module());
modules.add(TagCache.module());
modules.add(PureRevertCache.module());
+ modules.add(new ApprovalModule());
factory(CapabilityCollection.Factory.class);
factory(ChangeData.AssistedFactory.class);
factory(ChangeIsVisibleToPredicate.Factory.class);
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 404906d..f9195c0 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -131,6 +131,7 @@
"//lib/ow2:ow2-asm-util",
"//lib/prolog:runtime",
"//proto:cache_java_proto",
+ "//proto:entities_java_proto",
],
)
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index 005ae3b..d60bc8f 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -29,6 +29,7 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -149,8 +150,7 @@
projectCache.get(notes.getProjectName()).orElseThrow(illegalState(notes.getProjectName()));
ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
- for (PatchSetApproval ap :
- approvalsUtil.byPatchSet(notes, change.currentPatchSetId(), null, null)) {
+ for (PatchSetApproval ap : approvalsUtil.byPatchSet(notes, change.currentPatchSetId())) {
LabelType type = projectState.getLabelTypes(notes).byLabel(ap.label());
if (type != null && ap.value() == 1 && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
return true;
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index ab96c6b..764c46d 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -57,9 +57,9 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.FileResource;
import com.google.gerrit.server.change.RebaseUtil;
import com.google.gerrit.server.change.RevisionResource;
@@ -642,7 +642,7 @@
ListMultimapBuilder.treeKeys().arrayListValues().build();
try {
Iterable<PatchSetApproval> approvals =
- approvalsUtil.byPatchSet(revision.getNotes(), revision.getPatchSet().id(), null, null);
+ approvalsUtil.byPatchSet(revision.getNotes(), revision.getPatchSet().id());
AccountLoader accountLoader =
accountLoaderFactory.create(
EnumSet.of(
diff --git a/java/com/google/gerrit/server/approval/ApprovalCache.java b/java/com/google/gerrit/server/approval/ApprovalCache.java
new file mode 100644
index 0000000..5637249
--- /dev/null
+++ b/java/com/google/gerrit/server/approval/ApprovalCache.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.approval;
+
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.server.notedb.ChangeNotes;
+
+/**
+ * Cache that holds approvals per patch set and NoteDb state. This includes approvals copied forward
+ * from older patch sets.
+ */
+public interface ApprovalCache {
+ /** Returns {@link PatchSetApproval}s for the given patch set. */
+ Iterable<PatchSetApproval> get(ChangeNotes notes, PatchSet.Id psId);
+}
diff --git a/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java b/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java
new file mode 100644
index 0000000..93099eb
--- /dev/null
+++ b/java/com/google/gerrit/server/approval/ApprovalCacheImpl.java
@@ -0,0 +1,133 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.approval;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.proto.Entities;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.proto.Cache;
+import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
+import com.google.gerrit.server.cache.serialize.ProtobufSerializer;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+import com.google.protobuf.ByteString;
+import java.util.concurrent.ExecutionException;
+
+/** @see ApprovalCache */
+public class ApprovalCacheImpl implements ApprovalCache {
+ private static final String CACHE_NAME = "approvals";
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ bind(ApprovalCache.class).to(ApprovalCacheImpl.class);
+ persist(
+ CACHE_NAME,
+ Cache.PatchSetApprovalsKeyProto.class,
+ Cache.AllPatchSetApprovalsProto.class)
+ .version(1)
+ .loader(Loader.class)
+ .keySerializer(new ProtobufSerializer<>(Cache.PatchSetApprovalsKeyProto.parser()))
+ .valueSerializer(new ProtobufSerializer<>(Cache.AllPatchSetApprovalsProto.parser()));
+ }
+ };
+ }
+
+ private final LoadingCache<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto>
+ cache;
+
+ @Inject
+ ApprovalCacheImpl(
+ @Named(CACHE_NAME)
+ LoadingCache<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto> cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public Iterable<PatchSetApproval> get(ChangeNotes notes, PatchSet.Id psId) {
+ try {
+ return fromProto(
+ cache.get(
+ Cache.PatchSetApprovalsKeyProto.newBuilder()
+ .setChangeId(notes.getChangeId().get())
+ .setPatchSetId(psId.get())
+ .setProject(notes.getProjectName().get())
+ .setId(
+ ByteString.copyFrom(
+ ObjectIdCacheSerializer.INSTANCE.serialize(notes.getMetaId())))
+ .build()));
+ } catch (ExecutionException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ @Singleton
+ static class Loader
+ extends CacheLoader<Cache.PatchSetApprovalsKeyProto, Cache.AllPatchSetApprovalsProto> {
+ private final ApprovalInference approvalInference;
+ private final ChangeNotes.Factory changeNotesFactory;
+
+ @Inject
+ Loader(ApprovalInference approvalInference, ChangeNotes.Factory changeNotesFactory) {
+ this.approvalInference = approvalInference;
+ this.changeNotesFactory = changeNotesFactory;
+ }
+
+ @Override
+ public Cache.AllPatchSetApprovalsProto load(Cache.PatchSetApprovalsKeyProto key)
+ throws Exception {
+ Change.Id changeId = Change.id(key.getChangeId());
+ return toProto(
+ approvalInference.forPatchSet(
+ changeNotesFactory.createChecked(
+ Project.nameKey(key.getProject()),
+ changeId,
+ ObjectIdCacheSerializer.INSTANCE.deserialize(key.getId().toByteArray())),
+ PatchSet.id(changeId, key.getPatchSetId()),
+ null
+ /* revWalk= */ ,
+ null
+ /* repoConfig= */ ));
+ }
+ }
+
+ private static Iterable<PatchSetApproval> fromProto(Cache.AllPatchSetApprovalsProto proto) {
+ ImmutableList.Builder<PatchSetApproval> builder = ImmutableList.builder();
+ for (Entities.PatchSetApproval psa : proto.getApprovalList()) {
+ builder.add(PatchSetApprovalProtoConverter.INSTANCE.fromProto(psa));
+ }
+ return builder.build();
+ }
+
+ private static Cache.AllPatchSetApprovalsProto toProto(Iterable<PatchSetApproval> autoValue) {
+ Cache.AllPatchSetApprovalsProto.Builder builder = Cache.AllPatchSetApprovalsProto.newBuilder();
+ for (PatchSetApproval psa : autoValue) {
+ builder.addApproval(PatchSetApprovalProtoConverter.INSTANCE.toProto(psa));
+ }
+ return builder.build();
+ }
+}
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/approval/ApprovalInference.java
similarity index 87%
rename from java/com/google/gerrit/server/ApprovalInference.java
rename to java/com/google/gerrit/server/approval/ApprovalInference.java
index 675c470..0185598 100644
--- a/java/com/google/gerrit/server/ApprovalInference.java
+++ b/java/com/google/gerrit/server/approval/ApprovalInference.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server;
+package com.google.gerrit.server.approval;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
@@ -26,12 +26,12 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.LabelTypes;
-import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.change.LabelNormalizer;
import com.google.gerrit.server.logging.Metadata;
@@ -44,6 +44,11 @@
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.approval.ApprovalContext;
+import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
+import com.google.gerrit.server.query.approval.ListOfFilesUnchangedPredicate;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Collection;
@@ -61,24 +66,33 @@
* submit time, or refreshed on demand, as when reading approvals from the NoteDb.
*/
@Singleton
-public class ApprovalInference {
+class ApprovalInference {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ProjectCache projectCache;
private final ChangeKindCache changeKindCache;
private final LabelNormalizer labelNormalizer;
private final PatchListCache patchListCache;
+ private final ApprovalQueryBuilder approvalQueryBuilder;
+ private final OneOffRequestContext requestContext;
+ private final ListOfFilesUnchangedPredicate listOfFilesUnchangedPredicate;
@Inject
ApprovalInference(
ProjectCache projectCache,
ChangeKindCache changeKindCache,
LabelNormalizer labelNormalizer,
- PatchListCache patchListCache) {
+ PatchListCache patchListCache,
+ ApprovalQueryBuilder approvalQueryBuilder,
+ OneOffRequestContext requestContext,
+ ListOfFilesUnchangedPredicate listOfFilesUnchangedPredicate) {
this.projectCache = projectCache;
this.changeKindCache = changeKindCache;
this.labelNormalizer = labelNormalizer;
this.patchListCache = patchListCache;
+ this.approvalQueryBuilder = approvalQueryBuilder;
+ this.requestContext = requestContext;
+ this.listOfFilesUnchangedPredicate = listOfFilesUnchangedPredicate;
}
/**
@@ -105,7 +119,7 @@
}
}
- private static boolean canCopy(
+ private boolean canCopyBasedOnBooleanLabelConfigs(
ProjectState project,
PatchSetApproval psa,
PatchSet.Id psId,
@@ -172,12 +186,7 @@
project.getName());
return true;
} else if (type.isCopyAllScoresIfListOfFilesDidNotChange()
- && patchList.getPatches().stream()
- .noneMatch(
- p ->
- p.getChangeType() == ChangeType.ADDED
- || p.getChangeType() == ChangeType.DELETED
- || p.getChangeType() == ChangeType.RENAMED)) {
+ && listOfFilesUnchangedPredicate.match(patchList)) {
logger.atFine().log(
"approval %d on label %s of patch set %d of change %d can be copied"
+ " to patch set %d because the label has set "
@@ -309,6 +318,31 @@
}
}
+ private boolean canCopyBasedOnCopyCondition(
+ ChangeNotes changeNotes,
+ PatchSetApproval psa,
+ PatchSet.Id psId,
+ LabelType type,
+ ChangeKind changeKind) {
+ if (!type.getCopyCondition().isPresent()) {
+ return false;
+ }
+ ApprovalContext ctx = ApprovalContext.create(changeNotes, psa, psId, changeKind);
+ try {
+ // Use a request context to run checks as an internal user with expanded visibility. This is
+ // so that the output of the copy condition does not depend on who is running the current
+ // request (e.g. a group used in this query might not be visible to the person sending this
+ // request).
+ try (ManualRequestContext ignored = requestContext.open()) {
+ return approvalQueryBuilder.parse(type.getCopyCondition().get()).asMatchable().match(ctx);
+ }
+ } catch (QueryParseException e) {
+ logger.atWarning().withCause(e).log(
+ "Unable to copy label because config is invalid. This should have been caught before.");
+ return false;
+ }
+ }
+
private Collection<PatchSetApproval> getForPatchSetWithoutNormalization(
ChangeNotes notes,
ProjectState project,
@@ -380,7 +414,8 @@
if (patchList == null && type != null && type.isCopyAllScoresIfListOfFilesDidNotChange()) {
patchList = getPatchList(project, ps, priorPatchSet);
}
- if (!canCopy(project, psa, ps.id(), kind, type, patchList)) {
+ if (!canCopyBasedOnBooleanLabelConfigs(project, psa, ps.id(), kind, type, patchList)
+ && !canCopyBasedOnCopyCondition(notes, psa, ps.id(), type, kind)) {
continue;
}
resultByUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
similarity index 95%
rename from java/com/google/gerrit/server/ApprovalsUtil.java
rename to java/com/google/gerrit/server/approval/ApprovalsUtil.java
index 411768d..b1e85e9 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server;
+package com.google.gerrit.server.approval;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
@@ -39,6 +39,9 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -94,16 +97,19 @@
private final ApprovalInference approvalInference;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
+ private final ApprovalCache approvalCache;
@VisibleForTesting
@Inject
public ApprovalsUtil(
ApprovalInference approvalInference,
PermissionBackend permissionBackend,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ApprovalCache approvalCache) {
this.approvalInference = approvalInference;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
+ this.approvalCache = approvalCache;
}
/**
@@ -338,6 +344,10 @@
return approvalInference.forPatchSet(notes, psId, rw, repoConfig);
}
+ public Iterable<PatchSetApproval> byPatchSet(ChangeNotes notes, PatchSet.Id psId) {
+ return approvalCache.get(notes, psId);
+ }
+
public Iterable<PatchSetApproval> byPatchSetUser(
ChangeNotes notes,
PatchSet.Id psId,
@@ -347,6 +357,11 @@
return filterApprovals(byPatchSet(notes, psId, rw, repoConfig), accountId);
}
+ public Iterable<PatchSetApproval> byPatchSetUser(
+ ChangeNotes notes, PatchSet.Id psId, Account.Id accountId) {
+ return filterApprovals(byPatchSet(notes, psId), accountId);
+ }
+
public PatchSetApproval getSubmitter(ChangeNotes notes, PatchSet.Id c) {
if (c == null) {
return null;
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializer.java
index 4627cdb..c00961f 100644
--- a/java/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializer.java
+++ b/java/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializer.java
@@ -18,6 +18,7 @@
import com.google.common.base.Converter;
import com.google.common.base.Enums;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Shorts;
import com.google.gerrit.entities.LabelFunction;
@@ -39,6 +40,7 @@
.setAllowPostSubmit(proto.getAllowPostSubmit())
.setIgnoreSelfApproval(proto.getIgnoreSelfApproval())
.setDefaultValue(Shorts.saturatedCast(proto.getDefaultValue()))
+ .setCopyCondition(Strings.emptyToNull(proto.getCopyCondition()))
.setCopyAnyScore(proto.getCopyAnyScore())
.setCopyMinScore(proto.getCopyMinScore())
.setCopyMaxScore(proto.getCopyMaxScore())
@@ -67,6 +69,7 @@
.map(LabelValueSerializer::serialize)
.collect(toImmutableList()))
.setFunction(FUNCTION_CONVERTER.reverse().convert(autoValue.getFunction()))
+ .setCopyCondition(autoValue.getCopyCondition().orElse(""))
.setCopyAnyScore(autoValue.isCopyAnyScore())
.setCopyMinScore(autoValue.isCopyMinScore())
.setCopyMaxScore(autoValue.isCopyMaxScore())
diff --git a/java/com/google/gerrit/server/change/AddReviewersOp.java b/java/com/google/gerrit/server/change/AddReviewersOp.java
index 1d6fb3c..a333ce5 100644
--- a/java/com/google/gerrit/server/change/AddReviewersOp.java
+++ b/java/com/google/gerrit/server/change/AddReviewersOp.java
@@ -33,10 +33,10 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ProjectCache;
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index c067fcb..6728ba2 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -45,10 +45,10 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerModifier.InternalReviewerInput;
import com.google.gerrit.server.change.ReviewerModifier.ReviewerModification;
import com.google.gerrit.server.change.ReviewerModifier.ReviewerModificationList;
diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index 3729b59..27b71d6 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -30,12 +30,12 @@
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestResource.HasETag;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 64472ea..3a12ad4 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -29,12 +29,12 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.extensions.events.ReviewerDeleted;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
diff --git a/java/com/google/gerrit/server/change/DeleteReviewersUtil.java b/java/com/google/gerrit/server/change/DeleteReviewersUtil.java
index a4f306b..3212c8d 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewersUtil.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewersUtil.java
@@ -20,8 +20,8 @@
import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 647fdf0..d25dba0 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -29,11 +29,11 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.RevisionCreated;
import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java
index 761b57d..d5b74a8 100644
--- a/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -27,8 +27,8 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -94,7 +94,7 @@
out,
reviewerAccountId,
cd,
- approvalsUtil.byPatchSetUser(cd.notes(), psId, reviewerAccountId, null, null));
+ approvalsUtil.byPatchSetUser(cd.notes(), psId, reviewerAccountId));
}
public ReviewerInfo format(
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 4794858..5a74c78 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -77,7 +77,6 @@
import com.google.gerrit.extensions.webui.TopMenu;
import com.google.gerrit.extensions.webui.WebUiPlugin;
import com.google.gerrit.server.AnonymousUser;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CmdLineParserModule;
import com.google.gerrit.server.CreateGroupPermissionSyncer;
import com.google.gerrit.server.DynamicOptions;
@@ -101,6 +100,8 @@
import com.google.gerrit.server.account.ServiceUserClassifierImpl;
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
+import com.google.gerrit.server.approval.ApprovalCacheImpl;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.auth.AuthBackend;
import com.google.gerrit.server.auth.UniversalAuthBackend;
import com.google.gerrit.server.avatar.AvatarProvider;
@@ -179,6 +180,7 @@
import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
@@ -237,6 +239,7 @@
bind(RulesCache.class);
bind(BlameCache.class).to(BlameCacheImpl.class);
install(AccountCacheImpl.module());
+ install(ApprovalCacheImpl.module());
install(BatchUpdate.module());
install(ChangeKindCacheImpl.module());
install(ChangeFinder.module());
@@ -270,6 +273,7 @@
install(new SshAddressesModule());
install(new FileInfoJsonModule(cfg));
install(ThreadLocalRequestContext.module());
+ install(new ApprovalModule());
factory(CapabilityCollection.Factory.class);
factory(ChangeData.AssistedFactory.class);
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index caf495f..d3faac1 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -35,12 +35,12 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.index.IndexConfig;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.data.AccountAttribute;
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 2dbafd2..0e0185a 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -29,12 +29,12 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.NotifyResolver;
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 58df343..1da14f8 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -47,9 +47,9 @@
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -650,7 +650,7 @@
private Iterable<PatchSetApproval> safeGetApprovals(ChangeNotes notes, PatchSet.Id psId) {
try {
- return approvalsUtil.byPatchSet(notes, psId, null, null);
+ return approvalsUtil.byPatchSet(notes, psId);
} catch (StorageException e) {
logger.atSevere().withCause(e).log("Can't read approval records for %s", psId);
return Collections.emptyList();
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 15bc603..ec2ed4f 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -100,7 +100,6 @@
import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CreateGroupPermissionSyncer;
@@ -111,6 +110,7 @@
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.SetHashtagsOp;
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index cc908e4..b55e91b 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -43,11 +43,11 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.AddReviewersOp;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.change.NotifyResolver;
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 3a35d80..d805e39 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -43,7 +43,6 @@
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailMetadata;
import com.google.gerrit.mail.TextParser;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
@@ -51,6 +50,7 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.CommentAdded;
diff --git a/java/com/google/gerrit/server/mail/send/EmailArguments.java b/java/com/google/gerrit/server/mail/send/EmailArguments.java
index 808d6a4..7fff232 100644
--- a/java/com/google/gerrit/server/mail/send/EmailArguments.java
+++ b/java/com/google/gerrit/server/mail/send/EmailArguments.java
@@ -18,13 +18,13 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.AnonymousUser;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.IdentifiedUser.GenericFactory;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritInstanceName;
diff --git a/java/com/google/gerrit/server/mail/send/MergedSender.java b/java/com/google/gerrit/server/mail/send/MergedSender.java
index ea76ab8..6af2345 100644
--- a/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -78,8 +78,7 @@
try {
Table<Account.Id, String, PatchSetApproval> pos = HashBasedTable.create();
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
- for (PatchSetApproval ca :
- args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.id(), null, null)) {
+ for (PatchSetApproval ca : args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.id())) {
LabelType lt = labelTypes.byLabel(ca.labelId());
if (lt == null) {
continue;
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 9d23137..f56e933 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -579,10 +579,19 @@
@Override
protected boolean bypassMaxUpdates() {
- // Allow abandoning or submitting a change even if it would exceed the max update count.
+ return isAbandonChange() || isAttentionSetChangeOnly();
+ }
+
+ private boolean isAbandonChange() {
return status != null && status.isClosed();
}
+ private boolean isAttentionSetChangeOnly() {
+ return (plannedAttentionSetUpdates != null
+ && plannedAttentionSetUpdates.size() > 0
+ && comments.isEmpty());
+ }
+
@Override
protected CommitBuilder applyImpl(RevWalk rw, ObjectInserter ins, ObjectId curr)
throws IOException {
diff --git a/java/com/google/gerrit/server/notedb/OpenRepo.java b/java/com/google/gerrit/server/notedb/OpenRepo.java
index 351f31d..d02ec87 100644
--- a/java/com/google/gerrit/server/notedb/OpenRepo.java
+++ b/java/com/google/gerrit/server/notedb/OpenRepo.java
@@ -178,9 +178,9 @@
&& !update.bypassMaxUpdates()) {
throw new LimitExceededException(
String.format(
- "Change %s may not exceed %d updates. It may still be abandoned or submitted. To"
- + " continue working on this change, recreate it with a new Change-Id, then"
- + " abandon this one.",
+ "Change %s may not exceed %d updates. It may still be abandoned, submitted and you can add/remove"
+ + " reviewers to/from the attention-set. To continue working on this change, recreate it with a new"
+ + " Change-Id, then abandon this one.",
update.getId(), maxUpdates.get()));
}
curr = next;
diff --git a/java/com/google/gerrit/server/project/LabelDefinitionJson.java b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
index 730162f..63c9d22 100644
--- a/java/com/google/gerrit/server/project/LabelDefinitionJson.java
+++ b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
@@ -32,6 +32,7 @@
label.defaultValue = labelType.getDefaultValue();
label.branches = labelType.getRefPatterns() != null ? labelType.getRefPatterns() : null;
label.canOverride = toBoolean(labelType.isCanOverride());
+ label.copyCondition = labelType.getCopyCondition().orElse(null);
label.copyAnyScore = toBoolean(labelType.isCopyAnyScore());
label.copyMinScore = toBoolean(labelType.isCopyMinScore());
label.copyMaxScore = toBoolean(labelType.isCopyMaxScore());
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 5ac5ac7..027cf17 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -112,6 +112,7 @@
public static final String KEY_ALLOW_POST_SUBMIT = "allowPostSubmit";
public static final String KEY_IGNORE_SELF_APPROVAL = "ignoreSelfApproval";
public static final String KEY_COPY_ANY_SCORE = "copyAnyScore";
+ public static final String KEY_COPY_CONDITION = "copyCondition";
public static final String KEY_COPY_MAX_SCORE = "copyMaxScore";
public static final String KEY_COPY_ALL_SCORES_IF_LIST_OF_FILES_DID_NOT_CHANGE =
"copyAllScoresIfListOfFilesDidNotChange";
@@ -1075,6 +1076,7 @@
KEY_FUNCTION, name, Joiner.on(", ").join(LabelFunction.ALL.keySet()))));
}
label.setFunction(function.orElse(null));
+ label.setCopyCondition(rc.getString(LABEL, name, KEY_COPY_CONDITION));
if (!values.isEmpty()) {
short dv = (short) rc.getInt(LABEL, name, KEY_DEFAULT_VALUE, 0);
@@ -1664,6 +1666,11 @@
values.add(value.format().trim());
}
rc.setStringList(LABEL, name, KEY_VALUE, values);
+ if (label.getCopyCondition().isPresent()) {
+ rc.setString(LABEL, name, KEY_COPY_CONDITION, label.getCopyCondition().get());
+ } else {
+ rc.unset(LABEL, name, KEY_COPY_CONDITION);
+ }
List<String> refPatterns = label.getRefPatterns();
if (refPatterns != null && !refPatterns.isEmpty()) {
diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java
index e4da946..8f94089 100644
--- a/java/com/google/gerrit/server/query/account/AccountPredicates.java
+++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java
@@ -21,7 +21,6 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.IndexPredicate;
-import com.google.gerrit.index.query.Matchable;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryBuilder;
import com.google.gerrit.server.account.AccountState;
@@ -132,8 +131,7 @@
}
/** Predicate that is mapped to a field in the account index. */
- static class AccountPredicate extends IndexPredicate<AccountState>
- implements Matchable<AccountState> {
+ static class AccountPredicate extends IndexPredicate<AccountState> {
AccountPredicate(FieldDef<AccountState, ?> def, String value) {
super(def, value);
}
@@ -141,16 +139,6 @@
AccountPredicate(FieldDef<AccountState, ?> def, String name, String value) {
super(def, name, value);
}
-
- @Override
- public boolean match(AccountState object) {
- return true;
- }
-
- @Override
- public int getCost() {
- return 1;
- }
}
private AccountPredicates() {}
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalContext.java b/java/com/google/gerrit/server/query/approval/ApprovalContext.java
new file mode 100644
index 0000000..4c2c7e8
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ApprovalContext.java
@@ -0,0 +1,54 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.server.notedb.ChangeNotes;
+
+/** Entity representing all required information to match predicates for copying approvals. */
+@AutoValue
+public abstract class ApprovalContext {
+ /** Approval on the source patch set to be copied. */
+ public abstract PatchSetApproval patchSetApproval();
+
+ /** Target change and patch set for the approval. */
+ public abstract PatchSet.Id target();
+
+ /** {@link ChangeNotes} of the change in question. */
+ public abstract ChangeNotes changeNotes();
+
+ /** {@link ChangeKind} of the delta between the origin and target patch set. */
+ public abstract ChangeKind changeKind();
+
+ public static ApprovalContext create(
+ ChangeNotes changeNotes, PatchSetApproval psa, PatchSet.Id id, ChangeKind changeKind) {
+ checkState(
+ psa.patchSetId().changeId().equals(id.changeId()),
+ "approval and target must be the same change. got: %s, %s",
+ psa.patchSetId(),
+ id);
+ checkState(
+ psa.patchSetId().get() + 1 == id.get(),
+ "approvals can only be copied to the next consecutive patch set. got: %s, %s",
+ psa.patchSetId(),
+ id);
+ return new AutoValue_ApprovalContext(psa, id, changeNotes, changeKind);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalModule.java b/java/com/google/gerrit/server/query/approval/ApprovalModule.java
new file mode 100644
index 0000000..ff4d5ad
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ApprovalModule.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.extensions.config.FactoryModule;
+
+/** Module to bind logic related to approval copying. */
+public class ApprovalModule extends FactoryModule {
+
+ @Override
+ protected void configure() {
+ factory(MagicValuePredicate.Factory.class);
+ factory(UserInPredicate.Factory.class);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalPredicate.java b/java/com/google/gerrit/server/query/approval/ApprovalPredicate.java
new file mode 100644
index 0000000..a6f8153
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ApprovalPredicate.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.Predicate;
+
+public abstract class ApprovalPredicate extends Predicate<ApprovalContext>
+ implements Matchable<ApprovalContext> {
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
new file mode 100644
index 0000000..c3594f5
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
@@ -0,0 +1,105 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.GroupDescription;
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryBuilder;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.group.GroupResolver;
+import com.google.inject.Inject;
+import java.util.Arrays;
+
+public class ApprovalQueryBuilder extends QueryBuilder<ApprovalContext, ApprovalQueryBuilder> {
+ private static final QueryBuilder.Definition<ApprovalContext, ApprovalQueryBuilder> mydef =
+ new QueryBuilder.Definition<>(ApprovalQueryBuilder.class);
+
+ private final MagicValuePredicate.Factory magicValuePredicate;
+ private final UserInPredicate.Factory userInPredicate;
+ private final GroupResolver groupResolver;
+ private final GroupControl.Factory groupControl;
+ private final ListOfFilesUnchangedPredicate listOfFilesUnchangedPredicate;
+
+ @Inject
+ protected ApprovalQueryBuilder(
+ MagicValuePredicate.Factory magicValuePredicate,
+ UserInPredicate.Factory userInPredicate,
+ GroupResolver groupResolver,
+ GroupControl.Factory groupControl,
+ ListOfFilesUnchangedPredicate listOfFilesUnchangedPredicate) {
+ super(mydef, null);
+ this.magicValuePredicate = magicValuePredicate;
+ this.userInPredicate = userInPredicate;
+ this.groupResolver = groupResolver;
+ this.groupControl = groupControl;
+ this.listOfFilesUnchangedPredicate = listOfFilesUnchangedPredicate;
+ }
+
+ @Operator
+ public Predicate<ApprovalContext> changeKind(String term) throws QueryParseException {
+ return new ChangeKindPredicate(toEnumValue(ChangeKind.class, term));
+ }
+
+ @Operator
+ public Predicate<ApprovalContext> is(String term) throws QueryParseException {
+ return magicValuePredicate.create(toEnumValue(MagicValuePredicate.MagicValue.class, term));
+ }
+
+ @Operator
+ public Predicate<ApprovalContext> approverin(String group) throws QueryParseException {
+ return userInPredicate.create(UserInPredicate.Field.APPROVER, parseGroupOrThrow(group));
+ }
+
+ @Operator
+ public Predicate<ApprovalContext> uploaderin(String group) throws QueryParseException {
+ return userInPredicate.create(UserInPredicate.Field.UPLOADER, parseGroupOrThrow(group));
+ }
+
+ @Operator
+ public Predicate<ApprovalContext> has(String value) throws QueryParseException {
+ if (value.equals("unchanged-files")) {
+ return listOfFilesUnchangedPredicate;
+ }
+ throw error(
+ String.format(
+ "'%s' is not a supported argument for has. only 'unchanged-files' is supported",
+ value));
+ }
+
+ private static <T extends Enum<T>> T toEnumValue(Class<T> clazz, String term)
+ throws QueryParseException {
+ try {
+ return Enum.valueOf(clazz, term.toUpperCase().replace('-', '_'));
+ } catch (
+ @SuppressWarnings("UnusedException")
+ IllegalArgumentException unused) {
+ throw new QueryParseException(
+ String.format(
+ "%s is not a valid term. valid options: %s",
+ term, Arrays.asList(clazz.getEnumConstants())));
+ }
+ }
+
+ private AccountGroup.UUID parseGroupOrThrow(String maybeUUID) throws QueryParseException {
+ GroupDescription.Basic g = groupResolver.parseId(maybeUUID);
+ if (g == null || !groupControl.controlFor(g).isVisible()) {
+ throw error("Group " + maybeUUID + " not found");
+ }
+ return g.getGroupUUID();
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ChangeKindPredicate.java b/java/com/google/gerrit/server/query/approval/ChangeKindPredicate.java
new file mode 100644
index 0000000..78711fd
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ChangeKindPredicate.java
@@ -0,0 +1,56 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.index.query.Predicate;
+import java.util.Collection;
+import java.util.Objects;
+
+/**
+ * Predicate that matches patch set approvals we want to copy if the diff between the old and new
+ * patch set is of a certain kind.
+ */
+public class ChangeKindPredicate extends ApprovalPredicate {
+ private final ChangeKind changeKind;
+
+ ChangeKindPredicate(ChangeKind changeKind) {
+ this.changeKind = changeKind;
+ }
+
+ @Override
+ public boolean match(ApprovalContext ctx) {
+ return ctx.changeKind().equals(changeKind);
+ }
+
+ @Override
+ public Predicate<ApprovalContext> copy(
+ Collection<? extends Predicate<ApprovalContext>> children) {
+ return new ChangeKindPredicate(changeKind);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(changeKind);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof ChangeKindPredicate)) {
+ return false;
+ }
+ return ((ChangeKindPredicate) other).changeKind.equals(changeKind);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
new file mode 100644
index 0000000..30097d8
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.entities.Patch.ChangeType;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.patch.PatchList;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Objects;
+
+/** Predicate that matches when the new patch-set includes the same files as the old patch-set. */
+@Singleton
+public class ListOfFilesUnchangedPredicate extends ApprovalPredicate {
+ private final PatchListCache patchListCache;
+
+ @Inject
+ public ListOfFilesUnchangedPredicate(PatchListCache patchListCache) {
+ this.patchListCache = patchListCache;
+ }
+
+ @Override
+ public boolean match(ApprovalContext ctx) {
+ PatchSet currentPatchset = ctx.changeNotes().getCurrentPatchSet();
+ Map.Entry<PatchSet.Id, PatchSet> priorPatchSet =
+ ctx.changeNotes().getPatchSets().lowerEntry(currentPatchset.id());
+ PatchListKey key =
+ PatchListKey.againstCommit(
+ priorPatchSet.getValue().commitId(),
+ currentPatchset.commitId(),
+ DiffPreferencesInfo.Whitespace.IGNORE_NONE);
+ try {
+ return match(patchListCache.get(key, ctx.changeNotes().getProjectName()));
+ } catch (PatchListNotAvailableException ex) {
+ throw new StorageException(
+ "failed to compute difference in files, so won't copy"
+ + " votes on labels even if list of files is the same and "
+ + "copyAllIfListOfFilesDidNotChange",
+ ex);
+ }
+ }
+
+ public boolean match(PatchList patchList) {
+ return patchList.getPatches().stream()
+ .noneMatch(
+ p ->
+ p.getChangeType() == ChangeType.ADDED
+ || p.getChangeType() == ChangeType.DELETED
+ || p.getChangeType() == ChangeType.RENAMED);
+ }
+
+ @Override
+ public Predicate<ApprovalContext> copy(
+ Collection<? extends Predicate<ApprovalContext>> children) {
+ return new ListOfFilesUnchangedPredicate(patchListCache);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(patchListCache);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof ListOfFilesUnchangedPredicate)) {
+ return false;
+ }
+ ListOfFilesUnchangedPredicate o = (ListOfFilesUnchangedPredicate) other;
+ return Objects.equals(o.patchListCache, patchListCache);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java b/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java
new file mode 100644
index 0000000..2924e6e
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/MagicValuePredicate.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.util.Collection;
+import java.util.Objects;
+
+/** Predicate that matches patch set approvals we want to copy based on the value. */
+public class MagicValuePredicate extends ApprovalPredicate {
+ enum MagicValue {
+ MIN,
+ MAX,
+ ANY
+ }
+
+ public interface Factory {
+ MagicValuePredicate create(MagicValue value);
+ }
+
+ private final MagicValue value;
+ private final ProjectCache projectCache;
+
+ @Inject
+ MagicValuePredicate(ProjectCache projectCache, @Assisted MagicValue value) {
+ this.projectCache = projectCache;
+ this.value = value;
+ }
+
+ @Override
+ public boolean match(ApprovalContext ctx) {
+ short pValue;
+ switch (value) {
+ case ANY:
+ return true;
+ case MIN:
+ pValue =
+ getLabelType(ctx.changeNotes().getProjectName(), ctx.patchSetApproval().labelId())
+ .getMaxNegative();
+ break;
+ case MAX:
+ pValue =
+ getLabelType(ctx.changeNotes().getProjectName(), ctx.patchSetApproval().labelId())
+ .getMaxPositive();
+ break;
+ default:
+ throw new IllegalArgumentException("unrecognized label value: " + value);
+ }
+ return pValue == ctx.patchSetApproval().value();
+ }
+
+ private LabelType getLabelType(Project.NameKey project, LabelId labelId) {
+ return projectCache
+ .get(project)
+ .orElseThrow(() -> new IllegalStateException(project + " absent"))
+ .getLabelTypes()
+ .byLabel(labelId);
+ }
+
+ @Override
+ public Predicate<ApprovalContext> copy(
+ Collection<? extends Predicate<ApprovalContext>> children) {
+ return new MagicValuePredicate(projectCache, value);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(value);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof MagicValuePredicate)) {
+ return false;
+ }
+ return ((MagicValuePredicate) other).value.equals(value);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/UserInPredicate.java b/java/com/google/gerrit/server/query/approval/UserInPredicate.java
new file mode 100644
index 0000000..7e16fcb
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/UserInPredicate.java
@@ -0,0 +1,71 @@
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.util.Collection;
+import java.util.Objects;
+
+/** Predicate that matches group memberships of users such as uploader or approver. */
+public class UserInPredicate extends ApprovalPredicate {
+ interface Factory {
+ UserInPredicate create(Field field, AccountGroup.UUID group);
+ }
+
+ enum Field {
+ UPLOADER,
+ APPROVER
+ }
+
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
+ private final Field field;
+ private final AccountGroup.UUID group;
+
+ @Inject
+ UserInPredicate(
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ @Assisted Field field,
+ @Assisted AccountGroup.UUID group) {
+ this.identifiedUserFactory = identifiedUserFactory;
+ this.field = field;
+ this.group = group;
+ }
+
+ @Override
+ public boolean match(ApprovalContext ctx) {
+ Account.Id accountId;
+ if (field == Field.UPLOADER) {
+ PatchSet patchSet = ctx.changeNotes().getPatchSets().get(ctx.target());
+ accountId = patchSet.uploader();
+ } else if (field == Field.APPROVER) {
+ accountId = ctx.patchSetApproval().accountId();
+ } else {
+ throw new IllegalStateException("unknown field in group membership check: " + field);
+ }
+ return identifiedUserFactory.create(accountId).getEffectiveGroups().contains(group);
+ }
+
+ @Override
+ public Predicate<ApprovalContext> copy(
+ Collection<? extends Predicate<ApprovalContext>> children) {
+ return new UserInPredicate(identifiedUserFactory, field, group);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(field, group);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof UserInPredicate)) {
+ return false;
+ }
+ UserInPredicate o = (UserInPredicate) other;
+ return Objects.equals(o.field, field) && Objects.equals(o.group, group);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 84c6de0..6f8b097 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -58,7 +58,6 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.index.RefState;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
@@ -68,6 +67,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.StarRef;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.CommentThread;
import com.google.gerrit.server.change.CommentThreads;
import com.google.gerrit.server.change.MergeabilityCache;
@@ -586,8 +586,7 @@
} else {
try {
currentApprovals =
- ImmutableList.copyOf(
- approvalsUtil.byPatchSet(notes(), c.currentPatchSetId(), null, null));
+ ImmutableList.copyOf(approvalsUtil.byPatchSet(notes(), c.currentPatchSetId()));
} catch (StorageException e) {
if (e.getCause() instanceof NoSuchChangeException) {
currentApprovals = Collections.emptyList();
@@ -1323,7 +1322,7 @@
}
draftsByUser = new HashMap<>();
- for (Ref ref : commentsUtil.getDraftRefs(notes.getChangeId())) {
+ for (Ref ref : commentsUtil.getDraftRefs(notes().getChangeId())) {
Account.Id account = Account.Id.fromRefSuffix(ref.getName());
if (account != null
// Double-check that any drafts exist for this user after
diff --git a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
index c4ca460..ccd4109 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
@@ -14,34 +14,12 @@
package com.google.gerrit.server.query.change;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
-
-import com.google.common.base.CharMatcher;
-import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
-import com.google.common.primitives.Ints;
-import com.google.common.primitives.Longs;
import com.google.gerrit.index.FieldDef;
-import com.google.gerrit.index.FieldType;
import com.google.gerrit.index.query.IndexPredicate;
-import com.google.gerrit.index.query.Matchable;
import com.google.gerrit.index.query.Predicate;
-import java.util.Objects;
-import java.util.Set;
-import java.util.stream.StreamSupport;
/** Predicate that is mapped to a field in the change index. */
-public class ChangeIndexPredicate extends IndexPredicate<ChangeData>
- implements Matchable<ChangeData> {
- /**
- * Text segmentation to be applied to both the query string and the indexed field for full-text
- * queries. This is inspired by http://unicode.org/reports/tr29/ which is what Lucene uses, but
- * complexity was reduced to the bare minimum at the cost of small discrepancies to the Unicode
- * spec.
- */
- private static final Splitter FULL_TEXT_SPLITTER = Splitter.on(CharMatcher.anyOf(" ,.-\\/_\n"));
-
+public class ChangeIndexPredicate extends IndexPredicate<ChangeData> {
/**
* Returns an index predicate that matches no changes in the index.
*
@@ -61,70 +39,4 @@
protected ChangeIndexPredicate(FieldDef<ChangeData, ?> def, String name, String value) {
super(def, name, value);
}
-
- /**
- * This method matches documents without calling an index subsystem. For primitive fields (e.g.
- * integer, long) , the matching logic is consistent across this method and all known index
- * implementations. For text fields (i.e. prefix and full-text) the semantics vary between this
- * implementation and known index implementations:
- * <li>Prefix: Lucene as well as {@link #match(ChangeData)} matches terms as true prefixes
- * (prefix:foo -> `foo bar` matches, but `baz foo bar` does not match). The index
- * implementation at Google tokenizes both the query and the indexed text and matches tokens
- * individually (prefix:fo ba -> `baz foo bar` matches).
- * <li>Full text: Lucene uses a {@code PhraseQuery} to search for terms in full text fields
- * in-order. The index implementation at Google as well as {@link #match(ChangeData)}
- * tokenizes both the query and the indexed text and matches tokens individually.
- *
- * @return true if the predicate matches the provided {@link ChangeData}.
- */
- @Override
- public boolean match(ChangeData cd) {
- if (getField().isRepeatable()) {
- Iterable<Object> values = (Iterable<Object>) getField().get(cd);
- for (Object v : values) {
- if (matchesSingleObject(v)) {
- return true;
- }
- }
- return false;
- } else {
- return matchesSingleObject(getField().get(cd));
- }
- }
-
- @Override
- public int getCost() {
- return 1;
- }
-
- private boolean matchesSingleObject(Object fieldValueFromObject) {
- String fieldTypeName = getField().getType().getName();
- if (fieldTypeName.equals(FieldType.INTEGER.getName())) {
- return Objects.equals(fieldValueFromObject, Ints.tryParse(value));
- } else if (fieldTypeName.equals(FieldType.EXACT.getName())) {
- return Objects.equals(fieldValueFromObject, value);
- } else if (fieldTypeName.equals(FieldType.LONG.getName())) {
- return Objects.equals(fieldValueFromObject, Longs.tryParse(value));
- } else if (fieldTypeName.equals(FieldType.PREFIX.getName())) {
- return String.valueOf(fieldValueFromObject).startsWith(value);
- } else if (fieldTypeName.equals(FieldType.FULL_TEXT.getName())) {
- Set<String> tokenizedField = tokenizeString(String.valueOf(fieldValueFromObject));
- Set<String> tokenizedValue = tokenizeString(value);
- return Sets.intersection(tokenizedField, tokenizedValue).size() == tokenizedValue.size();
- } else if (fieldTypeName.equals(FieldType.STORED_ONLY.getName())) {
- throw new IllegalStateException("can't filter for storedOnly field " + getField().getName());
- } else if (fieldTypeName.equals(FieldType.TIMESTAMP.getName())) {
- throw new IllegalStateException("timestamp queries must be handled in subclasses");
- } else if (fieldTypeName.equals(FieldType.INTEGER_RANGE.getName())) {
- throw new IllegalStateException("integer range queries must be handled in subclasses");
- } else {
- throw new IllegalStateException("unrecognized field " + fieldTypeName);
- }
- }
-
- private static ImmutableSet<String> tokenizeString(String value) {
- return StreamSupport.stream(FULL_TEXT_SPLITTER.split(value.toLowerCase()).spliterator(), false)
- .filter(s -> !s.trim().isEmpty())
- .collect(toImmutableSet());
- }
}
diff --git a/java/com/google/gerrit/server/query/group/GroupPredicates.java b/java/com/google/gerrit/server/query/group/GroupPredicates.java
index 9d8171c..8a2dc8d 100644
--- a/java/com/google/gerrit/server/query/group/GroupPredicates.java
+++ b/java/com/google/gerrit/server/query/group/GroupPredicates.java
@@ -19,7 +19,6 @@
import com.google.gerrit.entities.InternalGroup;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.IndexPredicate;
-import com.google.gerrit.index.query.Matchable;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.index.group.GroupField;
import java.util.Locale;
@@ -45,7 +44,7 @@
}
public static Predicate<InternalGroup> name(String name) {
- return new NameGroupPredicate(name);
+ return new GroupPredicate(GroupField.NAME, name);
}
public static Predicate<InternalGroup> owner(AccountGroup.UUID ownerUuid) {
@@ -76,25 +75,5 @@
}
}
- // TODO(hiesel): This is just a one-off to make index tests work. Remove in favor of a more
- // generic solution.
- // This is required because Gerrit needs to look up groups by name on every request.
- static class NameGroupPredicate extends IndexPredicate<InternalGroup>
- implements Matchable<InternalGroup> {
- NameGroupPredicate(String value) {
- super(GroupField.NAME, value);
- }
-
- @Override
- public boolean match(InternalGroup group) {
- return group.getName().equals(getValue());
- }
-
- @Override
- public int getCost() {
- return 1;
- }
- }
-
private GroupPredicates() {}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index dc1cd10..5375936 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -33,11 +33,11 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 8ae902d..35cadb7 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -33,12 +33,12 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.ReviewerResource;
diff --git a/java/com/google/gerrit/server/restapi/change/ListReviewers.java b/java/com/google/gerrit/server/restapi/change/ListReviewers.java
index 3d07d43..12dbf4e 100644
--- a/java/com/google/gerrit/server/restapi/change/ListReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/ListReviewers.java
@@ -19,7 +19,7 @@
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerJson;
import com.google.gerrit.server.change.ReviewerResource;
diff --git a/java/com/google/gerrit/server/restapi/change/ListRevisionReviewers.java b/java/com/google/gerrit/server/restapi/change/ListRevisionReviewers.java
index b44f637..2df2d29 100644
--- a/java/com/google/gerrit/server/restapi/change/ListRevisionReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/ListRevisionReviewers.java
@@ -20,7 +20,7 @@
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerJson;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.RevisionResource;
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 22fcbc7..50b7516 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -40,11 +40,11 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.GerritServerConfig;
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 8d413fa..6816361 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -81,7 +81,6 @@
import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommentsUtil;
@@ -91,6 +90,7 @@
import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.change.ModifyReviewersEmail;
@@ -1418,11 +1418,7 @@
return !del.isEmpty() || !ups.isEmpty();
}
- /**
- * Approval is copied over if it doesn't exist in the approvals of the current patch-set
- * according to change notes (which means it was computed in {@link
- * com.google.gerrit.server.ApprovalInference})
- */
+ /** Approval is copied over if it doesn't exist in the approvals of the current patch-set. */
private boolean isApprovalCopiedOver(
PatchSetApproval patchSetApproval, ChangeNotes changeNotes) {
return !changeNotes.getApprovals().get(changeNotes.getChange().currentPatchSetId()).stream()
diff --git a/java/com/google/gerrit/server/restapi/change/PutAssignee.java b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
index 17ee92e..dcf616c 100644
--- a/java/com/google/gerrit/server/restapi/change/PutAssignee.java
+++ b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
@@ -26,11 +26,11 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerModifier;
import com.google.gerrit.server.change.ReviewerModifier.ReviewerModification;
diff --git a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
index 4723d70..0356cdd 100644
--- a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
+++ b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
@@ -27,11 +27,11 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.ServiceUserClassifier;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.AttentionSetUnchangedOp;
import com.google.gerrit.server.change.CommentThread;
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
index d80ab696..9e74eec 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -24,10 +24,10 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestedReviewer;
import com.google.gerrit.server.config.GerritServerConfig;
diff --git a/java/com/google/gerrit/server/restapi/change/Reviewers.java b/java/com/google/gerrit/server/restapi/change/Reviewers.java
index 4bfcf14..9da7c88 100644
--- a/java/com/google/gerrit/server/restapi/change/Reviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/Reviewers.java
@@ -22,8 +22,8 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/server/restapi/change/RevisionReviewers.java b/java/com/google/gerrit/server/restapi/change/RevisionReviewers.java
index 2651ab5..97383cda 100644
--- a/java/com/google/gerrit/server/restapi/change/RevisionReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/RevisionReviewers.java
@@ -24,7 +24,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.restapi.account.AccountsCollection;
diff --git a/java/com/google/gerrit/server/restapi/change/Votes.java b/java/com/google/gerrit/server/restapi/change/Votes.java
index d899002..0f4b960 100644
--- a/java/com/google/gerrit/server/restapi/change/Votes.java
+++ b/java/com/google/gerrit/server/restapi/change/Votes.java
@@ -24,7 +24,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.VoteResource;
import com.google.inject.Inject;
@@ -83,9 +83,7 @@
approvalsUtil.byPatchSetUser(
rsrc.getChangeResource().getNotes(),
rsrc.getChange().currentPatchSetId(),
- rsrc.getReviewerUser().getAccountId(),
- null,
- null);
+ rsrc.getReviewerUser().getAccountId());
for (PatchSetApproval psa : byPatchSetUser) {
votes.put(psa.label(), psa.value());
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index eceab43..92038b0 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -30,7 +30,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index 2ae1b05..d2f4161 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -36,6 +37,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -51,6 +53,7 @@
private final MetaDataUpdate.User updateFactory;
private final ProjectConfig.Factory projectConfigFactory;
private final ProjectCache projectCache;
+ private final ApprovalQueryBuilder approvalQueryBuilder;
@Inject
public CreateLabel(
@@ -58,12 +61,14 @@
PermissionBackend permissionBackend,
MetaDataUpdate.User updateFactory,
ProjectConfig.Factory projectConfigFactory,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ApprovalQueryBuilder approvalQueryBuilder) {
this.user = user;
this.permissionBackend = permissionBackend;
this.updateFactory = updateFactory;
this.projectConfigFactory = projectConfigFactory;
this.projectCache = projectCache;
+ this.approvalQueryBuilder = approvalQueryBuilder;
}
@Override
@@ -166,6 +171,23 @@
labelType.setCopyAnyScore(input.copyAnyScore);
}
+ if (input.copyCondition != null) {
+ try {
+ approvalQueryBuilder.parse(input.copyCondition);
+ } catch (QueryParseException e) {
+ throw new BadRequestException(
+ "unable to parse copy condition. got: " + input.copyCondition + ". " + e.getMessage(),
+ e);
+ }
+ if (Boolean.TRUE.equals(input.unsetCopyCondition)) {
+ throw new BadRequestException("can't set and unset copyCondition in the same request");
+ }
+ labelType.setCopyCondition(Strings.emptyToNull(input.copyCondition));
+ }
+ if (Boolean.TRUE.equals(input.unsetCopyCondition)) {
+ labelType.setCopyCondition(null);
+ }
+
if (input.copyMinScore != null) {
labelType.setCopyMinScore(input.copyMinScore);
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index b1bcb15..d69abef 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -32,6 +33,7 @@
import com.google.gerrit.server.project.LabelResource;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -45,6 +47,7 @@
private final MetaDataUpdate.User updateFactory;
private final ProjectConfig.Factory projectConfigFactory;
private final ProjectCache projectCache;
+ private final ApprovalQueryBuilder approvalQueryBuilder;
@Inject
public SetLabel(
@@ -52,12 +55,14 @@
PermissionBackend permissionBackend,
MetaDataUpdate.User updateFactory,
ProjectConfig.Factory projectConfigFactory,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ApprovalQueryBuilder approvalQueryBuilder) {
this.user = user;
this.permissionBackend = permissionBackend;
this.updateFactory = updateFactory;
this.projectConfigFactory = projectConfigFactory;
this.projectCache = projectCache;
+ this.approvalQueryBuilder = approvalQueryBuilder;
}
@Override
@@ -174,6 +179,26 @@
dirty = true;
}
+ input.copyCondition = Strings.emptyToNull(input.copyCondition);
+ if (input.copyCondition != null) {
+ try {
+ approvalQueryBuilder.parse(input.copyCondition);
+ } catch (QueryParseException e) {
+ throw new BadRequestException(
+ "unable to parse copy condition. got: " + input.copyCondition + ". " + e.getMessage(),
+ e);
+ }
+ labelTypeBuilder.setCopyCondition(input.copyCondition);
+ dirty = true;
+ if (Boolean.TRUE.equals(input.unsetCopyCondition)) {
+ throw new BadRequestException("can't set and unset copyCondition in the same request");
+ }
+ }
+ if (Boolean.TRUE.equals(input.unsetCopyCondition)) {
+ labelTypeBuilder.setCopyCondition(null);
+ dirty = true;
+ }
+
if (input.copyAnyScore != null) {
labelTypeBuilder.setCopyAnyScore(input.copyAnyScore);
dirty = true;
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java
index 530c53f..6291e6c 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -26,12 +26,12 @@
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.LabelNormalizer;
import com.google.gerrit.server.change.RebaseChangeOp;
import com.google.gerrit.server.change.SetPrivateOp;
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index a63c7dc..09470d4 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -32,9 +32,9 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.LabelNormalizer;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 503ab11..53a9364 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -28,15 +28,14 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
-import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.change.ChangeKindCreator;
import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -45,26 +44,19 @@
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import java.util.EnumSet;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Set;
-import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -73,6 +65,7 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ChangeOperations changeOperations;
+ @Inject private ChangeKindCreator changeKindCreator;
@Inject
@Named("change_kind")
@@ -135,11 +128,11 @@
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, 1, -1);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, changeKind);
assertVotes(c, user, 1, 0, changeKind);
@@ -147,6 +140,49 @@
}
@Test
+ public void stickyWhenCopyConditionIsTrue() throws Exception {
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("is:ANY"));
+ u.save();
+ }
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, 1, -1);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, changeKind);
+ assertVotes(c, user, 1, 0, changeKind);
+ }
+ }
+
+ @Test
+ public void stickyEvenWhenUserCantSeeUploaderInGroup() throws Exception {
+ // user can't see admin group
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+ u.getConfig()
+ .updateLabelType(
+ LabelId.CODE_REVIEW, b -> b.setCopyCondition("approverin:" + administratorsUUID));
+ u.save();
+ }
+
+ String changeId = createChange().getChangeId();
+ approve(changeId);
+ amendChange(changeId);
+ vote(user, changeId, 1, -1); // Invalidate cache
+ requestScopeOperations.setApiUser(user.id());
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, 1, -1);
+ }
+
+ @Test
public void stickyOnMinScore() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMinScore(true));
@@ -157,11 +193,11 @@
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, -1, 1);
vote(user, changeId, -2, -1);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 0, 0, changeKind);
assertVotes(c, user, -2, 0, changeKind);
@@ -169,6 +205,30 @@
}
@Test
+ public void stickyWhenEitherBooleanConfigsOrCopyConditionAreTrue() throws Exception {
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .updateLabelType(
+ LabelId.CODE_REVIEW, b -> b.setCopyCondition("is:MAX").setCopyMinScore(true));
+ u.save();
+ }
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, changeKind);
+ assertVotes(c, user, -2, 0, changeKind);
+ }
+ }
+
+ @Test
public void stickyOnMaxScore() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMaxScore(true));
@@ -179,11 +239,11 @@
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, 1, -1);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, changeKind);
assertVotes(c, user, 0, 0, changeKind);
@@ -205,12 +265,12 @@
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, -1, 1);
vote(user, changeId, -2, -1);
vote(user2, changeId, 1, -1);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, -1, 0, changeKind);
assertVotes(c, user, 0, 0, changeKind);
@@ -226,16 +286,16 @@
u.save();
}
- String changeId = createChange(TRIVIAL_REBASE);
+ String changeId = changeKindCreator.createChange(TRIVIAL_REBASE, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- updateChange(changeId, NO_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, NO_CHANGE);
assertVotes(c, user, -2, 0, NO_CHANGE);
- updateChange(changeId, TRIVIAL_REBASE);
+ changeKindCreator.updateChange(changeId, TRIVIAL_REBASE, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, TRIVIAL_REBASE);
assertVotes(c, user, -2, 0, TRIVIAL_REBASE);
@@ -248,7 +308,8 @@
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- String cherryPickChangeId = cherryPick(changeId, TRIVIAL_REBASE);
+ String cherryPickChangeId =
+ changeKindCreator.cherryPick(changeId, TRIVIAL_REBASE, testRepo, admin, project);
c = detailedChange(cherryPickChangeId);
assertVotes(c, admin, 2, 0);
assertVotes(c, user, -2, 0);
@@ -259,7 +320,7 @@
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- cherryPickChangeId = cherryPick(changeId, REWORK);
+ cherryPickChangeId = changeKindCreator.cherryPick(changeId, REWORK, testRepo, admin, project);
c = detailedChange(cherryPickChangeId);
assertVotes(c, admin, 0, 0);
assertVotes(c, user, 0, 0);
@@ -272,16 +333,16 @@
u.save();
}
- String changeId = createChange(NO_CODE_CHANGE);
+ String changeId = changeKindCreator.createChange(NO_CODE_CHANGE, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- updateChange(changeId, NO_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 0, 1, NO_CHANGE);
assertVotes(c, user, 0, -1, NO_CHANGE);
- updateChange(changeId, NO_CODE_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 0, 1, NO_CODE_CHANGE);
assertVotes(c, user, 0, -1, NO_CODE_CHANGE);
@@ -298,16 +359,16 @@
u.save();
}
- String changeId = createChange(MERGE_FIRST_PARENT_UPDATE);
+ String changeId = changeKindCreator.createChange(MERGE_FIRST_PARENT_UPDATE, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- updateChange(changeId, NO_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, NO_CHANGE);
assertVotes(c, user, -2, 0, NO_CHANGE);
- updateChange(changeId, MERGE_FIRST_PARENT_UPDATE);
+ changeKindCreator.updateChange(changeId, MERGE_FIRST_PARENT_UPDATE, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, MERGE_FIRST_PARENT_UPDATE);
assertVotes(c, user, -2, 0, MERGE_FIRST_PARENT_UPDATE);
@@ -322,11 +383,11 @@
u.save();
}
- String changeId = createChangeForMergeCommit();
+ String changeId = changeKindCreator.createChangeForMergeCommit(testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
- updateSecondParent(changeId);
+ changeKindCreator.updateSecondParent(changeId, testRepo, admin);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 0, 0, null);
assertVotes(c, user, 0, 0, null);
@@ -439,7 +500,7 @@
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, 2, 1);
vote(user, changeId, -2, -1);
@@ -450,7 +511,7 @@
assertVotes(c, admin, 0, 0, null);
assertVotes(c, user, 0, 0, null);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 0, 0, changeKind);
assertVotes(c, user, 0, 0, changeKind);
@@ -465,16 +526,16 @@
u.save();
}
- String changeId = createChange(REWORK);
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
vote(admin, changeId, 2, 1);
for (int i = 0; i < 5; i++) {
- updateChange(changeId, NO_CODE_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 1, NO_CODE_CHANGE);
}
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, REWORK);
}
@@ -491,11 +552,11 @@
u.save();
}
- String changeId = createChange(REWORK);
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
vote(admin, changeId, 2, 1);
- updateChange(changeId, NO_CODE_CHANGE);
- updateChange(changeId, NO_CODE_CHANGE);
- updateChange(changeId, NO_CODE_CHANGE);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
Map<Integer, ObjectId> revisions = new HashMap<>();
gApi.changes()
@@ -524,24 +585,24 @@
}
// Vote max score on PS1
- String changeId = createChange(REWORK);
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
vote(admin, changeId, 2, 1);
// Have someone else vote min score on PS2
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
vote(user, changeId, -2, 0);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, REWORK);
assertVotes(c, user, -2, 0, REWORK);
// No vote changes on PS3
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 2, 0, REWORK);
assertVotes(c, user, -2, 0, REWORK);
// Both users revote on PS4
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
vote(admin, changeId, 1, 1);
vote(user, changeId, 1, 1);
c = detailedChange(changeId);
@@ -549,7 +610,7 @@
assertVotes(c, user, 1, 1, REWORK);
// New approvals shouldn't carry through to PS5
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
c = detailedChange(changeId);
assertVotes(c, admin, 0, 0, REWORK);
assertVotes(c, user, 0, 0, REWORK);
@@ -564,10 +625,10 @@
}
// Vote max score on PS1
- String changeId = createChange(REWORK);
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
vote(admin, changeId, label, 2);
assertVotes(detailedChange(changeId), admin, label, 2, null);
- updateChange(changeId, REWORK);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
assertVotes(detailedChange(changeId), admin, label, 2, REWORK);
// Delete vote that was copied via sticky approval
@@ -622,209 +683,17 @@
for (ChangeKind changeKind : changeKinds) {
testRepo.reset(projectOperations.project(project).getHead("master"));
- String changeId = createChange(changeKind);
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
vote(admin, changeId, +2, 1);
vote(user, changeId, -2, -1);
- updateChange(changeId, changeKind);
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
ChangeInfo c = detailedChange(changeId);
assertVotes(c, admin, 0, 0, changeKind);
assertVotes(c, user, 0, 0, changeKind);
}
}
- private String createChange(ChangeKind kind) throws Exception {
- switch (kind) {
- case NO_CODE_CHANGE:
- case REWORK:
- case TRIVIAL_REBASE:
- case NO_CHANGE:
- return createChange().getChangeId();
- case MERGE_FIRST_PARENT_UPDATE:
- return createChangeForMergeCommit();
- default:
- throw new IllegalStateException("unexpected change kind: " + kind);
- }
- }
-
- private void updateChange(String changeId, ChangeKind changeKind) throws Exception {
- switch (changeKind) {
- case NO_CODE_CHANGE:
- noCodeChange(changeId);
- return;
- case REWORK:
- rework(changeId);
- return;
- case TRIVIAL_REBASE:
- trivialRebase(changeId);
- return;
- case MERGE_FIRST_PARENT_UPDATE:
- updateFirstParent(changeId);
- return;
- case NO_CHANGE:
- noChange(changeId);
- return;
- default:
- assertWithMessage("unexpected change kind: " + changeKind).fail();
- }
- }
-
- private void noCodeChange(String changeId) throws Exception {
- TestRepository<?>.CommitBuilder commitBuilder =
- testRepo.amendRef("HEAD").insertChangeId(changeId.substring(1));
- commitBuilder
- .message("New subject " + System.nanoTime())
- .author(admin.newIdent())
- .committer(new PersonIdent(admin.newIdent(), testRepo.getDate()));
- commitBuilder.create();
- GitUtil.pushHead(testRepo, "refs/for/master", false);
- assertThat(getChangeKind(changeId)).isEqualTo(NO_CODE_CHANGE);
- }
-
- private void noChange(String changeId) throws Exception {
- ChangeInfo change = gApi.changes().id(changeId).get();
- String commitMessage = change.revisions.get(change.currentRevision).commit.message;
-
- TestRepository<?>.CommitBuilder commitBuilder =
- testRepo.amendRef("HEAD").insertChangeId(changeId.substring(1));
- commitBuilder
- .message(commitMessage)
- .author(admin.newIdent())
- .committer(new PersonIdent(admin.newIdent(), testRepo.getDate()));
- commitBuilder.create();
- GitUtil.pushHead(testRepo, "refs/for/master", false);
- assertThat(getChangeKind(changeId)).isEqualTo(NO_CHANGE);
- }
-
- private void rework(String changeId) throws Exception {
- PushOneCommit push =
- pushFactory.create(
- admin.newIdent(),
- testRepo,
- PushOneCommit.SUBJECT,
- PushOneCommit.FILE_NAME,
- "new content " + System.nanoTime(),
- changeId);
- push.to("refs/for/master").assertOkStatus();
- assertThat(getChangeKind(changeId)).isEqualTo(REWORK);
- }
-
- private void trivialRebase(String changeId) throws Exception {
- requestScopeOperations.setApiUser(admin.id());
- testRepo.reset(projectOperations.project(project).getHead("master"));
- PushOneCommit push =
- pushFactory.create(
- admin.newIdent(),
- testRepo,
- "Other Change",
- "a" + System.nanoTime() + ".txt",
- PushOneCommit.FILE_CONTENT);
- PushOneCommit.Result r = push.to("refs/for/master");
- r.assertOkStatus();
- RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
- ReviewInput in = new ReviewInput().label(LabelId.CODE_REVIEW, 2).label(LabelId.VERIFIED, 1);
- revision.review(in);
- revision.submit();
-
- gApi.changes().id(changeId).current().rebase();
- assertThat(getChangeKind(changeId)).isEqualTo(TRIVIAL_REBASE);
- }
-
- private String createChangeForMergeCommit() throws Exception {
- ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
-
- PushOneCommit.Result parent1 = createChange("parent 1", "p1.txt", "content 1");
-
- testRepo.reset(initial);
- PushOneCommit.Result parent2 = createChange("parent 2", "p2.txt", "content 2");
-
- testRepo.reset(parent1.getCommit());
-
- PushOneCommit merge = pushFactory.create(admin.newIdent(), testRepo);
- merge.setParents(ImmutableList.of(parent1.getCommit(), parent2.getCommit()));
- PushOneCommit.Result result = merge.to("refs/for/master");
- result.assertOkStatus();
- return result.getChangeId();
- }
-
- private void updateFirstParent(String changeId) throws Exception {
- ChangeInfo c = detailedChange(changeId);
- List<CommitInfo> parents = c.revisions.get(c.currentRevision).commit.parents;
- String parent1 = parents.get(0).commit;
- String parent2 = parents.get(1).commit;
- RevCommit commitParent2 = testRepo.getRevWalk().parseCommit(ObjectId.fromString(parent2));
-
- testRepo.reset(parent1);
- PushOneCommit.Result newParent1 = createChange("new parent 1", "p1-1.txt", "content 1-1");
-
- PushOneCommit merge = pushFactory.create(admin.newIdent(), testRepo, changeId);
- merge.setParents(ImmutableList.of(newParent1.getCommit(), commitParent2));
- PushOneCommit.Result result = merge.to("refs/for/master");
- result.assertOkStatus();
-
- assertThat(getChangeKind(changeId)).isEqualTo(MERGE_FIRST_PARENT_UPDATE);
- }
-
- private void updateSecondParent(String changeId) throws Exception {
- ChangeInfo c = detailedChange(changeId);
- List<CommitInfo> parents = c.revisions.get(c.currentRevision).commit.parents;
- String parent1 = parents.get(0).commit;
- String parent2 = parents.get(1).commit;
- RevCommit commitParent1 = testRepo.getRevWalk().parseCommit(ObjectId.fromString(parent1));
-
- testRepo.reset(parent2);
- PushOneCommit.Result newParent2 = createChange("new parent 2", "p2-2.txt", "content 2-2");
-
- PushOneCommit merge = pushFactory.create(admin.newIdent(), testRepo, changeId);
- merge.setParents(ImmutableList.of(commitParent1, newParent2.getCommit()));
- PushOneCommit.Result result = merge.to("refs/for/master");
- result.assertOkStatus();
-
- assertThat(getChangeKind(changeId)).isEqualTo(REWORK);
- }
-
- private String cherryPick(String changeId, ChangeKind changeKind) throws Exception {
- switch (changeKind) {
- case REWORK:
- case TRIVIAL_REBASE:
- break;
- case NO_CODE_CHANGE:
- case NO_CHANGE:
- case MERGE_FIRST_PARENT_UPDATE:
- default:
- assertWithMessage("unexpected change kind: " + changeKind).fail();
- }
-
- testRepo.reset(projectOperations.project(project).getHead("master"));
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- testRepo,
- PushOneCommit.SUBJECT,
- "other.txt",
- "new content " + System.nanoTime())
- .to("refs/for/master");
- r.assertOkStatus();
- vote(admin, r.getChangeId(), 2, 1);
- merge(r);
-
- String subject =
- TRIVIAL_REBASE.equals(changeKind)
- ? PushOneCommit.SUBJECT
- : "Reworked change " + System.nanoTime();
- CherryPickInput in = new CherryPickInput();
- in.destination = "master";
- in.message = String.format("%s\n\nChange-Id: %s", subject, changeId);
- ChangeInfo c = gApi.changes().id(changeId).current().cherryPick(in).get();
- return c.changeId;
- }
-
- private ChangeKind getChangeKind(String changeId) throws Exception {
- ChangeInfo c = gApi.changes().id(changeId).get(CURRENT_REVISION);
- return c.revisions.get(c.currentRevision).kind;
- }
-
private void vote(TestAccount user, String changeId, String label, int vote) throws Exception {
requestScopeOperations.setApiUser(user.id());
gApi.changes().id(changeId).current().review(new ReviewInput().label(label, vote));
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
index fbb0b1a..31292d5 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
@@ -36,7 +36,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
-import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.query.change.ChangeData;
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index bf8de93..650334f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -63,10 +63,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.account.AccountControl;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 796ce38..d967f48 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -85,8 +85,8 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.TestSubmitInput;
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
index 6a98b8b..dfe69f9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
@@ -430,6 +430,57 @@
}
@Test
+ public void createWithCopyCondition() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.copyCondition = "is:MAX";
+
+ LabelDefinitionInfo createdLabel =
+ gApi.projects().name(project.get()).label("foo").create(input).get();
+ assertThat(createdLabel.copyCondition).isEqualTo("is:MAX");
+ }
+
+ @Test
+ public void createCopyConditionPerformsGroupVisibilityCheckWhenUserInPredicateIsUsed()
+ throws Exception {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.copyCondition = "uploaderin:" + administratorsUUID;
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ // User can't see admin group
+ requestScopeOperations.setApiUser(user.id());
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(project.get()).label("foo").create(input));
+ assertThat(thrown).hasMessageThat().contains("Group " + administratorsUUID + " not found");
+
+ // Admin can see admin group
+ requestScopeOperations.setApiUser(admin.id());
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").create(input).get();
+ assertThat(updatedLabel.copyCondition).isEqualTo(input.copyCondition);
+ }
+
+ @Test
+ public void createWithInvalidCopyCondition() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.copyCondition = "blarg::asd";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(project.get()).label("Bar").create(input));
+ assertThat(thrown).hasMessageThat().contains("unable to parse copy condition");
+ }
+
+ @Test
public void createWithCopyMaxScore() throws Exception {
LabelDefinitionInput input = new LabelDefinitionInput();
input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
index 2e68b54..b4938c1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
@@ -516,6 +516,83 @@
}
@Test
+ public void setCopyCondition() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyCondition).isNull();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyCondition = "is:MAX";
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyCondition).isEqualTo("is:MAX");
+ }
+
+ @Test
+ public void setCopyConditionPerformsGroupVisibilityCheckWhenUserInPredicateIsUsed()
+ throws Exception {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+ configLabel("foo", LabelFunction.NO_OP);
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyCondition).isNull();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyCondition = "uploaderin:" + administratorsUUID;
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ // User can't see admin group
+ requestScopeOperations.setApiUser(user.id());
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(project.get()).label("foo").update(input));
+ assertThat(thrown).hasMessageThat().contains("Group " + administratorsUUID + " not found");
+
+ // Admin can see admin group
+ requestScopeOperations.setApiUser(admin.id());
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyCondition).isEqualTo(input.copyCondition);
+ }
+
+ @Test
+ public void setInvalidCopyCondition() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyCondition).isNull();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyCondition = "foo:::bar";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(project.get()).label("foo").update(input));
+ assertThat(thrown).hasMessageThat().contains("unable to parse copy condition");
+ }
+
+ @Test
+ public void unsetCopyCondition() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyCondition("is:MAX"));
+ u.save();
+ }
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyCondition)
+ .isEqualTo("is:MAX");
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.unsetCopyCondition = true;
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyCondition).isNull();
+
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyCondition).isNull();
+ }
+
+ @Test
public void setCopyMinScore() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
assertThat(gApi.projects().name(project.get()).label("foo").get().copyMinScore).isNull();
diff --git a/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
new file mode 100644
index 0000000..9392219
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
@@ -0,0 +1,274 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.query;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.change.ChangeKindCreator;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.change.ChangeKindCache;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.query.approval.ApprovalContext;
+import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
+import com.google.inject.Inject;
+import java.util.Date;
+import org.junit.Test;
+
+public class ApprovalQueryIT extends AbstractDaemonTest {
+ @Inject private ApprovalQueryBuilder queryBuilder;
+ @Inject private ChangeKindCreator changeKindCreator;
+ @Inject private ChangeNotes.Factory changeNotesFactory;
+ @Inject private ChangeKindCache changeKindCache;
+ @Inject private ChangeOperations changeOperations;
+
+ @Test
+ public void magicValuePredicate() throws Exception {
+ assertTrue(queryBuilder.parse("is:MAX").asMatchable().match(contextForCodeReviewLabel(2)));
+ assertTrue(queryBuilder.parse("is:mAx").asMatchable().match(contextForCodeReviewLabel(2)));
+ assertFalse(queryBuilder.parse("is:MAX").asMatchable().match(contextForCodeReviewLabel(-2)));
+ assertFalse(queryBuilder.parse("is:MAX").asMatchable().match(contextForCodeReviewLabel(1)));
+ assertFalse(queryBuilder.parse("is:MAX").asMatchable().match(contextForCodeReviewLabel(5000)));
+
+ assertTrue(queryBuilder.parse("is:MIN").asMatchable().match(contextForCodeReviewLabel(-2)));
+ assertTrue(queryBuilder.parse("is:mIn").asMatchable().match(contextForCodeReviewLabel(-2)));
+ assertFalse(queryBuilder.parse("is:MIN").asMatchable().match(contextForCodeReviewLabel(2)));
+ assertFalse(queryBuilder.parse("is:MIN").asMatchable().match(contextForCodeReviewLabel(-1)));
+ assertFalse(queryBuilder.parse("is:MIN").asMatchable().match(contextForCodeReviewLabel(5000)));
+
+ assertTrue(queryBuilder.parse("is:ANY").asMatchable().match(contextForCodeReviewLabel(-2)));
+ assertTrue(queryBuilder.parse("is:ANY").asMatchable().match(contextForCodeReviewLabel(2)));
+ assertTrue(queryBuilder.parse("is:aNy").asMatchable().match(contextForCodeReviewLabel(2)));
+ }
+
+ @Test
+ public void changeKindPredicate_noCodeChange() throws Exception {
+ String change = changeKindCreator.createChange(ChangeKind.NO_CODE_CHANGE, testRepo, admin);
+ changeKindCreator.updateChange(change, ChangeKind.NO_CODE_CHANGE, testRepo, admin, project);
+ PatchSet.Id ps1 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ assertTrue(
+ queryBuilder
+ .parse("changekind:no-code-change")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
+
+ changeKindCreator.updateChange(change, ChangeKind.TRIVIAL_REBASE, testRepo, admin, project);
+ PatchSet.Id ps2 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ assertFalse(
+ queryBuilder
+ .parse("changekind:no-code-change")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps2, admin.id())));
+ }
+
+ @Test
+ public void changeKindPredicate_trivialRebase() throws Exception {
+ String change = changeKindCreator.createChange(ChangeKind.TRIVIAL_REBASE, testRepo, admin);
+ changeKindCreator.updateChange(change, ChangeKind.TRIVIAL_REBASE, testRepo, admin, project);
+ PatchSet.Id ps1 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ assertTrue(
+ queryBuilder
+ .parse("changekind:trivial-rebase")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
+
+ changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
+ PatchSet.Id ps2 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ assertFalse(
+ queryBuilder
+ .parse("changekind:trivial-rebase")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps2, admin.id())));
+ }
+
+ @Test
+ public void changeKindPredicate_reworkAndNotRework() throws Exception {
+ String change = changeKindCreator.createChange(ChangeKind.REWORK, testRepo, admin);
+ changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
+ PatchSet.Id ps1 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ assertTrue(
+ queryBuilder
+ .parse("changekind:rework")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
+
+ changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
+ PatchSet.Id ps2 =
+ PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ assertFalse(
+ queryBuilder
+ .parse("-changekind:rework")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ -2, ps2, admin.id())));
+ }
+
+ @Test
+ public void uploaderInPredicate() throws Exception {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+
+ PushOneCommit.Result pushResult = createChange();
+ String changeCreatedByAdmin = pushResult.getChangeId();
+ approve(changeCreatedByAdmin);
+ // PS2 uploaded by admin
+ amendChange(changeCreatedByAdmin);
+ // PS3 uploaded by user
+ amendChangeWithUploader(pushResult, project, user).assertOkStatus();
+
+ // can copy approval from patchset 1 -> 2
+ assertTrue(
+ queryBuilder
+ .parse("uploaderin:" + administratorsUUID)
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2,
+ PatchSet.id(pushResult.getChange().getId(), /* psId= */ 1),
+ admin.id())));
+ // can not copy approval from patchset 2 -> 3
+ assertFalse(
+ queryBuilder
+ .parse("uploaderin:" + administratorsUUID)
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2,
+ PatchSet.id(pushResult.getChange().getId(), /* psId= */ 2),
+ admin.id())));
+ }
+
+ @Test
+ public void approverInPredicate() throws Exception {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+
+ PushOneCommit.Result pushResult = createChange();
+ amendChange(pushResult.getChangeId());
+ amendChange(pushResult.getChangeId());
+ // can copy approval from patchset 1 -> 2
+ assertTrue(
+ queryBuilder
+ .parse("approverin:" + administratorsUUID)
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2,
+ PatchSet.id(pushResult.getChange().getId(), /* psId= */ 1),
+ admin.id())));
+ // can not copy approval from patchset 2 -> 3
+ assertFalse(
+ queryBuilder
+ .parse("approverin:" + administratorsUUID)
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2,
+ PatchSet.id(pushResult.getChange().getId(), /* psId= */ 2),
+ user.id())));
+ }
+
+ @Test
+ public void userInPredicate_groupNotFound() {
+ QueryParseException thrown =
+ assertThrows(
+ QueryParseException.class,
+ () ->
+ queryBuilder
+ .parse("uploaderin:foobar")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ 2)));
+ assertThat(thrown).hasMessageThat().contains("Group foobar not found");
+ }
+
+ @Test
+ public void hasChangedFilesPredicate() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
+
+ // can copy approval from patch-set 1 -> 2
+ assertTrue(
+ queryBuilder
+ .parse("has:unchanged-files")
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2, PatchSet.id(changeId, /* psId= */ 1), admin.id())));
+ changeOperations.change(changeId).newPatchset().file("file").delete().create();
+
+ // can not copy approval from patch-set 2 -> 3
+ assertFalse(
+ queryBuilder
+ .parse("has:unchanged-files")
+ .asMatchable()
+ .match(
+ contextForCodeReviewLabel(
+ /* value= */ 2, PatchSet.id(changeId, /* psId= */ 2), admin.id())));
+ }
+
+ @Test
+ public void hasChangedFilesPredicate_unsupportedOperator() {
+ QueryParseException thrown =
+ assertThrows(
+ QueryParseException.class,
+ () ->
+ queryBuilder
+ .parse("has:invalid")
+ .asMatchable()
+ .match(contextForCodeReviewLabel(/* value= */ 2)));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(
+ "'invalid' is not a supported argument for has. only 'unchanged-files' is supported");
+ }
+
+ private ApprovalContext contextForCodeReviewLabel(int value) throws Exception {
+ PushOneCommit.Result result = createChange();
+ amendChange(result.getChangeId());
+ PatchSet.Id psId = PatchSet.id(result.getChange().getId(), 1);
+ return contextForCodeReviewLabel(value, psId, admin.id());
+ }
+
+ private ApprovalContext contextForCodeReviewLabel(
+ int value, PatchSet.Id psId, Account.Id approver) {
+ ChangeNotes changeNotes = changeNotesFactory.create(project, psId.changeId());
+ PatchSet.Id newPsId = PatchSet.id(psId.changeId(), psId.get() + 1);
+ ChangeKind changeKind =
+ changeKindCache.getChangeKind(
+ changeNotes.getChange(), changeNotes.getPatchSets().get(newPsId));
+ PatchSetApproval approval =
+ PatchSetApproval.builder()
+ .postSubmit(false)
+ .granted(new Date())
+ .key(PatchSetApproval.key(psId, approver, LabelId.create("Code-Review")))
+ .value(value)
+ .build();
+ return ApprovalContext.create(changeNotes, approval, newPsId, changeKind);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/query/BUILD b/javatests/com/google/gerrit/acceptance/server/query/BUILD
new file mode 100644
index 0000000..f7d13a0
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/query/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "server_query",
+ labels = ["server"],
+)
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializerTest.java
index ad460cd..614dcf0 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/LabelTypeSerializerTest.java
@@ -35,6 +35,7 @@
.setIgnoreSelfApproval(!LabelType.DEF_IGNORE_SELF_APPROVAL)
.setRefPatterns(ImmutableList.of("refs/heads/*", "refs/tags/*"))
.setDefaultValue((short) 1)
+ .setCopyCondition("is:ANY")
.setCopyAnyScore(!LabelType.DEF_COPY_ANY_SCORE)
.setCopyMaxScore(!LabelType.DEF_COPY_MAX_SCORE)
.setCopyMinScore(!LabelType.DEF_COPY_MIN_SCORE)
@@ -57,7 +58,8 @@
@Test
public void roundTripWithMinimalValues() {
- LabelType autoValue = ALL_VALUES_SET.toBuilder().setRefPatterns(null).build();
+ LabelType autoValue =
+ ALL_VALUES_SET.toBuilder().setRefPatterns(null).setCopyCondition(null).build();
assertThat(deserialize(serialize(autoValue))).isEqualTo(autoValue);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeUpdateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeUpdateTest.java
new file mode 100644
index 0000000..dbd1939
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/ChangeUpdateTest.java
@@ -0,0 +1,102 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.AttentionSetUpdate;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.CommentRange;
+import com.google.gerrit.entities.HumanComment;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Test;
+
+public class ChangeUpdateTest extends AbstractChangeNotesTest {
+
+ @Test
+ public void bypassMaxUpdatesShouldBeTrueWhenChangingAttentionSetOnly() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+
+ // Add to attention set
+ AttentionSetUpdate attentionSetUpdate =
+ AttentionSetUpdate.createForWrite(
+ otherUser.getAccountId(), AttentionSetUpdate.Operation.ADD, "test");
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+
+ update.commit();
+
+ assertThat(update.bypassMaxUpdates()).isTrue();
+ }
+
+ @Test
+ public void bypassMaxUpdatesShouldBeTrueWhenClosingChange() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+
+ update.setStatus(Change.Status.ABANDONED);
+
+ update.commit();
+
+ assertThat(update.bypassMaxUpdates()).isTrue();
+ }
+
+ @Test
+ public void bypassMaxUpdatesShouldBeFalseWhenNotAbandoningChangeAndNotChangingAttentionSetOnly()
+ throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+
+ update.commit();
+
+ assertThat(update.bypassMaxUpdates()).isFalse();
+ }
+
+ @Test
+ public void bypassMaxUpdatesShouldBeFalseWhenCommentsAndChangesToAttentionSetCoexist()
+ throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+
+ // Add to attention set
+ AttentionSetUpdate attentionSetUpdate =
+ AttentionSetUpdate.createForWrite(
+ otherUser.getAccountId(), AttentionSetUpdate.Operation.ADD, "test");
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+
+ // Add a comment
+ RevCommit commit = tr.commit().message("PS2").create();
+ update.putComment(
+ HumanComment.Status.PUBLISHED,
+ newComment(
+ c.currentPatchSetId(),
+ "a.txt",
+ "uuid1",
+ new CommentRange(1, 2, 3, 4),
+ 1,
+ changeOwner,
+ null,
+ TimeUtil.nowTs(),
+ "Comment",
+ (short) 1,
+ commit,
+ false));
+ update.commit();
+
+ assertThat(update.bypassMaxUpdates()).isFalse();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index f6b3317..b7be40b 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -607,7 +607,6 @@
@Test
public void reindex() throws Exception {
AccountInfo user1 = newAccountWithFullName("tester", "Test Usre");
-
// update account without reindex so that account index is stale
Account.Id accountId = Account.id(user1._accountId);
String newName = "Test User";
@@ -621,10 +620,11 @@
.setAccountDelta(AccountDelta.builder().setFullName(newName).build())
.commit(md);
}
-
- assertQuery("name:" + quote(user1.name), user1);
- assertQuery("name:" + quote(newName));
-
+ // Querying for the account here will not result in a stale document because
+ // we load AccountStates from the cache after reading documents from the index
+ // which means we always read fresh data when matching.
+ //
+ // Reindex document
gApi.accounts().id(user1.username).index();
assertQuery("name:" + quote(user1.name));
assertQuery("name:" + quote(newName), user1);
diff --git a/javatests/com/google/gerrit/server/query/account/FakeQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/FakeQueryAccountsTest.java
index b742bd8..31d256e 100644
--- a/javatests/com/google/gerrit/server/query/account/FakeQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/FakeQueryAccountsTest.java
@@ -14,10 +14,6 @@
package com.google.gerrit.server.query.account;
-import static org.junit.Assume.assumeFalse;
-
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.InMemoryModule;
@@ -25,7 +21,6 @@
import com.google.gerrit.testing.IndexVersions;
import com.google.inject.Guice;
import com.google.inject.Injector;
-import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Config;
@@ -52,16 +47,4 @@
fakeConfig.setString("index", null, "type", "fake");
return Guice.createInjector(new InMemoryModule(fakeConfig));
}
-
- @Override
- protected void validateAssumptions() {
- // TODO(hiesel): Account predicates are always matching (they return true on match), so we need
- // to skip all tests here. We are doing this to document existing behavior. We want to remove
- // this assume statement and make group predicates matchable.
- assumeFalse(
- AccountPredicates.equalsName("test")
- .asMatchable()
- .match(
- AccountState.forAccount(Account.builder(Account.id(1), new Timestamp(0)).build())));
- }
}
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 385d4b2..1e23420 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -18,8 +18,6 @@
import com.google.inject.Guice;
import com.google.inject.Injector;
import org.eclipse.jgit.lib.Config;
-import org.junit.Ignore;
-import org.junit.Test;
/**
* Test against {@link com.google.gerrit.index.testing.AbstractFakeIndex}. This test might seem
@@ -34,156 +32,4 @@
fakeConfig.setString("index", null, "type", "fake");
return Guice.createInjector(new InMemoryModule(fakeConfig));
}
-
- @Ignore
- @Test
- @Override
- public void byDefault() throws Exception {
- // TODO(hiesel): Fix bug in predicate and remove @Ignore.
- super.byDefault();
- }
-
- @Ignore
- @Test
- @Override
- public void byMergedBefore() throws Exception {
- // TODO(hiesel): Used predicate is not a matchable. Fix.
- super.byMergedBefore();
- }
-
- @Ignore
- @Test
- @Override
- public void reviewerAndCcByEmail() throws Exception {
- // TODO(hiesel): Fix bug in predicate and remove @Ignore.
- super.reviewerAndCcByEmail();
- }
-
- @Ignore
- @Test
- @Override
- public void byMessageExact() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byMessageExact();
- }
-
- @Ignore
- @Test
- @Override
- public void fullTextWithNumbers() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.fullTextWithNumbers();
- }
-
- @Ignore
- @Test
- @Override
- public void byTriplet() throws Exception {
- // TODO(hiesel): Fix bug in predicate and remove @Ignore.
- super.byTriplet();
- }
-
- @Ignore
- @Test
- @Override
- public void byAge() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byAge();
- }
-
- @Ignore
- @Test
- @Override
- public void byMessageSubstring() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byMessageSubstring();
- }
-
- @Ignore
- @Test
- @Override
- public void byBeforeUntil() throws Exception {
- // TODO(hiesel): Used predicate is not a matchable. Fix.
- super.byBeforeUntil();
- }
-
- @Ignore
- @Test
- @Override
- public void byTopic() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byTopic();
- }
-
- @Ignore
- @Test
- @Override
- public void userQuery() throws Exception {
- // TODO(hiesel): Account name predicate is always returning true in #match. Fix.
- super.userQuery();
- }
-
- @Ignore
- @Test
- @Override
- public void visible() throws Exception {
- // TODO(hiesel): Account name predicate is always returning true in #match. Fix.
- super.visible();
- }
-
- @Ignore
- @Test
- @Override
- public void userDestination() throws Exception {
- // TODO(hiesel): Account name predicate is always returning true in #match. Fix.
- super.userDestination();
- }
-
- @Ignore
- @Test
- @Override
- public void byAfterSince() throws Exception {
- // TODO(hiesel): Used predicate is not a matchable. Fix.
- super.byAfterSince();
- }
-
- @Ignore
- @Test
- @Override
- public void byMessageMixedCase() throws Exception {
- // TODO(hiesel): Used predicate is not a matchable. Fix.
- super.byMessageMixedCase();
- }
-
- @Ignore
- @Test
- @Override
- public void byCommit() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byCommit();
- }
-
- @Ignore
- @Test
- @Override
- public void byComment() throws Exception {
- // TODO(hiesel): Existing #match function uses the index causing a StackOverflowError. Fix.
- super.byComment();
- }
-
- @Ignore
- @Test
- @Override
- public void byMergedAfter() throws Exception {
- // TODO(hiesel): Used predicate is not a matchable. Fix.
- super.byMergedAfter();
- }
-
- @Ignore
- @Test
- @Override
- public void byOwnerInvalidQuery() throws Exception {
- // TODO(hiesel): Account name predicate is always returning true in #match. Fix.
- super.byMergedAfter();
- }
}
diff --git a/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
index 8bc1c30..e4f228a 100644
--- a/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.query.group;
-import static org.junit.Assume.assumeTrue;
-
import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.InMemoryModule;
@@ -48,12 +46,4 @@
fakeConfig.setString("index", null, "type", "fake");
return Guice.createInjector(new InMemoryModule(fakeConfig));
}
-
- @Override
- protected void validateAssumptions() {
- // TODO(hiesel): Group predicates are not matchable, so we need to skip all tests here.
- // We are doing this to document existing behavior. We want to remove this assume statement and
- // make group predicates matchable.
- assumeTrue(GroupPredicates.inname("test").isMatchable());
- }
}
diff --git a/javatests/com/google/gerrit/server/query/project/FakeQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/FakeQueryProjectsTest.java
index 8517ad2..6fc0568 100644
--- a/javatests/com/google/gerrit/server/query/project/FakeQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/FakeQueryProjectsTest.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.query.project;
-import static org.junit.Assume.assumeTrue;
-
import com.google.gerrit.index.project.ProjectSchemaDefinitions;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.InMemoryModule;
@@ -50,12 +48,4 @@
fakeConfig.setString("index", null, "type", "fake");
return Guice.createInjector(new InMemoryModule(fakeConfig));
}
-
- @Override
- protected void validateAssumptions() {
- // TODO(hiesel): Project predicates are not matchable, so we need to skip all tests here.
- // We are doing this to document existing behavior. We want to remove this assume statement and
- // make group predicates matchable.
- assumeTrue(ProjectPredicates.inname("test").isMatchable());
- }
}
diff --git a/plugins/replication b/plugins/replication
index 321e37e..fd593ee 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 321e37e448336c2ae7495808da5fa483e755141b
+Subproject commit fd593ee1d3cb19d73c49f94c0f9b4a78856d6198
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index fb0390a..35e6449 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit fb0390a8b49f0d601e11f8a1ac0658c429727f21
+Subproject commit 35e6449a517691a880c94e7467bc07360f8e6666
diff --git a/polygerrit-ui/app/api/checks.ts b/polygerrit-ui/app/api/checks.ts
index 454b3d5..2682158 100644
--- a/polygerrit-ui/app/api/checks.ts
+++ b/polygerrit-ui/app/api/checks.ts
@@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+import {CommentRange} from './core';
export declare interface ChecksPluginApi {
/**
@@ -382,6 +383,11 @@
links?: Link[];
/**
+ * Links to lines of code. The referenced path must be part of this patchset.
+ */
+ codePointers?: CodePointer[];
+
+ /**
* Callbacks to the plugin. Must be implemented individually by each
* plugin. Actions are rendered as buttons. If there are more than two actions
* per result, then further actions are put into an overflow menu. Sort order
@@ -430,6 +436,11 @@
icon: LinkIcon;
}
+export declare interface CodePointer {
+ path: string;
+ range: CommentRange;
+}
+
export enum LinkIcon {
EXTERNAL = 'external',
IMAGE = 'image',
@@ -438,4 +449,5 @@
DOWNLOAD_MOBILE = 'download_mobile',
HELP_PAGE = 'help_page',
REPORT_BUG = 'report_bug',
+ CODE = 'code',
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 2869750..fa3c9c6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -48,6 +48,7 @@
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {pluralize} from '../../../utils/string-util';
+import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
enum ChangeSize {
XS = 10,
@@ -105,7 +106,7 @@
changeURL?: string;
@property({type: Array, computed: '_changeStatuses(change)'})
- statuses?: string[];
+ statuses?: ChangeStates[];
@property({type: Boolean})
showStar = false;
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index ef8f250..bd9103e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -20,18 +20,19 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {appContext} from '../../../services/app-context';
import {
- allRunsLatest$,
+ allRunsLatestPatchsetLatestAttempt$,
aPluginHasRegistered$,
CheckResult,
CheckRun,
- errorMessage$,
- loginCallback$,
- someProvidersAreLoading$,
+ errorMessageLatest$,
+ loginCallbackLatest$,
+ someProvidersAreLoadingLatest$,
} from '../../../services/checks/checks-model';
-import {Category, Link, RunStatus} from '../../../api/checks';
+import {Category, RunStatus} from '../../../api/checks';
import {fireShowPrimaryTab} from '../../../utils/event-util';
import '../../shared/gr-avatar/gr-avatar';
import {
+ firstPrimaryLink,
getResultsOf,
hasCompletedWithoutResults,
hasResultsOf,
@@ -307,11 +308,11 @@
constructor() {
super();
- this.subscribe('runs', allRunsLatest$);
+ this.subscribe('runs', allRunsLatestPatchsetLatestAttempt$);
this.subscribe('showChecksSummary', aPluginHasRegistered$);
- this.subscribe('someProvidersAreLoading', someProvidersAreLoading$);
- this.subscribe('errorMessage', errorMessage$);
- this.subscribe('loginCallback', loginCallback$);
+ this.subscribe('someProvidersAreLoading', someProvidersAreLoadingLatest$);
+ this.subscribe('errorMessage', errorMessageLatest$);
+ this.subscribe('loginCallback', loginCallbackLatest$);
}
static get styles() {
@@ -455,13 +456,10 @@
this.detailsQuota -= runs.length;
return runs.map(run => {
this.detailsCheckNames.push(run.checkName);
- const allLinks = resultFilter(run)
- .reduce(
- (links, result) => links.concat(result.links ?? []),
- [] as Link[]
- )
- .filter(link => link.primary);
- const links = allLinks.length === 1 ? allLinks : [];
+ const allPrimaryLinks = resultFilter(run)
+ .map(firstPrimaryLink)
+ .filter(notUndefined);
+ const links = allPrimaryLinks.length === 1 ? allPrimaryLinks : [];
const text = `${run.checkName}`;
return html`<gr-checks-chip
class="${icon}"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index f3b746a8..659d5bd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -429,7 +429,7 @@
type: String,
computed: '_computeChangeStatusChips(_change, _mergeable, _submitEnabled)',
})
- _changeStatuses?: string[];
+ _changeStatuses?: ChangeStates[];
/** If false, then the "Show more" button was used to expand. */
@property({type: Boolean})
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 3d51ac6..5128948 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1222,7 +1222,7 @@
},
};
element._mergeable = true;
- const expectedStatuses = ['Merged', 'WIP'];
+ const expectedStatuses = [ChangeStates.MERGED, ChangeStates.WIP];
assert.deepEqual(element._changeStatuses, expectedStatuses);
flush();
const statusChips = element.shadowRoot!.querySelectorAll(
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 4b7aec5..53d25ec 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -17,6 +17,7 @@
import {html} from 'lit-html';
import {classMap} from 'lit-html/directives/class-map';
import {repeat} from 'lit-html/directives/repeat';
+import {ifDefined} from 'lit-html/directives/if-defined';
import {
css,
customElement,
@@ -39,12 +40,12 @@
} from '../../api/checks';
import {sharedStyles} from '../../styles/shared-styles';
import {
- allActions$,
- allLinks$,
+ topLevelActionsSelected$,
+ topLevelLinksSelected$,
CheckRun,
- checksPatchsetNumber$,
RunResult,
- someProvidersAreLoading$,
+ checksSelectedPatchsetNumber$,
+ someProvidersAreLoadingSelected$,
} from '../../services/checks/checks-model';
import {
allResults,
@@ -52,10 +53,11 @@
hasCompletedWithoutResults,
iconForCategory,
iconForLink,
- otherLinks,
- primaryLink,
+ otherPrimaryLinks,
+ firstPrimaryLink,
primaryRunAction,
tooltipForLink,
+ secondaryLinks,
} from '../../services/checks/checks-util';
import {assertIsDefined, check} from '../../utils/common-util';
import {toggleClass, whenVisible} from '../../utils/dom-util';
@@ -77,6 +79,7 @@
getRepresentativeValue,
valueString,
} from '../../utils/label-util';
+import {GerritNav} from '../core/gr-navigation/gr-navigation';
@customElement('gr-result-row')
class GrResultRow extends GrLitElement {
@@ -113,7 +116,7 @@
gr-result-expanded {
cursor: default;
}
- tr {
+ tr.container {
border-top: 1px solid var(--border-color);
}
iron-icon.link {
@@ -169,27 +172,30 @@
overflow: hidden;
text-overflow: ellipsis;
}
- tr:hover {
+ tr.container:hover {
background: var(--hover-background-color);
}
- tr td .summary-cell .links,
- tr td .summary-cell .actions,
- tr.collapsed:hover td .summary-cell .links,
- tr.collapsed:hover td .summary-cell .actions,
+ tr.container td .summary-cell .links,
+ tr.container td .summary-cell .actions,
+ tr.container.collapsed:hover td .summary-cell .links,
+ tr.container.collapsed:hover td .summary-cell .actions,
:host(.dropdown-open) tr td .summary-cell .links,
:host(.dropdown-open) tr td .summary-cell .actions {
display: inline-block;
margin-left: var(--spacing-s);
}
- tr.collapsed td .summary-cell .message {
+ tr.container.collapsed td .summary-cell .message {
color: var(--deemphasized-text-color);
}
- tr.collapsed td .summary-cell .links,
- tr.collapsed td .summary-cell .actions {
+ tr.container.collapsed td .summary-cell .links,
+ tr.container.collapsed td .summary-cell .actions {
display: none;
}
- tr.collapsed:hover .summary-cell .hoverHide.tags,
- tr.collapsed:hover .summary-cell .hoverHide.label {
+ tr.container.collapsed:hover .summary-cell .hoverHide.tags,
+ tr.container.collapsed:hover .summary-cell .hoverHide.label {
+ display: none;
+ }
+ tr.detailsRow.collapsed {
display: none;
}
td .summary-cell .tags .tag {
@@ -294,19 +300,21 @@
return html`
<tr class="${classMap({container: true, collapsed: !this.isExpanded})}">
<td class="iconCol" @click="${this.toggleExpanded}">
- <div>${this.renderIcon()}</div>
+ <div>${this.runningIcon()}</div>
</td>
<td class="nameCol" @click="${this.toggleExpanded}">
<div class="flex">
<gr-hovercard-run .run="${this.result}"></gr-hovercard-run>
- <div class="name">${this.result.checkName}</div>
+ <div class="name">
+ ${this.result.checkName} ${this.renderStatus()}
+ </div>
<div class="space"></div>
${this.renderPrimaryRunAction()}
</div>
</td>
<td class="summaryCol">
<div class="summary-cell">
- ${this.renderLink(primaryLink(this.result))}
+ ${this.renderLink(firstPrimaryLink(this.result))}
${this.renderSummary(this.result.summary)}
<div class="message" @click="${this.toggleExpanded}">
${this.isExpanded ? '' : this.result.message}
@@ -316,7 +324,6 @@
</div>
${this.renderLabel()} ${this.renderLinks()} ${this.renderActions()}
</div>
- ${this.renderExpanded()}
</td>
<td class="expanderCol" @click="${this.toggleExpanded}">
<div
@@ -338,6 +345,10 @@
</div>
</td>
</tr>
+ <tr class="${classMap({detailsRow: true, collapsed: !this.isExpanded})}">
+ <td></td>
+ <td colspan="3">${this.renderExpanded()}</td>
+ </tr>
`;
}
@@ -376,7 +387,12 @@
`;
}
- renderIcon() {
+ private renderStatus() {
+ if (this.result?.status !== RunStatus.RUNNING) return;
+ return html`<span>(Running)</span>`;
+ }
+
+ private runningIcon() {
if (this.result?.status !== RunStatus.RUNNING) return;
return html`<iron-icon icon="gr-icons:timelapse"></iron-icon>`;
}
@@ -397,12 +413,23 @@
}
renderLinks() {
- const links = otherLinks(this.result);
+ const links = otherPrimaryLinks(this.result)
+ // Showing the same icons twice without text is super confusing.
+ .filter(
+ (link: Link, index: number, array: Link[]) =>
+ array.findIndex(other => link.icon === other.icon) === index
+ )
+ // 4 is enough for the summary row. All are shown in expanded state.
+ .slice(0, 4);
if (links.length === 0) return;
- return html`<div class="links">${links.map(this.renderLink)}</div>`;
+ return html`<div class="links">
+ ${links.map(link => this.renderLink(link))}
+ </div>`;
}
renderLink(link?: Link) {
+ // The expanded state renders all links in more detail. Hide in summary.
+ if (this.isExpanded) return;
if (!link) return;
const tooltipText = link.tooltip ?? tooltipForLink(link.icon);
return html`<a href="${link.url}" target="_blank"
@@ -485,10 +512,23 @@
@property()
repoConfig?: ConfigInfo;
+ private changeService = appContext.changeService;
+
static get styles() {
return [
sharedStyles,
css`
+ .links {
+ white-space: normal;
+ padding: var(--spacing-s) 0;
+ }
+ .links a {
+ display: inline-block;
+ margin-right: var(--spacing-xl);
+ }
+ .links a iron-icon {
+ margin-right: var(--spacing-xs);
+ }
.message {
padding: var(--spacing-m) var(--spacing-m) var(--spacing-m) 0;
}
@@ -504,6 +544,8 @@
render() {
if (!this.result) return '';
return html`
+ ${this.renderFirstPrimaryLink()} ${this.renderOtherPrimaryLinks()}
+ ${this.renderSecondaryLinks()} ${this.renderCodePointers()}
<gr-endpoint-decorator name="check-result-expanded">
<gr-endpoint-param
name="run"
@@ -522,6 +564,67 @@
</gr-endpoint-decorator>
`;
}
+
+ private renderFirstPrimaryLink() {
+ const link = firstPrimaryLink(this.result);
+ if (!link) return;
+ return html`<div class="links">${this.renderLink(link)}</div>`;
+ }
+
+ private renderOtherPrimaryLinks() {
+ const links = otherPrimaryLinks(this.result);
+ if (links.length === 0) return;
+ return html`<div class="links">
+ ${links.map(link => this.renderLink(link))}
+ </div>`;
+ }
+
+ private renderSecondaryLinks() {
+ const links = secondaryLinks(this.result);
+ if (links.length === 0) return;
+ return html`<div class="links">
+ ${links.map(link => this.renderLink(link))}
+ </div>`;
+ }
+
+ private renderCodePointers() {
+ const pointers = this.result?.codePointers ?? [];
+ if (pointers.length === 0) return;
+ const links = pointers.map(pointer => {
+ let rangeText = '';
+ const start = pointer?.range?.start_line;
+ const end = pointer?.range?.end_line;
+ if (start) rangeText += `#${start}`;
+ if (end && start !== end) rangeText += `-${end}`;
+ const change = this.changeService.getChange();
+ assertIsDefined(change);
+ const path = pointer.path;
+ const patchset = this.result?.patchset as PatchSetNumber | undefined;
+ const line = pointer?.range?.start_line;
+ return {
+ icon: LinkIcon.CODE,
+ tooltip: `${path}${rangeText}`,
+ url: GerritNav.getUrlForDiff(change, path, patchset, undefined, line),
+ primary: true,
+ };
+ });
+ return links.map(
+ link => html`<div class="links">${this.renderLink(link, false)}</div>`
+ );
+ }
+
+ private renderLink(link?: Link, targetBlank = true) {
+ if (!link) return;
+ const text = link.tooltip ?? tooltipForLink(link.icon);
+ const target = targetBlank ? '_blank' : undefined;
+ return html`<a href="${link.url}" target="${ifDefined(target)}">
+ <iron-icon
+ class="link"
+ icon="gr-icons:${iconForLink(link.icon)}"
+ ></iron-icon
+ ><span>${text}</span>
+ </a>`;
+ }
}
const SHOW_ALL_THRESHOLDS: Map<Category, number> = new Map();
@@ -611,11 +714,11 @@
constructor() {
super();
- this.subscribe('actions', allActions$);
- this.subscribe('links', allLinks$);
- this.subscribe('checksPatchsetNumber', checksPatchsetNumber$);
+ this.subscribe('actions', topLevelActionsSelected$);
+ this.subscribe('links', topLevelLinksSelected$);
+ this.subscribe('checksPatchsetNumber', checksSelectedPatchsetNumber$);
this.subscribe('latestPatchsetNumber', latestPatchNum$);
- this.subscribe('someProvidersAreLoading', someProvidersAreLoading$);
+ this.subscribe('someProvidersAreLoading', someProvidersAreLoadingSelected$);
}
static get styles() {
@@ -634,6 +737,9 @@
var(--spacing-xl);
border-bottom: 1px solid var(--border-color);
}
+ .header.notLatest {
+ background-color: var(--emphasis-color);
+ }
.headerTopRow,
.headerBottomRow {
max-width: 1600px;
@@ -665,10 +771,20 @@
.headerBottomRow {
margin-top: var(--spacing-s);
}
+ .headerTopRow .right,
.headerBottomRow .right {
display: flex;
align-items: center;
}
+ .headerTopRow .right .goToLatest {
+ display: none;
+ }
+ .notLatest .headerTopRow .right .goToLatest {
+ display: block;
+ }
+ .headerTopRow .right .goToLatest gr-button {
+ margin-right: var(--spacing-m);
+ }
.headerBottomRow iron-icon {
color: var(--link-color);
}
@@ -820,8 +936,22 @@
}
render() {
- return html`
- <div class="header">
+ // To pass CSS mixins for @apply to Polymer components, they need to appear
+ // in <style> inside the template.
+ const style = html`<style>
+ .headerTopRow .right .goToLatest gr-button {
+ --gr-button: {
+ padding: var(--spacing-s) var(--spacing-m);
+ text-transform: none;
+ }
+ }
+ </style>`;
+ const headerClasses = classMap({
+ header: true,
+ notLatest: !!this.checksPatchsetNumber,
+ });
+ return html`${style}
+ <div class="${headerClasses}">
<div class="headerTopRow">
<div class="left">
<h2 class="heading-2">Results</h2>
@@ -831,8 +961,13 @@
</div>
</div>
<div class="right">
+ <div class="goToLatest">
+ <gr-button @click="${this.goToLatestPatchset}" link
+ >Go to latest patchset</gr-button
+ >
+ </div>
<gr-dropdown-list
- value="${this.checksPatchsetNumber}"
+ value="${this.checksPatchsetNumber ?? this.latestPatchsetNumber}"
.items="${this.createPatchsetDropdownItems()}"
@value-change="${this.onPatchsetSelected}"
></gr-dropdown-list>
@@ -852,14 +987,20 @@
${this.renderSection(Category.WARNING)}
${this.renderSection(Category.INFO)}
${this.renderSection(Category.SUCCESS)}
- </div>
- `;
+ </div>`;
}
private renderLinks() {
const links = this.links ?? [];
if (links.length === 0) return;
- const primaryLinks = links.filter(a => a.primary).slice(0, 4);
+ const primaryLinks = links
+ .filter(a => a.primary)
+ // Showing the same icons twice without text is super confusing.
+ .filter(
+ (link: Link, index: number, array: Link[]) =>
+ array.findIndex(other => link.icon === other.icon) === index
+ )
+ .slice(0, 4);
const overflowLinks = links.filter(a => !primaryLinks.includes(a));
return html`
${primaryLinks.map(this.renderLink)}
@@ -957,6 +1098,10 @@
this.checksService.setPatchset(patchset as PatchSetNumber);
}
+ private goToLatestPatchset() {
+ this.checksService.setPatchset(undefined);
+ }
+
private createPatchsetDropdownItems() {
if (!this.latestPatchsetNumber) return [];
return Array.from(Array(this.latestPatchsetNumber), (_, i) => {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 21d0459..093eecb 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -40,9 +40,11 @@
worstCategory,
} from '../../services/checks/checks-util';
import {
+ allRunsSelectedPatchset$,
CheckRun,
- allRuns$,
+ ChecksPatchset,
fakeActions,
+ fakeLinks,
fakeRun0,
fakeRun1,
fakeRun2,
@@ -52,7 +54,6 @@
fakeRun4_3,
fakeRun4_4,
updateStateSetResults,
- fakeLinks,
} from '../../services/checks/checks-model';
import {assertIsDefined} from '../../utils/common-util';
import {whenVisible} from '../../utils/dom-util';
@@ -334,7 +335,7 @@
constructor() {
super();
- this.subscribe('runs', allRuns$);
+ this.subscribe('runs', allRunsSelectedPatchset$);
}
static get styles() {
@@ -443,8 +444,8 @@
?hidden="${!this.showFilter()}"
@input="${this.onInput}"
/>
- ${this.renderSection(RunStatus.COMPLETED)}
${this.renderSection(RunStatus.RUNNING)}
+ ${this.renderSection(RunStatus.COMPLETED)}
${this.renderSection(RunStatus.RUNNABLE)} ${this.renderFakeControls()}
`;
}
@@ -514,24 +515,31 @@
}
none() {
- updateStateSetResults('f0', [], []);
- updateStateSetResults('f1', []);
- updateStateSetResults('f2', []);
- updateStateSetResults('f3', []);
- updateStateSetResults('f4', []);
+ updateStateSetResults('f0', [], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f1', [], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f2', [], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f3', [], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f4', [], [], [], ChecksPatchset.LATEST);
}
all() {
- updateStateSetResults('f0', [fakeRun0], fakeActions, fakeLinks);
- updateStateSetResults('f1', [fakeRun1]);
- updateStateSetResults('f2', [fakeRun2]);
- updateStateSetResults('f3', [fakeRun3]);
- updateStateSetResults('f4', [
- fakeRun4_1,
- fakeRun4_2,
- fakeRun4_3,
- fakeRun4_4,
- ]);
+ updateStateSetResults(
+ 'f0',
+ [fakeRun0],
+ fakeActions,
+ fakeLinks,
+ ChecksPatchset.LATEST
+ );
+ updateStateSetResults('f1', [fakeRun1], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f2', [fakeRun2], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults('f3', [fakeRun3], [], [], ChecksPatchset.LATEST);
+ updateStateSetResults(
+ 'f4',
+ [fakeRun4_1, fakeRun4_2, fakeRun4_3, fakeRun4_4],
+ [],
+ [],
+ ChecksPatchset.LATEST
+ );
}
toggle(
@@ -541,7 +549,13 @@
links: Link[] = []
) {
const newRuns = this.runs.includes(runs[0]) ? [] : runs;
- updateStateSetResults(plugin, newRuns, actions, links);
+ updateStateSetResults(
+ plugin,
+ newRuns,
+ actions,
+ links,
+ ChecksPatchset.LATEST
+ );
}
renderSection(status: RunStatus) {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index a04c190..963d28a 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -21,9 +21,9 @@
import {
CheckResult,
CheckRun,
- allResults$,
- allRuns$,
- checksPatchsetNumber$,
+ allResultsSelected$,
+ checksSelectedPatchsetNumber$,
+ allRunsSelectedPatchset$,
} from '../../services/checks/checks-model';
import './gr-checks-runs';
import './gr-checks-results';
@@ -72,9 +72,9 @@
constructor() {
super();
- this.subscribe('runs', allRuns$);
- this.subscribe('results', allResults$);
- this.subscribe('checksPatchsetNumber', checksPatchsetNumber$);
+ this.subscribe('runs', allRunsSelectedPatchset$);
+ this.subscribe('results', allResultsSelected$);
+ this.subscribe('checksPatchsetNumber', checksSelectedPatchsetNumber$);
this.subscribe('changeNum', changeNum$);
this.addEventListener('action-triggered', (e: ActionTriggeredEvent) =>
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 4985c1b..351e1bb 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -217,6 +217,8 @@
url,
trace,
});
+ } else if (response.status === 429) {
+ this._showQuotaExceeded({status, statusText});
} else {
this._showErrorDialog(
this._constructServerErrorMsg({
@@ -260,6 +262,19 @@
});
}
+ _showQuotaExceeded({status, statusText}: ErrorMsg) {
+ const tip = 'Try again later';
+ const errorText = 'Too many requests from this client';
+ this._showErrorDialog(
+ this._constructServerErrorMsg({
+ status,
+ statusText,
+ errorText,
+ tip,
+ })
+ );
+ }
+
_constructServerErrorMsg({
errorText,
status,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index 424df2f..f70974f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -315,7 +315,9 @@
const firstGroupIsSkipped = !!contextGroups[0].skip;
const lastGroupIsSkipped = !!contextGroups[contextGroups.length - 1].skip;
- const showAbove = leftStart > 1 && !firstGroupIsSkipped;
+ const containsWholeFile = this._numLinesLeft === leftEnd - leftStart + 1;
+ const showAbove =
+ (leftStart > 1 && !firstGroupIsSkipped) || containsWholeFile;
const showBelow = leftEnd < this._numLinesLeft && !lastGroupIsSkipped;
if (showAbove) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 010786f..68bdaa6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -55,8 +55,21 @@
private preventAutoScrollOnManualScroll = false;
set side(side: Side) {
+ if (this.sideInternal === side) {
+ return;
+ }
+ if (this.sideInternal && this.diffRow) {
+ this.fireCursorMoved(
+ 'line-cursor-moved-out',
+ this.diffRow,
+ this.sideInternal
+ );
+ }
this.sideInternal = side;
this.updateSideClass();
+ if (this.diffRow) {
+ this.fireCursorMoved('line-cursor-moved-in', this.diffRow, this.side);
+ }
}
get side(): Side {
@@ -478,19 +491,6 @@
toggleClass(this.diffRow, RIGHT_SIDE_CLASS, this.side === Side.RIGHT);
}
- _updateSide(_: Side, oldSide: Side) {
- if (!this.diffRow) {
- return;
- }
- if (oldSide) {
- this.fireCursorMoved('line-cursor-moved-out', this.diffRow, oldSide);
- }
- this.updateSideClass();
- if (this.diffRow) {
- this.fireCursorMoved('line-cursor-moved-in', this.diffRow, this.side);
- }
- }
-
_isActionType(type: GrDiffRowType) {
return (
type !== GrDiffLineType.BLANK && type !== GrDiffGroupType.CONTEXT_CONTROL
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 2ce2abe..d626fe5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -1233,6 +1233,9 @@
}
return;
}
+ // shift + m navigates to next unreviewed file so request list of reviewed
+ // files even if manual review is not set
+ this._getReviewedFiles(this._changeNum, patchRange.patchNum);
if (paramsRecord.base?.view === GerritNav.View.DIFF) {
this._setReviewed(true);
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
index 131825a..948578d 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
@@ -36,9 +36,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.ts b/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.ts
index ac015e1..df0d71a 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.ts
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.ts
@@ -118,9 +118,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
index 7ef5a73..d278d22 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
@@ -105,6 +105,8 @@
})
);
+ promises.push(this.restApiService.invalidateAccountsDetailCache());
+
promises.push(
this.restApiService.getAccount().then(account => {
if (!account) return;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index a4f46db..12173d0 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -28,13 +28,15 @@
import {ParsedChangeInfo} from '../../../types/types';
export enum ChangeStates {
- MERGED = 'Merged',
ABANDONED = 'Abandoned',
+ ACTIVE = 'Active',
MERGE_CONFLICT = 'Merge Conflict',
- WIP = 'WIP',
+ MERGED = 'Merged',
PRIVATE = 'Private',
+ READY_TO_SUBMIT = 'Ready to submit',
REVERT_CREATED = 'Revert Created',
REVERT_SUBMITTED = 'Revert Submitted',
+ WIP = 'WIP',
}
const WIP_TOOLTIP =
@@ -52,7 +54,7 @@
'current reviewers (or anyone with "View Private Changes" permission).';
@customElement('gr-change-status')
-class GrChangeStatus extends PolymerElement {
+export class GrChangeStatus extends PolymerElement {
static get template() {
return htmlTemplate;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.js b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.js
deleted file mode 100644
index 7bc2466..0000000
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.js
+++ /dev/null
@@ -1,153 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import sinon from 'sinon/pkg/sinon-esm';
-import '../../../test/common-test-setup-karma.js';
-import {createChange} from '../../../test/test-data-generators.js';
-import './gr-change-status.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {MERGE_CONFLICT_TOOLTIP} from './gr-change-status.js';
-
-const basicFixture = fixtureFromElement('gr-change-status');
-
-const WIP_TOOLTIP = 'This change isn\'t ready to be reviewed or submitted. ' +
- 'It will not appear on dashboards unless you are CC\'ed or assigned, ' +
- 'and email notifications will be silenced until the review is started.';
-
-const PRIVATE_TOOLTIP = 'This change is only visible to its owner and ' +
- 'current reviewers (or anyone with "View Private Changes" permission).';
-
-suite('gr-change-status tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
- });
-
- test('WIP', () => {
- element.status = 'WIP';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, 'Work in Progress');
- assert.equal(element.tooltipText, WIP_TOOLTIP);
- assert.isTrue(element.classList.contains('wip'));
- });
-
- test('WIP flat', () => {
- element.flat = true;
- element.status = 'WIP';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, 'WIP');
- assert.isDefined(element.tooltipText);
- assert.isTrue(element.classList.contains('wip'));
- assert.isTrue(element.hasAttribute('flat'));
- });
-
- test('merged', () => {
- element.status = 'Merged';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, '');
- assert.isTrue(element.classList.contains('merged'));
- assert.isFalse(
- element.showResolveIcon([{url: 'http://google.com'}], 'Merged'));
- });
-
- test('abandoned', () => {
- element.status = 'Abandoned';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, '');
- assert.isTrue(element.classList.contains('abandoned'));
- });
-
- test('merge conflict', () => {
- const status = 'Merge Conflict';
- element.status = status;
- flush();
-
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, MERGE_CONFLICT_TOOLTIP);
- assert.isTrue(element.classList.contains('merge-conflict'));
- assert.isFalse(element.hasStatusLink(undefined, [], status));
- assert.isFalse(element.showResolveIcon([], status));
- });
-
- test('merge conflict with resolve link', () => {
- const status = 'Merge Conflict';
- const url = 'http://google.com';
- const weblinks = [{url}];
-
- assert.isTrue(element.hasStatusLink(undefined, weblinks, status));
- assert.equal(element.getStatusLink(undefined, weblinks, status), url);
- assert.isTrue(element.showResolveIcon(weblinks, status));
- });
-
- test('reverted change', () => {
- const url = 'http://google.com';
- const status = 'Revert Submitted';
- const revertedChange = createChange();
- sinon.stub(GerritNav, 'getUrlForSearchQuery').returns(url);
-
- assert.isTrue(element.hasStatusLink(revertedChange, [], status));
- assert.equal(element.getStatusLink(revertedChange, [], status), url);
- });
-
- test('private', () => {
- element.status = 'Private';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, PRIVATE_TOOLTIP);
- assert.isTrue(element.classList.contains('private'));
- });
-
- test('active', () => {
- element.status = 'Active';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, '');
- assert.isTrue(element.classList.contains('active'));
- });
-
- test('ready to submit', () => {
- element.status = 'Ready to submit';
- flush();
- assert.equal(element.shadowRoot
- .querySelector('.chip').innerText, element.status);
- assert.equal(element.tooltipText, '');
- assert.isTrue(element.classList.contains('ready-to-submit'));
- });
-
- test('updating status removes the previous class', () => {
- element.status = 'Private';
- flush();
- assert.isTrue(element.classList.contains('private'));
- assert.isFalse(element.classList.contains('wip'));
-
- element.status = 'WIP';
- flush();
- assert.isFalse(element.classList.contains('private'));
- assert.isTrue(element.classList.contains('wip'));
- });
-});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts
new file mode 100644
index 0000000..a56f6f1
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts
@@ -0,0 +1,172 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import sinon from 'sinon/pkg/sinon-esm';
+import '../../../test/common-test-setup-karma';
+import {createChange} from '../../../test/test-data-generators';
+import './gr-change-status';
+import {ChangeStates, GrChangeStatus} from './gr-change-status';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {MERGE_CONFLICT_TOOLTIP} from './gr-change-status';
+
+const basicFixture = fixtureFromElement('gr-change-status');
+
+const WIP_TOOLTIP =
+ "This change isn't ready to be reviewed or submitted. " +
+ "It will not appear on dashboards unless you are CC'ed or assigned, " +
+ 'and email notifications will be silenced until the review is started.';
+
+const PRIVATE_TOOLTIP =
+ 'This change is only visible to its owner and ' +
+ 'current reviewers (or anyone with "View Private Changes" permission).';
+
+suite('gr-change-status tests', () => {
+ let element: GrChangeStatus;
+
+ setup(() => {
+ element = basicFixture.instantiate();
+ });
+
+ test('WIP', () => {
+ element.status = ChangeStates.WIP;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Work in Progress'
+ );
+ assert.equal(element.tooltipText, WIP_TOOLTIP);
+ assert.isTrue(element.classList.contains('wip'));
+ });
+
+ test('WIP flat', () => {
+ element.flat = true;
+ element.status = ChangeStates.WIP;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'WIP'
+ );
+ assert.isDefined(element.tooltipText);
+ assert.isTrue(element.classList.contains('wip'));
+ assert.isTrue(element.hasAttribute('flat'));
+ });
+
+ test('merged', () => {
+ element.status = ChangeStates.MERGED;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Merged'
+ );
+ assert.equal(element.tooltipText, '');
+ assert.isTrue(element.classList.contains('merged'));
+ assert.isFalse(
+ element.showResolveIcon([{url: 'http://google.com'}], ChangeStates.MERGED)
+ );
+ });
+
+ test('abandoned', () => {
+ element.status = ChangeStates.ABANDONED;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Abandoned'
+ );
+ assert.equal(element.tooltipText, '');
+ assert.isTrue(element.classList.contains('abandoned'));
+ });
+
+ test('merge conflict', () => {
+ const status = ChangeStates.MERGE_CONFLICT;
+ element.status = status;
+ flush();
+
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Merge Conflict'
+ );
+ assert.equal(element.tooltipText, MERGE_CONFLICT_TOOLTIP);
+ assert.isTrue(element.classList.contains('merge-conflict'));
+ assert.isFalse(element.hasStatusLink(undefined, [], status));
+ assert.isFalse(element.showResolveIcon([], status));
+ });
+
+ test('merge conflict with resolve link', () => {
+ const status = ChangeStates.MERGE_CONFLICT;
+ const url = 'http://google.com';
+ const weblinks = [{url}];
+
+ assert.isTrue(element.hasStatusLink(undefined, weblinks, status));
+ assert.equal(element.getStatusLink(undefined, weblinks, status), url);
+ assert.isTrue(element.showResolveIcon(weblinks, status));
+ });
+
+ test('reverted change', () => {
+ const url = 'http://google.com';
+ const status = ChangeStates.REVERT_SUBMITTED;
+ const revertedChange = createChange();
+ sinon.stub(GerritNav, 'getUrlForSearchQuery').returns(url);
+
+ assert.isTrue(element.hasStatusLink(revertedChange, [], status));
+ assert.equal(element.getStatusLink(revertedChange, [], status), url);
+ });
+
+ test('private', () => {
+ element.status = ChangeStates.PRIVATE;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Private'
+ );
+ assert.equal(element.tooltipText, PRIVATE_TOOLTIP);
+ assert.isTrue(element.classList.contains('private'));
+ });
+
+ test('active', () => {
+ element.status = ChangeStates.ACTIVE;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Active'
+ );
+ assert.equal(element.tooltipText, '');
+ assert.isTrue(element.classList.contains('active'));
+ });
+
+ test('ready to submit', () => {
+ element.status = ChangeStates.READY_TO_SUBMIT;
+ flush();
+ assert.equal(
+ element.shadowRoot!.querySelector<HTMLDivElement>('.chip')!.innerText,
+ 'Ready to submit'
+ );
+ assert.equal(element.tooltipText, '');
+ assert.isTrue(element.classList.contains('ready-to-submit'));
+ });
+
+ test('updating status removes the previous class', () => {
+ element.status = ChangeStates.PRIVATE;
+ flush();
+ assert.isTrue(element.classList.contains('private'));
+ assert.isFalse(element.classList.contains('wip'));
+
+ element.status = ChangeStates.WIP;
+ flush();
+ assert.isFalse(element.classList.contains('private'));
+ assert.isTrue(element.classList.contains('wip'));
+ });
+});
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
index e3a34c8..8968e18 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
@@ -148,6 +148,8 @@
<g id="playArrow"><path d="M0 0h24v24H0z" fill="none"/><path d="M8 5v14l11-7z"/></g>
<!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material%20Icons%3Apause-->
<g id="pause"><path d="M0 0h24v24H0z" fill="none"/><path d="M6 19h4V5H6v14zm8-14v14h4V5h-4z"/></g>
+ <!-- This SVG is a copy from material.io https://material.io/icons/#code-->
+ <g id="code"><path d="M0 0h24v24H0V0z" fill="none"/><path d="M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z"/></g>
</defs>
</svg>
</iron-iconset-svg>`;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.js b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
similarity index 68%
rename from polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.js
rename to polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
index a67fbc4..b2cdba1 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
@@ -15,27 +15,30 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-linked-text.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+import '../../../test/common-test-setup-karma';
+import './gr-linked-text';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {html} from '@polymer/polymer/lib/utils/html-tag';
+import {GrLinkedText} from './gr-linked-text';
+import {CommentLinks} from '../../../types/common';
+import {queryAndAssert} from '../../../test/test-utils';
const basicFixture = fixtureFromTemplate(html`
-<gr-linked-text>
- <div id="output"></div>
- </gr-linked-text>
+ <gr-linked-text>
+ <div id="output"></div>
+ </gr-linked-text>
`);
suite('gr-linked-text tests', () => {
- let element;
+ let element: GrLinkedText;
- let originalCanonicalPath;
+ let originalCanonicalPath: string | undefined;
setup(() => {
originalCanonicalPath = window.CANONICAL_PATH;
- element = basicFixture.instantiate();
+ element = basicFixture.instantiate() as GrLinkedText;
- sinon.stub(GerritNav, 'mapCommentlinks').value( x => x);
+ sinon.stub(GerritNav, 'mapCommentlinks').value((x: CommentLinks) => x);
element.config = {
ph: {
match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
@@ -86,7 +89,8 @@
// Regular inline link.
const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
element.content = url;
- const linkEl = element.$.output.childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
assert.equal(linkEl.target, '_blank');
assert.equal(linkEl.rel, 'noopener');
assert.equal(linkEl.href, url);
@@ -97,14 +101,16 @@
// "Issue/Bug" pattern.
element.content = 'Issue 3650';
- let linkEl = element.$.output.childNodes[0];
+ let linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
assert.equal(linkEl.target, '_blank');
assert.equal(linkEl.href, url);
assert.equal(linkEl.textContent, 'Issue 3650');
element.content = 'Bug 3650';
- linkEl = element.$.output.childNodes[0];
+ linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
assert.equal(linkEl.target, '_blank');
assert.equal(linkEl.rel, 'noopener');
assert.equal(linkEl.href, url);
@@ -115,8 +121,9 @@
// Pattern starts with the same prefix (`http`) as the url.
element.content = 'httpexample 3650';
- assert.equal(element.$.output.childNodes.length, 1);
- const linkEl = element.$.output.childNodes[0];
+ assert.equal(queryAndAssert(element, '#output').childNodes.length, 1);
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
assert.equal(linkEl.target, '_blank');
assert.equal(linkEl.href, url);
@@ -129,8 +136,9 @@
const prefix = 'Change-Id: ';
element.content = prefix + changeID;
- const textNode = element.$.output.childNodes[0];
- const linkEl = element.$.output.childNodes[1];
+ const textNode = queryAndAssert(element, '#output').childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[1] as HTMLAnchorElement;
assert.equal(textNode.textContent, prefix);
const url = '/q/' + changeID;
assert.isFalse(linkEl.hasAttribute('target'));
@@ -147,8 +155,9 @@
const prefix = 'Change-Id: ';
element.content = prefix + changeID;
- const textNode = element.$.output.childNodes[0];
- const linkEl = element.$.output.childNodes[1];
+ const textNode = queryAndAssert(element, '#output').childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[1] as HTMLAnchorElement;
assert.equal(textNode.textContent, prefix);
const url = '/r/q/' + changeID;
assert.isFalse(linkEl.hasAttribute('target'));
@@ -159,17 +168,23 @@
test('Multiple matches', () => {
element.content = 'Issue 3650\nIssue 3450';
- const linkEl1 = element.$.output.childNodes[0];
- const linkEl2 = element.$.output.childNodes[2];
+ const linkEl1 = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
+ const linkEl2 = queryAndAssert(element, '#output')
+ .childNodes[2] as HTMLAnchorElement;
assert.equal(linkEl1.target, '_blank');
- assert.equal(linkEl1.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650');
+ assert.equal(
+ linkEl1.href,
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650'
+ );
assert.equal(linkEl1.textContent, 'Issue 3650');
assert.equal(linkEl2.target, '_blank');
- assert.equal(linkEl2.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450');
+ assert.equal(
+ linkEl2.href,
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450'
+ );
assert.equal(linkEl2.textContent, 'Issue 3450');
});
@@ -186,9 +201,11 @@
element.content = prefix + changeID + bug;
- const textNode = element.$.output.childNodes[0];
- const changeLinkEl = element.$.output.childNodes[1];
- const bugLinkEl = element.$.output.childNodes[2];
+ const textNode = queryAndAssert(element, '#output').childNodes[0];
+ const changeLinkEl = queryAndAssert(element, '#output')
+ .childNodes[1] as HTMLAnchorElement;
+ const bugLinkEl = queryAndAssert(element, '#output')
+ .childNodes[2] as HTMLAnchorElement;
assert.equal(textNode.textContent, prefix);
@@ -203,15 +220,19 @@
test('html field in link config', () => {
element.content = 'google:do a barrel roll';
- const linkEl = element.$.output.childNodes[0];
- assert.equal(linkEl.getAttribute('href'),
- 'https://google.com/search?q=do a barrel roll');
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
+ assert.equal(
+ linkEl.getAttribute('href'),
+ 'https://google.com/search?q=do a barrel roll'
+ );
assert.equal(linkEl.textContent, 'do a barrel roll');
});
test('removing hash from links', () => {
element.content = 'hash:foo';
- const linkEl = element.$.output.childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
assert.isTrue(linkEl.href.endsWith('/awesomesauce'));
assert.equal(linkEl.textContent, 'foo');
});
@@ -220,7 +241,8 @@
window.CANONICAL_PATH = '/r';
element.content = 'test foo';
- const linkEl = element.$.output.childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
assert.equal(linkEl.textContent, 'foo');
});
@@ -229,7 +251,8 @@
window.CANONICAL_PATH = '/r';
element.content = 'a test foo';
- const linkEl = element.$.output.childNodes[1];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[1] as HTMLAnchorElement;
assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
assert.equal(linkEl.textContent, 'foo');
});
@@ -238,94 +261,119 @@
window.CANONICAL_PATH = '/r';
element.content = 'hash:foo';
- const linkEl = element.$.output.childNodes[0];
+ const linkEl = queryAndAssert(element, '#output')
+ .childNodes[0] as HTMLAnchorElement;
assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
assert.equal(linkEl.textContent, 'foo');
});
test('disabled config', () => {
element.content = 'foo:baz';
- assert.equal(element.$.output.innerHTML, 'foo:baz');
+ assert.equal(queryAndAssert(element, '#output').innerHTML, 'foo:baz');
});
test('R=email labels link correctly', () => {
element.removeZeroWidthSpace = true;
element.content = 'R=\u200Btest@google.com';
- assert.equal(element.$.output.textContent, 'R=test@google.com');
- assert.equal(element.$.output.innerHTML.match(/(R=<a)/g).length, 1);
+ assert.equal(
+ queryAndAssert(element, '#output').textContent,
+ 'R=test@google.com'
+ );
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.match(/(R=<a)/g)!.length,
+ 1
+ );
});
test('CC=email labels link correctly', () => {
element.removeZeroWidthSpace = true;
element.content = 'CC=\u200Btest@google.com';
- assert.equal(element.$.output.textContent, 'CC=test@google.com');
- assert.equal(element.$.output.innerHTML.match(/(CC=<a)/g).length, 1);
+ assert.equal(
+ queryAndAssert(element, '#output').textContent,
+ 'CC=test@google.com'
+ );
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.match(/(CC=<a)/g)!.length,
+ 1
+ );
});
test('only {http,https,mailto} protocols are linkified', () => {
element.content = 'xx mailto:test@google.com yy';
- let links = element.$.output.querySelectorAll('a');
+ let links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
assert.equal(links[0].innerHTML, 'mailto:test@google.com');
element.content = 'xx http://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'http://google.com');
assert.equal(links[0].innerHTML, 'http://google.com');
element.content = 'xx https://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'https://google.com');
assert.equal(links[0].innerHTML, 'https://google.com');
element.content = 'xx ssh://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 0);
element.content = 'xx ftp://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 0);
});
test('links without leading whitespace are linkified', () => {
element.content = 'xx abcmailto:test@google.com yy';
- assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx abc');
- let links = element.$.output.querySelectorAll('a');
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.substr(0, 6),
+ 'xx abc'
+ );
+ let links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
assert.equal(links[0].innerHTML, 'mailto:test@google.com');
element.content = 'xx defhttp://google.com yy';
- assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx def');
- links = element.$.output.querySelectorAll('a');
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.substr(0, 6),
+ 'xx def'
+ );
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'http://google.com');
assert.equal(links[0].innerHTML, 'http://google.com');
element.content = 'xx qwehttps://google.com yy';
- assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx qwe');
- links = element.$.output.querySelectorAll('a');
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.substr(0, 6),
+ 'xx qwe'
+ );
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'https://google.com');
assert.equal(links[0].innerHTML, 'https://google.com');
// Non-latin character
element.content = 'xx абвhttps://google.com yy';
- assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx абв');
- links = element.$.output.querySelectorAll('a');
+ assert.equal(
+ queryAndAssert(element, '#output').innerHTML.substr(0, 6),
+ 'xx абв'
+ );
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'https://google.com');
assert.equal(links[0].innerHTML, 'https://google.com');
element.content = 'xx ssh://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 0);
element.content = 'xx ftp://google.com yy';
- links = element.$.output.querySelectorAll('a');
+ links = queryAndAssert(element, '#output').querySelectorAll('a');
assert.equal(links.length, 0);
});
@@ -341,11 +389,13 @@
},
};
element.content = '- B: 123, 45';
- const links = element.root.querySelectorAll('a');
+ const links = element.root!.querySelectorAll('a');
assert.equal(links.length, 2);
- assert.equal(element.shadowRoot
- .querySelector('span').textContent, '- B: 123, 45');
+ assert.equal(
+ queryAndAssert<HTMLSpanElement>(element, 'span').textContent,
+ '- B: 123, 45'
+ );
assert.equal(links[0].href, 'ftp://foo/123');
assert.equal(links[0].textContent, '123');
@@ -362,4 +412,3 @@
assert.isTrue(contentConfigStub.called);
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 852f0b6..28bc229 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -1443,6 +1443,10 @@
this._restApiHelper.invalidateFetchPromisesPrefix('/accounts/');
}
+ invalidateAccountsDetailCache() {
+ this._restApiHelper.invalidateFetchPromisesPrefix('/accounts/self/detail');
+ }
+
getGroups(filter: string, groupsPerPage: number, offset?: number) {
const url = this._getGroupsUrl(filter, groupsPerPage, offset);
diff --git a/polygerrit-ui/app/elements/shared/gr-select/gr-select_test.js b/polygerrit-ui/app/elements/shared/gr-select/gr-select_test.ts
similarity index 75%
rename from polygerrit-ui/app/elements/shared/gr-select/gr-select_test.js
rename to polygerrit-ui/app/elements/shared/gr-select/gr-select_test.ts
index c697850..245d7df 100644
--- a/polygerrit-ui/app/elements/shared/gr-select/gr-select_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-select/gr-select_test.ts
@@ -15,41 +15,42 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-select.js';
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+import '../../../test/common-test-setup-karma';
+import './gr-select';
+import {html} from '@polymer/polymer/lib/utils/html-tag';
+import {GrSelect} from './gr-select';
const basicFixture = fixtureFromTemplate(html`
-<gr-select>
- <select>
- <option value="1">One</option>
- <option value="2">Two</option>
- <option value="3">Three</option>
- </select>
- </gr-select>
+ <gr-select>
+ <select>
+ <option value="1">One</option>
+ <option value="2">Two</option>
+ <option value="3">Three</option>
+ </select>
+ </gr-select>
`);
const noOptionsFixture = fixtureFromTemplate(html`
-<gr-select>
- <select>
- </select>
- </gr-select>
+ <gr-select>
+ <select></select>
+ </gr-select>
`);
suite('gr-select tests', () => {
- let element;
+ let element: GrSelect;
setup(() => {
- element = basicFixture.instantiate();
+ element = basicFixture.instantiate() as GrSelect;
});
test('bindValue must be set to the first option value', () => {
assert.equal(element.bindValue, '1');
+ assert.equal(element.nativeSelect.value, '1');
});
test('value of 0 should still trigger value updates', () => {
- element.bindValue = 0;
- assert.equal(element.nativeSelect.value, 0);
+ element.bindValue = '0';
+ assert.equal(element.nativeSelect.value, '');
});
test('bidirectional binding property-to-attribute', () => {
@@ -82,9 +83,11 @@
// Now change the value.
element.nativeSelect.value = '3';
element.dispatchEvent(
- new CustomEvent('change', {
- composed: true, bubbles: true,
- }));
+ new CustomEvent('change', {
+ composed: true,
+ bubbles: true,
+ })
+ );
// It should be updated.
assert.equal(element.nativeSelect.value, '3');
@@ -93,10 +96,10 @@
});
suite('gr-select no options tests', () => {
- let element;
+ let element: GrSelect;
setup(() => {
- element = noOptionsFixture.instantiate();
+ element = noOptionsFixture.instantiate() as GrSelect;
});
test('bindValue must not be changed', () => {
@@ -104,4 +107,3 @@
});
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index b3d1578..a747ac4 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -227,11 +227,13 @@
this._setEmoji(this.$.emojiSuggestions.getCurrentText());
}
- _handleEnterByKey(e: KeyboardEvent) {
+ _handleEnterByKey(e: CustomEvent<{keyboardEvent: KeyboardEvent}>) {
// Enter should have newline behavior if the picker is closed or if the user
- // has only typed ':'.
+ // has only typed ':'. Also make sure that shortcuts aren't clobbered.
if (this._hideEmojiAutocomplete || this.disableEnterKeyForSelectingEmoji) {
- this.indent(e);
+ if (!e.detail.keyboardEvent.metaKey && !e.detail.keyboardEvent.ctrlKey) {
+ this.indent(e);
+ }
return;
}
@@ -402,7 +404,7 @@
);
}
- private indent(e: KeyboardEvent): void {
+ private indent(e: CustomEvent<{keyboardEvent: KeyboardEvent}>): void {
if (!document.queryCommandSupported('insertText')) {
return;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.js b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.js
index 1747fce..7c2f209 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.js
@@ -230,6 +230,26 @@
assert.deepEqual(indentCommand.args[0], ['insertText', false, '\n ']);
});
+ test('ctrl+enter and meta+enter do not indent', async () => {
+ const indentCommand = sinon.stub(document, 'execCommand');
+ element.$.textarea.value = ' a';
+ element._handleEnterByKey(
+ new CustomEvent('keydown', {
+ detail: {keyboardEvent: {keyCode: 13, ctrlKey: true}},
+ })
+ );
+ await flush();
+ assert.isTrue(indentCommand.notCalled);
+
+ element._handleEnterByKey(
+ new CustomEvent('keydown', {
+ detail: {keyboardEvent: {keyCode: 13, metaKey: true}},
+ })
+ );
+ await flush();
+ assert.isTrue(indentCommand.notCalled);
+ });
+
test('emoji dropdown is closed when iron-overlay-closed is fired', () => {
const resetSpy = sinon.spy(element, '_resetEmojiDropdown');
element.$.emojiSuggestions.dispatchEvent(
diff --git a/polygerrit-ui/app/services/change/change-model.ts b/polygerrit-ui/app/services/change/change-model.ts
index 9036871..312e78d 100644
--- a/polygerrit-ui/app/services/change/change-model.ts
+++ b/polygerrit-ui/app/services/change/change-model.ts
@@ -48,7 +48,7 @@
export function updateState(change?: ParsedChangeInfo) {
const current = privateState$.getValue();
// We want to make it easy for subscribers to react to change changes, so we
- // are explicitly emitting and additional `undefined` when the change number
+ // are explicitly emitting an additional `undefined` when the change number
// changes. So if you are subscribed to the latestPatchsetNumber for example,
// then you can rely on emissions even if the old and the new change have the
// same latestPatchsetNumber.
diff --git a/polygerrit-ui/app/services/change/change-service.ts b/polygerrit-ui/app/services/change/change-service.ts
index 1c6fc4c..0b9a1f2 100644
--- a/polygerrit-ui/app/services/change/change-service.ts
+++ b/polygerrit-ui/app/services/change/change-service.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import {routerChangeNum$} from '../router/router-model';
-import {updateState} from './change-model';
+import {change$, updateState} from './change-model';
import {ParsedChangeInfo} from '../../types/types';
import {appContext} from '../app-context';
import {ChangeInfo} from '../../types/common';
@@ -25,6 +25,8 @@
} from '../../utils/patch-set-util';
export class ChangeService {
+ private change?: ParsedChangeInfo;
+
private readonly restApiService = appContext.restApiService;
constructor() {
@@ -34,6 +36,9 @@
routerChangeNum$.subscribe(changeNum => {
if (!changeNum) updateState(undefined);
});
+ change$.subscribe(change => {
+ this.change = change;
+ });
}
/**
@@ -48,6 +53,15 @@
}
/**
+ * Typically you would just subscribe to change$ yourself to get updates. But
+ * sometimes it is nice to also be able to get the current ChangeInfo on
+ * demand. So here it is for your convenience.
+ */
+ getChange() {
+ return this.change;
+ }
+
+ /**
* Check whether there is no newer patch than the latest patch that was
* available when this change was loaded.
*
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 60cb780..5d3da42 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -21,7 +21,6 @@
Category,
CheckResult as CheckResultApi,
CheckRun as CheckRunApi,
- ChecksApiConfig,
Link,
LinkIcon,
RunStatus,
@@ -32,6 +31,17 @@
import {AttemptDetail, createAttemptMap} from './checks-util';
import {assertIsDefined} from '../../utils/common-util';
+/**
+ * The checks model maintains the state of checks for two patchsets: the latest
+ * and (if different) also for the one selected in the checks tab. So we need
+ * the distinction in a lot of places for checks about whether the code affects
+ * the checks data of the LATEST or the SELECTED patchset.
+ */
+export enum ChecksPatchset {
+ LATEST = 'LATEST',
+ SELECTED = 'SELECTED',
+}
+
export interface CheckResult extends CheckResultApi {
/**
* Internally we want to uniquely identify a run with an id, for example when
@@ -75,21 +85,33 @@
errorMessage?: string;
/** Presence of loginCallback implicitly means that the provider is in NOT_LOGGED_IN state. */
loginCallback?: () => void;
- config?: ChecksApiConfig;
runs: CheckRun[];
actions: Action[];
links: Link[];
}
interface ChecksState {
- patchsetNumber?: PatchSetNumber;
- providerNameToState: {
+ /**
+ * This is the patchset number selected by the user. The *latest* patchset
+ * can be picked up from the change model.
+ */
+ patchsetNumberSelected?: PatchSetNumber;
+ /** Checks data for the latest patchset. */
+ pluginStateLatest: {
+ [name: string]: ChecksProviderState;
+ };
+ /**
+ * Checks data for the selected patchset. Note that `checksSelected$` below
+ * falls back to the data for the latest patchset, if no patchset is selected.
+ */
+ pluginStateSelected: {
[name: string]: ChecksProviderState;
};
}
const initialState: ChecksState = {
- providerNameToState: {},
+ pluginStateLatest: {},
+ pluginStateSelected: {},
};
const privateState$ = new BehaviorSubject(initialState);
@@ -97,29 +119,45 @@
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const checksState$: Observable<ChecksState> = privateState$;
-export const checksPatchsetNumber$ = checksState$.pipe(
- map(state => state.patchsetNumber),
+export const checksSelectedPatchsetNumber$ = checksState$.pipe(
+ map(state => state.patchsetNumberSelected),
distinctUntilChanged()
);
-export const checksProviderState$ = checksState$.pipe(
- map(state => state.providerNameToState),
+export const checksLatest$ = checksState$.pipe(
+ map(state => state.pluginStateLatest),
distinctUntilChanged()
);
-export const aPluginHasRegistered$ = checksProviderState$.pipe(
+export const checksSelected$ = checksState$.pipe(
+ map(state =>
+ state.patchsetNumberSelected
+ ? state.pluginStateSelected
+ : state.pluginStateLatest
+ ),
+ distinctUntilChanged()
+);
+
+export const aPluginHasRegistered$ = checksLatest$.pipe(
map(state => Object.keys(state).length > 0),
distinctUntilChanged()
);
-export const someProvidersAreLoading$ = checksProviderState$.pipe(
+export const someProvidersAreLoadingLatest$ = checksLatest$.pipe(
map(state =>
Object.values(state).some(providerState => providerState.loading)
),
distinctUntilChanged()
);
-export const errorMessage$ = checksProviderState$.pipe(
+export const someProvidersAreLoadingSelected$ = checksSelected$.pipe(
+ map(state =>
+ Object.values(state).some(providerState => providerState.loading)
+ ),
+ distinctUntilChanged()
+);
+
+export const errorMessageLatest$ = checksLatest$.pipe(
map(
state =>
Object.values(state).find(
@@ -129,7 +167,7 @@
distinctUntilChanged()
);
-export const loginCallback$ = checksProviderState$.pipe(
+export const loginCallbackLatest$ = checksLatest$.pipe(
map(
state =>
Object.values(state).find(
@@ -139,7 +177,7 @@
distinctUntilChanged()
);
-export const allActions$ = checksProviderState$.pipe(
+export const topLevelActionsSelected$ = checksSelected$.pipe(
map(state =>
Object.values(state).reduce(
(allActions: Action[], providerState: ChecksProviderState) => [
@@ -151,7 +189,7 @@
)
);
-export const allLinks$ = checksProviderState$.pipe(
+export const topLevelLinksSelected$ = checksSelected$.pipe(
map(state =>
Object.values(state).reduce(
(allActions: Link[], providerState: ChecksProviderState) => [
@@ -163,7 +201,7 @@
)
);
-export const allRuns$ = checksProviderState$.pipe(
+export const allRunsLatestPatchset$ = checksLatest$.pipe(
map(state =>
Object.values(state).reduce(
(allRuns: CheckRun[], providerState: ChecksProviderState) => [
@@ -175,11 +213,23 @@
)
);
-export const allRunsLatest$ = allRuns$.pipe(
+export const allRunsSelectedPatchset$ = checksSelected$.pipe(
+ map(state =>
+ Object.values(state).reduce(
+ (allRuns: CheckRun[], providerState: ChecksProviderState) => [
+ ...allRuns,
+ ...providerState.runs,
+ ],
+ []
+ )
+ )
+);
+
+export const allRunsLatestPatchsetLatestAttempt$ = allRunsLatestPatchset$.pipe(
map(runs => runs.filter(run => run.isLatestAttempt))
);
-export const checkToPluginMap$ = checksProviderState$.pipe(
+export const checkToPluginMap$ = checksLatest$.pipe(
map(state => {
const map = new Map<string, string>();
for (const [pluginName, providerState] of Object.entries(state)) {
@@ -191,7 +241,7 @@
})
);
-export const allResults$ = checksProviderState$.pipe(
+export const allResultsSelected$ = checksSelected$.pipe(
map(state =>
Object.values(state)
.reduce(
@@ -211,16 +261,12 @@
// Must only be used by the checks service or whatever is in control of this
// model.
-export function updateStateSetProvider(
- pluginName: string,
- config?: ChecksApiConfig
-) {
+export function updateStateSetProvider(pluginName: string) {
const nextState = {...privateState$.getValue()};
- nextState.providerNameToState = {...nextState.providerNameToState};
- nextState.providerNameToState[pluginName] = {
+ nextState.pluginStateLatest = {...nextState.pluginStateLatest};
+ nextState.pluginStateLatest[pluginName] = {
pluginName,
loading: false,
- config,
runs: [],
actions: [],
links: [],
@@ -253,6 +299,7 @@
internalResultId: 'f0r1',
category: Category.ERROR,
summary: 'Running the mighty test has failed by crashing.',
+ message: 'Btw, 1 is also not equal to 3. Did you know?',
actions: [
{
name: 'Ignore',
@@ -276,8 +323,16 @@
],
tags: [{name: 'INTERRUPTED', color: TagColor.BROWN}, {name: 'WINDOWS'}],
links: [
- {primary: true, url: 'https://google.com', icon: LinkIcon.EXTERNAL},
+ {primary: false, url: 'https://google.com', icon: LinkIcon.EXTERNAL},
{primary: true, url: 'https://google.com', icon: LinkIcon.DOWNLOAD},
+ {
+ primary: true,
+ url: 'https://google.com',
+ icon: LinkIcon.DOWNLOAD_MOBILE,
+ },
+ {primary: true, url: 'https://google.com', icon: LinkIcon.IMAGE},
+ {primary: true, url: 'https://google.com', icon: LinkIcon.IMAGE},
+ {primary: false, url: 'https://google.com', icon: LinkIcon.IMAGE},
{primary: true, url: 'https://google.com', icon: LinkIcon.REPORT_BUG},
{primary: true, url: 'https://google.com', icon: LinkIcon.HELP_PAGE},
{primary: true, url: 'https://google.com', icon: LinkIcon.HISTORY},
@@ -290,6 +345,7 @@
export const fakeRun1: CheckRun = {
internalRunId: 'f1',
checkName: 'FAKE Super Check',
+ patchset: 1,
labelName: 'Verified',
isSingleAttempt: true,
isLatestAttempt: true,
@@ -302,6 +358,26 @@
message: `There is a lot to be said. A lot. I say, a lot.\n
So please keep reading.`,
tags: [{name: 'INTERRUPTED', color: TagColor.PURPLE}, {name: 'WINDOWS'}],
+ codePointers: [
+ {
+ path: '/COMMIT_MSG',
+ range: {
+ start_line: 10,
+ start_character: 0,
+ end_line: 10,
+ end_character: 0,
+ },
+ },
+ {
+ path: 'polygerrit-ui/app/api/checks.ts',
+ range: {
+ start_line: 5,
+ start_character: 0,
+ end_line: 7,
+ end_character: 0,
+ },
+ },
+ ],
links: [
{primary: true, url: 'https://google.com', icon: LinkIcon.EXTERNAL},
{primary: true, url: 'https://google.com', icon: LinkIcon.DOWNLOAD},
@@ -311,6 +387,18 @@
icon: LinkIcon.DOWNLOAD_MOBILE,
},
{primary: true, url: 'https://google.com', icon: LinkIcon.IMAGE},
+ {
+ primary: false,
+ url: 'https://google.com',
+ tooltip: 'look at this',
+ icon: LinkIcon.IMAGE,
+ },
+ {
+ primary: false,
+ url: 'https://google.com',
+ tooltip: 'not at this',
+ icon: LinkIcon.IMAGE,
+ },
],
},
],
@@ -472,32 +560,82 @@
{
url: 'https://www.google.com',
primary: true,
- tooltip: 'Tooltip for Bug Report Fake Link',
+ tooltip: 'Fake Bug Report 1',
icon: LinkIcon.REPORT_BUG,
},
{
url: 'https://www.google.com',
- primary: false,
- tooltip: 'Tooltip for External Fake Link',
+ primary: true,
+ tooltip: 'Fake Bug Report 2',
+ icon: LinkIcon.REPORT_BUG,
+ },
+ {
+ url: 'https://www.google.com',
+ primary: true,
+ tooltip: 'Fake Link 1',
icon: LinkIcon.EXTERNAL,
},
+ {
+ url: 'https://www.google.com',
+ primary: false,
+ tooltip: 'Fake Link 2',
+ icon: LinkIcon.EXTERNAL,
+ },
+ {
+ url: 'https://www.google.com',
+ primary: true,
+ tooltip: 'Fake Code Link',
+ icon: LinkIcon.CODE,
+ },
+ {
+ url: 'https://www.google.com',
+ primary: true,
+ tooltip: 'Fake Image Link',
+ icon: LinkIcon.IMAGE,
+ },
+ {
+ url: 'https://www.google.com',
+ primary: true,
+ tooltip: 'Fake Help Link',
+ icon: LinkIcon.HELP_PAGE,
+ },
];
-export function updateStateSetLoading(pluginName: string) {
+export function getPluginState(
+ state: ChecksState,
+ patchset: ChecksPatchset = ChecksPatchset.LATEST
+) {
+ if (patchset === ChecksPatchset.LATEST) {
+ state.pluginStateLatest = {...state.pluginStateLatest};
+ return state.pluginStateLatest;
+ } else {
+ state.pluginStateSelected = {...state.pluginStateSelected};
+ return state.pluginStateSelected;
+ }
+}
+
+export function updateStateSetLoading(
+ pluginName: string,
+ patchset: ChecksPatchset
+) {
const nextState = {...privateState$.getValue()};
- nextState.providerNameToState = {...nextState.providerNameToState};
- nextState.providerNameToState[pluginName] = {
- ...nextState.providerNameToState[pluginName],
+ const pluginState = getPluginState(nextState, patchset);
+ pluginState[pluginName] = {
+ ...pluginState[pluginName],
loading: true,
};
privateState$.next(nextState);
}
-export function updateStateSetError(pluginName: string, errorMessage: string) {
+export function updateStateSetError(
+ pluginName: string,
+ errorMessage: string,
+ patchset: ChecksPatchset
+) {
const nextState = {...privateState$.getValue()};
- nextState.providerNameToState = {...nextState.providerNameToState};
- nextState.providerNameToState[pluginName] = {
- ...nextState.providerNameToState[pluginName],
+ const pluginState = getPluginState(nextState, patchset);
+ pluginState[pluginName] = {
+ ...pluginState[pluginName],
loading: false,
errorMessage,
loginCallback: undefined,
@@ -509,12 +647,13 @@
export function updateStateSetNotLoggedIn(
pluginName: string,
- loginCallback: () => void
+ loginCallback: () => void,
+ patchset: ChecksPatchset
) {
const nextState = {...privateState$.getValue()};
- nextState.providerNameToState = {...nextState.providerNameToState};
- nextState.providerNameToState[pluginName] = {
- ...nextState.providerNameToState[pluginName],
+ const pluginState = getPluginState(nextState, patchset);
+ pluginState[pluginName] = {
+ ...pluginState[pluginName],
loading: false,
errorMessage: undefined,
loginCallback,
@@ -528,7 +667,8 @@
pluginName: string,
runs: CheckRunApi[],
actions: Action[] = [],
- links: Link[] = []
+ links: Link[] = [],
+ patchset: ChecksPatchset
) {
const attemptMap = createAttemptMap(runs);
for (const attemptInfo of attemptMap.values()) {
@@ -537,9 +677,9 @@
attemptInfo.attempts.sort((a, b) => (b.attempt ?? -1) - (a.attempt ?? -1));
}
const nextState = {...privateState$.getValue()};
- nextState.providerNameToState = {...nextState.providerNameToState};
- nextState.providerNameToState[pluginName] = {
- ...nextState.providerNameToState[pluginName],
+ const pluginState = getPluginState(nextState, patchset);
+ pluginState[pluginName] = {
+ ...pluginState[pluginName],
loading: false,
errorMessage: undefined,
loginCallback: undefined,
@@ -569,6 +709,6 @@
export function updateStateSetPatchset(patchsetNumber?: PatchSetNumber) {
const nextState = {...privateState$.getValue()};
- nextState.patchsetNumber = patchsetNumber;
+ nextState.patchsetNumberSelected = patchsetNumber;
privateState$.next(nextState);
}
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts
index 48d61e2..e1d435b 100644
--- a/polygerrit-ui/app/services/checks/checks-service.ts
+++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -32,7 +32,8 @@
} from '../../api/checks';
import {change$, changeNum$, latestPatchNum$} from '../change/change-model';
import {
- checksPatchsetNumber$,
+ ChecksPatchset,
+ checksSelectedPatchsetNumber$,
checkToPluginMap$,
updateStateSetError,
updateStateSetLoading,
@@ -55,6 +56,7 @@
import {getShaByPatchNum} from '../../utils/patch-set-util';
import {assertIsDefined} from '../../utils/common-util';
import {ReportingService} from '../gr-reporting/gr-reporting';
+import {routerPatchNum$} from '../router/router-model';
export class ChecksService {
private readonly providers: {[name: string]: ChecksProvider} = {};
@@ -63,15 +65,26 @@
private checkToPluginMap = new Map<string, string>();
+ private latestPatchNum?: PatchSetNumber;
+
private readonly documentVisibilityChange$ = new BehaviorSubject(undefined);
constructor(readonly reporting: ReportingService) {
checkToPluginMap$.subscribe(map => {
this.checkToPluginMap = map;
});
- latestPatchNum$.subscribe(num => {
- updateStateSetPatchset(num);
- });
+ combineLatest([routerPatchNum$, latestPatchNum$]).subscribe(
+ ([routerPs, latestPs]) => {
+ this.latestPatchNum = latestPs;
+ if (latestPs === undefined) {
+ this.setPatchset(undefined);
+ } else if (typeof routerPs === 'number') {
+ this.setPatchset(routerPs);
+ } else {
+ this.setPatchset(latestPs);
+ }
+ }
+ );
document.addEventListener('visibilitychange', () => {
this.documentVisibilityChange$.next(undefined);
});
@@ -80,8 +93,8 @@
});
}
- setPatchset(num: PatchSetNumber) {
- updateStateSetPatchset(num);
+ setPatchset(num?: PatchSetNumber) {
+ updateStateSetPatchset(num === this.latestPatchNum ? undefined : num);
}
reload(pluginName: string) {
@@ -105,7 +118,16 @@
) {
this.providers[pluginName] = provider;
this.reloadSubjects[pluginName] = new BehaviorSubject<void>(undefined);
- updateStateSetProvider(pluginName, config);
+ updateStateSetProvider(pluginName);
+ this.initFetchingOfData(pluginName, config, ChecksPatchset.LATEST);
+ this.initFetchingOfData(pluginName, config, ChecksPatchset.SELECTED);
+ }
+
+ initFetchingOfData(
+ pluginName: string,
+ config: ChecksApiConfig,
+ patchset: ChecksPatchset
+ ) {
const pollIntervalMs = (config?.fetchPollingIntervalSeconds ?? 60) * 1000;
// Various events should trigger fetching checks from the provider:
// 1. Change number and patchset number changes.
@@ -114,7 +136,9 @@
// 4. A hidden Gerrit tab becoming visible.
combineLatest([
changeNum$,
- checksPatchsetNumber$,
+ patchset === ChecksPatchset.LATEST
+ ? latestPatchNum$
+ : checksSelectedPatchsetNumber$,
this.reloadSubjects[pluginName].pipe(throttleTime(1000)),
timer(0, pollIntervalMs),
this.documentVisibilityChange$,
@@ -125,27 +149,13 @@
withLatestFrom(change$),
switchMap(
([[changeNum, patchNum], change]): Observable<FetchResponse> => {
- if (
- !change ||
- !changeNum ||
- !patchNum ||
- typeof patchNum !== 'number'
- ) {
- return of({
- responseCode: ResponseCode.OK,
- runs: [],
- });
- }
+ if (!change || !changeNum || !patchNum) return of(this.empty());
+ if (typeof patchNum !== 'number') return of(this.empty());
assertIsDefined(change.revisions, 'change.revisions');
const patchsetSha = getShaByPatchNum(change.revisions, patchNum);
// Sometimes patchNum is updated earlier than change, so change
// revisions don't have patchNum yet
- if (!patchsetSha) {
- return of({
- responseCode: ResponseCode.OK,
- runs: [],
- });
- }
+ if (!patchsetSha) return of(this.empty());
const data: ChangeData = {
changeNumber: changeNum,
patchsetNumber: patchNum,
@@ -154,7 +164,7 @@
commmitMessage: getCurrentRevision(change)?.commit?.message,
changeInfo: change,
};
- return this.fetchResults(pluginName, data);
+ return this.fetchResults(pluginName, data, patchset);
}
),
catchError(e => {
@@ -169,24 +179,36 @@
switch (response.responseCode) {
case ResponseCode.ERROR:
assertIsDefined(response.errorMessage, 'errorMessage');
- updateStateSetError(pluginName, response.errorMessage);
+ updateStateSetError(pluginName, response.errorMessage, patchset);
break;
case ResponseCode.NOT_LOGGED_IN:
assertIsDefined(response.loginCallback, 'loginCallback');
- updateStateSetNotLoggedIn(pluginName, response.loginCallback);
+ updateStateSetNotLoggedIn(
+ pluginName,
+ response.loginCallback,
+ patchset
+ );
break;
case ResponseCode.OK:
updateStateSetResults(
pluginName,
response.runs ?? [],
response.actions ?? [],
- response.links ?? []
+ response.links ?? [],
+ patchset
);
break;
}
});
}
+ private empty(): FetchResponse {
+ return {
+ responseCode: ResponseCode.OK,
+ runs: [],
+ };
+ }
+
private createErrorResponse(
pluginName: string,
message: string
@@ -199,9 +221,10 @@
private fetchResults(
pluginName: string,
- data: ChangeData
+ data: ChangeData,
+ patchset: ChecksPatchset
): Observable<FetchResponse> {
- updateStateSetLoading(pluginName);
+ updateStateSetLoading(pluginName, patchset);
const timer = this.reporting.getTimer('ChecksPluginFetch');
const fetchPromise = this.providers[pluginName]
.fetch(data)
diff --git a/polygerrit-ui/app/services/checks/checks-util.ts b/polygerrit-ui/app/services/checks/checks-util.ts
index f08732f..9381f30 100644
--- a/polygerrit-ui/app/services/checks/checks-util.ts
+++ b/polygerrit-ui/app/services/checks/checks-util.ts
@@ -43,6 +43,8 @@
return 'help-outline';
case LinkIcon.REPORT_BUG:
return 'bug';
+ case LinkIcon.CODE:
+ return 'code';
default:
// We don't throw an assertion error here, because plugins don't have to
// be written in TypeScript, so we may encounter arbitrary strings for
@@ -312,14 +314,19 @@
};
}
-export function primaryLink(result?: CheckResultApi): Link | undefined {
- const links = result?.links ?? [];
- return links.find(link => link.primary);
+function allPrimaryLinks(result?: CheckResultApi): Link[] {
+ return (result?.links ?? []).filter(link => link.primary);
}
-export function otherLinks(result?: CheckResultApi): Link[] {
- const primary = primaryLink(result);
- const links = result?.links ?? [];
- // Just filter the one primary link, not all primary links.
- return links.filter(link => link !== primary);
+export function firstPrimaryLink(result?: CheckResultApi): Link | undefined {
+ return allPrimaryLinks(result).find(link => link.icon === LinkIcon.EXTERNAL);
+}
+
+export function otherPrimaryLinks(result?: CheckResultApi): Link[] {
+ const first = firstPrimaryLink(result);
+ return allPrimaryLinks(result).filter(link => link !== first);
+}
+
+export function secondaryLinks(result?: CheckResultApi): Link[] {
+ return (result?.links ?? []).filter(link => !link.primary);
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 3a890d9..cd3fab3 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -534,6 +534,7 @@
invalidateGroupsCache(): void;
invalidateReposCache(): void;
invalidateAccountsCache(): void;
+ invalidateAccountsDetailCache(): void;
removeFromAttentionSet(
changeNum: NumericChangeId,
user: AccountId,
diff --git a/polygerrit-ui/app/styles/dashboard-header-styles.ts b/polygerrit-ui/app/styles/dashboard-header-styles.ts
index 2354f65..0c9f38d 100644
--- a/polygerrit-ui/app/styles/dashboard-header-styles.ts
+++ b/polygerrit-ui/app/styles/dashboard-header-styles.ts
@@ -55,9 +55,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-change-list-styles.ts b/polygerrit-ui/app/styles/gr-change-list-styles.ts
index d1fcdc9..e0a7a28 100644
--- a/polygerrit-ui/app/styles/gr-change-list-styles.ts
+++ b/polygerrit-ui/app/styles/gr-change-list-styles.ts
@@ -202,9 +202,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-change-metadata-shared-styles.ts b/polygerrit-ui/app/styles/gr-change-metadata-shared-styles.ts
index 3d07d2e..67a6963 100644
--- a/polygerrit-ui/app/styles/gr-change-metadata-shared-styles.ts
+++ b/polygerrit-ui/app/styles/gr-change-metadata-shared-styles.ts
@@ -55,9 +55,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.ts b/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.ts
index 57c8d78..145f0d5 100644
--- a/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.ts
+++ b/polygerrit-ui/app/styles/gr-change-view-integration-shared-styles.ts
@@ -22,6 +22,15 @@
const $_documentContainer = document.createElement('template');
+/*
+ These are shared styles for change-view-integration endpoints.
+ All plugins that registered that endpoint should include this in
+ the component to have a consistent UX:
+
+ <style include="gr-change-view-integration-shared-styles"></style>
+
+ And use those defined class to apply these styles.
+*/
$_documentContainer.innerHTML = `<dom-module id="gr-change-view-integration-shared-styles">
<template>
<style include="shared-styles">
@@ -61,18 +70,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- This is shared styles for change-view-integration endpoints.
- All plugins that registered that endpoint should include this in
- the component to have a consistent UX:
-
- <style include="gr-change-view-integration-shared-styles"></style>
-
- And use those defined class to apply these styles.
-*/
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-form-styles.ts b/polygerrit-ui/app/styles/gr-form-styles.ts
index 3284ad5..f58a02c 100644
--- a/polygerrit-ui/app/styles/gr-form-styles.ts
+++ b/polygerrit-ui/app/styles/gr-form-styles.ts
@@ -124,9 +124,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-menu-page-styles.ts b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
index e9a79c1f..d46f136 100644
--- a/polygerrit-ui/app/styles/gr-menu-page-styles.ts
+++ b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
@@ -79,9 +79,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-page-nav-styles.ts b/polygerrit-ui/app/styles/gr-page-nav-styles.ts
index 9010b2d..8c29b85 100644
--- a/polygerrit-ui/app/styles/gr-page-nav-styles.ts
+++ b/polygerrit-ui/app/styles/gr-page-nav-styles.ts
@@ -72,9 +72,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-subpage-styles.ts b/polygerrit-ui/app/styles/gr-subpage-styles.ts
index 41ee952..5aab0dc 100644
--- a/polygerrit-ui/app/styles/gr-subpage-styles.ts
+++ b/polygerrit-ui/app/styles/gr-subpage-styles.ts
@@ -42,9 +42,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-table-styles.ts b/polygerrit-ui/app/styles/gr-table-styles.ts
index 52fdc67..09d1161 100644
--- a/polygerrit-ui/app/styles/gr-table-styles.ts
+++ b/polygerrit-ui/app/styles/gr-table-styles.ts
@@ -116,9 +116,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/gr-voting-styles.ts b/polygerrit-ui/app/styles/gr-voting-styles.ts
index b50aee6..cb8b0be8 100644
--- a/polygerrit-ui/app/styles/gr-voting-styles.ts
+++ b/polygerrit-ui/app/styles/gr-voting-styles.ts
@@ -48,9 +48,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/styles/shared-styles.ts b/polygerrit-ui/app/styles/shared-styles.ts
index 1bee660..287cf68 100644
--- a/polygerrit-ui/app/styles/shared-styles.ts
+++ b/polygerrit-ui/app/styles/shared-styles.ts
@@ -304,9 +304,3 @@
</dom-module>`;
document.head.appendChild($_documentContainer.content);
-
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
index 3971fa5..a3df94c 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -411,6 +411,7 @@
invalidateAccountsCache(): void {},
invalidateGroupsCache(): void {},
invalidateReposCache(): void {},
+ invalidateAccountsDetailCache(): void {},
probePath(): Promise<boolean> {
return Promise.resolve(true);
},
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index dd91f7b..414a80c 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -24,6 +24,7 @@
RelatedChangeAndCommitInfo,
} from '../types/common';
import {ParsedChangeInfo} from '../types/types';
+import {ChangeStates} from '../elements/shared/gr-change-status/gr-change-status';
// This can be wrong! See WARNING above
interface ChangeStatusesOptions {
@@ -134,28 +135,27 @@
return change?.status === ChangeStatus.NEW;
}
-// TODO(TS): use enum ChangeStates in gr-change-status
export function changeStatuses(
change: ChangeInfo,
opt_options?: ChangeStatusesOptions
-) {
+): ChangeStates[] {
const states = [];
if (change.status === ChangeStatus.MERGED) {
- states.push('Merged');
+ states.push(ChangeStates.MERGED);
} else if (change.status === ChangeStatus.ABANDONED) {
- states.push('Abandoned');
+ states.push(ChangeStates.ABANDONED);
} else if (
change.mergeable === false ||
(opt_options && opt_options.mergeable === false)
) {
// 'mergeable' prop may not always exist (@see Issue 6819)
- states.push('Merge Conflict');
+ states.push(ChangeStates.MERGE_CONFLICT);
}
if (change.work_in_progress) {
- states.push('WIP');
+ states.push(ChangeStates.WIP);
}
if (change.is_private) {
- states.push('Private');
+ states.push(ChangeStates.PRIVATE);
}
// If there are any pre-defined statuses, only return those. Otherwise,
@@ -166,10 +166,10 @@
// If no missing requirements, either active or ready to submit.
if (change.submittable && opt_options.submitEnabled) {
- states.push('Ready to submit');
+ states.push(ChangeStates.READY_TO_SUBMIT);
} else {
// Otherwise it is active.
- states.push('Active');
+ states.push(ChangeStates.ACTIVE);
}
return states;
}
diff --git a/polygerrit-ui/app/utils/change-util_test.js b/polygerrit-ui/app/utils/change-util_test.js
deleted file mode 100644
index f348239..0000000
--- a/polygerrit-ui/app/utils/change-util_test.js
+++ /dev/null
@@ -1,202 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../test/common-test-setup-karma.js';
-import {
- changeBaseURL,
- changePath,
- changeStatuses,
- isRemovableReviewer,
-} from './change-util.js';
-
-suite('change-util tests', () => {
- let originalCanonicalPath;
-
- suiteSetup(() => {
- originalCanonicalPath = window.CANONICAL_PATH;
- window.CANONICAL_PATH = '/r';
- });
-
- suiteTeardown(() => {
- window.CANONICAL_PATH = originalCanonicalPath;
- });
-
- test('changeBaseURL', () => {
- assert.deepEqual(
- changeBaseURL('test/project', '1', '2'),
- '/r/changes/test%2Fproject~1/revisions/2'
- );
- });
-
- test('changePath', () => {
- assert.deepEqual(changePath('1'), '/r/c/1');
- });
-
- test('Open status', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'NEW',
- labels: {},
- mergeable: true,
- };
- let statuses = changeStatuses(change);
- assert.deepEqual(statuses, []);
-
- change.submittable = false;
- statuses = changeStatuses(change,
- {includeDerived: true});
- assert.deepEqual(statuses, ['Active']);
-
- // With no missing labels but no submitEnabled option.
- change.submittable = true;
- statuses = changeStatuses(change,
- {includeDerived: true});
- assert.deepEqual(statuses, ['Active']);
-
- // Without missing labels and enabled submit
- statuses = changeStatuses(change,
- {includeDerived: true, submitEnabled: true});
- assert.deepEqual(statuses, ['Ready to submit']);
-
- change.mergeable = false;
- change.submittable = true;
- statuses = changeStatuses(change,
- {includeDerived: true});
- assert.deepEqual(statuses, ['Merge Conflict']);
-
- delete change.mergeable;
- change.submittable = true;
- statuses = changeStatuses(change,
- {includeDerived: true, mergeable: true, submitEnabled: true});
- assert.deepEqual(statuses, ['Ready to submit']);
-
- change.submittable = true;
- statuses = changeStatuses(change,
- {includeDerived: true, mergeable: false});
- assert.deepEqual(statuses, ['Merge Conflict']);
- });
-
- test('Merge conflict', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'NEW',
- labels: {},
- mergeable: false,
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, ['Merge Conflict']);
- });
-
- test('mergeable prop undefined', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'NEW',
- labels: {},
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, []);
- });
-
- test('Merged status', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'MERGED',
- labels: {},
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, ['Merged']);
- });
-
- test('Abandoned status', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'ABANDONED',
- labels: {},
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, ['Abandoned']);
- });
-
- test('Open status with private and wip', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'NEW',
- is_private: true,
- work_in_progress: true,
- labels: {},
- mergeable: true,
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, ['WIP', 'Private']);
- });
-
- test('Merge conflict with private and wip', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev1: {_number: 1},
- },
- current_revision: 'rev1',
- status: 'NEW',
- is_private: true,
- work_in_progress: true,
- labels: {},
- mergeable: false,
- };
- const statuses = changeStatuses(change);
- assert.deepEqual(statuses, ['Merge Conflict', 'WIP', 'Private']);
- });
-
- test('isRemovableReviewer', () => {
- let change = {
- removable_reviewers: [{_account_id: 1}],
- };
- const reviewer = {_account_id: 1};
-
- assert.equal(isRemovableReviewer(change, reviewer), true);
-
- change = {
- removable_reviewers: [{_account_id: 2}],
- };
- assert.equal(isRemovableReviewer(change, reviewer), false);
- });
-});
-
diff --git a/polygerrit-ui/app/utils/change-util_test.ts b/polygerrit-ui/app/utils/change-util_test.ts
new file mode 100644
index 0000000..8232e56
--- /dev/null
+++ b/polygerrit-ui/app/utils/change-util_test.ts
@@ -0,0 +1,198 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {ChangeStatus} from '../constants/constants';
+import {ChangeStates} from '../elements/shared/gr-change-status/gr-change-status';
+import '../test/common-test-setup-karma';
+import {createChange, createRevisions} from '../test/test-data-generators';
+import {
+ AccountId,
+ CommitId,
+ NumericChangeId,
+ PatchSetNum,
+} from '../types/common';
+import {
+ changeBaseURL,
+ changePath,
+ changeStatuses,
+ isRemovableReviewer,
+} from './change-util';
+
+suite('change-util tests', () => {
+ let originalCanonicalPath: string | undefined;
+
+ suiteSetup(() => {
+ originalCanonicalPath = window.CANONICAL_PATH;
+ window.CANONICAL_PATH = '/r';
+ });
+
+ suiteTeardown(() => {
+ window.CANONICAL_PATH = originalCanonicalPath;
+ });
+
+ test('changeBaseURL', () => {
+ assert.deepEqual(
+ changeBaseURL('test/project', 1 as NumericChangeId, '2' as PatchSetNum),
+ '/r/changes/test%2Fproject~1/revisions/2'
+ );
+ });
+
+ test('changePath', () => {
+ assert.deepEqual(changePath(1 as NumericChangeId), '/r/c/1');
+ });
+
+ test('Open status', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ mergeable: true,
+ };
+ let statuses = changeStatuses(change);
+ assert.deepEqual(statuses, []);
+
+ change.submittable = false;
+ statuses = changeStatuses(change, {mergeable: true, submitEnabled: false});
+ assert.deepEqual(statuses, [ChangeStates.ACTIVE]);
+
+ // With no missing labels but no submitEnabled option.
+ change.submittable = true;
+ statuses = changeStatuses(change, {mergeable: true, submitEnabled: false});
+ assert.deepEqual(statuses, [ChangeStates.ACTIVE]);
+
+ // Without missing labels and enabled submit
+ statuses = changeStatuses(change, {mergeable: true, submitEnabled: true});
+ assert.deepEqual(statuses, [ChangeStates.READY_TO_SUBMIT]);
+
+ change.mergeable = false;
+ change.submittable = true;
+ statuses = changeStatuses(change, {mergeable: false, submitEnabled: false});
+ assert.deepEqual(statuses, [ChangeStates.MERGE_CONFLICT]);
+
+ change.mergeable = true;
+ statuses = changeStatuses(change, {mergeable: true, submitEnabled: true});
+ assert.deepEqual(statuses, [ChangeStates.READY_TO_SUBMIT]);
+
+ change.submittable = true;
+ statuses = changeStatuses(change, {mergeable: false, submitEnabled: false});
+ assert.deepEqual(statuses, [ChangeStates.MERGE_CONFLICT]);
+ });
+
+ test('Merge conflict', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ mergeable: false,
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, [ChangeStates.MERGE_CONFLICT]);
+ });
+
+ test('mergeable prop undefined', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, []);
+ });
+
+ test('Merged status', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.MERGED,
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, [ChangeStates.MERGED]);
+ });
+
+ test('Abandoned status', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.ABANDONED,
+ mergeable: false,
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, [ChangeStates.ABANDONED]);
+ });
+
+ test('Open status with private and wip', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ mergeable: true,
+ is_private: true,
+ work_in_progress: true,
+ labels: {},
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, [ChangeStates.WIP, ChangeStates.PRIVATE]);
+ });
+
+ test('Merge conflict with private and wip', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ mergeable: false,
+ is_private: true,
+ work_in_progress: true,
+ labels: {},
+ };
+ const statuses = changeStatuses(change);
+ assert.deepEqual(statuses, [
+ ChangeStates.MERGE_CONFLICT,
+ ChangeStates.WIP,
+ ChangeStates.PRIVATE,
+ ]);
+ });
+
+ test('isRemovableReviewer', () => {
+ let change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ mergeable: false,
+ removable_reviewers: [{_account_id: 1 as AccountId}],
+ };
+ const reviewer = {_account_id: 1 as AccountId};
+
+ assert.equal(isRemovableReviewer(change, reviewer), true);
+
+ change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.NEW,
+ mergeable: false,
+ removable_reviewers: [{_account_id: 2 as AccountId}],
+ };
+ assert.equal(isRemovableReviewer(change, reviewer), false);
+ });
+});
diff --git a/proto/cache.proto b/proto/cache.proto
index b1722b4..4efce6f 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -445,7 +445,7 @@
}
// Serialized form of com.google.gerrit.common.data.LabelType.
-// Next ID: 19
+// Next ID: 21
message LabelTypeProto {
string name = 1;
string function = 2; // ENUM as String
@@ -466,6 +466,7 @@
bool can_override = 17;
repeated string ref_patterns = 18;
bool copy_all_scores_if_list_of_files_did_not_change = 19;
+ string copy_condition = 20;
}
// Serialized form of com.google.gerrit.entities.SubmitRequirement.
@@ -685,3 +686,18 @@
bytes new_commit = 10;
ComparisonType comparison_type = 11;
}
+
+// Serialized form of com.google.gerrit.server.approval.ApprovalCacheImpl.Key.
+// Next ID: 5
+message PatchSetApprovalsKeyProto {
+ string project = 1;
+ int32 change_id = 2;
+ int32 patch_set_id = 3;
+ bytes id = 4;
+}
+
+// Repeated version of PatchSetApprovalProto
+// Next ID: 2
+message AllPatchSetApprovalsProto {
+ repeated devtools.gerritcodereview.PatchSetApproval approval = 1;
+}
diff --git a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
index 3a40d22..e1d6f22 100755
--- a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
+++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
@@ -16,6 +16,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+set -u
+
# avoid [[ which is not POSIX sh.
if test "$#" != 1 ; then
echo "$0 requires an argument."
@@ -28,12 +30,17 @@
fi
# Do not create a change id if requested
-if test "false" = "`git config --bool --get gerrit.createChangeId`" ; then
+if test "false" = "$(git config --bool --get gerrit.createChangeId)" ; then
exit 0
fi
-# $RANDOM will be undefined if not using bash, so don't use set -u
-random=$( (whoami ; hostname ; date; cat $1 ; echo $RANDOM) | git hash-object --stdin)
+if git rev-parse --verify HEAD >/dev/null 2>&1; then
+ refhash="$(git rev-parse HEAD)"
+else
+ refhash="$(git hash-object -t tree /dev/null)"
+fi
+
+random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } | git hash-object --stdin)
dest="$1.tmp.${random}"
trap 'rm -f "${dest}"' EXIT
