Delete a change from the index when it is not in the DB

If for some reason the secondary index is out of date, i.e. the  change
was deleted from the database but wasn't deleted from the secondary
index, it was impossible to re-index (remove) that change.

Add logic to automatically remove the change from the secondary index
if this change is requested through the Change REST API endpoint and
doesn't exist in the database.

If a user click on search result from a stale change, he will a get a
404 page and the change will be removed from the index.

To fix the problem without opening the change page, run a command like:
curl --user <user name>  -X POST http://<host>:<port>/a/changes/<change id>/index

Issue: 2996
Change-Id: I1db5373e31585e99c5f45e05274d86d69b4f24e6

8 files changed, 85 insertions, 6 deletions

diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index c396ae6b7b..d03e202538 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -322,6 +322,19 @@ public class LuceneChangeIndex implements ChangeIndex {
     }
   }
 
+  @SuppressWarnings("unchecked")
+  @Override
+  public void delete(int id) throws IOException {
+    Term idTerm = QueryBuilder.idTerm(id);
+    try {
+      Futures.allAsList(
+          openIndex.delete(idTerm),
+          closedIndex.delete(idTerm)).get();
+    } catch (ExecutionException | InterruptedException e) {
+      throw new IOException(e);
+    }
+  }
+
   @Override
   public void deleteAll() throws IOException {
     openIndex.deleteAll();
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
index 3221b8ae9f..392d1645ca 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
@@ -55,6 +55,10 @@ public class QueryBuilder {
     return intTerm(ID_FIELD, cd.getId().get());
   }
 
+  public static Term idTerm(int id) {
+    return intTerm(ID_FIELD, id);
+  }
+
   private final Schema<ChangeData> schema;
   private final org.apache.lucene.util.QueryBuilder queryBuilder;
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
index f7038549df..454ec00cb9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.change;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.primitives.Ints;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -23,7 +24,9 @@ import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+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;
@@ -31,6 +34,7 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
@@ -41,6 +45,7 @@ public class ChangesCollection implements
   private final ChangeControl.GenericFactory changeControlFactory;
   private final Provider<QueryChanges> queryFactory;
   private final DynamicMap<RestView<ChangeResource>> views;
+  private final ChangeIndexer changeIndexer;
 
   @Inject
   ChangesCollection(
@@ -48,12 +53,15 @@ public class ChangesCollection implements
       Provider<CurrentUser> user,
       ChangeControl.GenericFactory changeControlFactory,
       Provider<QueryChanges> queryFactory,
-      DynamicMap<RestView<ChangeResource>> views) {
+      DynamicMap<RestView<ChangeResource>> views,
+      ChangeUtil changeUtil,
+      ChangeIndexer changeIndexer) {
     this.db = dbProvider;
     this.user = user;
     this.changeControlFactory = changeControlFactory;
     this.queryFactory = queryFactory;
     this.views = views;
+    this.changeIndexer = changeIndexer;
   }
 
   @Override
@@ -70,6 +78,16 @@ public class ChangesCollection implements
   public ChangeResource parse(TopLevelResource root, IdString id)
       throws ResourceNotFoundException, OrmException {
     List<Change> changes = findChanges(id.encoded());
+    if (changes.isEmpty()) {
+      Integer changeId = Ints.tryParse(id.get());
+      if (changeId != null) {
+        try {
+          changeIndexer.delete(changeId);
+        } catch (IOException e) {
+          throw new ResourceNotFoundException(id.get(), e);
+        }
+      }
+    }
     if (changes.size() != 1) {
       throw new ResourceNotFoundException(id);
     }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
index a710a108a3..167367898a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
@@ -73,6 +73,15 @@ public interface ChangeIndex {
   public void delete(ChangeData cd) throws IOException;
 
   /**
+   * Delete a change document from the index by id.
+   *
+   * @param id change document id
+   *
+   * @throws IOException
+   */
+  public void delete(int id) throws IOException;
+
+  /**
    * Delete all change documents from the index.
    *
    * @throws IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
index 437f55927d..72cb88c90a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
@@ -166,6 +166,17 @@ public class ChangeIndexer {
   }
 
   /**
+   * Synchronously delete a change by id.
+   *
+   * @param id change to delete.
+   */
+  public void delete(int id) throws IOException {
+    for (ChangeIndex i : getWriteIndexes()) {
+      i.delete(id);
+    }
+  }
+
+  /**
    * Synchronously delete a change.
    *
    * @param change change to delete.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/DummyIndex.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/DummyIndex.java
index 9f1c353876..ff554ff0ef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/DummyIndex.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/DummyIndex.java
@@ -57,4 +57,8 @@ public class DummyIndex implements ChangeIndex {
   @Override
   public void markReady(boolean ready) throws IOException {
   }
+
+  @Override
+  public void delete(int id) throws IOException {
+  }
 }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
index 4db3b27764..a14bbf13f7 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
@@ -22,6 +22,8 @@ import com.google.gerrit.server.query.change.ChangeDataSource;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
 
+import java.io.IOException;
+
 class FakeIndex implements ChangeIndex {
   static Schema<ChangeData> V1 = new Schema<ChangeData>(1, false,
     ImmutableList.<FieldDef<ChangeData, ?>> of(
@@ -106,4 +108,8 @@ class FakeIndex implements ChangeIndex {
   public void markReady(boolean ready) {
     throw new UnsupportedOperationException();
   }
+
+  @Override
+  public void delete(int id) throws IOException {
+  }
 }
diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
index 8c8b0071ac..830db27106 100644
--- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
+++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
@@ -189,13 +189,27 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
     String id = cd.getId().toString();
     try {
       if (cd.change().getStatus().isOpen()) {
-        openIndex.deleteById(id);
-        commit(openIndex);
+        delete(id, openIndex);
       } else {
-        closedIndex.deleteById(id);
-        commit(closedIndex);
+        delete(id, closedIndex);
       }
-    } catch (OrmException | SolrServerException e) {
+    } catch (OrmException e) {
+      throw new IOException(e);
+    }
+  }
+
+  @Override
+  public void delete(int id) throws IOException {
+    String idString = Integer.toString(id);
+    delete(idString, openIndex);
+    delete(idString, closedIndex);
+  }
+
+  private void delete(String id, CloudSolrServer index) throws IOException {
+    try {
+      index.deleteById(id);
+      commit(index);
+    } catch (SolrServerException e) {
       throw new IOException(e);
     }
   }