Index a field for reviewing a change after last owner update
This is a generalization of the "reviewed" bit in ChangeJson, which indicates that the current user has commented on the change more recently than the change owner commented or added a patch set. At index time, record this field for each user for whom this is true. This will help us avoid another path by which we touch the database when rendering search results. Searching by this field takes two forms. Searching for "reviewedby:<user>" finds all changes where a comment by a specific user was newer than the last update by the owner. Searching for "is:reviewed" finds all changes where at least one specific user left a comment more recently than the last update by the owner. This is implemented by internally converting to "-reviewedby:none", where "none" is a special sentinel field stored in the index. This replaces the existing "is:reviewed" semantics, which were somewhat confusing as they bore no relation whatsoever to the "reviewed" field in ChangeInfo. The new semantics mean "reviewedby:self" can match exactly the set of changes for which the "reviewed" field would be set. Change-Id: Icb7192eb5c41890ee3f3ef0fecb3d91d15b744f9
This commit is contained in:
@@ -263,8 +263,8 @@ and thus is likely to notify the user when it updates.
|
||||
|
||||
is:reviewed::
|
||||
+
|
||||
True if there is at least one non-zero score on the change, in any
|
||||
approval category, by any user.
|
||||
True if any user has commented on the change more recently than the
|
||||
last update (comment or patch set) from the change owner.
|
||||
|
||||
is:owner::
|
||||
+
|
||||
@@ -306,8 +306,9 @@ merge pending'.
|
||||
|
||||
status:reviewed::
|
||||
+
|
||||
Same as 'is:reviewed', matches if there is at least one non-zero
|
||||
score on the change, in any approval category, by any user.
|
||||
Same as 'is:reviewed', matches if any user has commented on the change
|
||||
more recently than the last update (comment or patch set) from the
|
||||
change owner.
|
||||
|
||||
status:submitted::
|
||||
+
|
||||
@@ -350,6 +351,12 @@ from:'USER'::
|
||||
Changes containing a top-level or inline comment by 'USER', or owned by
|
||||
'USER'. Equivalent to `(owner:USER OR commentby:USER)`.
|
||||
|
||||
[[reviewedby]]
|
||||
reviewedby:'USER'::
|
||||
+
|
||||
Changes where 'USER' has commented on the change more recently than the
|
||||
last update (comment or patch set) from the change owner.
|
||||
|
||||
|
||||
== Argument Quoting
|
||||
|
||||
|
||||
@@ -828,7 +828,7 @@ public class ChangeJson {
|
||||
return reviewed;
|
||||
}
|
||||
|
||||
private boolean isChangeReviewed(Account.Id self, ChangeData cd,
|
||||
public static boolean isChangeReviewed(Account.Id self, ChangeData cd,
|
||||
Iterable<ChangeMessage> msgs) throws OrmException {
|
||||
// Sort messages to keep the most recent ones at the beginning.
|
||||
List<ChangeMessage> reversed = ChangeNotes.MESSAGE_BY_TIME.reverse()
|
||||
|
||||
@@ -43,6 +43,7 @@ import org.eclipse.jgit.revwalk.FooterLine;
|
||||
import java.io.ByteArrayOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.sql.Timestamp;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
@@ -338,7 +339,8 @@ public class ChangeField {
|
||||
};
|
||||
|
||||
/** Set true if the change has a non-zero label score. */
|
||||
public static final FieldDef<ChangeData, String> REVIEWED =
|
||||
@Deprecated
|
||||
public static final FieldDef<ChangeData, String> LEGACY_REVIEWED =
|
||||
new FieldDef.Single<ChangeData, String>(
|
||||
"reviewed", FieldType.EXACT, false) {
|
||||
@Override
|
||||
@@ -585,6 +587,36 @@ public class ChangeField {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Users the change was reviewed by since the last author update.
|
||||
* <p>
|
||||
* A change is considered reviewed by a user if the latest update by that user
|
||||
* is newer than the latest update by the change author. Both top-level change
|
||||
* messages and new patch sets are considered to be updates.
|
||||
* <p>
|
||||
* If the latest update is by the change owner, then the special value {@link
|
||||
* #NOT_REVIEWED} is emitted.
|
||||
*/
|
||||
public static final FieldDef<ChangeData, Iterable<Integer>> REVIEWEDBY =
|
||||
new FieldDef.Repeatable<ChangeData, Integer>(
|
||||
ChangeQueryBuilder.FIELD_REVIEWEDBY, FieldType.INTEGER, true) {
|
||||
@Override
|
||||
public Iterable<Integer> get(ChangeData input, FillArgs args)
|
||||
throws OrmException {
|
||||
Set<Account.Id> reviewedBy = input.reviewedBy();
|
||||
if (reviewedBy.isEmpty()) {
|
||||
return ImmutableSet.of(NOT_REVIEWED);
|
||||
}
|
||||
List<Integer> result = new ArrayList<>(reviewedBy.size());
|
||||
for (Account.Id id : reviewedBy) {
|
||||
result.add(id.get());
|
||||
}
|
||||
return result;
|
||||
}
|
||||
};
|
||||
|
||||
public static final Integer NOT_REVIEWED = -1;
|
||||
|
||||
private static String getTopic(ChangeData input) throws OrmException {
|
||||
Change c = input.change();
|
||||
if (c == null) {
|
||||
|
||||
@@ -47,7 +47,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -77,7 +77,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -105,7 +105,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -133,7 +133,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -145,6 +145,7 @@ public class ChangeSchemas {
|
||||
ChangeField.HASHTAG,
|
||||
ChangeField.COMMENTBY);
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static final Schema<ChangeData> V16 = schema(
|
||||
ChangeField.LEGACY_ID,
|
||||
ChangeField.ID,
|
||||
@@ -161,7 +162,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -173,6 +174,7 @@ public class ChangeSchemas {
|
||||
ChangeField.HASHTAG,
|
||||
ChangeField.COMMENTBY);
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static final Schema<ChangeData> V17 = schema(
|
||||
ChangeField.LEGACY_ID,
|
||||
ChangeField.ID,
|
||||
@@ -189,7 +191,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -202,6 +204,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMENTBY,
|
||||
ChangeField.PATCH_SET);
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static final Schema<ChangeData> V18 = schema(
|
||||
ChangeField.LEGACY_ID,
|
||||
ChangeField.ID,
|
||||
@@ -218,7 +221,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -232,6 +235,7 @@ public class ChangeSchemas {
|
||||
ChangeField.PATCH_SET,
|
||||
ChangeField.GROUP);
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static final Schema<ChangeData> V19 = schema(
|
||||
ChangeField.LEGACY_ID,
|
||||
ChangeField.ID,
|
||||
@@ -248,7 +252,7 @@ public class ChangeSchemas {
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.REVIEWED,
|
||||
ChangeField.LEGACY_REVIEWED,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
@@ -263,6 +267,37 @@ public class ChangeSchemas {
|
||||
ChangeField.GROUP,
|
||||
ChangeField.EDITBY);
|
||||
|
||||
static final Schema<ChangeData> V20 = schema(
|
||||
ChangeField.LEGACY_ID,
|
||||
ChangeField.ID,
|
||||
ChangeField.STATUS,
|
||||
ChangeField.PROJECT,
|
||||
ChangeField.PROJECTS,
|
||||
ChangeField.REF,
|
||||
ChangeField.TOPIC,
|
||||
ChangeField.UPDATED,
|
||||
ChangeField.FILE_PART,
|
||||
ChangeField.PATH,
|
||||
ChangeField.OWNER,
|
||||
ChangeField.REVIEWER,
|
||||
ChangeField.COMMIT,
|
||||
ChangeField.TR,
|
||||
ChangeField.LABEL,
|
||||
ChangeField.COMMIT_MESSAGE,
|
||||
ChangeField.COMMENT,
|
||||
ChangeField.CHANGE,
|
||||
ChangeField.APPROVAL,
|
||||
ChangeField.MERGEABLE,
|
||||
ChangeField.ADDED,
|
||||
ChangeField.DELETED,
|
||||
ChangeField.DELTA,
|
||||
ChangeField.HASHTAG,
|
||||
ChangeField.COMMENTBY,
|
||||
ChangeField.PATCH_SET,
|
||||
ChangeField.GROUP,
|
||||
ChangeField.EDITBY,
|
||||
ChangeField.REVIEWEDBY);
|
||||
|
||||
private static Schema<ChangeData> schema(Collection<FieldDef<ChangeData, ?>> fields) {
|
||||
return new Schema<>(ImmutableList.copyOf(fields));
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ package com.google.gerrit.server.query.change;
|
||||
|
||||
import static com.google.gerrit.server.ApprovalsUtil.sortApprovals;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.common.base.MoreObjects;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Iterables;
|
||||
@@ -69,11 +70,13 @@ import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.sql.Timestamp;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
@@ -230,6 +233,7 @@ public class ChangeData {
|
||||
private ChangedLines changedLines;
|
||||
private Boolean mergeable;
|
||||
private Set<Account.Id> editsByUser;
|
||||
private Set<Account.Id> reviewedBy;
|
||||
|
||||
@AssistedInject
|
||||
private ChangeData(
|
||||
@@ -686,6 +690,55 @@ public class ChangeData {
|
||||
return editsByUser;
|
||||
}
|
||||
|
||||
public Set<Account.Id> reviewedBy() throws OrmException {
|
||||
if (reviewedBy == null) {
|
||||
Change c = change();
|
||||
if (c == null) {
|
||||
return Collections.emptySet();
|
||||
}
|
||||
List<ReviewedByEvent> events = new ArrayList<>();
|
||||
for (ChangeMessage msg : messages()) {
|
||||
if (msg.getAuthor() != null) {
|
||||
events.add(ReviewedByEvent.create(msg));
|
||||
}
|
||||
}
|
||||
for (PatchSet ps : patchSets()) {
|
||||
events.add(ReviewedByEvent.create(ps));
|
||||
}
|
||||
Collections.sort(events, Collections.reverseOrder());
|
||||
reviewedBy = new LinkedHashSet<>();
|
||||
Account.Id owner = c.getOwner();
|
||||
for (ReviewedByEvent event : events) {
|
||||
if (owner.equals(event.author())) {
|
||||
break;
|
||||
}
|
||||
reviewedBy.add(event.author());
|
||||
}
|
||||
}
|
||||
return reviewedBy;
|
||||
}
|
||||
|
||||
@AutoValue
|
||||
abstract static class ReviewedByEvent implements Comparable<ReviewedByEvent> {
|
||||
private static ReviewedByEvent create(PatchSet ps) {
|
||||
return new AutoValue_ChangeData_ReviewedByEvent(
|
||||
ps.getUploader(), ps.getCreatedOn());
|
||||
}
|
||||
|
||||
private static ReviewedByEvent create(ChangeMessage msg) {
|
||||
return new AutoValue_ChangeData_ReviewedByEvent(
|
||||
msg.getAuthor(), msg.getWrittenOn());
|
||||
}
|
||||
|
||||
public abstract Account.Id author();
|
||||
public abstract Timestamp ts();
|
||||
|
||||
@Override
|
||||
public int compareTo(ReviewedByEvent other) {
|
||||
return ts().compareTo(other.ts());
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
MoreObjects.ToStringHelper h = MoreObjects.toStringHelper(this);
|
||||
|
||||
@@ -117,6 +117,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
public static final String FIELD_PROJECTS = "projects";
|
||||
public static final String FIELD_QUERY = "query";
|
||||
public static final String FIELD_REF = "ref";
|
||||
public static final String FIELD_REVIEWEDBY = "reviewedby";
|
||||
public static final String FIELD_REVIEWER = "reviewer";
|
||||
public static final String FIELD_REVIEWERIN = "reviewerin";
|
||||
public static final String FIELD_STARREDBY = "starredby";
|
||||
@@ -363,7 +364,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
@Operator
|
||||
public Predicate<ChangeData> status(String statusName) {
|
||||
if ("reviewed".equalsIgnoreCase(statusName)) {
|
||||
return new IsReviewedPredicate();
|
||||
return IsReviewedPredicate.create(args.getSchema());
|
||||
} else {
|
||||
return ChangeStatusPredicate.parse(statusName);
|
||||
}
|
||||
@@ -404,7 +405,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
}
|
||||
|
||||
if ("reviewed".equalsIgnoreCase(value)) {
|
||||
return new IsReviewedPredicate();
|
||||
return IsReviewedPredicate.create(args.getSchema());
|
||||
}
|
||||
|
||||
if ("owner".equalsIgnoreCase(value)) {
|
||||
@@ -803,6 +804,12 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
throw new QueryParseException("Unknown named query: " + name);
|
||||
}
|
||||
|
||||
@Operator
|
||||
public Predicate<ChangeData> reviewedby(String who)
|
||||
throws QueryParseException, OrmException {
|
||||
return IsReviewedPredicate.create(args.getSchema(), parseAccount(who));
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Predicate<ChangeData> defaultField(String query) throws QueryParseException {
|
||||
if (query.startsWith("refs/")) {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// Copyright (C) 2010 The Android Open Source Project
|
||||
// Copyright (C) 2015 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.
|
||||
@@ -14,42 +14,69 @@
|
||||
|
||||
package com.google.gerrit.server.query.change;
|
||||
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.server.index.ChangeField;
|
||||
import com.google.gerrit.server.index.IndexPredicate;
|
||||
import com.google.gerrit.server.index.Schema;
|
||||
import com.google.gerrit.server.query.Predicate;
|
||||
import com.google.gerrit.server.query.QueryParseException;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
class IsReviewedPredicate extends IndexPredicate<ChangeData> {
|
||||
IsReviewedPredicate() {
|
||||
super(ChangeField.REVIEWED, "1");
|
||||
private static final Account.Id NOT_REVIEWED =
|
||||
new Account.Id(ChangeField.NOT_REVIEWED);
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static Predicate<ChangeData> create(Schema<ChangeData> schema) {
|
||||
if (schema == null || schema.hasField(ChangeField.LEGACY_REVIEWED)) {
|
||||
return new LegacyIsReviewedPredicate();
|
||||
}
|
||||
checkSchema(schema);
|
||||
return Predicate.not(new IsReviewedPredicate(NOT_REVIEWED));
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
static Predicate<ChangeData> create(Schema<ChangeData> schema,
|
||||
Collection<Account.Id> ids) throws QueryParseException {
|
||||
if (schema == null || schema.hasField(ChangeField.LEGACY_REVIEWED)) {
|
||||
throw new QueryParseException("Only is:reviewed is supported");
|
||||
}
|
||||
checkSchema(schema);
|
||||
List<Predicate<ChangeData>> predicates = new ArrayList<>(ids.size());
|
||||
for (Account.Id id : ids) {
|
||||
predicates.add(new IsReviewedPredicate(id));
|
||||
}
|
||||
return Predicate.or(predicates);
|
||||
}
|
||||
|
||||
private static void checkSchema(Schema<ChangeData> schema) {
|
||||
checkArgument(schema.hasField(ChangeField.REVIEWEDBY),
|
||||
"Schema %s missing field %s",
|
||||
schema.getVersion(), ChangeField.REVIEWEDBY.getName());
|
||||
}
|
||||
|
||||
private final Account.Id id;
|
||||
|
||||
private IsReviewedPredicate(Account.Id id) {
|
||||
super(ChangeField.REVIEWEDBY, Integer.toString(id.get()));
|
||||
this.id = id;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean match(final ChangeData object) throws OrmException {
|
||||
Change c = object.change();
|
||||
if (c == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
PatchSet.Id current = c.currentPatchSetId();
|
||||
for (PatchSetApproval p : object.approvals().get(current)) {
|
||||
if (p.getValue() != 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
public boolean match(ChangeData cd) throws OrmException {
|
||||
Set<Account.Id> reviewedBy = cd.reviewedBy();
|
||||
return !reviewedBy.isEmpty() ? reviewedBy.contains(id) : id == NOT_REVIEWED;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int getCost() {
|
||||
return 2;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "is:reviewed";
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
// Copyright (C) 2010 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 com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
||||
import com.google.gerrit.server.index.ChangeField;
|
||||
import com.google.gerrit.server.index.IndexPredicate;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
|
||||
@Deprecated
|
||||
class LegacyIsReviewedPredicate extends IndexPredicate<ChangeData> {
|
||||
@Deprecated
|
||||
LegacyIsReviewedPredicate() {
|
||||
super(ChangeField.LEGACY_REVIEWED, "1");
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean match(final ChangeData object) throws OrmException {
|
||||
Change c = object.change();
|
||||
if (c == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
PatchSet.Id current = c.currentPatchSetId();
|
||||
for (PatchSetApproval p : object.approvals().get(current)) {
|
||||
if (p.getValue() != 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int getCost() {
|
||||
return 2;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "is:reviewed";
|
||||
}
|
||||
}
|
||||
@@ -42,6 +42,7 @@ import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.Branch;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.Patch;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
@@ -50,7 +51,10 @@ import com.google.gerrit.server.account.AccountManager;
|
||||
import com.google.gerrit.server.account.AuthRequest;
|
||||
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.change.PatchSetInserter.ValidatePolicy;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.project.ProjectControl;
|
||||
import com.google.gerrit.server.schema.SchemaCreator;
|
||||
import com.google.gerrit.server.util.RequestContext;
|
||||
@@ -106,6 +110,8 @@ public abstract class AbstractQueryChangesTest {
|
||||
@ConfigSuite.Parameter public Config config;
|
||||
@Inject protected AccountManager accountManager;
|
||||
@Inject protected ChangeInserter.Factory changeFactory;
|
||||
@Inject protected PatchSetInserter.Factory patchSetFactory;
|
||||
@Inject protected ChangeControl.GenericFactory changeControlFactory;
|
||||
@Inject protected GerritApi gApi;
|
||||
@Inject protected IdentifiedUser.GenericFactory userFactory;
|
||||
@Inject protected InMemoryDatabase schemaFactory;
|
||||
@@ -144,7 +150,7 @@ public abstract class AbstractQueryChangesTest {
|
||||
requestContext.setContext(newRequestContext(userAccount.getId()));
|
||||
}
|
||||
|
||||
private RequestContext newRequestContext(Account.Id requestUserId) {
|
||||
protected RequestContext newRequestContext(Account.Id requestUserId) {
|
||||
final CurrentUser requestUser =
|
||||
userFactory.create(Providers.of(db), requestUserId);
|
||||
return new RequestContext() {
|
||||
@@ -1063,6 +1069,44 @@ public abstract class AbstractQueryChangesTest {
|
||||
assertQuery("conflicts:" + change4.getId().get());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void reviewedBy() throws Exception {
|
||||
clockStepMs = MILLISECONDS.convert(2, MINUTES);
|
||||
TestRepository<Repo> repo = createProject("repo");
|
||||
Change change1 = newChange(repo, null, null, null, null).insert();
|
||||
Change change2 = newChange(repo, null, null, null, null).insert();
|
||||
Change change3 = newChange(repo, null, null, null, null).insert();
|
||||
|
||||
gApi.changes()
|
||||
.id(change1.getId().get())
|
||||
.current()
|
||||
.review(new ReviewInput().message("comment"));
|
||||
|
||||
Account.Id user2 = accountManager
|
||||
.authenticate(AuthRequest.forUser("anotheruser"))
|
||||
.getAccountId();
|
||||
requestContext.setContext(newRequestContext(user2));
|
||||
|
||||
gApi.changes()
|
||||
.id(change2.getId().get())
|
||||
.current()
|
||||
.review(new ReviewInput().message("comment"));
|
||||
|
||||
PatchSet.Id ps3_1 = change3.currentPatchSetId();
|
||||
change3 = newPatchSet(repo, change3);
|
||||
assertThat(change3.currentPatchSetId()).isNotEqualTo(ps3_1);
|
||||
// Response to previous patch set still counts as reviewing.
|
||||
gApi.changes()
|
||||
.id(change3.getId().get())
|
||||
.revision(ps3_1.get())
|
||||
.review(new ReviewInput().message("comment"));
|
||||
|
||||
assertQuery("is:reviewed", change3, change2);
|
||||
assertQuery("-is:reviewed", change1);
|
||||
assertQuery("reviewedby:" + userId.get());
|
||||
assertQuery("reviewedby:" + user2.get(), change3, change2);
|
||||
}
|
||||
|
||||
protected ChangeInserter newChange(
|
||||
TestRepository<Repo> repo,
|
||||
@Nullable RevCommit commit, @Nullable String key, @Nullable Integer owner,
|
||||
@@ -1099,6 +1143,25 @@ public abstract class AbstractQueryChangesTest {
|
||||
commit);
|
||||
}
|
||||
|
||||
protected Change newPatchSet(TestRepository<Repo> repo, Change c)
|
||||
throws Exception {
|
||||
// Add a new file so the patch set is not a trivial rebase, to avoid default
|
||||
// Code-Review label copying.
|
||||
int n = c.currentPatchSetId().get() + 1;
|
||||
RevCommit commit = repo.parseBody(
|
||||
repo.commit()
|
||||
.message("message")
|
||||
.add("file" + n, "contents " + n)
|
||||
.create());
|
||||
ChangeControl ctl = changeControlFactory.controlFor(c.getId(), user);
|
||||
return patchSetFactory.create(
|
||||
repo.getRepository(), repo.getRevWalk(), ctl, commit)
|
||||
.setSendMail(false)
|
||||
.setRunHooks(false)
|
||||
.setValidatePolicy(ValidatePolicy.NONE)
|
||||
.insert();
|
||||
}
|
||||
|
||||
protected void assertBadQuery(Object query) throws Exception {
|
||||
assertBadQuery(newQuery(query));
|
||||
}
|
||||
|
||||
@@ -14,10 +14,21 @@
|
||||
|
||||
package com.google.gerrit.server.query.change;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static java.util.concurrent.TimeUnit.MILLISECONDS;
|
||||
import static java.util.concurrent.TimeUnit.MINUTES;
|
||||
|
||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.server.account.AuthRequest;
|
||||
import com.google.gerrit.testutil.InMemoryModule;
|
||||
import com.google.gerrit.testutil.InMemoryRepositoryManager.Repo;
|
||||
import com.google.inject.Guice;
|
||||
import com.google.inject.Injector;
|
||||
|
||||
import org.eclipse.jgit.junit.TestRepository;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Test;
|
||||
@@ -52,4 +63,47 @@ public class LuceneQueryChangesV14Test extends LuceneQueryChangesTest {
|
||||
public void byTopic() {
|
||||
// Ignore.
|
||||
}
|
||||
|
||||
@Override
|
||||
@Ignore
|
||||
@Test
|
||||
public void reviewedBy() throws Exception {
|
||||
// Ignore.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void isReviewed() throws Exception {
|
||||
clockStepMs = MILLISECONDS.convert(2, MINUTES);
|
||||
TestRepository<Repo> repo = createProject("repo");
|
||||
Change change1 = newChange(repo, null, null, null, null).insert();
|
||||
Change change2 = newChange(repo, null, null, null, null).insert();
|
||||
Change change3 = newChange(repo, null, null, null, null).insert();
|
||||
|
||||
gApi.changes()
|
||||
.id(change1.getId().get())
|
||||
.current()
|
||||
.review(new ReviewInput().message("comment"));
|
||||
|
||||
Account.Id user2 = accountManager
|
||||
.authenticate(AuthRequest.forUser("anotheruser"))
|
||||
.getAccountId();
|
||||
requestContext.setContext(newRequestContext(user2));
|
||||
|
||||
gApi.changes()
|
||||
.id(change2.getId().get())
|
||||
.current()
|
||||
.review(ReviewInput.recommend());
|
||||
|
||||
PatchSet.Id ps3_1 = change3.currentPatchSetId();
|
||||
change3 = newPatchSet(repo, change3);
|
||||
assertThat(change3.currentPatchSetId()).isNotEqualTo(ps3_1);
|
||||
// Nonzero score on previous patch set does not count.
|
||||
gApi.changes()
|
||||
.id(change3.getId().get())
|
||||
.revision(ps3_1.get())
|
||||
.review(ReviewInput.recommend());
|
||||
|
||||
assertQuery("is:reviewed", change2);
|
||||
assertQuery("-is:reviewed", change3, change1);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user