ChangeNoteUtil: Distinguish between 0 and NaN

RawParseUtils.parseBase10 returns 0 if the input was invalid. The only
way to distinguish between this and a real 0 is by checking whether
the pointer advanced. This was preventing reading changes in NoteDb
with 0 as the line or character offset, which is valid.

Change-Id: I7124abe8a2a2543671bf9da0a51792d44a8ac381
This commit is contained in:
Dave Borowitz
2016-04-20 18:44:44 -04:00
parent 8462ab7e56
commit 2d07ee0143
2 changed files with 58 additions and 9 deletions

View File

@@ -264,8 +264,11 @@ public class ChangeNoteUtil {
private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) { private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) {
CommentRange range = new CommentRange(-1, -1, -1, -1); CommentRange range = new CommentRange(-1, -1, -1, -1);
int last = ptr.value;
int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr); int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (startLine == 0) { if (ptr.value == last) {
return null;
} else if (startLine == 0) {
range.setEndLine(0); range.setEndLine(0);
ptr.value += 1; ptr.value += 1;
return range; return range;
@@ -282,30 +285,33 @@ public class ChangeNoteUtil {
return null; return null;
} }
last = ptr.value;
int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr); int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (note[ptr.value] == '-') { if (ptr.value == last) {
return null;
} else if (note[ptr.value] == '-') {
range.setStartCharacter(startChar); range.setStartCharacter(startChar);
ptr.value += 1; ptr.value += 1;
} else { } else {
return null; return null;
} }
last = ptr.value;
int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr); int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (endLine == 0) { if (ptr.value == last) {
return null; return null;
} } else if (note[ptr.value] == ':') {
if (note[ptr.value] == ':') {
range.setEndLine(endLine); range.setEndLine(endLine);
ptr.value += 1; ptr.value += 1;
} else { } else {
return null; return null;
} }
last = ptr.value;
int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr); int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (endChar == 0) { if (ptr.value == last) {
return null; return null;
} } else if (note[ptr.value] == '\n') {
if (note[ptr.value] == '\n') {
range.setEndCharacter(endChar); range.setEndCharacter(endChar);
ptr.value += 1; ptr.value += 1;
} else { } else {
@@ -377,11 +383,15 @@ public class ChangeNoteUtil {
int startOfLength = int startOfLength =
RawParseUtils.endOfFooterLineKey(note, curr.value) + 1; RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
MutableInteger i = new MutableInteger(); MutableInteger i = new MutableInteger();
i.value = startOfLength;
int commentLength = int commentLength =
RawParseUtils.parseBase10(note, startOfLength, i); RawParseUtils.parseBase10(note, startOfLength, i);
if (i.value == startOfLength) {
throw parseException(changeId, "could not parse %s", LENGTH);
}
int endOfLine = RawParseUtils.nextLF(note, curr.value); int endOfLine = RawParseUtils.nextLF(note, curr.value);
if (i.value != endOfLine - 1) { if (i.value != endOfLine - 1) {
throw parseException(changeId, "could not parse %s", PATCH_SET); throw parseException(changeId, "could not parse %s", LENGTH);
} }
curr.value = endOfLine; curr.value = endOfLine;
return checkResult(commentLength, "comment length", changeId); return checkResult(commentLength, "comment length", changeId);

View File

@@ -1223,6 +1223,45 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(cm.get(1).getPatchSetId()).isEqualTo(ps1); assertThat(cm.get(1).getPatchSetId()).isEqualTo(ps1);
} }
@Test
public void patchLineCommentsFileComment() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234");
PatchLineComment comment = newPublishedComment(psId, "file1",
"uuid", null, 0, otherUser, null,
TimeUtil.nowTs(), "message", (short) 1, revId.get());
update.setPatchSetId(psId);
update.putComment(comment);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getComments())
.isEqualTo(ImmutableMultimap.of(revId, comment));
}
@Test
public void patchLineCommentsZeroColumns() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 0, 2, 0);
PatchLineComment comment = newPublishedComment(psId, "file1",
"uuid", range, range.getEndLine(), otherUser, null,
TimeUtil.nowTs(), "message", (short) 1, revId.get());
update.setPatchSetId(psId);
update.putComment(comment);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getComments())
.isEqualTo(ImmutableMultimap.of(revId, comment));
}
@Test @Test
public void patchLineCommentNotesFormatSide1() throws Exception { public void patchLineCommentNotesFormatSide1() throws Exception {
Change c = newChange(); Change c = newChange();