Add reviewers by email to ChangeIndex
This change adds reviewers by email to the ChangeIndex and adds tests
for the new code.
It also expands the 'reviewer' and 'cc' queries to match on
reviewerByEmail as well.
Bug: Issue 4134
Change-Id: Iae9ada56e2ab9a03b6d5c20de4bca53ec27a4767
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index 2911b62..bc841ad 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -239,6 +240,34 @@
gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@gerritcodereview.com>");
}
+ @Test
+ public void reviewersByEmailAreServedFromIndex() throws Exception {
+ assume().that(notesMigration.enabled()).isTrue();
+ AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+ for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+ PushOneCommit.Result r = createChange();
+
+ AddReviewerInput input = new AddReviewerInput();
+ input.reviewer = toRfcAddressString(acc);
+ input.state = state;
+ gApi.changes().id(r.getChangeId()).addReviewer(input);
+
+ notesMigration.setFailOnLoad(true);
+ try {
+ ChangeInfo info =
+ Iterables.getOnlyElement(
+ gApi.changes()
+ .query(r.getChangeId())
+ .withOption(ListChangesOption.DETAILED_LABELS)
+ .get());
+ assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc)));
+ } finally {
+ notesMigration.setFailOnLoad(false);
+ }
+ }
+ }
+
private static String toRfcAddressString(AccountInfo info) {
return (new Address(info.name, info.email)).toString();
}
diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 933e84b..fdc14d7 100644
--- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -34,6 +34,7 @@
import com.google.gerrit.reviewdb.client.Change.Id;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
@@ -342,6 +343,16 @@
cd.setReviewers(ReviewerSet.empty());
}
+ if (source.get(ChangeField.REVIEWER_BY_EMAIL.getName()) != null) {
+ cd.setReviewersByEmail(
+ ChangeField.parseReviewerByEmailFieldValues(
+ FluentIterable.from(
+ source.get(ChangeField.REVIEWER_BY_EMAIL.getName()).getAsJsonArray())
+ .transform(JsonElement::getAsString)));
+ } else if (fields.contains(ChangeField.REVIEWER_BY_EMAIL.getName())) {
+ cd.setReviewersByEmail(ReviewerByEmailSet.empty());
+ }
+
decodeSubmitRecords(
source,
ChangeField.STORED_SUBMIT_RECORD_STRICT.getName(),
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index e68e29f..3afcb07 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -121,6 +121,7 @@
private static final String REF_STATE_PATTERN_FIELD = ChangeField.REF_STATE_PATTERN.getName();
private static final String REVIEWEDBY_FIELD = ChangeField.REVIEWEDBY.getName();
private static final String REVIEWER_FIELD = ChangeField.REVIEWER.getName();
+ private static final String REVIEWER_BY_EMAIL_FIELD = ChangeField.REVIEWER_BY_EMAIL.getName();
private static final String HASHTAG_FIELD = ChangeField.HASHTAG_CASE_AWARE.getName();
private static final String STAR_FIELD = ChangeField.STAR.getName();
private static final String SUBMIT_RECORD_LENIENT_FIELD =
@@ -459,6 +460,9 @@
if (fields.contains(REVIEWER_FIELD)) {
decodeReviewers(doc, cd);
}
+ if (fields.contains(REVIEWER_BY_EMAIL_FIELD)) {
+ decodeReviewersByEmail(doc, cd);
+ }
decodeSubmitRecords(
doc, SUBMIT_RECORD_STRICT_FIELD, ChangeField.SUBMIT_RULE_OPTIONS_STRICT, cd);
decodeSubmitRecords(
@@ -555,6 +559,13 @@
FluentIterable.from(doc.get(REVIEWER_FIELD)).transform(IndexableField::stringValue)));
}
+ private void decodeReviewersByEmail(ListMultimap<String, IndexableField> doc, ChangeData cd) {
+ cd.setReviewersByEmail(
+ ChangeField.parseReviewerByEmailFieldValues(
+ FluentIterable.from(doc.get(REVIEWER_BY_EMAIL_FIELD))
+ .transform(IndexableField::stringValue)));
+ }
+
private void decodeSubmitRecords(
ListMultimap<String, IndexableField> doc,
String field,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index bfa2ee9..066c1a2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -532,9 +532,8 @@
cd.reviewers().asTable().rowMap().entrySet()) {
out.reviewers.put(e.getKey().asReviewerState(), toAccountInfo(e.getValue().keySet()));
}
- // TODO(hiesel) Load from ChangeData instead after the data was added there
for (Map.Entry<ReviewerStateInternal, Map<Address, Timestamp>> e :
- cd.notes().getReviewersByEmail().asTable().rowMap().entrySet()) {
+ cd.reviewersByEmail().asTable().rowMap().entrySet()) {
out.reviewers.put(
e.getKey().asReviewerState(), toAccountInfoByEmail(e.getValue().keySet()));
}
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 6a33b31..d7fe713 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
@@ -46,6 +46,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.OutputFormat;
+import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.index.FieldDef;
@@ -53,6 +54,7 @@
import com.google.gerrit.server.index.SchemaUtil;
import com.google.gerrit.server.index.change.StalenessChecker.RefState;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
+import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -184,6 +186,12 @@
public static final FieldDef<ChangeData, Iterable<String>> REVIEWER =
exact("reviewer2").stored().buildRepeatable(cd -> getReviewerFieldValues(cd.reviewers()));
+ /** Reviewer(s) associated with the change that do not have a gerrit account. */
+ public static final FieldDef<ChangeData, Iterable<String>> REVIEWER_BY_EMAIL =
+ exact("reviewer_by_email")
+ .stored()
+ .buildRepeatable(cd -> getReviewerByEmailFieldValues(cd.reviewersByEmail()));
+
@VisibleForTesting
static List<String> getReviewerFieldValues(ReviewerSet reviewers) {
List<String> r = new ArrayList<>(reviewers.asTable().size() * 2);
@@ -200,6 +208,27 @@
return state.toString() + ',' + id;
}
+ @VisibleForTesting
+ static List<String> getReviewerByEmailFieldValues(ReviewerByEmailSet reviewersByEmail) {
+ List<String> r = new ArrayList<>(reviewersByEmail.asTable().size() * 2);
+ for (Table.Cell<ReviewerStateInternal, Address, Timestamp> c :
+ reviewersByEmail.asTable().cellSet()) {
+ String v = getReviewerByEmailFieldValue(c.getRowKey(), c.getColumnKey());
+ r.add(v);
+ if (c.getColumnKey().getName() != null) {
+ // Add another entry without the name to provide search functionality on the email
+ Address emailOnly = new Address(c.getColumnKey().getEmail());
+ r.add(getReviewerByEmailFieldValue(c.getRowKey(), emailOnly));
+ }
+ r.add(v + ',' + c.getValue().getTime());
+ }
+ return r;
+ }
+
+ public static String getReviewerByEmailFieldValue(ReviewerStateInternal state, Address adr) {
+ return state.toString() + ',' + adr;
+ }
+
public static ReviewerSet parseReviewerFieldValues(Iterable<String> values) {
ImmutableTable.Builder<ReviewerStateInternal, Account.Id, Timestamp> b =
ImmutableTable.builder();
@@ -220,6 +249,25 @@
return ReviewerSet.fromTable(b.build());
}
+ public static ReviewerByEmailSet parseReviewerByEmailFieldValues(Iterable<String> values) {
+ ImmutableTable.Builder<ReviewerStateInternal, Address, Timestamp> b = ImmutableTable.builder();
+ for (String v : values) {
+ int f = v.indexOf(',');
+ if (f < 0) {
+ continue;
+ }
+ int l = v.lastIndexOf(',');
+ if (l == f) {
+ continue;
+ }
+ b.put(
+ ReviewerStateInternal.valueOf(v.substring(0, f)),
+ Address.parse(v.substring(f + 1, l)),
+ new Timestamp(Long.valueOf(v.substring(l + 1, v.length()))));
+ }
+ return ReviewerByEmailSet.fromTable(b.build());
+ }
+
/** Commit ID of any patch set on the change, using prefix match. */
public static final FieldDef<ChangeData, Iterable<String>> COMMIT =
prefix(ChangeQueryBuilder.FIELD_COMMIT).buildRepeatable(ChangeField::getRevisions);
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 038c35e..ba7c1ec 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
@@ -92,7 +92,9 @@
@Deprecated static final Schema<ChangeData> V39 = schema(V38);
- static final Schema<ChangeData> V40 = schema(V39, ChangeField.PRIVATE);
+ @Deprecated static final Schema<ChangeData> V40 = schema(V39, ChangeField.PRIVATE);
+
+ static final Schema<ChangeData> V41 = schema(V40, ChangeField.REVIEWER_BY_EMAIL);
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/mail/send/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 3c76319..18fc083 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -183,13 +183,11 @@
if (notify.ordinal() >= NotifyHandling.OWNER_REVIEWERS.ordinal()) {
try {
- // TODO(hiesel) Load from index instead
addByEmail(
- RecipientType.CC,
- changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.CC));
+ RecipientType.CC, changeData.reviewersByEmail().byState(ReviewerStateInternal.CC));
addByEmail(
RecipientType.TO,
- changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.REVIEWER));
+ changeData.reviewersByEmail().byState(ReviewerStateInternal.REVIEWER));
} catch (OrmException e) {
throw new EmailException("Failed to add unregistered CCs " + change.getChangeId(), e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index b62e3eb..1dbe5cd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -52,6 +52,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
@@ -352,6 +353,7 @@
private StarsOf starsOf;
private ImmutableMap<Account.Id, StarRef> starRefs;
private ReviewerSet reviewers;
+ private ReviewerByEmailSet reviewersByEmail;
private List<ReviewerStatusUpdate> reviewerUpdates;
private PersonIdent author;
private PersonIdent committer;
@@ -954,6 +956,24 @@
return reviewers;
}
+ public ReviewerByEmailSet reviewersByEmail() throws OrmException {
+ if (reviewersByEmail == null) {
+ if (!lazyLoad) {
+ return ReviewerByEmailSet.empty();
+ }
+ reviewersByEmail = notes().getReviewersByEmail();
+ }
+ return reviewersByEmail;
+ }
+
+ public void setReviewersByEmail(ReviewerByEmailSet reviewersByEmail) {
+ this.reviewersByEmail = reviewersByEmail;
+ }
+
+ public ReviewerByEmailSet getReviewersByEmail() {
+ return reviewersByEmail;
+ }
+
public List<ReviewerStatusUpdate> reviewerUpdates() throws OrmException {
if (reviewerUpdates == null) {
if (!lazyLoad) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
index 0604f8b..0362c85 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
@@ -14,9 +14,12 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.query.Matchable;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
public abstract class ChangeIndexPredicate extends IndexPredicate<ChangeData>
implements Matchable<ChangeData> {
@@ -27,4 +30,11 @@
protected ChangeIndexPredicate(FieldDef<ChangeData, ?> def, String name, String value) {
super(def, name, value);
}
+
+ protected static Predicate<ChangeData> create(Arguments args, Predicate<ChangeData> p) {
+ if (!args.allowsDrafts) {
+ return Predicate.and(p, Predicate.not(new ChangeStatusPredicate(Change.Status.DRAFT)));
+ }
+ return p;
+ }
}
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 c47bbb5..6b66c41 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
@@ -61,8 +61,10 @@
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexRewriter;
+import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ListChildProjects;
@@ -942,17 +944,12 @@
@Operator
public Predicate<ChangeData> reviewer(String who) throws QueryParseException, OrmException {
- return Predicate.or(
- parseAccount(who)
- .stream()
- .map(id -> ReviewerPredicate.reviewer(args, id))
- .collect(toList()));
+ return reviewerByState(who, ReviewerStateInternal.REVIEWER);
}
@Operator
public Predicate<ChangeData> cc(String who) throws QueryParseException, OrmException {
- return Predicate.or(
- parseAccount(who).stream().map(id -> ReviewerPredicate.cc(args, id)).collect(toList()));
+ return reviewerByState(who, ReviewerStateInternal.CC);
}
@Operator
@@ -1181,4 +1178,37 @@
private Account.Id self() throws QueryParseException {
return args.getIdentifiedUser().getAccountId();
}
+
+ public Predicate<ChangeData> reviewerByState(String who, ReviewerStateInternal state)
+ throws QueryParseException, OrmException {
+ Predicate<ChangeData> reviewerByEmailPredicate = null;
+ if (args.index.getSchema().hasField(ChangeField.REVIEWER_BY_EMAIL)) {
+ Address address = Address.tryParse(who);
+ if (address != null) {
+ reviewerByEmailPredicate = ReviewerByEmailPredicate.forState(args, address, state);
+ }
+ }
+
+ Predicate<ChangeData> reviewerPredicate = null;
+ try {
+ reviewerPredicate =
+ Predicate.or(
+ parseAccount(who)
+ .stream()
+ .map(id -> ReviewerPredicate.forState(args, id, state))
+ .collect(toList()));
+ } catch (QueryParseException e) {
+ // Propagate this exception only if we can't use 'who' to query by email
+ if (reviewerByEmailPredicate == null) {
+ throw e;
+ }
+ }
+
+ if (reviewerPredicate != null && reviewerByEmailPredicate != null) {
+ return Predicate.or(reviewerPredicate, reviewerByEmailPredicate);
+ } else if (reviewerPredicate != null) {
+ return reviewerPredicate;
+ }
+ return reviewerByEmailPredicate;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java
new file mode 100644
index 0000000..a040e18
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java
@@ -0,0 +1,55 @@
+// 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.common.base.Preconditions.checkArgument;
+
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
+import com.google.gwtorm.server.OrmException;
+
+class ReviewerByEmailPredicate extends ChangeIndexPredicate {
+
+ static Predicate<ChangeData> forState(Arguments args, Address adr, ReviewerStateInternal state) {
+ checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer");
+ return create(args, new ReviewerByEmailPredicate(state, adr));
+ }
+
+ private final ReviewerStateInternal state;
+ private final Address adr;
+
+ private ReviewerByEmailPredicate(ReviewerStateInternal state, Address adr) {
+ super(ChangeField.REVIEWER_BY_EMAIL, ChangeField.getReviewerByEmailFieldValue(state, adr));
+ this.state = state;
+ this.adr = adr;
+ }
+
+ Address getAddress() {
+ return adr;
+ }
+
+ @Override
+ public boolean match(ChangeData cd) throws OrmException {
+ return cd.reviewersByEmail().asTable().get(state, adr) != null;
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
index 6ce02fb..5b86494 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
@@ -14,10 +14,10 @@
package com.google.gerrit.server.query.change;
+import static com.google.common.base.Preconditions.checkArgument;
import static java.util.stream.Collectors.toList;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.query.Predicate;
@@ -26,6 +26,12 @@
import java.util.stream.Stream;
class ReviewerPredicate extends ChangeIndexPredicate {
+ static Predicate<ChangeData> forState(
+ Arguments args, Account.Id id, ReviewerStateInternal state) {
+ checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer");
+ return create(args, new ReviewerPredicate(state, id));
+ }
+
static Predicate<ChangeData> reviewer(Arguments args, Account.Id id) {
Predicate<ChangeData> p;
if (args.notesMigration.readChanges()) {
@@ -54,15 +60,6 @@
.collect(toList()));
}
- private static Predicate<ChangeData> create(Arguments args, Predicate<ChangeData> p) {
- if (!args.allowsDrafts) {
- // TODO(dborowitz): This really belongs much higher up e.g. QueryProcessor. Also, why are we
- // even doing this?
- return Predicate.and(p, Predicate.not(new ChangeStatusPredicate(Change.Status.DRAFT)));
- }
- return p;
- }
-
private final ReviewerStateInternal state;
private final Account.Id id;
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 2b937ff..c2f28eb 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
@@ -71,6 +71,8 @@
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.QueryOptions;
@@ -83,6 +85,7 @@
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.util.RequestContext;
@@ -151,6 +154,8 @@
@Inject protected SchemaCreator schemaCreator;
@Inject protected Sequences seq;
@Inject protected ThreadLocalRequestContext requestContext;
+ @Inject protected ProjectCache projectCache;
+ @Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
protected Injector injector;
protected LifecycleManager lifecycle;
@@ -1545,6 +1550,71 @@
}
@Test
+ public void reviewerAndCcByEmail() throws Exception {
+ assume().that(notesMigration.enabled()).isTrue();
+
+ Project.NameKey project = new Project.NameKey("repo");
+ TestRepository<Repo> repo = createProject(project.get());
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ cfg.setEnableReviewerByEmail(true);
+ saveProjectConfig(project, cfg);
+
+ String userByEmail = "un.registered@reviewer.com";
+ String userByEmailWithName = "John Doe <" + userByEmail + ">";
+
+ Change change1 = insert(repo, newChange(repo));
+ Change change2 = insert(repo, newChange(repo));
+ insert(repo, newChange(repo));
+
+ AddReviewerInput rin = new AddReviewerInput();
+ rin.reviewer = userByEmailWithName;
+ rin.state = ReviewerState.REVIEWER;
+ gApi.changes().id(change1.getId().get()).addReviewer(rin);
+
+ rin = new AddReviewerInput();
+ rin.reviewer = userByEmailWithName;
+ rin.state = ReviewerState.CC;
+ gApi.changes().id(change2.getId().get()).addReviewer(rin);
+
+ assertQuery("reviewer:\"" + userByEmailWithName + "\"", change1);
+ assertQuery("cc:\"" + userByEmailWithName + "\"", change2);
+
+ // Omitting the name:
+ assertQuery("reviewer:\"" + userByEmail + "\"", change1);
+ assertQuery("cc:\"" + userByEmail + "\"", change2);
+ }
+
+ @Test
+ public void reviewerAndCcByEmailWithQueryForDifferentUser() throws Exception {
+ assume().that(notesMigration.enabled()).isTrue();
+
+ Project.NameKey project = new Project.NameKey("repo");
+ TestRepository<Repo> repo = createProject(project.get());
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ cfg.setEnableReviewerByEmail(true);
+ saveProjectConfig(project, cfg);
+
+ String userByEmail = "John Doe <un.registered@reviewer.com>";
+
+ Change change1 = insert(repo, newChange(repo));
+ Change change2 = insert(repo, newChange(repo));
+ insert(repo, newChange(repo));
+
+ AddReviewerInput rin = new AddReviewerInput();
+ rin.reviewer = userByEmail;
+ rin.state = ReviewerState.REVIEWER;
+ gApi.changes().id(change1.getId().get()).addReviewer(rin);
+
+ rin = new AddReviewerInput();
+ rin.reviewer = userByEmail;
+ rin.state = ReviewerState.CC;
+ gApi.changes().id(change2.getId().get()).addReviewer(rin);
+
+ assertQuery("reviewer:\"someone@example.com\"");
+ assertQuery("cc:\"someone@example.com\"");
+ }
+
+ @Test
public void submitRecords() throws Exception {
Account.Id user1 = createAccount("user1");
TestRepository<Repo> repo = createProject("repo");
@@ -2027,4 +2097,12 @@
Patch.COMMIT_MSG, ImmutableList.<ReviewInput.CommentInput>of(comment));
gApi.changes().id(changeId).current().review(input);
}
+
+ private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws Exception {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(p)) {
+ md.setAuthor(userFactory.create(userId));
+ cfg.commit(md);
+ }
+ projectCache.evict(cfg.getProject());
+ }
}
