fix: optional plugin dependencies not working correctly (#7094)

#### What type of PR is this? /kind bug /area core /milestone 2.20.x #### What this PR does / why we need it: 修复可选插件依赖功能无法正常工作的问题 #### Special notes for your reviewer: 使用以下两个插件测试可选依赖： [测试插件集合.zip](https://github.com/user-attachments/files/17989250/default.zip) 使用以下测试用例进行测试：测试用例1：plugin-feed 插件提供 RSS 扩展功能 - **前置条件：** 安装并启用 `plugin-feed` 插件。 - **操作步骤：** 访问 `http://localhost:8090/feed/rss.xml`。 - **期望结果：** 返回 `plugin-feed` 提供的 RSS 内容。 --- 测试用例 2: plugin-moments 扩展了 plugin-feed 的 RSS 功能（依赖于 plugin-feed） - **前置条件：** 安装并启用 `plugin-feed` 和 `plugin-moments` 插件。 - **操作步骤：** 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** 返回 `plugin-moments` 提供的 RSS 内容。 --- 测试用例 3: plugin-feed 启用时安装 plugin-moments - **前置条件：** 启用 `plugin-feed` 插件。 - **操作步骤：** 1. 安装 `plugin-moments` 插件。 2. 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** `plugin-moments` 提供的 RSS 路由可访问，并返回正确内容。 --- 测试用例 4: plugin-feed 未启用时安装 plugin-moments - **前置条件：** 未安装或未启用 `plugin-feed` 插件。 - **操作步骤：** 1. 安装并启用 `plugin-moments` 插件。 2. 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** - `plugin-moments` 的 RSS 路由不可访问，返回 404。 - `plugin-moments` 的其他功能正常运行。 --- 测试用例 5: plugin-moments 启用后安装 plugin-feed - **前置条件：** 已安装并启用 `plugin-moments` 插件。 - **操作步骤：** 1. 安装并启用 `plugin-feed` 插件。 2. 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** `plugin-moments` 提供的 RSS 路由可访问，并返回正确内容。 --- 测试用例 6: 停止 plugin-feed 后验证 RSS 路由 - **前置条件：** 已启用 `plugin-feed` 和 `plugin-moments` 插件。 - **操作步骤：** 1. 停止 `plugin-feed` 插件。 2. 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** - `plugin-feed` 停止成功。 - `plugin-moments` 提供的 RSS 路由不可访问，返回 404。 --- 测试用例 7: 重启 Halo 后验证可选依赖插件的启动顺序 - **前置条件：** 已启用 `plugin-feed` 和 `plugin-moments` 插件。 - **操作步骤：** 1. 重启 Halo 服务。 2. 访问 `http://localhost:8090/feed/moments/rss.xml`。 - **期望结果：** - `plugin-moments` 提供的 RSS 路由**始终可访问**。 --- 测试用例 8: 必选依赖插件验证 - **场景 1: 安装 seo 插件时未安装应用市场** - **前置条件：** 未安装 `app-store-integration` 插件。 - **操作步骤：** 安装 `plugin-seo` 插件。 - **期望结果：** 提示需要先安装 `app-store-integration` 插件。 - **场景 2: 停止应用市场插件时 seo 插件仍启用** - **前置条件：** 已启用 `app-store-integration` 和 `plugin-seo` 插件。 - **操作步骤：** 停止 `app-store-integration` 插件。 - **期望结果：** 提示需要先停止 `plugin-seo` 插件。 #### Does this PR introduce a user-facing change? ```release-note 修复可选插件依赖功能无法正常工作的问题 ```
halo-dev · Dec 4, 2024 · fef06ed · fef06ed
1 parent d06b40c
commit fef06ed
Show file tree

Hide file tree

Showing 10 changed files with 366 additions and 68 deletions.
diff --git a/application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java b/application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java
@@ -32,7 +32,6 @@
 import java.util.function.Supplier;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
-import org.pf4j.PluginDependency;
 import org.pf4j.PluginState;
 import org.pf4j.PluginWrapper;
 import org.pf4j.RuntimeMode;
@@ -48,6 +47,7 @@
 import run.halo.app.extension.ExtensionClient;
 import run.halo.app.extension.ExtensionUtil;
 import run.halo.app.extension.Metadata;
+import run.halo.app.extension.MetadataUtil;
 import run.halo.app.extension.Unstructured;
 import run.halo.app.extension.controller.Controller;
 import run.halo.app.extension.controller.ControllerBuilder;
@@ -60,8 +60,10 @@
 import run.halo.app.infra.utils.PathUtils;
 import run.halo.app.infra.utils.SettingUtils;
 import run.halo.app.infra.utils.YamlUnstructuredLoader;
+import run.halo.app.plugin.OptionalDependentResolver;
 import run.halo.app.plugin.PluginConst;
 import run.halo.app.plugin.PluginProperties;
+import run.halo.app.plugin.PluginService;
 import run.halo.app.plugin.SpringPluginManager;
 
 /**
@@ -85,13 +87,16 @@ public class PluginReconciler implements Reconciler<Request> {
 
     private final PluginProperties pluginProperties;
 
+    private final PluginService pluginService;
+
     private Clock clock;
 
     public PluginReconciler(ExtensionClient client, SpringPluginManager pluginManager,
-        PluginProperties pluginProperties) {
+        PluginProperties pluginProperties, PluginService pluginService) {
         this.client = client;
         this.pluginManager = pluginManager;
         this.pluginProperties = pluginProperties;
+        this.pluginService = pluginService;
         this.clock = Clock.systemUTC();
     }
 
@@ -280,18 +285,9 @@ private Result enablePlugin(Plugin plugin) {
         status.setPhase(Plugin.Phase.STARTING);
 
         // check if the parent plugin is started
-        var pw = pluginManager.getPlugin(pluginName);
-        var unstartedDependencies = pw.getDescriptor().getDependencies()
-            .stream()
-            .filter(pd -> {
-                if (pd.isOptional()) {
-                    return false;
-                }
-                var parent = pluginManager.getPlugin(pd.getPluginId());
-                return parent == null || !PluginState.STARTED.equals(parent.getPluginState());
-            })
-            .map(PluginDependency::getPluginId)
-            .toList();
+        var unstartedDependencies = pluginService.getRequiredDependencies(plugin, pw ->
+            pw == null || !PluginState.STARTED.equals(pw.getPluginState())
+        );
         var conditions = status.getConditions();
         if (!CollectionUtils.isEmpty(unstartedDependencies)) {
             removeConditionBy(conditions, ConditionType.READY);
@@ -337,9 +333,31 @@ private Result enablePlugin(Plugin plugin) {
         status.setPhase(Plugin.Phase.STARTED);
 
         log.info("Started plugin {}", pluginName);
+
+        requestToReloadPluginsOptionallyDependentOn(pluginName);
+
         return null;
     }
 
+    void requestToReloadPluginsOptionallyDependentOn(String pluginName) {
+        var startedPlugins = pluginManager.getStartedPlugins()
+            .stream()
+            .map(PluginWrapper::getDescriptor)
+            .toList();
+        var resolver = new OptionalDependentResolver(startedPlugins);
+        var dependents = resolver.getOptionalDependents(pluginName);
+        for (String dependentName : dependents) {
+            client.fetch(Plugin.class, dependentName)
+                .ifPresent(childPlugin -> {
+                    var annotations = MetadataUtil.nullSafeAnnotations(childPlugin);
+                    // loadLocation never be null for started plugins
+                    annotations.put(RELOAD_ANNO,
+                        childPlugin.getStatus().getLoadLocation().toString());
+                    client.update(childPlugin);
+                });
+        }
+    }
+
     private Result disablePlugin(Plugin plugin) {
         var pluginName = plugin.getMetadata().getName();
         var status = plugin.getStatus();
@@ -515,15 +533,9 @@ private Result loadOrReload(Plugin plugin) {
         }
 
         // check dependencies before loading
-        var unresolvedParentPlugins = plugin.getSpec().getPluginDependencies().keySet()
-            .stream()
-            .filter(dependency -> {
-                var parentPlugin = pluginManager.getPlugin(dependency);
-                return parentPlugin == null
-                    || pluginManager.getUnresolvedPlugins().contains(parentPlugin);
-            })
-            .sorted()
-            .toList();
+        var unresolvedParentPlugins = pluginService.getRequiredDependencies(plugin,
+            pw -> pw == null || pluginManager.getUnresolvedPlugins().contains(pw)
+        );
         if (!unresolvedParentPlugins.isEmpty()) {
             // requeue if the parent plugin is not loaded yet.
             removeConditionBy(conditions, ConditionType.INITIALIZED);

diff --git a/application/src/main/java/run/halo/app/plugin/DefaultPluginApplicationContextFactory.java b/application/src/main/java/run/halo/app/plugin/DefaultPluginApplicationContextFactory.java
@@ -11,6 +11,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.pf4j.PluginRuntimeException;
 import org.springframework.beans.factory.support.DefaultBeanNameGenerator;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.boot.env.PropertySourceLoader;
 import org.springframework.boot.env.YamlPropertySourceLoader;
 import org.springframework.context.ApplicationContext;
@@ -65,13 +66,33 @@ public ApplicationContext create(String pluginId) {
 
         var sw = new StopWatch("CreateApplicationContextFor" + pluginId);
         sw.start("Create");
-        var context = new PluginApplicationContext(pluginId, pluginManager);
+
+        var pluginWrapper = pluginManager.getPlugin(pluginId);
+        var classLoader = pluginWrapper.getPluginClassLoader();
+
+        /*
+         * Manually creating a BeanFactory and setting the plugin's ClassLoader is necessary
+         * to ensure that conditional annotations (e.g., @ConditionalOnClass) within the plugin
+         * context can correctly load classes.
+         * When PluginApplicationContext is created, its constructor initializes an
+         * AnnotatedBeanDefinitionReader, which in turn creates a ConditionEvaluator.
+         * ConditionEvaluator is responsible for evaluating conditional annotations such as
+         * @ConditionalOnClass.
+         * It relies on the BeanDefinitionRegistry's ClassLoader to load the classes specified in
+         * the annotations.
+         * Without explicitly setting the plugin's ClassLoader, the default application
+         * ClassLoader is used, which fails to load classes from the plugin.
+         * Therefore, a custom DefaultListableBeanFactory with the plugin ClassLoader is required
+         * to resolve this issue.
+         */
+        var beanFactory = new DefaultListableBeanFactory();
+        beanFactory.setBeanClassLoader(classLoader);
+
+        var context = new PluginApplicationContext(pluginId, pluginManager, beanFactory);
         context.setBeanNameGenerator(DefaultBeanNameGenerator.INSTANCE);
         context.registerShutdownHook();
         context.setParent(pluginManager.getSharedContext());
 
-        var pluginWrapper = pluginManager.getPlugin(pluginId);
-        var classLoader = pluginWrapper.getPluginClassLoader();
         var resourceLoader = new DefaultResourceLoader(classLoader);
         context.setResourceLoader(resourceLoader);
         sw.stop();
@@ -84,7 +105,6 @@ public ApplicationContext create(String pluginId) {
         sw.stop();
 
         sw.start("RegisterBeans");
-        var beanFactory = context.getBeanFactory();
         beanFactory.registerSingleton("pluginWrapper", pluginWrapper);
         context.registerBean(AggregatedRouterFunction.class);
 

diff --git a/application/src/main/java/run/halo/app/plugin/OptionalDependentResolver.java b/application/src/main/java/run/halo/app/plugin/OptionalDependentResolver.java
@@ -0,0 +1,56 @@
+package run.halo.app.plugin;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.pf4j.PluginDependency;
+import org.pf4j.PluginDescriptor;
+import org.pf4j.util.DirectedGraph;
+
+/**
+ * <p>Pass in a list of started plugin names to resolve dependency relationships, and return a
+ * list of plugin names that depend on the specified plugin.</p>
+ * <p>Do not consider the problem of circular dependency, because all the plugins that have been
+ * started must not have this problem.</p>
+ */
+public class OptionalDependentResolver {
+    private final DirectedGraph<String> dependentsGraph;
+    private final List<PluginDescriptor> plugins;
+
+    public OptionalDependentResolver(List<PluginDescriptor> startedPlugins) {
+        this.plugins = startedPlugins;
+        this.dependentsGraph = new DirectedGraph<>();
+        resolve();
+    }
+
+    private void resolve() {
+        // populate graphs
+        for (PluginDescriptor plugin : plugins) {
+            addPlugin(plugin);
+        }
+    }
+
+    public List<String> getOptionalDependents(String pluginId) {
+        return new ArrayList<>(dependentsGraph.getNeighbors(pluginId));
+    }
+
+    private void addPlugin(PluginDescriptor descriptor) {
+        String pluginId = descriptor.getPluginId();
+        List<PluginDependency> dependencies = descriptor.getDependencies();
+        if (dependencies.isEmpty()) {
+            dependentsGraph.addVertex(pluginId);
+        } else {
+            boolean edgeAdded = false;
+            for (PluginDependency dependency : dependencies) {
+                if (dependency.isOptional()) {
+                    edgeAdded = true;
+                    dependentsGraph.addEdge(dependency.getPluginId(), pluginId);
+                }
+            }
+
+            // Register the plugin without dependencies, if all of its dependencies are optional.
+            if (!edgeAdded) {
+                dependentsGraph.addVertex(pluginId);
+            }
+        }
+    }
+}
diff --git a/application/src/main/java/run/halo/app/plugin/PluginApplicationContext.java b/application/src/main/java/run/halo/app/plugin/PluginApplicationContext.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.concurrent.locks.StampedLock;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.core.ResolvableType;
@@ -27,7 +28,9 @@ public class PluginApplicationContext extends AnnotationConfigApplicationContext
 
     private final SpringPluginManager pluginManager;
 
-    public PluginApplicationContext(String pluginId, SpringPluginManager pluginManager) {
+    public PluginApplicationContext(String pluginId, SpringPluginManager pluginManager,
+        DefaultListableBeanFactory beanFactory) {
+        super(beanFactory);
         this.pluginId = pluginId;
         this.pluginManager = pluginManager;
     }

diff --git a/application/src/main/java/run/halo/app/plugin/PluginService.java b/application/src/main/java/run/halo/app/plugin/PluginService.java
@@ -1,6 +1,9 @@
 package run.halo.app.plugin;
 
 import java.nio.file.Path;
+import java.util.List;
+import java.util.function.Predicate;
+import org.pf4j.PluginWrapper;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.web.server.ServerWebInputException;
@@ -102,4 +105,14 @@ public interface PluginService {
      */
     Mono<Plugin> changeState(String pluginName, boolean requestToEnable, boolean wait);
 
+    /**
+     * Gets required dependencies of the given plugin.
+     *
+     * @param plugin the plugin to get dependencies from
+     * {@link Plugin.PluginSpec#getPluginDependencies()}
+     * @param predicate the predicate to filter by {@link PluginWrapper},such as enabled or disabled
+     * @return plugin names of required dependencies
+     */
+    List<String> getRequiredDependencies(Plugin plugin,
+        Predicate<PluginWrapper> predicate);
 }
diff --git a/application/src/main/java/run/halo/app/plugin/PluginServiceImpl.java b/application/src/main/java/run/halo/app/plugin/PluginServiceImpl.java
@@ -21,10 +21,12 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.pf4j.DependencyResolver;
+import org.pf4j.PluginDependency;
 import org.pf4j.PluginDescriptor;
 import org.pf4j.PluginWrapper;
 import org.reactivestreams.Publisher;
@@ -269,12 +271,7 @@ public Flux<DataBuffer> uglifyJsBundle() {
             .sort(Comparator.comparing(PluginWrapper::getPluginId))
             .flatMapSequential(pluginWrapper -> {
                 var pluginId = pluginWrapper.getPluginId();
-                return Mono.<Resource>fromSupplier(
-                        () -> BundleResourceUtils.getJsBundleResource(
-                            pluginManager, pluginId, BundleResourceUtils.JS_BUNDLE
-                        )
-                    )
-                    .filter(Resource::isReadable)
+                return getBundleResource(pluginId, BundleResourceUtils.JS_BUNDLE)
                     .flatMapMany(resource -> {
                         var head = Mono.<DataBuffer>fromSupplier(
                             () -> dataBufferFactory.wrap(
@@ -297,10 +294,7 @@ public Flux<DataBuffer> uglifyCssBundle() {
             .flatMapSequential(pluginWrapper -> {
                 var pluginId = pluginWrapper.getPluginId();
                 var dataBufferFactory = DefaultDataBufferFactory.sharedInstance;
-                return Mono.<Resource>fromSupplier(() -> BundleResourceUtils.getJsBundleResource(
-                        pluginManager, pluginId, BundleResourceUtils.CSS_BUNDLE
-                    ))
-                    .filter(Resource::isReadable)
+                return getBundleResource(pluginId, BundleResourceUtils.CSS_BUNDLE)
                     .flatMapMany(resource -> {
                         var head = Mono.<DataBuffer>fromSupplier(() -> dataBufferFactory.wrap(
                             ("/* Generated from plugin " + pluginId + " */\n").getBytes()
@@ -313,6 +307,13 @@ public Flux<DataBuffer> uglifyCssBundle() {
             });
     }
 
+    private Mono<Resource> getBundleResource(String pluginName, String bundleName) {
+        return Mono.fromSupplier(
+                () -> BundleResourceUtils.getJsBundleResource(pluginManager, pluginName, bundleName)
+            )
+            .filter(Resource::isReadable);
+    }
+
     @Override
     public Mono<String> generateBundleVersion() {
         if (pluginManager.isDevelopment()) {
@@ -344,14 +345,9 @@ public Mono<Plugin> changeState(String pluginName, boolean requestToEnable, bool
                     // preflight check
                     if (requestToEnable) {
                         // make sure the dependencies are enabled
-                        var dependencies = plugin.getSpec().getPluginDependencies().keySet();
-                        var notStartedDependencies = dependencies.stream()
-                            .filter(dependency -> {
-                                var pluginWrapper = pluginManager.getPlugin(dependency);
-                                return pluginWrapper == null
-                                    || !Objects.equals(STARTED, pluginWrapper.getPluginState());
-                            })
-                            .toList();
+                        var notStartedDependencies = getRequiredDependencies(plugin,
+                            pw -> pw == null || !Objects.equals(STARTED, pw.getPluginState())
+                        );
                         if (!CollectionUtils.isEmpty(notStartedDependencies)) {
                             return Mono.error(
                                 new PluginDependenciesNotEnabledException(notStartedDependencies)
@@ -416,6 +412,28 @@ public Mono<Plugin> changeState(String pluginName, boolean requestToEnable, bool
         return updatedPlugin;
     }
 
+    @Override
+    public List<String> getRequiredDependencies(Plugin plugin,
+        Predicate<PluginWrapper> predicate) {
+        return getPluginDependency(plugin)
+            .stream()
+            .filter(dependency -> !dependency.isOptional())
+            .map(PluginDependency::getPluginId)
+            .filter(dependencyPlugin -> {
+                var pluginWrapper = pluginManager.getPlugin(dependencyPlugin);
+                return predicate.test(pluginWrapper);
+            })
+            .sorted()
+            .toList();
+    }
+
+    private static List<PluginDependency> getPluginDependency(Plugin plugin) {
+        return plugin.getSpec().getPluginDependencies().keySet()
+            .stream()
+            .map(PluginDependency::new)
+            .toList();
+    }
+
     Mono<Plugin> findPluginManifest(Path path) {
         return Mono.fromSupplier(
                 () -> {