Combine ChangeNoteUtil and CommentsInNotesUtil Now that we have to actually inject both of these into ChangeNotes, it was getting a little unwieldy. There was practically nothing in ChangeNoteUtil itself anyway. Merge CommentsInNotesUtil into ChangeNoteUtil so we only have one thing to inject. Change-Id: I41ef88dd2911b9d007f8b3a4cf7d3d7159fa25e6
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java index f86fe5a..9669a6b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -44,7 +44,7 @@ protected final GitRepositoryManager repoManager; protected final ChangeControl ctl; protected final String anonymousCowardName; - protected final ChangeNoteUtil changeNoteUtil; + protected final ChangeNoteUtil noteUtil; protected final Date when; private final PersonIdent serverIdent; @@ -56,14 +56,14 @@ ChangeControl ctl, PersonIdent serverIdent, String anonymousCowardName, - ChangeNoteUtil changeNoteUtil, + ChangeNoteUtil noteUtil, Date when) { this.migration = migration; this.repoManager = repoManager; this.ctl = ctl; this.serverIdent = serverIdent; this.anonymousCowardName = anonymousCowardName; - this.changeNoteUtil = changeNoteUtil; + this.noteUtil = noteUtil; this.when = when; checkArgument( (ctl.getUser() instanceof IdentifiedUser) @@ -99,7 +99,7 @@ private PersonIdent newAuthorIdent() { CurrentUser u = getUser(); if (u instanceof IdentifiedUser) { - return changeNoteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, + return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent, anonymousCowardName); } else if (u instanceof InternalUser) { return serverIdent; @@ -108,8 +108,7 @@ } protected PersonIdent newIdent(Account author, Date when) { - return changeNoteUtil.newIdent(author, when, serverIdent, - anonymousCowardName); + return noteUtil.newIdent(author, when, serverIdent, anonymousCowardName); } /** Whether no updates have been done. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java index 59133a5..1ab1d70 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -78,7 +78,6 @@ private final AllUsersName draftsProject; private final Account.Id accountId; - private final CommentsInNotesUtil commentsUtil; // TODO: can go back to a list? private Map<Key, PatchLineComment> put; @@ -91,14 +90,12 @@ GitRepositoryManager repoManager, NotesMigration migration, AllUsersName allUsers, - ChangeNoteUtil changeNoteUtil, - CommentsInNotesUtil commentsUtil, + ChangeNoteUtil noteUtil, @Assisted ChangeControl ctl, @Assisted Date when) { super(migration, repoManager, ctl, serverIdent, anonymousCowardName, - changeNoteUtil, when); + noteUtil, when); this.draftsProject = allUsers; - this.commentsUtil = commentsUtil; checkState(ctl.getUser().isIdentifiedUser(), "Current user must be identified"); IdentifiedUser user = ctl.getUser().asIdentifiedUser(); @@ -152,7 +149,7 @@ for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) { updatedRevs.add(e.getKey()); ObjectId id = ObjectId.fromString(e.getKey().get()); - byte[] data = e.getValue().build(commentsUtil); + byte[] data = e.getValue().build(noteUtil); if (data.length == 0) { rnm.noteMap.remove(id); } else { @@ -197,7 +194,7 @@ // Even though reading from changes might not be enabled, we need to // parse any existing revision notes so we can merge them. return RevisionNoteMap.parse( - commentsUtil, ctl.getId(), rw.getObjectReader(), noteMap, true); + noteUtil, ctl.getId(), rw.getObjectReader(), noteMap, true); } @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java index 7ab23a8..2071358 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -14,21 +14,49 @@ package com.google.gerrit.server.notedb; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.gerrit.server.notedb.ChangeNotes.parseException; +import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; import com.google.common.primitives.Ints; import com.google.gerrit.reviewdb.client.Account; 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.RefNames; +import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.PatchLineCommentsUtil; +import com.google.gerrit.server.account.AccountCache; +import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.GerritServerId; import com.google.inject.Inject; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.revwalk.FooterKey; +import org.eclipse.jgit.util.GitDateFormatter; +import org.eclipse.jgit.util.GitDateFormatter.Format; +import org.eclipse.jgit.util.GitDateParser; +import org.eclipse.jgit.util.MutableInteger; +import org.eclipse.jgit.util.QuotedString; +import org.eclipse.jgit.util.RawParseUtils; +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; +import java.sql.Timestamp; +import java.text.ParseException; import java.util.Date; +import java.util.List; +import java.util.Locale; public class ChangeNoteUtil { static final FooterKey FOOTER_BRANCH = new FooterKey("Branch"); @@ -45,6 +73,16 @@ new FooterKey("Submitted-with"); static final FooterKey FOOTER_TOPIC = new FooterKey("Topic"); + private static final String AUTHOR = "Author"; + private static final String BASE_PATCH_SET = "Base-for-patch-set"; + private static final String COMMENT_RANGE = "Comment-range"; + private static final String FILE = "File"; + private static final String LENGTH = "Bytes"; + private static final String PARENT = "Parent"; + private static final String PATCH_SET = "Patch-set"; + private static final String REVISION = "Revision"; + private static final String UUID = "UUID"; + public static String changeRefName(Change.Id id) { StringBuilder r = new StringBuilder(); r.append(RefNames.REFS_CHANGES); @@ -60,10 +98,26 @@ return r.toString(); } + public static String formatTime(PersonIdent ident, Timestamp t) { + GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT); + // TODO(dborowitz): Use a ThreadLocal or use Joda. + PersonIdent newIdent = new PersonIdent(ident, t); + return dateFormatter.formatDate(newIdent); + } + + private final AccountCache accountCache; + private final PersonIdent serverIdent; + private final String anonymousCowardName; private final String serverId; @Inject - ChangeNoteUtil(@GerritServerId String serverId) { + public ChangeNoteUtil(AccountCache accountCache, + @GerritPersonIdent PersonIdent serverIdent, + @AnonymousCowardName String anonymousCowardName, + @GerritServerId String serverId) { + this.accountCache = accountCache; + this.serverIdent = serverIdent; + this.anonymousCowardName = anonymousCowardName; this.serverId = serverId; } @@ -92,4 +146,361 @@ throw parseException(changeId, "invalid identity, expected <id>@%s: %s", serverId, email); } + + public List<PatchLineComment> parseNote(byte[] note, MutableInteger p, + Change.Id changeId, Status status) throws ConfigInvalidException { + if (p.value >= note.length) { + return ImmutableList.of(); + } + List<PatchLineComment> result = Lists.newArrayList(); + int sizeOfNote = note.length; + + boolean isForBase = + (RawParseUtils.match(note, p.value, PATCH_SET.getBytes(UTF_8))) < 0; + + PatchSet.Id psId = parsePsId(note, p, changeId, isForBase ? BASE_PATCH_SET : PATCH_SET); + + RevId revId = + new RevId(parseStringField(note, p, changeId, REVISION)); + + PatchLineComment c = null; + while (p.value < sizeOfNote) { + String previousFileName = c == null ? + null : c.getKey().getParentKey().getFileName(); + c = parseComment(note, p, previousFileName, psId, revId, + isForBase, status); + result.add(c); + } + return result; + } + + private PatchLineComment parseComment(byte[] note, MutableInteger curr, + String currentFileName, PatchSet.Id psId, RevId revId, boolean isForBase, + Status status) throws ConfigInvalidException { + Change.Id changeId = psId.getParentKey(); + + // Check if there is a new file. + boolean newFile = + (RawParseUtils.match(note, curr.value, FILE.getBytes(UTF_8))) != -1; + if (newFile) { + // If so, parse the new file name. + currentFileName = parseFilename(note, curr, changeId); + } else if (currentFileName == null) { + throw parseException(changeId, "could not parse %s", FILE); + } + + CommentRange range = parseCommentRange(note, curr); + if (range == null) { + throw parseException(changeId, "could not parse %s", COMMENT_RANGE); + } + + Timestamp commentTime = parseTimestamp(note, curr, changeId); + Account.Id aId = parseAuthor(note, curr, changeId); + + boolean hasParent = + (RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1; + String parentUUID = null; + if (hasParent) { + parentUUID = parseStringField(note, curr, changeId, PARENT); + } + + String uuid = parseStringField(note, curr, changeId, UUID); + int commentLength = parseCommentLength(note, curr, changeId); + + String message = RawParseUtils.decode( + UTF_8, note, curr.value, curr.value + commentLength); + checkResult(message, "message contents", changeId); + + PatchLineComment plc = new PatchLineComment( + new PatchLineComment.Key(new Patch.Key(psId, currentFileName), uuid), + range.getEndLine(), aId, parentUUID, commentTime); + plc.setMessage(message); + plc.setSide((short) (isForBase ? 0 : 1)); + if (range.getStartCharacter() != -1) { + plc.setRange(range); + } + plc.setRevId(revId); + plc.setStatus(status); + + curr.value = RawParseUtils.nextLF(note, curr.value + commentLength); + curr.value = RawParseUtils.nextLF(note, curr.value); + return plc; + } + + private static String parseStringField(byte[] note, MutableInteger curr, + Change.Id changeId, String fieldName) throws ConfigInvalidException { + int endOfLine = RawParseUtils.nextLF(note, curr.value); + checkHeaderLineFormat(note, curr, fieldName, changeId); + int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; + curr.value = endOfLine; + return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1); + } + + /** + * @return a comment range. If the comment range line in the note only has + * one number, we return a CommentRange with that one number as the end + * line and the other fields as -1. If the comment range line in the note + * contains a whole comment range, then we return a CommentRange with all + * fields set. If the line is not correctly formatted, return null. + */ + private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) { + CommentRange range = new CommentRange(-1, -1, -1, -1); + + int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr); + if (startLine == 0) { + range.setEndLine(0); + ptr.value += 1; + return range; + } + + if (note[ptr.value] == '\n') { + range.setEndLine(startLine); + ptr.value += 1; + return range; + } else if (note[ptr.value] == ':') { + range.setStartLine(startLine); + ptr.value += 1; + } else { + return null; + } + + int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr); + if (note[ptr.value] == '-') { + range.setStartCharacter(startChar); + ptr.value += 1; + } else { + return null; + } + + int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr); + if (endLine == 0) { + return null; + } + if (note[ptr.value] == ':') { + range.setEndLine(endLine); + ptr.value += 1; + } else { + return null; + } + + int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr); + if (endChar == 0) { + return null; + } + if (note[ptr.value] == '\n') { + range.setEndCharacter(endChar); + ptr.value += 1; + } else { + return null; + } + return range; + } + + private static PatchSet.Id parsePsId(byte[] note, MutableInteger curr, + Change.Id changeId, String fieldName) throws ConfigInvalidException { + checkHeaderLineFormat(note, curr, fieldName, changeId); + int startOfPsId = + RawParseUtils.endOfFooterLineKey(note, curr.value) + 1; + MutableInteger i = new MutableInteger(); + int patchSetId = + RawParseUtils.parseBase10(note, startOfPsId, i); + int endOfLine = RawParseUtils.nextLF(note, curr.value); + if (i.value != endOfLine - 1) { + throw parseException(changeId, "could not parse %s", fieldName); + } + checkResult(patchSetId, "patchset id", changeId); + curr.value = endOfLine; + return new PatchSet.Id(changeId, patchSetId); + } + + private static String parseFilename(byte[] note, MutableInteger curr, + Change.Id changeId) throws ConfigInvalidException { + checkHeaderLineFormat(note, curr, FILE, changeId); + int startOfFileName = + RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; + int endOfLine = RawParseUtils.nextLF(note, curr.value); + curr.value = endOfLine; + curr.value = RawParseUtils.nextLF(note, curr.value); + return QuotedString.GIT_PATH.dequote( + RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1)); + } + + private static Timestamp parseTimestamp(byte[] note, MutableInteger curr, + Change.Id changeId) throws ConfigInvalidException { + int endOfLine = RawParseUtils.nextLF(note, curr.value); + Timestamp commentTime; + String dateString = + RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1); + try { + commentTime = new Timestamp( + GitDateParser.parse(dateString, null, Locale.US).getTime()); + } catch (ParseException e) { + throw new ConfigInvalidException("could not parse comment timestamp", e); + } + curr.value = endOfLine; + return checkResult(commentTime, "comment timestamp", changeId); + } + + private Account.Id parseAuthor(byte[] note, MutableInteger curr, + Change.Id changeId) throws ConfigInvalidException { + checkHeaderLineFormat(note, curr, AUTHOR, changeId); + int startOfAccountId = + RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; + PersonIdent ident = + RawParseUtils.parsePersonIdent(note, startOfAccountId); + Account.Id aId = parseIdent(ident, changeId); + curr.value = RawParseUtils.nextLF(note, curr.value); + return checkResult(aId, "comment author", changeId); + } + + private static int parseCommentLength(byte[] note, MutableInteger curr, + Change.Id changeId) throws ConfigInvalidException { + checkHeaderLineFormat(note, curr, LENGTH, changeId); + int startOfLength = + RawParseUtils.endOfFooterLineKey(note, curr.value) + 1; + MutableInteger i = new MutableInteger(); + int commentLength = + RawParseUtils.parseBase10(note, startOfLength, i); + int endOfLine = RawParseUtils.nextLF(note, curr.value); + if (i.value != endOfLine-1) { + throw parseException(changeId, "could not parse %s", PATCH_SET); + } + curr.value = endOfLine; + return checkResult(commentLength, "comment length", changeId); + } + + private static <T> T checkResult(T o, String fieldName, + Change.Id changeId) throws ConfigInvalidException { + if (o == null) { + throw parseException(changeId, "could not parse %s", fieldName); + } + return o; + } + + private static int checkResult(int i, String fieldName, Change.Id changeId) + throws ConfigInvalidException { + if (i <= 0) { + throw parseException(changeId, "could not parse %s", fieldName); + } + return i; + } + + private void appendHeaderField(PrintWriter writer, + String field, String value) { + writer.print(field); + writer.print(": "); + writer.print(value); + writer.print('\n'); + } + + private static void checkHeaderLineFormat(byte[] note, MutableInteger curr, + String fieldName, Change.Id changeId) throws ConfigInvalidException { + boolean correct = + RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1; + int p = curr.value + fieldName.length(); + correct &= (p < note.length && note[p] == ':'); + p++; + correct &= (p < note.length && note[p] == ' '); + if (!correct) { + throw parseException(changeId, "could not parse %s", fieldName); + } + } + + public byte[] buildNote(List<PatchLineComment> comments) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + buildNote(comments, out); + return out.toByteArray(); + } + + /** + * Build a note that contains the metadata for and the contents of all of the + * comments in the given list of comments. + * + * @param comments A list of the comments to be written to the + * output stream. All of the comments in this list must have the + * same side and must share the same patch set ID. + * @param out output stream to write to. + */ + void buildNote(List<PatchLineComment> comments, OutputStream out) { + if (comments.isEmpty()) { + return; + } + OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8); + try (PrintWriter writer = new PrintWriter(streamWriter)) { + PatchLineComment first = comments.get(0); + + short side = first.getSide(); + PatchSet.Id psId = PatchLineCommentsUtil.getCommentPsId(first); + appendHeaderField(writer, side == 0 + ? BASE_PATCH_SET + : PATCH_SET, + Integer.toString(psId.get())); + appendHeaderField(writer, REVISION, first.getRevId().get()); + + String currentFilename = null; + + for (PatchLineComment c : comments) { + PatchSet.Id currentPsId = PatchLineCommentsUtil.getCommentPsId(c); + checkArgument(psId.equals(currentPsId), + "All comments being added must all have the same PatchSet.Id. The" + + "comment below does not have the same PatchSet.Id as the others " + + "(%s).\n%s", psId.toString(), c.toString()); + checkArgument(side == c.getSide(), + "All comments being added must all have the same side. The" + + "comment below does not have the same side as the others " + + "(%s).\n%s", side, c.toString()); + String commentFilename = + QuotedString.GIT_PATH.quote(c.getKey().getParentKey().getFileName()); + + if (!commentFilename.equals(currentFilename)) { + currentFilename = commentFilename; + writer.print("File: "); + writer.print(commentFilename); + writer.print("\n\n"); + } + + // The CommentRange field for a comment is allowed to be null. + // If it is indeed null, then in the first line, we simply use the line + // number field for a comment instead. If it isn't null, we write the + // comment range itself. + CommentRange range = c.getRange(); + if (range != null) { + writer.print(range.getStartLine()); + writer.print(':'); + writer.print(range.getStartCharacter()); + writer.print('-'); + writer.print(range.getEndLine()); + writer.print(':'); + writer.print(range.getEndCharacter()); + } else { + writer.print(c.getLine()); + } + writer.print("\n"); + + writer.print(formatTime(serverIdent, c.getWrittenOn())); + writer.print("\n"); + + PersonIdent ident = newIdent( + accountCache.get(c.getAuthor()).getAccount(), + c.getWrittenOn(), serverIdent, anonymousCowardName); + String nameString = ident.getName() + " <" + ident.getEmailAddress() + + ">"; + appendHeaderField(writer, AUTHOR, nameString); + + String parent = c.getParentUuid(); + if (parent != null) { + appendHeaderField(writer, PARENT, parent); + } + + appendHeaderField(writer, UUID, c.getKey().get()); + + byte[] messageBytes = c.getMessage().getBytes(UTF_8); + appendHeaderField(writer, LENGTH, + Integer.toString(messageBytes.length)); + + writer.print(c.getMessage()); + writer.print("\n\n"); + } + } + } }
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 a8ea014..2938d6b 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
@@ -119,8 +119,7 @@ private final AllUsersName allUsers; private final Provider<InternalChangeQuery> queryProvider; private final ProjectCache projectCache; - private final ChangeNoteUtil changeNoteUtil; - private final CommentsInNotesUtil commentsUtil; + private final ChangeNoteUtil noteUtil; @VisibleForTesting @Inject @@ -129,15 +128,13 @@ AllUsersName allUsers, Provider<InternalChangeQuery> queryProvider, ProjectCache projectCache, - ChangeNoteUtil changeNoteUtil, - CommentsInNotesUtil commentsUtil) { + ChangeNoteUtil noteUtil) { this.repoManager = repoManager; this.migration = migration; this.allUsers = allUsers; this.queryProvider = queryProvider; this.projectCache = projectCache; - this.changeNoteUtil = changeNoteUtil; - this.commentsUtil = commentsUtil; + this.noteUtil = noteUtil; } public ChangeNotes createChecked(ReviewDb db, Change c) @@ -182,8 +179,8 @@ project, changeId, change.getProject()); // TODO: Throw NoSuchChangeException when the change is not found in the // database - return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, project, change).load(); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + project, change).load(); } /** @@ -195,13 +192,13 @@ * @return change notes */ public ChangeNotes createFromIndexedChange(Change change) { - return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, change.getProject(), change); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + change.getProject(), change); } public ChangeNotes createForNew(Change change) throws OrmException { - return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + change.getProject(), change).load(); } // TODO(dborowitz): Remove when deleting index schemas <27. @@ -210,8 +207,8 @@ checkState(!migration.readChanges(), "do not call" + " createFromIdOnlyWhenNotedbDisabled when notedb is enabled"); Change change = unwrap(db).changes().get(changeId); - return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + change.getProject(), change).load(); } // TODO(ekempin): Remove when database backend is deleted @@ -223,8 +220,8 @@ throws OrmException { checkState(!migration.readChanges(), "do not call" + " createFromChangeWhenNotedbDisabled when notedb is enabled"); - return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + change.getProject(), change).load(); } public CheckedFuture<ChangeNotes, OrmException> createAsync( @@ -244,7 +241,7 @@ + " but actual project is %s", project, changeId, change.getProject()); return new ChangeNotes(repoManager, migration, allUsers, - changeNoteUtil, commentsUtil, project, change).load(); + noteUtil, project, change).load(); } }); } @@ -383,8 +380,7 @@ } } - private final ChangeNoteUtil changeNoteUtil; - private final CommentsInNotesUtil commentsUtil; + private final ChangeNoteUtil noteUtil; private final Project.NameKey project; private final Change change; private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets; @@ -406,13 +402,11 @@ @VisibleForTesting public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName allUsers, ChangeNoteUtil changeNoteUtil, - CommentsInNotesUtil commentsUtil, Project.NameKey project, - Change change) { + AllUsersName allUsers, ChangeNoteUtil noteUtil, + Project.NameKey project, Change change) { super(repoManager, migration, change != null ? change.getId() : null); this.allUsers = allUsers; - this.changeNoteUtil = changeNoteUtil; - this.commentsUtil = commentsUtil; + this.noteUtil = noteUtil; this.project = project; this.change = change != null ? new Change(change) : null; } @@ -510,7 +504,7 @@ if (draftCommentNotes == null || !author.equals(draftCommentNotes.getAuthor())) { draftCommentNotes = new DraftCommentNotes(repoManager, migration, - allUsers, commentsUtil, getChangeId(), author); + allUsers, noteUtil, getChangeId(), author); draftCommentNotes.load(); } } @@ -556,9 +550,8 @@ return; } try (RevWalk walk = new RevWalk(reader); - ChangeNotesParser parser = new ChangeNotesParser(project, - change.getId(), rev, walk, repoManager, changeNoteUtil, - commentsUtil)) { + ChangeNotesParser parser = new ChangeNotesParser( + project, change.getId(), rev, walk, repoManager, noteUtil)) { parser.parseAll(); if (parser.status != null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index cc0eb7a3..a57dff0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -114,8 +114,7 @@ PatchSet.Id currentPatchSetId; RevisionNoteMap revisionNoteMap; - private final ChangeNoteUtil changeNoteUtil; - private final CommentsInNotesUtil commentsUtil; + private final ChangeNoteUtil noteUtil; private final Change.Id id; private final ObjectId tip; private final RevWalk walk; @@ -127,14 +126,13 @@ ChangeNotesParser(Project.NameKey project, Change.Id changeId, ObjectId tip, RevWalk walk, GitRepositoryManager repoManager, - ChangeNoteUtil changeNoteUtil, CommentsInNotesUtil commentsUtil) + ChangeNoteUtil noteUtil) throws RepositoryNotFoundException, IOException { this.id = changeId; this.tip = tip; this.walk = walk; this.repo = repoManager.openMetadataRepository(project); - this.changeNoteUtil = changeNoteUtil; - this.commentsUtil = commentsUtil; + this.noteUtil = noteUtil; approvals = Maps.newHashMap(); reviewers = Maps.newLinkedHashMap(); allPastReviewers = Lists.newArrayList(); @@ -505,7 +503,7 @@ ObjectReader reader = walk.getObjectReader(); RevCommit tipCommit = walk.parseCommit(tip); revisionNoteMap = RevisionNoteMap.parse( - commentsUtil, id, reader, NoteMap.read(reader, tipCommit), false); + noteUtil, id, reader, NoteMap.read(reader, tipCommit), false); Map<RevId, RevisionNote> rns = revisionNoteMap.revisionNotes; for (Map.Entry<RevId, RevisionNote> e : rns.entrySet()) { @@ -544,7 +542,7 @@ labelVoteStr = line.substring(0, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = changeNoteUtil.parseIdent(ident, id); + accountId = noteUtil.parseIdent(ident, id); } else { labelVoteStr = line; accountId = committerId; @@ -582,7 +580,7 @@ label = line.substring(1, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = changeNoteUtil.parseIdent(ident, id); + accountId = noteUtil.parseIdent(ident, id); } else { label = line.substring(1); accountId = committerId; @@ -665,7 +663,7 @@ PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2)); checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line); - label.appliedBy = changeNoteUtil.parseIdent(ident, id); + label.appliedBy = noteUtil.parseIdent(ident, id); } else { label.label = line.substring(c + 2); } @@ -683,7 +681,7 @@ && a.getEmailAddress().equals(c.getEmailAddress())) { return null; } - return changeNoteUtil.parseIdent(commit.getAuthorIdent(), id); + return noteUtil.parseIdent(commit.getAuthorIdent(), id); } private void parseReviewer(ReviewerStateInternal state, String line) @@ -692,7 +690,7 @@ if (ident == null) { throw invalidFooter(state.getFooterKey(), line); } - Account.Id accountId = changeNoteUtil.parseIdent(ident, id); + Account.Id accountId = noteUtil.parseIdent(ident, id); if (!reviewers.containsKey(accountId)) { reviewers.put(accountId, state); }
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 99ca9f3..453f0bd 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
@@ -100,7 +100,6 @@ } private final AccountCache accountCache; - private final CommentsInNotesUtil commentsUtil; private final ChangeDraftUpdate.Factory draftUpdateFactory; private final NoteDbUpdateManager.Factory updateManagerFactory; @@ -136,11 +135,10 @@ ChangeDraftUpdate.Factory draftUpdateFactory, ProjectCache projectCache, @Assisted ChangeControl ctl, - CommentsInNotesUtil commentsUtil, - ChangeNoteUtil changeNoteUtil) { + ChangeNoteUtil noteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, - projectCache, ctl, serverIdent.getWhen(), commentsUtil, changeNoteUtil); + projectCache, ctl, serverIdent.getWhen(), noteUtil); } @AssistedInject @@ -155,13 +153,12 @@ ProjectCache projectCache, @Assisted ChangeControl ctl, @Assisted Date when, - CommentsInNotesUtil commentsUtil, - ChangeNoteUtil changeNoteUtil) { + ChangeNoteUtil noteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, ctl, when, projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(), - commentsUtil, changeNoteUtil); + noteUtil); } private static Project.NameKey getProjectName(ChangeControl ctl) { @@ -180,12 +177,10 @@ @Assisted ChangeControl ctl, @Assisted Date when, @Assisted Comparator<String> labelNameComparator, - CommentsInNotesUtil commentsUtil, - ChangeNoteUtil changeNoteUtil) { + ChangeNoteUtil noteUtil) { super(migration, repoManager, ctl, serverIdent, - anonymousCowardName, changeNoteUtil, when); + anonymousCowardName, noteUtil, when); this.accountCache = accountCache; - this.commentsUtil = commentsUtil; this.draftUpdateFactory = draftUpdateFactory; this.updateManagerFactory = updateManagerFactory; @@ -378,7 +373,7 @@ for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) { ObjectId data = inserter.insert( - OBJ_BLOB, e.getValue().build(commentsUtil)); + OBJ_BLOB, e.getValue().build(noteUtil)); rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data); } @@ -404,7 +399,7 @@ // Even though reading from changes might not be enabled, we need to // parse any existing revision notes so we can merge them. return RevisionNoteMap.parse( - commentsUtil, ctl.getId(), rw.getObjectReader(), noteMap, false); + noteUtil, ctl.getId(), rw.getObjectReader(), noteMap, false); } private void checkComments(Map<RevId, RevisionNote> existingNotes,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java deleted file mode 100644 index 4eb028d..0000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java +++ /dev/null
@@ -1,451 +0,0 @@ -// Copyright (C) 2014 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.notedb; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.gerrit.server.notedb.ChangeNotes.parseException; -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; -import com.google.gerrit.reviewdb.client.Account; -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; -import com.google.gerrit.server.PatchLineCommentsUtil; -import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.config.AnonymousCowardName; -import com.google.inject.Inject; -import com.google.inject.Singleton; - -import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lib.PersonIdent; -import org.eclipse.jgit.util.GitDateFormatter; -import org.eclipse.jgit.util.GitDateFormatter.Format; -import org.eclipse.jgit.util.GitDateParser; -import org.eclipse.jgit.util.MutableInteger; -import org.eclipse.jgit.util.QuotedString; -import org.eclipse.jgit.util.RawParseUtils; - -import java.io.ByteArrayOutputStream; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.sql.Timestamp; -import java.text.ParseException; -import java.util.List; -import java.util.Locale; - -/** - * Utility functions to parse PatchLineComments out of a note byte array and - * store a list of PatchLineComments in the form of a note (in a byte array). - **/ -@Singleton -public class CommentsInNotesUtil { - private static final String AUTHOR = "Author"; - private static final String BASE_PATCH_SET = "Base-for-patch-set"; - private static final String COMMENT_RANGE = "Comment-range"; - private static final String FILE = "File"; - private static final String LENGTH = "Bytes"; - private static final String PARENT = "Parent"; - private static final String PATCH_SET = "Patch-set"; - private static final String REVISION = "Revision"; - private static final String UUID = "UUID"; - - public static String formatTime(PersonIdent ident, Timestamp t) { - GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT); - // TODO(dborowitz): Use a ThreadLocal or use Joda. - PersonIdent newIdent = new PersonIdent(ident, t); - return dateFormatter.formatDate(newIdent); - } - - private final AccountCache accountCache; - private final PersonIdent serverIdent; - private final String anonymousCowardName; - private final ChangeNoteUtil changeNoteUtil; - - @Inject - public CommentsInNotesUtil(AccountCache accountCache, - @GerritPersonIdent PersonIdent serverIdent, - @AnonymousCowardName String anonymousCowardName, - ChangeNoteUtil changeNoteUtil) { - this.accountCache = accountCache; - this.serverIdent = serverIdent; - this.anonymousCowardName = anonymousCowardName; - this.changeNoteUtil = changeNoteUtil; - } - - public List<PatchLineComment> parseNote(byte[] note, MutableInteger p, - Change.Id changeId, Status status) throws ConfigInvalidException { - if (p.value >= note.length) { - return ImmutableList.of(); - } - List<PatchLineComment> result = Lists.newArrayList(); - int sizeOfNote = note.length; - - boolean isForBase = - (RawParseUtils.match(note, p.value, PATCH_SET.getBytes(UTF_8))) < 0; - - PatchSet.Id psId = parsePsId(note, p, changeId, isForBase ? BASE_PATCH_SET : PATCH_SET); - - RevId revId = - new RevId(parseStringField(note, p, changeId, REVISION)); - - PatchLineComment c = null; - while (p.value < sizeOfNote) { - String previousFileName = c == null ? - null : c.getKey().getParentKey().getFileName(); - c = parseComment(note, p, previousFileName, psId, revId, - isForBase, status); - result.add(c); - } - return result; - } - - private PatchLineComment parseComment(byte[] note, MutableInteger curr, - String currentFileName, PatchSet.Id psId, RevId revId, boolean isForBase, - Status status) throws ConfigInvalidException { - Change.Id changeId = psId.getParentKey(); - - // Check if there is a new file. - boolean newFile = - (RawParseUtils.match(note, curr.value, FILE.getBytes(UTF_8))) != -1; - if (newFile) { - // If so, parse the new file name. - currentFileName = parseFilename(note, curr, changeId); - } else if (currentFileName == null) { - throw parseException(changeId, "could not parse %s", FILE); - } - - CommentRange range = parseCommentRange(note, curr); - if (range == null) { - throw parseException(changeId, "could not parse %s", COMMENT_RANGE); - } - - Timestamp commentTime = parseTimestamp(note, curr, changeId); - Account.Id aId = parseAuthor(note, curr, changeId); - - boolean hasParent = - (RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1; - String parentUUID = null; - if (hasParent) { - parentUUID = parseStringField(note, curr, changeId, PARENT); - } - - String uuid = parseStringField(note, curr, changeId, UUID); - int commentLength = parseCommentLength(note, curr, changeId); - - String message = RawParseUtils.decode( - UTF_8, note, curr.value, curr.value + commentLength); - checkResult(message, "message contents", changeId); - - PatchLineComment plc = new PatchLineComment( - new PatchLineComment.Key(new Patch.Key(psId, currentFileName), uuid), - range.getEndLine(), aId, parentUUID, commentTime); - plc.setMessage(message); - plc.setSide((short) (isForBase ? 0 : 1)); - if (range.getStartCharacter() != -1) { - plc.setRange(range); - } - plc.setRevId(revId); - plc.setStatus(status); - - curr.value = RawParseUtils.nextLF(note, curr.value + commentLength); - curr.value = RawParseUtils.nextLF(note, curr.value); - return plc; - } - - private static String parseStringField(byte[] note, MutableInteger curr, - Change.Id changeId, String fieldName) throws ConfigInvalidException { - int endOfLine = RawParseUtils.nextLF(note, curr.value); - checkHeaderLineFormat(note, curr, fieldName, changeId); - int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; - curr.value = endOfLine; - return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1); - } - - /** - * @return a comment range. If the comment range line in the note only has - * one number, we return a CommentRange with that one number as the end - * line and the other fields as -1. If the comment range line in the note - * contains a whole comment range, then we return a CommentRange with all - * fields set. If the line is not correctly formatted, return null. - */ - private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) { - CommentRange range = new CommentRange(-1, -1, -1, -1); - - int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr); - if (startLine == 0) { - range.setEndLine(0); - ptr.value += 1; - return range; - } - - if (note[ptr.value] == '\n') { - range.setEndLine(startLine); - ptr.value += 1; - return range; - } else if (note[ptr.value] == ':') { - range.setStartLine(startLine); - ptr.value += 1; - } else { - return null; - } - - int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr); - if (note[ptr.value] == '-') { - range.setStartCharacter(startChar); - ptr.value += 1; - } else { - return null; - } - - int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr); - if (endLine == 0) { - return null; - } - if (note[ptr.value] == ':') { - range.setEndLine(endLine); - ptr.value += 1; - } else { - return null; - } - - int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr); - if (endChar == 0) { - return null; - } - if (note[ptr.value] == '\n') { - range.setEndCharacter(endChar); - ptr.value += 1; - } else { - return null; - } - return range; - } - - private static PatchSet.Id parsePsId(byte[] note, MutableInteger curr, - Change.Id changeId, String fieldName) throws ConfigInvalidException { - checkHeaderLineFormat(note, curr, fieldName, changeId); - int startOfPsId = - RawParseUtils.endOfFooterLineKey(note, curr.value) + 1; - MutableInteger i = new MutableInteger(); - int patchSetId = - RawParseUtils.parseBase10(note, startOfPsId, i); - int endOfLine = RawParseUtils.nextLF(note, curr.value); - if (i.value != endOfLine - 1) { - throw parseException(changeId, "could not parse %s", fieldName); - } - checkResult(patchSetId, "patchset id", changeId); - curr.value = endOfLine; - return new PatchSet.Id(changeId, patchSetId); - } - - private static String parseFilename(byte[] note, MutableInteger curr, - Change.Id changeId) throws ConfigInvalidException { - checkHeaderLineFormat(note, curr, FILE, changeId); - int startOfFileName = - RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; - int endOfLine = RawParseUtils.nextLF(note, curr.value); - curr.value = endOfLine; - curr.value = RawParseUtils.nextLF(note, curr.value); - return QuotedString.GIT_PATH.dequote( - RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1)); - } - - private static Timestamp parseTimestamp(byte[] note, MutableInteger curr, - Change.Id changeId) throws ConfigInvalidException { - int endOfLine = RawParseUtils.nextLF(note, curr.value); - Timestamp commentTime; - String dateString = - RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1); - try { - commentTime = new Timestamp( - GitDateParser.parse(dateString, null, Locale.US).getTime()); - } catch (ParseException e) { - throw new ConfigInvalidException("could not parse comment timestamp", e); - } - curr.value = endOfLine; - return checkResult(commentTime, "comment timestamp", changeId); - } - - private Account.Id parseAuthor(byte[] note, MutableInteger curr, - Change.Id changeId) throws ConfigInvalidException { - checkHeaderLineFormat(note, curr, AUTHOR, changeId); - int startOfAccountId = - RawParseUtils.endOfFooterLineKey(note, curr.value) + 2; - PersonIdent ident = - RawParseUtils.parsePersonIdent(note, startOfAccountId); - Account.Id aId = changeNoteUtil.parseIdent(ident, changeId); - curr.value = RawParseUtils.nextLF(note, curr.value); - return checkResult(aId, "comment author", changeId); - } - - private static int parseCommentLength(byte[] note, MutableInteger curr, - Change.Id changeId) throws ConfigInvalidException { - checkHeaderLineFormat(note, curr, LENGTH, changeId); - int startOfLength = - RawParseUtils.endOfFooterLineKey(note, curr.value) + 1; - MutableInteger i = new MutableInteger(); - int commentLength = - RawParseUtils.parseBase10(note, startOfLength, i); - int endOfLine = RawParseUtils.nextLF(note, curr.value); - if (i.value != endOfLine-1) { - throw parseException(changeId, "could not parse %s", PATCH_SET); - } - curr.value = endOfLine; - return checkResult(commentLength, "comment length", changeId); - } - - private static <T> T checkResult(T o, String fieldName, - Change.Id changeId) throws ConfigInvalidException { - if (o == null) { - throw parseException(changeId, "could not parse %s", fieldName); - } - return o; - } - - private static int checkResult(int i, String fieldName, Change.Id changeId) - throws ConfigInvalidException { - if (i <= 0) { - throw parseException(changeId, "could not parse %s", fieldName); - } - return i; - } - - private void appendHeaderField(PrintWriter writer, - String field, String value) { - writer.print(field); - writer.print(": "); - writer.print(value); - writer.print('\n'); - } - - private static void checkHeaderLineFormat(byte[] note, MutableInteger curr, - String fieldName, Change.Id changeId) throws ConfigInvalidException { - boolean correct = - RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1; - int p = curr.value + fieldName.length(); - correct &= (p < note.length && note[p] == ':'); - p++; - correct &= (p < note.length && note[p] == ' '); - if (!correct) { - throw parseException(changeId, "could not parse %s", fieldName); - } - } - - public byte[] buildNote(List<PatchLineComment> comments) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - buildNote(comments, out); - return out.toByteArray(); - } - - /** - * Build a note that contains the metadata for and the contents of all of the - * comments in the given list of comments. - * - * @param comments A list of the comments to be written to the - * output stream. All of the comments in this list must have the - * same side and must share the same patch set ID. - * @param out output stream to write to. - */ - void buildNote(List<PatchLineComment> comments, OutputStream out) { - if (comments.isEmpty()) { - return; - } - OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8); - try (PrintWriter writer = new PrintWriter(streamWriter)) { - PatchLineComment first = comments.get(0); - - short side = first.getSide(); - PatchSet.Id psId = PatchLineCommentsUtil.getCommentPsId(first); - appendHeaderField(writer, side == 0 - ? BASE_PATCH_SET - : PATCH_SET, - Integer.toString(psId.get())); - appendHeaderField(writer, REVISION, first.getRevId().get()); - - String currentFilename = null; - - for (PatchLineComment c : comments) { - PatchSet.Id currentPsId = PatchLineCommentsUtil.getCommentPsId(c); - checkArgument(psId.equals(currentPsId), - "All comments being added must all have the same PatchSet.Id. The" - + "comment below does not have the same PatchSet.Id as the others " - + "(%s).\n%s", psId.toString(), c.toString()); - checkArgument(side == c.getSide(), - "All comments being added must all have the same side. The" - + "comment below does not have the same side as the others " - + "(%s).\n%s", side, c.toString()); - String commentFilename = - QuotedString.GIT_PATH.quote(c.getKey().getParentKey().getFileName()); - - if (!commentFilename.equals(currentFilename)) { - currentFilename = commentFilename; - writer.print("File: "); - writer.print(commentFilename); - writer.print("\n\n"); - } - - // The CommentRange field for a comment is allowed to be null. - // If it is indeed null, then in the first line, we simply use the line - // number field for a comment instead. If it isn't null, we write the - // comment range itself. - CommentRange range = c.getRange(); - if (range != null) { - writer.print(range.getStartLine()); - writer.print(':'); - writer.print(range.getStartCharacter()); - writer.print('-'); - writer.print(range.getEndLine()); - writer.print(':'); - writer.print(range.getEndCharacter()); - } else { - writer.print(c.getLine()); - } - writer.print("\n"); - - writer.print(formatTime(serverIdent, c.getWrittenOn())); - writer.print("\n"); - - PersonIdent ident = - changeNoteUtil.newIdent(accountCache.get(c.getAuthor()).getAccount(), - c.getWrittenOn(), serverIdent, anonymousCowardName); - String nameString = ident.getName() + " <" + ident.getEmailAddress() - + ">"; - appendHeaderField(writer, AUTHOR, nameString); - - String parent = c.getParentUuid(); - if (parent != null) { - appendHeaderField(writer, PARENT, parent); - } - - appendHeaderField(writer, UUID, c.getKey().get()); - - byte[] messageBytes = c.getMessage().getBytes(UTF_8); - appendHeaderField(writer, LENGTH, - Integer.toString(messageBytes.length)); - - writer.print(c.getMessage()); - writer.print("\n\n"); - } - } - } -}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java index f03caab..b0cb3e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
@@ -48,40 +48,40 @@ private final GitRepositoryManager repoManager; private final NotesMigration migration; private final AllUsersName draftsProject; - private final CommentsInNotesUtil commentsUtil; + private final ChangeNoteUtil noteUtil; @VisibleForTesting @Inject public Factory(GitRepositoryManager repoManager, NotesMigration migration, AllUsersName allUsers, - CommentsInNotesUtil commentsUtil) { + ChangeNoteUtil noteUtil) { this.repoManager = repoManager; this.migration = migration; this.draftsProject = allUsers; - this.commentsUtil = commentsUtil; + this.noteUtil = noteUtil; } public DraftCommentNotes create(Change.Id changeId, Account.Id accountId) { return new DraftCommentNotes(repoManager, migration, draftsProject, - commentsUtil, changeId, accountId); + noteUtil, changeId, accountId); } } private final AllUsersName draftsProject; - private final CommentsInNotesUtil commentsUtil; + private final ChangeNoteUtil noteUtil; private final Account.Id author; private ImmutableListMultimap<RevId, PatchLineComment> comments; private RevisionNoteMap revisionNoteMap; DraftCommentNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName draftsProject, CommentsInNotesUtil commentsUtil, - Change.Id changeId, Account.Id author) { + AllUsersName draftsProject, ChangeNoteUtil noteUtil, Change.Id changeId, + Account.Id author) { super(repoManager, migration, changeId); this.draftsProject = draftsProject; this.author = author; - this.commentsUtil = commentsUtil; + this.noteUtil = noteUtil; } RevisionNoteMap getRevisionNoteMap() { @@ -122,7 +122,7 @@ try (RevWalk walk = new RevWalk(reader)) { RevCommit tipCommit = walk.parseCommit(rev); revisionNoteMap = RevisionNoteMap.parse( - commentsUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit), + noteUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit), true); Multimap<RevId, PatchLineComment> cs = ArrayListMultimap.create(); for (RevisionNote rn : revisionNoteMap.revisionNotes.values()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java index 586beaf..e6d7107 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java
@@ -63,7 +63,7 @@ final ImmutableList<PatchLineComment> comments; final String pushCert; - RevisionNote(CommentsInNotesUtil commentsUtil, Change.Id changeId, + RevisionNote(ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, ObjectId noteId, boolean draftsOnly) throws ConfigInvalidException, IOException { byte[] bytes = reader.open(noteId, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ); @@ -79,6 +79,6 @@ ? PatchLineComment.Status.DRAFT : PatchLineComment.Status.PUBLISHED; comments = ImmutableList.copyOf( - commentsUtil.parseNote(bytes, p, changeId, status)); + noteUtil.parseNote(bytes, p, changeId, status)); } }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java index 3d79795..5179146 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
@@ -91,7 +91,7 @@ this.pushCert = pushCert; } - byte[] build(CommentsInNotesUtil commentsUtil) { + byte[] build(ChangeNoteUtil noteUtil) { ByteArrayOutputStream out = new ByteArrayOutputStream(); if (pushCert != null) { byte[] certBytes = pushCert.getBytes(UTF_8); @@ -107,7 +107,7 @@ } } Collections.sort(all, PLC_ORDER); - commentsUtil.buildNote(all, out); + noteUtil.buildNote(all, out); return out.toByteArray(); }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteMap.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteMap.java index 0c2420a..cd70528 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteMap.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
@@ -31,13 +31,13 @@ final NoteMap noteMap; final ImmutableMap<RevId, RevisionNote> revisionNotes; - static RevisionNoteMap parse(CommentsInNotesUtil commentsUtil, + static RevisionNoteMap parse(ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap, boolean draftsOnly) throws ConfigInvalidException, IOException { Map<RevId, RevisionNote> result = new HashMap<>(); for (Note note : noteMap) { RevisionNote rn = new RevisionNote( - commentsUtil, changeId, reader, note.getData(), draftsOnly); + noteUtil, changeId, reader, note.getData(), draftsOnly); result.put(new RevId(note.name()), rn); } return new RevisionNoteMap(noteMap, ImmutableMap.copyOf(result));
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java index d01ae0b..b387405 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -108,10 +108,7 @@ protected AllUsersName allUsers; @Inject - protected ChangeNoteUtil changeNoteUtil; - - @Inject - protected CommentsInNotesUtil commentsUtil; + protected ChangeNoteUtil noteUtil; private Injector injector; private String systemTimeZone; @@ -207,8 +204,8 @@ } protected ChangeNotes newNotes(Change c) throws OrmException { - return new ChangeNotes(repoManager, MIGRATION, allUsers, changeNoteUtil, - commentsUtil, c.getProject(), c).load(); + return new ChangeNotes(repoManager, MIGRATION, allUsers, noteUtil, + c.getProject(), c).load(); } protected static SubmitRecord submitRecord(String status,
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index fe3c2eb..729ea7d 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -415,7 +415,7 @@ } private RevCommit writeCommit(String body) throws Exception { - return writeCommit(body, changeNoteUtil.newIdent( + return writeCommit(body, noteUtil.newIdent( changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent, "Anonymous Coward")); } @@ -462,6 +462,6 @@ private ChangeNotesParser newParser(ObjectId tip) throws Exception { Change c = newChange(); return new ChangeNotesParser(c.getProject(), c.getId(), tip, walk, - repoManager, changeNoteUtil, commentsUtil); + repoManager, noteUtil); } }
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 ba7cd37..9a14c19 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
@@ -825,7 +825,7 @@ + "File: a.txt\n" + "\n" + "1:2-3:4\n" - + CommentsInNotesUtil.formatTime(serverIdent, ts) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, ts) + "\n" + "Author: Change Owner <1@gerrit>\n" + "UUID: uuid1\n" + "Bytes: 7\n" @@ -910,8 +910,7 @@ try (RevWalk rw = new RevWalk(repo)) { try (ChangeNotesParser notesWithComments = new ChangeNotesParser(project, - c.getId(), commitWithComments.copy(), rw, repoManager, changeNoteUtil, - commentsUtil)) { + c.getId(), commitWithComments.copy(), rw, repoManager, noteUtil)) { notesWithComments.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals1 = notesWithComments.buildApprovals(); @@ -923,7 +922,7 @@ try (RevWalk rw = new RevWalk(repo)) { try (ChangeNotesParser notesWithApprovals = new ChangeNotesParser(project, c.getId(), commitWithApprovals.copy(), rw, repoManager, - changeNoteUtil, commentsUtil)) { + noteUtil)) { notesWithApprovals.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals2 = notesWithApprovals.buildApprovals(); @@ -1168,14 +1167,14 @@ + "File: file1\n" + "\n" + "1:1-2:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, time1) + "\n" + "Author: Other Account <2@gerrit>\n" + "UUID: uuid1\n" + "Bytes: 9\n" + "comment 1\n" + "\n" + "2:1-3:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, time2) + "\n" + "Author: Other Account <2@gerrit>\n" + "UUID: uuid2\n" + "Bytes: 9\n" @@ -1184,7 +1183,7 @@ + "File: file2\n" + "\n" + "3:0-4:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, time3) + "\n" + "Author: Other Account <2@gerrit>\n" + "UUID: uuid3\n" + "Bytes: 9\n" @@ -1238,14 +1237,14 @@ + "File: file1\n" + "\n" + "1:1-2:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, time1) + "\n" + "Author: Other Account <2@gerrit>\n" + "UUID: uuid1\n" + "Bytes: 9\n" + "comment 1\n" + "\n" + "2:1-3:1\n" - + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + + ChangeNoteUtil.formatTime(serverIdent, time2) + "\n" + "Author: Other Account <2@gerrit>\n" + "UUID: uuid2\n" + "Bytes: 9\n"
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java index cfa63dc..386b724 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java
@@ -34,7 +34,6 @@ import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; -import com.google.gerrit.server.notedb.CommentsInNotesUtil; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.project.ChangeControl; import com.google.gwtorm.server.OrmException; @@ -105,7 +104,6 @@ stubChangeControl( repoManager, migration, c, allUsers, injector.getInstance(ChangeNoteUtil.class), - injector.getInstance(CommentsInNotesUtil.class), user), TimeUtil.nowTs(), Ordering.<String> natural()); @@ -139,15 +137,15 @@ private static ChangeControl stubChangeControl( GitRepositoryManager repoManager, NotesMigration migration, - Change c, AllUsersName allUsers, ChangeNoteUtil changeNoteUtil, - CommentsInNotesUtil commentsUtil, CurrentUser user) throws OrmException { + Change c, AllUsersName allUsers, ChangeNoteUtil noteUtil, + CurrentUser user) throws OrmException { ChangeControl ctl = EasyMock.createMock(ChangeControl.class); expect(ctl.getChange()).andStubReturn(c); expect(ctl.getProject()).andStubReturn(new Project(c.getProject())); expect(ctl.getUser()).andStubReturn(user); ChangeNotes notes = - new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, - commentsUtil, c.getProject(), c).load(); + new ChangeNotes(repoManager, migration, allUsers, noteUtil, + c.getProject(), c).load(); expect(ctl.getNotes()).andStubReturn(notes); expect(ctl.getId()).andStubReturn(c.getId()); EasyMock.replay(ctl);