diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java index 6f0d13967..6796fe91b 100644 --- a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java +++ b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java @@ -12,6 +12,7 @@ import static run.halo.app.plugin.PluginExtensionLoaderUtils.lookupExtensions; import static run.halo.app.plugin.PluginUtils.generateFileName; import static run.halo.app.plugin.PluginUtils.isDevelopmentMode; +import com.fasterxml.jackson.core.type.TypeReference; import java.io.FileNotFoundException; import java.io.IOException; import java.net.MalformedURLException; @@ -21,17 +22,24 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.Clock; +import java.time.Duration; import java.util.ArrayList; +import java.util.Comparator; import java.util.HashMap; +import java.util.List; import java.util.Objects; import java.util.Set; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; -import org.pf4j.PluginManager; +import org.pf4j.PluginRuntimeException; import org.pf4j.PluginState; +import org.pf4j.PluginStateEvent; +import org.pf4j.PluginStateListener; +import org.pf4j.PluginWrapper; import org.pf4j.RuntimeMode; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.stereotype.Component; +import org.springframework.util.CollectionUtils; import org.springframework.util.ResourceUtils; import org.springframework.web.util.UriComponentsBuilder; import run.halo.app.core.extension.Plugin; @@ -50,10 +58,13 @@ import run.halo.app.extension.controller.Reconciler.Request; import run.halo.app.extension.controller.RequeueException; import run.halo.app.infra.Condition; import run.halo.app.infra.ConditionStatus; +import run.halo.app.infra.utils.JsonParseException; +import run.halo.app.infra.utils.JsonUtils; import run.halo.app.infra.utils.PathUtils; import run.halo.app.infra.utils.YamlUnstructuredLoader; import run.halo.app.plugin.PluginConst; import run.halo.app.plugin.PluginProperties; +import run.halo.app.plugin.SpringPluginManager; /** * Plugin reconciler. @@ -66,19 +77,23 @@ import run.halo.app.plugin.PluginProperties; @Component public class PluginReconciler implements Reconciler { private static final String FINALIZER_NAME = "plugin-protection"; + private static final String DEPENDENTS_ANNO_KEY = "plugin.halo.run/dependents-snapshot"; private final ExtensionClient client; - private final PluginManager pluginManager; + private final SpringPluginManager pluginManager; private final PluginProperties pluginProperties; private Clock clock; - public PluginReconciler(ExtensionClient client, PluginManager pluginManager, + public PluginReconciler(ExtensionClient client, SpringPluginManager pluginManager, PluginProperties pluginProperties) { this.client = client; this.pluginManager = pluginManager; this.pluginProperties = pluginProperties; this.clock = Clock.systemUTC(); + + this.pluginManager.addPluginStateListener(new PluginStartedListener()); + this.pluginManager.addPluginStateListener(new PluginStoppedListener()); } /** @@ -95,6 +110,11 @@ public class PluginReconciler implements Reconciler { return client.fetch(Plugin.class, request.name()) .map(plugin -> { if (ExtensionUtil.isDeleted(plugin)) { + if (!checkDependents(plugin)) { + client.update(plugin); + // Check dependents every 10 seconds + return Result.requeue(Duration.ofSeconds(10)); + } // CleanUp resources and remove finalizer. if (removeFinalizers(plugin.getMetadata(), Set.of(FINALIZER_NAME))) { cleanupResources(plugin); @@ -117,10 +137,10 @@ public class PluginReconciler implements Reconciler { if (requestToEnable(plugin)) { // Start - startPlugin(plugin); + enablePlugin(plugin); } else { // stop the plugin and disable it - stopAndDisablePlugin(plugin); + disablePlugin(plugin); } } catch (Throwable t) { // populate condition @@ -145,6 +165,28 @@ public class PluginReconciler implements Reconciler { .orElseGet(Result::doNotRetry); } + private boolean checkDependents(Plugin plugin) { + var pluginId = plugin.getMetadata().getName(); + var dependents = pluginManager.getDependents(pluginId); + if (CollectionUtils.isEmpty(dependents)) { + return true; + } + var status = plugin.statusNonNull(); + var condition = Condition.builder() + .type(PluginState.FAILED.toString()) + .reason("DependentsExist") + .message( + "The plugin has dependents %s, please delete them first." + .formatted(dependents.stream().map(PluginWrapper::getPluginId).toList()) + ) + .status(ConditionStatus.FALSE) + .lastTransitionTime(clock.instant()) + .build(); + nullSafeConditions(status).addAndEvictFIFO(condition); + status.setPhase(Plugin.Phase.FAILED); + return false; + } + private void syncPluginState(Plugin plugin) { var pluginName = plugin.getMetadata().getName(); var p = pluginManager.getPlugin(pluginName); @@ -191,32 +233,87 @@ public class PluginReconciler implements Reconciler { } } - private void startPlugin(Plugin plugin) { + private void enablePlugin(Plugin plugin) { // start the plugin var pluginName = plugin.getMetadata().getName(); - var wrapper = pluginManager.getPlugin(pluginName); + log.info("Starting plugin {}", pluginName); plugin.statusNonNull().setPhase(Plugin.Phase.STARTING); - if (!PluginState.STARTED.equals(wrapper.getPluginState())) { - var pluginState = pluginManager.startPlugin(pluginName); - if (!PluginState.STARTED.equals(pluginState)) { - throw new IllegalStateException("Failed to start plugin " + pluginName); - } - plugin.statusNonNull().setLastStartTime(clock.instant()); - var condition = Condition.builder() - .type(PluginState.STARTED.toString()) - .reason(PluginState.STARTED.toString()) - .message("Started successfully") - .lastTransitionTime(clock.instant()) - .status(ConditionStatus.TRUE) - .build(); - nullSafeConditions(plugin.statusNonNull()).addAndEvictFIFO(condition); + var pluginState = pluginManager.startPlugin(pluginName); + if (!PluginState.STARTED.equals(pluginState)) { + throw new IllegalStateException("Failed to start plugin " + pluginName); } + + var dependents = getAndRemoveDependents(plugin); + log.info("Starting dependents {} for plugin {}", dependents, pluginName); + dependents.stream() + .sorted(Comparator.reverseOrder()) + .forEach(dependent -> { + if (pluginManager.getPlugin(dependent) != null) { + pluginManager.startPlugin(dependent); + } + }); + log.info("Started dependents {} for plugin {}", dependents, pluginName); + + plugin.statusNonNull().setLastStartTime(clock.instant()); + var condition = Condition.builder() + .type(PluginState.STARTED.toString()) + .reason(PluginState.STARTED.toString()) + .message("Started successfully") + .lastTransitionTime(clock.instant()) + .status(ConditionStatus.TRUE) + .build(); + nullSafeConditions(plugin.statusNonNull()).addAndEvictFIFO(condition); plugin.statusNonNull().setPhase(Plugin.Phase.STARTED); + + log.info("Started plugin {}", pluginName); } - private void stopAndDisablePlugin(Plugin plugin) { + private List getAndRemoveDependents(Plugin plugin) { + var pluginName = plugin.getMetadata().getName(); + var annotations = plugin.getMetadata().getAnnotations(); + if (annotations == null) { + return List.of(); + } + var dependentsAnno = annotations.remove(DEPENDENTS_ANNO_KEY); + List dependents = List.of(); + if (StringUtils.isNotBlank(dependentsAnno)) { + try { + dependents = JsonUtils.jsonToObject(dependentsAnno, new TypeReference<>() { + }); + } catch (JsonParseException ignored) { + log.error("Failed to parse dependents annotation {} for plugin {}", + dependentsAnno, pluginName); + // Keep going to start the plugin. + } + } + return dependents; + } + + private void setDependents(Plugin plugin) { + var pluginName = plugin.getMetadata().getName(); + var annotations = plugin.getMetadata().getAnnotations(); + if (annotations == null) { + annotations = new HashMap<>(); + plugin.getMetadata().setAnnotations(annotations); + } + if (!annotations.containsKey(DEPENDENTS_ANNO_KEY)) { + // get dependents + var dependents = pluginManager.getDependents(pluginName) + .stream() + .filter(pluginWrapper -> + Objects.equals(PluginState.STARTED, pluginWrapper.getPluginState()) + ) + .map(PluginWrapper::getPluginId) + .toList(); + + annotations.put(DEPENDENTS_ANNO_KEY, JsonUtils.objectToJson(dependents)); + } + } + + private void disablePlugin(Plugin plugin) { var pluginName = plugin.getMetadata().getName(); if (pluginManager.getPlugin(pluginName) != null) { + setDependents(plugin); pluginManager.disablePlugin(pluginName); } plugin.statusNonNull().setPhase(Plugin.Phase.DISABLED); @@ -284,29 +381,29 @@ public class PluginReconciler implements Reconciler { } private void loadOrReload(Plugin plugin) { - // TODO Try to check dependencies before. var pluginName = plugin.getMetadata().getName(); - try { - var p = pluginManager.getPlugin(pluginName); - var requestToReload = requestToReload(plugin); - if (requestToReload) { - log.info("Unloading plugin {}", pluginName); - if (p != null) { - pluginManager.unloadPlugin(pluginName); - } - } - if (p == null || requestToReload) { - log.info("Loading plugin {}", pluginName); + var p = pluginManager.getPlugin(pluginName); + var requestToReload = requestToReload(plugin); + if (requestToReload) { + if (p != null) { var loadLocation = plugin.getStatus().getLoadLocation(); - pluginManager.loadPlugin(Paths.get(loadLocation)); - log.info("Loaded plugin {}", pluginName); + setDependents(plugin); + var unloaded = pluginManager.reloadPlugin(pluginName, Paths.get(loadLocation)); + if (!unloaded) { + throw new PluginRuntimeException("Failed to reload plugin " + pluginName); + } + p = pluginManager.getPlugin(pluginName); } - } catch (Throwable t) { - // unload the plugin - if (pluginManager.getPlugin(pluginName) != null) { - pluginManager.unloadPlugin(pluginName); - } - throw t; + } + if (p != null && pluginManager.getUnresolvedPlugins().contains(p)) { + pluginManager.unloadPlugin(pluginName); + p = null; + } + if (p == null) { + var loadLocation = plugin.getStatus().getLoadLocation(); + log.info("Loading plugin {} from {}", pluginName, loadLocation); + pluginManager.loadPlugin(Paths.get(loadLocation)); + log.info("Loaded plugin {} from {}", pluginName, loadLocation); } } @@ -480,4 +577,38 @@ public class PluginReconciler implements Reconciler { return pluginName + "-system-generated-reverse-proxy"; } + public class PluginStartedListener implements PluginStateListener { + + @Override + public void pluginStateChanged(PluginStateEvent event) { + if (PluginState.STARTED.equals(event.getPluginState())) { + var pluginId = event.getPlugin().getPluginId(); + client.fetch(Plugin.class, pluginId) + .ifPresent(plugin -> { + if (!Objects.equals(true, plugin.getSpec().getEnabled())) { + plugin.getSpec().setEnabled(true); + client.update(plugin); + } + }); + } + } + } + + public class PluginStoppedListener implements PluginStateListener { + + @Override + public void pluginStateChanged(PluginStateEvent event) { + if (PluginState.STOPPED.equals(event.getPluginState())) { + var pluginId = event.getPlugin().getPluginId(); + client.fetch(Plugin.class, pluginId) + .ifPresent(plugin -> { + if (!requestToReload(plugin) + && Objects.equals(true, plugin.getSpec().getEnabled())) { + plugin.getSpec().setEnabled(false); + client.update(plugin); + } + }); + } + } + } } diff --git a/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java b/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java index 6244f0fd2..ab7b8fcc1 100644 --- a/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java +++ b/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java @@ -2,7 +2,11 @@ package run.halo.app.plugin; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Comparator; import java.util.List; +import java.util.Objects; +import java.util.Stack; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.pf4j.CompoundPluginLoader; @@ -18,6 +22,7 @@ import org.pf4j.PluginDescriptorFinder; import org.pf4j.PluginFactory; import org.pf4j.PluginLoader; import org.pf4j.PluginRepository; +import org.pf4j.PluginRuntimeException; import org.pf4j.PluginStatusProvider; import org.pf4j.PluginWrapper; import org.springframework.context.ApplicationContext; @@ -157,4 +162,50 @@ public class HaloPluginManager extends DefaultPluginManager implements SpringPlu return sharedContext.get(); } + public boolean reloadPlugin(String pluginId, Path loadLocation) { + log.info("Reloading plugin {} from {}", pluginId, loadLocation); + + var dependents = getDependents(pluginId); + dependents.forEach(plugin -> unloadPlugin(plugin.getPluginId(), false)); + + var unloaded = unloadPlugin(pluginId, false); + if (!unloaded) { + return false; + } + + // load the root plugin + var loadedPluginId = loadPlugin(loadLocation); + + if (!Objects.equals(pluginId, loadedPluginId)) { + throw new PluginRuntimeException(""" + The plugin {} is reloaded successfully, \ + but the plugin id is different from the original one. + """); + } + + // load all dependents with reverse order + dependents.stream() + .map(PluginWrapper::getPluginPath) + .sorted(Comparator.reverseOrder()) + .forEach(this::loadPlugin); + + log.info("Reloaded plugin {} from {}", loadedPluginId, loadLocation); + return true; + } + + @Override + public List getDependents(String pluginId) { + var dependents = new ArrayList(); + var stack = new Stack(); + dependencyResolver.getDependents(pluginId).forEach(stack::push); + while (!stack.isEmpty()) { + var dependent = stack.pop(); + var pluginWrapper = getPlugin(dependent); + if (pluginWrapper != null) { + dependents.add(pluginWrapper); + dependencyResolver.getDependents(dependent).forEach(stack::push); + } + } + return dependents; + } } diff --git a/application/src/main/java/run/halo/app/plugin/SpringPluginManager.java b/application/src/main/java/run/halo/app/plugin/SpringPluginManager.java index c730a3e66..b2fb52591 100644 --- a/application/src/main/java/run/halo/app/plugin/SpringPluginManager.java +++ b/application/src/main/java/run/halo/app/plugin/SpringPluginManager.java @@ -1,6 +1,9 @@ package run.halo.app.plugin; +import java.nio.file.Path; +import java.util.List; import org.pf4j.PluginManager; +import org.pf4j.PluginWrapper; import org.springframework.context.ApplicationContext; public interface SpringPluginManager extends PluginManager { @@ -9,4 +12,21 @@ public interface SpringPluginManager extends PluginManager { ApplicationContext getSharedContext(); + /** + * Reload the plugin and the plugins that depend on it. + * + * @param pluginId plugin id + * @param loadLocation new load location + * @return true if reload successfully, otherwise false + */ + boolean reloadPlugin(String pluginId, Path loadLocation); + + /** + * Get all dependents recursively. + * + * @param pluginId plugin id + * @return a list of plugin wrapper. The order of the list is from the farthest dependent to + * the nearest dependent. + */ + List getDependents(String pluginId); } diff --git a/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java b/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java index 0e70ba477..0d892daa8 100644 --- a/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java +++ b/application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java @@ -7,7 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doThrow; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -40,7 +40,6 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.pf4j.PluginManager; import org.pf4j.PluginState; import org.pf4j.PluginWrapper; import org.pf4j.RuntimeMode; @@ -56,6 +55,7 @@ import run.halo.app.extension.controller.Reconciler.Request; import run.halo.app.extension.controller.RequeueException; import run.halo.app.infra.ConditionStatus; import run.halo.app.plugin.PluginProperties; +import run.halo.app.plugin.SpringPluginManager; /** * Tests for {@link PluginReconciler}. @@ -67,7 +67,7 @@ import run.halo.app.plugin.PluginProperties; class PluginReconcilerTest { @Mock - PluginManager pluginManager; + SpringPluginManager pluginManager; @Mock ExtensionClient client; @@ -185,41 +185,6 @@ class PluginReconcilerTest { mode for plugin fake-plugin.""", gotException.getMessage()); } - @Test - void shouldUnloadIfFailedToLoad() { - var fakePlugin = createPlugin(name, plugin -> { - var spec = plugin.getSpec(); - spec.setVersion("1.2.3"); - spec.setLogo("fake-logo.svg"); - spec.setEnabled(true); - }); - - when(client.fetch(Plugin.class, name)).thenReturn(Optional.of(fakePlugin)); - when(pluginManager.getPluginsRoots()).thenReturn(List.of(tempPath)); - when(pluginManager.getPlugin(name)) - // before loading - .thenReturn(null) - .thenReturn(mock(PluginWrapper.class)) - ; - var expectException = mock(RuntimeException.class); - when(expectException.getMessage()).thenReturn("Something went wrong."); - doThrow(expectException).when(pluginManager).loadPlugin(any(Path.class)); - - var gotException = assertThrows(RuntimeException.class, - () -> reconciler.reconcile(new Request(name))); - - assertEquals(expectException, gotException); - var condition = fakePlugin.getStatus().getConditions().peek(); - assertEquals("FAILED", condition.getType()); - assertEquals(ConditionStatus.FALSE, condition.getStatus()); - assertEquals("UnexpectedState", condition.getReason()); - assertEquals(expectException.getMessage(), condition.getMessage()); - assertEquals(clock.instant(), condition.getLastTransitionTime()); - verify(pluginManager, times(3)).getPlugin(name); - verify(pluginManager).loadPlugin(any(Path.class)); - verify(pluginManager).unloadPlugin(name); - } - @Test void shouldReloadIfReloadAnnotationPresent() { var fakePlugin = createPlugin(name, plugin -> { @@ -233,15 +198,14 @@ class PluginReconcilerTest { when(client.fetch(Plugin.class, name)).thenReturn(Optional.of(fakePlugin)); when(pluginManager.getPluginsRoots()).thenReturn(List.of(tempPath)); when(pluginManager.getPlugin(name)).thenReturn(mock(PluginWrapper.class)); - when(pluginManager.unloadPlugin(name)).thenReturn(true); + when(pluginManager.reloadPlugin(eq(name), any(Path.class))).thenReturn(true); when(pluginManager.startPlugin(name)).thenReturn(PluginState.STARTED); var result = reconciler.reconcile(new Request(name)); assertFalse(result.reEnqueue()); - verify(pluginManager).unloadPlugin(name); var loadLocation = Paths.get(fakePlugin.getStatus().getLoadLocation()); - verify(pluginManager).loadPlugin(loadLocation); + verify(pluginManager).reloadPlugin(name, loadLocation); } @Test @@ -262,11 +226,7 @@ class PluginReconcilerTest { .thenReturn(null) // get setting extension .thenReturn(mockPluginWrapperForSetting()) - .thenReturn(mockPluginWrapperForStaticResources()) - // before starting - .thenReturn(mockPluginWrapper(PluginState.RESOLVED)) - // sync plugin state - .thenReturn(mockPluginWrapper(PluginState.STARTED)); + .thenReturn(mockPluginWrapperForStaticResources()); when(pluginManager.startPlugin(name)).thenReturn(PluginState.FAILED); var e = assertThrows(IllegalStateException.class, @@ -293,8 +253,6 @@ class PluginReconcilerTest { // get setting extension .thenReturn(mockPluginWrapperForSetting()) .thenReturn(mockPluginWrapperForStaticResources()) - // before starting - .thenReturn(mockPluginWrapper(PluginState.RESOLVED)) // sync plugin state .thenReturn(mockPluginWrapper(PluginState.STARTED)); when(pluginManager.startPlugin(name)).thenReturn(PluginState.STARTED); @@ -325,7 +283,7 @@ class PluginReconcilerTest { 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));