opendev/gerrit
Code Issues Proposed changes

Modify PatchLineComment parser to incorporate status

Browse Source 
In anticipation of the ability to write draft PatchLineComments to
NoteMaps, I modified some of the parsing code for reading comments out
of NoteMaps to take status as a parameter so that when it parses a
List of PatchLineComments out of the notes, they have the correct
status. This is valid to do because all comments in the same note
will all have the same status.

Additionally, I had to modify the ChangeNotesTest class in combination
with this to make sure that the comments being created for testing
purposes had the status flag set correctly.

Change-Id: If32e7075427abd2e6f02721824e8099243806243
This commit is contained in:
Yacob Yonas
2014-07-14 14:09:48 -07:00
parent 9733ae83b3
commit 2ef433bc92
3 changed files with 51 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@@ -44,6 +44,7 @@ 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.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
@@ -323,7 +324,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
throws IOException, ConfigInvalidException, ParseException {
commentNoteMap = CommentsInNotesUtil.parseCommentsFromNotes(repo,
ChangeNoteUtil.changeRefName(changeId), walk, changeId,
commentsForBase, commentsForPs);
commentsForBase, commentsForPs, Status.PUBLISHED);
}
private void parseApproval(PatchSet.Id psId, Account.Id accountId,

13
gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
View File

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent;
@@ -82,7 +83,8 @@ public class CommentsInNotesUtil {
public static NoteMap parseCommentsFromNotes(Repository repo, String refName,
RevWalk walk, Change.Id changeId,
Multimap<PatchSet.Id, PatchLineComment> commentsForBase,
Multimap<PatchSet.Id, PatchLineComment> commentsForPs)
Multimap<PatchSet.Id, PatchLineComment> commentsForPs,
Status status)
throws IOException, ConfigInvalidException {
Ref ref = repo.getRef(refName);
if (ref == null) {
@@ -94,7 +96,7 @@ public class CommentsInNotesUtil {
for (Note note: noteMap) {
byte[] bytes = walk.getObjectReader().open(
note.getData(), Constants.OBJ_BLOB).getBytes();
List<PatchLineComment> result = parseNote(bytes, changeId);
List<PatchLineComment> result = parseNote(bytes, changeId, status);
if ((result == null) || (result.isEmpty())) {
continue;
}
@@ -110,7 +112,7 @@ public class CommentsInNotesUtil {
}
public static List<PatchLineComment> parseNote(byte[] note,
Change.Id changeId) throws ConfigInvalidException {
Change.Id changeId, Status status) throws ConfigInvalidException {
List<PatchLineComment> result = Lists.newArrayList();
int sizeOfNote = note.length;
Charset enc = RawParseUtils.parseEncoding(note);
@@ -131,7 +133,7 @@ public class CommentsInNotesUtil {
String previousFileName = c == null ?
null : c.getKey().getParentKey().getFileName();
c = parseComment(note, curr, previousFileName, psId, revId,
isForBase, enc);
isForBase, enc, status);
result.add(c);
}
return result;
@@ -150,7 +152,7 @@ public class CommentsInNotesUtil {
private static PatchLineComment parseComment(byte[] note, MutableInteger curr,
String currentFileName, PatchSet.Id psId, RevId revId, boolean isForBase,
Charset enc)
Charset enc, Status status)
throws ConfigInvalidException {
Change.Id changeId = psId.getParentKey();
@@ -195,6 +197,7 @@ public class CommentsInNotesUtil {
plc.setRange(range);
}
plc.setRevId(revId);
plc.setStatus(status);
curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
curr.value = RawParseUtils.nextLF(note, curr.value);

72
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -38,6 +38,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
@@ -853,8 +854,8 @@ public class ChangeNotesTest {
Timestamp time3 = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPatchLineComment(psId, "file1", uuid,
range1, range1.getEndLine(), otherUser, null, time1, message1,
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment1);
@@ -862,8 +863,8 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPatchLineComment(psId, "file1", uuid,
range2, range2.getEndLine(), otherUser, null, time2, message2,
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment2);
@@ -871,8 +872,8 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range3 = new CommentRange(3, 1, 4, 1);
PatchLineComment comment3 = newPatchLineComment(psId, "file2", uuid,
range3, range3.getEndLine(), otherUser, null, time3, message3,
PatchLineComment comment3 = newPublishedPatchLineComment(psId, "file2",
uuid, range3, range3.getEndLine(), otherUser, null, time3, message3,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment3);
@@ -931,8 +932,8 @@ public class ChangeNotesTest {
Timestamp time2 = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPatchLineComment(psId, "file1", uuid,
range1, range1.getEndLine(), otherUser, null, time1, message1,
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment1);
@@ -940,8 +941,8 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPatchLineComment(psId, "file1", uuid,
range2, range2.getEndLine(), otherUser, null, time2, message2,
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment2);
@@ -993,7 +994,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment commentForBase =
newPatchLineComment(psId, "filename", uuid,
newPublishedPatchLineComment(psId, "filename", uuid,
range, range.getEndLine(), otherUser, null, now, messageForBase,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
@@ -1002,7 +1003,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
PatchLineComment commentForPS =
newPatchLineComment(psId, "filename", uuid,
newPublishedPatchLineComment(psId, "filename", uuid,
range, range.getEndLine(), otherUser, null, now, messageForPS,
(short) 1, "abcd4567abcd4567abcd4567abcd4567abcd4567");
update.setPatchSetId(psId);
@@ -1035,17 +1036,17 @@ public class ChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp timeForComment1 = TimeUtil.nowTs();
Timestamp timeForComment2 = TimeUtil.nowTs();
PatchLineComment comment1 = newPatchLineComment(psId, filename, uuid, range,
range.getEndLine(), otherUser, null, timeForComment1, "comment 1", side,
"abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment1,
"comment 1", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment1);
update.commit();
update = newUpdate(c, otherUser);
PatchLineComment comment2 = newPatchLineComment(psId, filename, uuid, range,
range.getEndLine(), otherUser, null, timeForComment2, "comment 2", side,
"abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment2,
"comment 2", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment2);
update.commit();
@@ -1081,17 +1082,17 @@ public class ChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
PatchLineComment comment1 = newPatchLineComment(psId, filename1, uuid,
range, range.getEndLine(), otherUser, null, now, "comment 1", side,
"abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename1,
uuid, range, range.getEndLine(), otherUser, null, now, "comment 1",
side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment1);
update.commit();
update = newUpdate(c, otherUser);
PatchLineComment comment2 = newPatchLineComment(psId, filename2, uuid,
range, range.getEndLine(), otherUser, null, now, "comment 2", side,
"abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename2,
uuid, range, range.getEndLine(), otherUser, null, now, "comment 2",
side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.putComment(comment2);
update.commit();
@@ -1125,9 +1126,9 @@ public class ChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
PatchLineComment comment1 = newPatchLineComment(ps1, filename, uuid,
range, range.getEndLine(), otherUser, null, now, "comment on ps1", side,
"abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment1 = newPublishedPatchLineComment(ps1, filename,
uuid, range, range.getEndLine(), otherUser, null, now, "comment on ps1",
side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(ps1);
update.putComment(comment1);
update.commit();
@@ -1137,9 +1138,9 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
now = TimeUtil.nowTs();
PatchLineComment comment2 = newPatchLineComment(ps2, filename, uuid,
range, range.getEndLine(), otherUser, null, now, "comment on ps2", side,
"abcd4567abcd4567abcd4567abcd4567abcd4567");
PatchLineComment comment2 = newPublishedPatchLineComment(ps2, filename,
uuid, range, range.getEndLine(), otherUser, null, now, "comment on ps2",
side, "abcd4567abcd4567abcd4567abcd4567abcd4567");
update.setPatchSetId(ps2);
update.putComment(comment2);
update.commit();
@@ -1168,10 +1169,18 @@ public class ChangeNotesTest {
return TestChanges.newChange(project, changeOwner);
}
private PatchLineComment newPatchLineComment(PatchSet.Id psId,
private PatchLineComment newPublishedPatchLineComment(PatchSet.Id psId,
String filename, String UUID, CommentRange range, int line,
IdentifiedUser commenter, String parentUUID, Timestamp t,
String message, short side, String commitSHA1) {
return newPatchLineComment(psId, filename, UUID, range, line, commenter,
parentUUID, t, message, side, commitSHA1, Status.PUBLISHED);
}
private PatchLineComment newPatchLineComment(PatchSet.Id psId,
String filename, String UUID, CommentRange range, int line,
IdentifiedUser commenter, String parentUUID, Timestamp t,
String message, short side, String commitSHA1, Status status) {
PatchLineComment comment = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(psId, filename), UUID),
@@ -1180,6 +1189,7 @@ public class ChangeNotesTest {
comment.setMessage(message);
comment.setRange(range);
comment.setRevId(new RevId(commitSHA1));
comment.setStatus(status);
return comment;
}