Refactor plugin management to use SpringPluginManager and improve started plugins retrieval

application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java

@@ -408,7 +408,7 @@ private Result enablePlugin(Plugin plugin) {
     }
 
     void requestToReloadPluginsOptionallyDependentOn(String pluginName) {
-        var startedPlugins = pluginManager.getStartedPlugins()
+        var startedPlugins = pluginManager.startedPlugins()
             .stream()
             .map(PluginWrapper::getDescriptor)
             .toList();
@@ -454,6 +454,7 @@ private Result disablePlugin(Plugin plugin) {
                 removeStartTaskIfPresent(pluginName);
                 pluginManager.disablePlugin(pluginName);
             } catch (Throwable e) {
+                log.error("Error occurred when disabling plugin {}", pluginName, e);
                 conditions.addAndEvictFIFO(Condition.builder()
                     .type(ConditionType.READY)
                     .status(ConditionStatus.FALSE)

application/src/main/java/run/halo/app/plugin/HaloPluginManager.java

@@ -181,6 +181,15 @@ public List<PluginWrapper> getDependents(String pluginId) {
         return dependents;
     }
 
+    @Override
+    public List<PluginWrapper> startedPlugins() {
+        return List.copyOf(super.getStartedPlugins())
+            .stream()
+            // Make sure the plugin is really started
+            .filter(p -> p.getPluginState().isStarted())
+            .toList();
+    }
+
     /**
      * Listener for plugin started event.
      *

application/src/main/java/run/halo/app/plugin/PluginBeforeStopSyncListener.java

@@ -3,6 +3,7 @@
 import java.time.Duration;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.context.event.EventListener;
+import org.springframework.dao.OptimisticLockingFailureException;
 import org.springframework.lang.NonNull;
 import org.springframework.retry.RetryException;
 import org.springframework.stereotype.Component;
@@ -11,6 +12,7 @@
 import reactor.util.retry.Retry;
 import run.halo.app.extension.GroupVersionKind;
 import run.halo.app.extension.ReactiveExtensionClient;
+import run.halo.app.extension.exception.SchemeNotFoundException;
 import run.halo.app.plugin.event.HaloPluginBeforeStopEvent;
 
 /**
@@ -21,7 +23,9 @@
  */
 @Slf4j
 @Component
-public class PluginBeforeStopSyncListener {
+class PluginBeforeStopSyncListener {
+
+    private static final Duration CLEANUP_TIMEOUT = Duration.ofMinutes(1);
 
     private final ReactiveExtensionClient client;
 
@@ -30,7 +34,7 @@ public PluginBeforeStopSyncListener(ReactiveExtensionClient client) {
     }
 
     @EventListener
-    public void onApplicationEvent(@NonNull HaloPluginBeforeStopEvent event) {
+    void onApplicationEvent(@NonNull HaloPluginBeforeStopEvent event) {
         var pluginWrapper = event.getPlugin();
         var p = pluginWrapper.getPlugin();
         if (!(p instanceof SpringPlugin springPlugin)) {
@@ -40,15 +44,20 @@ public void onApplicationEvent(@NonNull HaloPluginBeforeStopEvent event) {
         if (!(applicationContext instanceof PluginApplicationContext pluginApplicationContext)) {
             return;
         }
-        cleanUpPluginExtensionResources(pluginApplicationContext).block(Duration.ofMinutes(1));
+        cleanUpPluginExtensionResources(pluginApplicationContext).block(CLEANUP_TIMEOUT);
     }
 
     private Mono<Void> cleanUpPluginExtensionResources(PluginApplicationContext context) {
         var gvkExtensionNames = context.extensionNamesMapping();
         return Flux.fromIterable(gvkExtensionNames.entrySet())
             .flatMap(entry -> Flux.fromIterable(entry.getValue())
-                .flatMap(extensionName -> client.fetch(entry.getKey(), extensionName))
-                .flatMap(client::delete)
+                .flatMap(extensionName -> client.fetch(entry.getKey(), extensionName)
+                    .onErrorComplete(SchemeNotFoundException.class)
+                    .flatMap(client::delete)
+                    .retryWhen(Retry.backoff(10, Duration.ofMillis(100))
+                        .filter(OptimisticLockingFailureException.class::isInstance)
+                    )
+                )
                 .flatMap(e -> waitForDeleted(e.groupVersionKind(), e.getMetadata().getName())))
             .then();
     }

application/src/main/java/run/halo/app/plugin/PluginServiceImpl.java

@@ -244,7 +244,7 @@ public Mono<Plugin> reload(String name) {
 
     @Override
     public Flux<DataBuffer> uglifyJsBundle() {
-        var startedPlugins = List.copyOf(pluginManager.getStartedPlugins());
+        var startedPlugins = pluginManager.startedPlugins();
         var dataBufferFactory = DefaultDataBufferFactory.sharedInstance;
         var end = Mono.fromSupplier(
             () -> {
@@ -289,7 +289,7 @@ public Flux<DataBuffer> uglifyJsBundle() {
 
     @Override
     public Flux<DataBuffer> uglifyCssBundle() {
-        return Flux.fromIterable(pluginManager.getStartedPlugins())
+        return Flux.fromIterable(pluginManager.startedPlugins())
             .sort(Comparator.comparing(PluginWrapper::getPluginId))
             .flatMapSequential(pluginWrapper -> {
                 var pluginId = pluginWrapper.getPluginId();
@@ -319,7 +319,7 @@ public Mono<String> generateBundleVersion() {
         if (pluginManager.isDevelopment()) {
             return Mono.just(String.valueOf(clock.instant().toEpochMilli()));
         }
-        return Flux.fromIterable(new ArrayList<>(pluginManager.getStartedPlugins()))
+        return Flux.fromIterable(pluginManager.startedPlugins())
             .sort(Comparator.comparing(PluginWrapper::getPluginId))
             .map(pw -> pw.getPluginId() + ':' + pw.getDescriptor().getVersion())
             .collect(Collectors.joining())
@@ -429,7 +429,7 @@ public List<String> getRequiredDependencies(Plugin plugin,
 
     @Override
     public Flux<String> getStartedPluginNames() {
-        return Flux.fromIterable(List.copyOf(pluginManager.getStartedPlugins()))
+        return Flux.fromIterable(pluginManager.startedPlugins())
             .map(PluginWrapper::getPluginId);
     }
 

application/src/main/java/run/halo/app/plugin/PluginStartedListener.java

@@ -4,6 +4,7 @@
 import static run.halo.app.plugin.PluginExtensionLoaderUtils.isSetting;
 import static run.halo.app.plugin.PluginExtensionLoaderUtils.lookupExtensions;
 
+import java.time.Duration;
 import java.util.HashMap;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.context.event.EventListener;
@@ -24,7 +25,9 @@
  */
 @Slf4j
 @Component
-public class PluginStartedListener {
+class PluginStartedListener {
+
+    private static final Duration TIMEOUT = Duration.ofMinutes(1);
 
     private final ReactiveExtensionClient client;
 
@@ -44,19 +47,19 @@ private Mono<Unstructured> createOrUpdate(Unstructured unstructured) {
     }
 
     @EventListener
-    public Mono<Void> onApplicationEvent(HaloPluginStartedEvent event) {
+    void onApplicationEvent(HaloPluginStartedEvent event) {
         var pluginWrapper = event.getPlugin();
         var p = pluginWrapper.getPlugin();
         if (!(p instanceof SpringPlugin springPlugin)) {
-            return Mono.empty();
+            return;
         }
         var applicationContext = springPlugin.getApplicationContext();
         if (!(applicationContext instanceof PluginApplicationContext pluginApplicationContext)) {
-            return Mono.empty();
+            return;
         }
         var pluginName = pluginWrapper.getPluginId();
 
-        return client.get(Plugin.class, pluginName)
+        client.get(Plugin.class, pluginName)
             .flatMap(plugin -> Flux.fromStream(
                     () -> {
                         log.debug("Collecting extensions for plugin {}", pluginName);
@@ -80,6 +83,8 @@ public Mono<Void> onApplicationEvent(HaloPluginStartedEvent event) {
                     labels.put(PLUGIN_NAME_LABEL_NAME, plugin.getMetadata().getName());
                 })
                 .flatMap(this::createOrUpdate)
-                .then());
+                .then()
+            )
+            .block(TIMEOUT);
     }
 }

application/src/main/java/run/halo/app/plugin/SharedEventDispatcher.java

@@ -1,7 +1,6 @@
 package run.halo.app.plugin;
 
-import java.util.ArrayList;
-import org.pf4j.PluginManager;
+import lombok.RequiredArgsConstructor;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.Lifecycle;
@@ -10,24 +9,20 @@
 import org.springframework.stereotype.Component;
 
 @Component
+@RequiredArgsConstructor
 public class SharedEventDispatcher {
 
-    private final PluginManager pluginManager;
+    private final SpringPluginManager pluginManager;
 
     private final ApplicationEventPublisher publisher;
 
-    public SharedEventDispatcher(PluginManager pluginManager, ApplicationEventPublisher publisher) {
-        this.pluginManager = pluginManager;
-        this.publisher = publisher;
-    }
-
     @EventListener(ApplicationEvent.class)
     void onApplicationEvent(ApplicationEvent event) {
         if (AnnotationUtils.findAnnotation(event.getClass(), SharedEvent.class) == null) {
             return;
         }
         // we should copy the plugins list to avoid ConcurrentModificationException
-        var startedPlugins = new ArrayList<>(pluginManager.getStartedPlugins());
+        var startedPlugins = pluginManager.startedPlugins();
         // broadcast event to all started plugins except the publisher
         for (var startedPlugin : startedPlugins) {
             var plugin = startedPlugin.getPlugin();

application/src/main/java/run/halo/app/plugin/SpringPlugin.java

@@ -23,6 +23,7 @@ public interface SpringPlugin {
      * Gets application context of the plugin.
      *
      * @return application context of the plugin
+     * @throws IllegalStateException if the application context is not ready yet
      */
     @NonNull
     ApplicationContext getApplicationContext();

application/src/main/java/run/halo/app/plugin/SpringPluginManager.java

@@ -40,4 +40,21 @@ public interface SpringPluginManager extends PluginManager {
      */
     @NonNull
     List<PluginWrapper> getDependents(@NonNull String pluginId);
+
+    /**
+     * Gets all started plugins.
+     *
+     * @return a list of started plugins. Mutable.
+     * @apiNote The plugin inside this list may not be really started
+     */
+    @Override
+    List<PluginWrapper> getStartedPlugins();
+
+    /**
+     * Gets all really started plugins.
+     *
+     * @return a list of really started plugins. Immutable.
+     */
+    List<PluginWrapper> startedPlugins();
+
 }

application/src/main/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetter.java

@@ -7,7 +7,6 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.pf4j.ExtensionPoint;
-import org.pf4j.PluginManager;
 import org.pf4j.PluginWrapper;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
@@ -18,6 +17,7 @@
 import run.halo.app.infra.SystemConfigFetcher;
 import run.halo.app.infra.SystemSetting.ExtensionPointEnabled;
 import run.halo.app.plugin.SpringPlugin;
+import run.halo.app.plugin.SpringPluginManager;
 
 @Slf4j
 @Component
@@ -26,7 +26,7 @@ public class DefaultExtensionGetter implements ExtensionGetter {
 
     private final SystemConfigFetcher systemConfigFetcher;
 
-    private final PluginManager pluginManager;
+    private final SpringPluginManager pluginManager;
 
     private final BeanFactory beanFactory;
 
@@ -104,13 +104,10 @@ private Mono<ExtensionPointDefinition> fetchExtensionPointDefinition(
     protected <T> List<T> lookExtensions(Class<T> type) {
         List<T> beans = new ArrayList<>();
         // avoid concurrent modification
-        var startedPlugins = List.copyOf(pluginManager.getStartedPlugins());
+        var startedPlugins = pluginManager.startedPlugins();
         for (PluginWrapper startedPlugin : startedPlugins) {
             if (startedPlugin.getPlugin() instanceof SpringPlugin springPlugin) {
                 var pluginApplicationContext = springPlugin.getApplicationContext();
-                if (pluginApplicationContext == null) {
-                    continue;
-                }
                 try {
                     pluginApplicationContext.getBeansOfType(type)
                         .forEach((name, bean) -> beans.add(bean));

application/src/test/java/run/halo/app/plugin/PluginServiceImplTest.java

@@ -226,7 +226,7 @@ void generateBundleVersionTest() {
         var plugin1 = mock(PluginWrapper.class);
         var plugin2 = mock(PluginWrapper.class);
         var plugin3 = mock(PluginWrapper.class);
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(plugin1, plugin2, plugin3));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(plugin1, plugin2, plugin3));
 
         var descriptor1 = mock(PluginDescriptor.class);
         var descriptor2 = mock(PluginDescriptor.class);
@@ -259,7 +259,7 @@ void generateBundleVersionTest() {
         when(descriptor4.getVersion()).thenReturn("3.0.0");
         var str2 = "fake-1:1.0.0fake-2:2.0.0fake-4:3.0.0";
         var result2 = Hashing.sha256().hashUnencodedChars(str2).toString();
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(plugin1, plugin2, plugin4));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(plugin1, plugin2, plugin4));
         pluginService.generateBundleVersion()
             .as(StepVerifier::create)
             .consumeNextWith(version -> assertThat(version).isEqualTo(result2))
@@ -278,7 +278,7 @@ void shouldGenerateRandomBundleVersionInDevelopment() {
             .expectNext(String.valueOf(clock.instant().toEpochMilli()))
             .verifyComplete();
 
-        verify(pluginManager, never()).getStartedPlugins();
+        verify(pluginManager, never()).startedPlugins();
     }
 
     @Test
@@ -287,7 +287,7 @@ void shouldGetStartedPluginNames() {
         when(plugin1.getPluginId()).thenReturn("plugin-1");
         var plugin2 = mock(PluginWrapper.class);
         when(plugin2.getPluginId()).thenReturn("plugin-2");
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(plugin1, plugin2));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(plugin1, plugin2));
 
         pluginService.getStartedPluginNames()
             .as(StepVerifier::create)

application/src/test/java/run/halo/app/plugin/SharedEventDispatcherTest.java

@@ -13,7 +13,6 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.pf4j.Plugin;
-import org.pf4j.PluginManager;
 import org.pf4j.PluginWrapper;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationEvent;
@@ -24,7 +23,7 @@
 class SharedEventDispatcherTest {
 
     @Mock
-    PluginManager pluginManager;
+    SpringPluginManager pluginManager;
 
     @Mock
     ApplicationEventPublisher publisher;
@@ -47,7 +46,7 @@ void shouldDispatchEventToAllStartedPlugins() {
         when(((Lifecycle) context).isRunning()).thenReturn(true);
         when(((SpringPlugin) plugin).getApplicationContext()).thenReturn(context);
         when(pw.getPlugin()).thenReturn(plugin);
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(pw));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(pw));
 
         var event = new FakeSharedEvent(this);
         dispatcher.onApplicationEvent(event);
@@ -64,7 +63,7 @@ void shouldNotDispatchEventToAllStartedPluginsWhilePluginContextIsNotRunning() {
         when(((Lifecycle) context).isRunning()).thenReturn(false);
         when(((SpringPlugin) plugin).getApplicationContext()).thenReturn(context);
         when(pw.getPlugin()).thenReturn(plugin);
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(pw));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(pw));
         var event = new FakeSharedEvent(this);
         dispatcher.onApplicationEvent(event);
         verify(context, never()).publishEvent(event);
@@ -77,7 +76,7 @@ void shouldNotDispatchEventToAllStartedPluginsWhilePluginContextIsNotLifecycle()
         var context = mock(ApplicationContext.class);
         when(((SpringPlugin) plugin).getApplicationContext()).thenReturn(context);
         when(pw.getPlugin()).thenReturn(plugin);
-        when(pluginManager.getStartedPlugins()).thenReturn(List.of(pw));
+        when(pluginManager.startedPlugins()).thenReturn(List.of(pw));
         var event = new FakeSharedEvent(this);
         dispatcher.onApplicationEvent(event);
         verify(context, never()).publishEvent(event);