Browse Source

Improve #292

pull/320/head
Decebal Suiu 6 years ago
parent
commit
8a2674c539
  1. 41
      pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
  2. 4
      pf4j/src/main/java/org/pf4j/BasePluginRepository.java
  3. 2
      pf4j/src/main/java/org/pf4j/CompoundPluginRepository.java
  4. 2
      pf4j/src/main/java/org/pf4j/DefaultPluginRepository.java
  5. 20
      pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java
  6. 12
      pf4j/src/main/java/org/pf4j/PluginManager.java
  7. 2
      pf4j/src/main/java/org/pf4j/PluginRepository.java
  8. 6
      pf4j/src/main/java/org/pf4j/PluginStatusProvider.java
  9. 2
      pf4j/src/test/java/org/pf4j/DefaultPluginRepositoryTest.java
  10. 21
      pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java
  11. 3
      pf4j/src/test/java/org/pf4j/JarPluginRepositoryTest.java

41
pf4j/src/main/java/org/pf4j/AbstractPluginManager.java

@ -232,11 +232,11 @@ public abstract class AbstractPluginManager implements PluginManager {
* Unload the specified plugin and it's dependents. * Unload the specified plugin and it's dependents.
*/ */
@Override @Override
public boolean unloadPlugin(String pluginId) { public boolean unloadPlugin(String pluginId) throws PluginException {
return unloadPlugin(pluginId, true); return unloadPlugin(pluginId, true);
} }
private boolean unloadPlugin(String pluginId, boolean unloadDependents) { private boolean unloadPlugin(String pluginId, boolean unloadDependents) throws PluginException {
try { try {
if (unloadDependents) { if (unloadDependents) {
List<String> dependents = dependencyResolver.getDependents(pluginId); List<String> dependents = dependencyResolver.getDependents(pluginId);
@ -269,7 +269,7 @@ public abstract class AbstractPluginManager implements PluginManager {
try { try {
((Closeable) classLoader).close(); ((Closeable) classLoader).close();
} catch (IOException e) { } catch (IOException e) {
log.error("Cannot close classloader", e); throw new PluginException("Cannot close classloader", e);
} }
} }
} }
@ -283,7 +283,7 @@ public abstract class AbstractPluginManager implements PluginManager {
} }
@Override @Override
public boolean deletePlugin(String pluginId) { public boolean deletePlugin(String pluginId) throws PluginException {
checkPluginId(pluginId); checkPluginId(pluginId);
PluginWrapper pluginWrapper = getPlugin(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId);
@ -304,12 +304,7 @@ public abstract class AbstractPluginManager implements PluginManager {
} }
// notify the plugin as it's deleted // notify the plugin as it's deleted
try {
plugin.delete(); plugin.delete();
} catch (PluginException e) {
log.error(e.getMessage(), e);
return false;
}
Path pluginPath = pluginWrapper.getPluginPath(); Path pluginPath = pluginWrapper.getPluginPath();
@ -342,7 +337,7 @@ public abstract class AbstractPluginManager implements PluginManager {
* Start the specified plugin and its dependencies. * Start the specified plugin and its dependencies.
*/ */
@Override @Override
public PluginState startPlugin(String pluginId) { public PluginState startPlugin(String pluginId) throws PluginException {
checkPluginId(pluginId); checkPluginId(pluginId);
PluginWrapper pluginWrapper = getPlugin(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId);
@ -369,16 +364,12 @@ public abstract class AbstractPluginManager implements PluginManager {
startPlugin(dependency.getPluginId()); startPlugin(dependency.getPluginId());
} }
try {
log.info("Start plugin '{}'", getPluginLabel(pluginDescriptor)); log.info("Start plugin '{}'", getPluginLabel(pluginDescriptor));
pluginWrapper.getPlugin().start(); pluginWrapper.getPlugin().start();
pluginWrapper.setPluginState(PluginState.STARTED); pluginWrapper.setPluginState(PluginState.STARTED);
startedPlugins.add(pluginWrapper); startedPlugins.add(pluginWrapper);
firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState)); firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
} catch (PluginException e) {
log.error(e.getMessage(), e);
}
return pluginWrapper.getPluginState(); return pluginWrapper.getPluginState();
} }
@ -413,11 +404,11 @@ public abstract class AbstractPluginManager implements PluginManager {
* Stop the specified plugin and it's dependents. * Stop the specified plugin and it's dependents.
*/ */
@Override @Override
public PluginState stopPlugin(String pluginId) { public PluginState stopPlugin(String pluginId) throws PluginException {
return stopPlugin(pluginId, true); return stopPlugin(pluginId, true);
} }
private PluginState stopPlugin(String pluginId, boolean stopDependents) { private PluginState stopPlugin(String pluginId, boolean stopDependents) throws PluginException {
checkPluginId(pluginId); checkPluginId(pluginId);
PluginWrapper pluginWrapper = getPlugin(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId);
@ -443,16 +434,12 @@ public abstract class AbstractPluginManager implements PluginManager {
} }
} }
try {
log.info("Stop plugin '{}'", getPluginLabel(pluginDescriptor)); log.info("Stop plugin '{}'", getPluginLabel(pluginDescriptor));
pluginWrapper.getPlugin().stop(); pluginWrapper.getPlugin().stop();
pluginWrapper.setPluginState(PluginState.STOPPED); pluginWrapper.setPluginState(PluginState.STOPPED);
startedPlugins.remove(pluginWrapper); startedPlugins.remove(pluginWrapper);
firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState)); firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
} catch (PluginException e) {
log.error(e.getMessage(), e);
}
return pluginWrapper.getPluginState(); return pluginWrapper.getPluginState();
} }
@ -464,7 +451,7 @@ public abstract class AbstractPluginManager implements PluginManager {
} }
@Override @Override
public boolean disablePlugin(String pluginId) { public boolean disablePlugin(String pluginId) throws PluginException {
checkPluginId(pluginId); checkPluginId(pluginId);
PluginWrapper pluginWrapper = getPlugin(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId);
@ -480,10 +467,7 @@ public abstract class AbstractPluginManager implements PluginManager {
firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, PluginState.STOPPED)); firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, PluginState.STOPPED));
if (!pluginStatusProvider.disablePlugin(pluginId)) { pluginStatusProvider.disablePlugin(pluginId);
return false;
}
log.info("Disabled plugin '{}'", getPluginLabel(pluginDescriptor)); log.info("Disabled plugin '{}'", getPluginLabel(pluginDescriptor));
return true; return true;
@ -493,7 +477,7 @@ public abstract class AbstractPluginManager implements PluginManager {
} }
@Override @Override
public boolean enablePlugin(String pluginId) { public boolean enablePlugin(String pluginId) throws PluginException {
checkPluginId(pluginId); checkPluginId(pluginId);
PluginWrapper pluginWrapper = getPlugin(pluginId); PluginWrapper pluginWrapper = getPlugin(pluginId);
@ -509,9 +493,7 @@ public abstract class AbstractPluginManager implements PluginManager {
return true; return true;
} }
if (!pluginStatusProvider.enablePlugin(pluginId)) { pluginStatusProvider.enablePlugin(pluginId);
return false;
}
pluginWrapper.setPluginState(PluginState.CREATED); pluginWrapper.setPluginState(PluginState.CREATED);
@ -612,7 +594,6 @@ public abstract class AbstractPluginManager implements PluginManager {
return extensionFactory; return extensionFactory;
} }
// TODO remove
public PluginLoader getPluginLoader() { public PluginLoader getPluginLoader() {
return pluginLoader; return pluginLoader;
} }

4
pf4j/src/main/java/org/pf4j/BasePluginRepository.java

@ -86,7 +86,7 @@ public class BasePluginRepository implements PluginRepository {
} }
@Override @Override
public boolean deletePluginPath(Path pluginPath) { public boolean deletePluginPath(Path pluginPath) throws PluginException {
if (!filter.accept(pluginPath.toFile())) { if (!filter.accept(pluginPath.toFile())) {
return false; return false;
} }
@ -97,7 +97,7 @@ public class BasePluginRepository implements PluginRepository {
} catch (NoSuchFileException e) { } catch (NoSuchFileException e) {
return false; // Return false on not found to be compatible with previous API (#135) return false; // Return false on not found to be compatible with previous API (#135)
} catch (IOException e) { } catch (IOException e) {
throw new RuntimeException(e); throw new PluginException(e);
} }
} }

2
pf4j/src/main/java/org/pf4j/CompoundPluginRepository.java

@ -48,7 +48,7 @@ public class CompoundPluginRepository implements PluginRepository {
} }
@Override @Override
public boolean deletePluginPath(Path pluginPath) { public boolean deletePluginPath(Path pluginPath) throws PluginException {
for (PluginRepository repository : repositories) { for (PluginRepository repository : repositories) {
if (repository.deletePluginPath(pluginPath)) { if (repository.deletePluginPath(pluginPath)) {
return true; return true;

2
pf4j/src/main/java/org/pf4j/DefaultPluginRepository.java

@ -54,7 +54,7 @@ public class DefaultPluginRepository extends BasePluginRepository {
} }
@Override @Override
public boolean deletePluginPath(Path pluginPath) { public boolean deletePluginPath(Path pluginPath) throws PluginException {
FileUtils.optimisticDelete(FileUtils.findWithEnding(pluginPath, ".zip", ".ZIP", ".Zip")); FileUtils.optimisticDelete(FileUtils.findWithEnding(pluginPath, ".zip", ".ZIP", ".Zip"));
return super.deletePluginPath(pluginPath); return super.deletePluginPath(pluginPath);
} }

20
pf4j/src/main/java/org/pf4j/DefaultPluginStatusProvider.java

@ -66,31 +66,23 @@ public class DefaultPluginStatusProvider implements PluginStatusProvider {
} }
@Override @Override
public boolean disablePlugin(String pluginId) { public void disablePlugin(String pluginId) throws PluginException {
if (disabledPlugins.add(pluginId)) { disabledPlugins.add(pluginId);
try { try {
FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile()); FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile());
} catch (IOException e) { } catch (IOException e) {
log.error("Failed to disable plugin {}", pluginId, e); throw new PluginException(e);
return false;
} }
} }
return true;
}
@Override @Override
public boolean enablePlugin(String pluginId) { public void enablePlugin(String pluginId) throws PluginException {
if (disabledPlugins.remove(pluginId)) { disabledPlugins.remove(pluginId);
try { try {
FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile()); FileUtils.writeLines(disabledPlugins, pluginsRoot.resolve("disabled.txt").toFile());
} catch (IOException e) { } catch (IOException e) {
log.error("Failed to enable plugin {}", pluginId, e); throw new PluginException(e);
return false;
} }
} }
return true;
}
} }

12
pf4j/src/main/java/org/pf4j/PluginManager.java

@ -85,7 +85,7 @@ public interface PluginManager {
* *
* @return the plugin state * @return the plugin state
*/ */
PluginState startPlugin(String pluginId); PluginState startPlugin(String pluginId) throws PluginException;
/** /**
* Stop all active plugins. * Stop all active plugins.
@ -97,7 +97,7 @@ public interface PluginManager {
* *
* @return the plugin state * @return the plugin state
*/ */
PluginState stopPlugin(String pluginId); PluginState stopPlugin(String pluginId) throws PluginException;
/** /**
* Unload a plugin. * Unload a plugin.
@ -105,7 +105,7 @@ public interface PluginManager {
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if the plugin was unloaded * @return true if the plugin was unloaded
*/ */
boolean unloadPlugin(String pluginId); boolean unloadPlugin(String pluginId) throws PluginException;
/** /**
* Disables a plugin from being loaded. * Disables a plugin from being loaded.
@ -113,7 +113,7 @@ public interface PluginManager {
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if plugin is disabled * @return true if plugin is disabled
*/ */
boolean disablePlugin(String pluginId); boolean disablePlugin(String pluginId) throws PluginException;
/** /**
* Enables a plugin that has previously been disabled. * Enables a plugin that has previously been disabled.
@ -121,7 +121,7 @@ public interface PluginManager {
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if plugin is enabled * @return true if plugin is enabled
*/ */
boolean enablePlugin(String pluginId); boolean enablePlugin(String pluginId) throws PluginException;
/** /**
* Deletes a plugin. * Deletes a plugin.
@ -129,7 +129,7 @@ public interface PluginManager {
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if the plugin was deleted * @return true if the plugin was deleted
*/ */
boolean deletePlugin(String pluginId); boolean deletePlugin(String pluginId) throws PluginException;
ClassLoader getPluginClassLoader(String pluginId); ClassLoader getPluginClassLoader(String pluginId);

2
pf4j/src/main/java/org/pf4j/PluginRepository.java

@ -39,6 +39,6 @@ public interface PluginRepository {
* @param pluginPath the plugin path * @param pluginPath the plugin path
* @return true if deleted * @return true if deleted
*/ */
boolean deletePluginPath(Path pluginPath); boolean deletePluginPath(Path pluginPath) throws PluginException;
} }

6
pf4j/src/main/java/org/pf4j/PluginStatusProvider.java

@ -33,16 +33,14 @@ public interface PluginStatusProvider {
* Disables a plugin from being loaded. * Disables a plugin from being loaded.
* *
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if plugin is disabled
*/ */
boolean disablePlugin(String pluginId); void disablePlugin(String pluginId) throws PluginException;
/** /**
* Enables a plugin that has previously been disabled. * Enables a plugin that has previously been disabled.
* *
* @param pluginId the unique plugin identifier, specified in its metadata * @param pluginId the unique plugin identifier, specified in its metadata
* @return true if plugin is enabled
*/ */
boolean enablePlugin(String pluginId); void enablePlugin(String pluginId) throws PluginException;
} }

2
pf4j/src/test/java/org/pf4j/DefaultPluginRepositoryTest.java

@ -85,7 +85,7 @@ public class DefaultPluginRepositoryTest {
* Test of {@link DefaultPluginRepository#deletePluginPath(Path)} method. * Test of {@link DefaultPluginRepository#deletePluginPath(Path)} method.
*/ */
@Test @Test
public void testDeletePluginPath() { public void testDeletePluginPath() throws PluginException {
PluginRepository repository = new DefaultPluginRepository(pluginsPath, false); PluginRepository repository = new DefaultPluginRepository(pluginsPath, false);
assertTrue(Files.exists(pluginsPath.resolve("plugin-1.zip"))); assertTrue(Files.exists(pluginsPath.resolve("plugin-1.zip")));

21
pf4j/src/test/java/org/pf4j/DefaultPluginStatusProviderTest.java

@ -60,58 +60,59 @@ public class DefaultPluginStatusProviderTest {
} }
@Test @Test
public void testDisablePlugin() throws IOException { public void testDisablePlugin() throws Exception {
createEnabledFile(); createEnabledFile();
createDisabledFile(); createDisabledFile();
PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath); PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
statusProvider.disablePlugin("plugin-1");
assertTrue(statusProvider.disablePlugin("plugin-1"));
assertTrue(statusProvider.isPluginDisabled("plugin-1")); assertTrue(statusProvider.isPluginDisabled("plugin-1"));
assertTrue(statusProvider.isPluginDisabled("plugin-2")); assertTrue(statusProvider.isPluginDisabled("plugin-2"));
assertTrue(statusProvider.isPluginDisabled("plugin-3")); assertTrue(statusProvider.isPluginDisabled("plugin-3"));
} }
@Test @Test
public void testDisablePluginWithEnableEmpty() throws IOException { public void testDisablePluginWithEnableEmpty() throws Exception {
createDisabledFile(); createDisabledFile();
PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath); PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
statusProvider.disablePlugin("plugin-1");
assertTrue(statusProvider.disablePlugin("plugin-1"));
assertTrue(statusProvider.isPluginDisabled("plugin-1")); assertTrue(statusProvider.isPluginDisabled("plugin-1"));
assertTrue(statusProvider.isPluginDisabled("plugin-2")); assertTrue(statusProvider.isPluginDisabled("plugin-2"));
assertFalse(statusProvider.isPluginDisabled("plugin-3")); assertFalse(statusProvider.isPluginDisabled("plugin-3"));
} }
@Test @Test
public void testEnablePlugin() throws IOException { public void testEnablePlugin() throws Exception {
createEnabledFile(); createEnabledFile();
PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath); PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
statusProvider.enablePlugin("plugin-2");
assertTrue(statusProvider.enablePlugin("plugin-2"));
assertFalse(statusProvider.isPluginDisabled("plugin-1")); assertFalse(statusProvider.isPluginDisabled("plugin-1"));
assertFalse(statusProvider.isPluginDisabled("plugin-2")); assertFalse(statusProvider.isPluginDisabled("plugin-2"));
assertTrue(statusProvider.isPluginDisabled("plugin-3")); assertTrue(statusProvider.isPluginDisabled("plugin-3"));
} }
@Test @Test
public void testEnablePluginWithEnableEmpty() { public void testEnablePluginWithEnableEmpty() throws Exception{
PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath); PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
statusProvider.enablePlugin("plugin-2");
assertTrue(statusProvider.enablePlugin("plugin-2"));
assertFalse(statusProvider.isPluginDisabled("plugin-1")); assertFalse(statusProvider.isPluginDisabled("plugin-1"));
assertFalse(statusProvider.isPluginDisabled("plugin-2")); assertFalse(statusProvider.isPluginDisabled("plugin-2"));
assertFalse(statusProvider.isPluginDisabled("plugin-3")); assertFalse(statusProvider.isPluginDisabled("plugin-3"));
} }
@Test @Test
public void testDisablePluginWithoutDisabledFile() { public void testDisablePluginWithoutDisabledFile() throws Exception {
PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath); PluginStatusProvider statusProvider = new DefaultPluginStatusProvider(pluginsPath);
assertFalse(statusProvider.isPluginDisabled("plugin-1")); assertFalse(statusProvider.isPluginDisabled("plugin-1"));
assertTrue(statusProvider.disablePlugin("plugin-1"));
statusProvider.disablePlugin("plugin-1");
assertTrue(statusProvider.isPluginDisabled("plugin-1")); assertTrue(statusProvider.isPluginDisabled("plugin-1"));
} }

3
pf4j/src/test/java/org/pf4j/JarPluginRepositoryTest.java

@ -18,7 +18,6 @@ package org.pf4j;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.api.io.TempDir;
import java.io.IOException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.List; import java.util.List;
@ -39,7 +38,7 @@ public class JarPluginRepositoryTest {
* Test of {@link JarPluginRepository#deletePluginPath(Path)} method. * Test of {@link JarPluginRepository#deletePluginPath(Path)} method.
*/ */
@Test @Test
public void testDeletePluginPath() throws IOException { public void testDeletePluginPath() throws Exception {
PluginRepository repository = new JarPluginRepository(pluginsPath); PluginRepository repository = new JarPluginRepository(pluginsPath);
Path plugin1Path = Files.createDirectory(pluginsPath.resolve("plugin-1")); Path plugin1Path = Files.createDirectory(pluginsPath.resolve("plugin-1"));

Loading…
Cancel
Save