opendev/gerrit
Code Issues Proposed changes

Merge "Support "cc:" query"

Browse Source
This commit is contained in:
David Pursehouse 2017-02-23 01:55:01 +00:00 committed by Gerrit Code Review
commit 5dc6f95268
7 changed files with 117 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/user-search.txt
View File

@ -118,6 +118,12 @@ Changes that have been, or need to be, reviewed by 'USER'. The
special case of `reviewer:self` will find changes where the caller
has been added as a reviewer.
[[cc]]
cc:'USER'::
+
Changes that have the given user CC'ed on them. The special case of `cc:self`
will find changes where the caller has been CC'ed.
[[reviewerin]]
reviewerin:'GROUP'::
+

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
View File

@ -40,7 +40,8 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
"author:",
"committer:",
"from:",
"assignee:"),
"assignee:",
"cc:"),
new AccountSuggestOracle() {
@Override
public void onRequestSuggestions(final Request request, final Callback done) {
@ -152,6 +153,7 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
suggestions.add("unresolved:");
if (Gerrit.isNoteDbEnabled()) {
suggestions.add("cc:");
suggestions.add("hashtag:");
}

81
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@ -16,6 +16,7 @@ package com.google.gerrit.server.query.change;
import static com.google.gerrit.reviewdb.client.Change.CHANGE_ID_PATTERN;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
@ -61,6 +62,7 @@ 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.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ListChildProjects;
@ -173,34 +175,35 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@VisibleForTesting
public static class Arguments {
final Provider<ReviewDb> db;
final Provider<InternalChangeQuery> queryProvider;
final ChangeIndexRewriter rewriter;
final DynamicMap<ChangeOperatorFactory> opFactories;
final DynamicMap<ChangeHasOperandFactory> hasOperands;
final IdentifiedUser.GenericFactory userFactory;
final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeNotes.Factory notesFactory;
final ChangeData.Factory changeDataFactory;
final FieldDef.FillArgs fillArgs;
final CommentsUtil commentsUtil;
final AccountCache accountCache;
final AccountResolver accountResolver;
final GroupBackend groupBackend;
final AllProjectsName allProjectsName;
final AllUsersName allUsersName;
final PatchListCache patchListCache;
final GitRepositoryManager repoManager;
final ProjectCache projectCache;
final Provider<ListChildProjects> listChildProjects;
final SubmitDryRun submitDryRun;
final ConflictsCache conflictsCache;
final TrackingFooters trackingFooters;
final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory;
final ChangeIndex index;
final ChangeIndexRewriter rewriter;
final ChangeNotes.Factory notesFactory;
final CommentsUtil commentsUtil;
final ConflictsCache conflictsCache;
final DynamicMap<ChangeHasOperandFactory> hasOperands;
final DynamicMap<ChangeOperatorFactory> opFactories;
final FieldDef.FillArgs fillArgs;
final GitRepositoryManager repoManager;
final GroupBackend groupBackend;
final IdentifiedUser.GenericFactory userFactory;
final IndexConfig indexConfig;
final NotesMigration notesMigration;
final PatchListCache patchListCache;
final ProjectCache projectCache;
final Provider<InternalChangeQuery> queryProvider;
final Provider<ListChildProjects> listChildProjects;
final Provider<ListMembers> listMembers;
final Provider<ReviewDb> db;
final StarredChangesUtil starredChangesUtil;
final AccountCache accountCache;
final SubmitDryRun submitDryRun;
final TrackingFooters trackingFooters;
final boolean allowsDrafts;
private final Provider<CurrentUser> self;
@ -237,7 +240,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
Provider<ListMembers> listMembers,
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
@GerritServerConfig Config cfg) {
@GerritServerConfig Config cfg,
NotesMigration notesMigration) {
this(
db,
queryProvider,
@ -268,7 +272,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
listMembers,
starredChangesUtil,
accountCache,
cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true));
cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true),
notesMigration);
}
private Arguments(
@ -301,7 +306,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
Provider<ListMembers> listMembers,
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
boolean allowsDrafts) {
boolean allowsDrafts,
NotesMigration notesMigration) {
this.db = db;
this.queryProvider = queryProvider;
this.rewriter = rewriter;
@ -332,6 +338,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.accountCache = accountCache;
this.allowsDrafts = allowsDrafts;
this.hasOperands = hasOperands;
this.notesMigration = notesMigration;
}
Arguments asUser(CurrentUser otherUser) {
@ -365,7 +372,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
listMembers,
starredChangesUtil,
accountCache,
allowsDrafts);
allowsDrafts,
notesMigration);
}
Arguments asUser(Account.Id otherId) {
@ -553,7 +561,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
if ("reviewer".equalsIgnoreCase(value)) {
return ReviewerPredicate.create(args, self());
return ReviewerPredicate.reviewer(args, self());
}
if ("cc".equalsIgnoreCase(value)) {
return ReviewerPredicate.cc(args, self());
}
if ("mergeable".equalsIgnoreCase(value)) {
@ -925,12 +937,17 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@Operator
public Predicate<ChangeData> reviewer(String who) throws QueryParseException, OrmException {
Set<Account.Id> m = parseAccount(who);
List<Predicate<ChangeData>> p = Lists.newArrayListWithCapacity(m.size());
for (Account.Id id : m) {
p.add(ReviewerPredicate.create(args, id));
}
return Predicate.or(p);
return Predicate.or(
parseAccount(who)
.stream()
.map(id -> ReviewerPredicate.reviewer(args, id))
.collect(toList()));
}
@Operator
public Predicate<ChangeData> cc(String who) throws QueryParseException, OrmException {
return Predicate.or(
parseAccount(who).stream().map(id -> ReviewerPredicate.cc(args, id)).collect(toList()));
}
@Operator

50
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.query.change;
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;
@ -21,32 +23,50 @@ 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;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
class ReviewerPredicate extends ChangeIndexPredicate {
static Predicate<ChangeData> create(Arguments args, Account.Id id) {
List<Predicate<ChangeData>> and = new ArrayList<>(2);
ReviewerStateInternal[] states = ReviewerStateInternal.values();
List<Predicate<ChangeData>> or = new ArrayList<>(states.length - 1);
for (ReviewerStateInternal state : states) {
if (state != ReviewerStateInternal.REMOVED) {
or.add(new ReviewerPredicate(state, id));
}
static Predicate<ChangeData> reviewer(Arguments args, Account.Id id) {
Predicate<ChangeData> p;
if (args.notesMigration.readChanges()) {
// With NoteDb, Reviewer/CC are clearly distinct states, so only choose reviewer.
p = new ReviewerPredicate(ReviewerStateInternal.REVIEWER, id);
} else {
// Without NoteDb, Reviewer/CC are a bit unpredictable; maintain the old behavior of matching
// any reviewer state.
p = anyReviewerState(id);
}
and.add(Predicate.or(or));
return create(args, p);
}
// TODO(dborowitz): This really belongs much higher up e.g. QueryProcessor.
static Predicate<ChangeData> cc(Arguments args, Account.Id id) {
// As noted above, CC is nebulous without NoteDb, but it certainly doesn't make sense to return
// Reviewers for cc:foo. Most likely this will just not match anything, but let the index sort
// it out.
return create(args, new ReviewerPredicate(ReviewerStateInternal.CC, id));
}
private static Predicate<ChangeData> anyReviewerState(Account.Id id) {
return Predicate.or(
Stream.of(ReviewerStateInternal.values())
.filter(s -> s != ReviewerStateInternal.REMOVED)
.map(s -> new ReviewerPredicate(s, id))
.collect(toList()));
}
private static Predicate<ChangeData> create(Arguments args, Predicate<ChangeData> p) {
if (!args.allowsDrafts) {
and.add(Predicate.not(new ChangeStatusPredicate(Change.Status.DRAFT)));
// 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 Predicate.and(and);
return p;
}
private final ReviewerStateInternal state;
private final Account.Id id;
ReviewerPredicate(ReviewerStateInternal state, Account.Id id) {
private ReviewerPredicate(ReviewerStateInternal state, Account.Id id) {
super(ChangeField.REVIEWER, ChangeField.getReviewerFieldValue(state, id));
this.state = state;
this.id = id;

2
gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
View File

@ -28,7 +28,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
new ChangeQueryBuilder.Arguments(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, null, null, indexes, null, null, null, null, null, null,
null, null));
null, null, null));
}
@Operator

25
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -34,6 +34,7 @@ import com.google.common.truth.ThrowableSubject;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
@ -43,6 +44,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
@ -1480,17 +1482,30 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
}
@Test
public void reviewer() throws Exception {
public void reviewerAndCc() throws Exception {
Account.Id user1 = createAccount("user1");
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
insert(repo, newChange(repo));
gApi.changes().id(change1.getId().get()).current().review(ReviewInput.approve());
gApi.changes().id(change2.getId().get()).current().review(ReviewInput.approve());
AddReviewerInput rin = new AddReviewerInput();
rin.reviewer = user1.toString();
rin.state = ReviewerState.REVIEWER;
gApi.changes().id(change1.getId().get()).addReviewer(rin);
Account.Id id = user.getAccountId();
assertQuery("reviewer:" + id, change2, change1);
rin = new AddReviewerInput();
rin.reviewer = user1.toString();
rin.state = ReviewerState.CC;
gApi.changes().id(change2.getId().get()).addReviewer(rin);
if (notesMigration.readChanges()) {
assertQuery("reviewer:" + user1, change1);
assertQuery("cc:" + user1, change2);
} else {
assertQuery("reviewer:" + user1, change2, change1);
assertQuery("cc:" + user1);
}
}
@Test

3
polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
View File

@ -22,6 +22,8 @@
'author',
'branch',
'bug',
'cc',
'cc:self',
'change',
'comment',
'commentby',
@ -237,6 +239,7 @@
return this._fetchProjects(predicate, expression);
case 'author':
case 'cc':
case 'commentby':
case 'committer':
case 'from':