Store ChangeMessage in body of commit message Taught ChangeUpdate how to write out the ChangeMessage into the message. Also taught ChangeNotes how to parse the ChangeMessage out of the commit message. Change-Id: I563eabbd848364150fdf8a5c4af5a2e1562aaefe
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 37be365..e491ae3 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
@@ -58,8 +58,10 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.RawParseUtils; +import org.eclipse.jgit.util.StringUtils; import java.io.IOException; +import java.nio.charset.Charset; import java.sql.Timestamp; import java.util.Collection; import java.util.Collections; @@ -102,6 +104,7 @@ private final Map<Account.Id, ReviewerState> reviewers; private final List<SubmitRecord> submitRecords; private Change.Status status; + private List<String> changeMessages; private Parser(Change.Id changeId, ObjectId tip, RevWalk walk) { this.changeId = changeId; @@ -110,6 +113,7 @@ approvals = Maps.newHashMap(); reviewers = Maps.newLinkedHashMap(); submitRecords = Lists.newArrayListWithExpectedSize(1); + changeMessages = Lists.newArrayList(); } private void parseAll() throws ConfigInvalidException, IOException { @@ -142,6 +146,8 @@ } PatchSet.Id psId = parsePatchSetId(commit); Account.Id accountId = parseIdent(commit); + parseChangeMessage(commit); + if (submitRecords.isEmpty()) { // Only parse the most recent set of submit records; any older ones are @@ -189,6 +195,56 @@ return new PatchSet.Id(changeId, psId); } + private void parseChangeMessage(RevCommit commit) { + byte[] raw = commit.getRawBuffer(); + int size = raw.length; + Charset enc = RawParseUtils.parseEncoding(raw); + + int subjectStart = RawParseUtils.commitMessage(raw, 0); + if (subjectStart < 0 || subjectStart >= size) { + return; + } + + int subjectEnd = RawParseUtils.endOfParagraph(raw, subjectStart); + if (subjectEnd == size) { + return; + } + + int changeMessageStart; + + if (raw[subjectEnd] == '\n') { + changeMessageStart = subjectEnd + 2; //\n\n ends paragraph + } else if (raw[subjectEnd] == '\r') { + changeMessageStart = subjectEnd + 4; //\r\n\r\n ends paragraph + } else { + return; + } + + int ptr = size - 1; + int changeMessageEnd = -1; + while(ptr > changeMessageStart) { + ptr = RawParseUtils.prevLF(raw, ptr, '\r'); + if (ptr == -1) { + break; + } + if (raw[ptr] == '\n') { + changeMessageEnd = ptr - 1; + break; + } else if (raw[ptr] == '\r') { + changeMessageEnd = ptr - 3; + break; + } + } + + if (ptr <= changeMessageStart) { + return; + } + + String changeMessage = RawParseUtils.decode(enc, raw, + changeMessageStart, changeMessageEnd + 1); + changeMessages.add(changeMessage); + } + private void parseApproval(PatchSet.Id psId, Account.Id accountId, RevCommit commit, String line) throws ConfigInvalidException { Table<Account.Id, String, Optional<PatchSetApproval>> curr = @@ -353,6 +409,7 @@ private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals; private ImmutableSetMultimap<ReviewerState, Account.Id> reviewers; private ImmutableList<SubmitRecord> submitRecords; + private ImmutableList<String> changeMessages; @VisibleForTesting ChangeNotes(GitRepositoryManager repoManager, Change change) { @@ -405,6 +462,14 @@ return submitRecords; } + /** + * @return change messages in reverse chronological order. + */ + public ImmutableList<String> getChangeMessages() { + return changeMessages; + + } + @Override protected String getRefName() { return ChangeNoteUtil.changeRefName(change.getId()); @@ -435,6 +500,7 @@ } this.reviewers = reviewers.build(); submitRecords = ImmutableList.copyOf(parser.submitRecords); + changeMessages = ImmutableList.copyOf(parser.changeMessages); } finally { walk.release(); } @@ -444,6 +510,7 @@ approvals = ImmutableListMultimap.of(); reviewers = ImmutableSetMultimap.of(); submitRecords = ImmutableList.of(); + changeMessages = ImmutableList.of(); } @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index 74ca198..12b276f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -88,6 +88,7 @@ private String subject; private PatchSet.Id psId; private List<SubmitRecord> submitRecords; + private String changeMessage; @AssistedInject private ChangeUpdate( @@ -186,6 +187,10 @@ this.psId = psId; } + public void setChangeMessage(String changeMessage) { + this.changeMessage = changeMessage; + } + public void putReviewer(Account.Id reviewer, ReviewerState type) { checkArgument(type != ReviewerState.REMOVED, "invalid ReviewerType"); reviewers.put(reviewer, type); @@ -297,6 +302,13 @@ msg.append("Update patch set ").append(ps); } msg.append("\n\n"); + + if (changeMessage != null) { + msg.append(changeMessage); + msg.append("\n\n"); + } + + addFooter(msg, FOOTER_PATCH_SET, ps); if (status != null) { addFooter(msg, FOOTER_STATUS, status.name().toLowerCase()); @@ -352,7 +364,8 @@ return approvals.isEmpty() && reviewers.isEmpty() && status == null - && submitRecords == null; + && submitRecords == null + && changeMessage == null; } private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {
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 f2ad057..32a0f6d 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
@@ -213,6 +213,31 @@ } @Test + public void changeMessageCommitFormatSimple() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeMessage("Just a little code change.\n" + + "How about a new line"); + update.commit(); + assertEquals("refs/changes/01/1/meta", update.getRefName()); + + RevWalk walk = new RevWalk(repo); + try { + RevCommit commit = walk.parseCommit(update.getRevision()); + walk.parseBody(commit); + assertEquals("Update patch set 1\n" + + "\n" + + "Just a little code change.\n" + + "How about a new line\n" + + "\n" + + "Patch-set: 1\n", + commit.getFullMessage()); + } finally { + walk.release(); + } + } + + @Test public void approvalTombstoneCommitFormat() throws Exception { Change c = newChange(); ChangeUpdate update = newUpdate(c, changeOwner); @@ -608,6 +633,138 @@ assertEquals((short) 2, psas.get(1).getValue()); } + @Test + public void changeMessageOnePatchSet() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.putReviewer(changeOwner.getAccount().getId(), REVIEWER); + update.setChangeMessage("Just a little code change.\n"); + update.commit(); + + ChangeNotes notes = newNotes(c); + List<String> changeMessages = notes.getChangeMessages(); + assertEquals(1, changeMessages.size()); + assertEquals("Just a little code change.\n", changeMessages.get(0)); + } + + @Test + public void changeMessagesMultiplePatchSets() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.putReviewer(changeOwner.getAccount().getId(), REVIEWER); + update.setChangeMessage("This is the change message for the first PS."); + update.commit(); + PatchSet.Id ps1 = c.currentPatchSetId(); + + incrementPatchSet(c); + update = newUpdate(c, changeOwner); + + update.setChangeMessage("This is the change message for the second PS."); + update.commit(); + PatchSet.Id ps2 = c.currentPatchSetId(); + + ChangeNotes notes = newNotes(c); + List<String> changeMessages = notes.getChangeMessages(); + assertEquals(2, changeMessages.size()); + assertEquals("This is the change message for the second PS.", changeMessages.get(0)); + assertEquals("This is the change message for the first PS.", changeMessages.get(1)); + } + + @Test + public void noChangeMessage() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.putReviewer(changeOwner.getAccount().getId(), REVIEWER); + update.commit(); + + RevWalk walk = new RevWalk(repo); + try { + RevCommit commit = walk.parseCommit(update.getRevision()); + walk.parseBody(commit); + assertEquals("Update patch set 1\n" + + "\n" + + "Patch-set: 1\n" + + "Reviewer: Change Owner <1@gerrit>\n", + commit.getFullMessage()); + } finally { + walk.release(); + } + + ChangeNotes notes = newNotes(c); + List<String> changeMessages = notes.getChangeMessages(); + assertEquals(0, changeMessages.size()); + } + + @Test + public void changeMessageWithTrailingDoubleNewline() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeMessage("Testing trailing double newline\n" + + "\n"); + update.commit(); + + RevWalk walk = new RevWalk(repo); + try { + RevCommit commit = walk.parseCommit(update.getRevision()); + walk.parseBody(commit); + assertEquals("Update patch set 1\n" + + "\n" + + "Testing trailing double newline\n" + + "\n" + + "\n" + + "\n" + + "Patch-set: 1\n", + commit.getFullMessage()); + } finally { + walk.release(); + } + + ChangeNotes notes = newNotes(c); + List<String> changeMessages = notes.getChangeMessages(); + assertEquals(1, changeMessages.size()); + assertEquals("Testing trailing double newline\n" + + "\n", changeMessages.get(0)); + } + + @Test + public void changeMessageWithMultipleParagraphs() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeMessage("Testing paragraph 1\n" + + "\n" + + "Testing paragraph 2\n" + + "\n" + + "Testing paragraph 3"); + update.commit(); + + RevWalk walk = new RevWalk(repo); + try { + RevCommit commit = walk.parseCommit(update.getRevision()); + walk.parseBody(commit); + assertEquals("Update patch set 1\n" + + "\n" + + "Testing paragraph 1\n" + + "\n" + + "Testing paragraph 2\n" + + "\n" + + "Testing paragraph 3\n" + + "\n" + + "Patch-set: 1\n", + commit.getFullMessage()); + } finally { + walk.release(); + } + + ChangeNotes notes = newNotes(c); + List<String> changeMessages = notes.getChangeMessages(); + assertEquals(1, changeMessages.size()); + assertEquals("Testing paragraph 1\n" + + "\n" + + "Testing paragraph 2\n" + + "\n" + + "Testing paragraph 3", changeMessages.get(0)); + } + private Change newChange() { Change.Id changeId = new Change.Id(1); Change c = new Change(