Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
DeleteProjectIT: Check that project gets unwatched after deletion
DeleteProjectIT: Assert that after forced delete reindexing happened
DeleteProjectIT: Add ssh delete watched project test similar to http
Fix reindex after project deletion
Change-Id: I29fca09892a5a6878547c5d0bbd2966310d986a3
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java
index 040f7e4..459d819 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java
+++ b/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 @@
DeleteLog deleteLog,
DeletePreconditions preConditions,
Configuration cfg,
- HideProject hideProject,
- NotesMigration migration) {
+ HideProject hideProject) {
super(
dbHandler,
fsHandler,
@@ -47,8 +45,7 @@
deleteLog,
preConditions,
cfg,
- hideProject,
- migration);
+ hideProject);
this.protectedProjects = protectedProjects;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
index e3fa6b9..4b6e75f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
@@ -21,7 +21,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;
@@ -50,7 +49,6 @@
private final DeleteLog deleteLog;
private final Configuration cfg;
private final HideProject hideProject;
- private NotesMigration migration;
@Inject
DeleteProject(
@@ -61,8 +59,7 @@
DeleteLog deleteLog,
DeletePreconditions preConditions,
Configuration cfg,
- HideProject hideProject,
- NotesMigration migration) {
+ HideProject hideProject) {
this.dbHandler = dbHandler;
this.fsHandler = fsHandler;
this.cacheHandler = cacheHandler;
@@ -71,7 +68,6 @@
this.preConditions = preConditions;
this.cfg = cfg;
this.hideProject = hideProject;
- this.migration = migration;
}
@Override
@@ -91,9 +87,7 @@
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) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java
index 5dff8c5..cca8386 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java
+++ b/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.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -30,7 +31,11 @@
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.ProjectWatches.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;
@@ -56,6 +61,9 @@
private final ChangeIndexer indexer;
private final Provider<InternalAccountQuery> accountQueryProvider;
private final Provider<AccountsUpdate> accountsUpdateProvider;
+ private final ChangeNotes.Factory schemaFactoryNoteDb;
+ private final GitRepositoryManager repoManager;
+ private final NotesMigration migration;
@Inject
public DatabaseDeleteHandler(
@@ -63,6 +71,9 @@
StarredChangesUtil starredChangesUtil,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
ChangeIndexer indexer,
+ ChangeNotes.Factory schemaFactoryNoteDb,
+ NotesMigration migration,
+ GitRepositoryManager repoManager,
Provider<InternalAccountQuery> accountQueryProvider,
@UserInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
this.dbProvider = dbProvider;
@@ -71,29 +82,40 @@
this.indexer = indexer;
this.accountQueryProvider = accountQueryProvider;
this.accountsUpdateProvider = accountsUpdateProvider;
+ 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);
+ 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);
}
- } catch (SQLException e) {
- try {
- conn.rollback();
- } catch (SQLException ex) {
- throw new OrmException(ex);
- }
- 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 = ?")) {
@@ -110,6 +132,20 @@
}
}
+ 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.atFine().log(
+ "Number of changes in noteDb related to project %s are %d",
+ projectKey.get(),
+ changeIds.size());
+ return changeIds;
+ }
+
private void deleteChanges(ReviewDb db, Project.NameKey project, List<Change.Id> changeIds)
throws OrmException {
@@ -119,18 +155,19 @@
} catch (NoSuchChangeException e) {
// we can ignore the exception during delete
}
- ResultSet<PatchSet> patchSets = db.patchSets().byChange(id);
- if (patchSets != null) {
- deleteFromPatchSets(db, patchSets);
+ if (isReviewDb()) {
+ 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));
+
+ 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);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProjectIT.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProjectIT.java
index 206f231..36d1ab3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProjectIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProjectIT.java
@@ -28,9 +28,11 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.client.ProjectState;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gwtorm.server.OrmException;
import com.googlesource.gerrit.plugins.deleteproject.DeleteProject.Input;
import java.io.File;
import java.io.IOException;
@@ -79,6 +81,7 @@
RestResponse r = httpDeleteProjectHelper(true);
r.assertNoContent();
assertThat(projectDir.exists()).isFalse();
+ assertAllChangesDeletedInIndex();
}
@Test
@@ -97,6 +100,8 @@
RestResponse r = httpDeleteProjectHelper(true);
r.assertNoContent();
assertThat(projectDir.exists()).isFalse();
+ assertAllChangesDeletedInIndex();
+ assertWatchRemoved();
}
@Test
@@ -139,6 +144,19 @@
adminSshSession.exec(cmd);
assertThat(adminSshSession.getError()).isNull();
assertThat(projectDir.exists()).isFalse();
+ assertAllChangesDeletedInIndex();
+ }
+
+ @Test
+ @UseLocalDisk
+ public void testSshDeleteProjectWithWatches() throws Exception {
+ watch(project.get());
+ String cmd = createDeleteCommand(project.get());
+ adminSshSession.exec(cmd);
+ assertThat(adminSshSession.getError()).isNull();
+ assertThat(projectDir.exists()).isFalse();
+ assertAllChangesDeletedInIndex();
+ assertWatchRemoved();
}
@Test
@@ -237,6 +255,7 @@
assertThat(isEmpty(archiveFolder.toPath())).isFalse();
assertThat(containsDeletedProject(archiveFolder.toPath(), project.get())).isTrue();
assertThat(projectDir.exists()).isFalse();
+ assertAllChangesDeletedInIndex();
}
@Test
@@ -268,7 +287,7 @@
assertThat(containsDeletedProject(archiveFolder.toPath().resolve(PARENT_FOLDER), name))
.isTrue();
assertThat(projectDir.exists()).isFalse();
-
+ assertAllChangesDeletedInIndex();
assertThat(parentFolder.toFile().exists()).isFalse();
}
@@ -321,4 +340,12 @@
return dirStream.anyMatch(d -> d.toString().contains(projectName));
}
}
+
+ private void assertAllChangesDeletedInIndex() throws OrmException {
+ assertThat(queryProvider.get().byProject(project)).isEmpty();
+ }
+
+ private void assertWatchRemoved() throws RestApiException {
+ assertThat(gApi.accounts().self().getWatchedProjects()).isEmpty();
+ }
}
