diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index d99a5b897a..9a4921395d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -22,20 +22,17 @@ import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Predicate; -import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.collect.Ordering; -import com.google.common.collect.Tables; import com.google.common.util.concurrent.AsyncFunction; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; @@ -44,7 +41,6 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchLineComment; @@ -54,7 +50,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.git.RefCache; import com.google.gerrit.server.git.RepoRefCache; @@ -378,15 +373,7 @@ public class ChangeNotes extends AbstractChangeNotes { private final RefCache refs; private Change change; - private ImmutableSortedMap patchSets; - private ImmutableListMultimap approvals; - private ReviewerSet reviewers; - private ImmutableList allPastReviewers; - private ImmutableList submitRecords; - private ImmutableList allChangeMessages; - private ImmutableListMultimap changeMessagesByPatchSet; - private ImmutableListMultimap comments; - private ImmutableSet hashtags; + private ChangeNotesState state; // Parsed note map state, used by ChangeUpdate to make in-place editing of // notes easier. @@ -411,15 +398,15 @@ public class ChangeNotes extends AbstractChangeNotes { } public ImmutableMap getPatchSets() { - return patchSets; + return state.patchSets(); } public ImmutableListMultimap getApprovals() { - return approvals; + return state.approvals(); } public ReviewerSet getReviewers() { - return reviewers; + return state.reviewers(); } /** @@ -427,14 +414,14 @@ public class ChangeNotes extends AbstractChangeNotes { * @return a ImmutableSet of all hashtags for this change sorted in alphabetical order. */ public ImmutableSet getHashtags() { - return ImmutableSortedSet.copyOf(hashtags); + return ImmutableSortedSet.copyOf(state.hashtags()); } /** * @return a list of all users who have ever been a reviewer on this change. */ public ImmutableList getAllPastReviewers() { - return allPastReviewers; + return state.allPastReviewers(); } /** @@ -442,12 +429,12 @@ public class ChangeNotes extends AbstractChangeNotes { * changes that were actually submitted. */ public ImmutableList getSubmitRecords() { - return submitRecords; + return state.submitRecords(); } /** @return all change messages, in chronological order, oldest first. */ public ImmutableList getChangeMessages() { - return allChangeMessages; + return state.allChangeMessages(); } /** @@ -456,18 +443,19 @@ public class ChangeNotes extends AbstractChangeNotes { */ public ImmutableListMultimap getChangeMessagesByPatchSet() { - return changeMessagesByPatchSet; + return state.changeMessagesByPatchSet(); } /** @return inline comments on each revision. */ public ImmutableListMultimap getComments() { - return comments; + return state.publishedComments(); } public ImmutableListMultimap getDraftComments( Account.Id author) throws OrmException { loadDraftComments(author); - final Multimap published = comments; + final Multimap published = + state.publishedComments(); // Filter out any draft comments that also exist in the published map, in // case the update to All-Users to delete them during the publish operation // failed. @@ -533,7 +521,7 @@ public class ChangeNotes extends AbstractChangeNotes { public PatchSet getCurrentPatchSet() { PatchSet.Id psId = change.currentPatchSetId(); - return checkNotNull(patchSets.get(psId), + return checkNotNull(state.patchSets().get(psId), "missing current patch set %s", psId.get()); } @@ -547,57 +535,14 @@ public class ChangeNotes extends AbstractChangeNotes { } ChangeNotesParser parser = new ChangeNotesParser( change.getId(), rev, handle.walk(), args.noteUtil, args.metrics); - parser.parseAll(); - - if (parser.status != null) { - change.setStatus(parser.status); - } - approvals = parser.buildApprovals(); - changeMessagesByPatchSet = parser.buildMessagesByPatchSet(); - allChangeMessages = parser.buildAllMessages(); - comments = ImmutableListMultimap.copyOf(parser.comments); - revisionNoteMap = parser.revisionNoteMap; - change.setKey(new Change.Key(parser.changeId)); - change.setDest(new Branch.NameKey(change.getProject(), parser.branch)); - change.setTopic(Strings.emptyToNull(parser.topic)); - change.setCreatedOn(parser.createdOn); - change.setLastUpdatedOn(parser.lastUpdatedOn); - change.setOwner(parser.ownerId); - change.setSubmissionId(parser.submissionId); - patchSets = ImmutableSortedMap.copyOf( - parser.patchSets, ReviewDbUtil.intKeyOrdering()); - - if (!patchSets.isEmpty()) { - change.setCurrentPatchSet( - parser.currentPatchSetId, parser.subject, parser.originalSubject); - } else { - // TODO(dborowitz): This should be an error, but for now it's required for - // some tests to pass. - change.clearCurrentPatchSet(); - } - - if (parser.hashtags != null) { - hashtags = ImmutableSet.copyOf(parser.hashtags); - } else { - hashtags = ImmutableSet.of(); - } - this.reviewers = ReviewerSet.fromTable(Tables.transpose(parser.reviewers)); - this.allPastReviewers = ImmutableList.copyOf(parser.allPastReviewers); - - submitRecords = ImmutableList.copyOf(parser.submitRecords); + state = parser.parseAll(); + state.copyColumnsTo(change); + revisionNoteMap = parser.getRevisionNoteMap(); } @Override protected void loadDefaults() { - approvals = ImmutableListMultimap.of(); - reviewers = ReviewerSet.empty(); - submitRecords = ImmutableList.of(); - allChangeMessages = ImmutableList.of(); - changeMessagesByPatchSet = ImmutableListMultimap.of(); - comments = ImmutableListMultimap.of(); - hashtags = ImmutableSet.of(); - patchSets = ImmutableSortedMap.of(); - allPastReviewers = ImmutableList.of(); + state = ChangeNotesState.empty(change); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 02dd441aa5..af16f472ca 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -37,8 +37,6 @@ import com.google.common.base.Splitter; import com.google.common.base.Supplier; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.HashBasedTable; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.Lists; @@ -61,6 +59,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDbUtil; +import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk; import com.google.gerrit.server.util.LabelVote; @@ -95,38 +94,42 @@ class ChangeNotesParser { private static final RevId PARTIAL_PATCH_SET = new RevId("INVALID PARTIAL PATCH SET"); - final Table reviewers; - final List allPastReviewers; - final List submitRecords; - final Multimap comments; - final TreeMap patchSets; - final Map patchSetStates; - - String branch; - Change.Status status; - String topic; - Set hashtags; - Timestamp createdOn; - Timestamp lastUpdatedOn; - Account.Id ownerId; - String changeId; - String subject; - String originalSubject; - String submissionId; - String tag; - PatchSet.Id currentPatchSetId; - RevisionNoteMap revisionNoteMap; - + // Private final members initialized in the constructor. private final ChangeNoteUtil noteUtil; private final NoteDbMetrics metrics; private final Change.Id id; private final ObjectId tip; private final ChangeNotesRevWalk walk; + + // Private final but mutable members initialized in the constructor and filled + // in during the parsing process. + private final Table reviewers; + private final List allPastReviewers; + private final List submitRecords; + private final Multimap comments; + private final TreeMap patchSets; + private final Map patchSetStates; private final Map, Optional>> approvals; private final List allChangeMessages; private final Multimap changeMessagesByPatchSet; + // Non-final private members filled in during the parsing process. + private String branch; + private Change.Status status; + private String topic; + private Set hashtags; + private Timestamp createdOn; + private Timestamp lastUpdatedOn; + private Account.Id ownerId; + private String changeId; + private String subject; + private String originalSubject; + private String submissionId; + private String tag; + private PatchSet.Id currentPatchSetId; + private RevisionNoteMap revisionNoteMap; + ChangeNotesParser(Change.Id changeId, ObjectId tip, ChangeNotesRevWalk walk, ChangeNoteUtil noteUtil, NoteDbMetrics metrics) { this.id = changeId; @@ -145,7 +148,8 @@ class ChangeNotesParser { patchSetStates = new HashMap<>(); } - void parseAll() throws ConfigInvalidException, IOException { + ChangeNotesState parseAll() + throws ConfigInvalidException, IOException { // Don't include initial parse in timer, as this might do more I/O to page // in the block containing most commits. Later reads are not guaranteed to // avoid I/O, but often should. @@ -162,10 +166,41 @@ class ChangeNotesParser { updatePatchSetStates(); checkMandatoryFooters(); } + + return buildState(); } - ImmutableListMultimap - buildApprovals() { + RevisionNoteMap getRevisionNoteMap() { + return revisionNoteMap; + } + + private ChangeNotesState buildState() { + return ChangeNotesState.create( + id, + new Change.Key(changeId), + createdOn, + lastUpdatedOn, + ownerId, + branch, + currentPatchSetId, + subject, + topic, + originalSubject, + submissionId, + status, + + hashtags, + patchSets, + buildApprovals(), + ReviewerSet.fromTable(Tables.transpose(reviewers)), + allPastReviewers, + submitRecords, + buildAllMessages(), + buildMessagesByPatchSet(), + comments); + } + + private Multimap buildApprovals() { Multimap result = ArrayListMultimap.create(approvals.keySet().size(), 3); for (Table> curr : approvals.values()) { @@ -178,19 +213,19 @@ class ChangeNotesParser { for (Collection v : result.asMap().values()) { Collections.sort((List) v, ChangeNotes.PSA_BY_TIME); } - return ImmutableListMultimap.copyOf(result); + return result; } - ImmutableList buildAllMessages() { - return ImmutableList.copyOf(Lists.reverse(allChangeMessages)); + private List buildAllMessages() { + return Lists.reverse(allChangeMessages); } - ImmutableListMultimap buildMessagesByPatchSet() { + private Multimap buildMessagesByPatchSet() { for (Collection v : changeMessagesByPatchSet.asMap().values()) { Collections.reverse((List) v); } - return ImmutableListMultimap.copyOf(changeMessagesByPatchSet); + return changeMessagesByPatchSet; } private void parse(ChangeNotesCommit commit) throws ConfigInvalidException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java new file mode 100644 index 0000000000..baa0a39ba9 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -0,0 +1,185 @@ +// Copyright (C) 2016 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.notedb; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Multimap; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.data.SubmitRecord; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.ChangeMessage; +import com.google.gerrit.reviewdb.client.PatchLineComment; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.reviewdb.server.ReviewDbUtil; +import com.google.gerrit.server.ReviewerSet; + +import java.sql.Timestamp; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Immutable state associated with a change meta ref at a given commit. + *

+ * One instance is the output of a single {@link ChangeNotesParser}, and + * contains types required to support public methods on {@link ChangeNotes}. It + * is intended to be cached in-process. + *

+ * Note that {@link ChangeNotes} contains more than just a single {@code + * ChangeNoteState}, such as per-draft information, so that class is not cached + * directly. + */ +@AutoValue +abstract class ChangeNotesState { + static ChangeNotesState empty(Change change) { + return new AutoValue_ChangeNotesState( + change.getId(), + null, + ImmutableSet.of(), + ImmutableSortedMap.of(), + ImmutableListMultimap.of(), + ReviewerSet.empty(), + ImmutableList.of(), + ImmutableList.of(), + ImmutableList.of(), + ImmutableListMultimap.of(), + ImmutableListMultimap.of()); + } + + static ChangeNotesState create( + Change.Id changeId, + Change.Key changeKey, + Timestamp createdOn, + Timestamp lastUpdatedOn, + Account.Id owner, + String branch, + @Nullable PatchSet.Id currentPatchSetId, + String subject, + @Nullable String topic, + @Nullable String originalSubject, + @Nullable String submissionId, + @Nullable Change.Status status, + @Nullable Set hashtags, + Map patchSets, + Multimap approvals, + ReviewerSet reviewers, + List allPastReviewers, + List submitRecords, + List allChangeMessages, + Multimap changeMessagesByPatchSet, + Multimap publishedComments) { + if (hashtags == null) { + hashtags = ImmutableSet.of(); + } + return new AutoValue_ChangeNotesState( + changeId, + new AutoValue_ChangeNotesState_ChangeColumns( + changeKey, + createdOn, + lastUpdatedOn, + owner, + branch, + currentPatchSetId, + subject, + topic, + originalSubject, + submissionId, + status), + ImmutableSet.copyOf(hashtags), + ImmutableSortedMap.copyOf(patchSets, ReviewDbUtil.intKeyOrdering()), + ImmutableListMultimap.copyOf(approvals), + reviewers, + ImmutableList.copyOf(allPastReviewers), + ImmutableList.copyOf(submitRecords), + ImmutableList.copyOf(allChangeMessages), + ImmutableListMultimap.copyOf(changeMessagesByPatchSet), + ImmutableListMultimap.copyOf(publishedComments)); + } + + + /** + * Subset of Change columns that can be represented in NoteDb. + *

+ * Notable exceptions include rowVersion and noteDbState, which are only make + * sense when read from NoteDb, so they cannot be cached. + *

+ * Fields are in listed column order. + */ + @AutoValue + static abstract class ChangeColumns { + abstract Change.Key changeKey(); + abstract Timestamp createdOn(); + abstract Timestamp lastUpdatedOn(); + abstract Account.Id owner(); + abstract String branch(); // Project not included. + @Nullable abstract PatchSet.Id currentPatchSetId(); + abstract String subject(); + @Nullable abstract String topic(); + @Nullable abstract String originalSubject(); + @Nullable abstract String submissionId(); + // TODO(dborowitz): Use a sensible default other than null + @Nullable abstract Change.Status status(); + } + + abstract Change.Id changeId(); + + @Nullable abstract ChangeColumns columns(); + + // Other related to this Change. + abstract ImmutableSet hashtags(); + abstract ImmutableSortedMap patchSets(); + abstract ImmutableListMultimap approvals(); + abstract ReviewerSet reviewers(); + abstract ImmutableList allPastReviewers(); + abstract ImmutableList submitRecords(); + abstract ImmutableList allChangeMessages(); + abstract ImmutableListMultimap + changeMessagesByPatchSet(); + abstract ImmutableListMultimap publishedComments(); + + void copyColumnsTo(Change change) { + ChangeColumns c = checkNotNull(columns()); + if (c.status() != null) { + change.setStatus(c.status()); + } + change.setKey(c.changeKey()); + change.setDest(new Branch.NameKey(change.getProject(), c.branch())); + change.setTopic(Strings.emptyToNull(c.topic())); + change.setCreatedOn(c.createdOn()); + change.setLastUpdatedOn(c.lastUpdatedOn()); + change.setOwner(c.owner()); + change.setSubmissionId(c.submissionId()); + + if (!patchSets().isEmpty()) { + change.setCurrentPatchSet( + c.currentPatchSetId(), c.subject(), c.originalSubject()); + } else { + // TODO(dborowitz): This should be an error, but for now it's required for + // some tests to pass. + change.clearCurrentPatchSet(); + } + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index 16cc3b9ef6..ab37ec9ae8 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.notedb; import static org.junit.Assert.fail; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -311,21 +312,21 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { public void parseCommit() throws Exception { assertParseSucceeds("Update change\n" + "\n" - + "Patch-set: 1\n" + + "Patch-set: 2\n" + "Branch: refs/heads/master\n" + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + "Subject: Some subject of a change\n" + "Commit: abcd1234abcd1234abcd1234abcd1234abcd1234"); assertParseFails("Update change\n" + "\n" - + "Patch-set: 1\n" + + "Patch-set: 2\n" + "Branch: refs/heads/master\n" + "Subject: Some subject of a change\n" + "Commit: abcd1234abcd1234abcd1234abcd1234abcd1234\n" + "Commit: deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); assertParseFails("Update patch set 1\n" + "Uploaded patch set 1.\n" - + "Patch-set: 1\n" + + "Patch-set: 2\n" + "Branch: refs/heads/master\n" + "Subject: Some subject of a change\n" + "Commit: beef"); @@ -362,22 +363,15 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { public void parsePatchSetGroups() throws Exception { assertParseSucceeds("Update change\n" + "\n" - + "Patch-set: 1\n" + + "Patch-set: 2\n" + "Branch: refs/heads/master\n" + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + "Commit: abcd1234abcd1234abcd1234abcd1234abcd1234\n" + "Subject: Change subject\n" + "Groups: a,b,c\n"); - // No patch set commit parsed on which we can set groups. assertParseFails("Update change\n" + "\n" - + "Patch-set: 1\n" - + "Branch: refs/heads/master\n" - + "Subject: Change subject\n" - + "Groups: a,b,c\n"); - assertParseFails("Update change\n" - + "\n" - + "Patch-set: 1\n" + + "Patch-set: 2\n" + "Branch: refs/heads/master\n" + "Commit: abcd1234abcd1234abcd1234abcd1234abcd1234\n" + "Subject: Change subject\n" @@ -462,8 +456,11 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { private RevCommit writeCommit(String body, PersonIdent author) throws Exception { + Change change = newChange(); + ChangeNotes notes = newNotes(change).load(); try (ObjectInserter ins = testRepo.getRepository().newObjectInserter()) { CommitBuilder cb = new CommitBuilder(); + cb.setParentId(notes.getRevision()); cb.setAuthor(author); cb.setCommitter(new PersonIdent(serverIdent, author.getWhen())); cb.setTreeId(testRepo.tree()); @@ -498,6 +495,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { } private ChangeNotesParser newParser(ObjectId tip) throws Exception { + walk.reset(); return new ChangeNotesParser( newChange().getId(), tip, walk, noteUtil, args.metrics); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 869324eec9..3f03668f95 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -1056,21 +1056,17 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) { ChangeNotesParser notesWithComments = new ChangeNotesParser( c.getId(), commitWithComments.copy(), rw, noteUtil, args.metrics); - notesWithComments.parseAll(); - ImmutableListMultimap approvals1 = - notesWithComments.buildApprovals(); - assertThat(approvals1).isEmpty(); - assertThat(notesWithComments.comments).hasSize(1); + ChangeNotesState state = notesWithComments.parseAll(); + assertThat(state.approvals()).isEmpty(); + assertThat(state.publishedComments()).hasSize(1); } try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) { ChangeNotesParser notesWithApprovals = new ChangeNotesParser(c.getId(), commitWithApprovals.copy(), rw, noteUtil, args.metrics); - notesWithApprovals.parseAll(); - ImmutableListMultimap approvals2 = - notesWithApprovals.buildApprovals(); - assertThat(approvals2).hasSize(1); - assertThat(notesWithApprovals.comments).hasSize(1); + ChangeNotesState state = notesWithApprovals.parseAll(); + assertThat(state.approvals()).hasSize(1); + assertThat(state.publishedComments()).hasSize(1); } }