From 832c86071ac99195206701060fba19bf65d282de Mon Sep 17 00:00:00 2001
From: guqing <38999863+guqing@users.noreply.github.com>
Date: Fri, 21 Jul 2023 11:36:14 +0800
Subject: [PATCH] fix: plugin delete lifecycle method will not be triggered
 when the plugin is uninstalled (#4241)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#### What type of PR is this?
/kind bug
/kind improvement
/area core
/milestone 2.8.x

#### What this PR does / why we need it:
修复插件被卸载时 delete 生命周期方法不会被触发的问题

how to test it?
1. 测试开发模式下卸载插件，delete 生命周期方法被触发且不会误删项目目录
2. 测试生产模式下插件卸载，文件正确被删除且触发 delete 生命生命周期方法

#### Which issue(s) this PR fixes:
Fixes #4238

#### Does this PR introduce a user-facing change?
```release-note
修复插件被卸载时 delete 生命周期方法不会被触发的问题
```
---
 .../reconciler/PluginReconciler.java          | 18 +-------
 .../DefaultDevelopmentPluginRepository.java   | 11 +++++
 ...efaultDevelopmentPluginRepositoryTest.java | 41 +++++++++++++++++++
 3 files changed, 54 insertions(+), 16 deletions(-)
 create mode 100644 application/src/test/java/run/halo/app/plugin/DefaultDevelopmentPluginRepositoryTest.java

diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java
index 1ba7f1356..441e82a53 100644
--- a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java
+++ b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java
@@ -712,22 +712,8 @@ public class PluginReconciler implements Reconciler<Request> {
         if (pluginWrapper != null) {
             // pluginWrapper must not be null in below code
             // stop and unload plugin, see also PluginBeforeStopSyncListener
-            if (!haloPluginManager.unloadPlugin(name)) {
-                throw new IllegalStateException("Failed to unload plugin: " + name);
-            }
-        }
-
-        // delete plugin resources
-        Path pluginPath = Optional.ofNullable(plugin.statusNonNull().getLoadLocation())
-            .map(URI::getPath)
-            .map(Paths::get)
-            .orElse(null);
-        if (pluginPath != null && isJarFile(pluginPath)) {
-            // delete plugin file
-            try {
-                Files.deleteIfExists(pluginPath);
-            } catch (IOException e) {
-                throw new RuntimeException(e);
+            if (!haloPluginManager.deletePlugin(name)) {
+                throw new IllegalStateException("Failed to delete plugin: " + name);
             }
         }
     }
diff --git a/application/src/main/java/run/halo/app/plugin/DefaultDevelopmentPluginRepository.java b/application/src/main/java/run/halo/app/plugin/DefaultDevelopmentPluginRepository.java
index dac831c51..454a4cd24 100644
--- a/application/src/main/java/run/halo/app/plugin/DefaultDevelopmentPluginRepository.java
+++ b/application/src/main/java/run/halo/app/plugin/DefaultDevelopmentPluginRepository.java
@@ -7,6 +7,11 @@ import org.pf4j.DevelopmentPluginRepository;
 import org.springframework.util.CollectionUtils;
 
 /**
+ * <p>A {@link org.pf4j.PluginRepository} implementation that can add fixed plugin paths for
+ * development {@link org.pf4j.RuntimeMode#DEVELOPMENT}.</p>
+ * <p>change {@link #deletePluginPath(Path)} to a no-op method.</p>
+ * Note: This class is not thread-safe.
+ *
  * @author guqing
  * @since 2.0.0
  */
@@ -39,4 +44,10 @@ public class DefaultDevelopmentPluginRepository extends DevelopmentPluginReposit
         paths.addAll(super.getPluginPaths());
         return paths;
     }
+
+    @Override
+    public boolean deletePluginPath(Path pluginPath) {
+        // do nothing
+        return true;
+    }
 }
diff --git a/application/src/test/java/run/halo/app/plugin/DefaultDevelopmentPluginRepositoryTest.java b/application/src/test/java/run/halo/app/plugin/DefaultDevelopmentPluginRepositoryTest.java
new file mode 100644
index 000000000..0c8684b74
--- /dev/null
+++ b/application/src/test/java/run/halo/app/plugin/DefaultDevelopmentPluginRepositoryTest.java
@@ -0,0 +1,41 @@
+package run.halo.app.plugin;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.pf4j.PluginRepository;
+
+/**
+ * Tests for {@link DefaultDevelopmentPluginRepository}.
+ *
+ * @author guqing
+ * @since 2.8.0
+ */
+class DefaultDevelopmentPluginRepositoryTest {
+
+    private PluginRepository developmentPluginRepository;
+
+    @TempDir
+    private Path tempDir;
+
+    @BeforeEach
+    void setUp() {
+        this.developmentPluginRepository =
+            new DefaultDevelopmentPluginRepository();
+    }
+
+    @Test
+    void deletePluginPath() {
+        boolean deleted = developmentPluginRepository.deletePluginPath(null);
+        assertThat(deleted).isTrue();
+
+        // deletePluginPath is a no-op
+        deleted = developmentPluginRepository.deletePluginPath(tempDir);
+        assertThat(deleted).isTrue();
+        assertThat(Files.exists(tempDir)).isTrue();
+    }
+}