From 89d902355b52d739616c28e38a868fd010631782 Mon Sep 17 00:00:00 2001 From: guqing <1484563614@qq.com> Date: Fri, 29 Nov 2024 17:46:15 +0800 Subject: [PATCH] fix: optional plugin dependencies not working correctly --- .../app/core/reconciler/PluginReconciler.java | 29 +++++------ ...efaultPluginApplicationContextFactory.java | 14 +++-- .../app/plugin/PluginApplicationContext.java | 5 +- .../run/halo/app/plugin/PluginService.java | 13 +++++ .../halo/app/plugin/PluginServiceImpl.java | 51 ++++++++++++------- .../DefaultExtensionGetter.java | 32 ++++++++++-- .../core/reconciler/PluginReconcilerTest.java | 11 +++- .../DefaultExtensionGetterTest.java | 43 ++++++++++------ 8 files changed, 139 insertions(+), 59 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 index 5a5a2ecd21..936d4f547d 100644 --- a/application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java +++ b/application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java @@ -62,6 +62,7 @@ import run.halo.app.infra.utils.YamlUnstructuredLoader; 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 +86,16 @@ public class PluginReconciler implements Reconciler { 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(); } @@ -343,13 +347,12 @@ private Result enablePlugin(Plugin plugin) { private Result disablePlugin(Plugin plugin) { var pluginName = plugin.getMetadata().getName(); var status = plugin.getStatus(); - if (pluginManager.getPlugin(pluginName) != null) { + var pluginWrapper = pluginManager.getPlugin(pluginName); + if (pluginWrapper != null) { // check if the plugin has children - var dependents = pluginManager.getDependents(pluginName) - .stream() - .filter(pw -> PluginState.STARTED.equals(pw.getPluginState())) - .map(PluginWrapper::getPluginId) - .toList(); + var dependents = pluginService.getRequiredDependencies(plugin, + pw -> PluginState.STARTED.equals(pw.getPluginState()) + ); var conditions = status.getConditions(); if (!CollectionUtils.isEmpty(dependents)) { removeConditionBy(conditions, ConditionType.READY); @@ -515,15 +518,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 index 8a6ba01266..f15cfa2f0c 100644 --- 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,19 @@ 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(); + + // create a custom bean factory to set the class loader + 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 +91,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/PluginApplicationContext.java b/application/src/main/java/run/halo/app/plugin/PluginApplicationContext.java index 19b01ec1d8..6ad179ad21 100644 --- 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 index 3e8bb6e601..2c75360219 100644 --- 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 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 getRequiredDependencies(Plugin plugin, + Predicate 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 index 2bc3812c0a..0def2b0936 100644 --- 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,11 +271,11 @@ public Flux uglifyJsBundle() { .sort(Comparator.comparing(PluginWrapper::getPluginId)) .flatMapSequential(pluginWrapper -> { var pluginId = pluginWrapper.getPluginId(); - return Mono.fromSupplier( - () -> BundleResourceUtils.getJsBundleResource( + return Mono.fromSupplier(() -> BundleResourceUtils.getJsBundleResource( pluginManager, pluginId, BundleResourceUtils.JS_BUNDLE ) ) + .cast(Resource.class) .filter(Resource::isReadable) .flatMapMany(resource -> { var head = Mono.fromSupplier( @@ -297,9 +299,10 @@ public Flux uglifyCssBundle() { .flatMapSequential(pluginWrapper -> { var pluginId = pluginWrapper.getPluginId(); var dataBufferFactory = DefaultDataBufferFactory.sharedInstance; - return Mono.fromSupplier(() -> BundleResourceUtils.getJsBundleResource( + return Mono.fromSupplier(() -> BundleResourceUtils.getJsBundleResource( pluginManager, pluginId, BundleResourceUtils.CSS_BUNDLE )) + .cast(Resource.class) .filter(Resource::isReadable) .flatMapMany(resource -> { var head = Mono.fromSupplier(() -> dataBufferFactory.wrap( @@ -344,14 +347,9 @@ public Mono 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) @@ -359,13 +357,9 @@ public Mono changeState(String pluginName, boolean requestToEnable, bool } } else { // make sure the dependents are disabled - var dependents = pluginManager.getDependents(pluginName); - var notDisabledDependents = dependents.stream() - .filter( - dependent -> Objects.equals(STARTED, dependent.getPluginState()) - ) - .map(PluginWrapper::getPluginId) - .toList(); + var notDisabledDependents = getRequiredDependencies(plugin, + pw -> pw != null && Objects.equals(STARTED, pw.getPluginState()) + ); if (!CollectionUtils.isEmpty(notDisabledDependents)) { return Mono.error( new PluginDependentsNotDisabledException(notDisabledDependents) @@ -416,6 +410,27 @@ public Mono changeState(String pluginName, boolean requestToEnable, bool return updatedPlugin; } + @Override + public List getRequiredDependencies(Plugin plugin, + Predicate predicate) { + return getPluginDependency(plugin) + .stream() + .filter(dependency -> !dependency.isOptional()) + .map(PluginDependency::getPluginId) + .filter(dependencyPlugin -> { + var pluginWrapper = pluginManager.getPlugin(dependencyPlugin); + return predicate.test(pluginWrapper); + }) + .toList(); + } + + private static List getPluginDependency(Plugin plugin) { + return plugin.getSpec().getPluginDependencies().keySet() + .stream() + .map(PluginDependency::new) + .toList(); + } + Mono findPluginManifest(Path path) { return Mono.fromSupplier( () -> { diff --git a/application/src/main/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetter.java b/application/src/main/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetter.java index 42b833e16d..e843c80745 100644 --- a/application/src/main/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetter.java +++ b/application/src/main/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetter.java @@ -2,16 +2,19 @@ import static run.halo.app.extension.index.query.QueryFactory.equal; +import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Objects; -import java.util.Optional; 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; import org.springframework.data.domain.Sort; +import org.springframework.lang.NonNull; import org.springframework.stereotype.Component; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -22,7 +25,9 @@ import run.halo.app.extension.router.selector.FieldSelector; import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; import run.halo.app.infra.SystemSetting.ExtensionPointEnabled; +import run.halo.app.plugin.SpringPlugin; +@Slf4j @Component @RequiredArgsConstructor public class DefaultExtensionGetter implements ExtensionGetter { @@ -37,7 +42,7 @@ public class DefaultExtensionGetter implements ExtensionGetter { @Override public Flux getExtensions(Class extensionPoint) { - return Flux.fromIterable(pluginManager.getExtensions(extensionPoint)) + return Flux.fromIterable(lookExtensions(extensionPoint)) .concatWith( Flux.fromStream(() -> beanFactory.getBeanProvider(extensionPoint).orderedStream()) ) @@ -47,8 +52,7 @@ public Flux getExtensions(Class extensionPoint) @Override public List getExtensionList(Class extensionPoint) { var extensions = new LinkedList(); - Optional.ofNullable(pluginManager.getExtensions(extensionPoint)) - .ifPresent(extensions::addAll); + extensions.addAll(lookExtensions(extensionPoint)); extensions.addAll(beanFactory.getBeanProvider(extensionPoint).orderedStream().toList()); extensions.sort(new AnnotationAwareOrderComparator()); return extensions; @@ -112,4 +116,24 @@ private Mono fetchExtensionPointDefinition( .flatMap(list -> Mono.justOrEmpty(ListResult.first(list))); } + @NonNull + protected List lookExtensions(Class type) { + List beans = new ArrayList<>(); + for (PluginWrapper startedPlugin : pluginManager.getStartedPlugins()) { + if (startedPlugin.getPlugin() instanceof SpringPlugin springPlugin) { + var pluginApplicationContext = springPlugin.getApplicationContext(); + try { + pluginApplicationContext.getBeansOfType(type) + .forEach((name, bean) -> beans.add(bean)); + } catch (Throwable e) { + // Ignore + log.error("Error while looking for extensions of type {}", type, e); + } + } else { + var extensions = pluginManager.getExtensions(type, startedPlugin.getPluginId()); + beans.addAll(extensions); + } + } + return beans; + } } diff --git a/application/src/test/java/run/halo/app/core/reconciler/PluginReconcilerTest.java b/application/src/test/java/run/halo/app/core/reconciler/PluginReconcilerTest.java index a22492cfcd..e07b1a2e62 100644 --- a/application/src/test/java/run/halo/app/core/reconciler/PluginReconcilerTest.java +++ b/application/src/test/java/run/halo/app/core/reconciler/PluginReconcilerTest.java @@ -48,7 +48,6 @@ import run.halo.app.core.extension.Plugin; import run.halo.app.core.extension.ReverseProxy; import run.halo.app.core.extension.Setting; -import run.halo.app.core.reconciler.PluginReconciler; import run.halo.app.extension.ConfigMap; import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.Metadata; @@ -58,6 +57,7 @@ import run.halo.app.infra.Condition; import run.halo.app.infra.ConditionStatus; import run.halo.app.plugin.PluginProperties; +import run.halo.app.plugin.PluginService; import run.halo.app.plugin.SpringPluginManager; /** @@ -78,6 +78,9 @@ class PluginReconcilerTest { @Mock PluginProperties pluginProperties; + @Mock + private PluginService pluginService; + @InjectMocks PluginReconciler reconciler; @@ -111,6 +114,12 @@ class WhenNotDeleting { @TempDir Path tempPath; + @BeforeEach + void setUp() { + lenient().when(pluginService.getRequiredDependencies(any(), any())) + .thenReturn(List.of()); + } + @Test void shouldNotStartPluginWithDevModeInNonDevEnv() { var fakePlugin = createPlugin(name, plugin -> { diff --git a/application/src/test/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetterTest.java b/application/src/test/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetterTest.java index b876aa10c7..42bf8c9f3a 100644 --- a/application/src/test/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetterTest.java +++ b/application/src/test/java/run/halo/app/plugin/extensionpoint/DefaultExtensionGetterTest.java @@ -2,8 +2,11 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import static run.halo.app.infra.SystemSetting.ExtensionPointEnabled.GROUP; @@ -83,10 +86,12 @@ void shouldGetExtensionBySingletonDefinitionWhenExtensionPointEnabledSet() { when(beanFactory.getBeanProvider(FakeExtensionPoint.class)).thenReturn(objectProvider); var extensionImpl = new FakeExtensionPointImpl(); - when(pluginManager.getExtensions(FakeExtensionPoint.class)) - .thenReturn(List.of(extensionImpl)); - getter.getEnabledExtensions(FakeExtensionPoint.class) + var spyGetter = spy(getter); + doReturn(List.of(extensionImpl)).when(spyGetter) + .lookExtensions(eq(FakeExtensionPoint.class)); + + spyGetter.getEnabledExtensions(FakeExtensionPoint.class) .as(StepVerifier::create) .expectNext(extensionImpl) .verifyComplete(); @@ -112,10 +117,11 @@ void shouldGetDefaultSingletonDefinitionWhileExtensionPointEnabledNotSet() { .thenReturn(Stream.of(extensionDefaultImpl)); when(beanFactory.getBeanProvider(FakeExtensionPoint.class)).thenReturn(objectProvider); - when(pluginManager.getExtensions(FakeExtensionPoint.class)) - .thenReturn(List.of()); + var spyGetter = spy(getter); + doReturn(List.of()).when(spyGetter) + .lookExtensions(eq(FakeExtensionPoint.class)); - getter.getEnabledExtensions(FakeExtensionPoint.class) + spyGetter.getEnabledExtensions(FakeExtensionPoint.class) .as(StepVerifier::create) .expectNext(extensionDefaultImpl) .verifyComplete(); @@ -162,10 +168,12 @@ void shouldGetMultiInstanceExtensionWhileExtensionPointEnabledSet() { var extensionImpl = new FakeExtensionPointImpl(); var anotherExtensionImpl = new FakeExtensionPoint() { }; - when(pluginManager.getExtensions(FakeExtensionPoint.class)) - .thenReturn(List.of(extensionImpl, anotherExtensionImpl)); - getter.getEnabledExtensions(FakeExtensionPoint.class) + var spyGetter = spy(getter); + doReturn(List.of(extensionImpl, anotherExtensionImpl)).when(spyGetter) + .lookExtensions(eq(FakeExtensionPoint.class)); + + spyGetter.getEnabledExtensions(FakeExtensionPoint.class) .as(StepVerifier::create) // should keep the order of enabled extensions .expectNext(extensionDefaultImpl) @@ -198,10 +206,12 @@ void shouldGetMultiInstanceExtensionWhileExtensionPointEnabledNotSet() { var extensionImpl = new FakeExtensionPointImpl(); var anotherExtensionImpl = new FakeExtensionPoint() { }; - when(pluginManager.getExtensions(FakeExtensionPoint.class)) - .thenReturn(List.of(extensionImpl, anotherExtensionImpl)); - getter.getEnabledExtensions(FakeExtensionPoint.class) + var spyGetter = spy(getter); + doReturn(List.of(extensionImpl, anotherExtensionImpl)).when(spyGetter) + .lookExtensions(eq(FakeExtensionPoint.class)); + + spyGetter.getEnabledExtensions(FakeExtensionPoint.class) .as(StepVerifier::create) // should keep the order according to @Order annotation // order is 1 @@ -217,13 +227,16 @@ void shouldGetMultiInstanceExtensionWhileExtensionPointEnabledNotSet() { void shouldGetExtensionsFromPluginManagerAndApplicationContext() { var extensionFromPlugin = new FakeExtensionPointDefaultImpl(); var extensionFromAppContext = new FakeExtensionPointImpl(); - when(pluginManager.getExtensions(FakeExtensionPoint.class)) - .thenReturn(List.of(extensionFromPlugin)); + + var spyGetter = spy(getter); + doReturn(List.of(extensionFromPlugin)).when(spyGetter) + .lookExtensions(eq(FakeExtensionPoint.class)); + when(beanFactory.getBeanProvider(FakeExtensionPoint.class)) .thenReturn(extensionPointObjectProvider); when(extensionPointObjectProvider.orderedStream()) .thenReturn(Stream.of(extensionFromAppContext)); - var extensions = getter.getExtensionList(FakeExtensionPoint.class); + var extensions = spyGetter.getExtensionList(FakeExtensionPoint.class); assertEquals(List.of(extensionFromAppContext, extensionFromPlugin), extensions); }