Merge "Introduce deep-equal and migrate code to use it."
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 4499f97..694e5d8 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -49,10 +49,6 @@
returned results. Search can also be performed by typing only a
text with no operator, which will match against a variety of fields.
-Operators can escape characters, for example double quotes, by enclosing the
-value with double quotes and escaping double quotes inside the value with
-a backslash.
-
[[age]]
age:'AGE'::
+
diff --git a/antlr3/com/google/gerrit/index/query/Query.g b/antlr3/com/google/gerrit/index/query/Query.g
index 75c75c7..1bf20aa 100644
--- a/antlr3/com/google/gerrit/index/query/Query.g
+++ b/antlr3/com/google/gerrit/index/query/Query.g
@@ -157,12 +157,13 @@
;
EXACT_PHRASE
-@init { final StringBuilder buf = new StringBuilder(); }
- : '"' ( ESCAPE[buf] | i = ~('\\'|'"') { buf.appendCodePoint(i); } )* '"' {
- setText(buf.toString());
+ : '"' ( ~('"') )* '"' {
+ String s = $text;
+ setText(s.substring(1, s.length() - 1));
}
- | '{' ( ESCAPE[buf] | i = ~('\\'|'{'|'}') { buf.appendCodePoint(i); } )* '}' {
- setText(buf.toString());
+ | '{' ( ~('{'|'}') )* '}' {
+ String s = $text;
+ setText(s.substring(1, s.length() - 1));
}
;
@@ -196,11 +197,3 @@
// | '~' permit
)
;
-
-fragment ESCAPE[StringBuilder buf] :
- '\\'
- ( 't' { buf.append('\t'); }
- | 'n' { buf.append('\n'); }
- | 'r' { buf.append('\r'); }
- | i = (~('t'|'n'|'r')) { buf.appendCodePoint(i); }
- );
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index ee9fd77..b24fdad 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -81,7 +81,6 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
-import com.google.gerrit.server.query.FileEditsPredicate;
import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
@@ -207,8 +206,6 @@
modules.add(new DefaultSubmitRuleModule());
modules.add(new IgnoreSelfApprovalRuleModule());
- factory(FileEditsPredicate.Factory.class);
-
bind(ChangeJson.Factory.class).toProvider(Providers.of(null));
bind(EventUtil.class).toProvider(Providers.of(null));
bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 1647c5c..24882cb 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -190,7 +190,6 @@
import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
-import com.google.gerrit.server.query.FileEditsPredicate;
import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
@@ -490,7 +489,6 @@
factory(GitModules.Factory.class);
factory(VersionedAuthorizedKeys.Factory.class);
factory(StoreSubmitRequirementsOp.Factory.class);
- factory(FileEditsPredicate.Factory.class);
bind(AccountManager.class);
bind(SubscriptionGraph.Factory.class).to(ConfiguredSubscriptionGraphFactory.class);
diff --git a/java/com/google/gerrit/server/query/FileEditsPredicate.java b/java/com/google/gerrit/server/query/FileEditsPredicate.java
deleted file mode 100644
index e874929..0000000
--- a/java/com/google/gerrit/server/query/FileEditsPredicate.java
+++ /dev/null
@@ -1,197 +0,0 @@
-// 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;
-
-import com.google.auto.value.AutoValue;
-import com.google.common.collect.Iterables;
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Patch;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.patch.DiffNotAvailableException;
-import com.google.gerrit.server.patch.DiffOperations;
-import com.google.gerrit.server.patch.DiffOptions;
-import com.google.gerrit.server.patch.FilePathAdapter;
-import com.google.gerrit.server.patch.Text;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.gerrit.server.patch.filediff.TaggedEdit;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
-import com.google.inject.assistedinject.Assisted;
-import com.google.inject.assistedinject.AssistedInject;
-import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-import java.util.regex.Pattern;
-import java.util.stream.Collectors;
-import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevTree;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.TreeWalk;
-
-/**
- * A submit-requirement predicate that can be used in submit requirements expressions. This
- * predicate is fulfilled if the diff between the latest patchset of the change and the base commit
- * includes a specific file path pattern with some specific content modification. The modification
- * could be an added, deleted or replaced content.
- */
-public class FileEditsPredicate extends SubmitRequirementPredicate {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private DiffOperations diffOperations;
- private GitRepositoryManager repoManager;
- private final FileEditsArgs fileEditsArgs;
-
- public interface Factory {
- FileEditsPredicate create(FileEditsArgs fileEditsArgs);
- }
-
- @AutoValue
- public abstract static class FileEditsArgs {
- abstract String filePattern();
-
- abstract String editPattern();
-
- public static FileEditsArgs create(String filePattern, String contentPattern) {
- return new AutoValue_FileEditsPredicate_FileEditsArgs(filePattern, contentPattern);
- }
- }
-
- @AssistedInject
- public FileEditsPredicate(
- DiffOperations diffOperations,
- GitRepositoryManager repoManager,
- @Assisted FileEditsPredicate.FileEditsArgs fileEditsArgs) {
- super("fileEdits", fileEditsArgs.filePattern() + "," + fileEditsArgs.editPattern());
- this.diffOperations = diffOperations;
- this.repoManager = repoManager;
- this.fileEditsArgs = fileEditsArgs;
- }
-
- @Override
- public boolean match(ChangeData cd) {
- try {
- Map<String, FileDiffOutput> modifiedFiles =
- diffOperations.listModifiedFilesAgainstParent(
- cd.project(),
- cd.currentPatchSet().commitId(),
- /* parentNum= */ 0,
- DiffOptions.DEFAULTS);
- FileDiffOutput firstDiff =
- Iterables.getFirst(modifiedFiles.values(), /* defaultValue= */ null);
- if (firstDiff == null) {
- // No available diffs. We cannot identify old and new commit IDs.
- // engine.fail();
- return false;
- }
-
- Pattern filePattern = null;
- Pattern editPattern = null;
- if (fileEditsArgs.filePattern().startsWith("^")) {
- // We validated the pattern before creating this predicate. No need to revalidate.
- String pattern = fileEditsArgs.filePattern();
- filePattern = Pattern.compile(pattern);
- }
- if (fileEditsArgs.editPattern().startsWith("^")) {
- // We validated the pattern before creating this predicate. No need to revalidate.
- String pattern = fileEditsArgs.editPattern();
- editPattern = Pattern.compile(pattern);
- }
- try (Repository repo = repoManager.openRepository(cd.project());
- ObjectReader reader = repo.newObjectReader();
- RevWalk rw = new RevWalk(reader)) {
- RevTree aTree =
- firstDiff.oldCommitId().equals(ObjectId.zeroId())
- ? null
- : rw.parseTree(firstDiff.oldCommitId());
- RevTree bTree = rw.parseCommit(firstDiff.newCommitId()).getTree();
-
- for (FileDiffOutput entry : modifiedFiles.values()) {
- String newName =
- FilePathAdapter.getNewPath(entry.oldPath(), entry.newPath(), entry.changeType());
- String oldName = FilePathAdapter.getOldPath(entry.oldPath(), entry.changeType());
-
- if (Patch.isMagic(newName)) {
- continue;
- }
-
- if (match(newName, fileEditsArgs.filePattern(), filePattern)
- || (oldName != null && match(oldName, fileEditsArgs.filePattern(), filePattern))) {
- List<Edit> edits =
- entry.edits().stream().map(TaggedEdit::jgitEdit).collect(Collectors.toList());
- if (edits.isEmpty()) {
- continue;
- }
- Text tA;
- if (oldName != null) {
- tA = load(aTree, oldName, reader);
- } else {
- tA = load(aTree, newName, reader);
- }
- Text tB = load(bTree, newName, reader);
- for (Edit edit : edits) {
- if (tA != Text.EMPTY) {
- String aDiff = tA.getString(edit.getBeginA(), edit.getEndA(), true);
- if (match(aDiff, fileEditsArgs.editPattern(), editPattern)) {
- return true;
- }
- }
- if (tB != Text.EMPTY) {
- String bDiff = tB.getString(edit.getBeginB(), edit.getEndB(), true);
- if (match(bDiff, fileEditsArgs.editPattern(), editPattern)) {
- return true;
- }
- }
- }
- }
- }
- } catch (IOException e) {
- logger.atSevere().log("Error while evaluating commit edits.", e);
- return false;
- }
- } catch (DiffNotAvailableException e) {
- logger.atSevere().log("Diff error while evaluating commit edits.", e);
- return false;
- }
- return false;
- }
-
- @Override
- public int getCost() {
- return 10;
- }
-
- private Text load(@Nullable ObjectId tree, String path, ObjectReader reader) throws IOException {
- if (tree == null || path == null) {
- return Text.EMPTY;
- }
- final TreeWalk tw = TreeWalk.forPath(reader, path, tree);
- if (tw == null) {
- return Text.EMPTY;
- }
- if (tw.getFileMode(0).getObjectType() != Constants.OBJ_BLOB) {
- return Text.EMPTY;
- }
- return new Text(reader.open(tw.getObjectId(0), Constants.OBJ_BLOB));
- }
-
- private boolean match(String text, String search, @Nullable Pattern searchPattern) {
- return searchPattern == null ? text.contains(search) : searchPattern.matcher(text).find();
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 759b707..65751b6 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -890,12 +890,12 @@
}
@Operator
- public Predicate<ChangeData> f(String file) throws QueryParseException {
+ public Predicate<ChangeData> f(String file) {
return file(file);
}
@Operator
- public Predicate<ChangeData> file(String file) throws QueryParseException {
+ public Predicate<ChangeData> file(String file) {
if (file.startsWith("^")) {
return new RegexPathPredicate(file);
}
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index a403d55..cc24dd04 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -18,12 +18,7 @@
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.query.FileEditsPredicate;
-import com.google.gerrit.server.query.FileEditsPredicate.FileEditsArgs;
import com.google.inject.Inject;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
/**
* A query builder for submit requirement expressions that includes all {@link ChangeQueryBuilder}
@@ -36,23 +31,9 @@
private static final QueryBuilder.Definition<ChangeData, ChangeQueryBuilder> def =
new QueryBuilder.Definition<>(SubmitRequirementChangeQueryBuilder.class);
- /**
- * Regular expression for the {@link #file(String)} operator. Field value is of the form:
- *
- * <p>'$fileRegex',withDiffContaining='$contentRegex'
- *
- * <p>Both $fileRegex and $contentRegex may contain escaped single or double quotes.
- */
- private static final Pattern FILE_EDITS_PATTERN =
- Pattern.compile("'((?:(?:\\\\')|(?:[^']))*)',withDiffContaining='((?:(?:\\\\')|(?:[^']))*)'");
-
- private final FileEditsPredicate.Factory fileEditsPredicateFactory;
-
@Inject
- SubmitRequirementChangeQueryBuilder(
- Arguments args, FileEditsPredicate.Factory fileEditsPredicateFactory) {
+ SubmitRequirementChangeQueryBuilder(Arguments args) {
super(def, args);
- this.fileEditsPredicateFactory = fileEditsPredicateFactory;
}
@Override
@@ -68,44 +49,4 @@
}
return super.is(value);
}
-
- /**
- * A SR operator that can match with file path and content pattern. The value should be of the
- * form:
- *
- * <p>file:"'$filePattern',withDiffContaining='$contentPattern'"
- *
- * <p>The operator matches with changes that have their latest PS vs. base diff containing a file
- * path matching the {@code filePattern} with an edit (added, deleted, modified) matching the
- * {@code contentPattern}. {@code filePattern} and {@code contentPattern} can start with "^" to
- * use regular expression matching.
- *
- * <p>If the specified value does not match this form, we fall back to the operator's
- * implementation in {@link ChangeQueryBuilder}.
- */
- @Override
- public Predicate<ChangeData> file(String value) throws QueryParseException {
- Matcher matcher = FILE_EDITS_PATTERN.matcher(value);
- if (!matcher.find()) {
- return super.file(value);
- }
- String filePattern = matcher.group(1);
- String contentPattern = matcher.group(2);
- if (filePattern.startsWith("^")) {
- validateRegularExpression(filePattern, "Invalid file pattern.");
- }
- if (contentPattern.startsWith("^")) {
- validateRegularExpression(contentPattern, "Invalid content pattern.");
- }
- return fileEditsPredicateFactory.create(FileEditsArgs.create(filePattern, contentPattern));
- }
-
- private static void validateRegularExpression(String pattern, String errorMessage)
- throws QueryParseException {
- try {
- Pattern.compile(pattern);
- } catch (PatternSyntaxException e) {
- throw new QueryParseException(errorMessage, e);
- }
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index ce11202..a23fb7b 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -23,7 +23,6 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
@@ -50,18 +49,14 @@
@Inject SubmitRequirementsEvaluator evaluator;
@Inject private ProjectOperations projectOperations;
@Inject private Provider<InternalChangeQuery> changeQueryProvider;
- @Inject private ChangeOperations changeOperations;
private ChangeData changeData;
private String changeId;
- private static final String FILE_NAME = "file.txt";
- private static final String CONTENT = "line 1\nline 2\nline 3\n";
-
@Before
public void setUp() throws Exception {
PushOneCommit.Result pushResult =
- createChange(testRepo, "refs/heads/master", "Fix a bug", FILE_NAME, CONTENT, "topic");
+ createChange(testRepo, "refs/heads/master", "Fix a bug", "file.txt", "content", "topic");
changeData = pushResult.getChange();
changeId = pushResult.getChangeId();
}
@@ -273,361 +268,6 @@
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
}
- @Test
- public void byFileEdits_deletedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 2\n", ""))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='line 2'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_deletedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 1\n", ""))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='line 2'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_addedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT + "line 4\n")
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='line 4'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_addedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT + "line 4\n")
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='line 5'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_addedFile_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file("new_file.txt")
- .content("content of the new file")
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^new.*\\\\.txt',withDiffContaining='of the new'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_addedFile_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file("new_file.txt")
- .content("content of the new file")
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^new.*\\.txt',withDiffContaining='not_exist'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_modifiedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='three'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_modifiedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.txt',withDiffContaining='ten'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_modifiedContentPattern_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^.*\\.txt',withDiffContaining='^.*th[rR]ee$'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_exactMatchingWithFilePath_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- String.format("file:\"'%s',withDiffContaining='three'\"", FILE_NAME));
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_exactMatchingWithFilePath_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'non_existent.txt',withDiffContaining='three'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_notMatchingWithFilePath() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- // commit edit only matches with files ending with ".java". Since our modified file name ends
- // with ".txt", the applicability expression will not match.
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^.*\\.java',withDiffContaining='three'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_escapeSingleQuotes() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line 'three' is modified\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^.*\\.txt',withDiffContaining='line \\'three\\' is'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_doubleEscapeSingleQuote() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- // This will be written to the file as: line \'three\' is modified.
- .content(CONTENT.replace("line 3\n", "line \\'three\\' is modified\n"))
- .childOf()
- .change(parent)
- .create();
-
- // Users can still provide back-slashes in regexes by escaping them.
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^.*\\\\.txt',withDiffContaining='line \\\\'three\\\\' is'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_escapeDoubleQuotes() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line \"three\" is modified\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^.*\\.txt',withDiffContaining='line \\\"three\\\" is'\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- }
-
- @Test
- public void byFileEdits_invalidSyntax() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
- Change.Id childId =
- changeOperations
- .newChange()
- .file(FILE_NAME)
- .content(CONTENT.replace("line 3\n", "line three\n"))
- .childOf()
- .change(parent)
- .create();
-
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create(
- "file:\"'^.*\\.txt',withDiffContaining=forgot single quotes\"");
-
- ChangeData childChangeData = changeQueryProvider.get().byLegacyChangeId(childId).get(0);
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, childChangeData);
- // If the format is invalid, the operator falls back to the default operator of
- // ChangeQueryBuilder which does not match the change, i.e. returns false.
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
- }
-
- @Test
- public void byFileEdits_invalidFilePattern() throws Exception {
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'^**',withDiffContaining='content'\"");
-
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, changeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.ERROR);
- assertThat(srResult.errorMessage().get()).isEqualTo("Invalid file pattern.");
- }
-
- @Test
- public void byFileEdits_invalidContentPattern() throws Exception {
- SubmitRequirementExpression exp =
- SubmitRequirementExpression.create("file:\"'fileName\\.txt',withDiffContaining='^**'\"");
-
- SubmitRequirementExpressionResult srResult = evaluator.evaluateExpression(exp, changeData);
- assertThat(srResult.status()).isEqualTo(SubmitRequirementExpressionResult.Status.ERROR);
- assertThat(srResult.errorMessage().get()).isEqualTo("Invalid content pattern.");
- }
-
private void voteLabel(String changeId, String labelName, int score) throws RestApiException {
gApi.changes().id(changeId).current().review(new ReviewInput().label(labelName, score));
}
diff --git a/javatests/com/google/gerrit/index/query/QueryParserTest.java b/javatests/com/google/gerrit/index/query/QueryParserTest.java
index f478803..776a2c4 100644
--- a/javatests/com/google/gerrit/index/query/QueryParserTest.java
+++ b/javatests/com/google/gerrit/index/query/QueryParserTest.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.index.query.QueryParser.AND;
import static com.google.gerrit.index.query.QueryParser.COLON;
import static com.google.gerrit.index.query.QueryParser.DEFAULT_FIELD;
-import static com.google.gerrit.index.query.QueryParser.EXACT_PHRASE;
import static com.google.gerrit.index.query.QueryParser.FIELD_NAME;
import static com.google.gerrit.index.query.QueryParser.SINGLE_WORD;
import static com.google.gerrit.index.query.QueryParser.parse;
@@ -205,43 +204,6 @@
assertThat(r).child(2).hasNoChildren();
}
- @Test
- public void fieldNameWithEscapedDoubleQuotesInValue() throws Exception {
- // Actual String: A \"special\" word
- String search = "message:\"A \\\"special\\\" word\"";
- Tree r = parse(search);
- assertThat(r).hasType(FIELD_NAME);
- assertThat(r).hasChildCount(1);
- assertThat(r).hasText("message");
- assertThat(r).child(0).hasType(EXACT_PHRASE);
- // Antlr escaped the double quotes in the phrase.
- assertThat(r).child(0).hasText("A \"special\" word");
- }
-
- @Test
- public void fieldNameWithEscapedTabCharacterIsPreserved() throws Exception {
- String[] searches = {"message:\"A \\t word\"", "message:{A \\t word}"};
- for (String search : searches) {
- Tree r = parse(search);
- assertThat(r).hasType(FIELD_NAME);
- assertThat(r).hasChildCount(1);
- assertThat(r).hasText("message");
- assertThat(r).child(0).hasType(EXACT_PHRASE);
- assertThat(r).child(0).hasText("A \t word");
- }
- }
-
- @Test
- public void fieldNameWithEscapedBackslashIsIncludedInOutput() throws Exception {
- String search = "message:\"A backslash \\\\ in phrase\"";
- Tree r = parse(search);
- assertThat(r).hasType(FIELD_NAME);
- assertThat(r).hasChildCount(1);
- assertThat(r).hasText("message");
- assertThat(r).child(0).hasType(EXACT_PHRASE);
- assertThat(r).child(0).hasText("A backslash \\ in phrase");
- }
-
private static void assertParseFails(String query) {
assertThrows(QueryParseException.class, () -> parse(query));
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 2495fe4..5ae5271 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -243,6 +243,7 @@
* Is the column disabled by a server config or experiment?
*/
_isColumnEnabled(column: string, config: ServerInfo, experiments: string[]) {
+ if (!columnNames.includes(column)) return false;
if (!config || !config.change) return true;
if (column === 'Comments') return experiments.includes('comments-column');
if (column === 'Requirements')
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
index ff8a0f0..841a781 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
@@ -337,6 +337,11 @@
});
});
+ test('obsolete column in preferences not visible', () => {
+ assert.isTrue(element._isColumnEnabled('Subject'));
+ assert.isFalse(element._isColumnEnabled('Assignee'));
+ });
+
suite('random column does not exist', () => {
let element;
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
index a18b772..5327dff 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
@@ -140,9 +140,11 @@
}
} else if (isQuickLabelInfo(this.label)) {
if (this.label.approved) {
- return '👍️';
+ return '👍';
} else if (this.label.rejected) {
- return '👎️';
+ return '👎';
+ } else if (this.label.disliked || this.label.recommended) {
+ return valueString(this.label.value);
}
}
return '';
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
index 1958087..a3f23df 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
@@ -49,9 +49,7 @@
test('renders', () => {
expect(element).shadowDom.to.equal(`<span class="container">
- <div class="positive vote-chip">
- 👍️
- </div>
+ <div class="max vote-chip">👍</div>
</span>`);
});
});
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 1a48a7b..0aa4a51 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -90,8 +90,10 @@
: LabelStatus.RECOMMENDED;
}
} else if (isQuickLabelInfo(label)) {
- if (label.approved) return LabelStatus.RECOMMENDED;
- if (label.rejected) return LabelStatus.DISLIKED;
+ if (label.approved) return LabelStatus.APPROVED;
+ if (label.rejected) return LabelStatus.REJECTED;
+ if (label.disliked) return LabelStatus.DISLIKED;
+ if (label.recommended) return LabelStatus.RECOMMENDED;
}
return LabelStatus.NEUTRAL;
}
@@ -178,7 +180,12 @@
);
}
if (isQuickLabelInfo(labelInfo)) {
- return !!labelInfo.rejected || !!labelInfo.approved;
+ return (
+ !!labelInfo.rejected ||
+ !!labelInfo.approved ||
+ !!labelInfo.recommended ||
+ !!labelInfo.disliked
+ );
}
return false;
}
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 142c607..6cb04af 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -187,8 +187,12 @@
let labelInfo: QuickLabelInfo = {};
assert.equal(getLabelStatus(labelInfo), LabelStatus.NEUTRAL);
labelInfo = {approved: createAccountWithEmail()};
- assert.equal(getLabelStatus(labelInfo), LabelStatus.RECOMMENDED);
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.APPROVED);
labelInfo = {rejected: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.REJECTED);
+ labelInfo = {recommended: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.RECOMMENDED);
+ labelInfo = {disliked: createAccountWithEmail()};
assert.equal(getLabelStatus(labelInfo), LabelStatus.DISLIKED);
});
