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

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -62,6 +62,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 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 @@ public abstract class AbstractDaemonTest {
   @Inject
   protected FakeEmailSender sender;
 
+  @Inject
+  protected ChangeNoteUtil changeNoteUtil;
+
   protected TestRepository<InMemoryRepository> testRepo;
   protected GerritServer server;
   protected TestAccount admin;

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java

@@ -43,6 +43,7 @@ import com.google.gwtorm.server.SchemaFactory;
 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.common.keyprovider.KeyPairProvider;
 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 @@ class InMemoryTestingDatabaseModule extends LifecycleModule {
       .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 @@ class InMemoryTestingDatabaseModule extends LifecycleModule {
       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;
+    }
+  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -1082,7 +1082,7 @@ public class ChangeIT extends AbstractDaemonTest {
 
       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 @@ public class ChangeIT extends AbstractDaemonTest {
           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())

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -127,7 +127,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
 
       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);

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java

@@ -21,6 +21,7 @@ import com.google.common.collect.Sets;
 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 @@ class InitDatabase implements InitStep {
   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 @@ class InitDatabase implements InitStep {
     this.site = site;
     this.libraries = libraries;
     this.database = sections.get("database", null);
+    this.idSection = sections.get(GerritServerIdProvider.SECTION, null);
   }
 
   @Override
@@ -91,6 +94,13 @@ class InitDatabase implements InitStep {
     }
 
     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

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;
+  }
+}

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 {
+}

gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java

@@ -44,6 +44,7 @@ public abstract class AbstractChangeUpdate {
   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 @@ public abstract class AbstractChangeUpdate {
       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 @@ public abstract class AbstractChangeUpdate {
   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 @@ public abstract class AbstractChangeUpdate {
   }
 
   protected PersonIdent newIdent(Account author, Date when) {
-    return ChangeNoteUtil.newIdent(author, when, serverIdent,
+    return changeNoteUtil.newIdent(author, when, serverIdent,
         anonymousCowardName);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java

@@ -91,10 +91,12 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
       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 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
     // 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

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 @@ public class ChangeNoteUtil {
     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 (host.equals(serverId)) {
-      if (id != null && host.equals(GERRIT_PLACEHOLDER_HOST)) {
+        Integer id = Ints.tryParse(email.substring(0, at));
-        return new Account.Id(id);
+        if (id != null) {
+          return new Account.Id(id);
+        }
       }
     }
-    return null;
+    throw parseException(changeId, "invalid identity, expected <id>@%s: %s",
-  }
+        serverId, email);
-
-  private ChangeNoteUtil() {
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java

@@ -17,7 +17,6 @@ package com.google.gerrit.server.notedb;
 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.ListMultimap;
 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 com.google.inject.Singleton;
 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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         + 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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
           project, changeId, change.getProject());
       // TODO: Throw NoSuchChangeException when the change is not found in the
       // database
-      return new ChangeNotes(repoManager, migration, allUsers, project,
+      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
-          change).load();
+          commentsUtil, project, change).load();
     }
 
     /**
@@ -207,13 +195,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
      * @return change notes
      */
     public ChangeNotes createFromIndexedChange(Change change) {
-      return new ChangeNotes(repoManager, migration, allUsers,
+      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
-          change.getProject(), change);
+          commentsUtil, change.getProject(), change);
     }
 
     public ChangeNotes createForNew(Change change) throws OrmException {
-      return new ChangeNotes(repoManager, migration, allUsers,
+      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
-          change.getProject(), change).load();
+          commentsUtil, change.getProject(), change).load();
     }
 
     // TODO(dborowitz): Remove when deleting index schemas <27.
@@ -222,8 +210,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     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, changeNoteUtil,
-          change.getProject(), change).load();
+          commentsUtil, change.getProject(), change).load();
     }
 
     // TODO(ekempin): Remove when database backend is deleted
@@ -235,8 +223,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         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, changeNoteUtil,
-          change.getProject(), change).load();
+          commentsUtil, change.getProject(), change).load();
     }
 
     public CheckedFuture<ChangeNotes, OrmException> createAsync(
@@ -255,8 +243,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
                           "passed project %s when creating ChangeNotes for %s,"
                               + " but actual project is %s",
                           project, changeId, change.getProject());
-                      return new ChangeNotes(repoManager, migration,
+                      return new ChangeNotes(repoManager, migration, allUsers,
-                          allUsers, project, change).load();
+                          changeNoteUtil, commentsUtil, project, change).load();
                     }
                   });
                 }
@@ -395,6 +383,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
   }
 
+  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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
     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) {

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_SUBJECT;
 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 @@ class ChangeNotesParser implements AutoCloseable {
   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 @@ class ChangeNotesParser implements AutoCloseable {
   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 @@ class ChangeNotesParser implements AutoCloseable {
     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 @@ class ChangeNotesParser implements AutoCloseable {
       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 @@ class ChangeNotesParser implements AutoCloseable {
       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 @@ class ChangeNotesParser implements AutoCloseable {
           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 @@ class ChangeNotesParser implements AutoCloseable {
         && a.getEmailAddress().equals(c.getEmailAddress())) {
       return null;
     }
-    return parseIdent(commit.getAuthorIdent());
+    return changeNoteUtil.parseIdent(commit.getAuthorIdent(), id);
-  }
-
-  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;
   }
 
   private void parseReviewer(ReviewerStateInternal state, String line)
@@ -698,7 +692,7 @@ class ChangeNotesParser implements AutoCloseable {
     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);
     }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilder.java

@@ -56,6 +56,7 @@ import com.google.gwtorm.server.SchemaFactory;
 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 @@ public class ChangeRebuilder {
   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 @@ public class ChangeRebuilder {
       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 @@ public class ChangeRebuilder {
     this.updateFactory = updateFactory;
     this.draftUpdateFactory = draftUpdateFactory;
     this.updateManagerFactory = updateManagerFactory;
+    this.changeNoteUtil = changeNoteUtil;
   }
 
   public ListenableFuture<?> rebuildAsync(final Change.Id id,
@@ -125,7 +129,8 @@ public class ChangeRebuilder {
   }
 
   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 @@ public class ChangeRebuilder {
   }
 
   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 @@ public class ChangeRebuilder {
     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) {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java

@@ -136,10 +136,11 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       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 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       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 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       @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 @@ public class ChangeUpdate extends AbstractChangeUpdate {
     // 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,

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.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;
 
@@ -72,7 +69,30 @@ public class CommentsInNotesUtil {
   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 @@ public class CommentsInNotesUtil {
     return result;
   }
 
-  public static String formatTime(PersonIdent ident, Timestamp t) {
+  private PatchLineComment parseComment(byte[] note, MutableInteger curr,
-    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();
@@ -273,14 +286,14 @@ public class CommentsInNotesUtil {
     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 @@ public class CommentsInNotesUtil {
     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 @@ public class CommentsInNotesUtil {
     }
   }
 
-  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 @@ public class CommentsInNotesUtil {
         writer.print("\n");
 
         PersonIdent ident =
-            newIdent(accountCache.get(c.getAuthor()).getAccount(),
+            changeNoteUtil.newIdent(accountCache.get(c.getAuthor()).getAccount(),
-                c.getWrittenOn());
+                c.getWrittenOn(), serverIdent, anonymousCowardName);
         String nameString = ident.getName() + " <" + ident.getEmailAddress()
             + ">";
         appendHeaderField(writer, AUTHOR, nameString);

gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java

@@ -48,34 +48,40 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     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 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     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) {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java

@@ -63,8 +63,9 @@ class RevisionNote {
   final ImmutableList<PatchLineComment> comments;
   final String pushCert;
 
-  RevisionNote(Change.Id changeId, ObjectReader reader, ObjectId noteId,
+  RevisionNote(CommentsInNotesUtil commentsUtil, Change.Id changeId,
-      boolean draftsOnly) throws ConfigInvalidException, IOException {
+      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 @@ class RevisionNote {
         ? PatchLineComment.Status.DRAFT
         : PatchLineComment.Status.PUBLISHED;
     comments = ImmutableList.copyOf(
-        CommentsInNotesUtil.parseNote(bytes, p, changeId, status));
+        commentsUtil.parseNote(bytes, p, changeId, status));
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNoteMap.java

@@ -31,13 +31,13 @@ class RevisionNoteMap {
   final NoteMap noteMap;
   final ImmutableMap<RevId, RevisionNote> revisionNotes;
 
-  static RevisionNoteMap parse(Change.Id changeId, ObjectReader reader,
+  static RevisionNoteMap parse(CommentsInNotesUtil commentsUtil,
-      NoteMap noteMap, boolean draftsOnly)
+      Change.Id changeId, ObjectReader reader, NoteMap noteMap,
-      throws ConfigInvalidException, IOException {
+      boolean draftsOnly) throws ConfigInvalidException, IOException {
     Map<RevId, RevisionNote> result = new HashMap<>();
     for (Note note : noteMap) {
-      RevisionNote rn =
+      RevisionNote rn = new RevisionNote(
-          new RevisionNote(changeId, reader, note.getData(), draftsOnly);
+          commentsUtil, changeId, reader, note.getData(), draftsOnly);
       result.put(new RevId(note.name()), rn);
     }
     return new RevisionNoteMap(noteMap, ImmutableMap.copyOf(result));

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaModule.java

@@ -19,12 +19,14 @@ import static com.google.inject.Scopes.SINGLETON;
 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 @@ public class SchemaModule extends FactoryModule {
 
     bind(String.class).annotatedWith(AnonymousCowardName.class).toProvider(
         AnonymousCowardNameProvider.class);
+
+    bind(String.class).annotatedWith(GerritServerId.class)
+      .toProvider(GerritServerIdProvider.class)
+      .in(SINGLETON);
   }
 }

gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -46,6 +46,7 @@ import com.google.gerrit.server.config.AnonymousCowardNameProvider;
 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 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
   @Inject
   protected AllUsersName allUsers;
 
+  @Inject
+  protected ChangeNoteUtil changeNoteUtil;
+
+  @Inject
+  protected CommentsInNotesUtil commentsUtil;
+
   private Injector injector;
   private String systemTimeZone;
 
@@ -137,6 +144,8 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
         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 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
   }
 
   protected ChangeNotes newNotes(Change c) throws OrmException {
-    return new ChangeNotes(repoManager, MIGRATION, allUsers, c.getProject(), c)
+    return new ChangeNotes(repoManager, MIGRATION, allUsers, changeNoteUtil,
-        .load();
+        commentsUtil, c.getProject(), c).load();
   }
 
   protected static SubmitRecord submitRecord(String status,

gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java

@@ -415,7 +415,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   }
 
   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 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   private ChangeNotesParser newParser(ObjectId tip) throws Exception {
     Change c = newChange();
     return new ChangeNotesParser(c.getProject(), c.getId(), tip, walk,
-        repoManager);
+        repoManager, changeNoteUtil, commentsUtil);
   }
 }

gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java

@@ -910,7 +910,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
 
     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 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
 
     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();

gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java

@@ -39,6 +39,7 @@ import com.google.gerrit.server.config.CanonicalWebUrlModule;
 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 @@ public class InMemoryModule extends FactoryModule {
         .annotatedWith(GerritPersonIdent.class)
         .toProvider(GerritPersonIdentProvider.class);
     bind(String.class)
-      .annotatedWith(AnonymousCowardName.class)
+        .annotatedWith(AnonymousCowardName.class)
-      .toProvider(AnonymousCowardNameProvider.class);
+        .toProvider(AnonymousCowardNameProvider.class);
+    bind(String.class)
+        .annotatedWith(GerritServerId.class)
+        .toInstance("gerrit");
     bind(AllProjectsName.class)
         .toProvider(AllProjectsNameProvider.class);
     bind(AllUsersName.class)

gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java

@@ -31,8 +31,10 @@ import com.google.gerrit.server.CurrentUser;
 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 @@ public class TestChanges {
       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),
+    ChangeUpdate update = injector.getInstance(ChangeUpdate.Factory.class)
-        TimeUtil.nowTs(), Ordering.<String> natural());
+        .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 @@ public class TestChanges {
 
   private static ChangeControl stubChangeControl(
       GitRepositoryManager repoManager, NotesMigration migration,
-      Change c, AllUsersName allUsers,
+      Change c, AllUsersName allUsers, ChangeNoteUtil changeNoteUtil,
-      CurrentUser user) throws OrmException {
+      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)
+        new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
-            .load();
+            commentsUtil, c.getProject(), c).load();
     expect(ctl.getNotes()).andStubReturn(notes);
     expect(ctl.getId()).andStubReturn(c.getId());
     EasyMock.replay(ctl);