mirror of https://github.com/halo-dev/halo
Fix the problem where extension point might not be obtained when the plugin was started (#6006)
#### What type of PR is this? /kind bug /area plugin /area core #### What this PR does / why we need it: This PR refactors SpringComponentsFinder to prevent unexpected cleanup, which might lead to not be able to obtain plugin extension when the plugin was started. The real reason is that entries is initialized by AttachmentReconciler during initialization of some plugins. Please note the problem cannot be reproduced stably. Refs: -pull/6011/heade3125f2998/pf4j/src/main/java/org/pf4j/AbstractExtensionFinder.java (L229-L249)
-e3125f2998/pf4j/src/main/java/org/pf4j/AbstractExtensionFinder.java (L312-L316)
#### Which issue(s) this PR fixes: Fixes https://github.com/halo-dev/halo/issues/5999 #### Does this PR introduce a user-facing change? ```release-note 修复插件启动成功但仍然可能无法获取扩展点导致页面无法访问的问题 ```
parent
608f2bbca3
commit
08ff7e6e00
|
@ -5,19 +5,17 @@ import java.io.InputStream;
|
||||||
import java.io.InputStreamReader;
|
import java.io.InputStreamReader;
|
||||||
import java.io.Reader;
|
import java.io.Reader;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.util.Collections;
|
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.LinkedHashMap;
|
|
||||||
import java.util.List;
|
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.concurrent.locks.StampedLock;
|
import java.util.concurrent.ConcurrentHashMap;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.pf4j.AbstractExtensionFinder;
|
import org.pf4j.AbstractExtensionFinder;
|
||||||
import org.pf4j.PluginManager;
|
import org.pf4j.PluginManager;
|
||||||
|
import org.pf4j.PluginState;
|
||||||
|
import org.pf4j.PluginStateEvent;
|
||||||
import org.pf4j.PluginWrapper;
|
import org.pf4j.PluginWrapper;
|
||||||
import org.pf4j.processor.ExtensionStorage;
|
import org.pf4j.processor.ExtensionStorage;
|
||||||
import org.springframework.util.Assert;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* <p>The spring component finder. it will read {@code META-INF/plugin-components.idx} file in
|
* <p>The spring component finder. it will read {@code META-INF/plugin-components.idx} file in
|
||||||
|
@ -31,53 +29,41 @@ import org.springframework.util.Assert;
|
||||||
@Slf4j
|
@Slf4j
|
||||||
public class SpringComponentsFinder extends AbstractExtensionFinder {
|
public class SpringComponentsFinder extends AbstractExtensionFinder {
|
||||||
public static final String EXTENSIONS_RESOURCE = "META-INF/plugin-components.idx";
|
public static final String EXTENSIONS_RESOURCE = "META-INF/plugin-components.idx";
|
||||||
private final StampedLock entryStampedLock = new StampedLock();
|
|
||||||
|
|
||||||
public SpringComponentsFinder(PluginManager pluginManager) {
|
public SpringComponentsFinder(PluginManager pluginManager) {
|
||||||
super(pluginManager);
|
super(pluginManager);
|
||||||
|
entries = new ConcurrentHashMap<>();
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Map<String, Set<String>> readClasspathStorages() {
|
public Map<String, Set<String>> readClasspathStorages() {
|
||||||
log.debug("Reading components storages from classpath");
|
throw new UnsupportedOperationException();
|
||||||
return Collections.emptyMap();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Map<String, Set<String>> readPluginsStorages() {
|
public Map<String, Set<String>> readPluginsStorages() {
|
||||||
// We have to copy the source code from `org.pf4j.LegacyExtensionFinder.readPluginsStorages`
|
throw new UnsupportedOperationException();
|
||||||
// because we have to adapt to the new extensions resource location
|
}
|
||||||
// `META-INF/plugin-components.idx`.
|
|
||||||
log.debug("Reading components storages from plugins");
|
|
||||||
Map<String, Set<String>> result = new LinkedHashMap<>();
|
|
||||||
|
|
||||||
List<PluginWrapper> plugins = pluginManager.getPlugins();
|
private Set<String> readPluginStorage(PluginWrapper pluginWrapper) {
|
||||||
for (PluginWrapper plugin : plugins) {
|
var pluginId = pluginWrapper.getPluginId();
|
||||||
String pluginId = plugin.getDescriptor().getPluginId();
|
|
||||||
log.debug("Reading extensions storage from plugin '{}'", pluginId);
|
log.debug("Reading extensions storage from plugin '{}'", pluginId);
|
||||||
Set<String> bucket = new HashSet<>();
|
var bucket = new HashSet<String>();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
log.debug("Read '{}'", EXTENSIONS_RESOURCE);
|
log.debug("Read '{}'", EXTENSIONS_RESOURCE);
|
||||||
ClassLoader pluginClassLoader = plugin.getPluginClassLoader();
|
var classLoader = pluginWrapper.getPluginClassLoader();
|
||||||
try (var resourceStream =
|
try (var resourceStream = classLoader.getResourceAsStream(EXTENSIONS_RESOURCE)) {
|
||||||
pluginClassLoader.getResourceAsStream(EXTENSIONS_RESOURCE)) {
|
|
||||||
if (resourceStream == null) {
|
if (resourceStream == null) {
|
||||||
log.debug("Cannot find '{}'", EXTENSIONS_RESOURCE);
|
log.debug("Cannot find '{}'", EXTENSIONS_RESOURCE);
|
||||||
} else {
|
} else {
|
||||||
collectExtensions(resourceStream, bucket);
|
collectExtensions(resourceStream, bucket);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
debugExtensions(bucket);
|
debugExtensions(bucket);
|
||||||
|
|
||||||
result.put(pluginId, bucket);
|
|
||||||
} catch (IOException e) {
|
} catch (IOException e) {
|
||||||
log.error("Failed to read components from " + EXTENSIONS_RESOURCE, e);
|
log.error("Failed to read components from " + EXTENSIONS_RESOURCE, e);
|
||||||
}
|
}
|
||||||
}
|
return bucket;
|
||||||
|
|
||||||
return result;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private void collectExtensions(InputStream inputStream, Set<String> bucket) throws IOException {
|
private void collectExtensions(InputStream inputStream, Set<String> bucket) throws IOException {
|
||||||
|
@ -86,20 +72,15 @@ public class SpringComponentsFinder extends AbstractExtensionFinder {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
protected boolean containsComponentsStorage(String pluginId) {
|
@Override
|
||||||
Assert.notNull(pluginId, "The pluginId cannot be null");
|
public void pluginStateChanged(PluginStateEvent event) {
|
||||||
long stamp = entryStampedLock.tryOptimisticRead();
|
var pluginState = event.getPluginState();
|
||||||
boolean contains = super.entries != null && super.entries.containsKey(pluginId);
|
String pluginId = event.getPlugin().getPluginId();
|
||||||
if (!entryStampedLock.validate(stamp)) {
|
if (pluginState == PluginState.UNLOADED) {
|
||||||
stamp = entryStampedLock.readLock();
|
entries.remove(pluginId);
|
||||||
try {
|
} else if (pluginState == PluginState.CREATED || pluginState == PluginState.RESOLVED) {
|
||||||
return super.entries != null && entries.containsKey(pluginId);
|
entries.computeIfAbsent(pluginId, id -> readPluginStorage(event.getPlugin()));
|
||||||
} finally {
|
|
||||||
entryStampedLock.unlockRead(stamp);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return contains;
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,18 +1,25 @@
|
||||||
package run.halo.app.plugin;
|
package run.halo.app.plugin;
|
||||||
|
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
import static org.mockito.Mockito.mock;
|
||||||
|
import static org.mockito.Mockito.when;
|
||||||
|
|
||||||
import java.io.File;
|
|
||||||
import java.io.FileNotFoundException;
|
import java.io.FileNotFoundException;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import java.net.URL;
|
||||||
|
import java.net.URLClassLoader;
|
||||||
|
import java.util.Set;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.extension.ExtendWith;
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
import org.mockito.InjectMocks;
|
import org.mockito.InjectMocks;
|
||||||
import org.mockito.Mock;
|
import org.mockito.Mock;
|
||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
import org.pf4j.PluginClassLoader;
|
import org.pf4j.PluginManager;
|
||||||
|
import org.pf4j.PluginState;
|
||||||
|
import org.pf4j.PluginStateEvent;
|
||||||
import org.pf4j.PluginWrapper;
|
import org.pf4j.PluginWrapper;
|
||||||
import org.springframework.util.ResourceUtils;
|
import org.springframework.util.ResourceUtils;
|
||||||
|
|
||||||
|
@ -25,29 +32,64 @@ import org.springframework.util.ResourceUtils;
|
||||||
@ExtendWith(MockitoExtension.class)
|
@ExtendWith(MockitoExtension.class)
|
||||||
class SpringComponentsFinderTest {
|
class SpringComponentsFinderTest {
|
||||||
|
|
||||||
private File testFile;
|
|
||||||
@Mock
|
@Mock
|
||||||
private HaloPluginManager pluginManager;
|
private PluginManager pluginManager;
|
||||||
@Mock
|
|
||||||
private PluginWrapper pluginWrapper;
|
|
||||||
@Mock
|
|
||||||
private PluginClassLoader pluginClassLoader;
|
|
||||||
|
|
||||||
@InjectMocks
|
@InjectMocks
|
||||||
private SpringComponentsFinder springComponentsFinder;
|
private SpringComponentsFinder finder;
|
||||||
|
|
||||||
@BeforeEach
|
@Test
|
||||||
void setUp() throws FileNotFoundException {
|
void shouldNotInvokeReadClasspathStorages() {
|
||||||
testFile = ResourceUtils.getFile("classpath:plugin/test-plugin-components.idx");
|
assertThrows(UnsupportedOperationException.class,
|
||||||
|
() -> finder.readClasspathStorages()
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void containsPlugin() {
|
void shouldNotInvokeReadPluginsStorages() {
|
||||||
boolean exist = springComponentsFinder.containsComponentsStorage("NotExist");
|
assertThrows(UnsupportedOperationException.class,
|
||||||
assertThat(exist).isFalse();
|
() -> finder.readPluginsStorages()
|
||||||
|
);
|
||||||
assertThatThrownBy(() -> springComponentsFinder.containsComponentsStorage(null))
|
|
||||||
.hasMessage("The pluginId cannot be null");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldPutEntryIfPluginCreated() throws FileNotFoundException {
|
||||||
|
var pluginWrapper = mockPluginWrapper();
|
||||||
|
when(pluginWrapper.getPluginState()).thenReturn(PluginState.CREATED);
|
||||||
|
|
||||||
|
var event = new PluginStateEvent(pluginManager, pluginWrapper, null);
|
||||||
|
finder.pluginStateChanged(event);
|
||||||
|
|
||||||
|
var classNames = finder.findClassNames("fake-plugin");
|
||||||
|
assertEquals(Set.of("run.halo.fake.FakePlugin"), classNames);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldRemoveEntryIfPluginUnloaded() throws FileNotFoundException {
|
||||||
|
var pluginWrapper = mockPluginWrapper();
|
||||||
|
when(pluginWrapper.getPluginState()).thenReturn(PluginState.CREATED);
|
||||||
|
|
||||||
|
var event = new PluginStateEvent(pluginManager, pluginWrapper, null);
|
||||||
|
finder.pluginStateChanged(event);
|
||||||
|
|
||||||
|
var classNames = finder.findClassNames("fake-plugin");
|
||||||
|
assertFalse(classNames.isEmpty());
|
||||||
|
|
||||||
|
when(pluginWrapper.getPluginState()).thenReturn(PluginState.UNLOADED);
|
||||||
|
event = new PluginStateEvent(pluginManager, pluginWrapper, null);
|
||||||
|
finder.pluginStateChanged(event);
|
||||||
|
|
||||||
|
classNames = finder.findClassNames("fake-plugin");
|
||||||
|
assertTrue(classNames.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
private PluginWrapper mockPluginWrapper() throws FileNotFoundException {
|
||||||
|
var pluginWrapper = mock(PluginWrapper.class);
|
||||||
|
when(pluginWrapper.getPluginId()).thenReturn("fake-plugin");
|
||||||
|
|
||||||
|
var pluginRootUrl = ResourceUtils.getURL("classpath:plugin/plugin-for-finder/");
|
||||||
|
var classLoader = new URLClassLoader(new URL[] {pluginRootUrl});
|
||||||
|
when(pluginWrapper.getPluginClassLoader()).thenReturn(classLoader);
|
||||||
|
return pluginWrapper;
|
||||||
|
}
|
||||||
}
|
}
|
|
@ -0,0 +1,2 @@
|
||||||
|
# Generated by Halo
|
||||||
|
run.halo.fake.FakePlugin
|
|
@ -1,2 +0,0 @@
|
||||||
# Generated by Halo
|
|
||||||
run.halo.links.LinkPlugin
|
|
Loading…
Reference in New Issue