Merge "Fix 'author' and 'committer' search operators"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index 8765607..a3bd230 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -74,10 +74,12 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Stream;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.FooterLine;
/**
@@ -324,10 +326,36 @@
return SchemaUtil.getPersonParts(cd.getAuthor());
}
+ public static Set<String> getAuthorNameAndEmail(ChangeData cd) throws OrmException, IOException {
+ return getNameAndEmail(cd.getAuthor());
+ }
+
public static Set<String> getCommitterParts(ChangeData cd) throws OrmException, IOException {
return SchemaUtil.getPersonParts(cd.getCommitter());
}
+ public static Set<String> getCommitterNameAndEmail(ChangeData cd)
+ throws OrmException, IOException {
+ return getNameAndEmail(cd.getCommitter());
+ }
+
+ private static Set<String> getNameAndEmail(PersonIdent person) {
+ if (person == null) {
+ return ImmutableSet.of();
+ }
+
+ String name = person.getName().toLowerCase(Locale.US);
+ String email = person.getEmailAddress().toLowerCase(Locale.US);
+
+ StringBuilder nameEmailBuilder = new StringBuilder();
+ PersonIdent.appendSanitized(nameEmailBuilder, name);
+ nameEmailBuilder.append(" <");
+ PersonIdent.appendSanitized(nameEmailBuilder, email);
+ nameEmailBuilder.append('>');
+
+ return ImmutableSet.of(name, email, nameEmailBuilder.toString());
+ }
+
/**
* The exact email address, or any part of the author name or email address, in the current patch
* set.
@@ -335,6 +363,11 @@
public static final FieldDef<ChangeData, Iterable<String>> AUTHOR =
fullText(ChangeQueryBuilder.FIELD_AUTHOR).buildRepeatable(ChangeField::getAuthorParts);
+ /** The exact name, email address and NameEmail of the author. */
+ public static final FieldDef<ChangeData, Iterable<String>> EXACT_AUTHOR =
+ exact(ChangeQueryBuilder.FIELD_EXACTAUTHOR)
+ .buildRepeatable(ChangeField::getAuthorNameAndEmail);
+
/**
* The exact email address, or any part of the committer name or email address, in the current
* patch set.
@@ -342,6 +375,11 @@
public static final FieldDef<ChangeData, Iterable<String>> COMMITTER =
fullText(ChangeQueryBuilder.FIELD_COMMITTER).buildRepeatable(ChangeField::getCommitterParts);
+ /** The exact name, email address, and NameEmail of the committer. */
+ public static final FieldDef<ChangeData, Iterable<String>> EXACT_COMMITTER =
+ exact(ChangeQueryBuilder.FIELD_EXACTCOMMITTER)
+ .buildRepeatable(ChangeField::getCommitterNameAndEmail);
+
public static final ProtobufCodec<Change> CHANGE_CODEC = CodecFactory.encoder(Change.class);
/** Serialized change object, used for pre-populating results. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index ec507f4..be4f24b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -72,8 +72,10 @@
@Deprecated static final Schema<ChangeData> V40 = schema(V39, ChangeField.PRIVATE);
@Deprecated static final Schema<ChangeData> V41 = schema(V40, ChangeField.REVIEWER_BY_EMAIL);
+ @Deprecated static final Schema<ChangeData> V42 = schema(V41, ChangeField.WIP);
- static final Schema<ChangeData> V42 = schema(V41, ChangeField.WIP);
+ static final Schema<ChangeData> V43 =
+ schema(V42, ChangeField.EXACT_AUTHOR, ChangeField.EXACT_COMMITTER);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 39e9241..af0a3b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -57,6 +57,7 @@
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.Schema;
+import com.google.gerrit.server.index.SchemaUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -87,7 +88,9 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Function;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
@@ -127,6 +130,7 @@
public static final String FIELD_AGE = "age";
public static final String FIELD_ASSIGNEE = "assignee";
public static final String FIELD_AUTHOR = "author";
+ public static final String FIELD_EXACTAUTHOR = "exactauthor";
public static final String FIELD_BEFORE = "before";
public static final String FIELD_CHANGE = "change";
public static final String FIELD_CHANGE_ID = "change_id";
@@ -134,6 +138,7 @@
public static final String FIELD_COMMENTBY = "commentby";
public static final String FIELD_COMMIT = "commit";
public static final String FIELD_COMMITTER = "committer";
+ public static final String FIELD_EXACTCOMMITTER = "exactcommitter";
public static final String FIELD_CONFLICTS = "conflicts";
public static final String FIELD_DELETED = "deleted";
public static final String FIELD_DELTA = "delta";
@@ -1119,13 +1124,21 @@
}
@Operator
- public Predicate<ChangeData> author(String who) {
- return new AuthorPredicate(who);
+ public Predicate<ChangeData> author(String who) throws QueryParseException {
+ if (args.getSchema().hasField(ChangeField.EXACT_AUTHOR)) {
+ return getAuthorOrCommitterPredicate(
+ who.trim(), ExactAuthorPredicate::new, AuthorPredicate::new);
+ }
+ return getAuthorOrCommitterFullTextPredicate(who.trim(), AuthorPredicate::new);
}
@Operator
- public Predicate<ChangeData> committer(String who) {
- return new CommitterPredicate(who);
+ public Predicate<ChangeData> committer(String who) throws QueryParseException {
+ if (args.getSchema().hasField(ChangeField.EXACT_COMMITTER)) {
+ return getAuthorOrCommitterPredicate(
+ who.trim(), ExactCommitterPredicate::new, CommitterPredicate::new);
+ }
+ return getAuthorOrCommitterFullTextPredicate(who.trim(), CommitterPredicate::new);
}
@Operator
@@ -1199,6 +1212,30 @@
return Predicate.or(predicates);
}
+ private Predicate<ChangeData> getAuthorOrCommitterPredicate(
+ String who,
+ Function<String, Predicate<ChangeData>> exactPredicateFunc,
+ Function<String, Predicate<ChangeData>> fullPredicateFunc)
+ throws QueryParseException {
+ if (Address.tryParse(who) != null) {
+ return exactPredicateFunc.apply(who);
+ }
+ return getAuthorOrCommitterFullTextPredicate(who, fullPredicateFunc);
+ }
+
+ private Predicate<ChangeData> getAuthorOrCommitterFullTextPredicate(
+ String who, Function<String, Predicate<ChangeData>> fullPredicateFunc)
+ throws QueryParseException {
+ Set<String> parts = SchemaUtil.getNameParts(who);
+ if (parts.isEmpty()) {
+ throw error("invalid value");
+ }
+
+ List<Predicate<ChangeData>> predicates =
+ parts.stream().map(fullPredicateFunc).collect(Collectors.toList());
+ return Predicate.and(predicates);
+ }
+
private Set<Account.Id> parseAccount(String who) throws QueryParseException, OrmException {
if (isSelf(who)) {
return Collections.singleton(self());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactAuthorPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactAuthorPredicate.java
new file mode 100644
index 0000000..bca5d3b
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactAuthorPredicate.java
@@ -0,0 +1,43 @@
+// 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.
+
+package com.google.gerrit.server.query.change;
+
+import static com.google.gerrit.server.index.change.ChangeField.EXACT_AUTHOR;
+import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_EXACTAUTHOR;
+
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
+import java.util.Locale;
+
+public class ExactAuthorPredicate extends ChangeIndexPredicate {
+ public ExactAuthorPredicate(String value) {
+ super(EXACT_AUTHOR, FIELD_EXACTAUTHOR, value.toLowerCase(Locale.US));
+ }
+
+ @Override
+ public boolean match(ChangeData object) throws OrmException {
+ try {
+ return ChangeField.getAuthorNameAndEmail(object).contains(getValue());
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitterPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitterPredicate.java
new file mode 100644
index 0000000..3fae5e5
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitterPredicate.java
@@ -0,0 +1,43 @@
+// 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.
+
+package com.google.gerrit.server.query.change;
+
+import static com.google.gerrit.server.index.change.ChangeField.EXACT_COMMITTER;
+import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_EXACTCOMMITTER;
+
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
+import java.util.Locale;
+
+public class ExactCommitterPredicate extends ChangeIndexPredicate {
+ public ExactCommitterPredicate(String value) {
+ super(EXACT_COMMITTER, FIELD_EXACTCOMMITTER, value.toLowerCase(Locale.US));
+ }
+
+ @Override
+ public boolean match(ChangeData object) throws OrmException {
+ try {
+ return ChangeField.getCommitterNameAndEmail(object).contains(getValue());
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 34536ab..ba37a5e 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -124,6 +124,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -493,57 +494,82 @@
}
@Test
- public void byAuthor() throws Exception {
- TestRepository<Repo> repo = createProject("repo");
- Change change1 = insert(repo, newChange(repo), userId);
-
- // By exact email address
- assertQuery("author:jauthor@example.com", change1);
-
- // By email address part
- assertQuery("author:jauthor", change1);
- assertQuery("author:example", change1);
- assertQuery("author:example.com", change1);
-
- // By name part
- assertQuery("author:Author", change1);
-
- // Case insensitive
- assertQuery("author:jAuThOr", change1);
- assertQuery("author:ExAmPlE", change1);
-
- // By non-existing email address / name / part
- assertQuery("author:jcommitter@example.com");
- assertQuery("author:somewhere.com");
- assertQuery("author:jcommitter");
- assertQuery("author:Committer");
+ public void byAuthorExact() throws Exception {
+ byAuthorOrCommitterExact("author:");
}
@Test
- public void byCommitter() throws Exception {
+ public void byAuthorFullText() throws Exception {
+ byAuthorOrCommitterFullText("author:");
+ }
+
+ @Test
+ public void byCommitterExact() throws Exception {
+ byAuthorOrCommitterExact("committer:");
+ }
+
+ @Test
+ public void byCommitterFullText() throws Exception {
+ byAuthorOrCommitterFullText("committer:");
+ }
+
+ private void byAuthorOrCommitterExact(String searchOperator) throws Exception {
TestRepository<Repo> repo = createProject("repo");
- Change change1 = insert(repo, newChange(repo), userId);
+ PersonIdent johnDoe = new PersonIdent("John Doe", "john.doe@example.com");
+ PersonIdent john = new PersonIdent("John", "john@example.com");
+ PersonIdent doeSmith = new PersonIdent("Doe Smith", "doe_smith@example.com");
+ Change change1 = createChange(repo, johnDoe);
+ Change change2 = createChange(repo, john);
+ Change change3 = createChange(repo, doeSmith);
- // By exact email address
- assertQuery("committer:jcommitter@example.com", change1);
+ // Only email address.
+ assertQuery(searchOperator + "john.doe@example.com", change1);
+ assertQuery(searchOperator + "john@example.com", change2);
+ assertQuery(searchOperator + "Doe_SmIth@example.com", change3); // Case insensitive.
- // By email address part
- assertQuery("committer:jcommitter", change1);
- assertQuery("committer:example", change1);
- assertQuery("committer:example.com", change1);
+ // Right combination of email address and name.
+ assertQuery(searchOperator + "\"John Doe <john.doe@example.com>\"", change1);
+ assertQuery(searchOperator + "\" John <john@example.com> \"", change2);
+ assertQuery(searchOperator + "\"doE SMITH <doe_smitH@example.com>\"", change3);
- // By name part
- assertQuery("committer:Committer", change1);
+ // Wrong combination of email address and name.
+ assertQuery(searchOperator + "\"John <john.doe@example.com>\"");
+ assertQuery(searchOperator + "\"Doe John <john@example.com>\"");
+ assertQuery(searchOperator + "\"Doe John <doe_smith@example.com>\"");
+ }
- // Case insensitive
- assertQuery("committer:jCoMmItTeR", change1);
- assertQuery("committer:ExAmPlE", change1);
+ private void byAuthorOrCommitterFullText(String searchOperator) throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ PersonIdent johnDoe = new PersonIdent("John Doe", "john.doe@example.com");
+ PersonIdent john = new PersonIdent("John", "john@example.com");
+ PersonIdent doeSmith = new PersonIdent("Doe Smith", "doe_smith@example.com");
+ Change change1 = createChange(repo, johnDoe);
+ Change change2 = createChange(repo, john);
+ Change change3 = createChange(repo, doeSmith);
- // By non-existing email address / name / part
- assertQuery("committer:jauthor@example.com");
- assertQuery("committer:somewhere.com");
- assertQuery("committer:jauthor");
- assertQuery("committer:Author");
+ // By exact name.
+ assertQuery(searchOperator + "\"John Doe\"", change1);
+ assertQuery(searchOperator + "\"john\"", change2, change1);
+ assertQuery(searchOperator + "\"Doe smith\"", change3);
+
+ // By name part.
+ assertQuery(searchOperator + "Doe", change3, change1);
+ assertQuery(searchOperator + "smith", change3);
+
+ // By wrong combination.
+ assertQuery(searchOperator + "\"John Smith\"");
+
+ // By invalid query.
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("invalid value");
+ // SchemaUtil.getNameParts will return an empty set for query only containing these characters.
+ assertQuery(searchOperator + "@.- /_");
+ }
+
+ private Change createChange(TestRepository<Repo> repo, PersonIdent person) throws Exception {
+ RevCommit commit =
+ repo.parseBody(repo.commit().message("message").author(person).committer(person).create());
+ return insert(repo, newChangeForCommit(repo, commit), null);
}
@Test
