ChangeNotesState: Remove changeMessagesByPatchSet field
The only caller outside of tests that used this field was
Submit#getConflictMessage, which no longer exists.
Rename the remaining field from allChangeMessages to changeMessages,
since there is no longer overlap with another name.
Change-Id: I8f2846d7b21b41a6e0c934fd73ff87f6dc8e58d9
diff --git a/java/com/google/gerrit/server/ChangeMessagesUtil.java b/java/com/google/gerrit/server/ChangeMessagesUtil.java
index 75a9991..e635072 100644
--- a/java/com/google/gerrit/server/ChangeMessagesUtil.java
+++ b/java/com/google/gerrit/server/ChangeMessagesUtil.java
@@ -116,14 +116,6 @@
return notes.load().getChangeMessages();
}
- public Iterable<ChangeMessage> byPatchSet(ReviewDb db, ChangeNotes notes, PatchSet.Id psId)
- throws OrmException {
- if (!migration.readChanges()) {
- return db.changeMessages().byPatchSet(psId);
- }
- return notes.load().getChangeMessagesByPatchSet().get(psId);
- }
-
public void addChangeMessage(ReviewDb db, ChangeUpdate update, ChangeMessage changeMessage)
throws OrmException {
checkState(
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 7081f29..1bbecc8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -564,12 +564,7 @@
/** @return all change messages, in chronological order, oldest first. */
public ImmutableList<ChangeMessage> getChangeMessages() {
- return state.allChangeMessages();
- }
-
- /** @return change messages by patch set, in chronological order, oldest first. */
- public ImmutableListMultimap<PatchSet.Id, ChangeMessage> getChangeMessagesByPatchSet() {
- return state.changeMessagesByPatchSet();
+ return state.changeMessages();
}
/** @return inline comments on each revision. */
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 676dbb8..5658569 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -128,10 +128,7 @@
+ P
+ list(state.submitRecords(), P + list(2, str(4) + P + K) + P)
+ P
- + list(state.allChangeMessages(), changeMessage())
- // Just key overhead for map, already counted messages in previous.
- + P
- + map(state.changeMessagesByPatchSet().asMap(), patchSetId())
+ + list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
+ T // readOnlyUntil
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 2df6e14..2eb30ff 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -45,7 +45,6 @@
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
-import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
@@ -146,7 +145,6 @@
private final Map<ApprovalKey, PatchSetApproval> approvals;
private final List<PatchSetApproval> bufferedApprovals;
private final List<ChangeMessage> allChangeMessages;
- private final ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet;
// Non-final private members filled in during the parsing process.
private String branch;
@@ -194,7 +192,6 @@
reviewerUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
- changeMessagesByPatchSet = LinkedListMultimap.create();
comments = MultimapBuilder.hashKeys().arrayListValues().build();
patchSets = new HashMap<>();
deletedPatchSets = new HashSet<>();
@@ -265,7 +262,6 @@
buildReviewerUpdates(),
submitRecords,
buildAllMessages(),
- buildMessagesByPatchSet(),
comments,
readOnlyUntil,
firstNonNull(isPrivate, false),
@@ -319,13 +315,6 @@
return Lists.reverse(allChangeMessages);
}
- private ListMultimap<PatchSet.Id, ChangeMessage> buildMessagesByPatchSet() {
- for (Collection<ChangeMessage> v : changeMessagesByPatchSet.asMap().values()) {
- Collections.reverse((List<ChangeMessage>) v);
- }
- return changeMessagesByPatchSet;
- }
-
private void parse(ChangeNotesCommit commit) throws ConfigInvalidException {
Timestamp ts = new Timestamp(commit.getCommitterIdent().getWhen().getTime());
@@ -752,7 +741,6 @@
changeMessage.setMessage(changeMsgString);
changeMessage.setTag(tag);
changeMessage.setRealAuthor(realAccountId);
- changeMessagesByPatchSet.put(psId, changeMessage);
allChangeMessages.add(changeMessage);
}
@@ -1089,8 +1077,6 @@
// (or otherwise missing) patch sets. This is safer than trying to prevent
// insertion, as it will also filter out items racily added after the patch
// set was deleted.
- changeMessagesByPatchSet.keys().retainAll(patchSets.keySet());
-
int pruned =
pruneEntitiesForMissingPatchSets(allChangeMessages, ChangeMessage::getPatchSetId, missing);
pruned +=
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index a55fd7f..c4944c8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -74,7 +74,6 @@
ImmutableList.of(),
ImmutableList.of(),
ImmutableListMultimap.of(),
- ImmutableListMultimap.of(),
null);
}
@@ -104,8 +103,7 @@
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
List<SubmitRecord> submitRecords,
- List<ChangeMessage> allChangeMessages,
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet,
+ List<ChangeMessage> changeMessages,
ListMultimap<RevId, Comment> publishedComments,
@Nullable Timestamp readOnlyUntil,
boolean isPrivate,
@@ -143,8 +141,7 @@
ImmutableList.copyOf(allPastReviewers),
ImmutableList.copyOf(reviewerUpdates),
ImmutableList.copyOf(submitRecords),
- ImmutableList.copyOf(allChangeMessages),
- ImmutableListMultimap.copyOf(changeMessagesByPatchSet),
+ ImmutableList.copyOf(changeMessages),
ImmutableListMultimap.copyOf(publishedComments),
readOnlyUntil);
}
@@ -233,9 +230,7 @@
abstract ImmutableList<SubmitRecord> submitRecords();
- abstract ImmutableList<ChangeMessage> allChangeMessages();
-
- abstract ImmutableListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet();
+ abstract ImmutableList<ChangeMessage> changeMessages();
abstract ImmutableListMultimap<RevId, Comment> publishedComments();
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 9cbf946..9d38704 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -1078,7 +1078,6 @@
ChangeNotes notes = newNotes(c);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1, psId2);
assertThat(notes.getApprovals()).isNotEmpty();
- assertThat(notes.getChangeMessagesByPatchSet()).isNotEmpty();
assertThat(notes.getChangeMessages()).isNotEmpty();
assertThat(notes.getComments()).isNotEmpty();
@@ -1095,7 +1094,6 @@
notes = newNotes(c);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1);
assertThat(notes.getApprovals()).isEmpty();
- assertThat(notes.getChangeMessagesByPatchSet()).isEmpty();
assertThat(notes.getChangeMessages()).isEmpty();
assertThat(notes.getComments()).isEmpty();
}
@@ -1349,16 +1347,12 @@
update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
update.setChangeMessage("Just a little code change.\n");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages.keySet()).containsExactly(ps1);
-
- ChangeMessage cm = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm.getMessage()).isEqualTo("Just a little code change.\n");
assertThat(cm.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
- assertThat(cm.getPatchSetId()).isEqualTo(ps1);
+ assertThat(cm.getPatchSetId()).isEqualTo(c.currentPatchSetId());
}
@Test
@@ -1378,13 +1372,9 @@
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Testing trailing double newline\n\n");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(1);
-
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm1.getMessage()).isEqualTo("Testing trailing double newline\n\n");
assertThat(cm1.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
}
@@ -1395,13 +1385,9 @@
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Testing paragraph 1\n\nTesting paragraph 2\n\nTesting paragraph 3");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(1);
-
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm1.getMessage())
.isEqualTo(
"Testing paragraph 1\n"
@@ -1429,15 +1415,15 @@
PatchSet.Id ps2 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(2);
+ assertThat(notes.getChangeMessages()).hasSize(2);
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = notes.getChangeMessages().get(0);
+ assertThat(cm1.getPatchSetId()).isEqualTo(ps1);
assertThat(cm1.getMessage()).isEqualTo("This is the change message for the first PS.");
assertThat(cm1.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
- ChangeMessage cm2 = Iterables.getOnlyElement(changeMessages.get(ps2));
- assertThat(cm1.getPatchSetId()).isEqualTo(ps1);
+ ChangeMessage cm2 = notes.getChangeMessages().get(1);
+ assertThat(cm2.getPatchSetId()).isEqualTo(ps2);
assertThat(cm2.getMessage()).isEqualTo("This is the change message for the second PS.");
assertThat(cm2.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
assertThat(cm2.getPatchSetId()).isEqualTo(ps2);
@@ -1459,10 +1445,8 @@
update.commit();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages.keySet()).hasSize(1);
- List<ChangeMessage> cm = changeMessages.get(ps1);
+ List<ChangeMessage> cm = notes.getChangeMessages();
assertThat(cm).hasSize(2);
assertThat(cm.get(0).getMessage()).isEqualTo("First change message.\n");
assertThat(cm.get(0).getAuthor()).isEqualTo(changeOwner.getAccount().getId());
