New LoadPluginsTest (#141)

pf4j/pom.xml

@@ -34,7 +34,7 @@
         </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
-            <artifactId>slf4j-simple</artifactId>
+            <artifactId>slf4j-log4j12</artifactId>
             <version>1.7.5</version>
             <scope>test</scope>
         </dependency>

pf4j/src/main/java/ro/fortsoft/pf4j/AbstractPluginManager.java

@@ -85,11 +85,6 @@ public abstract class AbstractPluginManager implements PluginManager {
      */
     private Version systemVersion = Version.forIntegers(0, 0, 0);
 
-    /**
-     * A relation between 'pluginPath' and 'pluginId'
-     */
-    private Map<Path, String> pathToIdMap;
-
     private PluginRepository pluginRepository;
     private PluginFactory pluginFactory;
     private ExtensionFactory extensionFactory;
@@ -258,7 +253,6 @@ public abstract class AbstractPluginManager implements PluginManager {
             // remove the plugin
             plugins.remove(pluginId);
             getResolvedPlugins().remove(pluginWrapper);
-            pathToIdMap.remove(pluginWrapper.getPluginPath());
 
             firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
 
@@ -654,7 +648,6 @@ public abstract class AbstractPluginManager implements PluginManager {
         startedPlugins = new ArrayList<>();
 
         pluginStateListeners = new ArrayList<>();
-        pathToIdMap = new HashMap<>();
 
         if (pluginsRoot == null) {
             pluginsRoot = createPluginsRoot();
@@ -735,7 +728,8 @@ public abstract class AbstractPluginManager implements PluginManager {
 
     protected PluginWrapper loadPluginFromPath(Path pluginPath) throws PluginException {
         // test for plugin duplication
-        if (plugins.get(pathToIdMap.get(pluginPath)) != null) {
+        if (idForPath(pluginPath) != null) {
+            log.warn("Plugin {} already loaded", idForPath(pluginPath));
             return null;
         }
 
@@ -784,6 +778,20 @@ public abstract class AbstractPluginManager implements PluginManager {
         return pluginWrapper;
     }
 
+    /**
+     * Tests for already loaded plugins on given path
+     * @param pluginPath the path to investigate
+     * @return id of plugin or null if not loaded
+     */
+    protected String idForPath(Path pluginPath) {
+        for (PluginWrapper plugin : plugins.values()) {
+            if (plugin.getPluginPath().equals(pluginPath)) {
+                return plugin.getPluginId();
+            }
+        }
+        return null;
+    }
+
     /**
      * Override this to change the validation criteria
      * @param descriptor the plugin descriptor to validate

pf4j/src/main/java/ro/fortsoft/pf4j/util/FileUtils.java

@@ -166,11 +166,12 @@ public class FileUtils {
      * @throws IOException on error
      */
     public static Path expandIfZip(Path filePath) throws IOException {
-        String fileName = filePath.getFileName().toString();
+        if (!isZipFile(filePath)) {
-        if (!ZIP_EXTENSIONS.contains(fileName.substring(fileName.lastIndexOf(".")))) {
             return filePath;
         }
+
         FileTime pluginZipDate = Files.getLastModifiedTime(filePath);
+        String fileName = filePath.getFileName().toString();
         Path pluginDirectory = filePath.resolveSibling(fileName.substring(0, fileName.lastIndexOf(".")));
 
         if (!Files.exists(pluginDirectory) || pluginZipDate.compareTo(Files.getLastModifiedTime(pluginDirectory)) > 0) {
@@ -192,4 +193,13 @@ public class FileUtils {
 
         return pluginDirectory;
     }
+
+    /**
+     * Return true only if path is a zip file
+     * @param path to a file/dir
+     * @return true if file with .zip ending
+     */
+    public static boolean isZipFile(Path path) {
+        return Files.isRegularFile(path) && path.toString().toLowerCase().endsWith(".zip");
+    }
 }

pf4j/src/test/java/ro/fortsoft/pf4j/LoadPluginsTest.java

@@ -0,0 +1,169 @@
+/*
+ * Copyright 2017 Decebal Suiu
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ro.fortsoft.pf4j;
+
+import com.github.zafarkhaja.semver.Version;
+import org.junit.Before;
+import org.junit.Test;
+import ro.fortsoft.pf4j.plugin.MockPluginManager;
+
+import java.io.BufferedWriter;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.*;
+import java.util.Collections;
+
+import static junit.framework.TestCase.assertNull;
+import static org.junit.Assert.*;
+
+public class LoadPluginsTest {
+    private Path tmpDir;
+    private MockPluginManager pluginManager;
+    private MockZipPlugin p1;
+    private MockZipPlugin p2;
+    private MockZipPlugin p3;
+
+    @Before
+    public void setup() throws IOException {
+        tmpDir = Files.createTempDirectory("pf4j-test");
+        tmpDir.toFile().deleteOnExit();
+        p1 = new MockZipPlugin("myPlugin", "1.2.3", "my-plugin-1.2.3", "my-plugin-1.2.3.zip");
+        p2 = new MockZipPlugin("myPlugin", "2.0.0", "my-plugin-2.0.0", "my-plugin-2.0.0.ZIP");
+        p3 = new MockZipPlugin("other", "3.0.0", "other-3.0.0", "other-3.0.0.Zip");
+        pluginManager = new MockPluginManager(
+            tmpDir,
+            new PropertiesPluginDescriptorFinder("my.properties"));
+    }
+
+    @Test
+    public void load() throws Exception {
+        p1.create();
+        assertTrue(Files.exists(p1.zipFile));
+        assertEquals(0, pluginManager.getPlugins().size());
+        pluginManager.loadPlugins();
+        assertTrue(Files.exists(p1.zipFile));
+        assertTrue(Files.exists(p1.unzipped));
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertEquals(p1.id, pluginManager.idForPath(p1.unzipped));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void loadNonExisting() throws Exception {
+        pluginManager.loadPlugin(Paths.get("nonexisting"));
+    }
+
+    @Test
+    public void loadTwiceFails() throws Exception {
+        p1.create();
+        assertNotNull(pluginManager.loadPluginFromPath(p1.zipFile));
+        assertNull(pluginManager.loadPluginFromPath(p1.zipFile));
+    }
+
+    @Test
+    public void loadUnloadLoad() throws Exception {
+        p1.create();
+        pluginManager.loadPlugins();
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertTrue(pluginManager.unloadPlugin(pluginManager.idForPath(p1.unzipped)));
+        // duplicate check
+        assertNull(pluginManager.idForPath(p1.unzipped));
+        // Double unload ok
+        assertFalse(pluginManager.unloadPlugin(pluginManager.idForPath(p1.unzipped)));
+        assertNotNull(pluginManager.loadPlugin(p1.unzipped));
+    }
+
+    @Test
+    public void upgrade() throws Exception {
+        p1.create();
+        pluginManager.loadPlugins();
+        pluginManager.startPlugins();
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertEquals(Version.valueOf("1.2.3"), pluginManager.getPlugin(p2.id).getDescriptor().getVersion());
+        assertEquals(1, pluginManager.getStartedPlugins().size());
+        p2.create();
+        pluginManager.loadPlugins();
+        pluginManager.startPlugin(p2.id);
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertEquals(Version.valueOf("2.0.0"), pluginManager.getPlugin(p2.id).getDescriptor().getVersion());
+        assertEquals(Version.valueOf("2.0.0"), pluginManager.getStartedPlugins().get(1).getDescriptor().getVersion());
+    }
+
+    @Test
+    public void getRoot() throws Exception {
+        assertEquals(tmpDir, pluginManager.getPluginsRoot());
+    }
+
+    @Test
+    public void notAPlugin() throws Exception {
+        Path notAPlugin = tmpDir.resolve("not-a-zip");
+        Files.createFile(notAPlugin);
+        pluginManager.loadPlugins();
+        assertEquals(0, pluginManager.getPlugins().size());
+    }
+
+    @Test
+    public void deletePlugin() throws Exception {
+        p1.create();
+        p3.create();
+        pluginManager.loadPlugins();
+        pluginManager.startPlugins();
+        assertEquals(2, pluginManager.getPlugins().size());
+        pluginManager.deletePlugin(p1.id);
+        assertEquals(1, pluginManager.getPlugins().size());
+        assertFalse(Files.exists(p1.zipFile));
+        assertFalse(Files.exists(p1.unzipped));
+        assertTrue(Files.exists(p3.zipFile));
+        assertTrue(Files.exists(p3.unzipped));
+    }
+
+    private class MockZipPlugin {
+        public final String id;
+        public final String version;
+        public final String filename;
+        public final Path zipFile;
+        public final Path unzipped;
+        public final Path propsFile;
+        public final URI fileURI;
+        public String zipname;
+
+        public MockZipPlugin(String id, String version, String filename, String zipname) throws IOException {
+            this.id = id;
+            this.version = version;
+            this.filename = filename;
+            this.zipname = zipname;
+
+            zipFile = tmpDir.resolve(zipname).toAbsolutePath();
+            unzipped = tmpDir.resolve(filename);
+            propsFile = tmpDir.resolve("my.properties");
+            fileURI = URI.create("jar:file:"+zipFile.toString());
+        }
+
+        public void create() throws IOException {
+            try (FileSystem zipfs = FileSystems.newFileSystem(fileURI, Collections.singletonMap("create", "true"))) {
+                Path propsInZip = zipfs.getPath("/" + propsFile.getFileName().toString());
+                BufferedWriter br = new BufferedWriter(new FileWriter(propsFile.toString()));
+                br.write("plugin.id=" + id);
+                br.newLine();
+                br.write("plugin.version=" + version);
+                br.newLine();
+                br.write("plugin.class=ro.fortsoft.pf4j.plugin.TestPlugin");
+                br.close();
+                Files.move(propsFile, propsInZip);
+            }
+        }
+    }
+}

pf4j/src/test/java/ro/fortsoft/pf4j/plugin/MockPluginManager.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2017 Decebal Suiu
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ro.fortsoft.pf4j.plugin;
+
+import ro.fortsoft.pf4j.DefaultPluginClasspath;
+import ro.fortsoft.pf4j.DefaultPluginDescriptorFinder;
+import ro.fortsoft.pf4j.DefaultPluginManager;
+import ro.fortsoft.pf4j.PluginDescriptorFinder;
+
+import java.nio.file.Path;
+
+/**
+ * Manager for testing
+ */
+public class MockPluginManager extends DefaultPluginManager {
+    private PluginDescriptorFinder finder = new DefaultPluginDescriptorFinder(new DefaultPluginClasspath());
+
+    public MockPluginManager() {
+        super();
+    }
+
+    public MockPluginManager(Path root, PluginDescriptorFinder finder) {
+        super(root);
+        this.finder = finder;
+    }
+
+    @Override
+    protected PluginDescriptorFinder getPluginDescriptorFinder() {
+        return finder;
+    }
+
+    @Override
+    protected PluginDescriptorFinder createPluginDescriptorFinder() {
+        return finder;
+    }
+}

pf4j/src/test/java/ro/fortsoft/pf4j/util/FileUtilsTest.java

@@ -60,6 +60,12 @@ public class FileUtilsTest {
 
         // Non-zip file remains unchanged
         assertEquals(propsFile, FileUtils.expandIfZip(propsFile));
+        // File without .suffix
+        Path extra = Files.createFile(tmpDir.resolve("extra"));
+        assertEquals(extra, FileUtils.expandIfZip(extra));
+        // Folder
+        Path folder = Files.createFile(tmpDir.resolve("folder"));
+        assertEquals(folder, FileUtils.expandIfZip(folder));
     }
 
 }