opendev/gerrit
Code Issues Proposed changes

Add attention set to index

Browse Source 
Change-Id: I22d2aa7b73cbbdf0d24cc4e49a0263705ea0229d
This commit is contained in:
Joerg Zieren
2020-03-24 14:41:20 +01:00
parent 6318b62300
commit 82b6f4f848
13 changed files with 258 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/user-search.txt
View File

@@ -73,6 +73,11 @@ assignee:'USER'::
+
Changes assigned to the given user.
[[attention]]
attention:'USER'::
+
Changes whose attention set includes the given user.
[[before_until]]
before:'TIME'/until:'TIME'::
+

9
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -405,6 +405,15 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
// Unresolved-comment-count.
decodeUnresolvedCommentCount(source, ChangeField.UNRESOLVED_COMMENT_COUNT.getName(), cd);
// Attention set.
if (fields.contains(ChangeField.ATTENTION_SET_FULL.getName())) {
ChangeField.parseAttentionSet(
FluentIterable.from(source.getAsJsonArray(ChangeField.ATTENTION_SET_FULL.getName()))
.transform(JsonElement::getAsString)
.toSet(),
cd);
}
return cd;
}

2
java/com/google/gerrit/index/FieldDef.java
View File

@@ -91,7 +91,9 @@ public final class FieldDef<I, T> {
private final String name;
private final FieldType<?> type;
/** Allow reading the actual data from the index. */
private final boolean stored;
private final boolean repeatable;
private final Getter<I, T> getter;

13
java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.lucene;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
@@ -138,6 +139,7 @@ public class LuceneChangeIndex implements ChangeIndex {
private static final String TOTAL_COMMENT_COUNT_FIELD = ChangeField.TOTAL_COMMENT_COUNT.getName();
private static final String UNRESOLVED_COMMENT_COUNT_FIELD =
ChangeField.UNRESOLVED_COMMENT_COUNT.getName();
private static final String ATTENTION_SET_FULL_FIELD = ChangeField.ATTENTION_SET_FULL.getName();
@FunctionalInterface
static interface IdTerm {
@@ -548,6 +550,9 @@ public class LuceneChangeIndex implements ChangeIndex {
if (fields.contains(PENDING_REVIEWER_BY_EMAIL_FIELD)) {
decodePendingReviewersByEmail(doc, cd);
}
if (fields.contains(ATTENTION_SET_FULL_FIELD)) {
decodeAttentionSet(doc, cd);
}
decodeSubmitRecords(
doc, SUBMIT_RECORD_STRICT_FIELD, ChangeField.SUBMIT_RULE_OPTIONS_STRICT, cd);
decodeSubmitRecords(
@@ -672,6 +677,14 @@ public class LuceneChangeIndex implements ChangeIndex {
.transform(IndexableField::stringValue)));
}
private void decodeAttentionSet(ListMultimap<String, IndexableField> doc, ChangeData cd) {
ChangeField.parseAttentionSet(
doc.get(ATTENTION_SET_FULL_FIELD).stream()
.map(field -> field.binaryValue().utf8ToString())
.collect(toImmutableSet()),
cd);
}
private void decodeSubmitRecords(
ListMultimap<String, IndexableField> doc,
String field,

7
java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -33,6 +33,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.SKIP_DIFFSTA
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS;
import static com.google.gerrit.server.ChangeMessagesUtil.createChangeMessageInfo;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Joiner;
@@ -50,7 +51,6 @@ import com.google.gerrit.common.data.SubmitRecord.Status;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
@@ -510,9 +510,8 @@ public class ChangeJson {
out.topic = in.getTopic();
if (!cd.attentionSet().isEmpty()) {
out.attentionSet =
cd.attentionSet().stream()
// This filtering should match GetAttentionSet.
.filter(a -> a.operation() == AttentionSetUpdate.Operation.ADD)
// This filtering should match GetAttentionSet.
additionsOnly(cd.attentionSet()).stream()
.collect(
toImmutableMap(
a -> a.account().get(),

70
java/com/google/gerrit/server/index/change/ChangeField.java
View File

@@ -24,6 +24,7 @@ import static com.google.gerrit.index.FieldDef.integer;
import static com.google.gerrit.index.FieldDef.prefix;
import static com.google.gerrit.index.FieldDef.storedOnly;
import static com.google.gerrit.index.FieldDef.timestamp;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
@@ -45,6 +46,8 @@ import com.google.common.primitives.Longs;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSetApproval;
@@ -74,6 +77,7 @@ import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeStatusPredicate;
import com.google.gson.Gson;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
@@ -288,6 +292,45 @@ public class ChangeField {
? cd.change().getCherryPickOf().get()
: null);
/** This class decouples the internal and API types from storage. */
private static class StoredAttentionSetEntry {
final long timestampMillis;
final int userId;
final String reason;
final Operation operation;
StoredAttentionSetEntry(AttentionSetUpdate attentionSetUpdate) {
timestampMillis = attentionSetUpdate.timestamp().toEpochMilli();
userId = attentionSetUpdate.account().get();
reason = attentionSetUpdate.reason();
operation = attentionSetUpdate.operation();
}
AttentionSetUpdate toAttentionSetUpdate() {
return AttentionSetUpdate.createFromRead(
Instant.ofEpochMilli(timestampMillis), Account.id(userId), operation, reason);
}
}
/**
* Users included in the attention set of the change. This omits timestamp, reason and possible
* future fields.
*
* @see #ATTENTION_SET_FULL
*/
public static final FieldDef<ChangeData, Iterable<Integer>> ATTENTION_SET_USERS =
integer(ChangeQueryBuilder.FIELD_ATTENTION_SET_USERS)
.buildRepeatable(ChangeField::getAttentionSetUserIds);
/**
* The full attention set data including timestamp, reason and possible future fields.
*
* @see #ATTENTION_SET_USERS
*/
public static final FieldDef<ChangeData, Iterable<byte[]>> ATTENTION_SET_FULL =
storedOnly(ChangeQueryBuilder.FIELD_ATTENTION_SET_FULL)
.buildRepeatable(ChangeField::storedAttentionSet);
/** The user assigned to the change. */
public static final FieldDef<ChangeData, Integer> ASSIGNEE =
integer(ChangeQueryBuilder.FIELD_ASSIGNEE)
@@ -463,6 +506,33 @@ public class ChangeField {
return ReviewerByEmailSet.fromTable(b.build());
}
private static ImmutableSet<Integer> getAttentionSetUserIds(ChangeData changeData) {
return additionsOnly(changeData.attentionSet()).stream()
.map(update -> update.account().get())
.collect(toImmutableSet());
}
private static ImmutableSet<byte[]> storedAttentionSet(ChangeData changeData) {
return changeData.attentionSet().stream()
.map(StoredAttentionSetEntry::new)
.map(storedAttentionSetEntry -> GSON.toJson(storedAttentionSetEntry).getBytes(UTF_8))
.collect(toImmutableSet());
}
/**
* Deserializes the specified attention set entries from JSON and stores them in the specified
* change.
*/
public static void parseAttentionSet(
Collection<String> storedAttentionSetEntriesJson, ChangeData changeData) {
ImmutableSet<AttentionSetUpdate> attentionSet =
storedAttentionSetEntriesJson.stream()
.map(
entry -> GSON.fromJson(entry, StoredAttentionSetEntry.class).toAttentionSetUpdate())
.collect(toImmutableSet());
changeData.setAttentionSet(attentionSet);
}
/** 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);

30
java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
View File

@@ -85,12 +85,17 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
ChangeField.UPDATED,
ChangeField.WIP);
// The computation of the 'extension' field is changed, hence reindexing is required.
/**
* The computation of the {@link ChangeField#EXTENSION} field is changed, hence reindexing is
* required.
*/
@Deprecated static final Schema<ChangeData> V56 = schema(V55);
// New numeric types: use dimensional points using the k-d tree geo-spatial data structure
// to offer fast single- and multi-dimensional numeric range. As the consequense, integer
// document id type is replaced with string document id type.
/**
* New numeric types: use dimensional points using the k-d tree geo-spatial data structure to
* offer fast single- and multi-dimensional numeric range. As the consequense, {@link
* ChangeField#LEGACY_ID} is replaced with {@link ChangeField#LEGACY_ID_STR}.
*/
@Deprecated
static final Schema<ChangeData> V57 =
new Schema.Builder<ChangeData>()
@@ -100,7 +105,11 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
.legacyNumericFields(false)
.build();
// Add new field CHERRY_PICK_OF
/**
* Added new fields {@link ChangeField#CHERRY_PICK_OF_CHANGE} and {@link
* ChangeField#CHERRY_PICK_OF_PATCHSET}.
*/
@Deprecated
static final Schema<ChangeData> V58 =
new Schema.Builder<ChangeData>()
.add(V57)
@@ -108,6 +117,17 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
.add(ChangeField.CHERRY_PICK_OF_PATCHSET)
.build();
/**
* Added new fields {@link ChangeField#ATTENTION_SET_USERS} and {@link
* ChangeField#ATTENTION_SET_FULL}.
*/
static final Schema<ChangeData> V59 =
new Schema.Builder<ChangeData>()
.add(V58)
.add(ChangeField.ATTENTION_SET_USERS)
.add(ChangeField.ATTENTION_SET_FULL)
.build();
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/

41
java/com/google/gerrit/server/query/change/AttentionSetPredicate.java Normal file
View File

@@ -0,0 +1,41 @@
// Copyright (C) 2020 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.gerrit.server.util.AttentionSetUtil.additionsOnly;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.index.change.ChangeField;
/** Simple predicate for searching by attention set. */
public class AttentionSetPredicate extends ChangeIndexPredicate {
protected final Account.Id id;
AttentionSetPredicate(Account.Id id) {
super(ChangeField.ATTENTION_SET_USERS, id.toString());
this.id = id;
}
@Override
public boolean match(ChangeData changeData) {
return additionsOnly(changeData.attentionSet()).stream()
.anyMatch(update -> update.account().equals(id));
}
@Override
public int getCost() {
return 1;
}
}

15
java/com/google/gerrit/server/query/change/ChangeData.java
View File

@@ -611,6 +611,21 @@ public class ChangeData {
return attentionSet;
}
/**
* Sets the specified attention set. If two or more entries refer to the same user, throws an
* {@link IllegalStateException}.
*/
public void setAttentionSet(ImmutableSet<AttentionSetUpdate> attentionSet) {
if (attentionSet.stream().map(AttentionSetUpdate::account).distinct().count()
!= attentionSet.size()) {
throw new IllegalStateException(
String.format(
"Stored attention set for change %d contains duplicate update",
change.getId().get()));
}
this.attentionSet = attentionSet;
}
/** @return patches for the change, in patch set ID order. */
public Collection<PatchSet> patchSets() {
if (patchSets == null) {

17
java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.entities.Change.CHANGE_ID_PATTERN;
import static com.google.gerrit.server.account.AccountResolver.isSelf;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
@@ -136,6 +137,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
public static final String FIELD_ADDED = "added";
public static final String FIELD_AGE = "age";
public static final String FIELD_ATTENTION_SET_USERS = "attentionusers";
public static final String FIELD_ATTENTION_SET_FULL = "attentionfull";
public static final String FIELD_ASSIGNEE = "assignee";
public static final String FIELD_AUTHOR = "author";
public static final String FIELD_EXACTAUTHOR = "exactauthor";
@@ -1056,6 +1059,20 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
return owner(accounts);
}
@Operator
public Predicate<ChangeData> attention(String who)
throws QueryParseException, IOException, ConfigInvalidException {
if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
throw new QueryParseException(
"'attention' operator is not supported by change index version");
}
return attention(parseAccount(who, (AccountState s) -> true));
}
private Predicate<ChangeData> attention(Set<Account.Id> who) {
return Predicate.or(who.stream().map(AttentionSetPredicate::new).collect(toImmutableSet()));
}
@Operator
public Predicate<ChangeData> assignee(String who)
throws QueryParseException, IOException, ConfigInvalidException {

7
java/com/google/gerrit/server/restapi/change/GetAttentionSet.java
View File

@@ -15,9 +15,9 @@
package com.google.gerrit.server.restapi.change;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.extensions.common.AttentionSetEntry;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -45,9 +45,8 @@ public class GetAttentionSet implements RestReadView<ChangeResource> {
throws PermissionBackendException {
AccountLoader accountLoader = accountLoaderFactory.create(true);
ImmutableSet<AttentionSetEntry> response =
changeResource.getNotes().getAttentionSet().stream()
// This filtering should match ChangeJson.
.filter(a -> a.operation() == Operation.ADD)
// This filtering should match ChangeJson.
additionsOnly(changeResource.getNotes().getAttentionSet()).stream()
.map(
a ->
new AttentionSetEntry(

19
java/com/google/gerrit/server/util/AttentionSetUtil.java Normal file
View File

@@ -0,0 +1,19 @@
package com.google.gerrit.server.util;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import java.util.Collection;
/** Common helpers for dealing with attention set data structures. */
public class AttentionSetUtil {
/** Returns only updates where the user was added. */
public static ImmutableSet<AttentionSetUpdate> additionsOnly(
Collection<AttentionSetUpdate> updates) {
return updates.stream()
.filter(u -> u.operation() == Operation.ADD)
.collect(ImmutableSet.toImmutableSet());
}
private AttentionSetUtil() {}
}

36
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -54,6 +54,7 @@ import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
@@ -2940,6 +2941,41 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
mergedOwned);
}
@Test
public void attentionSetIndexed() throws Exception {
assume().that(getSchema().hasField(ChangeField.ATTENTION_SET_USERS)).isTrue();
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "some reason");
gApi.changes().id(change1.getChangeId()).addToAttentionSet(input);
assertQuery("attention:" + user.getUserName().get(), change1);
assertQuery("-attention:" + userId.toString(), change2);
}
@Test
public void attentionSetStored() throws Exception {
assume().that(getSchema().hasField(ChangeField.ATTENTION_SET_USERS)).isTrue();
TestRepository<Repo> repo = createProject("repo");
Change change = insert(repo, newChange(repo));
AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "reason 1");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
Account.Id user2Id =
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
input = new AddToAttentionSetInput(user2Id.toString(), "reason 2");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
List<ChangeInfo> result = newQuery("attention:" + user2Id.toString()).get();
assertThat(result).hasSize(1);
ChangeInfo changeInfo = Iterables.getOnlyElement(result);
assertThat(changeInfo.attentionSet.keySet()).containsExactly(userId.get(), user2Id.get());
assertThat(changeInfo.attentionSet.get(userId.get()).reason).isEqualTo("reason 1");
assertThat(changeInfo.attentionSet.get(user2Id.get()).reason).isEqualTo("reason 2");
}
@Test
public void assignee() throws Exception {
TestRepository<Repo> repo = createProject("repo");