From 9d1bd96c3f736c9793070c4e6b86f9e4c37b0f29 Mon Sep 17 00:00:00 2001 From: Yacob Yonas Date: Mon, 9 Jun 2014 14:27:47 -0700 Subject: [PATCH] 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 --- .../gerrit/server/notedb/ChangeNotes.java | 67 ++++++++ .../gerrit/server/notedb/ChangeUpdate.java | 15 +- .../gerrit/server/notedb/ChangeNotesTest.java | 157 ++++++++++++++++++ 3 files changed, 238 insertions(+), 1 deletion(-) 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(