commit f54de699f0391f39de3383fa2fe3533eb23b3f52
author Christoforos Miliotis <christoforos.miliotis.dev@gmail.com> Fri Apr 28 11:18:35 2023 +0100
committer Christoforos Miliotis <christoforos.miliotis.dev@gmail.com> Wed May 17 08:08:15 2023 +0000
tree b4a0b8e2a5a8d81037d8a87a9df4c111d027f8cc
parent c78b586a3b02afd2044ccbd625ccc42e2479bb56

Use delete-project plugin's repository deletion logic to delete a git
repo

The previous implementation was relying on the replication plugin's
LocalFS class, which was failing to delete directories hosted on NFS
filesystem, because of a lingering file handle.

We are therefore swapping that implementation with the one from the
delete-project plugin, which can effectively handle repos hosted on
NFS.

Worth pointing out that this change introduces inter-plugin dependency
with the delete-project plugin.

Bug: Issue 16730
Change-Id: Iea80a01b84ca72aef5de21db3a05e07a626db079

BUILD

diff --git a/BUILD b/BUILD
index f6a6e48..9428043 100644
--- a/BUILD
+++ b/BUILD
@@ -16,6 +16,7 @@
     resources = glob(["src/main/resources/**/*"]),
     deps = [
         "//lib/commons:io",
+        "//plugins/delete-project",
         "//plugins/replication",
         "@events-broker//jar:neverlink",
     ],
@@ -32,6 +33,7 @@
     deps = PLUGIN_TEST_DEPS + PLUGIN_DEPS + [
         ":pull-replication__plugin",
         ":pull_replication_util",
+        "//plugins/delete-project",
         "//plugins/replication",
         "@events-broker//jar",
     ],
@@ -49,6 +51,7 @@
     ),
     deps = PLUGIN_TEST_DEPS + PLUGIN_DEPS + [
         ":pull-replication__plugin",
-        "//plugins/replication:replication",
+        "//plugins/delete-project",
+        "//plugins/replication",
     ],
 )

src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
index 2e1c5d4..574f632 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
@@ -14,6 +14,8 @@
 
 package com.googlesource.gerrit.plugins.replication.pull.api;
 
+import static com.googlesource.gerrit.plugins.replication.pull.PullReplicationLogger.repLog;
+
 import com.google.gerrit.extensions.api.access.PluginPermission;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -27,9 +29,11 @@
 import com.google.gerrit.server.project.ProjectResource;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-import com.googlesource.gerrit.plugins.replication.LocalFS;
+import com.googlesource.gerrit.plugins.deleteproject.fs.RepositoryDelete;
 import com.googlesource.gerrit.plugins.replication.pull.GerritConfigOps;
+import java.io.IOException;
 import java.util.Optional;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.transport.URIish;
 
 class ProjectDeletionAction
@@ -42,15 +46,18 @@
   private final Provider<CurrentUser> userProvider;
   private final GerritConfigOps gerritConfigOps;
   private final PermissionBackend permissionBackend;
+  private final RepositoryDelete repositoryDelete;
 
   @Inject
   ProjectDeletionAction(
       GerritConfigOps gerritConfigOps,
       PermissionBackend permissionBackend,
-      Provider<CurrentUser> userProvider) {
+      Provider<CurrentUser> userProvider,
+      RepositoryDelete repositoryDelete) {
     this.gerritConfigOps = gerritConfigOps;
     this.permissionBackend = permissionBackend;
     this.userProvider = userProvider;
+    this.repositoryDelete = repositoryDelete;
   }
 
   @Override
@@ -67,11 +74,19 @@
         gerritConfigOps.getGitRepositoryURI(String.format("%s.git", projectResource.getName()));
 
     if (maybeRepoURI.isPresent()) {
-      if (new LocalFS(maybeRepoURI.get()).deleteProject(projectResource.getNameKey())) {
+      try {
+        // reuse repo deletion logic from delete-project plugin, as it can successfully delete
+        // the git directories hosted on nfs.
+        repositoryDelete.execute(projectResource.getNameKey());
+        repLog.info("Deleted local repository {}", projectResource.getName());
         return Response.ok();
+      } catch (RepositoryNotFoundException e) {
+        throw new ResourceNotFoundException(
+            String.format("Repository %s not found", projectResource.getName()), e);
+      } catch (IOException e) {
+        throw new UnprocessableEntityException(
+            String.format("Could not delete project %s", projectResource.getName()));
       }
-      throw new UnprocessableEntityException(
-          String.format("Could not delete project %s", projectResource.getName()));
     }
     throw new ResourceNotFoundException(
         String.format("Could not compute URI for repo: %s", projectResource.getName()));