Include a UUID portion in NoteDb author identities Author identities include per-server specific account IDs, so it is not safe to mix IDs from different servers. Ensure each server only ever produces identities with one ID during its lifetime, by writing out a random UUID to gerrit.config as gerrit.serverId. This happens during init, and optionally lazily during startup. For now NoteDb changes can be migrated between servers as long as this file is kept intact. Eventually, when federating changes between servers, we will need come up with some mechanism for coalescing various per-server identities into a single account, like the current AccountExternalId mapping (except not exactly that because Shawn regrets it). Such a mechanism will simply need to know how to handle this kind of UUID format. Change-Id: I9492c9c561892488703d15f7cde6094aa03f957b
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..f86fe5a 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 changeNoteUtil; protected final Date when; private final PersonIdent serverIdent; @@ -55,12 +56,14 @@ ChangeControl ctl, PersonIdent serverIdent, String anonymousCowardName, + ChangeNoteUtil changeNoteUtil, Date when) { this.migration = migration; this.repoManager = repoManager; this.ctl = ctl; this.serverIdent = serverIdent; this.anonymousCowardName = anonymousCowardName; + this.changeNoteUtil = changeNoteUtil; 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 changeNoteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent, anonymousCowardName); } else if (u instanceof InternalUser) { return serverIdent; @@ -105,7 +108,7 @@ } protected PersonIdent newIdent(Account author, Date when) { - return ChangeNoteUtil.newIdent(author, when, serverIdent, + return changeNoteUtil.newIdent(author, when, serverIdent, anonymousCowardName); }
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..59133a5 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
@@ -91,10 +91,12 @@ GitRepositoryManager repoManager, NotesMigration migration, AllUsersName allUsers, + ChangeNoteUtil changeNoteUtil, CommentsInNotesUtil commentsUtil, @Assisted ChangeControl ctl, @Assisted Date when) { - super(migration, repoManager, ctl, serverIdent, anonymousCowardName, when); + super(migration, repoManager, ctl, serverIdent, anonymousCowardName, + changeNoteUtil, when); this.draftsProject = allUsers; this.commentsUtil = commentsUtil; checkState(ctl.getUser().isIdentifiedUser(), @@ -195,7 +197,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); + commentsUtil, 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..7ab23a8 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,23 @@ package com.google.gerrit.server.notedb; +import static com.google.gerrit.server.notedb.ChangeNotes.parseException; + import com.google.common.annotations.VisibleForTesting; 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.RefNames; +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 java.util.Date; 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"); @@ -57,28 +60,36 @@ return r.toString(); } + private final String serverId; + + @Inject + ChangeNoteUtil(@GerritServerId String serverId) { + 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; - } - - private ChangeNoteUtil() { + throw parseException(changeId, "invalid identity, expected <id>@%s: %s", + serverId, email); } }
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..a8ea014 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,8 @@ private final AllUsersName allUsers; private final Provider<InternalChangeQuery> queryProvider; private final ProjectCache projectCache; + private final ChangeNoteUtil changeNoteUtil; + private final CommentsInNotesUtil commentsUtil; @VisibleForTesting @Inject @@ -144,12 +128,16 @@ NotesMigration migration, AllUsersName allUsers, Provider<InternalChangeQuery> queryProvider, - ProjectCache projectCache) { + ProjectCache projectCache, + ChangeNoteUtil changeNoteUtil, + CommentsInNotesUtil commentsUtil) { this.repoManager = repoManager; this.migration = migration; this.allUsers = allUsers; this.queryProvider = queryProvider; this.projectCache = projectCache; + this.changeNoteUtil = changeNoteUtil; + this.commentsUtil = commentsUtil; } public ChangeNotes createChecked(ReviewDb db, Change c) @@ -194,8 +182,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, changeNoteUtil, + commentsUtil, project, change).load(); } /** @@ -207,13 +195,13 @@ * @return change notes */ public ChangeNotes createFromIndexedChange(Change change) { - return new ChangeNotes(repoManager, migration, allUsers, - change.getProject(), change); + return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, + commentsUtil, change.getProject(), change); } public ChangeNotes createForNew(Change change) throws OrmException { - return new ChangeNotes(repoManager, migration, allUsers, - change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, + commentsUtil, change.getProject(), change).load(); } // TODO(dborowitz): Remove when deleting index schemas <27. @@ -222,8 +210,8 @@ checkState(!migration.readChanges(), "do not call" + " createFromIdOnlyWhenNotedbDisabled when notedb is enabled"); Change change = unwrap(db).changes().get(changeId); - return new ChangeNotes(repoManager, migration, allUsers, - change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, + commentsUtil, change.getProject(), change).load(); } // TODO(ekempin): Remove when database backend is deleted @@ -235,8 +223,8 @@ throws OrmException { checkState(!migration.readChanges(), "do not call" + " createFromChangeWhenNotedbDisabled when notedb is enabled"); - return new ChangeNotes(repoManager, migration, allUsers, - change.getProject(), change).load(); + return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil, + commentsUtil, change.getProject(), change).load(); } public CheckedFuture<ChangeNotes, OrmException> createAsync( @@ -255,8 +243,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, + changeNoteUtil, commentsUtil, project, change).load(); } }); } @@ -395,6 +383,8 @@ } } + private final ChangeNoteUtil changeNoteUtil; + private final CommentsInNotesUtil commentsUtil; private final Project.NameKey project; private final Change change; private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets; @@ -416,10 +406,13 @@ @VisibleForTesting public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName allUsers, Project.NameKey project, + AllUsersName allUsers, ChangeNoteUtil changeNoteUtil, + CommentsInNotesUtil commentsUtil, Project.NameKey project, Change change) { super(repoManager, migration, change != null ? change.getId() : null); this.allUsers = allUsers; + this.changeNoteUtil = changeNoteUtil; + this.commentsUtil = commentsUtil; this.project = project; this.change = change != null ? new Change(change) : null; } @@ -517,7 +510,7 @@ if (draftCommentNotes == null || !author.equals(draftCommentNotes.getAuthor())) { draftCommentNotes = new DraftCommentNotes(repoManager, migration, - allUsers, getChangeId(), author); + allUsers, commentsUtil, getChangeId(), author); draftCommentNotes.load(); } } @@ -564,7 +557,8 @@ } try (RevWalk walk = new RevWalk(reader); ChangeNotesParser parser = new ChangeNotesParser(project, - change.getId(), rev, walk, repoManager)) { + change.getId(), rev, walk, repoManager, changeNoteUtil, + commentsUtil)) { 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..cc0eb7a3 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,8 @@ PatchSet.Id currentPatchSetId; RevisionNoteMap revisionNoteMap; + private final ChangeNoteUtil changeNoteUtil; + private final CommentsInNotesUtil commentsUtil; private final Change.Id id; private final ObjectId tip; private final RevWalk walk; @@ -125,12 +126,15 @@ 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 changeNoteUtil, CommentsInNotesUtil commentsUtil) throws RepositoryNotFoundException, IOException { this.id = changeId; this.tip = tip; this.walk = walk; this.repo = repoManager.openMetadataRepository(project); + this.changeNoteUtil = changeNoteUtil; + this.commentsUtil = commentsUtil; approvals = Maps.newHashMap(); reviewers = Maps.newLinkedHashMap(); allPastReviewers = Lists.newArrayList(); @@ -501,7 +505,7 @@ ObjectReader reader = walk.getObjectReader(); RevCommit tipCommit = walk.parseCommit(tip); revisionNoteMap = RevisionNoteMap.parse( - id, reader, NoteMap.read(reader, tipCommit), false); + commentsUtil, id, reader, NoteMap.read(reader, tipCommit), false); Map<RevId, RevisionNote> rns = revisionNoteMap.revisionNotes; for (Map.Entry<RevId, RevisionNote> e : rns.entrySet()) { @@ -540,7 +544,7 @@ labelVoteStr = line.substring(0, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = parseIdent(ident); + accountId = changeNoteUtil.parseIdent(ident, id); } else { labelVoteStr = line; accountId = committerId; @@ -578,7 +582,7 @@ label = line.substring(1, s); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); - accountId = parseIdent(ident); + accountId = changeNoteUtil.parseIdent(ident, id); } else { label = line.substring(1); accountId = committerId; @@ -661,7 +665,7 @@ PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2)); checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line); - label.appliedBy = parseIdent(ident); + label.appliedBy = changeNoteUtil.parseIdent(ident, id); } else { label.label = line.substring(c + 2); } @@ -679,17 +683,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 changeNoteUtil.parseIdent(commit.getAuthorIdent(), id); } private void parseReviewer(ReviewerStateInternal state, String line) @@ -698,7 +692,7 @@ if (ident == null) { throw invalidFooter(state.getFooterKey(), line); } - Account.Id accountId = parseIdent(ident); + Account.Id accountId = changeNoteUtil.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..99ca9f3 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
@@ -136,10 +136,11 @@ ChangeDraftUpdate.Factory draftUpdateFactory, ProjectCache projectCache, @Assisted ChangeControl ctl, - CommentsInNotesUtil commentsUtil) { + CommentsInNotesUtil commentsUtil, + ChangeNoteUtil changeNoteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, - projectCache, ctl, serverIdent.getWhen(), commentsUtil); + projectCache, ctl, serverIdent.getWhen(), commentsUtil, changeNoteUtil); } @AssistedInject @@ -154,12 +155,13 @@ ProjectCache projectCache, @Assisted ChangeControl ctl, @Assisted Date when, - CommentsInNotesUtil commentsUtil) { + CommentsInNotesUtil commentsUtil, + ChangeNoteUtil changeNoteUtil) { this(serverIdent, anonymousCowardName, repoManager, migration, accountCache, updateManagerFactory, draftUpdateFactory, ctl, when, projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(), - commentsUtil); + commentsUtil, changeNoteUtil); } private static Project.NameKey getProjectName(ChangeControl ctl) { @@ -178,9 +180,10 @@ @Assisted ChangeControl ctl, @Assisted Date when, @Assisted Comparator<String> labelNameComparator, - CommentsInNotesUtil commentsUtil) { + CommentsInNotesUtil commentsUtil, + ChangeNoteUtil changeNoteUtil) { super(migration, repoManager, ctl, serverIdent, - anonymousCowardName, when); + anonymousCowardName, changeNoteUtil, when); this.accountCache = accountCache; this.commentsUtil = commentsUtil; this.draftUpdateFactory = draftUpdateFactory; @@ -401,7 +404,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); + commentsUtil, 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 index e660147..4eb028d 100644 --- 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
@@ -15,13 +15,11 @@ 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; @@ -52,7 +50,6 @@ import java.io.PrintWriter; import java.sql.Timestamp; import java.text.ParseException; -import java.util.Date; import java.util.List; import java.util.Locale; @@ -72,7 +69,30 @@ private static final String REVISION = "Revision"; private static final String UUID = "UUID"; - public static List<PatchLineComment> parseNote(byte[] note, MutableInteger p, + public static String formatTime(PersonIdent ident, Timestamp t) { + GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT); + // TODO(dborowitz): Use a ThreadLocal or use Joda. + PersonIdent newIdent = new PersonIdent(ident, t); + return dateFormatter.formatDate(newIdent); + } + + private final AccountCache accountCache; + private final PersonIdent serverIdent; + private final String anonymousCowardName; + private final ChangeNoteUtil changeNoteUtil; + + @Inject + public CommentsInNotesUtil(AccountCache accountCache, + @GerritPersonIdent PersonIdent serverIdent, + @AnonymousCowardName String anonymousCowardName, + ChangeNoteUtil changeNoteUtil) { + this.accountCache = accountCache; + this.serverIdent = serverIdent; + this.anonymousCowardName = anonymousCowardName; + this.changeNoteUtil = changeNoteUtil; + } + + public List<PatchLineComment> parseNote(byte[] note, MutableInteger p, Change.Id changeId, Status status) throws ConfigInvalidException { if (p.value >= note.length) { return ImmutableList.of(); @@ -99,14 +119,7 @@ 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, + 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(); @@ -273,14 +286,14 @@ return checkResult(commentTime, "comment timestamp", changeId); } - private static Account.Id parseAuthor(byte[] note, MutableInteger curr, + 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); + Account.Id aId = changeNoteUtil.parseIdent(ident, changeId); curr.value = RawParseUtils.nextLF(note, curr.value); return checkResult(aId, "comment author", changeId); } @@ -317,28 +330,6 @@ 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); @@ -360,19 +351,6 @@ } } - 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); @@ -448,8 +426,8 @@ writer.print("\n"); PersonIdent ident = - newIdent(accountCache.get(c.getAuthor()).getAccount(), - c.getWrittenOn()); + changeNoteUtil.newIdent(accountCache.get(c.getAuthor()).getAccount(), + c.getWrittenOn(), serverIdent, anonymousCowardName); String nameString = ident.getName() + " <" + ident.getEmailAddress() + ">"; appendHeaderField(writer, AUTHOR, nameString);
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..f03caab 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 CommentsInNotesUtil commentsUtil; @VisibleForTesting @Inject public Factory(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersName allUsers) { + AllUsersName allUsers, + CommentsInNotesUtil commentsUtil) { this.repoManager = repoManager; this.migration = migration; this.draftsProject = allUsers; + this.commentsUtil = commentsUtil; } public DraftCommentNotes create(Change.Id changeId, Account.Id accountId) { return new DraftCommentNotes(repoManager, migration, draftsProject, - changeId, accountId); + commentsUtil, changeId, accountId); } } private final AllUsersName draftsProject; + private final CommentsInNotesUtil commentsUtil; 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, CommentsInNotesUtil commentsUtil, + Change.Id changeId, Account.Id author) { super(repoManager, migration, changeId); this.draftsProject = draftsProject; this.author = author; + this.commentsUtil = commentsUtil; } 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); + commentsUtil, 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..586beaf 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(CommentsInNotesUtil commentsUtil, 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)); + commentsUtil.parseNote(bytes, p, changeId, status)); } }
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..0c2420a 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(CommentsInNotesUtil commentsUtil, + 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( + commentsUtil, 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..d01ae0b 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,12 @@ @Inject protected AllUsersName allUsers; + @Inject + protected ChangeNoteUtil changeNoteUtil; + + @Inject + protected CommentsInNotesUtil commentsUtil; + private Injector injector; private String systemTimeZone; @@ -137,6 +144,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 +207,8 @@ } protected ChangeNotes newNotes(Change c) throws OrmException { - return new ChangeNotes(repoManager, MIGRATION, allUsers, c.getProject(), c) - .load(); + return new ChangeNotes(repoManager, MIGRATION, allUsers, changeNoteUtil, + commentsUtil, 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..fe3c2eb 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, changeNoteUtil.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, changeNoteUtil, commentsUtil); } }
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..ba7cd37 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
@@ -910,7 +910,8 @@ try (RevWalk rw = new RevWalk(repo)) { try (ChangeNotesParser notesWithComments = new ChangeNotesParser(project, - c.getId(), commitWithComments.copy(), rw, repoManager)) { + c.getId(), commitWithComments.copy(), rw, repoManager, changeNoteUtil, + commentsUtil)) { notesWithComments.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals1 = notesWithComments.buildApprovals(); @@ -921,7 +922,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, + changeNoteUtil, commentsUtil)) { notesWithApprovals.parseAll(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals2 = notesWithApprovals.buildApprovals();
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..cfa63dc 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,8 +31,10 @@ 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.CommentsInNotesUtil; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.project.ChangeControl; import com.google.gwtorm.server.OrmException; @@ -90,16 +92,22 @@ 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), + injector.getInstance(CommentsInNotesUtil.class), + user), + TimeUtil.nowTs(), Ordering.<String> natural()); ChangeNotes notes = update.getChangeNotes(); boolean hasPatchSets = notes.getPatchSets() != null @@ -131,15 +139,15 @@ private static ChangeControl stubChangeControl( GitRepositoryManager repoManager, NotesMigration migration, - Change c, AllUsersName allUsers, - CurrentUser user) throws OrmException { + Change c, AllUsersName allUsers, ChangeNoteUtil changeNoteUtil, + CommentsInNotesUtil commentsUtil, 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, changeNoteUtil, + commentsUtil, c.getProject(), c).load(); expect(ctl.getNotes()).andStubReturn(notes); expect(ctl.getId()).andStubReturn(c.getId()); EasyMock.replay(ctl);