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 37be3655d2..e491ae3dbf 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.FooterKey; 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 @@ public class ChangeNotes extends VersionedMetaData { private final Map reviewers; private final List submitRecords; private Change.Status status; + private List changeMessages; private Parser(Change.Id changeId, ObjectId tip, RevWalk walk) { this.changeId = changeId; @@ -110,6 +113,7 @@ public class ChangeNotes extends VersionedMetaData { approvals = Maps.newHashMap(); reviewers = Maps.newLinkedHashMap(); submitRecords = Lists.newArrayListWithExpectedSize(1); + changeMessages = Lists.newArrayList(); } private void parseAll() throws ConfigInvalidException, IOException { @@ -142,6 +146,8 @@ public class ChangeNotes extends VersionedMetaData { } 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 @@ public class ChangeNotes extends VersionedMetaData { 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> curr = @@ -353,6 +409,7 @@ public class ChangeNotes extends VersionedMetaData { private ImmutableListMultimap approvals; private ImmutableSetMultimap reviewers; private ImmutableList submitRecords; + private ImmutableList changeMessages; @VisibleForTesting ChangeNotes(GitRepositoryManager repoManager, Change change) { @@ -405,6 +462,14 @@ public class ChangeNotes extends VersionedMetaData { return submitRecords; } + /** + * @return change messages in reverse chronological order. + */ + public ImmutableList getChangeMessages() { + return changeMessages; + + } + @Override protected String getRefName() { return ChangeNoteUtil.changeRefName(change.getId()); @@ -435,6 +500,7 @@ public class ChangeNotes extends VersionedMetaData { } this.reviewers = reviewers.build(); submitRecords = ImmutableList.copyOf(parser.submitRecords); + changeMessages = ImmutableList.copyOf(parser.changeMessages); } finally { walk.release(); } @@ -444,6 +510,7 @@ public class ChangeNotes extends VersionedMetaData { 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 74ca198cdb..12b276f40f 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 @@ public class ChangeUpdate extends VersionedMetaData { private String subject; private PatchSet.Id psId; private List submitRecords; + private String changeMessage; @AssistedInject private ChangeUpdate( @@ -186,6 +187,10 @@ public class ChangeUpdate extends VersionedMetaData { 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 @@ public class ChangeUpdate extends VersionedMetaData { 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 @@ public class ChangeUpdate extends VersionedMetaData { 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 f2ad057247..32a0f6d4cc 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 @@ -212,6 +212,31 @@ public class ChangeNotesTest { } } + @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(); @@ -608,6 +633,138 @@ public class ChangeNotesTest { 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 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 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 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 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 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(