From fa9ea61381b37ac78ca2e74a4cbde90168d21c13 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 | 57 ++++---- ...efaultPluginApplicationContextFactory.java | 14 +- .../app/plugin/OptionalDependentResolver.java | 56 ++++++++ .../app/plugin/PluginApplicationContext.java | 5 +- .../run/halo/app/plugin/PluginService.java | 13 ++ .../halo/app/plugin/PluginServiceImpl.java | 41 ++++-- .../DefaultExtensionGetter.java | 32 ++++- .../core/reconciler/PluginReconcilerTest.java | 15 ++- .../plugin/OptionalDependentResolverTest.java | 126 ++++++++++++++++++ .../DefaultExtensionGetterTest.java | 43 +++--- 10 files changed, 341 insertions(+), 61 deletions(-) create mode 100644 application/src/main/java/run/halo/app/plugin/OptionalDependentResolver.java create mode 100644 application/src/test/java/run/halo/app/plugin/OptionalDependentResolverTest.java 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..04cb16d5d7 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 @@ -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 { 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,30 @@ 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 labels = MetadataUtil.nullSafeLabels(childPlugin); + // loadLocation never be null for started plugins + labels.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 +532,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/OptionalDependentResolver.java b/application/src/main/java/run/halo/app/plugin/OptionalDependentResolver.java new file mode 100644 index 0000000000..d2fbba0e12 --- /dev/null +++ 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; + +/** + *

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.

+ *

Do not consider the problem of circular dependency, because all the plugins that have been + * started must not have this problem.

+ */ +public class OptionalDependentResolver { + private final DirectedGraph dependentsGraph; + private final List plugins; + + public OptionalDependentResolver(List startedPlugins) { + this.plugins = startedPlugins; + this.dependentsGraph = new DirectedGraph<>(); + resolve(); + } + + private void resolve() { + // populate graphs + for (PluginDescriptor plugin : plugins) { + addPlugin(plugin); + } + } + + public List getOptionalDependents(String pluginId) { + return new ArrayList<>(dependentsGraph.getNeighbors(pluginId)); + } + + private void addPlugin(PluginDescriptor descriptor) { + String pluginId = descriptor.getPluginId(); + List 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 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..9008704e40 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) @@ -416,6 +414,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..4b870c6501 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 -> { @@ -277,7 +286,7 @@ void shouldEnablePluginIfEnabled() throws IOException { .thenReturn(mockPluginWrapperForSetting()) .thenReturn(mockPluginWrapperForStaticResources()) // before starting - .thenReturn(mockPluginWrapper(PluginState.STOPPED)) + .thenReturn(mockPluginWrapper(PluginState.STARTED)) // sync plugin state .thenReturn(mockPluginWrapper(PluginState.STARTED)); when(pluginManager.startPlugin(name)).thenReturn(PluginState.STARTED); @@ -308,7 +317,7 @@ void shouldEnablePluginIfEnabled() throws IOException { verify(pluginManager).startPlugin(name); verify(pluginManager).loadPlugin(loadLocation); - verify(pluginManager, times(5)).getPlugin(name); + verify(pluginManager, times(4)).getPlugin(name); verify(client).update(fakePlugin); verify(client).fetch(Setting.class, settingName); verify(client).create(any(Setting.class)); diff --git a/application/src/test/java/run/halo/app/plugin/OptionalDependentResolverTest.java b/application/src/test/java/run/halo/app/plugin/OptionalDependentResolverTest.java new file mode 100644 index 0000000000..1e37f6488b --- /dev/null +++ b/application/src/test/java/run/halo/app/plugin/OptionalDependentResolverTest.java @@ -0,0 +1,126 @@ +package run.halo.app.plugin; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; + +import java.util.List; +import org.junit.jupiter.api.Test; +import org.pf4j.PluginDependency; +import org.pf4j.PluginDescriptor; + +/** + * Tests for {@link OptionalDependentResolver}. + * + * @author guqing + * @since 2.20.11 + */ +class OptionalDependentResolverTest { + + @Test + void testNoPlugins() { + OptionalDependentResolver resolver = new OptionalDependentResolver(List.of()); + assertTrue(resolver.getOptionalDependents("nonexistent").isEmpty(), + "No dependents expected for non-existent plugin"); + } + + @Test + void testSinglePluginNoDependencies() { + var pluginA = createpluginDescriptor("A", List.of()); + OptionalDependentResolver resolver = new OptionalDependentResolver(List.of(pluginA)); + + assertTrue(resolver.getOptionalDependents("A").isEmpty(), "A has no dependents"); + } + + @Test + void testSingleOptionalDependency() { + var pluginA = createpluginDescriptor("A", List.of(new PluginDependency("B?"))); + var pluginB = createpluginDescriptor("B", List.of()); + OptionalDependentResolver resolver = + new OptionalDependentResolver(List.of(pluginA, pluginB)); + + assertEquals(List.of("A"), resolver.getOptionalDependents("B"), + "B should have A as dependent"); + assertTrue(resolver.getOptionalDependents("A").isEmpty(), "A has no dependents"); + } + + @Test + void testMultipleOptionalDependencies() { + var pluginA = createpluginDescriptor("A", List.of( + new PluginDependency("B?"), + new PluginDependency("C?")) + ); + + var pluginB = createpluginDescriptor("B", List.of()); + + var pluginC = createpluginDescriptor("C", List.of()); + + OptionalDependentResolver resolver = + new OptionalDependentResolver(List.of(pluginA, pluginB, pluginC)); + + assertEquals(List.of("A"), resolver.getOptionalDependents("B"), + "B should have A as dependent"); + assertEquals(List.of("A"), resolver.getOptionalDependents("C"), + "C should have A as dependent"); + assertTrue(resolver.getOptionalDependents("A").isEmpty(), "A has no dependents"); + } + + @Test + void testNestedDependencies() { + var pluginA = createpluginDescriptor("A", List.of( + new PluginDependency("B?") + )); + var pluginB = createpluginDescriptor("B", List.of( + new PluginDependency("C?") + )); + var pluginC = createpluginDescriptor("C", List.of()); + + OptionalDependentResolver resolver = + new OptionalDependentResolver(List.of(pluginA, pluginB, pluginC)); + + assertEquals(List.of("B"), resolver.getOptionalDependents("C"), + "C should have B as dependent"); + assertEquals(List.of("A"), resolver.getOptionalDependents("B"), + "B should have A as dependent"); + assertTrue(resolver.getOptionalDependents("A").isEmpty(), "A has no dependents"); + } + + @Test + void testCircularDependencies() { + var pluginA = createpluginDescriptor("A", List.of( + new PluginDependency("B?") + )); + var pluginB = createpluginDescriptor("B", List.of( + new PluginDependency("A?") + )); + OptionalDependentResolver resolver = + new OptionalDependentResolver(List.of(pluginA, pluginB)); + + assertEquals(List.of("B"), resolver.getOptionalDependents("A"), + "A should have B as dependent"); + assertEquals(List.of("A"), resolver.getOptionalDependents("B"), + "B should have A as dependent"); + } + + @Test + void testNonOptionalDependencies() { + var pluginA = createpluginDescriptor("A", List.of( + new PluginDependency("B") + )); + var pluginB = createpluginDescriptor("B", List.of()); + OptionalDependentResolver resolver = + new OptionalDependentResolver(List.of(pluginA, pluginB)); + + assertTrue(resolver.getOptionalDependents("B").isEmpty(), "B should have no dependents"); + assertTrue(resolver.getOptionalDependents("A").isEmpty(), "A should have no dependents"); + } + + PluginDescriptor createpluginDescriptor(String pluginName, + List dependencies) { + var descriptor = mock(PluginDescriptor.class); + lenient().when(descriptor.getPluginId()).thenReturn(pluginName); + lenient().when(descriptor.getDependencies()).thenReturn(dependencies); + return descriptor; + } +} 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); }