ChangesCollection: Lookup change in index when legacy ID is given

So far when a legacy change ID was specified we looked up the change
in the database (via ChangeControl#controlFor(Change.Id,CurrentUser)).
We cannot load the change via ChangeNotes.Factory since we do not know
the project. Hence lookup the change in the secondary index like we
already do it when a Change-Id is given.

ConsistencyCheckerIT creates changes manually and must now take care
to put them into the index, so that the query can find them.

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

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java

@@ -607,6 +607,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
   private Change insertChange() throws Exception {
     Change c = newChange(project, adminId);
     db.changes().insert(singleton(c));
+    indexer.index(db, c);
 
     ChangeUpdate u = changeUpdateFactory.create(
         changeControlFactory.controlFor(c, userFactory.create(adminId)));

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

@@ -16,7 +16,6 @@ package com.google.gerrit.server;
 
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
 import com.google.common.primitives.Ints;
@@ -166,7 +165,6 @@ public class ChangeUtil {
   private final Sequences seq;
   private final Provider<InternalChangeQuery> queryProvider;
   private final PatchSetUtil psUtil;
-  private final ChangeControl.GenericFactory changeControlFactory;
   private final RevertedSender.Factory revertedSenderFactory;
   private final ChangeInserter.Factory changeInserterFactory;
   private final GitRepositoryManager gitManager;
@@ -180,7 +178,6 @@ public class ChangeUtil {
       Sequences seq,
       Provider<InternalChangeQuery> queryProvider,
       PatchSetUtil psUtil,
-      ChangeControl.GenericFactory changeControlFactory,
       RevertedSender.Factory revertedSenderFactory,
       ChangeInserter.Factory changeInserterFactory,
       GitRepositoryManager gitManager,
@@ -192,7 +189,6 @@ public class ChangeUtil {
     this.seq = seq;
     this.queryProvider = queryProvider;
     this.psUtil = psUtil;
-    this.changeControlFactory = changeControlFactory;
     this.revertedSenderFactory = revertedSenderFactory;
     this.changeInserterFactory = changeInserterFactory;
     this.gitManager = gitManager;
@@ -331,24 +327,19 @@ public class ChangeUtil {
    */
   public List<ChangeControl> findChanges(String id, CurrentUser user)
       throws OrmException {
-    // Try legacy id
-    if (!id.isEmpty() && id.charAt(0) != '0') {
-      Integer n = Ints.tryParse(id);
-      try {
-        if (n != null) {
-          return ImmutableList.of(
-              changeControlFactory.controlFor(new Change.Id(n), user));
-        }
-      } catch (NoSuchChangeException e) {
-        return Collections.emptyList();
-      }
-    }
-
     // Use the index to search for changes, but don't return any stored fields,
     // to force rereading in case the index is stale.
     InternalChangeQuery query = queryProvider.get()
         .setRequestedFields(ImmutableSet.<String> of());
 
+    // Try legacy id
+    if (!id.isEmpty() && id.charAt(0) != '0') {
+      Integer n = Ints.tryParse(id);
+      if (n != null) {
+        return asChangeControls(query.byLegacyChangeId(new Change.Id(n)), user);
+      }
+    }
+
     // Try isolated changeId
     if (!id.contains("~")) {
       return asChangeControls(query.byKeyPrefix(id), user);
@@ -366,6 +357,15 @@ public class ChangeUtil {
     return Collections.emptyList();
   }
 
+  public List<ChangeControl> findChanges(Change.Id id, CurrentUser user)
+      throws OrmException {
+    // Use the index to search for changes, but don't return any stored fields,
+    // to force rereading in case the index is stale.
+    InternalChangeQuery query = queryProvider.get()
+        .setRequestedFields(ImmutableSet.<String> of());
+    return asChangeControls(query.byLegacyChangeId(id), user);
+  }
+
   private List<ChangeControl> asChangeControls(List<ChangeData> cds,
       CurrentUser user) throws OrmException {
     List<ChangeControl> ctls = new ArrayList<>(cds.size());

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java

@@ -29,7 +29,6 @@ import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.change.QueryChanges;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -45,7 +44,6 @@ public class ChangesCollection implements
     AcceptsPost<TopLevelResource> {
   private final Provider<ReviewDb> db;
   private final Provider<CurrentUser> user;
-  private final ChangeControl.GenericFactory changeControlFactory;
   private final Provider<QueryChanges> queryFactory;
   private final DynamicMap<RestView<ChangeResource>> views;
   private final ChangeUtil changeUtil;
@@ -56,7 +54,6 @@ public class ChangesCollection implements
   ChangesCollection(
       Provider<ReviewDb> db,
       Provider<CurrentUser> user,
-      ChangeControl.GenericFactory changeControlFactory,
       Provider<QueryChanges> queryFactory,
       DynamicMap<RestView<ChangeResource>> views,
       ChangeUtil changeUtil,
@@ -64,7 +61,6 @@ public class ChangesCollection implements
       ChangeIndexer changeIndexer) {
     this.db = db;
     this.user = user;
-    this.changeControlFactory = changeControlFactory;
     this.queryFactory = queryFactory;
     this.views = views;
     this.changeUtil = changeUtil;
@@ -112,15 +108,23 @@ public class ChangesCollection implements
 
   public ChangeResource parse(Change.Id id)
       throws ResourceNotFoundException, OrmException {
-    try {
-      ChangeControl ctl = changeControlFactory.controlFor(id, user.get());
-      if (!ctl.isVisible(db.get())) {
-        throw new ResourceNotFoundException(toIdString(id));
+    List<ChangeControl> ctls = changeUtil.findChanges(id, user.get());
+    if (ctls.isEmpty()) {
+      try {
+        changeIndexer.delete(id);
+      } catch (IOException e) {
+        throw new ResourceNotFoundException(toIdString(id).get(), e);
       }
-      return new ChangeResource(ctl);
-    } catch (NoSuchChangeException e) {
       throw new ResourceNotFoundException(toIdString(id));
     }
+    if (ctls.size() != 1) {
+      throw new ResourceNotFoundException("Multiple changes found for " + id);
+    }
+    ChangeControl ctl = ctls.get(0);
+    if (!ctl.isVisible(db.get())) {
+      throw new ResourceNotFoundException(toIdString(id));
+    }
+    return new ChangeResource(ctl);
   }
 
   private static IdString toIdString(Change.Id id) {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java

@@ -118,6 +118,10 @@ public class InternalChangeQuery {
     return query(new ChangeIdPredicate(prefix));
   }
 
+  public List<ChangeData> byLegacyChangeId(Change.Id id) throws OrmException {
+    return query(new LegacyChangeIdPredicate(id));
+  }
+
   public List<ChangeData> byBranchKey(Branch.NameKey branch, Change.Key key)
       throws OrmException {
     return query(and(