Split ChangeNoteUtil into JSON/Read/Write parts
The write part needs the AccountCache, which is problematic in the
schema migration.
Change-Id: I2b8a6a0d946c765588107fb10abd19b96aef274c
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index ef2c9b3..a083a71 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -48,7 +48,8 @@
final GitRepositoryManager repoManager;
final NotesMigration migration;
final AllUsersName allUsers;
- final ChangeNoteUtil noteUtil;
+ final ChangeNoteJson changeNoteJson;
+ final LegacyChangeNoteRead legacyChangeNoteRead;
final NoteDbMetrics metrics;
final Provider<ReviewDb> db;
@@ -65,7 +66,8 @@
GitRepositoryManager repoManager,
NotesMigration migration,
AllUsersName allUsers,
- ChangeNoteUtil noteUtil,
+ ChangeNoteJson changeNoteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics,
Provider<ReviewDb> db,
Provider<ChangeRebuilder> rebuilder,
@@ -73,7 +75,8 @@
this.repoManager = repoManager;
this.migration = migration;
this.allUsers = allUsers;
- this.noteUtil = noteUtil;
+ this.legacyChangeNoteRead = legacyChangeNoteRead;
+ this.changeNoteJson = changeNoteJson;
this.metrics = metrics;
this.db = db;
this.rebuilder = rebuilder;
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 010c5c0..3653bc7 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -120,7 +120,9 @@
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
checkUserType(u);
if (u instanceof IdentifiedUser) {
- return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
+ return noteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
} else if (u instanceof InternalUser) {
return serverIdent;
}
@@ -175,7 +177,7 @@
}
protected PersonIdent newIdent(Account.Id authorId, Date when) {
- return noteUtil.newIdent(authorId, when, serverIdent);
+ return noteUtil.getLegacyChangeNoteWrite().newIdent(authorId, when, serverIdent);
}
/** Whether no updates have been done. */
diff --git a/java/com/google/gerrit/server/notedb/ChangeBundle.java b/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 7714c6e..1d3c752 100644
--- a/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -505,11 +505,11 @@
if (rn >= 0) {
s = s.substring(0, rn);
}
- return ChangeNoteUtil.sanitizeFooter(s);
+ return NoteDbUtil.sanitizeFooter(s);
}
private static String cleanNoteDbSubject(String s) {
- return ChangeNoteUtil.sanitizeFooter(s);
+ return NoteDbUtil.sanitizeFooter(s);
}
/**
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 71c0b9e..6b4bea7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -178,7 +178,12 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get());
- byte[] data = e.getValue().build(noteUtil, noteUtil.getWriteJson());
+ byte[] data =
+ e.getValue()
+ .build(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteWrite(),
+ noteUtil.getChangeNoteJson().getWriteJson());
if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true;
}
@@ -236,7 +241,12 @@
// 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(
- noteUtil, getId(), rw.getObjectReader(), noteMap, PatchLineComment.Status.DRAFT);
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ getId(),
+ rw.getObjectReader(),
+ noteMap,
+ PatchLineComment.Status.DRAFT);
}
@Override
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
new file mode 100644
index 0000000..0475fe3
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2018 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 com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.sql.Timestamp;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+public class ChangeNoteJson {
+ private final Gson gson = newGson();
+ private final boolean writeJson;
+
+ static Gson newGson() {
+ return new GsonBuilder()
+ .registerTypeAdapter(Timestamp.class, new CommentTimestampAdapter().nullSafe())
+ .setPrettyPrinting()
+ .create();
+ }
+
+ public Gson getGson() {
+ return gson;
+ }
+
+ public boolean getWriteJson() {
+ return writeJson;
+ }
+
+ @Inject
+ ChangeNoteJson(@GerritServerConfig Config config) {
+ this.writeJson = config.getBoolean("notedb", "writeJson", true);
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index e8e4dec..070f974 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -14,52 +14,8 @@
package com.google.gerrit.server.notedb;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER;
-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.base.CharMatcher;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ListMultimap;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Comment;
-import com.google.gerrit.reviewdb.client.CommentRange;
-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.account.AccountCache;
-import com.google.gerrit.server.account.AccountState;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.config.GerritServerId;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
import com.google.inject.Inject;
-import java.io.OutputStream;
-import java.io.OutputStreamWriter;
-import java.io.PrintWriter;
-import java.sql.Timestamp;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Optional;
-import java.util.Set;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
-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;
public class ChangeNoteUtil {
public static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
@@ -85,560 +41,43 @@
public static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
public static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
- 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 PARENT_NUMBER = "Parent-number";
- private static final String PATCH_SET = "Patch-set";
- private static final String REAL_AUTHOR = "Real-author";
- private static final String REVISION = "Revision";
- private static final String UUID = "UUID";
- private static final String UNRESOLVED = "Unresolved";
- private static final String TAG = FOOTER_TAG.getName();
+ static final String AUTHOR = "Author";
+ static final String BASE_PATCH_SET = "Base-for-patch-set";
+ static final String COMMENT_RANGE = "Comment-range";
+ static final String FILE = "File";
+ static final String LENGTH = "Bytes";
+ static final String PARENT = "Parent";
+ static final String PARENT_NUMBER = "Parent-number";
+ static final String PATCH_SET = "Patch-set";
+ static final String REAL_AUTHOR = "Real-author";
+ static final String REVISION = "Revision";
+ static final String UUID = "UUID";
+ static final String UNRESOLVED = "Unresolved";
+ static final String TAG = FOOTER_TAG.getName();
- 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);
- }
-
- static Gson newGson() {
- return new GsonBuilder()
- .registerTypeAdapter(Timestamp.class, new CommentTimestampAdapter().nullSafe())
- .setPrettyPrinting()
- .create();
- }
-
- private final AccountCache accountCache;
- private final PersonIdent serverIdent;
- private final String serverId;
- private final Gson gson = newGson();
- private final boolean writeJson;
+ private final LegacyChangeNoteRead legacyChangeNoteRead;
+ private final LegacyChangeNoteWrite legacyChangeNoteWrite;
+ private final ChangeNoteJson changeNoteJson;
@Inject
public ChangeNoteUtil(
- AccountCache accountCache,
- @GerritPersonIdent PersonIdent serverIdent,
- @GerritServerId String serverId,
- @GerritServerConfig Config config) {
- this.accountCache = accountCache;
- this.serverIdent = serverIdent;
- this.serverId = serverId;
- this.writeJson = config.getBoolean("notedb", "writeJson", true);
+ ChangeNoteJson changeNoteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
+ LegacyChangeNoteWrite legacyChangeNoteWrite) {
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = legacyChangeNoteRead;
+ this.legacyChangeNoteWrite = legacyChangeNoteWrite;
}
- public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
- Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
- return new PersonIdent(
- author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
- authorId.get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ public LegacyChangeNoteRead getLegacyChangeNoteRead() {
+ return legacyChangeNoteRead;
}
- @VisibleForTesting
- public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
- return new PersonIdent(
- author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
+ public ChangeNoteJson getChangeNoteJson() {
+ return changeNoteJson;
}
- public boolean getWriteJson() {
- return writeJson;
- }
-
- public Gson getGson() {
- return gson;
- }
-
- public String getServerId() {
- return serverId;
- }
-
- public Account.Id parseIdent(PersonIdent ident, Change.Id changeId)
- throws ConfigInvalidException {
- return NoteDbUtil.parseIdent(ident, serverId)
- .orElseThrow(
- () ->
- parseException(
- changeId,
- "invalid identity, expected <id>@%s: %s",
- serverId,
- ident.getEmailAddress()));
- }
-
- private static boolean match(byte[] note, MutableInteger p, byte[] expected) {
- int m = RawParseUtils.match(note, p.value, expected);
- return m == p.value + expected.length;
- }
-
- public List<Comment> parseNote(byte[] note, MutableInteger p, Change.Id changeId)
- throws ConfigInvalidException {
- if (p.value >= note.length) {
- return ImmutableList.of();
- }
- Set<Comment.Key> seen = new HashSet<>();
- List<Comment> result = new ArrayList<>();
- int sizeOfNote = note.length;
- byte[] psb = PATCH_SET.getBytes(UTF_8);
- byte[] bpsb = BASE_PATCH_SET.getBytes(UTF_8);
- byte[] bpn = PARENT_NUMBER.getBytes(UTF_8);
-
- RevId revId = new RevId(parseStringField(note, p, changeId, REVISION));
- String fileName = null;
- PatchSet.Id psId = null;
- boolean isForBase = false;
- Integer parentNumber = null;
-
- while (p.value < sizeOfNote) {
- boolean matchPs = match(note, p, psb);
- boolean matchBase = match(note, p, bpsb);
- if (matchPs) {
- fileName = null;
- psId = parsePsId(note, p, changeId, PATCH_SET);
- isForBase = false;
- } else if (matchBase) {
- fileName = null;
- psId = parsePsId(note, p, changeId, BASE_PATCH_SET);
- isForBase = true;
- if (match(note, p, bpn)) {
- parentNumber = parseParentNumber(note, p, changeId);
- }
- } else if (psId == null) {
- throw parseException(changeId, "missing %s or %s header", PATCH_SET, BASE_PATCH_SET);
- }
-
- Comment c = parseComment(note, p, fileName, psId, revId, isForBase, parentNumber);
- fileName = c.key.filename;
- if (!seen.add(c.key)) {
- throw parseException(changeId, "multiple comments for %s in note", c.key);
- }
- result.add(c);
- }
- return result;
- }
-
- private Comment parseComment(
- byte[] note,
- MutableInteger curr,
- String currentFileName,
- PatchSet.Id psId,
- RevId revId,
- boolean isForBase,
- Integer parentNumber)
- 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, AUTHOR);
- boolean hasRealAuthor =
- (RawParseUtils.match(note, curr.value, REAL_AUTHOR.getBytes(UTF_8))) != -1;
- Account.Id raId = null;
- if (hasRealAuthor) {
- raId = parseAuthor(note, curr, changeId, REAL_AUTHOR);
- }
-
- boolean hasParent = (RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1;
- String parentUUID = null;
- boolean unresolved = false;
- if (hasParent) {
- parentUUID = parseStringField(note, curr, changeId, PARENT);
- }
- boolean hasUnresolved =
- (RawParseUtils.match(note, curr.value, UNRESOLVED.getBytes(UTF_8))) != -1;
- if (hasUnresolved) {
- unresolved = parseBooleanField(note, curr, changeId, UNRESOLVED);
- }
-
- String uuid = parseStringField(note, curr, changeId, UUID);
-
- boolean hasTag = (RawParseUtils.match(note, curr.value, TAG.getBytes(UTF_8))) != -1;
- String tag = null;
- if (hasTag) {
- tag = parseStringField(note, curr, changeId, TAG);
- }
-
- int commentLength = parseCommentLength(note, curr, changeId);
-
- String message = RawParseUtils.decode(UTF_8, note, curr.value, curr.value + commentLength);
- checkResult(message, "message contents", changeId);
-
- Comment c =
- new Comment(
- new Comment.Key(uuid, currentFileName, psId.get()),
- aId,
- commentTime,
- isForBase ? (short) (parentNumber == null ? 0 : -parentNumber) : (short) 1,
- message,
- serverId,
- unresolved);
- c.lineNbr = range.getEndLine();
- c.parentUuid = parentUUID;
- c.tag = tag;
- c.setRevId(revId);
- if (raId != null) {
- c.setRealAuthor(raId);
- }
-
- if (range.getStartCharacter() != -1) {
- c.setRange(range);
- }
-
- curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
- curr.value = RawParseUtils.nextLF(note, curr.value);
- return c;
- }
-
- 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 last = ptr.value;
- int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else 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;
- }
-
- last = ptr.value;
- int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == '-') {
- range.setStartCharacter(startChar);
- ptr.value += 1;
- } else {
- return null;
- }
-
- last = ptr.value;
- int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == ':') {
- range.setEndLine(endLine);
- ptr.value += 1;
- } else {
- return null;
- }
-
- last = ptr.value;
- int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else 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 Integer parseParentNumber(byte[] note, MutableInteger curr, Change.Id changeId)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, PARENT_NUMBER, changeId);
-
- int start = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
- MutableInteger i = new MutableInteger();
- int parentNumber = RawParseUtils.parseBase10(note, start, i);
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- if (i.value != endOfLine - 1) {
- throw parseException(changeId, "could not parse %s", PARENT_NUMBER);
- }
- checkResult(parentNumber, "parent number", changeId);
- curr.value = endOfLine;
- return Integer.valueOf(parentNumber);
- }
-
- 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, String fieldName)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, fieldName, 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, fieldName, 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();
- i.value = startOfLength;
- int commentLength = RawParseUtils.parseBase10(note, startOfLength, i);
- if (i.value == startOfLength) {
- throw parseException(changeId, "could not parse %s", LENGTH);
- }
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- if (i.value != endOfLine - 1) {
- throw parseException(changeId, "could not parse %s", LENGTH);
- }
- curr.value = endOfLine;
- return checkResult(commentLength, "comment length", changeId);
- }
-
- private boolean parseBooleanField(
- byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
- throws ConfigInvalidException {
- String str = parseStringField(note, curr, changeId, fieldName);
- if ("true".equalsIgnoreCase(str)) {
- return true;
- } else if ("false".equalsIgnoreCase(str)) {
- return false;
- }
- throw parseException(changeId, "invalid boolean for %s: %s", fieldName, str);
- }
-
- 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);
- }
- }
-
- /**
- * Build a note that contains the metadata for and the contents of all of the comments in the
- * given comments.
- *
- * @param comments Comments to be written to the output stream, keyed by patch set ID; multiple
- * patch sets are allowed since base revisions may be shared across patch sets. All of the
- * comments must share the same RevId, and all the comments for a given patch set must have
- * the same side.
- * @param out output stream to write to.
- */
- void buildNote(ListMultimap<Integer, Comment> comments, OutputStream out) {
- if (comments.isEmpty()) {
- return;
- }
-
- List<Integer> psIds = new ArrayList<>(comments.keySet());
- Collections.sort(psIds);
-
- OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
- try (PrintWriter writer = new PrintWriter(streamWriter)) {
- String revId = comments.values().iterator().next().revId;
- appendHeaderField(writer, REVISION, revId);
-
- for (int psId : psIds) {
- List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId));
- Comment first = psComments.get(0);
-
- short side = first.side;
- appendHeaderField(writer, side <= 0 ? BASE_PATCH_SET : PATCH_SET, Integer.toString(psId));
- if (side < 0) {
- appendHeaderField(writer, PARENT_NUMBER, Integer.toString(-side));
- }
-
- String currentFilename = null;
-
- for (Comment c : psComments) {
- checkArgument(
- revId.equals(c.revId),
- "All comments being added must have all the same RevId. The "
- + "comment below does not have the same RevId as the others "
- + "(%s).\n%s",
- revId,
- c);
- checkArgument(
- side == c.side,
- "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);
- String commentFilename = QuotedString.GIT_PATH.quote(c.key.filename);
-
- if (!commentFilename.equals(currentFilename)) {
- currentFilename = commentFilename;
- writer.print("File: ");
- writer.print(commentFilename);
- writer.print("\n\n");
- }
-
- appendOneComment(writer, c);
- }
- }
- }
- }
-
- private void appendOneComment(PrintWriter writer, Comment c) {
- // The CommentRange field for a comment is allowed to be null. If it is
- // 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.
- Comment.Range range = c.range;
- if (range != null) {
- writer.print(range.startLine);
- writer.print(':');
- writer.print(range.startChar);
- writer.print('-');
- writer.print(range.endLine);
- writer.print(':');
- writer.print(range.endChar);
- } else {
- writer.print(c.lineNbr);
- }
- writer.print("\n");
-
- writer.print(formatTime(serverIdent, c.writtenOn));
- writer.print("\n");
-
- appendIdent(writer, AUTHOR, c.author.getId(), c.writtenOn);
- if (!c.getRealAuthor().equals(c.author)) {
- appendIdent(writer, REAL_AUTHOR, c.getRealAuthor().getId(), c.writtenOn);
- }
-
- String parent = c.parentUuid;
- if (parent != null) {
- appendHeaderField(writer, PARENT, parent);
- }
-
- appendHeaderField(writer, UNRESOLVED, Boolean.toString(c.unresolved));
- appendHeaderField(writer, UUID, c.key.uuid);
-
- if (c.tag != null) {
- appendHeaderField(writer, TAG, c.tag);
- }
-
- byte[] messageBytes = c.message.getBytes(UTF_8);
- appendHeaderField(writer, LENGTH, Integer.toString(messageBytes.length));
-
- writer.print(c.message);
- writer.print("\n\n");
- }
-
- private void appendIdent(PrintWriter writer, String header, Account.Id id, Timestamp ts) {
- PersonIdent ident = newIdent(id, ts, serverIdent);
- StringBuilder name = new StringBuilder();
- PersonIdent.appendSanitized(name, ident.getName());
- name.append(" <");
- PersonIdent.appendSanitized(name, ident.getEmailAddress());
- name.append('>');
- appendHeaderField(writer, header, name.toString());
- }
-
- private static final CharMatcher INVALID_FOOTER_CHARS = CharMatcher.anyOf("\r\n\0");
-
- static String sanitizeFooter(String value) {
- // Remove characters that would confuse JGit's footer parser if they were
- // included in footer values, for example by splitting the footer block into
- // multiple paragraphs.
- //
- // One painful example: RevCommit#getShorMessage() might return a message
- // containing "\r\r", which RevCommit#getFooterLines() will treat as an
- // empty paragraph for the purposes of footer parsing.
- return INVALID_FOOTER_CHARS.trimAndCollapseFrom(value, ' ');
+ public LegacyChangeNoteWrite getLegacyChangeNoteWrite() {
+ return legacyChangeNoteWrite;
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 06d940e..0bf2108 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -346,7 +346,13 @@
@Override
public ChangeNotesState call() throws ConfigInvalidException, IOException {
ChangeNotesParser parser =
- new ChangeNotesParser(key.changeId(), key.id(), rw, args.noteUtil, args.metrics);
+ new ChangeNotesParser(
+ key.changeId(),
+ key.id(),
+ rw,
+ args.changeNoteJson,
+ args.legacyChangeNoteRead,
+ args.metrics);
ChangeNotesState result = parser.parseAll();
// This assignment only happens if call() was actually called, which only
// happens when Cache#get(K, Callable<V>) incurs a cache miss.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 2eb30ff..689b024 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -124,7 +124,9 @@
}
// Private final members initialized in the constructor.
- private final ChangeNoteUtil noteUtil;
+ private final ChangeNoteJson changeNoteJson;
+ private final LegacyChangeNoteRead legacyChangeNoteRead;
+
private final NoteDbMetrics metrics;
private final Change.Id id;
private final ObjectId tip;
@@ -175,12 +177,14 @@
Change.Id changeId,
ObjectId tip,
ChangeNotesRevWalk walk,
- ChangeNoteUtil noteUtil,
+ ChangeNoteJson changeNoteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics) {
this.id = changeId;
this.tip = tip;
this.walk = walk;
- this.noteUtil = noteUtil;
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = legacyChangeNoteRead;
this.metrics = metrics;
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
@@ -446,7 +450,7 @@
return effectiveAccountId;
}
PersonIdent ident = RawParseUtils.parsePersonIdent(realUser);
- return noteUtil.parseIdent(ident, id);
+ return legacyChangeNoteRead.parseIdent(ident, id);
}
private String parseTopic(ChangeNotesCommit commit) throws ConfigInvalidException {
@@ -581,7 +585,7 @@
parsedAssignee = Optional.empty();
} else {
PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue);
- parsedAssignee = Optional.ofNullable(noteUtil.parseIdent(ident, id));
+ parsedAssignee = Optional.ofNullable(legacyChangeNoteRead.parseIdent(ident, id));
}
if (assignee == null) {
assignee = parsedAssignee;
@@ -749,7 +753,8 @@
ChangeNotesCommit tipCommit = walk.parseCommit(tip);
revisionNoteMap =
RevisionNoteMap.parse(
- noteUtil,
+ changeNoteJson,
+ legacyChangeNoteRead,
id,
reader,
NoteMap.read(reader, tipCommit),
@@ -807,7 +812,7 @@
labelVoteStr = line.substring(0, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
- effectiveAccountId = noteUtil.parseIdent(ident, id);
+ effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
} else {
labelVoteStr = line;
effectiveAccountId = committerId;
@@ -849,7 +854,7 @@
label = line.substring(1, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
- effectiveAccountId = noteUtil.parseIdent(ident, id);
+ effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
} else {
label = line.substring(1);
effectiveAccountId = committerId;
@@ -913,7 +918,7 @@
label.label = line.substring(c + 2, c2);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2));
checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line);
- label.appliedBy = noteUtil.parseIdent(ident, id);
+ label.appliedBy = legacyChangeNoteRead.parseIdent(ident, id);
} else {
label.label = line.substring(c + 2);
}
@@ -929,7 +934,7 @@
if (a.getName().equals(c.getName()) && a.getEmailAddress().equals(c.getEmailAddress())) {
return null;
}
- return noteUtil.parseIdent(commit.getAuthorIdent(), id);
+ return legacyChangeNoteRead.parseIdent(commit.getAuthorIdent(), id);
}
private void parseReviewer(Timestamp ts, ReviewerStateInternal state, String line)
@@ -938,7 +943,7 @@
if (ident == null) {
throw invalidFooter(state.getFooterKey(), line);
}
- Account.Id accountId = noteUtil.parseIdent(ident, id);
+ Account.Id accountId = legacyChangeNoteRead.parseIdent(ident, id);
reviewerUpdates.add(ReviewerStatusUpdate.create(ts, ownerId, accountId, state));
if (!reviewers.containsRow(accountId)) {
reviewers.put(accountId, state, ts);
diff --git a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
index 35e4a12..894e979 100644
--- a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
@@ -37,19 +37,22 @@
// See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE
private static final byte[] END_SIGNATURE = "-----END PGP SIGNATURE-----\n".getBytes(UTF_8);
- private final ChangeNoteUtil noteUtil;
+ private final ChangeNoteJson noteJson;
+ private final LegacyChangeNoteRead legacyChangeNoteRead;
private final Change.Id changeId;
private final PatchLineComment.Status status;
private String pushCert;
ChangeRevisionNote(
- ChangeNoteUtil noteUtil,
+ ChangeNoteJson noteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
Change.Id changeId,
ObjectReader reader,
ObjectId noteId,
PatchLineComment.Status status) {
super(reader, noteId);
- this.noteUtil = noteUtil;
+ this.legacyChangeNoteRead = legacyChangeNoteRead;
+ this.noteJson = noteJson;
this.changeId = changeId;
this.status = status;
}
@@ -65,7 +68,7 @@
p.value = offset;
if (isJson(raw, p.value)) {
- RevisionNoteData data = parseJson(noteUtil, raw, p.value);
+ RevisionNoteData data = parseJson(noteJson, raw, p.value);
if (status == PatchLineComment.Status.PUBLISHED) {
pushCert = data.pushCert;
} else {
@@ -80,7 +83,7 @@
} else {
pushCert = null;
}
- List<Comment> comments = noteUtil.parseNote(raw, p, changeId);
+ List<Comment> comments = legacyChangeNoteRead.parseNote(raw, p, changeId);
comments.forEach(c -> c.legacyFormat = true);
return comments;
}
@@ -89,7 +92,7 @@
return raw[offset] == '{' || raw[offset] == '[';
}
- private RevisionNoteData parseJson(ChangeNoteUtil noteUtil, byte[] raw, int offset)
+ private RevisionNoteData parseJson(ChangeNoteJson noteUtil, byte[] raw, int offset)
throws IOException {
try (InputStream is = new ByteArrayInputStream(raw, offset, raw.length - offset);
Reader r = new InputStreamReader(is, UTF_8)) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index ccc5859..445f7a0 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -40,7 +40,7 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TAG;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_WORK_IN_PROGRESS;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.sanitizeFooter;
+import static com.google.gerrit.server.notedb.NoteDbUtil.sanitizeFooter;
import static java.util.Comparator.comparing;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@@ -527,7 +527,13 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
ObjectId data =
- inserter.insert(OBJ_BLOB, e.getValue().build(noteUtil, noteUtil.getWriteJson()));
+ inserter.insert(
+ OBJ_BLOB,
+ e.getValue()
+ .build(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteWrite(),
+ noteUtil.getChangeNoteJson().getWriteJson()));
rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data);
}
@@ -555,7 +561,12 @@
// 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(
- noteUtil, getId(), rw.getObjectReader(), noteMap, PatchLineComment.Status.PUBLISHED);
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ getId(),
+ rw.getObjectReader(),
+ noteMap,
+ PatchLineComment.Status.PUBLISHED);
}
private void checkComments(
@@ -738,7 +749,7 @@
}
if (readOnlyUntil != null) {
- addFooter(msg, FOOTER_READ_ONLY_UNTIL, ChangeNoteUtil.formatTime(serverIdent, readOnlyUntil));
+ addFooter(msg, FOOTER_READ_ONLY_UNTIL, NoteDbUtil.formatTime(serverIdent, readOnlyUntil));
}
if (isPrivate != null) {
diff --git a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
index b3907eb..9a8c130 100644
--- a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
+++ b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
@@ -133,9 +133,14 @@
*/
@VisibleForTesting
public static Map<String, Comment> getPublishedComments(
- ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap)
+ ChangeNoteJson changeNoteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
+ Change.Id changeId,
+ ObjectReader reader,
+ NoteMap noteMap)
throws IOException, ConfigInvalidException {
- return RevisionNoteMap.parse(noteUtil, changeId, reader, noteMap, PUBLISHED)
+ return RevisionNoteMap.parse(
+ changeNoteJson, legacyChangeNoteRead, changeId, reader, noteMap, PUBLISHED)
.revisionNotes
.values()
.stream()
@@ -143,6 +148,16 @@
.collect(toMap(c -> c.key.uuid, Function.identity()));
}
+ public static Map<String, Comment> getPublishedComments(
+ ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap)
+ throws IOException, ConfigInvalidException {
+ return getPublishedComments(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ changeId,
+ reader,
+ noteMap);
+ }
/**
* Gets the comments put in by the current commit. The message of the target comment will be
* replaced by the new message.
@@ -205,7 +220,12 @@
throws IOException, ConfigInvalidException {
RevisionNoteMap<ChangeRevisionNote> revNotesMap =
RevisionNoteMap.parse(
- noteUtil, changeId, reader, NoteMap.read(reader, parentCommit), PUBLISHED);
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ changeId,
+ reader,
+ NoteMap.read(reader, parentCommit),
+ PUBLISHED);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
for (Comment c : putInComments) {
@@ -219,7 +239,13 @@
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders();
for (Map.Entry<RevId, RevisionNoteBuilder> entry : builders.entrySet()) {
ObjectId objectId = ObjectId.fromString(entry.getKey().get());
- byte[] data = entry.getValue().build(noteUtil, noteUtil.getWriteJson());
+ byte[] data =
+ entry
+ .getValue()
+ .build(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteWrite(),
+ noteUtil.getChangeNoteJson().getWriteJson());
if (data.length == 0) {
revNotesMap.noteMap.remove(objectId);
} else {
diff --git a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
index 008f31f..f66665c 100644
--- a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
+++ b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
@@ -155,7 +155,8 @@
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
RevisionNoteMap.parse(
- args.noteUtil,
+ args.changeNoteJson,
+ args.legacyChangeNoteRead,
getChangeId(),
reader,
NoteMap.read(reader, tipCommit),
diff --git a/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java b/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java
new file mode 100644
index 0000000..819c8ac
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java
@@ -0,0 +1,402 @@
+// Copyright (C) 2018 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.gerrit.server.notedb.ChangeNotes.parseException;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.reviewdb.client.Comment.Key;
+import com.google.gerrit.reviewdb.client.CommentRange;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.config.GerritServerId;
+import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.text.ParseException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.util.GitDateParser;
+import org.eclipse.jgit.util.MutableInteger;
+import org.eclipse.jgit.util.QuotedString;
+import org.eclipse.jgit.util.RawParseUtils;
+
+public class LegacyChangeNoteRead {
+ private final String serverId;
+
+ @Inject
+ public LegacyChangeNoteRead(@GerritServerId String serverId) {
+ this.serverId = serverId;
+ }
+
+ public Account.Id parseIdent(PersonIdent ident, Change.Id changeId)
+ throws ConfigInvalidException {
+ return NoteDbUtil.parseIdent(ident, serverId)
+ .orElseThrow(
+ () ->
+ parseException(
+ changeId,
+ "invalid identity, expected <id>@%s: %s",
+ serverId,
+ ident.getEmailAddress()));
+ }
+
+ private static boolean match(byte[] note, MutableInteger p, byte[] expected) {
+ int m = RawParseUtils.match(note, p.value, expected);
+ return m == p.value + expected.length;
+ }
+
+ public List<Comment> parseNote(byte[] note, MutableInteger p, Change.Id changeId)
+ throws ConfigInvalidException {
+ if (p.value >= note.length) {
+ return ImmutableList.of();
+ }
+ Set<Key> seen = new HashSet<>();
+ List<Comment> result = new ArrayList<>();
+ int sizeOfNote = note.length;
+ byte[] psb = ChangeNoteUtil.PATCH_SET.getBytes(UTF_8);
+ byte[] bpsb = ChangeNoteUtil.BASE_PATCH_SET.getBytes(UTF_8);
+ byte[] bpn = ChangeNoteUtil.PARENT_NUMBER.getBytes(UTF_8);
+
+ RevId revId = new RevId(parseStringField(note, p, changeId, ChangeNoteUtil.REVISION));
+ String fileName = null;
+ PatchSet.Id psId = null;
+ boolean isForBase = false;
+ Integer parentNumber = null;
+
+ while (p.value < sizeOfNote) {
+ boolean matchPs = match(note, p, psb);
+ boolean matchBase = match(note, p, bpsb);
+ if (matchPs) {
+ fileName = null;
+ psId = parsePsId(note, p, changeId, ChangeNoteUtil.PATCH_SET);
+ isForBase = false;
+ } else if (matchBase) {
+ fileName = null;
+ psId = parsePsId(note, p, changeId, ChangeNoteUtil.BASE_PATCH_SET);
+ isForBase = true;
+ if (match(note, p, bpn)) {
+ parentNumber = parseParentNumber(note, p, changeId);
+ }
+ } else if (psId == null) {
+ throw parseException(
+ changeId,
+ "missing %s or %s header",
+ ChangeNoteUtil.PATCH_SET,
+ ChangeNoteUtil.BASE_PATCH_SET);
+ }
+
+ Comment c = parseComment(note, p, fileName, psId, revId, isForBase, parentNumber);
+ fileName = c.key.filename;
+ if (!seen.add(c.key)) {
+ throw parseException(changeId, "multiple comments for %s in note", c.key);
+ }
+ result.add(c);
+ }
+ return result;
+ }
+
+ private Comment parseComment(
+ byte[] note,
+ MutableInteger curr,
+ String currentFileName,
+ PatchSet.Id psId,
+ RevId revId,
+ boolean isForBase,
+ Integer parentNumber)
+ throws ConfigInvalidException {
+ Change.Id changeId = psId.getParentKey();
+
+ // Check if there is a new file.
+ boolean newFile =
+ (RawParseUtils.match(note, curr.value, ChangeNoteUtil.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", ChangeNoteUtil.FILE);
+ }
+
+ CommentRange range = parseCommentRange(note, curr);
+ if (range == null) {
+ throw parseException(changeId, "could not parse %s", ChangeNoteUtil.COMMENT_RANGE);
+ }
+
+ Timestamp commentTime = parseTimestamp(note, curr, changeId);
+ Account.Id aId = parseAuthor(note, curr, changeId, ChangeNoteUtil.AUTHOR);
+ boolean hasRealAuthor =
+ (RawParseUtils.match(note, curr.value, ChangeNoteUtil.REAL_AUTHOR.getBytes(UTF_8))) != -1;
+ Account.Id raId = null;
+ if (hasRealAuthor) {
+ raId = parseAuthor(note, curr, changeId, ChangeNoteUtil.REAL_AUTHOR);
+ }
+
+ boolean hasParent =
+ (RawParseUtils.match(note, curr.value, ChangeNoteUtil.PARENT.getBytes(UTF_8))) != -1;
+ String parentUUID = null;
+ boolean unresolved = false;
+ if (hasParent) {
+ parentUUID = parseStringField(note, curr, changeId, ChangeNoteUtil.PARENT);
+ }
+ boolean hasUnresolved =
+ (RawParseUtils.match(note, curr.value, ChangeNoteUtil.UNRESOLVED.getBytes(UTF_8))) != -1;
+ if (hasUnresolved) {
+ unresolved = parseBooleanField(note, curr, changeId, ChangeNoteUtil.UNRESOLVED);
+ }
+
+ String uuid = parseStringField(note, curr, changeId, ChangeNoteUtil.UUID);
+
+ boolean hasTag =
+ (RawParseUtils.match(note, curr.value, ChangeNoteUtil.TAG.getBytes(UTF_8))) != -1;
+ String tag = null;
+ if (hasTag) {
+ tag = parseStringField(note, curr, changeId, ChangeNoteUtil.TAG);
+ }
+
+ int commentLength = parseCommentLength(note, curr, changeId);
+
+ String message = RawParseUtils.decode(UTF_8, note, curr.value, curr.value + commentLength);
+ checkResult(message, "message contents", changeId);
+
+ Comment c =
+ new Comment(
+ new Comment.Key(uuid, currentFileName, psId.get()),
+ aId,
+ commentTime,
+ isForBase ? (short) (parentNumber == null ? 0 : -parentNumber) : (short) 1,
+ message,
+ serverId,
+ unresolved);
+ c.lineNbr = range.getEndLine();
+ c.parentUuid = parentUUID;
+ c.tag = tag;
+ c.setRevId(revId);
+ if (raId != null) {
+ c.setRealAuthor(raId);
+ }
+
+ if (range.getStartCharacter() != -1) {
+ c.setRange(range);
+ }
+
+ curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
+ curr.value = RawParseUtils.nextLF(note, curr.value);
+ return c;
+ }
+
+ 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 last = ptr.value;
+ int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
+ if (ptr.value == last) {
+ return null;
+ } else 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;
+ }
+
+ last = ptr.value;
+ int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
+ if (ptr.value == last) {
+ return null;
+ } else if (note[ptr.value] == '-') {
+ range.setStartCharacter(startChar);
+ ptr.value += 1;
+ } else {
+ return null;
+ }
+
+ last = ptr.value;
+ int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
+ if (ptr.value == last) {
+ return null;
+ } else if (note[ptr.value] == ':') {
+ range.setEndLine(endLine);
+ ptr.value += 1;
+ } else {
+ return null;
+ }
+
+ last = ptr.value;
+ int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
+ if (ptr.value == last) {
+ return null;
+ } else 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 Integer parseParentNumber(byte[] note, MutableInteger curr, Change.Id changeId)
+ throws ConfigInvalidException {
+ checkHeaderLineFormat(note, curr, ChangeNoteUtil.PARENT_NUMBER, changeId);
+
+ int start = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
+ MutableInteger i = new MutableInteger();
+ int parentNumber = RawParseUtils.parseBase10(note, start, i);
+ int endOfLine = RawParseUtils.nextLF(note, curr.value);
+ if (i.value != endOfLine - 1) {
+ throw parseException(changeId, "could not parse %s", ChangeNoteUtil.PARENT_NUMBER);
+ }
+ checkResult(parentNumber, "parent number", changeId);
+ curr.value = endOfLine;
+ return Integer.valueOf(parentNumber);
+ }
+
+ private static String parseFilename(byte[] note, MutableInteger curr, Change.Id changeId)
+ throws ConfigInvalidException {
+ checkHeaderLineFormat(note, curr, ChangeNoteUtil.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, String fieldName)
+ throws ConfigInvalidException {
+ checkHeaderLineFormat(note, curr, fieldName, 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, fieldName, changeId);
+ }
+
+ private static int parseCommentLength(byte[] note, MutableInteger curr, Change.Id changeId)
+ throws ConfigInvalidException {
+ checkHeaderLineFormat(note, curr, ChangeNoteUtil.LENGTH, changeId);
+ int startOfLength = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
+ MutableInteger i = new MutableInteger();
+ i.value = startOfLength;
+ int commentLength = RawParseUtils.parseBase10(note, startOfLength, i);
+ if (i.value == startOfLength) {
+ throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
+ }
+ int endOfLine = RawParseUtils.nextLF(note, curr.value);
+ if (i.value != endOfLine - 1) {
+ throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
+ }
+ curr.value = endOfLine;
+ return checkResult(commentLength, "comment length", changeId);
+ }
+
+ private boolean parseBooleanField(
+ byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
+ throws ConfigInvalidException {
+ String str = parseStringField(note, curr, changeId, fieldName);
+ if ("true".equalsIgnoreCase(str)) {
+ return true;
+ } else if ("false".equalsIgnoreCase(str)) {
+ return false;
+ }
+ throw parseException(changeId, "invalid boolean for %s: %s", fieldName, str);
+ }
+
+ 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 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);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java b/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
new file mode 100644
index 0000000..1cf0c7c
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
@@ -0,0 +1,206 @@
+// Copyright (C) 2018 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.CommentsUtil.COMMENT_ORDER;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.GerritServerId;
+import com.google.inject.Inject;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.util.QuotedString;
+
+public class LegacyChangeNoteWrite {
+
+ private final AccountCache accountCache;
+ private final PersonIdent serverIdent;
+ private final String serverId;
+
+ @Inject
+ public LegacyChangeNoteWrite(
+ AccountCache accountCache,
+ @GerritPersonIdent PersonIdent serverIdent,
+ @GerritServerId String serverId) {
+ this.accountCache = accountCache;
+ this.serverIdent = serverIdent;
+ this.serverId = serverId;
+ }
+
+ public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
+ Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
+ return new PersonIdent(
+ author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
+ authorId.get() + "@" + serverId,
+ when,
+ serverIdent.getTimeZone());
+ }
+
+ @VisibleForTesting
+ public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
+ return new PersonIdent(
+ author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
+ }
+
+ public String getServerId() {
+ return serverId;
+ }
+
+ private void appendHeaderField(PrintWriter writer, String field, String value) {
+ writer.print(field);
+ writer.print(": ");
+ writer.print(value);
+ writer.print('\n');
+ }
+
+ /**
+ * Build a note that contains the metadata for and the contents of all of the comments in the
+ * given comments.
+ *
+ * @param comments Comments to be written to the output stream, keyed by patch set ID; multiple
+ * patch sets are allowed since base revisions may be shared across patch sets. All of the
+ * comments must share the same RevId, and all the comments for a given patch set must have
+ * the same side.
+ * @param out output stream to write to.
+ */
+ void buildNote(ListMultimap<Integer, Comment> comments, OutputStream out) {
+ if (comments.isEmpty()) {
+ return;
+ }
+
+ List<Integer> psIds = new ArrayList<>(comments.keySet());
+ Collections.sort(psIds);
+
+ OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
+ try (PrintWriter writer = new PrintWriter(streamWriter)) {
+ String revId = comments.values().iterator().next().revId;
+ appendHeaderField(writer, ChangeNoteUtil.REVISION, revId);
+
+ for (int psId : psIds) {
+ List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId));
+ Comment first = psComments.get(0);
+
+ short side = first.side;
+ appendHeaderField(
+ writer,
+ side <= 0 ? ChangeNoteUtil.BASE_PATCH_SET : ChangeNoteUtil.PATCH_SET,
+ Integer.toString(psId));
+ if (side < 0) {
+ appendHeaderField(writer, ChangeNoteUtil.PARENT_NUMBER, Integer.toString(-side));
+ }
+
+ String currentFilename = null;
+
+ for (Comment c : psComments) {
+ checkArgument(
+ revId.equals(c.revId),
+ "All comments being added must have all the same RevId. The "
+ + "comment below does not have the same RevId as the others "
+ + "(%s).\n%s",
+ revId,
+ c);
+ checkArgument(
+ side == c.side,
+ "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);
+ String commentFilename = QuotedString.GIT_PATH.quote(c.key.filename);
+
+ if (!commentFilename.equals(currentFilename)) {
+ currentFilename = commentFilename;
+ writer.print("File: ");
+ writer.print(commentFilename);
+ writer.print("\n\n");
+ }
+
+ appendOneComment(writer, c);
+ }
+ }
+ }
+ }
+
+ private void appendOneComment(PrintWriter writer, Comment c) {
+ // The CommentRange field for a comment is allowed to be null. If it is
+ // 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.
+ Comment.Range range = c.range;
+ if (range != null) {
+ writer.print(range.startLine);
+ writer.print(':');
+ writer.print(range.startChar);
+ writer.print('-');
+ writer.print(range.endLine);
+ writer.print(':');
+ writer.print(range.endChar);
+ } else {
+ writer.print(c.lineNbr);
+ }
+ writer.print("\n");
+
+ writer.print(NoteDbUtil.formatTime(serverIdent, c.writtenOn));
+ writer.print("\n");
+
+ appendIdent(writer, ChangeNoteUtil.AUTHOR, c.author.getId(), c.writtenOn);
+ if (!c.getRealAuthor().equals(c.author)) {
+ appendIdent(writer, ChangeNoteUtil.REAL_AUTHOR, c.getRealAuthor().getId(), c.writtenOn);
+ }
+
+ String parent = c.parentUuid;
+ if (parent != null) {
+ appendHeaderField(writer, ChangeNoteUtil.PARENT, parent);
+ }
+
+ appendHeaderField(writer, ChangeNoteUtil.UNRESOLVED, Boolean.toString(c.unresolved));
+ appendHeaderField(writer, ChangeNoteUtil.UUID, c.key.uuid);
+
+ if (c.tag != null) {
+ appendHeaderField(writer, ChangeNoteUtil.TAG, c.tag);
+ }
+
+ byte[] messageBytes = c.message.getBytes(UTF_8);
+ appendHeaderField(writer, ChangeNoteUtil.LENGTH, Integer.toString(messageBytes.length));
+
+ writer.print(c.message);
+ writer.print("\n\n");
+ }
+
+ private void appendIdent(PrintWriter writer, String header, Account.Id id, Timestamp ts) {
+ PersonIdent ident = newIdent(id, ts, serverIdent);
+ StringBuilder name = new StringBuilder();
+ PersonIdent.appendSanitized(name, ident.getName());
+ name.append(" <");
+ PersonIdent.appendSanitized(name, ident.getEmailAddress());
+ name.append('>');
+ appendHeaderField(writer, header, name.toString());
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUtil.java b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
index 59c4c62..21fada8 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUtil.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
@@ -14,12 +14,21 @@
package com.google.gerrit.server.notedb;
+import com.google.common.base.CharMatcher;
import com.google.common.primitives.Ints;
import com.google.gerrit.reviewdb.client.Account;
+import java.sql.Timestamp;
import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.util.GitDateFormatter;
+import org.eclipse.jgit.util.GitDateFormatter.Format;
public class NoteDbUtil {
+
+ /**
+ * Returns an AccountId for the given email address. Returns empty if the address isn't on this
+ * server.
+ */
public static Optional<Account.Id> parseIdent(PersonIdent ident, String serverId) {
String email = ident.getEmailAddress();
int at = email.indexOf('@');
@@ -36,4 +45,24 @@
}
private NoteDbUtil() {}
+
+ 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 static final CharMatcher INVALID_FOOTER_CHARS = CharMatcher.anyOf("\r\n\0");
+
+ static String sanitizeFooter(String value) {
+ // Remove characters that would confuse JGit's footer parser if they were
+ // included in footer values, for example by splitting the footer block into
+ // multiple paragraphs.
+ //
+ // One painful example: RevCommit#getShorMessage() might return a message
+ // containing "\r\r", which RevCommit#getFooterLines() will treat as an
+ // empty paragraph for the purposes of footer parsing.
+ return INVALID_FOOTER_CHARS.trimAndCollapseFrom(value, ' ');
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
index b341ea8..8bf286d 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
@@ -83,11 +83,17 @@
}
public byte[] build(ChangeNoteUtil noteUtil, boolean writeJson) throws IOException {
+ return build(noteUtil.getChangeNoteJson(), noteUtil.getLegacyChangeNoteWrite(), writeJson);
+ }
+
+ public byte[] build(
+ ChangeNoteJson changeNoteJson, LegacyChangeNoteWrite legacyChangeNoteWrite, boolean writeJson)
+ throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
if (writeJson) {
- buildNoteJson(noteUtil, out);
+ buildNoteJson(changeNoteJson, out);
} else {
- buildNoteLegacy(noteUtil, out);
+ buildNoteLegacy(legacyChangeNoteWrite, out);
}
return out.toByteArray();
}
@@ -122,7 +128,7 @@
return all;
}
- private void buildNoteJson(ChangeNoteUtil noteUtil, OutputStream out) throws IOException {
+ private void buildNoteJson(ChangeNoteJson noteUtil, OutputStream out) throws IOException {
ListMultimap<Integer, Comment> comments = buildCommentMap();
if (comments.isEmpty() && pushCert == null) {
return;
@@ -137,7 +143,8 @@
}
}
- private void buildNoteLegacy(ChangeNoteUtil noteUtil, OutputStream out) throws IOException {
+ private void buildNoteLegacy(LegacyChangeNoteWrite noteUtil, OutputStream out)
+ throws IOException {
if (pushCert != null) {
byte[] certBytes = pushCert.getBytes(UTF_8);
out.write(certBytes, 0, trimTrailingNewlines(certBytes));
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
index aa82d1a..17a061a 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
@@ -32,7 +32,8 @@
final ImmutableMap<RevId, T> revisionNotes;
static RevisionNoteMap<ChangeRevisionNote> parse(
- ChangeNoteUtil noteUtil,
+ ChangeNoteJson noteJson,
+ LegacyChangeNoteRead legacyChangeNoteRead,
Change.Id changeId,
ObjectReader reader,
NoteMap noteMap,
@@ -41,7 +42,8 @@
Map<RevId, ChangeRevisionNote> result = new HashMap<>();
for (Note note : noteMap) {
ChangeRevisionNote rn =
- new ChangeRevisionNote(noteUtil, changeId, reader, note.getData(), status);
+ new ChangeRevisionNote(
+ noteJson, legacyChangeNoteRead, changeId, reader, note.getData(), status);
rn.parse();
result.put(new RevId(note.name()), rn);
}
@@ -49,12 +51,12 @@
}
static RevisionNoteMap<RobotCommentsRevisionNote> parseRobotComments(
- ChangeNoteUtil noteUtil, ObjectReader reader, NoteMap noteMap)
+ ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws ConfigInvalidException, IOException {
Map<RevId, RobotCommentsRevisionNote> result = new HashMap<>();
for (Note note : noteMap) {
RobotCommentsRevisionNote rn =
- new RobotCommentsRevisionNote(noteUtil, reader, note.getData());
+ new RobotCommentsRevisionNote(changeNoteJson, reader, note.getData());
rn.parse();
result.put(new RevId(note.name()), rn);
}
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentNotes.java b/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
index 99d9615..7eb3a54 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
@@ -89,7 +89,8 @@
RevCommit tipCommit = handle.walk().parseCommit(metaId);
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
- RevisionNoteMap.parseRobotComments(args.noteUtil, reader, NoteMap.read(reader, tipCommit));
+ RevisionNoteMap.parseRobotComments(
+ args.changeNoteJson, reader, NoteMap.read(reader, tipCommit));
ListMultimap<RevId, RobotComment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (RobotCommentsRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
for (RobotComment c : rn.getComments()) {
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
index c28125f..3a0d595 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
@@ -202,7 +202,8 @@
}
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
- return RevisionNoteMap.parseRobotComments(noteUtil, rw.getObjectReader(), noteMap);
+ return RevisionNoteMap.parseRobotComments(
+ noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap);
}
@Override
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java b/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
index aa229ab..6c3cc86 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
@@ -27,9 +27,9 @@
import org.eclipse.jgit.lib.ObjectReader;
public class RobotCommentsRevisionNote extends RevisionNote<RobotComment> {
- private final ChangeNoteUtil noteUtil;
+ private final ChangeNoteJson noteUtil;
- RobotCommentsRevisionNote(ChangeNoteUtil noteUtil, ObjectReader reader, ObjectId noteId) {
+ RobotCommentsRevisionNote(ChangeNoteJson noteUtil, ObjectReader reader, ObjectId noteId) {
super(reader, noteId);
this.noteUtil = noteUtil;
}
diff --git a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 92a878c..3a0bfc1 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -546,7 +546,7 @@
if (id == null) {
return new PersonIdent(serverIdent, events.getWhen());
}
- return changeNoteUtil.newIdent(id, events.getWhen(), serverIdent);
+ return changeNoteUtil.getLegacyChangeNoteWrite().newIdent(id, events.getWhen(), serverIdent);
}
private List<HashtagsEvent> getHashtagsEvents(Change change, NoteDbUpdateManager manager)
@@ -564,7 +564,10 @@
for (RevCommit commit : rw) {
Account.Id authorId;
try {
- authorId = changeNoteUtil.parseIdent(commit.getAuthorIdent(), change.getId());
+ authorId =
+ changeNoteUtil
+ .getLegacyChangeNoteRead()
+ .parseIdent(commit.getAuthorIdent(), change.getId());
} catch (ConfigInvalidException e) {
continue; // Corrupt data, no valid hashtags in this commit.
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 882996e..b85e2f2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2676,7 +2676,9 @@
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id), c.updated, serverIdent.get());
+ changeNoteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(getAccount(admin.id), c.updated, serverIdent.get());
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitPatchSetCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
@@ -2684,7 +2686,10 @@
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
- expectedAuthor = changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ expectedAuthor =
+ changeNoteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitChangeCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 45294fb..1de9d29 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -433,7 +433,9 @@
if (notesMigration.commitChangeWrites()) {
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
tr.branch(RefNames.changeMetaRef(c3.getId()))
.commit()
.author(author)
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index e510e25..3e9b90c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -273,7 +273,9 @@
assertThat(commit.getShortMessage()).isEqualTo("Create change");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ changeNoteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commit.getCommitterIdent())
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index e53b997..9621c16 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -950,7 +950,7 @@
@Test
public void jsonCommentHasLegacyFormatFalse() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getWriteJson()).isTrue();
+ assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isTrue();
PushOneCommit.Result result = createChange();
Change.Id changeId = result.getChange().getId();
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index 29c043a..ea44bd7 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -902,7 +902,9 @@
}
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
serverSideTestRepo
.branch(RefNames.changeMetaRef(id))
.commit()
diff --git a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
index a3a0339..e029e7a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
@@ -53,7 +53,7 @@
@Test
public void legacyCommentHasLegacyFormatTrue() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getWriteJson()).isFalse();
+ assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isFalse();
PushOneCommit.Result result = createChange();
Change.Id changeId = result.getChange().getId();
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
index 95d96b5..b7ce7bc 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
@@ -17,8 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.formatTime;
import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
+import static com.google.gerrit.server.notedb.NoteDbUtil.formatTime;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index b8f544a..de964d8 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -498,7 +498,11 @@
private RevCommit writeCommit(String body) throws Exception {
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
- body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
+ body,
+ noteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ false);
}
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
@@ -509,7 +513,9 @@
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
body,
- noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ noteUtil
+ .getLegacyChangeNoteWrite()
+ .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
initWorkInProgress);
}
@@ -555,7 +561,9 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
- ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
- return new ChangeNotesParser(newChange().getId(), tip, walk, noteUtil, args.metrics);
+ ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
+ LegacyChangeNoteRead reader = injector.getInstance(LegacyChangeNoteRead.class);
+ return new ChangeNotesParser(
+ newChange().getId(), tip, walk, changeNoteJson, reader, args.metrics);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 9d38704..4b553fd 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -77,6 +77,8 @@
public class ChangeNotesTest extends AbstractChangeNotesTest {
@Inject private DraftCommentNotes.Factory draftNotesFactory;
+ @Inject private ChangeNoteJson changeNoteJson;
+ @Inject private LegacyChangeNoteRead legacyChangeNoteRead;
@Inject private ChangeNoteUtil noteUtil;
@Inject private @GerritServerId String serverId;
@@ -1195,7 +1197,7 @@
+ "File: a.txt\n"
+ "\n"
+ "1:2-3:4\n"
- + ChangeNoteUtil.formatTime(serverIdent, ts)
+ + NoteDbUtil.formatTime(serverIdent, ts)
+ "\n"
+ "Author: Change Owner <1@gerrit>\n"
+ "Unresolved: false\n"
@@ -1288,7 +1290,13 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithComments =
- new ChangeNotesParser(c.getId(), commitWithComments.copy(), rw, noteUtil, args.metrics);
+ new ChangeNotesParser(
+ c.getId(),
+ commitWithComments.copy(),
+ rw,
+ changeNoteJson,
+ legacyChangeNoteRead,
+ args.metrics);
ChangeNotesState state = notesWithComments.parseAll();
assertThat(state.approvals()).isEmpty();
assertThat(state.publishedComments()).hasSize(1);
@@ -1296,7 +1304,14 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithApprovals =
- new ChangeNotesParser(c.getId(), commitWithApprovals.copy(), rw, noteUtil, args.metrics);
+ new ChangeNotesParser(
+ c.getId(),
+ commitWithApprovals.copy(),
+ rw,
+ changeNoteJson,
+ legacyChangeNoteRead,
+ args.metrics);
+
ChangeNotesState state = notesWithApprovals.parseAll();
assertThat(state.approvals()).hasSize(1);
assertThat(state.publishedComments()).hasSize(1);
@@ -1666,7 +1681,7 @@
+ "File: file1\n"
+ "\n"
+ "1:1-2:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time1)
+ + NoteDbUtil.formatTime(serverIdent, time1)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1675,7 +1690,7 @@
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time2)
+ + NoteDbUtil.formatTime(serverIdent, time2)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1686,7 +1701,7 @@
+ "File: file2\n"
+ "\n"
+ "3:0-4:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time3)
+ + NoteDbUtil.formatTime(serverIdent, time3)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1766,7 +1781,7 @@
+ "File: file1\n"
+ "\n"
+ "1:1-2:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time1)
+ + NoteDbUtil.formatTime(serverIdent, time1)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1775,7 +1790,7 @@
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time2)
+ + NoteDbUtil.formatTime(serverIdent, time2)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1854,7 +1869,7 @@
+ "File: file1\n"
+ "\n"
+ "1:1-2:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time1)
+ + NoteDbUtil.formatTime(serverIdent, time1)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Unresolved: false\n"
@@ -1863,7 +1878,7 @@
+ "comment 1\n"
+ "\n"
+ "1:1-2:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time2)
+ + NoteDbUtil.formatTime(serverIdent, time2)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Parent: uuid1\n"
@@ -1951,7 +1966,7 @@
byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
String noteString = new String(bytes, UTF_8);
- String timeStr = ChangeNoteUtil.formatTime(serverIdent, time);
+ String timeStr = NoteDbUtil.formatTime(serverIdent, time);
if (!testJson()) {
assertThat(noteString)
@@ -2048,7 +2063,7 @@
+ "File: file\n"
+ "\n"
+ "1:1-2:1\n"
- + ChangeNoteUtil.formatTime(serverIdent, time)
+ + NoteDbUtil.formatTime(serverIdent, time)
+ "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Real-author: Change Owner <1@gerrit>\n"
@@ -2103,7 +2118,7 @@
byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
String noteString = new String(bytes, UTF_8);
- String timeStr = ChangeNoteUtil.formatTime(serverIdent, time);
+ String timeStr = NoteDbUtil.formatTime(serverIdent, time);
if (!testJson()) {
assertThat(noteString)
@@ -3511,7 +3526,7 @@
}
private boolean testJson() {
- return noteUtil.getWriteJson();
+ return noteUtil.getChangeNoteJson().getWriteJson();
}
private String readNote(ChangeNotes notes, ObjectId noteId) throws Exception {
diff --git a/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java b/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
index aa37d51..e7d8956 100644
--- a/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
@@ -68,7 +68,7 @@
// Match ChangeNoteUtil#gson as of 4e1f02db913d91f2988f559048e513e6093a1bce
legacyGson = new GsonBuilder().setPrettyPrinting().create();
- gson = ChangeNoteUtil.newGson();
+ gson = ChangeNoteJson.newGson();
}
@After
