diff --git a/java/com/google/gerrit/server/AssigneeStatusUpdate.java b/java/com/google/gerrit/server/AssigneeStatusUpdate.java new file mode 100644 index 0000000000..0da5edf981 --- /dev/null +++ b/java/com/google/gerrit/server/AssigneeStatusUpdate.java @@ -0,0 +1,21 @@ +package com.google.gerrit.server; + +import com.google.auto.value.AutoValue; +import com.google.gerrit.entities.Account; +import java.sql.Timestamp; +import java.util.Optional; + +/** Change to an assignee's status. */ +@AutoValue +public abstract class AssigneeStatusUpdate { + public static AssigneeStatusUpdate create( + Timestamp ts, Account.Id updatedBy, Optional currentAssignee) { + return new AutoValue_AssigneeStatusUpdate(ts, updatedBy, currentAssignee); + } + + public abstract Timestamp date(); + + public abstract Account.Id updatedBy(); + + public abstract Optional currentAssignee(); +} diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java index 384eebcb4e..8cf00461dc 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.notedb; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.gerrit.entities.RefNames.changeMetaRef; import static java.util.Comparator.comparing; import static java.util.Objects.requireNonNull; @@ -29,6 +30,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.ListMultimap; +import com.google.common.collect.Lists; import com.google.common.collect.MultimapBuilder; import com.google.common.collect.Multimaps; import com.google.common.collect.Ordering; @@ -48,6 +50,7 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; import com.google.gerrit.entities.RobotComment; import com.google.gerrit.exceptions.StorageException; +import com.google.gerrit.server.AssigneeStatusUpdate; import com.google.gerrit.server.ReviewerByEmailSet; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.ReviewerStatusUpdate; @@ -366,9 +369,24 @@ public class ChangeNotes extends AbstractChangeNotes { return state.reviewerUpdates(); } - /** @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. */ + /** + * @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. + */ public ImmutableSet getPastAssignees() { - return state.pastAssignees(); + return Lists.reverse(state.assigneeUpdates()).stream() + .map(AssigneeStatusUpdate::currentAssignee) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(toImmutableSet()); + } + + /** + * @return an ImmutableList of AssigneeStatusUpdate of all the updates to the assignee field to + * this change. The order of the list is from most recent updates to least recent. + */ + public ImmutableList getAssigneeUpdates() { + return state.assigneeUpdates(); } /** @return a ImmutableSet of all hashtags for this change sorted in alphabetical order. */ diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java index 03073d2fe2..8f5fefec59 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java @@ -61,7 +61,7 @@ public class ChangeNotesCache { .weigher(Weigher.class) .maximumWeight(10 << 20) .diskLimit(-1) - .version(1) + .version(2) .keySerializer(Key.Serializer.INSTANCE) .valueSerializer(ChangeNotesState.Serializer.INSTANCE); } @@ -148,11 +148,8 @@ public class ChangeNotesCache { + str(state.columns().originalSubject()) + P + str(state.columns().submissionId()) - + ptr(state.columns().assignee(), K) // assignee + P // status + P - + set(state.pastAssignees(), K) - + P + set(state.hashtags(), str(10)) + P + list(state.patchSets(), patchSet()) @@ -169,6 +166,8 @@ public class ChangeNotesCache { + P + list(state.reviewerUpdates(), 4 * O + K + K + P) + P + + list(state.assigneeUpdates(), 4 * O + K + K) + + P + list(state.submitRecords(), P + list(2, str(4) + P + K) + P) + P + list(state.changeMessages(), changeMessage()) diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index c4c227ee88..a3c47470a5 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -67,6 +67,7 @@ import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.entities.RefNames; import com.google.gerrit.mail.Address; import com.google.gerrit.metrics.Timer1; +import com.google.gerrit.server.AssigneeStatusUpdate; import com.google.gerrit.server.ReviewerByEmailSet; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.ReviewerStatusUpdate; @@ -115,6 +116,7 @@ class ChangeNotesParser { private final Table reviewersByEmail; private final List allPastReviewers; private final List reviewerUpdates; + private final List assigneeUpdates; private final List submitRecords; private final ListMultimap comments; private final Map patchSets; @@ -129,8 +131,6 @@ class ChangeNotesParser { private String branch; private Change.Status status; private String topic; - private Optional assignee; - private List pastAssignees; private Set hashtags; private Timestamp createdOn; private Timestamp lastUpdatedOn; @@ -172,6 +172,7 @@ class ChangeNotesParser { pendingReviewersByEmail = ReviewerByEmailSet.empty(); allPastReviewers = new ArrayList<>(); reviewerUpdates = new ArrayList<>(); + assigneeUpdates = new ArrayList<>(); submitRecords = Lists.newArrayListWithExpectedSize(1); allChangeMessages = new ArrayList<>(); comments = MultimapBuilder.hashKeys().arrayListValues().build(); @@ -231,9 +232,7 @@ class ChangeNotesParser { topic, originalSubject, submissionId, - assignee != null ? assignee.orElse(null) : null, status, - Sets.newLinkedHashSet(Lists.reverse(pastAssignees)), firstNonNull(hashtags, ImmutableSet.of()), buildPatchSets(), buildApprovals(), @@ -243,6 +242,7 @@ class ChangeNotesParser { pendingReviewersByEmail, allPastReviewers, buildReviewerUpdates(), + assigneeUpdates, submitRecords, buildAllMessages(), comments, @@ -361,7 +361,7 @@ class ChangeNotesParser { } parseHashtags(commit); - parseAssignee(commit); + parseAssigneeUpdates(ts, commit); if (submissionId == null) { submissionId = parseSubmissionId(commit); @@ -566,10 +566,8 @@ class ChangeNotesParser { } } - private void parseAssignee(ChangeNotesCommit commit) throws ConfigInvalidException { - if (pastAssignees == null) { - pastAssignees = Lists.newArrayList(); - } + private void parseAssigneeUpdates(Timestamp ts, ChangeNotesCommit commit) + throws ConfigInvalidException { String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE); if (assigneeValue != null) { Optional parsedAssignee; @@ -580,12 +578,7 @@ class ChangeNotesParser { PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue); parsedAssignee = Optional.ofNullable(legacyChangeNoteRead.parseIdent(ident, id)); } - if (assignee == null) { - assignee = parsedAssignee; - } - if (parsedAssignee.isPresent()) { - pastAssignees.add(parsedAssignee.get()); - } + assigneeUpdates.add(AssigneeStatusUpdate.create(ts, ownerId, parsedAssignee)); } } diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java index 53965086c0..896cca3ba4 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static java.util.Objects.requireNonNull; import com.google.auto.value.AutoValue; @@ -50,10 +49,12 @@ import com.google.gerrit.entities.converter.ProtoConverter; import com.google.gerrit.json.OutputFormat; import com.google.gerrit.mail.Address; import com.google.gerrit.proto.Protos; +import com.google.gerrit.server.AssigneeStatusUpdate; import com.google.gerrit.server.ReviewerByEmailSet; 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.ChangeColumnsProto; import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto; import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto; @@ -67,6 +68,7 @@ import com.google.protobuf.MessageLite; import java.sql.Timestamp; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.eclipse.jgit.lib.ObjectId; @@ -103,9 +105,7 @@ public abstract class ChangeNotesState { @Nullable String topic, @Nullable String originalSubject, @Nullable String submissionId, - @Nullable Account.Id assignee, @Nullable Change.Status status, - Set pastAssignees, Set hashtags, Map patchSets, ListMultimap approvals, @@ -115,6 +115,7 @@ public abstract class ChangeNotesState { ReviewerByEmailSet pendingReviewersByEmail, List allPastReviewers, List reviewerUpdates, + List assigneeUpdates, List submitRecords, List changeMessages, ListMultimap publishedComments, @@ -147,13 +148,11 @@ public abstract class ChangeNotesState { .topic(topic) .originalSubject(originalSubject) .submissionId(submissionId) - .assignee(assignee) .isPrivate(isPrivate) .workInProgress(workInProgress) .reviewStarted(reviewStarted) .revertOf(revertOf) .build()) - .pastAssignees(pastAssignees) .hashtags(hashtags) .serverId(serverId) .patchSets(patchSets.entrySet()) @@ -164,6 +163,7 @@ public abstract class ChangeNotesState { .pendingReviewersByEmail(pendingReviewersByEmail) .allPastReviewers(allPastReviewers) .reviewerUpdates(reviewerUpdates) + .assigneeUpdates(assigneeUpdates) .submitRecords(submitRecords) .changeMessages(changeMessages) .publishedComments(publishedComments) @@ -211,9 +211,6 @@ public abstract class ChangeNotesState { @Nullable abstract String submissionId(); - @Nullable - abstract Account.Id assignee(); - abstract boolean isPrivate(); abstract boolean workInProgress(); @@ -247,8 +244,6 @@ public abstract class ChangeNotesState { abstract Builder submissionId(@Nullable String submissionId); - abstract Builder assignee(@Nullable Account.Id assignee); - abstract Builder status(@Nullable Change.Status status); abstract Builder isPrivate(boolean isPrivate); @@ -274,8 +269,6 @@ public abstract class ChangeNotesState { abstract ChangeColumns columns(); // Other related to this Change. - abstract ImmutableSet pastAssignees(); - abstract ImmutableSet hashtags(); @Nullable @@ -297,6 +290,8 @@ public abstract class ChangeNotesState { abstract ImmutableList reviewerUpdates(); + abstract ImmutableList assigneeUpdates(); + abstract ImmutableList submitRecords(); abstract ImmutableList changeMessages(); @@ -339,7 +334,9 @@ public abstract class ChangeNotesState { change.setTopic(Strings.emptyToNull(c.topic())); change.setLastUpdatedOn(c.lastUpdatedOn()); change.setSubmissionId(c.submissionId()); - change.setAssignee(c.assignee()); + if (!assigneeUpdates().isEmpty()) { + change.setAssignee(assigneeUpdates().get(0).currentAssignee().orElse(null)); + } change.setPrivate(c.isPrivate()); change.setWorkInProgress(c.workInProgress()); change.setReviewStarted(c.reviewStarted()); @@ -359,7 +356,6 @@ public abstract class ChangeNotesState { static Builder empty(Change.Id changeId) { return new AutoValue_ChangeNotesState.Builder() .changeId(changeId) - .pastAssignees(ImmutableSet.of()) .hashtags(ImmutableSet.of()) .patchSets(ImmutableList.of()) .approvals(ImmutableList.of()) @@ -369,6 +365,7 @@ public abstract class ChangeNotesState { .pendingReviewersByEmail(ReviewerByEmailSet.empty()) .allPastReviewers(ImmutableList.of()) .reviewerUpdates(ImmutableList.of()) + .assigneeUpdates(ImmutableList.of()) .submitRecords(ImmutableList.of()) .changeMessages(ImmutableList.of()) .publishedComments(ImmutableListMultimap.of()) @@ -383,8 +380,6 @@ public abstract class ChangeNotesState { abstract Builder serverId(String serverId); - abstract Builder pastAssignees(Set pastAssignees); - abstract Builder hashtags(Iterable hashtags); abstract Builder patchSets(Iterable> patchSets); @@ -403,6 +398,8 @@ public abstract class ChangeNotesState { abstract Builder reviewerUpdates(List reviewerUpdates); + abstract Builder assigneeUpdates(List assigneeUpdates); + abstract Builder submitRecords(List submitRecords); abstract Builder changeMessages(List changeMessages); @@ -438,7 +435,6 @@ public abstract class ChangeNotesState { b.setServerId(object.serverId()); b.setHasServerId(true); } - object.pastAssignees().forEach(a -> b.addPastAssignee(a.get())); object.hashtags().forEach(b::addHashtag); object .patchSets() @@ -469,6 +465,7 @@ public abstract class ChangeNotesState { object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get())); object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u))); + object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u))); object .submitRecords() .forEach(r -> b.addSubmitRecord(GSON.toJson(new StoredSubmitRecord(r)))); @@ -508,9 +505,6 @@ public abstract class ChangeNotesState { if (cols.submissionId() != null) { b.setSubmissionId(cols.submissionId()).setHasSubmissionId(true); } - if (cols.assignee() != null) { - b.setAssignee(cols.assignee().get()).setHasAssignee(true); - } if (cols.status() != null) { b.setStatus(STATUS_CONVERTER.reverse().convert(cols.status())).setHasStatus(true); } @@ -550,6 +544,17 @@ public abstract class ChangeNotesState { .build(); } + private static AssigneeStatusUpdateProto toAssigneeStatusUpdateProto(AssigneeStatusUpdate u) { + AssigneeStatusUpdateProto.Builder builder = + AssigneeStatusUpdateProto.newBuilder() + .setDate(u.date().getTime()) + .setUpdatedBy(u.updatedBy().get()) + .setHasCurrentAssignee(u.currentAssignee().isPresent()); + + u.currentAssignee().ifPresent(assignee -> builder.setCurrentAssignee(assignee.get())); + return builder.build(); + } + @Override public ChangeNotesState deserialize(byte[] in) { ChangeNotesStateProto proto = Protos.parseUnchecked(ChangeNotesStateProto.parser(), in); @@ -561,8 +566,6 @@ public abstract class ChangeNotesState { .changeId(changeId) .columns(toChangeColumns(changeId, proto.getColumns())) .serverId(proto.getHasServerId() ? proto.getServerId() : null) - .pastAssignees( - proto.getPastAssigneeList().stream().map(Account::id).collect(toImmutableSet())) .hashtags(proto.getHashtagList()) .patchSets( proto.getPatchSetList().stream() @@ -581,6 +584,7 @@ public abstract class ChangeNotesState { .allPastReviewers( proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList())) .reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList())) + .assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList())) .submitRecords( proto.getSubmitRecordList().stream() .map(r -> GSON.fromJson(r, StoredSubmitRecord.class).toSubmitRecord()) @@ -624,9 +628,6 @@ public abstract class ChangeNotesState { if (proto.getHasSubmissionId()) { b.submissionId(proto.getSubmissionId()); } - if (proto.getHasAssignee()) { - b.assignee(Account.id(proto.getAssignee())); - } if (proto.getHasStatus()) { b.status(STATUS_CONVERTER.convert(proto.getStatus())); } @@ -677,5 +678,20 @@ public abstract class ChangeNotesState { } return b.build(); } + + private static ImmutableList toAssigneeStatusUpdateList( + List protos) { + ImmutableList.Builder b = ImmutableList.builder(); + for (AssigneeStatusUpdateProto proto : protos) { + b.add( + AssigneeStatusUpdate.create( + new Timestamp(proto.getDate()), + Account.id(proto.getUpdatedBy()), + proto.getHasCurrentAssignee() + ? Optional.of(Account.id(proto.getCurrentAssignee())) + : Optional.empty())); + } + return b.build(); + } } } diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index ed79be62fa..7882518f9a 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java @@ -213,6 +213,26 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { assertParseFails("Update change\n\nPatch-set: 1\nReviewer: 1@gerrit\n"); } + @Test + public void parseAssignee() throws Exception { + assertParseSucceeds( + "Update change\n" + + "\n" + + "Branch: refs/heads/master\n" + + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + + "Patch-set: 1\n" + + "Assignee: Change Owner <1@gerrit>\n" + + "Subject: This is a test change\n"); + assertParseSucceeds( + "Update change\n" + + "\n" + + "Branch: refs/heads/master\n" + + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + + "Patch-set: 2\n" + + "Assignee:\n" + + "Subject: This is a test change\n"); + } + @Test public void parseTopic() throws Exception { assertParseSucceeds( diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java index d381e99e96..0d7f2bd171 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java @@ -38,10 +38,12 @@ import com.google.gerrit.entities.converter.ChangeMessageProtoConverter; import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter; import com.google.gerrit.entities.converter.PatchSetProtoConverter; import com.google.gerrit.mail.Address; +import com.google.gerrit.server.AssigneeStatusUpdate; import com.google.gerrit.server.ReviewerByEmailSet; 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.ChangeColumnsProto; import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto; import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto; @@ -241,17 +243,6 @@ public class ChangeNotesStateTest { .build()); } - @Test - public void serializeAssignee() throws Exception { - assertRoundTrip( - newBuilder().columns(cols.toBuilder().assignee(Account.id(2000)).build()).build(), - ChangeNotesStateProto.newBuilder() - .setMetaId(SHA_BYTES) - .setChangeId(ID.get()) - .setColumns(colsProto.toBuilder().setAssignee(2000).setHasAssignee(true)) - .build()); - } - @Test public void serializeStatus() throws Exception { assertRoundTrip( @@ -307,19 +298,6 @@ public class ChangeNotesStateTest { .build()); } - @Test - public void serializePastAssignees() throws Exception { - assertRoundTrip( - newBuilder().pastAssignees(ImmutableSet.of(Account.id(2002), Account.id(2001))).build(), - ChangeNotesStateProto.newBuilder() - .setMetaId(SHA_BYTES) - .setChangeId(ID.get()) - .setColumns(colsProto) - .addPastAssignee(2002) - .addPastAssignee(2001) - .build()); - } - @Test public void serializeHashtags() throws Exception { assertRoundTrip( @@ -610,6 +588,35 @@ public class ChangeNotesStateTest { .build()); } + @Test + public void serializeAssigneeUpdates() throws Exception { + assertRoundTrip( + newBuilder() + .assigneeUpdates( + ImmutableList.of( + AssigneeStatusUpdate.create( + new Timestamp(1212L), Account.id(1000), Optional.of(Account.id(2001))), + AssigneeStatusUpdate.create( + new Timestamp(3434L), Account.id(1000), Optional.empty()))) + .build(), + ChangeNotesStateProto.newBuilder() + .setMetaId(SHA_BYTES) + .setChangeId(ID.get()) + .setColumns(colsProto) + .addAssigneeUpdate( + AssigneeStatusUpdateProto.newBuilder() + .setDate(1212L) + .setUpdatedBy(1000) + .setCurrentAssignee(2001) + .setHasCurrentAssignee(true)) + .addAssigneeUpdate( + AssigneeStatusUpdateProto.newBuilder() + .setDate(3434L) + .setUpdatedBy(1000) + .setHasCurrentAssignee(false)) + .build()); + } + @Test public void serializeSubmitRecords() throws Exception { SubmitRecord sr1 = new SubmitRecord(); @@ -721,7 +728,6 @@ public class ChangeNotesStateTest { .put("changeId", Change.Id.class) .put("serverId", String.class) .put("columns", ChangeColumns.class) - .put("pastAssignees", new TypeLiteral>() {}.getType()) .put("hashtags", new TypeLiteral>() {}.getType()) .put( "patchSets", @@ -738,6 +744,9 @@ public class ChangeNotesStateTest { .put( "reviewerUpdates", new TypeLiteral>() {}.getType()) + .put( + "assigneeUpdates", + new TypeLiteral>() {}.getType()) .put("submitRecords", new TypeLiteral>() {}.getType()) .put("changeMessages", new TypeLiteral>() {}.getType()) .put( @@ -762,7 +771,6 @@ public class ChangeNotesStateTest { .put("topic", String.class) .put("originalSubject", String.class) .put("submissionId", String.class) - .put("assignee", Account.Id.class) .put("status", Change.Status.class) .put("isPrivate", boolean.class) .put("workInProgress", boolean.class) @@ -843,6 +851,19 @@ public class ChangeNotesStateTest { "state", ReviewerStateInternal.class)); } + @Test + public void assigneeStatusUpdateMethods() throws Exception { + assertThatSerializedClass(AssigneeStatusUpdate.class) + .hasAutoValueMethods( + ImmutableMap.of( + "date", + Timestamp.class, + "updatedBy", + Account.Id.class, + "currentAssignee", + new TypeLiteral>() {}.getType())); + } + @Test public void submitRecordFields() throws Exception { assertThatSerializedClass(SubmitRecord.class) diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java index 2ed145ee0d..a4c395c11b 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -48,6 +48,7 @@ import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.mail.Address; +import com.google.gerrit.server.AssigneeStatusUpdate; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.ReviewerSet; @@ -744,6 +745,35 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(notes.getPastAssignees()).hasSize(2); } + @Test + public void assigneeStatusUpdateChangeNotes() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, otherUser); + update.setAssignee(otherUserId); + update.commit(); + + update = newUpdate(c, changeOwner); + update.removeAssignee(); + update.commit(); + + update = newUpdate(c, changeOwner); + update.setAssignee(changeOwner.getAccountId()); + update.commit(); + + update = newUpdate(c, changeOwner); + update.setAssignee(otherUserId); + update.commit(); + + ChangeNotes notes = newNotes(c); + ImmutableList statusUpdates = notes.getAssigneeUpdates(); + assertThat(statusUpdates).hasSize(4); + assertThat(statusUpdates.get(3).updatedBy()).isEqualTo(otherUserId); + assertThat(statusUpdates.get(3).currentAssignee()).hasValue(otherUserId); + assertThat(statusUpdates.get(2).currentAssignee()).isEmpty(); + assertThat(statusUpdates.get(1).currentAssignee()).hasValue(changeOwner.getAccountId()); + assertThat(statusUpdates.get(0).currentAssignee()).hasValue(otherUserId); + } + @Test public void hashtagCommit() throws Exception { Change c = newChange(); diff --git a/proto/cache.proto b/proto/cache.proto index 2a0764098c..10e0216cf1 100644 --- a/proto/cache.proto +++ b/proto/cache.proto @@ -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: 20 +// Next ID: 23 message ChangeNotesStateProto { // Effectively required, even though the corresponding ChangeNotesState field // is optional, since the field is only absent when NoteDb is disabled, in @@ -110,8 +110,8 @@ message ChangeNotesStateProto { string submission_id = 13; bool has_submission_id = 14; - int32 assignee = 15; - bool has_assignee = 16; + reserved 15; // assignee + reserved 16; // has_assignee string status = 17; bool has_status = 18; @@ -130,7 +130,7 @@ message ChangeNotesStateProto { // which case attempting to use the ChangeNotesCache is programmer error. ChangeColumnsProto columns = 3; - repeated int32 past_assignee = 4; + reserved 4; // past_assignee repeated string hashtag = 5; @@ -189,6 +189,14 @@ message ChangeNotesStateProto { string server_id = 20; bool has_server_id = 21; + + message AssigneeStatusUpdateProto { + int64 date = 1; + int32 updated_by = 2; + int32 current_assignee = 3; + bool has_current_assignee = 4; + } + repeated AssigneeStatusUpdateProto assignee_update = 22; }