Merge changes I41ef88dd,I9492c9c5 * changes: Combine ChangeNoteUtil and CommentsInNotesUtil Include a UUID portion in NoteDb author identities
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 1172d16..386fe73 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -62,6 +62,7 @@ import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.index.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ProjectCache; @@ -191,6 +192,9 @@ @Inject protected FakeEmailSender sender; + @Inject + protected ChangeNoteUtil changeNoteUtil; + protected TestRepository<InMemoryRepository> testRepo; protected GerritServer server; protected TestAccount admin;
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java index ed8bf38..f53202f 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
@@ -43,6 +43,7 @@ import com.google.inject.Inject; import com.google.inject.Key; import com.google.inject.Provides; +import com.google.inject.ProvisionException; import com.google.inject.Singleton; import com.google.inject.TypeLiteral; @@ -50,6 +51,8 @@ import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; import org.eclipse.jgit.lib.Config; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -67,9 +70,11 @@ .toInstance(cfg); // TODO(dborowitz): Use jimfs. + Path p = Paths.get(cfg.getString("gerrit", null, "tempSiteDir")); bind(Path.class) .annotatedWith(SitePath.class) - .toInstance(Paths.get(cfg.getString("gerrit", null, "tempSiteDir"))); + .toInstance(p); + makeSiteDirs(p); bind(GitRepositoryManager.class) .toInstance(new InMemoryRepositoryManager()); @@ -135,4 +140,14 @@ mem.drop(); } } + + private static void makeSiteDirs(Path p) { + try { + Files.createDirectories(p.resolve("etc")); + } catch (IOException e) { + ProvisionException pe = new ProvisionException(e.getMessage()); + pe.initCause(e); + throw pe; + } + } }
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index f2886e1..ddb1b60 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1082,7 +1082,7 @@ assertThat(commitPatchSetCreation.getShortMessage()) .isEqualTo("Create patch set 2"); - PersonIdent expectedAuthor = ChangeNoteUtil.newIdent( + PersonIdent expectedAuthor = changeNoteUtil.newIdent( accountCache.get(admin.id).getAccount(), c.updated, serverIdent.get(), AnonymousCowardNameProvider.DEFAULT); assertThat(commitPatchSetCreation.getAuthorIdent()) @@ -1095,7 +1095,7 @@ rw.parseCommit(commitPatchSetCreation.getParent(0)); assertThat(commitChangeCreation.getShortMessage()) .isEqualTo("Create change"); - expectedAuthor = ChangeNoteUtil.newIdent( + expectedAuthor = changeNoteUtil.newIdent( accountCache.get(admin.id).getAccount(), c.created, serverIdent.get(), AnonymousCowardNameProvider.DEFAULT); assertThat(commitChangeCreation.getAuthorIdent())
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index d222dea..adf67f1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -127,7 +127,7 @@ assertThat(commit.getShortMessage()).isEqualTo("Create change"); - PersonIdent expectedAuthor = ChangeNoteUtil.newIdent( + PersonIdent expectedAuthor = changeNoteUtil.newIdent( accountCache.get(admin.id).getAccount(), c.created, serverIdent.get(), AnonymousCowardNameProvider.DEFAULT); assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java index 26b1941..0ba731a 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java
@@ -21,6 +21,7 @@ import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.init.api.InitStep; import com.google.gerrit.pgm.init.api.Section; +import com.google.gerrit.server.api.config.GerritServerIdProvider; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Binding; import com.google.inject.Guice; @@ -43,6 +44,7 @@ private final SitePaths site; private final Libraries libraries; private final Section database; + private final Section idSection; @Inject InitDatabase(final ConsoleUI ui, final SitePaths site, final Libraries libraries, @@ -51,6 +53,7 @@ this.site = site; this.libraries = libraries; this.database = sections.get("database", null); + this.idSection = sections.get(GerritServerIdProvider.SECTION, null); } @Override @@ -91,6 +94,13 @@ } dci.initConfig(database); + + // Initialize UUID for NoteDb on first init. + String id = idSection.get(GerritServerIdProvider.KEY); + if (Strings.isNullOrEmpty(id)) { + idSection.set( + GerritServerIdProvider.KEY, GerritServerIdProvider.generate()); + } } @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/config/GerritServerIdProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/config/GerritServerIdProvider.java new file mode 100644 index 0000000..f57f98c --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/config/GerritServerIdProvider.java
@@ -0,0 +1,67 @@ +// Copyright (C) 2016 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.api.config; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.base.Strings; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.config.SitePaths; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; + +import java.io.IOException; +import java.nio.file.Files; +import java.util.UUID; + +public class GerritServerIdProvider implements Provider<String> { + public static final String SECTION = "gerrit"; + public static final String KEY = "serverId"; + + public static String generate() { + return UUID.randomUUID().toString(); + } + + private final String id; + + @Inject + GerritServerIdProvider(@GerritServerConfig Config cfg, + SitePaths sitePaths) throws IOException, ConfigInvalidException { + String origId = cfg.getString(SECTION, null, KEY); + if (!Strings.isNullOrEmpty(origId)) { + id = origId; + return; + } + + // We're not generally supposed to do work in provider constructors, but + // this is a bit of a special case because we really need to have the ID + // available by the time the dbInjector is created. This even applies during + // RebuildNoteDb, which otherwise would have been a reasonable place to do + // the ID generation. Fortunately, it's not much work, and it happens once. + id = generate(); + Config cfgCopy = new Config(); + cfgCopy.fromText(cfg.toText()); + cfg.setString(SECTION, null, KEY, id); + Files.write(sitePaths.gerrit_config, cfg.toText().getBytes(UTF_8)); + } + + @Override + public String get() { + return id; + } +}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerId.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerId.java new file mode 100644 index 0000000..f3fa9b1 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerId.java
@@ -0,0 +1,32 @@ +// Copyright (C) 2016 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.config; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.inject.BindingAnnotation; + +import java.lang.annotation.Retention; + +/** + * Marker on a string holding a unique identifier for the server. + * <p> + * This value is generated on first use and stored in {@code + * $site_path/etc/uuid}. + */ +@Retention(RUNTIME) +@BindingAnnotation +public @interface GerritServerId { +}
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 e1d309c..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,6 +44,7 @@ protected final GitRepositoryManager repoManager; protected final ChangeControl ctl; protected final String anonymousCowardName; + protected final ChangeNoteUtil noteUtil; protected final Date when; private final PersonIdent serverIdent; @@ -55,12 +56,14 @@ ChangeControl ctl, PersonIdent serverIdent, String anonymousCowardName, + ChangeNoteUtil noteUtil, Date when) { this.migration = migration; this.repoManager = repoManager; this.ctl = ctl; this.serverIdent = serverIdent; this.anonymousCowardName = anonymousCowardName; + this.noteUtil = noteUtil; this.when = when; checkArgument( (ctl.getUser() instanceof IdentifiedUser) @@ -96,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; @@ -105,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 0665092..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,12 +90,12 @@ GitRepositoryManager repoManager, NotesMigration migration, AllUsersName allUsers, - CommentsInNotesUtil commentsUtil, + ChangeNoteUtil noteUtil, @Assisted ChangeControl ctl, @Assisted Date when) { - super(migration, repoManager, ctl, serverIdent, anonymousCowardName, when); + super(migration, repoManager, ctl, serverIdent, anonymousCowardName, + noteUtil, when); this.draftsProject = allUsers; - this.commentsUtil = commentsUtil; checkState(ctl.getUser().isIdentifiedUser(), "Current user must be identified"); IdentifiedUser user = ctl.getUser().asIdentifiedUser(); @@ -150,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 { @@ -195,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( - 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 f6f7dd5..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,20 +14,51 @@ 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 String GERRIT_PLACEHOLDER_HOST = "gerrit"; - static final FooterKey FOOTER_BRANCH = new FooterKey("Branch"); static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id"); static final FooterKey FOOTER_COMMIT = new FooterKey("Commit"); @@ -42,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); @@ -57,28 +98,409 @@ 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 + 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; + } + @VisibleForTesting - public static PersonIdent newIdent(Account author, Date when, + public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent, String anonymousCowardName) { return new PersonIdent( author.getName(anonymousCowardName), - author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST, + author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone()); } - public static Account.Id parseIdent(PersonIdent ident) { + public Account.Id parseIdent(PersonIdent ident, Change.Id changeId) + throws ConfigInvalidException { String email = ident.getEmailAddress(); int at = email.indexOf('@'); if (at >= 0) { String host = email.substring(at + 1, email.length()); - Integer id = Ints.tryParse(email.substring(0, at)); - if (id != null && host.equals(GERRIT_PLACEHOLDER_HOST)) { - return new Account.Id(id); + if (host.equals(serverId)) { + Integer id = Ints.tryParse(email.substring(0, at)); + if (id != null) { + return new Account.Id(id); + } } } - return null; + throw parseException(changeId, "invalid identity, expected <id>@%s: %s", + serverId, email); } - private 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 = 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 dba863e..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
@@ -17,7 +17,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; @@ -36,7 +35,6 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.collect.Ordering; -import com.google.common.primitives.Ints; import com.google.common.util.concurrent.AsyncFunction; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; @@ -70,7 +68,6 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; @@ -113,21 +110,6 @@ + String.format(fmt, args)); } - public static Account.Id parseIdent(PersonIdent ident, Change.Id changeId) - throws ConfigInvalidException { - String email = ident.getEmailAddress(); - int at = email.indexOf('@'); - if (at >= 0) { - String host = email.substring(at + 1, email.length()); - Integer id = Ints.tryParse(email.substring(0, at)); - if (id != null && host.equals(GERRIT_PLACEHOLDER_HOST)) { - return new Account.Id(id); - } - } - throw parseException(changeId, "invalid identity, expected <id>@%s: %s", - GERRIT_PLACEHOLDER_HOST, email); - } - @Singleton public static class Factory { private static final Logger log = LoggerFactory.getLogger(Factory.class); @@ -137,6 +119,7 @@ private final AllUsersName allUsers; private final Provider<InternalChangeQuery> queryProvider; private final ProjectCache projectCache; + private final ChangeNoteUtil noteUtil; @VisibleForTesting @Inject @@ -144,12 +127,14 @@ NotesMigration migration, AllUsersName allUsers, Provider<InternalChangeQuery> queryProvider, - ProjectCache projectCache) { + ProjectCache projectCache, + ChangeNoteUtil noteUtil) { this.repoManager = repoManager; this.migration = migration; this.allUsers = allUsers; this.queryProvider = queryProvider; this.projectCache = projectCache; + this.noteUtil = noteUtil; } public ChangeNotes createChecked(ReviewDb db, Change c) @@ -194,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, project, - change).load(); + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, + project, change).load(); } /** @@ -207,12 +192,12 @@ * @return change notes */ public ChangeNotes createFromIndexedChange(Change change) { - return new ChangeNotes(repoManager, migration, allUsers, + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, change.getProject(), change); } public ChangeNotes createForNew(Change change) throws OrmException { - return new ChangeNotes(repoManager, migration, allUsers, + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, change.getProject(), change).load(); } @@ -222,7 +207,7 @@ checkState(!migration.readChanges(), "do not call" + " createFromIdOnlyWhenNotedbDisabled when notedb is enabled"); Change change = unwrap(db).changes().get(changeId); - return new ChangeNotes(repoManager, migration, allUsers, + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, change.getProject(), change).load(); } @@ -235,7 +220,7 @@ throws OrmException { checkState(!migration.readChanges(), "do not call" + " createFromChangeWhenNotedbDisabled when notedb is enabled"); - return new ChangeNotes(repoManager, migration, allUsers, + return new ChangeNotes(repoManager, migration, allUsers, noteUtil, change.getProject(), change).load(); } @@ -255,8 +240,8 @@ "passed project %s when creating ChangeNotes for %s," + " but actual project is %s", project, changeId, change.getProject()); - return new ChangeNotes(repoManager, migration, - allUsers, project, change).load(); + return new ChangeNotes(repoManager, migration, allUsers, + noteUtil, project, change).load(); } }); } @@ -395,6 +380,7 @@ } } + private final ChangeNoteUtil noteUtil; private final Project.NameKey project; private final Change change; private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets; @@ -416,10 +402,11 @@ @VisibleForTesting public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName allUsers, 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.noteUtil = noteUtil; this.project = project; this.change = change != null ? new Change(change) : null; } @@ -517,7 +504,7 @@ if (draftCommentNotes == null || !author.equals(draftCommentNotes.getAuthor())) { draftCommentNotes = new DraftCommentNotes(repoManager, migration, - allUsers, getChangeId(), author); + allUsers, noteUtil, getChangeId(), author); draftCommentNotes.load(); } } @@ -563,8 +550,8 @@ return; } try (RevWalk walk = new RevWalk(reader); - ChangeNotesParser parser = new ChangeNotesParser(project, - change.getId(), rev, walk, repoManager)) { + 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 64b4b62..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
@@ -26,7 +26,6 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; -import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; import com.google.common.base.Enums; import com.google.common.base.Function; @@ -115,6 +114,7 @@ PatchSet.Id currentPatchSetId; RevisionNoteMap revisionNoteMap; + private final ChangeNoteUtil noteUtil; private final Change.Id id; private final ObjectId tip; private final RevWalk walk; @@ -125,12 +125,14 @@ private final Multimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet; ChangeNotesParser(Project.NameKey project, Change.Id changeId, ObjectId tip, - RevWalk walk, GitRepositoryManager repoManager) + RevWalk walk, GitRepositoryManager repoManager, + ChangeNoteUtil noteUtil) throws RepositoryNotFoundException, IOException { this.id = changeId; this.tip = tip; this.walk = walk; this.repo = repoManager.openMetadataRepository(project); + this.noteUtil = noteUtil; approvals = Maps.newHashMap(); reviewers = Maps.newLinkedHashMap(); allPastReviewers = Lists.newArrayList(); @@ -501,7 +503,7 @@ ObjectReader reader = walk.getObjectReader(); RevCommit tipCommit = walk.parseCommit(tip); revisionNoteMap = RevisionNoteMap.parse( - 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()) { @@ -540,7 +542,7 @@ labelVoteStr = line.substring(0, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = parseIdent(ident); + accountId = noteUtil.parseIdent(ident, id); } else { labelVoteStr = line; accountId = committerId; @@ -578,7 +580,7 @@ label = line.substring(1, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = parseIdent(ident); + accountId = noteUtil.parseIdent(ident, id); } else { label = line.substring(1); accountId = committerId; @@ -661,7 +663,7 @@ PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2)); checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line); - label.appliedBy = parseIdent(ident); + label.appliedBy = noteUtil.parseIdent(ident, id); } else { label.label = line.substring(c + 2); } @@ -679,17 +681,7 @@ && a.getEmailAddress().equals(c.getEmailAddress())) { return null; } - return parseIdent(commit.getAuthorIdent()); - } - - private Account.Id parseIdent(PersonIdent ident) - throws ConfigInvalidException { - Account.Id id = ChangeNoteUtil.parseIdent(ident); - if (id == null) { - throw parseException("invalid identity, expected <id>@%s: %s", - GERRIT_PLACEHOLDER_HOST, ident.getEmailAddress()); - } - return id; + return noteUtil.parseIdent(commit.getAuthorIdent(), id); } private void parseReviewer(ReviewerStateInternal state, String line) @@ -698,7 +690,7 @@ if (ident == null) { throw invalidFooter(state.getFooterKey(), line); } - Account.Id accountId = parseIdent(ident); + 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/ChangeRebuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java index 5926750..bddb35e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java
@@ -56,6 +56,7 @@ import com.google.inject.Inject; import com.google.inject.util.Providers; +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -89,6 +90,7 @@ private final ChangeUpdate.Factory updateFactory; private final ChangeDraftUpdate.Factory draftUpdateFactory; private final NoteDbUpdateManager.Factory updateManagerFactory; + private final ChangeNoteUtil changeNoteUtil; @Inject ChangeRebuilder(SchemaFactory<ReviewDb> schemaFactory, @@ -99,7 +101,8 @@ PatchListCache patchListCache, ChangeUpdate.Factory updateFactory, ChangeDraftUpdate.Factory draftUpdateFactory, - NoteDbUpdateManager.Factory updateManagerFactory) { + NoteDbUpdateManager.Factory updateManagerFactory, + ChangeNoteUtil changeNoteUtil) { this.schemaFactory = schemaFactory; this.repoManager = repoManager; this.controlFactory = controlFactory; @@ -109,6 +112,7 @@ this.updateFactory = updateFactory; this.draftUpdateFactory = draftUpdateFactory; this.updateManagerFactory = updateManagerFactory; + this.changeNoteUtil = changeNoteUtil; } public ListenableFuture<?> rebuildAsync(final Change.Id id, @@ -125,7 +129,8 @@ } public void rebuild(ReviewDb db, Change.Id changeId) - throws NoSuchChangeException, IOException, OrmException { + throws NoSuchChangeException, IOException, OrmException, + ConfigInvalidException { Change change = db.changes().get(changeId); if (change == null) { return; @@ -254,7 +259,7 @@ } private List<HashtagsEvent> getHashtagsEvents(Change change, - NoteDbUpdateManager manager) throws IOException { + NoteDbUpdateManager manager) throws IOException, ConfigInvalidException { String refName = ChangeNoteUtil.changeRefName(change.getId()); ObjectId old = manager.getChangeCommands() .getObjectId(manager.getChangeRepo(), refName); @@ -268,7 +273,7 @@ rw.markStart(rw.parseCommit(old)); for (RevCommit commit : rw) { Account.Id authorId = - ChangeNoteUtil.parseIdent(commit.getAuthorIdent()); + changeNoteUtil.parseIdent(commit.getAuthorIdent(), change.getId()); PatchSet.Id psId = parsePatchSetId(change, commit); Set<String> hashtags = parseHashtags(commit); if (authorId == null || psId == null || hashtags == null) {
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 aded275..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,10 +135,10 @@ ChangeDraftUpdate.Factory draftUpdateFactory, ProjectCache projectCache, @Assisted ChangeControl ctl, - CommentsInNotesUtil commentsUtil) { + ChangeNoteUtil noteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, - projectCache, ctl, serverIdent.getWhen(), commentsUtil); + projectCache, ctl, serverIdent.getWhen(), noteUtil); } @AssistedInject @@ -154,12 +153,12 @@ ProjectCache projectCache, @Assisted ChangeControl ctl, @Assisted Date when, - CommentsInNotesUtil commentsUtil) { + ChangeNoteUtil noteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, ctl, when, projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(), - commentsUtil); + noteUtil); } private static Project.NameKey getProjectName(ChangeControl ctl) { @@ -178,11 +177,10 @@ @Assisted ChangeControl ctl, @Assisted Date when, @Assisted Comparator<String> labelNameComparator, - CommentsInNotesUtil commentsUtil) { + ChangeNoteUtil noteUtil) { super(migration, repoManager, ctl, serverIdent, - anonymousCowardName, when); + anonymousCowardName, noteUtil, when); this.accountCache = accountCache; - this.commentsUtil = commentsUtil; this.draftUpdateFactory = draftUpdateFactory; this.updateManagerFactory = updateManagerFactory; @@ -375,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); } @@ -401,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( - 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 e660147..0000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java +++ /dev/null
@@ -1,473 +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.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; -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.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.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.Date; -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 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; - } - - 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 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 static 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 PersonIdent newIdent(Account author, Date when) { - return new PersonIdent( - author.getName(anonymousCowardName), - author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST, - when, serverIdent.getTimeZone()); - } - - private static Account.Id parseIdent(PersonIdent ident, Change.Id changeId) - throws ConfigInvalidException { - String email = ident.getEmailAddress(); - int at = email.indexOf('@'); - if (at >= 0) { - String host = email.substring(at + 1, email.length()); - Integer id = Ints.tryParse(email.substring(0, at)); - if (id != null && host.equals(GERRIT_PLACEHOLDER_HOST)) { - return new Account.Id(id); - } - } - throw parseException(changeId, "invalid identity, expected <id>@%s: %s", - GERRIT_PLACEHOLDER_HOST, email); - } - - 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); - } - } - - private final AccountCache accountCache; - private final PersonIdent serverIdent; - private final String anonymousCowardName; - - @Inject - public CommentsInNotesUtil(AccountCache accountCache, - @GerritPersonIdent PersonIdent serverIdent, - @AnonymousCowardName String anonymousCowardName) { - this.accountCache = accountCache; - this.serverIdent = serverIdent; - this.anonymousCowardName = anonymousCowardName; - } - - 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()); - 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 13312dc..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,34 +48,40 @@ private final GitRepositoryManager repoManager; private final NotesMigration migration; private final AllUsersName draftsProject; + private final ChangeNoteUtil noteUtil; @VisibleForTesting @Inject public Factory(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName allUsers) { + AllUsersName allUsers, + ChangeNoteUtil noteUtil) { this.repoManager = repoManager; this.migration = migration; this.draftsProject = allUsers; + this.noteUtil = noteUtil; } public DraftCommentNotes create(Change.Id changeId, Account.Id accountId) { return new DraftCommentNotes(repoManager, migration, draftsProject, - changeId, accountId); + noteUtil, changeId, accountId); } } private final AllUsersName draftsProject; + private final ChangeNoteUtil noteUtil; private final Account.Id author; private ImmutableListMultimap<RevId, PatchLineComment> comments; private RevisionNoteMap revisionNoteMap; DraftCommentNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName draftsProject, 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.noteUtil = noteUtil; } RevisionNoteMap getRevisionNoteMap() { @@ -116,7 +122,8 @@ try (RevWalk walk = new RevWalk(reader)) { RevCommit tipCommit = walk.parseCommit(rev); revisionNoteMap = RevisionNoteMap.parse( - getChangeId(), reader, NoteMap.read(reader, tipCommit), true); + noteUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit), + true); Multimap<RevId, PatchLineComment> cs = ArrayListMultimap.create(); for (RevisionNote rn : revisionNoteMap.revisionNotes.values()) { for (PatchLineComment c : rn.comments) {
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 b14b5f0..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,8 +63,9 @@ final ImmutableList<PatchLineComment> comments; final String pushCert; - RevisionNote(Change.Id changeId, ObjectReader reader, ObjectId noteId, - boolean draftsOnly) throws ConfigInvalidException, IOException { + 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); MutableInteger p = new MutableInteger(); trimLeadingEmptyLines(bytes, p); @@ -78,6 +79,6 @@ ? PatchLineComment.Status.DRAFT : PatchLineComment.Status.PUBLISHED; comments = ImmutableList.copyOf( - CommentsInNotesUtil.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 a804d2b..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(Change.Id changeId, ObjectReader reader, - NoteMap noteMap, boolean draftsOnly) - throws ConfigInvalidException, IOException { + 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(changeId, reader, note.getData(), draftsOnly); + RevisionNote rn = new RevisionNote( + 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/main/java/com/google/gerrit/server/schema/SchemaModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaModule.java index 3770b82..2d4b65f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaModule.java
@@ -19,12 +19,14 @@ import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdentProvider; +import com.google.gerrit.server.api.config.GerritServerIdProvider; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsNameProvider; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersNameProvider; import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.AnonymousCowardNameProvider; +import com.google.gerrit.server.config.GerritServerId; import org.eclipse.jgit.lib.PersonIdent; @@ -45,5 +47,9 @@ bind(String.class).annotatedWith(AnonymousCowardName.class).toProvider( AnonymousCowardNameProvider.class); + + bind(String.class).annotatedWith(GerritServerId.class) + .toProvider(GerritServerIdProvider.class) + .in(SINGLETON); } }
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 4d4c698..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
@@ -46,6 +46,7 @@ import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.DisableReverseDnsLookup; import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.config.GerritServerId; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitModule; import com.google.gerrit.server.git.GitRepositoryManager; @@ -106,6 +107,9 @@ @Inject protected AllUsersName allUsers; + @Inject + protected ChangeNoteUtil noteUtil; + private Injector injector; private String systemTimeZone; @@ -137,6 +141,8 @@ install(new GitModule()); factory(NoteDbUpdateManager.Factory.class); bind(AllUsersName.class).toProvider(AllUsersNameProvider.class); + bind(String.class).annotatedWith(GerritServerId.class) + .toInstance("gerrit"); bind(NotesMigration.class).toInstance(MIGRATION); bind(GitRepositoryManager.class).toInstance(repoManager); bind(ProjectCache.class).toProvider(Providers.<ProjectCache> of(null)); @@ -198,8 +204,8 @@ } protected ChangeNotes newNotes(Change c) throws OrmException { - return new ChangeNotes(repoManager, MIGRATION, allUsers, 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 acabfc7..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); + 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 32a9c9f..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,7 +910,7 @@ try (RevWalk rw = new RevWalk(repo)) { try (ChangeNotesParser notesWithComments = new ChangeNotesParser(project, - c.getId(), commitWithComments.copy(), rw, repoManager)) { + c.getId(), commitWithComments.copy(), rw, repoManager, noteUtil)) { notesWithComments.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals1 = notesWithComments.buildApprovals(); @@ -921,7 +921,8 @@ try (RevWalk rw = new RevWalk(repo)) { try (ChangeNotesParser notesWithApprovals = new ChangeNotesParser(project, - c.getId(), commitWithApprovals.copy(), rw, repoManager)) { + c.getId(), commitWithApprovals.copy(), rw, repoManager, + noteUtil)) { notesWithApprovals.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals2 = notesWithApprovals.buildApprovals(); @@ -1166,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" @@ -1182,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" @@ -1236,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/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java index 25722a5..3cdc914 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
@@ -39,6 +39,7 @@ import com.google.gerrit.server.config.CanonicalWebUrlProvider; import com.google.gerrit.server.config.GerritGlobalModule; import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.config.GerritServerId; import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFootersProvider; @@ -151,8 +152,11 @@ .annotatedWith(GerritPersonIdent.class) .toProvider(GerritPersonIdentProvider.class); bind(String.class) - .annotatedWith(AnonymousCowardName.class) - .toProvider(AnonymousCowardNameProvider.class); + .annotatedWith(AnonymousCowardName.class) + .toProvider(AnonymousCowardNameProvider.class); + bind(String.class) + .annotatedWith(GerritServerId.class) + .toInstance("gerrit"); bind(AllProjectsName.class) .toProvider(AllProjectsNameProvider.class); bind(AllUsersName.class)
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 88ab551..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
@@ -31,6 +31,7 @@ import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeDraftUpdate; +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.NotesMigration; @@ -90,16 +91,21 @@ GitRepositoryManager repoManager, NotesMigration migration, Change c, final AllUsersName allUsers, final CurrentUser user) throws Exception { - ChangeUpdate update = injector.createChildInjector(new FactoryModule() { + injector = injector.createChildInjector(new FactoryModule() { @Override public void configure() { factory(ChangeUpdate.Factory.class); factory(ChangeDraftUpdate.Factory.class); bind(CurrentUser.class).toInstance(user); } - }).getInstance(ChangeUpdate.Factory.class).create( - stubChangeControl(repoManager, migration, c, allUsers, user), - TimeUtil.nowTs(), Ordering.<String> natural()); + }); + ChangeUpdate update = injector.getInstance(ChangeUpdate.Factory.class) + .create( + stubChangeControl( + repoManager, migration, c, allUsers, + injector.getInstance(ChangeNoteUtil.class), + user), + TimeUtil.nowTs(), Ordering.<String> natural()); ChangeNotes notes = update.getChangeNotes(); boolean hasPatchSets = notes.getPatchSets() != null @@ -131,15 +137,15 @@ private static ChangeControl stubChangeControl( GitRepositoryManager repoManager, NotesMigration migration, - Change c, AllUsersName allUsers, + 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, 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);