Skip to content

Commit

Permalink
fix: optional plugin dependencies not working correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
@guqing
guqing committed Dec 2, 2024
1 parent e5be856 commit fa9ea61
Show file tree
Hide file tree
Showing 10 changed files with 341 additions and 61 deletions.
57 changes: 34 additions & 23 deletions application/src/main/java/run/halo/app/core/reconciler/PluginReconciler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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();
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 10 additions & 4 deletions application/src/main/java/run/halo/app/plugin/DefaultPluginApplicationContextFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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);

Expand Down
56 changes: 56 additions & 0 deletions application/src/main/java/run/halo/app/plugin/OptionalDependentResolver.java
View file
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
5 changes: 4 additions & 1 deletion application/src/main/java/run/halo/app/plugin/PluginApplicationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions application/src/main/java/run/halo/app/plugin/PluginService.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
41 changes: 30 additions & 11 deletions application/src/main/java/run/halo/app/plugin/PluginServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -269,11 +271,11 @@ public Flux<DataBuffer> uglifyJsBundle() {
.sort(Comparator.comparing(PluginWrapper::getPluginId))
.flatMapSequential(pluginWrapper -> {
var pluginId = pluginWrapper.getPluginId();
return Mono.<Resource>fromSupplier(
() -> BundleResourceUtils.getJsBundleResource(
return Mono.fromSupplier(() -> BundleResourceUtils.getJsBundleResource(
pluginManager, pluginId, BundleResourceUtils.JS_BUNDLE
)
)
.cast(Resource.class)
.filter(Resource::isReadable)
.flatMapMany(resource -> {
var head = Mono.<DataBuffer>fromSupplier(
Expand All @@ -297,9 +299,10 @@ public Flux<DataBuffer> uglifyCssBundle() {
.flatMapSequential(pluginWrapper -> {
var pluginId = pluginWrapper.getPluginId();
var dataBufferFactory = DefaultDataBufferFactory.sharedInstance;
return Mono.<Resource>fromSupplier(() -> BundleResourceUtils.getJsBundleResource(
return Mono.fromSupplier(() -> BundleResourceUtils.getJsBundleResource(
pluginManager, pluginId, BundleResourceUtils.CSS_BUNDLE
))
.cast(Resource.class)
.filter(Resource::isReadable)
.flatMapMany(resource -> {
var head = Mono.<DataBuffer>fromSupplier(() -> dataBufferFactory.wrap(
Expand Down Expand Up @@ -344,14 +347,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)
Expand Down Expand Up @@ -416,6 +414,27 @@ 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);
})
.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(
() -> {
Expand Down
Loading

0 comments on commit fa9ea61

Please sign in to comment.