Read/write attention set in ChangeNotes
Context: https://www.gerritcodereview.com/design-docs/attention-set.html This change adds code for NoteDb to store and read attention sets. We considered storing this data in the file that also holds the comments (adding a dedicated file would require significant refactoring). This would better accomodate the structured attention set data, but this file is attached to a specific patch set whereas the attention set relates to the change as a whole. We agreed that the commit message is the better storage location, even when this is the first footer line that will have JSON in it and be potentially long. Change-Id: I9e48cf6c1bb6d6d8f28f9ca251cac2c3529af4c8
This commit is contained in:
Joerg Zieren
parent
8f2648461c
commit
361ea4b790
72
java/com/google/gerrit/entities/AttentionStatus.java
Normal file
72
java/com/google/gerrit/entities/AttentionStatus.java
Normal file
@ -0,0 +1,72 @@
|
||||
// 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.entities;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import java.time.Instant;
|
||||
|
||||
/**
|
||||
* A single update to the attention set. To reconstruct the attention set these instances are parsed
|
||||
* in reverse chronological order. Since each update contains all required information and
|
||||
* invalidates all previous state (hence the name -Status rather than -Update), only the most recent
|
||||
* record is relevant for each user.
|
||||
*
|
||||
* <p>See <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">here</a> for
|
||||
* details.
|
||||
*/
|
||||
@AutoValue
|
||||
public abstract class AttentionStatus {
|
||||
|
||||
/** Users can be added to or removed from the attention set. */
|
||||
public enum Operation {
|
||||
ADD,
|
||||
REMOVE
|
||||
}
|
||||
|
||||
/**
|
||||
* The time at which this status was set. This is null for instances to be written because the
|
||||
* timestamp in the commit message will be used.
|
||||
*/
|
||||
@Nullable
|
||||
public abstract Instant timestamp();
|
||||
|
||||
/** The user included in or excluded from the attention set. */
|
||||
public abstract Account.Id account();
|
||||
|
||||
/** Indicates whether the user is added to or removed from the attention set. */
|
||||
public abstract Operation operation();
|
||||
|
||||
/** A short human readable reason that explains this status (e.g. "manual"). */
|
||||
public abstract String reason();
|
||||
|
||||
/**
|
||||
* Create an instance from data read from NoteDB. This includes the timestamp taken from the
|
||||
* commit.
|
||||
*/
|
||||
public static AttentionStatus createFromRead(
|
||||
Instant timestamp, Account.Id account, Operation operation, String reason) {
|
||||
return new AutoValue_AttentionStatus(timestamp, account, operation, reason);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an instance to be written to NoteDB. This has no timestamp because the timestamp of the
|
||||
* commit will be used.
|
||||
*/
|
||||
public static AttentionStatus createForWrite(
|
||||
Account.Id account, Operation operation, String reason) {
|
||||
return new AutoValue_AttentionStatus(null, account, operation, reason);
|
||||
}
|
||||
}
|
||||
@ -110,7 +110,7 @@ public abstract class AbstractChangeUpdate {
|
||||
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
|
||||
checkUserType(u);
|
||||
if (u instanceof IdentifiedUser) {
|
||||
return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
|
||||
return noteUtil.newIdent(u.asIdentifiedUser().getAccount().id(), when, serverIdent);
|
||||
} else if (u instanceof InternalUser) {
|
||||
return serverIdent;
|
||||
}
|
||||
@ -164,10 +164,6 @@ public abstract class AbstractChangeUpdate {
|
||||
return accountId;
|
||||
}
|
||||
|
||||
protected PersonIdent newIdent(Account.Id authorId, Date when) {
|
||||
return noteUtil.newIdent(authorId, when, serverIdent);
|
||||
}
|
||||
|
||||
/** Whether no updates have been done. */
|
||||
public abstract boolean isEmpty();
|
||||
|
||||
|
||||
@ -15,10 +15,13 @@
|
||||
package com.google.gerrit.server.notedb;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.json.OutputFormat;
|
||||
import com.google.gerrit.server.config.GerritServerId;
|
||||
import com.google.gson.Gson;
|
||||
import com.google.inject.Inject;
|
||||
import java.time.Instant;
|
||||
import java.util.Date;
|
||||
import java.util.Optional;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
@ -27,42 +30,31 @@ import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.util.RawParseUtils;
|
||||
|
||||
public class ChangeNoteUtil {
|
||||
public static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
|
||||
public static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
|
||||
public static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
|
||||
public static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
|
||||
public static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
|
||||
public static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
|
||||
public static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
|
||||
public static final FooterKey FOOTER_LABEL = new FooterKey("Label");
|
||||
public static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
|
||||
public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
|
||||
new FooterKey("Patch-set-description");
|
||||
public static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
|
||||
public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
|
||||
public static final FooterKey FOOTER_STATUS = new FooterKey("Status");
|
||||
public static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
|
||||
public static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
|
||||
public static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
|
||||
public static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
|
||||
public static final FooterKey FOOTER_TAG = new FooterKey("Tag");
|
||||
public static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
|
||||
public static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
|
||||
public static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
|
||||
|
||||
static final String AUTHOR = "Author";
|
||||
static final String BASE_PATCH_SET = "Base-for-patch-set";
|
||||
static final String COMMENT_RANGE = "Comment-range";
|
||||
static final String FILE = "File";
|
||||
static final String LENGTH = "Bytes";
|
||||
static final String PARENT = "Parent";
|
||||
static final String PARENT_NUMBER = "Parent-number";
|
||||
static final String PATCH_SET = "Patch-set";
|
||||
static final String REAL_AUTHOR = "Real-author";
|
||||
static final String REVISION = "Revision";
|
||||
static final String UUID = "UUID";
|
||||
static final String UNRESOLVED = "Unresolved";
|
||||
static final String TAG = FOOTER_TAG.getName();
|
||||
static final FooterKey FOOTER_ATTENTION = new FooterKey("Attention");
|
||||
static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
|
||||
static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
|
||||
static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
|
||||
static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
|
||||
static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
|
||||
static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
|
||||
static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
|
||||
static final FooterKey FOOTER_LABEL = new FooterKey("Label");
|
||||
static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
|
||||
static final FooterKey FOOTER_PATCH_SET_DESCRIPTION = new FooterKey("Patch-set-description");
|
||||
static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
|
||||
static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
|
||||
static final FooterKey FOOTER_STATUS = new FooterKey("Status");
|
||||
static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
|
||||
static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
|
||||
static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
|
||||
static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
|
||||
static final FooterKey FOOTER_TAG = new FooterKey("Tag");
|
||||
static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
|
||||
static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
|
||||
static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
|
||||
|
||||
private static final Gson gson = OutputFormat.JSON_COMPACT.newGson();
|
||||
|
||||
private final ChangeNoteJson changeNoteJson;
|
||||
private final String serverId;
|
||||
@ -77,21 +69,17 @@ public class ChangeNoteUtil {
|
||||
return changeNoteJson;
|
||||
}
|
||||
|
||||
public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
|
||||
public PersonIdent newIdent(Account.Id accountId, Date when, PersonIdent serverIdent) {
|
||||
return new PersonIdent(
|
||||
"Gerrit User " + authorId.toString(),
|
||||
authorId.get() + "@" + serverId,
|
||||
when,
|
||||
serverIdent.getTimeZone());
|
||||
getUsername(accountId), getEmailAddress(accountId), when, serverIdent.getTimeZone());
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
|
||||
return new PersonIdent(
|
||||
"Gerrit User " + author.id(),
|
||||
author.id().get() + "@" + serverId,
|
||||
when,
|
||||
serverIdent.getTimeZone());
|
||||
private static String getUsername(Account.Id accountId) {
|
||||
return "Gerrit User " + accountId.toString();
|
||||
}
|
||||
|
||||
private String getEmailAddress(Account.Id accountId) {
|
||||
return accountId.get() + "@" + serverId;
|
||||
}
|
||||
|
||||
public static Optional<CommitMessageRange> parseCommitMessageRange(RevCommit commit) {
|
||||
@ -151,6 +139,7 @@ public class ChangeNoteUtil {
|
||||
|
||||
@AutoValue
|
||||
public abstract static class CommitMessageRange {
|
||||
|
||||
public abstract int subjectStart();
|
||||
|
||||
public abstract int subjectEnd();
|
||||
@ -165,6 +154,7 @@ public class ChangeNoteUtil {
|
||||
|
||||
@AutoValue.Builder
|
||||
public abstract static class Builder {
|
||||
|
||||
abstract Builder subjectStart(int subjectStart);
|
||||
|
||||
abstract Builder subjectEnd(int subjectEnd);
|
||||
@ -176,4 +166,51 @@ public class ChangeNoteUtil {
|
||||
abstract CommitMessageRange build();
|
||||
}
|
||||
}
|
||||
|
||||
/** Helper class for JSON serialization. Timestamp is taken from the commit. */
|
||||
private static class AttentionStatusInNoteDb {
|
||||
|
||||
final String personIdent;
|
||||
final AttentionStatus.Operation operation;
|
||||
final String reason;
|
||||
|
||||
AttentionStatusInNoteDb(
|
||||
String personIndent, AttentionStatus.Operation operation, String reason) {
|
||||
this.personIdent = personIndent;
|
||||
this.operation = operation;
|
||||
this.reason = reason;
|
||||
}
|
||||
}
|
||||
|
||||
/** The returned {@link Optional} holds the parsed entity or is empty if parsing failed. */
|
||||
static Optional<AttentionStatus> attentionStatusFromJson(
|
||||
Instant timestamp, String attentionString) {
|
||||
AttentionStatusInNoteDb inNoteDb =
|
||||
gson.fromJson(attentionString, AttentionStatusInNoteDb.class);
|
||||
PersonIdent personIdent = RawParseUtils.parsePersonIdent(inNoteDb.personIdent);
|
||||
if (personIdent == null) {
|
||||
return Optional.empty();
|
||||
}
|
||||
Optional<Account.Id> account = NoteDbUtil.parseIdent(personIdent);
|
||||
return account.map(
|
||||
id -> AttentionStatus.createFromRead(timestamp, id, inNoteDb.operation, inNoteDb.reason));
|
||||
}
|
||||
|
||||
String attentionStatusToJson(AttentionStatus attentionStatus) {
|
||||
PersonIdent personIdent =
|
||||
new PersonIdent(
|
||||
getUsername(attentionStatus.account()), getEmailAddress(attentionStatus.account()));
|
||||
StringBuilder stringBuilder = new StringBuilder();
|
||||
appendIdentString(stringBuilder, personIdent.getName(), personIdent.getEmailAddress());
|
||||
return gson.toJson(
|
||||
new AttentionStatusInNoteDb(
|
||||
stringBuilder.toString(), attentionStatus.operation(), attentionStatus.reason()));
|
||||
}
|
||||
|
||||
static void appendIdentString(StringBuilder stringBuilder, String name, String emailAddress) {
|
||||
PersonIdent.appendSanitized(stringBuilder, name);
|
||||
stringBuilder.append(" <");
|
||||
PersonIdent.appendSanitized(stringBuilder, emailAddress);
|
||||
stringBuilder.append('>');
|
||||
}
|
||||
}
|
||||
|
||||
@ -40,6 +40,7 @@ import com.google.common.flogger.FluentLogger;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.entities.BranchNameKey;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.ChangeMessage;
|
||||
@ -369,6 +370,10 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
|
||||
return state.reviewerUpdates();
|
||||
}
|
||||
|
||||
public ImmutableList<AttentionStatus> getAttentionUpdates() {
|
||||
return state.attentionUpdates();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. The
|
||||
* order of the set is the order in which they were assigned.
|
||||
|
||||
@ -16,6 +16,7 @@ package com.google.gerrit.server.notedb;
|
||||
|
||||
import static com.google.common.base.MoreObjects.firstNonNull;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
|
||||
@ -43,6 +44,7 @@ import static java.util.stream.Collectors.joining;
|
||||
import com.google.common.base.Enums;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.collect.HashBasedTable;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.collect.ImmutableTable;
|
||||
import com.google.common.collect.ListMultimap;
|
||||
@ -57,6 +59,7 @@ import com.google.common.primitives.Ints;
|
||||
import com.google.gerrit.common.data.LabelType;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.ChangeMessage;
|
||||
import com.google.gerrit.entities.Comment;
|
||||
@ -75,6 +78,7 @@ import com.google.gerrit.server.util.LabelVote;
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.sql.Timestamp;
|
||||
import java.time.Instant;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
@ -102,7 +106,6 @@ class ChangeNotesParser {
|
||||
|
||||
// Private final members initialized in the constructor.
|
||||
private final ChangeNoteJson changeNoteJson;
|
||||
|
||||
private final NoteDbMetrics metrics;
|
||||
private final Change.Id id;
|
||||
private final ObjectId tip;
|
||||
@ -114,6 +117,9 @@ class ChangeNotesParser {
|
||||
private final Table<Address, ReviewerStateInternal, Timestamp> reviewersByEmail;
|
||||
private final List<Account.Id> allPastReviewers;
|
||||
private final List<ReviewerStatusUpdate> reviewerUpdates;
|
||||
/** Holds only the most recent update per user. Older updates are discarded. */
|
||||
private final Map<Account.Id, AttentionStatus> latestAttentionStatus;
|
||||
|
||||
private final List<AssigneeStatusUpdate> assigneeUpdates;
|
||||
private final List<SubmitRecord> submitRecords;
|
||||
private final ListMultimap<ObjectId, Comment> comments;
|
||||
@ -169,6 +175,7 @@ class ChangeNotesParser {
|
||||
pendingReviewersByEmail = ReviewerByEmailSet.empty();
|
||||
allPastReviewers = new ArrayList<>();
|
||||
reviewerUpdates = new ArrayList<>();
|
||||
latestAttentionStatus = new HashMap<>();
|
||||
assigneeUpdates = new ArrayList<>();
|
||||
submitRecords = Lists.newArrayListWithExpectedSize(1);
|
||||
allChangeMessages = new ArrayList<>();
|
||||
@ -239,6 +246,7 @@ class ChangeNotesParser {
|
||||
pendingReviewersByEmail,
|
||||
allPastReviewers,
|
||||
buildReviewerUpdates(),
|
||||
ImmutableList.copyOf(latestAttentionStatus.values()),
|
||||
assigneeUpdates,
|
||||
submitRecords,
|
||||
buildAllMessages(),
|
||||
@ -359,6 +367,7 @@ class ChangeNotesParser {
|
||||
}
|
||||
|
||||
parseHashtags(commit);
|
||||
parseAttentionUpdates(commit);
|
||||
parseAssigneeUpdates(ts, commit);
|
||||
|
||||
if (submissionId == null) {
|
||||
@ -569,6 +578,21 @@ class ChangeNotesParser {
|
||||
}
|
||||
}
|
||||
|
||||
private void parseAttentionUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
|
||||
List<String> attentionStrings = commit.getFooterLineValues(FOOTER_ATTENTION);
|
||||
for (String attentionString : attentionStrings) {
|
||||
|
||||
Optional<AttentionStatus> attentionStatus =
|
||||
ChangeNoteUtil.attentionStatusFromJson(
|
||||
Instant.ofEpochSecond(commit.getCommitTime()), attentionString);
|
||||
if (!attentionStatus.isPresent()) {
|
||||
throw invalidFooter(FOOTER_ATTENTION, attentionString);
|
||||
}
|
||||
// Processing is in reverse chronological order. Keep only the latest update.
|
||||
latestAttentionStatus.putIfAbsent(attentionStatus.get().account(), attentionStatus.get());
|
||||
}
|
||||
}
|
||||
|
||||
private void parseAssigneeUpdates(Timestamp ts, ChangeNotesCommit commit)
|
||||
throws ConfigInvalidException {
|
||||
String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE);
|
||||
|
||||
@ -35,6 +35,7 @@ import com.google.common.collect.Table;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.entities.BranchNameKey;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.ChangeMessage;
|
||||
@ -55,6 +56,7 @@ import com.google.gerrit.server.ReviewerSet;
|
||||
import com.google.gerrit.server.ReviewerStatusUpdate;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
|
||||
@ -66,6 +68,7 @@ import com.google.gson.Gson;
|
||||
import com.google.protobuf.ByteString;
|
||||
import com.google.protobuf.MessageLite;
|
||||
import java.sql.Timestamp;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
@ -83,11 +86,12 @@ import org.eclipse.jgit.lib.ObjectId;
|
||||
*/
|
||||
@AutoValue
|
||||
public abstract class ChangeNotesState {
|
||||
|
||||
static ChangeNotesState empty(Change change) {
|
||||
return Builder.empty(change.getId()).build();
|
||||
}
|
||||
|
||||
static Builder builder() {
|
||||
private static Builder builder() {
|
||||
return new AutoValue_ChangeNotesState.Builder();
|
||||
}
|
||||
|
||||
@ -115,6 +119,7 @@ public abstract class ChangeNotesState {
|
||||
ReviewerByEmailSet pendingReviewersByEmail,
|
||||
List<Account.Id> allPastReviewers,
|
||||
List<ReviewerStatusUpdate> reviewerUpdates,
|
||||
List<AttentionStatus> attentionStatusUpdates,
|
||||
List<AssigneeStatusUpdate> assigneeUpdates,
|
||||
List<SubmitRecord> submitRecords,
|
||||
List<ChangeMessage> changeMessages,
|
||||
@ -165,6 +170,7 @@ public abstract class ChangeNotesState {
|
||||
.pendingReviewersByEmail(pendingReviewersByEmail)
|
||||
.allPastReviewers(allPastReviewers)
|
||||
.reviewerUpdates(reviewerUpdates)
|
||||
.attentionUpdates(attentionStatusUpdates)
|
||||
.assigneeUpdates(assigneeUpdates)
|
||||
.submitRecords(submitRecords)
|
||||
.changeMessages(changeMessages)
|
||||
@ -180,6 +186,7 @@ public abstract class ChangeNotesState {
|
||||
*/
|
||||
@AutoValue
|
||||
abstract static class ChangeColumns {
|
||||
|
||||
static Builder builder() {
|
||||
return new AutoValue_ChangeNotesState_ChangeColumns.Builder();
|
||||
}
|
||||
@ -229,6 +236,7 @@ public abstract class ChangeNotesState {
|
||||
|
||||
@AutoValue.Builder
|
||||
abstract static class Builder {
|
||||
|
||||
abstract Builder changeKey(Change.Key changeKey);
|
||||
|
||||
abstract Builder createdOn(Timestamp createdOn);
|
||||
@ -297,6 +305,8 @@ public abstract class ChangeNotesState {
|
||||
|
||||
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();
|
||||
|
||||
abstract ImmutableList<AttentionStatus> attentionUpdates();
|
||||
|
||||
abstract ImmutableList<AssigneeStatusUpdate> assigneeUpdates();
|
||||
|
||||
abstract ImmutableList<SubmitRecord> submitRecords();
|
||||
@ -361,6 +371,7 @@ public abstract class ChangeNotesState {
|
||||
|
||||
@AutoValue.Builder
|
||||
abstract static class Builder {
|
||||
|
||||
static Builder empty(Change.Id changeId) {
|
||||
return new AutoValue_ChangeNotesState.Builder()
|
||||
.changeId(changeId)
|
||||
@ -373,6 +384,7 @@ public abstract class ChangeNotesState {
|
||||
.pendingReviewersByEmail(ReviewerByEmailSet.empty())
|
||||
.allPastReviewers(ImmutableList.of())
|
||||
.reviewerUpdates(ImmutableList.of())
|
||||
.attentionUpdates(ImmutableList.of())
|
||||
.assigneeUpdates(ImmutableList.of())
|
||||
.submitRecords(ImmutableList.of())
|
||||
.changeMessages(ImmutableList.of())
|
||||
@ -406,6 +418,8 @@ public abstract class ChangeNotesState {
|
||||
|
||||
abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
|
||||
|
||||
abstract Builder attentionUpdates(List<AttentionStatus> attentionUpdates);
|
||||
|
||||
abstract Builder assigneeUpdates(List<AssigneeStatusUpdate> assigneeUpdates);
|
||||
|
||||
abstract Builder submitRecords(List<SubmitRecord> submitRecords);
|
||||
@ -473,6 +487,7 @@ public abstract class ChangeNotesState {
|
||||
|
||||
object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get()));
|
||||
object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u)));
|
||||
object.attentionUpdates().forEach(u -> b.addAttentionStatus(toAttentionStatusProto(u)));
|
||||
object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u)));
|
||||
object
|
||||
.submitRecords()
|
||||
@ -556,6 +571,15 @@ public abstract class ChangeNotesState {
|
||||
.build();
|
||||
}
|
||||
|
||||
private static AttentionStatusProto toAttentionStatusProto(AttentionStatus attentionStatus) {
|
||||
return AttentionStatusProto.newBuilder()
|
||||
.setDate(attentionStatus.timestamp().toEpochMilli())
|
||||
.setAccount(attentionStatus.account().get())
|
||||
.setOperation(attentionStatus.operation().name())
|
||||
.setReason(attentionStatus.reason())
|
||||
.build();
|
||||
}
|
||||
|
||||
private static AssigneeStatusUpdateProto toAssigneeStatusUpdateProto(AssigneeStatusUpdate u) {
|
||||
AssigneeStatusUpdateProto.Builder builder =
|
||||
AssigneeStatusUpdateProto.newBuilder()
|
||||
@ -596,6 +620,7 @@ public abstract class ChangeNotesState {
|
||||
.allPastReviewers(
|
||||
proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList()))
|
||||
.reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList()))
|
||||
.attentionUpdates(toAttentionUpdateList(proto.getAttentionStatusList()))
|
||||
.assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList()))
|
||||
.submitRecords(
|
||||
proto.getSubmitRecordList().stream()
|
||||
@ -694,6 +719,20 @@ public abstract class ChangeNotesState {
|
||||
return b.build();
|
||||
}
|
||||
|
||||
private static ImmutableList<AttentionStatus> toAttentionUpdateList(
|
||||
List<AttentionStatusProto> protos) {
|
||||
ImmutableList.Builder<AttentionStatus> b = ImmutableList.builder();
|
||||
for (AttentionStatusProto proto : protos) {
|
||||
b.add(
|
||||
AttentionStatus.createFromRead(
|
||||
Instant.ofEpochMilli(proto.getDate()),
|
||||
Account.id(proto.getAccount()),
|
||||
AttentionStatus.Operation.valueOf(proto.getOperation()),
|
||||
proto.getReason()));
|
||||
}
|
||||
return b.build();
|
||||
}
|
||||
|
||||
private static ImmutableList<AssigneeStatusUpdate> toAssigneeStatusUpdateList(
|
||||
List<AssigneeStatusUpdateProto> protos) {
|
||||
ImmutableList.Builder<AssigneeStatusUpdate> b = ImmutableList.builder();
|
||||
|
||||
@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
import static com.google.gerrit.entities.RefNames.changeMetaRef;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
|
||||
@ -52,6 +53,7 @@ import com.google.common.collect.Table;
|
||||
import com.google.common.collect.TreeBasedTable;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.Comment;
|
||||
import com.google.gerrit.entities.Project;
|
||||
@ -125,6 +127,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
private String submissionId;
|
||||
private String topic;
|
||||
private String commit;
|
||||
private List<AttentionStatus> attentionUpdates;
|
||||
private Optional<Account.Id> assignee;
|
||||
private Set<String> hashtags;
|
||||
private String changeMessage;
|
||||
@ -368,6 +371,21 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
this.hashtags = hashtags;
|
||||
}
|
||||
|
||||
/**
|
||||
* All updates must have a timestamp of null since we use the commit's timestamp. There also must
|
||||
* not be multiple updates for a single user.
|
||||
*/
|
||||
void setAttentionUpdates(List<AttentionStatus> attentionUpdates) {
|
||||
checkArgument(
|
||||
attentionUpdates.stream().noneMatch(x -> x.timestamp() != null),
|
||||
"must not specify timestamp for write");
|
||||
checkArgument(
|
||||
attentionUpdates.stream().map(AttentionStatus::account).distinct().count()
|
||||
== attentionUpdates.size(),
|
||||
"must not specify multiple updates for single user");
|
||||
this.attentionUpdates = attentionUpdates;
|
||||
}
|
||||
|
||||
public void setAssignee(Account.Id assignee) {
|
||||
checkArgument(assignee != null, "use removeAssignee");
|
||||
this.assignee = Optional.of(assignee);
|
||||
@ -578,6 +596,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
addFooter(msg, FOOTER_COMMIT, commit);
|
||||
}
|
||||
|
||||
if (attentionUpdates != null) {
|
||||
for (AttentionStatus attentionUpdate : attentionUpdates) {
|
||||
addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionStatusToJson(attentionUpdate));
|
||||
}
|
||||
}
|
||||
|
||||
if (assignee != null) {
|
||||
if (assignee.isPresent()) {
|
||||
addFooter(msg, FOOTER_ASSIGNEE);
|
||||
@ -713,6 +737,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
&& status == null
|
||||
&& submissionId == null
|
||||
&& submitRecords == null
|
||||
&& attentionUpdates == null
|
||||
&& assignee == null
|
||||
&& hashtags == null
|
||||
&& topic == null
|
||||
@ -774,12 +799,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
}
|
||||
|
||||
private StringBuilder addIdent(StringBuilder sb, Account.Id accountId) {
|
||||
PersonIdent ident = newIdent(accountId, when);
|
||||
|
||||
PersonIdent.appendSanitized(sb, ident.getName());
|
||||
sb.append(" <");
|
||||
PersonIdent.appendSanitized(sb, ident.getEmailAddress());
|
||||
sb.append('>');
|
||||
PersonIdent ident = noteUtil.newIdent(accountId, when, serverIdent);
|
||||
ChangeNoteUtil.appendIdentString(sb, ident.getName(), ident.getEmailAddress());
|
||||
return sb;
|
||||
}
|
||||
}
|
||||
|
||||
@ -3065,7 +3065,7 @@ public class ChangeIT extends AbstractDaemonTest {
|
||||
|
||||
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
|
||||
PersonIdent expectedAuthor =
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()), c.updated, serverIdent.get());
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.updated, serverIdent.get());
|
||||
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
|
||||
assertThat(commitPatchSetCreation.getCommitterIdent())
|
||||
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
|
||||
@ -3074,7 +3074,7 @@ public class ChangeIT extends AbstractDaemonTest {
|
||||
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
|
||||
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
|
||||
expectedAuthor =
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
|
||||
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
|
||||
assertThat(commitChangeCreation.getCommitterIdent())
|
||||
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
|
||||
|
||||
@ -1071,7 +1071,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
|
||||
PersonIdent committer = serverIdent.get();
|
||||
PersonIdent author =
|
||||
noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
|
||||
noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
|
||||
tr.branch(RefNames.changeMetaRef(cd3.getId()))
|
||||
.commit()
|
||||
.author(author)
|
||||
|
||||
@ -415,7 +415,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
|
||||
assertThat(commit.getShortMessage()).isEqualTo("Create change");
|
||||
|
||||
PersonIdent expectedAuthor =
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
|
||||
changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
|
||||
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
|
||||
|
||||
assertThat(commit.getCommitterIdent())
|
||||
|
||||
@ -830,7 +830,8 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
|
||||
|
||||
private void addNoteDbCommit(Change.Id id, String commitMessage) throws Exception {
|
||||
PersonIdent committer = serverIdent.get();
|
||||
PersonIdent author = noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
|
||||
PersonIdent author =
|
||||
noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
|
||||
serverSideTestRepo
|
||||
.branch(RefNames.changeMetaRef(id))
|
||||
.commit()
|
||||
|
||||
@ -518,7 +518,9 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
|
||||
private RevCommit writeCommit(String body) throws Exception {
|
||||
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
|
||||
return writeCommit(
|
||||
body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
|
||||
body,
|
||||
noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
|
||||
false);
|
||||
}
|
||||
|
||||
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
|
||||
@ -529,7 +531,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
|
||||
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
|
||||
return writeCommit(
|
||||
body,
|
||||
noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
|
||||
noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
|
||||
initWorkInProgress);
|
||||
}
|
||||
|
||||
|
||||
@ -28,6 +28,7 @@ import com.google.common.collect.Iterables;
|
||||
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.AttentionStatus;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.ChangeMessage;
|
||||
import com.google.gerrit.entities.Comment;
|
||||
@ -44,6 +45,7 @@ import com.google.gerrit.server.ReviewerSet;
|
||||
import com.google.gerrit.server.ReviewerStatusUpdate;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
|
||||
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
|
||||
@ -55,6 +57,7 @@ import com.google.inject.TypeLiteral;
|
||||
import com.google.protobuf.ByteString;
|
||||
import java.lang.reflect.Type;
|
||||
import java.sql.Timestamp;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
@ -64,6 +67,7 @@ import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
public class ChangeNotesStateTest {
|
||||
|
||||
private static final Change.Id ID = Change.id(123);
|
||||
private static final ObjectId SHA =
|
||||
ObjectId.fromString("1234567812345678123456781234567812345678");
|
||||
@ -95,6 +99,13 @@ public class ChangeNotesStateTest {
|
||||
return ChangeNotesState.Builder.empty(ID).metaId(SHA).columns(cols);
|
||||
}
|
||||
|
||||
private ChangeNotesStateProto.Builder newProtoBuilder() {
|
||||
return ChangeNotesStateProto.newBuilder()
|
||||
.setMetaId(SHA_BYTES)
|
||||
.setChangeId(ID.get())
|
||||
.setColumns(colsProto);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void serializeChangeKey() throws Exception {
|
||||
assertRoundTrip(
|
||||
@ -588,6 +599,39 @@ public class ChangeNotesStateTest {
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void serializeAttentionUpdates() throws Exception {
|
||||
assertRoundTrip(
|
||||
newBuilder()
|
||||
.attentionUpdates(
|
||||
ImmutableList.of(
|
||||
AttentionStatus.createFromRead(
|
||||
Instant.EPOCH.plusSeconds(23),
|
||||
Account.id(1000),
|
||||
AttentionStatus.Operation.ADD,
|
||||
"reason 1"),
|
||||
AttentionStatus.createFromRead(
|
||||
Instant.EPOCH.plusSeconds(42),
|
||||
Account.id(2000),
|
||||
AttentionStatus.Operation.REMOVE,
|
||||
"reason 2")))
|
||||
.build(),
|
||||
newProtoBuilder()
|
||||
.addAttentionStatus(
|
||||
AttentionStatusProto.newBuilder()
|
||||
.setDate(23_000) // epoch millis
|
||||
.setAccount(1000)
|
||||
.setOperation("ADD")
|
||||
.setReason("reason 1"))
|
||||
.addAttentionStatus(
|
||||
AttentionStatusProto.newBuilder()
|
||||
.setDate(42_000) // epoch millis
|
||||
.setAccount(2000)
|
||||
.setOperation("REMOVE")
|
||||
.setReason("reason 2"))
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void serializeAssigneeUpdates() throws Exception {
|
||||
assertRoundTrip(
|
||||
@ -744,6 +788,9 @@ public class ChangeNotesStateTest {
|
||||
.put(
|
||||
"reviewerUpdates",
|
||||
new TypeLiteral<ImmutableList<ReviewerStatusUpdate>>() {}.getType())
|
||||
.put(
|
||||
"attentionUpdates",
|
||||
new TypeLiteral<ImmutableList<AttentionStatus>>() {}.getType())
|
||||
.put(
|
||||
"assigneeUpdates",
|
||||
new TypeLiteral<ImmutableList<AssigneeStatusUpdate>>() {}.getType())
|
||||
@ -824,10 +871,10 @@ public class ChangeNotesStateTest {
|
||||
.hasFields(
|
||||
ImmutableMap.of(
|
||||
"table",
|
||||
new TypeLiteral<
|
||||
ImmutableTable<
|
||||
ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
|
||||
"accounts", new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
|
||||
new TypeLiteral<
|
||||
ImmutableTable<ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
|
||||
"accounts",
|
||||
new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
|
||||
}
|
||||
|
||||
@Test
|
||||
@ -836,9 +883,10 @@ public class ChangeNotesStateTest {
|
||||
.hasFields(
|
||||
ImmutableMap.of(
|
||||
"table",
|
||||
new TypeLiteral<
|
||||
ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
|
||||
"users", new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
|
||||
new TypeLiteral<
|
||||
ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
|
||||
"users",
|
||||
new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@ -38,6 +38,8 @@ import com.google.common.collect.ListMultimap;
|
||||
import com.google.common.collect.Lists;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.entities.Account;
|
||||
import com.google.gerrit.entities.AttentionStatus;
|
||||
import com.google.gerrit.entities.AttentionStatus.Operation;
|
||||
import com.google.gerrit.entities.BranchNameKey;
|
||||
import com.google.gerrit.entities.Change;
|
||||
import com.google.gerrit.entities.ChangeMessage;
|
||||
@ -58,6 +60,7 @@ import com.google.gerrit.server.util.time.TimeUtil;
|
||||
import com.google.gerrit.testing.TestChanges;
|
||||
import com.google.inject.Inject;
|
||||
import java.sql.Timestamp;
|
||||
import java.time.Instant;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@ -687,6 +690,92 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
|
||||
assertThat(updated.getObjectId()).isEqualTo(initial.getObjectId());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void defaultAttentionSetIsEmpty() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeNotes notes = newNotes(c);
|
||||
assertThat(notes.getAttentionUpdates()).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addAttentionStatus() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeUpdate update = newUpdate(c, changeOwner);
|
||||
AttentionStatus attentionStatus =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
|
||||
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
|
||||
update.commit();
|
||||
|
||||
ChangeNotes notes = newNotes(c);
|
||||
assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void filterLatestAttentionStatus() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeUpdate update = newUpdate(c, changeOwner);
|
||||
AttentionStatus attentionStatus =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
|
||||
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
|
||||
update.commit();
|
||||
update = newUpdate(c, changeOwner);
|
||||
attentionStatus =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
|
||||
update.setAttentionUpdates(ImmutableList.of(attentionStatus));
|
||||
update.commit();
|
||||
|
||||
ChangeNotes notes = newNotes(c);
|
||||
assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addAttentionStatus_rejectTimestamp() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeUpdate update = newUpdate(c, changeOwner);
|
||||
AttentionStatus attentionStatus =
|
||||
AttentionStatus.createFromRead(
|
||||
Instant.now(), changeOwner.getAccountId(), Operation.ADD, "test");
|
||||
IllegalArgumentException thrown =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> update.setAttentionUpdates(ImmutableList.of(attentionStatus)));
|
||||
assertThat(thrown).hasMessageThat().contains("must not specify timestamp for write");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addAttentionStatus_rejectMultiplePerUser() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeUpdate update = newUpdate(c, changeOwner);
|
||||
AttentionStatus attentionStatus0 =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
|
||||
AttentionStatus attentionStatus1 =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
|
||||
IllegalArgumentException thrown =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1)));
|
||||
assertThat(thrown)
|
||||
.hasMessageThat()
|
||||
.contains("must not specify multiple updates for single user");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addAttentionStatusForMultipleUsers() throws Exception {
|
||||
Change c = newChange();
|
||||
ChangeUpdate update = newUpdate(c, changeOwner);
|
||||
AttentionStatus attentionStatus0 =
|
||||
AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
|
||||
AttentionStatus attentionStatus1 =
|
||||
AttentionStatus.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
|
||||
|
||||
update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1));
|
||||
update.commit();
|
||||
|
||||
ChangeNotes notes = newNotes(c);
|
||||
assertThat(notes.getAttentionUpdates())
|
||||
.containsExactly(addTimestamp(attentionStatus0, c), addTimestamp(attentionStatus1, c));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void assigneeCommit() throws Exception {
|
||||
Change c = newChange();
|
||||
@ -3140,4 +3229,13 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
|
||||
update.commit();
|
||||
return tr.parseBody(commit);
|
||||
}
|
||||
|
||||
private AttentionStatus addTimestamp(AttentionStatus attentionStatus, Change c) {
|
||||
Timestamp timestamp = newNotes(c).getChange().getLastUpdatedOn();
|
||||
return AttentionStatus.createFromRead(
|
||||
timestamp.toInstant(),
|
||||
attentionStatus.account(),
|
||||
attentionStatus.operation(),
|
||||
attentionStatus.reason());
|
||||
}
|
||||
}
|
||||
|
||||
@ -75,7 +75,7 @@ message ChangeNotesKeyProto {
|
||||
// Instead, we just take the tedious yet simple approach of having a "has_foo"
|
||||
// field for each nullable field "foo", indicating whether or not foo is null.
|
||||
//
|
||||
// Next ID: 23
|
||||
// Next ID: 24
|
||||
message ChangeNotesStateProto {
|
||||
// Effectively required, even though the corresponding ChangeNotesState field
|
||||
// is optional, since the field is only absent when NoteDb is disabled, in
|
||||
@ -133,7 +133,7 @@ message ChangeNotesStateProto {
|
||||
// which case attempting to use the ChangeNotesCache is programmer error.
|
||||
ChangeColumnsProto columns = 3;
|
||||
|
||||
reserved 4; // past_assignee
|
||||
reserved 4; // past_assignee
|
||||
|
||||
repeated string hashtag = 5;
|
||||
|
||||
@ -167,6 +167,7 @@ message ChangeNotesStateProto {
|
||||
|
||||
// Next ID: 5
|
||||
message ReviewerStatusUpdateProto {
|
||||
// Epoch millis.
|
||||
int64 date = 1;
|
||||
int32 updated_by = 2;
|
||||
int32 reviewer = 3;
|
||||
@ -194,14 +195,26 @@ message ChangeNotesStateProto {
|
||||
bool has_server_id = 21;
|
||||
|
||||
message AssigneeStatusUpdateProto {
|
||||
// Epoch millis.
|
||||
int64 date = 1;
|
||||
int32 updated_by = 2;
|
||||
int32 current_assignee = 3;
|
||||
bool has_current_assignee = 4;
|
||||
}
|
||||
repeated AssigneeStatusUpdateProto assignee_update = 22;
|
||||
}
|
||||
|
||||
// An update to the attention set of the change. See class AttentionStatus for
|
||||
// context.
|
||||
message AttentionStatusProto {
|
||||
// Epoch millis.
|
||||
int64 date = 1;
|
||||
int32 account = 2;
|
||||
// Maps to enum AttentionStatus.Operation
|
||||
string operation = 3;
|
||||
string reason = 4;
|
||||
}
|
||||
repeated AttentionStatusProto attention_status = 23;
|
||||
}
|
||||
|
||||
// Serialized form of com.google.gerrit.server.query.change.ConflictKey
|
||||
message ConflictKeyProto {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user