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 2911b62a5c..bc841ada62 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 static com.google.common.truth.TruthJUnit.assume; 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 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar "); } + @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 933e84b969..fdc14d7c36 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; 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 @@ class ElasticChangeIndex extends AbstractElasticIndex 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 e68e29f479..3afcb07470 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 @@ public class LuceneChangeIndex implements ChangeIndex { 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 @@ public class LuceneChangeIndex implements ChangeIndex { 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 @@ public class LuceneChangeIndex implements ChangeIndex { FluentIterable.from(doc.get(REVIEWER_FIELD)).transform(IndexableField::stringValue))); } + private void decodeReviewersByEmail(ListMultimap doc, ChangeData cd) { + cd.setReviewersByEmail( + ChangeField.parseReviewerByEmailFieldValues( + FluentIterable.from(doc.get(REVIEWER_BY_EMAIL_FIELD)) + .transform(IndexableField::stringValue))); + } + private void decodeSubmitRecords( ListMultimap 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 bfa2ee93f6..066c1a2f4d 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 @@ public class ChangeJson { 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> 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 6a33b31393..d7fe713984 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.PatchSetApproval; 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.FieldDef.FillArgs; 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 class ChangeField { public static final FieldDef> 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> REVIEWER_BY_EMAIL = + exact("reviewer_by_email") + .stored() + .buildRepeatable(cd -> getReviewerByEmailFieldValues(cd.reviewersByEmail())); + @VisibleForTesting static List getReviewerFieldValues(ReviewerSet reviewers) { List r = new ArrayList<>(reviewers.asTable().size() * 2); @@ -200,6 +208,27 @@ public class ChangeField { return state.toString() + ',' + id; } + @VisibleForTesting + static List getReviewerByEmailFieldValues(ReviewerByEmailSet reviewersByEmail) { + List r = new ArrayList<>(reviewersByEmail.asTable().size() * 2); + for (Table.Cell 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 values) { ImmutableTable.Builder b = ImmutableTable.builder(); @@ -220,6 +249,25 @@ public class ChangeField { return ReviewerSet.fromTable(b.build()); } + public static ReviewerByEmailSet parseReviewerByEmailFieldValues(Iterable values) { + ImmutableTable.Builder 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> 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 038c35e461..ba7c1ecc59 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 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { @Deprecated static final Schema V39 = schema(V38); - static final Schema V40 = schema(V39, ChangeField.PRIVATE); + @Deprecated static final Schema V40 = schema(V39, ChangeField.PRIVATE); + + static final Schema 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 3c76319ef4..18fc08322c 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 @@ public abstract class ChangeEmail extends NotificationEmail { 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 b62e3ebb35..1dbe5cd28f 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.CommentsUtil; 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 @@ public class ChangeData { private StarsOf starsOf; private ImmutableMap starRefs; private ReviewerSet reviewers; + private ReviewerByEmailSet reviewersByEmail; private List reviewerUpdates; private PersonIdent author; private PersonIdent committer; @@ -954,6 +956,24 @@ public class ChangeData { 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 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 0604f8bb03..0362c85582 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 implements Matchable { @@ -27,4 +30,11 @@ public abstract class ChangeIndexPredicate extends IndexPredicate protected ChangeIndexPredicate(FieldDef def, String name, String value) { super(def, name, value); } + + protected static Predicate create(Arguments args, Predicate 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 c47bbb57a9..6b66c41879 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.ChangeField; 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 @@ public class ChangeQueryBuilder extends QueryBuilder { @Operator public Predicate 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 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 @@ public class ChangeQueryBuilder extends QueryBuilder { private Account.Id self() throws QueryParseException { return args.getIdentifiedUser().getAccountId(); } + + public Predicate reviewerByState(String who, ReviewerStateInternal state) + throws QueryParseException, OrmException { + Predicate 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 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 0000000000..a040e18c3b --- /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 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 6ce02fbf30..5b8649476d 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 com.google.gwtorm.server.OrmException; import java.util.stream.Stream; class ReviewerPredicate extends ChangeIndexPredicate { + static Predicate 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 reviewer(Arguments args, Account.Id id) { Predicate p; if (args.notesMigration.readChanges()) { @@ -54,15 +60,6 @@ class ReviewerPredicate extends ChangeIndexPredicate { .collect(toList())); } - private static Predicate create(Arguments args, Predicate 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 2b937fffd0..c2f28eb32f 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.ChangeInserter; 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.ChangeNotes; 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 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @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; @@ -1544,6 +1549,71 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { } } + @Test + public void reviewerAndCcByEmail() throws Exception { + assume().that(notesMigration.enabled()).isTrue(); + + Project.NameKey project = new Project.NameKey("repo"); + TestRepository 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 = createProject(project.get()); + ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); + cfg.setEnableReviewerByEmail(true); + saveProjectConfig(project, cfg); + + String userByEmail = "John Doe "; + + 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"); @@ -2027,4 +2097,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Patch.COMMIT_MSG, ImmutableList.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()); + } }