diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index bf580593a6..c0128c3ce2 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -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':: + diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index f659bcaf1a..d3fa8dad4f 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -405,6 +405,15 @@ class ElasticChangeIndex extends AbstractElasticIndex // 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; } diff --git a/java/com/google/gerrit/index/FieldDef.java b/java/com/google/gerrit/index/FieldDef.java index fb48104626..63f68879a8 100644 --- a/java/com/google/gerrit/index/FieldDef.java +++ b/java/com/google/gerrit/index/FieldDef.java @@ -91,7 +91,9 @@ public final class FieldDef { 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 getter; diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 495c7e5cdb..bf1a16645f 100644 --- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -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 doc, ChangeData cd) { + ChangeField.parseAttentionSet( + doc.get(ATTENTION_SET_FULL_FIELD).stream() + .map(field -> field.binaryValue().utf8ToString()) + .collect(toImmutableSet()), + cd); + } + private void decodeSubmitRecords( ListMultimap doc, String field, diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index e4148a57ac..b464104f82 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -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(), diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java index ffef684733..c6eb39843f 100644 --- a/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/java/com/google/gerrit/server/index/change/ChangeField.java @@ -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> 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> ATTENTION_SET_FULL = + storedOnly(ChangeQueryBuilder.FIELD_ATTENTION_SET_FULL) + .buildRepeatable(ChangeField::storedAttentionSet); + /** The user assigned to the change. */ public static final FieldDef ASSIGNEE = integer(ChangeQueryBuilder.FIELD_ASSIGNEE) @@ -463,6 +506,33 @@ public class ChangeField { return ReviewerByEmailSet.fromTable(b.build()); } + private static ImmutableSet getAttentionSetUserIds(ChangeData changeData) { + return additionsOnly(changeData.attentionSet()).stream() + .map(update -> update.account().get()) + .collect(toImmutableSet()); + } + + private static ImmutableSet 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 storedAttentionSetEntriesJson, ChangeData changeData) { + ImmutableSet 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> COMMIT = prefix(ChangeQueryBuilder.FIELD_COMMIT).buildRepeatable(ChangeField::getRevisions); diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java index 6d6d4b8c37..b232d3979a 100644 --- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java +++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java @@ -85,12 +85,17 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { 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 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 V57 = new Schema.Builder() @@ -100,7 +105,11 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { .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 V58 = new Schema.Builder() .add(V57) @@ -108,6 +117,17 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { .add(ChangeField.CHERRY_PICK_OF_PATCHSET) .build(); + /** + * Added new fields {@link ChangeField#ATTENTION_SET_USERS} and {@link + * ChangeField#ATTENTION_SET_FULL}. + */ + static final Schema V59 = + new Schema.Builder() + .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. */ diff --git a/java/com/google/gerrit/server/query/change/AttentionSetPredicate.java b/java/com/google/gerrit/server/query/change/AttentionSetPredicate.java new file mode 100644 index 0000000000..2b18767a7a --- /dev/null +++ b/java/com/google/gerrit/server/query/change/AttentionSetPredicate.java @@ -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; + } +} diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java index 0b54335c78..6e55bf2552 100644 --- a/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/java/com/google/gerrit/server/query/change/ChangeData.java @@ -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 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 patchSets() { if (patchSets == null) { diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index edd5411215..f5cb458e4c 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -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 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 attention(Set who) { + return Predicate.or(who.stream().map(AttentionSetPredicate::new).collect(toImmutableSet())); + } + @Operator public Predicate assignee(String who) throws QueryParseException, IOException, ConfigInvalidException { diff --git a/java/com/google/gerrit/server/restapi/change/GetAttentionSet.java b/java/com/google/gerrit/server/restapi/change/GetAttentionSet.java index b0dcf8bf5e..08a963bc43 100644 --- a/java/com/google/gerrit/server/restapi/change/GetAttentionSet.java +++ b/java/com/google/gerrit/server/restapi/change/GetAttentionSet.java @@ -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 { throws PermissionBackendException { AccountLoader accountLoader = accountLoaderFactory.create(true); ImmutableSet 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( diff --git a/java/com/google/gerrit/server/util/AttentionSetUtil.java b/java/com/google/gerrit/server/util/AttentionSetUtil.java new file mode 100644 index 0000000000..1cd398fe7d --- /dev/null +++ b/java/com/google/gerrit/server/util/AttentionSetUtil.java @@ -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 additionsOnly( + Collection updates) { + return updates.stream() + .filter(u -> u.operation() == Operation.ADD) + .collect(ImmutableSet.toImmutableSet()); + } + + private AttentionSetUtil() {} +} diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 040e2ebfb0..29b9f4f5c3 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -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 = 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 = 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 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 = createProject("repo");