Fix reindex after project deletion

Make plugin to properly support NoteDb.

DatabaseDeleteHandler:
- Check if ReviewDb is enabled from inside DatabaseHandler.
- Add functionality for deleteChanges to check whether ReviewDb is
  enabled, since NoteDb behaves differently compared to ReviewDb.

Bug: Issue 11650
Change-Id: I6578e6649e9fe0d8c139243cdade8c86e1b6c0ed

src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java

@@ -16,7 +16,6 @@
 
 import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ProjectResource;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -37,8 +36,7 @@ public class DeleteAction extends DeleteProject implements UiAction<ProjectResou
       DeleteLog deleteLog,
       DeletePreconditions preConditions,
       Configuration cfg,
-      HideProject hideProject,
-      NotesMigration migration) {
+      HideProject hideProject) {
     super(
         dbHandler,
         fsHandler,
@@ -47,8 +45,7 @@ public class DeleteAction extends DeleteProject implements UiAction<ProjectResou
         deleteLog,
         preConditions,
         cfg,
-        hideProject,
-        migration);
+        hideProject);
     this.protectedProjects = protectedProjects;
   }
 

src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java

@@ -22,7 +22,6 @@
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ProjectResource;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -51,7 +50,6 @@ static class Input {
   private final DeleteLog deleteLog;
   private final Configuration cfg;
   private final HideProject hideProject;
-  private NotesMigration migration;
 
   @Inject
   DeleteProject(
@@ -62,8 +60,7 @@ static class Input {
       DeleteLog deleteLog,
       DeletePreconditions preConditions,
       Configuration cfg,
-      HideProject hideProject,
-      NotesMigration migration) {
+      HideProject hideProject) {
     this.dbHandler = dbHandler;
     this.fsHandler = fsHandler;
     this.cacheHandler = cacheHandler;
@@ -72,7 +69,6 @@ static class Input {
     this.preConditions = preConditions;
     this.cfg = cfg;
     this.hideProject = hideProject;
-    this.migration = migration;
   }
 
   @Override
@@ -93,9 +89,7 @@ public void doDelete(ProjectResource rsrc, Input input)
     Exception ex = null;
     try {
       if (!preserve || !cfg.projectOnPreserveHidden()) {
-        if (!migration.disableChangeReviewDb()) {
-          dbHandler.delete(project);
-        }
+        dbHandler.delete(project);
         try {
           fsHandler.delete(project, preserve);
         } catch (RepositoryNotFoundException e) {

src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java

@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.deleteproject.database;
 
 import static java.util.Collections.singleton;
+import static java.util.stream.Collectors.toList;
 
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.reviewdb.client.Account;
@@ -29,7 +30,11 @@
 import com.google.gerrit.server.account.WatchConfig.Accessor;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gwtorm.jdbc.JdbcSchema;
@@ -57,13 +62,19 @@ public class DatabaseDeleteHandler {
   private final ChangeIndexer indexer;
   private final Provider<InternalAccountQuery> accountQueryProvider;
   private final Provider<Accessor> watchConfig;
+  private final ChangeNotes.Factory schemaFactoryNoteDb;
+  private final GitRepositoryManager repoManager;
+  private final NotesMigration migration;
 
   @Inject
   public DatabaseDeleteHandler(
       Provider<ReviewDb> dbProvider,
       StarredChangesUtil starredChangesUtil,
       DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
       ChangeIndexer indexer,
+      ChangeNotes.Factory schemaFactoryNoteDb,
+      NotesMigration migration,
+      GitRepositoryManager repoManager,
       Provider<InternalAccountQuery> accountQueryProvider,
       Provider<WatchConfig.Accessor> watchConfig) {
     this.accountQueryProvider = accountQueryProvider;
@@ -72,29 +83,40 @@ public DatabaseDeleteHandler(
     this.starredChangesUtil = starredChangesUtil;
     this.accountPatchReviewStore = accountPatchReviewStore;
     this.indexer = indexer;
+    this.schemaFactoryNoteDb = schemaFactoryNoteDb;
+    this.repoManager = repoManager;
+    this.migration = migration;
   }
 
-  public void delete(Project project) throws OrmException {
+  public void delete(Project project) throws OrmException, IOException {
     ReviewDb db = ReviewDbUtil.unwrapDb(dbProvider.get());
-    Connection conn = ((JdbcSchema) db).getConnection();
-    try {
-      conn.setAutoCommit(false);
+    if (isReviewDb()) {
+      Connection conn = ((JdbcSchema) db).getConnection();
       try {
-        atomicDelete(db, project, getChangesList(project, conn));
-        conn.commit();
-      } finally {
-        conn.setAutoCommit(true);
-      }
-    } catch (SQLException e) {
-      try {
-        conn.rollback();
-      } catch (SQLException ex) {
-        throw new OrmException(ex);
+        conn.setAutoCommit(false);
+        try {
+          atomicDelete(db, project, getChangesList(project, conn));
+          conn.commit();
+        } finally {
+          conn.setAutoCommit(true);
+        }
+      } catch (SQLException e) {
+        try {
+          conn.rollback();
+        } catch (SQLException ex) {
+          throw new OrmException(ex);
+        }
+        throw new OrmException(e);
       }
-      throw new OrmException(e);
+    } else {
+      atomicDelete(db, project, getChangesListFromNoteDb(project));
     }
   }
 
+  private boolean isReviewDb() {
+    return !migration.disableChangeReviewDb();
+  }
+
   private List<Change.Id> getChangesList(Project project, Connection conn) throws SQLException {
     try (PreparedStatement changesForProject =
         conn.prepareStatement("SELECT change_id FROM changes WHERE dest_project_name = ?")) {
@@ -111,6 +133,20 @@ private List<Change.Id> getChangesList(Project project, Connection conn) throws
     }
   }
 
+  private List<Change.Id> getChangesListFromNoteDb(Project project) throws IOException {
+    Project.NameKey projectKey = project.getNameKey();
+    List<Change.Id> changeIds =
+        schemaFactoryNoteDb
+            .scan(repoManager.openRepository(projectKey), dbProvider.get(), projectKey)
+            .map(ChangeNotesResult::id)
+            .collect(toList());
+    log.debug(
+        "Number of changes in noteDb related to project {} are {}",
+        projectKey.get(),
+        changeIds.size());
+    return changeIds;
+  }
+
   private void deleteChanges(ReviewDb db, Project.NameKey project, List<Change.Id> changeIds)
       throws OrmException {
 
@@ -120,18 +156,19 @@ private void deleteChanges(ReviewDb db, Project.NameKey project, List<Change.Id>
       } catch (NoSuchChangeException e) {
         // we can ignore the exception during delete
       }
-      ResultSet<PatchSet> patchSets = db.patchSets().byChange(id);
-      if (patchSets != null) {
-        deleteFromPatchSets(db, patchSets);
-      }
-
-      // In the future, use schemaVersion to decide what to delete.
-      db.patchComments().delete(db.patchComments().byChange(id));
-      db.patchSetApprovals().delete(db.patchSetApprovals().byChange(id));
+      if (isReviewDb()) {
+        ResultSet<PatchSet> patchSets = db.patchSets().byChange(id);
+        if (patchSets != null) {
+          deleteFromPatchSets(db, patchSets);
+        }
 
-      db.changeMessages().delete(db.changeMessages().byChange(id));
-      db.changes().deleteKeys(Collections.singleton(id));
+        // In the future, use schemaVersion to decide what to delete.
+        db.patchComments().delete(db.patchComments().byChange(id));
+        db.patchSetApprovals().delete(db.patchSetApprovals().byChange(id));
 
+        db.changeMessages().delete(db.changeMessages().byChange(id));
+        db.changes().deleteKeys(Collections.singleton(id));
+      }
       // Delete from the secondary index
       try {
         indexer.delete(id);