Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refresh the plugin wrapper when starting the plugin #4023

Merged
merged 3 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions api/src/main/java/run/halo/app/plugin/BasePlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import lombok.extern.slf4j.Slf4j;
import org.pf4j.Plugin;
import org.pf4j.PluginManager;
import org.pf4j.PluginWrapper;

/**
Expand All @@ -15,12 +14,12 @@
@Slf4j
public class BasePlugin extends Plugin {

@Deprecated
public BasePlugin(PluginWrapper wrapper) {
super(wrapper);
log.info("Initialized plugin {}", wrapper.getPluginId());
}

private PluginManager getPluginManager() {
return getWrapper().getPluginManager();
public BasePlugin() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,7 @@ void stopAction(String name) {

void stateTransition(String name, Function<PluginState, Boolean> stateAction,
PluginState desiredState) {
PluginWrapper pluginWrapper = getPluginWrapper(name);
PluginState currentState = pluginWrapper.getPluginState();
PluginState currentState = getPluginWrapper(name).getPluginState();
int maxRetries = PluginState.values().length;
for (int i = 0; i < maxRetries && currentState != desiredState; i++) {
try {
Expand All @@ -303,7 +302,7 @@ void stateTransition(String name, Function<PluginState, Boolean> stateAction,
break;
}
// update current state
currentState = pluginWrapper.getPluginState();
currentState = getPluginWrapper(name).getPluginState();
} catch (Throwable e) {
persistenceFailureStatus(name, e);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public Plugin create(PluginWrapper pluginWrapper) {
"No bean named 'basePlugin' found in the context create default instance");
DefaultListableBeanFactory beanFactory =
context.getDefaultListableBeanFactory();
BasePlugin pluginInstance = new BasePlugin(pluginWrapper);
BasePlugin pluginInstance = new BasePlugin();
beanFactory.registerSingleton(Plugin.class.getName(), pluginInstance);
return pluginInstance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ public boolean validatePluginVersion(PluginWrapper pluginWrapper) {
private PluginState doStartPlugin(String pluginId) {
checkPluginId(pluginId);

PluginWrapper pluginWrapper = getPlugin(pluginId);
// refresh plugin to ensure cache object of PluginWrapper.plugin is up-to-date
// see gh-4016 to know why we need this
// TODO if has a better way to do this?
PluginWrapper pluginWrapper = refreshPluginWrapper(pluginId);

checkExtensionFinderReady(pluginWrapper);

PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor();
Expand Down Expand Up @@ -423,6 +427,37 @@ private void removePluginComponentsCache(String pluginId) {
}
}

/**
* <p>Refresh plugin wrapper by plugin name.</p>
*
* <p>It will be create a new plugin wrapper and replace old plugin wrapper to clean
* {@link PluginWrapper#getPlugin()} cache object.</p>
*
* @param pluginName plugin name
* @return refreshed plugin wrapper instance, plugin cache object will be null
* @throws IllegalArgumentException if plugin not found
*/
protected synchronized PluginWrapper refreshPluginWrapper(String pluginName) {
checkPluginId(pluginName);
// get old plugin wrapper
PluginWrapper pluginWrapper = getPlugin(pluginName);
// create new plugin wrapper to replace old plugin wrapper
PluginWrapper refreshed = copyPluginWrapper(pluginWrapper);
this.plugins.put(pluginName, refreshed);
return refreshed;
}

@NonNull
PluginWrapper copyPluginWrapper(@NonNull PluginWrapper pluginWrapper) {
PluginWrapper refreshed =
createPluginWrapper(pluginWrapper.getDescriptor(), pluginWrapper.getPluginPath(),
pluginWrapper.getPluginClassLoader());
refreshed.setPluginFactory(getPluginFactory());
refreshed.setPluginState(pluginWrapper.getPluginState());
refreshed.setFailedException(pluginWrapper.getFailedException());
return refreshed;
}

@Override
public void destroy() throws Exception {
stopPlugins();
Expand Down