Merge "Label-predicate: Add support for change owner votes"

This commit is contained in:
Dave Borowitz
2016-09-22 15:49:03 +00:00
committed by Gerrit Code Review
7 changed files with 103 additions and 25 deletions

View File

@@ -482,6 +482,12 @@ Matches changes with a +2 code review where the reviewer or group is aname.
+
Matches changes with a +2 code review where the reviewer is jsmith.
`label:Code-Review=+2,user=owner`::
`label:Code-Review=+2,owner`::
+
The special "owner" parameter corresponds to the change owner. Matches
all changes that have a +2 vote from the change owner.
`label:Code-Review=+1,group=ldap/linux.workflow`::
+
Matches changes with a +1 code review where the reviewer is in the
@@ -499,7 +505,6 @@ Matches changes that are ready to be submitted.
+
Changes that are blocked from submission due to a blocking score.
== Magical Operators
Most of these operators exist to support features of Gerrit Code

View File

@@ -429,26 +429,47 @@ public class ChangeField {
};
/** List of labels on the current patch set. */
@Deprecated
public static final FieldDef<ChangeData, Iterable<String>> LABEL =
new FieldDef.Repeatable<ChangeData, String>(
ChangeQueryBuilder.FIELD_LABEL, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
Set<String> allApprovals = new HashSet<>();
Set<String> distinctApprovals = new HashSet<>();
for (PatchSetApproval a : input.currentApprovals()) {
if (a.getValue() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.getLabel(), a.getValue(),
a.getAccountId()));
distinctApprovals.add(formatLabel(a.getLabel(), a.getValue()));
}
}
allApprovals.addAll(distinctApprovals);
return allApprovals;
return getLabels(input, false);
}
};
/** List of labels on the current patch set including change owner votes. */
public static final FieldDef<ChangeData, Iterable<String>> LABEL2 =
new FieldDef.Repeatable<ChangeData, String>(
"label2", FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
return getLabels(input, true);
}
};
private static Iterable<String> getLabels(ChangeData input, boolean owners)
throws OrmException {
Set<String> allApprovals = new HashSet<>();
Set<String> distinctApprovals = new HashSet<>();
for (PatchSetApproval a : input.currentApprovals()) {
if (a.getValue() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.getLabel(), a.getValue(),
a.getAccountId()));
if (owners && input.change().getOwner().equals(a.getAccountId())) {
allApprovals.add(formatLabel(a.getLabel(), a.getValue(),
ChangeQueryBuilder.OWNER_ACCOUNT_ID));
}
distinctApprovals.add(formatLabel(a.getLabel(), a.getValue()));
}
}
allApprovals.addAll(distinctApprovals);
return allApprovals;
}
public static Set<String> getAuthorParts(ChangeData cd) throws OrmException {
try {
return SchemaUtil.getPersonParts(cd.getAuthor());
@@ -544,7 +565,14 @@ public class ChangeField {
public static String formatLabel(String label, int value, Account.Id accountId) {
return label.toLowerCase() + (value >= 0 ? "+" : "") + value
+ (accountId != null ? "," + accountId.get() : "");
+ (accountId != null ? "," + formatAccount(accountId) : "");
}
private static String formatAccount(Account.Id accountId) {
if (ChangeQueryBuilder.OWNER_ACCOUNT_ID.equals(accountId)) {
return ChangeQueryBuilder.ARG_ID_OWNER;
}
return Integer.toString(accountId.get());
}
/** Commit message of the current patch set. */

View File

@@ -88,9 +88,17 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
.add(ChangeField.REVIEWER)
.build();
@Deprecated
static final Schema<ChangeData> V33 =
schema(V32, ChangeField.ASSIGNEE);
@SuppressWarnings("deprecation")
static final Schema<ChangeData> V34 = new Schema.Builder<ChangeData>()
.add(V33)
.remove(ChangeField.LABEL)
.add(ChangeField.LABEL2)
.build();
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE =
new ChangeSchemaDefinitions();

View File

@@ -152,7 +152,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String ARG_ID_USER = "user";
public static final String ARG_ID_GROUP = "group";
public static final String ARG_ID_OWNER = "owner";
public static final Account.Id OWNER_ACCOUNT_ID = new Account.Id(0);
private static final QueryBuilder.Definition<ChangeData, ChangeQueryBuilder> mydef =
new QueryBuilder.Definition<>(ChangeQueryBuilder.class);
@@ -600,6 +601,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
// label:CodeReview=1,group=android_approvers or
// label:CodeReview=1,android_approvers
// user/groups without a label will first attempt to match user
// Special case: votes by owners can be tracked with ",owner":
// label:Code-Review+2,owner
// label:Code-Review+2,user=owner
String[] splitReviewer = name.split(",", 2);
name = splitReviewer[0]; // remove all but the vote piece, e.g.'CodeReview=1'
@@ -609,7 +613,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
for (Map.Entry<String, String> pair : lblArgs.keyValue.entrySet()) {
if (pair.getKey().equalsIgnoreCase(ARG_ID_USER)) {
accounts = parseAccount(pair.getValue());
if (pair.getValue().equals(ARG_ID_OWNER)) {
accounts = Collections.singleton(OWNER_ACCOUNT_ID);
} else {
accounts = parseAccount(pair.getValue());
}
} else if (pair.getKey().equalsIgnoreCase(ARG_ID_GROUP)) {
group = parseGroup(pair.getValue()).getUUID();
} else {
@@ -624,7 +632,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
value + ")");
}
try {
accounts = parseAccount(value);
if (value.equals(ARG_ID_OWNER)) {
accounts = Collections.singleton(OWNER_ACCOUNT_ID);
} else {
accounts = parseAccount(value);
}
} catch (QueryParseException qpex) {
// If it doesn't match an account, see if it matches a group
// (accounts get precedence)
@@ -652,9 +664,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
}
return new LabelPredicate(args.projectCache,
args.changeControlGenericFactory, args.userFactory, args.db,
name, accounts, group);
return new LabelPredicate(args, name, accounts, group);
}
@Operator

View File

@@ -43,7 +43,7 @@ class EqualsLabelPredicate extends ChangeIndexPredicate {
EqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal,
Account.Id account) {
super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
super(args.field, ChangeField.formatLabel(label, expVal, account));
this.ccFactory = args.ccFactory;
this.projectCache = args.projectCache;
this.userFactory = args.userFactory;

View File

@@ -19,6 +19,8 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.OrPredicate;
@@ -36,6 +38,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
private static final int MAX_LABEL_VALUE = 4;
static class Args {
final FieldDef<ChangeData, ?> field;
final ProjectCache projectCache;
final ChangeControl.GenericFactory ccFactory;
final IdentifiedUser.GenericFactory userFactory;
@@ -45,6 +48,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
final AccountGroup.UUID group;
private Args(
FieldDef<ChangeData, ?> field,
ProjectCache projectCache,
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory,
@@ -52,6 +56,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
String value,
Set<Account.Id> accounts,
AccountGroup.UUID group) {
this.field = field;
this.projectCache = projectCache;
this.ccFactory = ccFactory;
this.userFactory = userFactory;
@@ -76,11 +81,12 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
private final String value;
LabelPredicate(ProjectCache projectCache,
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
String value, Set<Account.Id> accounts, AccountGroup.UUID group) {
super(predicates(new Args(projectCache, ccFactory, userFactory, dbProvider,
@SuppressWarnings("deprecation")
LabelPredicate(ChangeQueryBuilder.Arguments a, String value,
Set<Account.Id> accounts, AccountGroup.UUID group) {
super(predicates(new Args(
a.getSchema().getField(ChangeField.LABEL2, ChangeField.LABEL).get(),
a.projectCache, a.changeControlGenericFactory, a.userFactory, a.db,
value, accounts, group)));
this.value = value;
}

View File

@@ -619,6 +619,27 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("label:Code-Review=+1,user=user", reviewPlus1Change);
assertQuery("label:Code-Review=+1,Administrators", reviewPlus1Change);
assertQuery("label:Code-Review=+1,group=Administrators", reviewPlus1Change);
assertQuery("label:Code-Review=+1,user=owner", reviewPlus1Change);
assertQuery("label:Code-Review=+1,owner", reviewPlus1Change);
assertQuery("label:Code-Review=+2,owner", reviewPlus2Change);
assertQuery("label:Code-Review=-2,owner", reviewMinus2Change);
}
@Test
public void byLabelNotOwner() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
Account.Id user1 = createAccount("user1");
Change reviewPlus1Change = insert(repo, ins);
// post a review with user1
requestContext.setContext(newRequestContext(user1));
gApi.changes().id(reviewPlus1Change.getId().get()).current()
.review(ReviewInput.recommend());
assertQuery("label:Code-Review=+1,user=user1", reviewPlus1Change);
assertQuery("label:Code-Review=+1,owner");
}
private Change[] codeReviewInRange(Map<Integer, Change> changes, int start,