Store information about starring changes in notedb

To keep track of a change that is starred by a user we create a
refs/starred-changes/YY/XXXX/ZZZZ ref in the All-Users notedb
repository, where YY/XXXX is the sharded account ID and ZZZZ is the
numeric change ID. The refs/starred-changes/* refs point to empty tree
objects since the existence of a ref is enough to know that a change
was starred by a user. The ref format is similar to the
refs/draft-comments/* refs that store draft comments in the All-Users
notedb repository, but it uses '/' instead of '-' to separate the
sharded account ID from the change ID. This is because we need to find
all refs that start with the prefix refs/starred-changes/YY/XXXX/ and
RefDirectory in jgit has explicit optimizations for the case when the
prefix ends with '/' and we want to take advantage of this.

Change-Id: I033b35a05749929c9c0ea778075c4a1085c5bf2a
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java

@@ -117,11 +117,12 @@ public class RebuildNotedb extends SiteProgram {
         sysInjector.getInstance(AllUsersName.class);
     try (Repository allUsersRepo =
         repoManager.openMetadataRepository(allUsersName)) {
-      deleteDraftRefs(allUsersRepo);
+      deleteRefs(RefNames.REFS_DRAFT_COMMENTS, allUsersRepo);
+      deleteRefs(RefNames.REFS_STARRED_CHANGES, allUsersRepo);
       for (final Project.NameKey project : changesByProject.keySet()) {
         try (Repository repo = repoManager.openMetadataRepository(project)) {
           final BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
-          final BatchRefUpdate bruForDrafts =
+          final BatchRefUpdate bruAllUsers =
               allUsersRepo.getRefDatabase().newBatchUpdate();
           List<ListenableFuture<?>> futures = Lists.newArrayList();
 
@@ -136,7 +137,7 @@ public class RebuildNotedb extends SiteProgram {
 
           for (final Change c : changesByProject.get(project)) {
             final ListenableFuture<?> future = rebuilder.rebuildAsync(c,
-                executor, bru, bruForDrafts, repo, allUsersRepo);
+                executor, bru, bruAllUsers, repo, allUsersRepo);
             futures.add(future);
             future.addListener(
                 new RebuildListener(c.getId(), future, ok, doneTask, failedTask),
@@ -149,7 +150,7 @@ public class RebuildNotedb extends SiteProgram {
                 public ListenableFuture<Void> apply(List<?> input)
                     throws Exception {
                   execute(bru, repo);
-                  execute(bruForDrafts, allUsersRepo);
+                  execute(bruAllUsers, allUsersRepo);
                   mpm.end();
                   return Futures.immediateFuture(null);
                 }
@@ -173,15 +174,22 @@ public class RebuildNotedb extends SiteProgram {
     try (RevWalk rw = new RevWalk(repo)) {
       bru.execute(rw, NullProgressMonitor.INSTANCE);
     }
+    for (ReceiveCommand command : bru.getCommands()) {
+      if (command.getResult() != ReceiveCommand.Result.OK) {
+        throw new IOException(String.format("Command %s failed: %s",
+            command.toString(), command.getResult()));
+      }
+    }
   }
 
-  private void deleteDraftRefs(Repository allUsersRepo) throws IOException {
+  private void deleteRefs(String prefix, Repository allUsersRepo)
+      throws IOException {
     RefDatabase refDb = allUsersRepo.getRefDatabase();
-    Map<String, Ref> allRefs = refDb.getRefs(RefNames.REFS_DRAFT_COMMENTS);
+    Map<String, Ref> allRefs = refDb.getRefs(prefix);
     BatchRefUpdate bru = refDb.newBatchUpdate();
     for (Map.Entry<String, Ref> ref : allRefs.entrySet()) {
       bru.addCommand(new ReceiveCommand(ref.getValue().getObjectId(),
-          ObjectId.zeroId(), RefNames.REFS_DRAFT_COMMENTS + ref.getKey()));
+          ObjectId.zeroId(), prefix + ref.getKey()));
     }
     execute(bru, allUsersRepo);
   }

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java

@@ -43,6 +43,9 @@ public class RefNames {
   /** Draft inline comments of a user on a change */
   public static final String REFS_DRAFT_COMMENTS = "refs/draft-comments/";
 
+  /** A change starred by a user */
+  public static final String REFS_STARRED_CHANGES = "refs/starred-changes/";
+
   /**
    * Prefix applied to merge commit base nodes.
    * <p>
@@ -113,6 +116,32 @@ public class RefNames {
     return r;
   }
 
+  public static String refsStarredChanges(Account.Id accountId,
+      Change.Id changeId) {
+    StringBuilder r = buildRefsPrefix(REFS_STARRED_CHANGES, accountId);
+    r.append(changeId.get());
+    return r.toString();
+  }
+
+  public static String refsStarredChangesPrefix(Account.Id accountId) {
+    return buildRefsPrefix(REFS_STARRED_CHANGES, accountId).toString();
+  }
+
+  private static StringBuilder buildRefsPrefix(String prefix,
+      Account.Id accountId) {
+    StringBuilder r = new StringBuilder();
+    r.append(prefix);
+    int n = accountId.get() % 100;
+    if (n < 10) {
+      r.append('0');
+    }
+    r.append(n);
+    r.append('/');
+    r.append(accountId.get());
+    r.append('/');
+    return r;
+  }
+
   /**
    * Returns reference for this change edit with sharded user and change number:
    * refs/users/UU/UUUU/edit-CCCC/P.

gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java

@@ -48,6 +48,18 @@ public class RefNamesTest {
       .isEqualTo("refs/draft-comments/23/1011123-");
   }
 
+  @Test
+  public void refsStarredChanges() throws Exception {
+    assertThat(RefNames.refsStarredChanges(accountId, changeId))
+      .isEqualTo("refs/starred-changes/23/1011123/67473");
+  }
+
+  @Test
+  public void refsStarredChangesPrefix() throws Exception {
+    assertThat(RefNames.refsStarredChangesPrefix(accountId))
+      .isEqualTo("refs/starred-changes/23/1011123/");
+  }
+
   @Test
   public void refsEdit() throws Exception {
     assertThat(RefNames.refsEdit(accountId, changeId, psId))

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -189,6 +189,7 @@ public class ChangeUtil {
   private final BatchUpdate.Factory updateFactory;
   private final ChangeMessagesUtil changeMessagesUtil;
   private final ChangeUpdate.Factory changeUpdateFactory;
+  private final StarredChangesUtil starredChangesUtil;
 
   @Inject
   ChangeUtil(Provider<IdentifiedUser> user,
@@ -202,7 +203,8 @@ public class ChangeUtil {
       ChangeIndexer indexer,
       BatchUpdate.Factory updateFactory,
       ChangeMessagesUtil changeMessagesUtil,
-      ChangeUpdate.Factory changeUpdateFactory) {
+      ChangeUpdate.Factory changeUpdateFactory,
+      StarredChangesUtil starredChangesUtil) {
     this.user = user;
     this.db = db;
     this.queryProvider = queryProvider;
@@ -215,6 +217,7 @@ public class ChangeUtil {
     this.updateFactory = updateFactory;
     this.changeMessagesUtil = changeMessagesUtil;
     this.changeUpdateFactory = changeUpdateFactory;
+    this.starredChangesUtil = starredChangesUtil;
   }
 
   public Change.Id revert(ChangeControl ctl, PatchSet.Id patchSetId,
@@ -366,7 +369,7 @@ public class ChangeUtil {
       db.patchSetApprovals().delete(db.patchSetApprovals().byChange(changeId));
       db.patchSets().delete(patchSets);
       db.changeMessages().delete(db.changeMessages().byChange(changeId));
-      db.starredChanges().delete(db.starredChanges().byChange(changeId));
+      starredChangesUtil.unstarAll(changeId);
       db.changes().delete(Collections.singleton(change));
 
       // Delete all refs at once.

gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server;
 
-import com.google.common.base.Function;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -23,7 +22,6 @@ import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountProjectWatch;
 import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.StarredChange;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountState;
@@ -68,6 +66,7 @@ public class IdentifiedUser extends CurrentUser {
   @Singleton
   public static class GenericFactory {
     private final CapabilityControl.Factory capabilityControlFactory;
+    private final StarredChangesUtil starredChangesUtil;
     private final AuthConfig authConfig;
     private final Realm realm;
     private final String anonymousCowardName;
@@ -79,6 +78,7 @@ public class IdentifiedUser extends CurrentUser {
     @Inject
     public GenericFactory(
         @Nullable CapabilityControl.Factory capabilityControlFactory,
+        @Nullable StarredChangesUtil starredChangesUtil,
         AuthConfig authConfig,
         Realm realm,
         @AnonymousCowardName String anonymousCowardName,
@@ -87,6 +87,7 @@ public class IdentifiedUser extends CurrentUser {
         AccountCache accountCache,
         GroupBackend groupBackend) {
       this.capabilityControlFactory = capabilityControlFactory;
+      this.starredChangesUtil = starredChangesUtil;
       this.authConfig = authConfig;
       this.realm = realm;
       this.anonymousCowardName = anonymousCowardName;
@@ -101,23 +102,23 @@ public class IdentifiedUser extends CurrentUser {
     }
 
     public IdentifiedUser create(Provider<ReviewDb> db, Account.Id id) {
-      return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+      return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
-          anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+          authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
-          disableReverseDnsLookup, null, db, id, null);
+          groupBackend, disableReverseDnsLookup, null, db, id, null);
     }
 
     public IdentifiedUser create(SocketAddress remotePeer, Account.Id id) {
-      return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+      return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
-          anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+          authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
-          disableReverseDnsLookup, Providers.of(remotePeer), null,
+          groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), null,
           id, null);
     }
 
     public CurrentUser runAs(SocketAddress remotePeer, Account.Id id,
         @Nullable CurrentUser caller) {
-      return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+      return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
-          anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+          authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
-          disableReverseDnsLookup, Providers.of(remotePeer), null,
+          groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), null,
           id, caller);
     }
   }
@@ -131,6 +132,7 @@ public class IdentifiedUser extends CurrentUser {
   @Singleton
   public static class RequestFactory {
     private final CapabilityControl.Factory capabilityControlFactory;
+    private final StarredChangesUtil starredChangesUtil;
     private final AuthConfig authConfig;
     private final Realm realm;
     private final String anonymousCowardName;
@@ -145,6 +147,7 @@ public class IdentifiedUser extends CurrentUser {
     @Inject
     RequestFactory(
         CapabilityControl.Factory capabilityControlFactory,
+        StarredChangesUtil starredChangesUtil,
         final AuthConfig authConfig,
         Realm realm,
         @AnonymousCowardName final String anonymousCowardName,
@@ -155,6 +158,7 @@ public class IdentifiedUser extends CurrentUser {
         @RemotePeer final Provider<SocketAddress> remotePeerProvider,
         final Provider<ReviewDb> dbProvider) {
       this.capabilityControlFactory = capabilityControlFactory;
+      this.starredChangesUtil = starredChangesUtil;
       this.authConfig = authConfig;
       this.realm = realm;
       this.anonymousCowardName = anonymousCowardName;
@@ -167,16 +171,16 @@ public class IdentifiedUser extends CurrentUser {
     }
 
     public IdentifiedUser create(Account.Id id) {
-      return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+      return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
-          anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+          authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
-          disableReverseDnsLookup, remotePeerProvider, dbProvider,
+          groupBackend, disableReverseDnsLookup, remotePeerProvider, dbProvider,
           id, null);
     }
 
     public IdentifiedUser runAs(Account.Id id, CurrentUser caller) {
-      return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+      return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
-          anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+          authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
-          disableReverseDnsLookup, remotePeerProvider, dbProvider,
+          groupBackend, disableReverseDnsLookup, remotePeerProvider, dbProvider,
           id, caller);
     }
   }
@@ -189,6 +193,9 @@ public class IdentifiedUser extends CurrentUser {
           SystemGroupBackend.ANONYMOUS_USERS,
           SystemGroupBackend.REGISTERED_USERS));
 
+  @Nullable
+  private final StarredChangesUtil starredChangesUtil;
+
   private final Provider<String> canonicalUrl;
   private final AccountCache accountCache;
   private final AuthConfig authConfig;
@@ -212,12 +219,13 @@ public class IdentifiedUser extends CurrentUser {
   private Set<String> invalidEmails;
   private GroupMembership effectiveGroups;
   private Set<Change.Id> starredChanges;
-  private ResultSet<StarredChange> starredQuery;
+  private ResultSet<Change.Id> starredQuery;
   private Collection<AccountProjectWatch> notificationFilters;
   private CurrentUser realUser;
 
   private IdentifiedUser(
       CapabilityControl.Factory capabilityControlFactory,
+      @Nullable StarredChangesUtil starredChangesUtil,
       final AuthConfig authConfig,
       Realm realm,
       final String anonymousCowardName,
@@ -230,6 +238,7 @@ public class IdentifiedUser extends CurrentUser {
       final Account.Id id,
       @Nullable CurrentUser realUser) {
     super(capabilityControlFactory);
+    this.starredChangesUtil = starredChangesUtil;
     this.canonicalUrl = canonicalUrl;
     this.accountCache = accountCache;
     this.groupBackend = groupBackend;
@@ -322,13 +331,16 @@ public class IdentifiedUser extends CurrentUser {
   @Override
   public Set<Change.Id> getStarredChanges() {
     if (starredChanges == null) {
-      checkRequestScope();
+      if (starredChangesUtil == null) {
+        throw new IllegalStateException("StarredChangesUtil is missing");
+      }
       try {
-        starredChanges = starredChangeIds(
+        starredChanges =
-            starredQuery != null ? starredQuery : starredQuery());
+            FluentIterable.from(
-      } catch (OrmException | RuntimeException e) {
+              starredQuery != null
-        log.warn("Cannot query starred changes", e);
+              ? starredQuery
-        starredChanges = Collections.emptySet();
+              : starredChangesUtil.query(accountId))
+            .toSet();
       } finally {
         starredQuery = null;
       }
@@ -344,14 +356,8 @@ public class IdentifiedUser extends CurrentUser {
   }
 
   public void asyncStarredChanges() {
-    if (starredChanges == null && dbProvider != null) {
+    if (starredChanges == null && starredChangesUtil != null) {
-      try {
+      starredQuery = starredChangesUtil.query(accountId);
-        starredQuery = starredQuery();
-      } catch (OrmException e) {
-        log.warn("Cannot query starred by user changes", e);
-        starredQuery = null;
-        starredChanges = Collections.emptySet();
-      }
     }
   }
 
@@ -371,21 +377,6 @@ public class IdentifiedUser extends CurrentUser {
     }
   }
 
-  private ResultSet<StarredChange> starredQuery() throws OrmException {
-    return dbProvider.get().starredChanges().byAccount(getAccountId());
-  }
-
-  private static ImmutableSet<Change.Id> starredChangeIds(
-      Iterable<StarredChange> scs) {
-    return FluentIterable.from(scs)
-        .transform(new Function<StarredChange, Change.Id>() {
-          @Override
-          public Change.Id apply(StarredChange in) {
-            return in.getChangeId();
-          }
-        }).toSet();
-  }
-
   @Override
   public Collection<AccountProjectWatch> getNotificationFilters() {
     if (notificationFilters == null) {

gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java

@@ -0,0 +1,277 @@
+// Copyright (C) 2015 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;
+
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.StarredChange;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gwtorm.server.ListResultSet;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.ResultSet;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+@Singleton
+public class StarredChangesUtil {
+  private static final Logger log =
+      LoggerFactory.getLogger(StarredChangesUtil.class);
+
+  private final GitRepositoryManager repoManager;
+  private final AllUsersName allUsers;
+  private final NotesMigration migration;
+  private final Provider<ReviewDb> dbProvider;
+  private final PersonIdent serverIdent;
+
+  @Inject
+  StarredChangesUtil(GitRepositoryManager repoManager,
+      AllUsersName allUsers,
+      NotesMigration migration,
+      Provider<ReviewDb> dbProvider,
+      @GerritPersonIdent PersonIdent serverIdent) {
+    this.repoManager = repoManager;
+    this.allUsers = allUsers;
+    this.migration = migration;
+    this.dbProvider = dbProvider;
+    this.serverIdent = serverIdent;
+  }
+
+  public void star(Account.Id accountId, Change.Id changeId)
+      throws OrmException {
+    dbProvider.get().starredChanges()
+        .insert(Collections.singleton(new StarredChange(
+            new StarredChange.Key(accountId, changeId))));
+    if (!migration.writeChanges()) {
+      return;
+    }
+    try (Repository repo = repoManager.openMetadataRepository(allUsers);
+        RevWalk rw = new RevWalk(repo)) {
+      RefUpdate u = repo.updateRef(
+          RefNames.refsStarredChanges(accountId, changeId));
+      u.setExpectedOldObjectId(ObjectId.zeroId());
+      u.setNewObjectId(emptyTree(repo));
+      u.setRefLogIdent(serverIdent);
+      u.setRefLogMessage("Star change " + changeId.get(), false);
+      RefUpdate.Result result = u.update(rw);
+      switch (result) {
+        case NEW:
+          return;
+        default:
+          throw new OrmException(
+              String.format("Star change %d for account %d failed: %s",
+                  changeId.get(), accountId.get(), result.name()));
+      }
+    } catch (IOException e) {
+      throw new OrmException(
+          String.format("Star change %d for account %d failed",
+              changeId.get(), accountId.get()), e);
+    }
+  }
+
+  private static ObjectId emptyTree(Repository repo) throws IOException {
+    try (ObjectInserter oi = repo.newObjectInserter()) {
+      ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {});
+      oi.flush();
+      return id;
+    }
+  }
+
+  public void unstar(Account.Id accountId, Change.Id changeId)
+      throws OrmException {
+    dbProvider.get().starredChanges()
+        .delete(Collections.singleton(new StarredChange(
+            new StarredChange.Key(accountId, changeId))));
+    if (!migration.writeChanges()) {
+      return;
+    }
+    try (Repository repo = repoManager.openMetadataRepository(allUsers);
+        RevWalk rw = new RevWalk(repo)) {
+      RefUpdate u = repo.updateRef(
+          RefNames.refsStarredChanges(accountId, changeId));
+      u.setForceUpdate(true);
+      u.setRefLogIdent(serverIdent);
+      u.setRefLogMessage("Unstar change " + changeId.get(), true);
+      RefUpdate.Result result = u.delete();
+      switch (result) {
+        case FORCED:
+          return;
+        default:
+          throw new OrmException(
+              String.format("Unstar change %d for account %d failed: %s",
+                  changeId.get(), accountId.get(), result.name()));
+      }
+    } catch (IOException e) {
+      throw new OrmException(
+          String.format("Unstar change %d for account %d failed",
+              changeId.get(), accountId.get()), e);
+    }
+  }
+
+  public void unstarAll(Change.Id changeId) throws OrmException {
+    dbProvider.get().starredChanges().delete(
+        dbProvider.get().starredChanges().byChange(changeId));
+    if (!migration.writeChanges()) {
+      return;
+    }
+    try (Repository repo = repoManager.openMetadataRepository(allUsers);
+        RevWalk rw = new RevWalk(repo)) {
+      BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
+      batchUpdate.setAllowNonFastForwards(true);
+      batchUpdate.setRefLogIdent(serverIdent);
+      batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
+      for (Account.Id accountId : byChange(changeId)) {
+        String refName = RefNames.refsStarredChanges(accountId, changeId);
+        Ref ref = repo.getRefDatabase().getRef(refName);
+        batchUpdate.addCommand(new ReceiveCommand(ref.getObjectId(),
+            ObjectId.zeroId(), refName));
+      }
+      batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
+      for (ReceiveCommand command : batchUpdate.getCommands()) {
+        if (command.getResult() != ReceiveCommand.Result.OK) {
+          throw new IOException(String.format(
+              "Unstar change %d failed, ref %s could not be deleted: %s",
+              changeId.get(), command.getRefName(), command.getResult()));
+        }
+      }
+    } catch (IOException e) {
+      throw new OrmException(
+          String.format("Unstar change %d failed", changeId.get()), e);
+    }
+  }
+
+  public Iterable<Account.Id> byChange(final Change.Id changeId)
+      throws OrmException {
+    if (!migration.readChanges()) {
+      return FluentIterable
+          .from(dbProvider.get().starredChanges().byChange(changeId))
+          .transform(new Function<StarredChange, Account.Id>() {
+            @Override
+            public Account.Id apply(StarredChange in) {
+              return in.getAccountId();
+            }
+          });
+    }
+    return FluentIterable.from(getRefNames(RefNames.REFS_STARRED_CHANGES))
+        .filter(new Predicate<String>() {
+          @Override
+          public boolean apply(String refPart) {
+            return refPart.endsWith("/" + changeId.get());
+          }
+        })
+        .transform(new Function<String, Account.Id>() {
+          @Override
+          public Account.Id apply(String refPart) {
+            return Account.Id.fromRefPart(refPart);
+          }
+        });
+  }
+
+  public ResultSet<Change.Id> query(Account.Id accountId) {
+    try {
+      if (!migration.readChanges()) {
+        return new ChangeIdResultSet(
+            dbProvider.get().starredChanges().byAccount(accountId));
+      }
+
+      return new ListResultSet<>(FluentIterable
+          .from(getRefNames(RefNames.refsStarredChangesPrefix(accountId)))
+          .transform(new Function<String, Change.Id>() {
+            @Override
+            public Change.Id apply(String changeId) {
+              return Change.Id.parse(changeId);
+            }
+          }).toList());
+    } catch (OrmException | RuntimeException e) {
+      log.warn(String.format("Cannot query starred changes for account %d",
+          accountId.get()), e);
+      List<Change.Id> empty = Collections.emptyList();
+      return new ListResultSet<>(empty);
+    }
+  }
+
+  private Set<String> getRefNames(String prefix) throws OrmException {
+    try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
+      RefDatabase refDb = repo.getRefDatabase();
+      return refDb.getRefs(prefix).keySet();
+    } catch (IOException e) {
+      throw new OrmException(e);
+    }
+  }
+
+  private static class ChangeIdResultSet implements ResultSet<Change.Id> {
+    private static final Function<StarredChange, Change.Id>
+        STARRED_CHANGE_TO_CHANGE_ID =
+            new Function<StarredChange, Change.Id>() {
+              @Override
+              public Change.Id apply(StarredChange starredChange) {
+                return starredChange.getChangeId();
+              }
+            };
+
+    private final ResultSet<StarredChange> starredChangesResultSet;
+
+    ChangeIdResultSet(ResultSet<StarredChange> starredChangesResultSet) {
+      this.starredChangesResultSet = starredChangesResultSet;
+    }
+
+    @Override
+    public Iterator<Change.Id> iterator() {
+      return Iterators.transform(starredChangesResultSet.iterator(),
+          STARRED_CHANGE_TO_CHANGE_ID);
+    }
+
+    @Override
+    public List<Change.Id> toList() {
+      return Lists.transform(starredChangesResultSet.toList(),
+          STARRED_CHANGE_TO_CHANGE_ID);
+    }
+
+    @Override
+    public void close() {
+      starredChangesResultSet.close();
+    }
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java

@@ -27,10 +27,9 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.reviewdb.client.StarredChange;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.change.ChangesCollection;
 import com.google.gerrit.server.query.change.QueryChanges;
@@ -43,8 +42,6 @@ import com.google.inject.Singleton;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Collections;
-
 @Singleton
 public class StarredChanges implements
     ChildCollection<AccountResource, AccountResource.StarredChange>,
@@ -117,13 +114,13 @@ public class StarredChanges implements
   @Singleton
   public static class Create implements RestModifyView<AccountResource, EmptyInput> {
     private final Provider<CurrentUser> self;
-    private final Provider<ReviewDb> dbProvider;
+    private final StarredChangesUtil starredChangesUtil;
     private ChangeResource change;
 
     @Inject
-    Create(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider) {
+    Create(Provider<CurrentUser> self, StarredChangesUtil starredChangesUtil) {
       this.self = self;
-      this.dbProvider = dbProvider;
+      this.starredChangesUtil = starredChangesUtil;
     }
 
     public Create setChange(ChangeResource change) {
@@ -138,10 +135,7 @@ public class StarredChanges implements
         throw new AuthException("not allowed to add starred change");
       }
       try {
-        dbProvider.get().starredChanges().insert(Collections.singleton(
+        starredChangesUtil.star(self.get().getAccountId(), change.getId());
-            new StarredChange(new StarredChange.Key(
-                rsrc.getUser().getAccountId(),
-                change.getId()))));
       } catch (OrmDuplicateKeyException e) {
         return Response.none();
       }
@@ -173,12 +167,12 @@ public class StarredChanges implements
   public static class Delete implements
       RestModifyView<AccountResource.StarredChange, EmptyInput> {
     private final Provider<CurrentUser> self;
-    private final Provider<ReviewDb> dbProvider;
+    private final StarredChangesUtil starredChangesUtil;
 
     @Inject
-    Delete(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider) {
+    Delete(Provider<CurrentUser> self, StarredChangesUtil starredChangesUtil) {
       this.self = self;
-      this.dbProvider = dbProvider;
+      this.starredChangesUtil = starredChangesUtil;
     }
 
     @Override
@@ -187,10 +181,8 @@ public class StarredChanges implements
       if (self.get() != rsrc.getUser()) {
         throw new AuthException("not allowed remove starred change");
       }
-      dbProvider.get().starredChanges().delete(Collections.singleton(
+      starredChangesUtil.unstar(self.get().getAccountId(),
-          new StarredChange(new StarredChange.Key(
+          rsrc.getChange().getId());
-              rsrc.getUser().getAccountId(),
-              rsrc.getChange().getId()))));
       return Response.none();
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java

@@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.StarredChange;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.mail.ProjectWatch.Watchers;
 import com.google.gerrit.server.patch.PatchList;
@@ -297,9 +296,9 @@ public abstract class ChangeEmail extends NotificationEmail {
     try {
       // BCC anyone who has starred this change.
       //
-      for (StarredChange w : args.db.get().starredChanges().byChange(
+      for (Account.Id accountId : args.starredChangesUtil
-          change.getId())) {
+          .byChange(change.getId())) {
-        super.add(RecipientType.BCC, w.getAccountId());
+        super.add(RecipientType.BCC, accountId);
       }
     } catch (OrmException err) {
       // Just don't BCC everyone. Better to send a partial message to those

gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java

@@ -22,6 +22,7 @@ import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.GerritPersonIdentProvider;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.IdentifiedUser.GenericFactory;
+import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.CapabilityControl;
 import com.google.gerrit.server.account.GroupBackend;
@@ -73,6 +74,7 @@ public class EmailArguments {
   final RuntimeInstance velocityRuntime;
   final EmailSettings settings;
   final DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners;
+  final StarredChangesUtil starredChangesUtil;
 
   @Inject
   EmailArguments(GitRepositoryManager server, ProjectCache projectCache,
@@ -96,7 +98,8 @@ public class EmailArguments {
       RuntimeInstance velocityRuntime,
       EmailSettings settings,
       @SshAdvertisedAddresses List<String> sshAddresses,
-      DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners) {
+      DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners,
+      StarredChangesUtil starredChangesUtil) {
     this.server = server;
     this.projectCache = projectCache;
     this.groupBackend = groupBackend;
@@ -122,5 +125,6 @@ public class EmailArguments {
     this.settings = settings;
     this.sshAddresses = sshAddresses;
     this.outgoingEmailValidationListeners = outgoingEmailValidationListeners;
+    this.starredChangesUtil = starredChangesUtil;
   }
 }

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

@@ -30,6 +30,8 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.StarredChange;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchLineCommentsUtil;
@@ -42,9 +44,12 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceiveCommand;
 
 import java.io.IOException;
 import java.sql.Timestamp;
@@ -95,7 +100,7 @@ public class ChangeRebuilder {
   }
 
   public void rebuild(Change change, BatchRefUpdate bru,
-      BatchRefUpdate bruForDrafts, Repository changeRepo,
+      BatchRefUpdate bruAllUsers, Repository changeRepo,
       Repository allUsersRepo) throws NoSuchChangeException, IOException,
       OrmException {
     deleteRef(change, changeRepo);
@@ -169,15 +174,36 @@ public class ChangeRebuilder {
           draftUpdate = draftUpdateFactory.create(
               controlFactory.controlFor(change, user), e.when);
           draftUpdate.setPatchSetId(e.psId);
-          batchForDrafts = draftUpdate.openUpdateInBatch(bruForDrafts);
+          batchForDrafts = draftUpdate.openUpdateInBatch(bruAllUsers);
         }
         e.applyDraft(draftUpdate);
       }
       writeToBatch(batchForDrafts, draftUpdate, allUsersRepo);
-      synchronized(bruForDrafts) {
+      synchronized(bruAllUsers) {
         batchForDrafts.commit();
       }
     }
+
+    createStarredChangesRefs(changeId, bruAllUsers, allUsersRepo);
+  }
+
+  private void createStarredChangesRefs(Change.Id changeId,
+      BatchRefUpdate bruAllUsers, Repository allUsersRepo)
+          throws IOException, OrmException {
+    ObjectId emptyTree = emptyTree(allUsersRepo);
+    for (StarredChange starred : dbProvider.get().starredChanges()
+        .byChange(changeId)) {
+      bruAllUsers.addCommand(new ReceiveCommand(ObjectId.zeroId(), emptyTree,
+          RefNames.refsStarredChanges(starred.getAccountId(), changeId)));
+    }
+  }
+
+  private static ObjectId emptyTree(Repository repo) throws IOException {
+    try (ObjectInserter oi = repo.newObjectInserter()) {
+      ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {});
+      oi.flush();
+      return id;
+    }
   }
 
   private void deleteRef(Change change, Repository changeRepo)

gerrit-server/src/test/java/com/google/gerrit/server/IdentifiedUserTest.java

@@ -95,7 +95,8 @@ public class IdentifiedUserTest {
         bind(CapabilityControl.Factory.class)
           .toProvider(Providers.<CapabilityControl.Factory>of(null));
         bind(Realm.class).toInstance(mockRealm);
-
+        bind(StarredChangesUtil.class)
+            .toProvider(Providers.<StarredChangesUtil> of(null));
       }
     };
 

gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java

@@ -47,6 +47,7 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchLineCommentsUtil;
+import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountLoader;
 import com.google.gerrit.server.account.CapabilityControl;
@@ -184,6 +185,8 @@ public class CommentsTest extends GerritServerTests {
             .toInstance(GitReferenceUpdated.DISABLED);
         bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class)
           .toInstance(serverIdent);
+        bind(StarredChangesUtil.class)
+          .toProvider(Providers.<StarredChangesUtil> of(null));
       }
 
       @Provides

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

@@ -32,6 +32,7 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.CapabilityControl;
 import com.google.gerrit.server.account.FakeRealm;
@@ -140,6 +141,8 @@ public class AbstractChangeNotesTest extends GerritBaseTests {
             .toInstance(serverIdent);
         bind(GitReferenceUpdated.class)
             .toInstance(GitReferenceUpdated.DISABLED);
+        bind(StarredChangesUtil.class)
+            .toProvider(Providers.<StarredChangesUtil> of(null));
       }
     });
 

gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java

@@ -37,6 +37,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.rules.PrologEnvironment;
 import com.google.gerrit.rules.RulesCache;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.CapabilityControl;
 import com.google.gerrit.server.account.FakeRealm;
@@ -325,6 +326,8 @@ public class Util {
         bind(ChangeKindCache.class).to(ChangeKindCacheImpl.NoCache.class);
         bind(MergeabilityCache.class)
           .to(MergeabilityCache.NotImplemented.class);
+        bind(StarredChangesUtil.class)
+          .toProvider(Providers.<StarredChangesUtil> of(null));
       }
     });