improve PluginDescriptorFinder implementations (see #70)

2 files changed, 93 insertions, 67 deletions

diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java b/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java
index 3328782..eed54bd 100644
--- a/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java
+++ b/pf4j/src/main/java/ro/fortsoft/pf4j/ManifestPluginDescriptorFinder.java
@@ -13,6 +13,10 @@
 package ro.fortsoft.pf4j;
 
 import com.github.zafarkhaja.semver.Version;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import ro.fortsoft.pf4j.util.StringUtils;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -20,11 +24,6 @@ import java.io.IOException;
 import java.util.jar.Attributes;
 import java.util.jar.Manifest;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import ro.fortsoft.pf4j.util.StringUtils;
-
 /**
  * Read the plugin descriptor from the manifest file.
  *
@@ -42,74 +41,87 @@ public class ManifestPluginDescriptorFinder implements PluginDescriptorFinder {
 
 	@Override
 	public PluginDescriptor find(File pluginRepository) throws PluginException {
-    	// TODO it's ok with first classes directory? Another idea is to specify in PluginClasspath the folder.
-		String classes = pluginClasspath.getClassesDirectories().get(0);
+        Manifest manifest = readManifest(pluginRepository);
+
+        PluginDescriptor pluginDescriptor = createPluginDescriptor(manifest);
+        validatePluginDescriptor(pluginDescriptor);
+
+		return pluginDescriptor;
+	}
+
+    protected Manifest readManifest(File pluginRepository) throws PluginException {
+        // TODO it's ok with first classes directory? Another idea is to specify in PluginClasspath the folder.
+        String classes = pluginClasspath.getClassesDirectories().get(0);
         File manifestFile = new File(pluginRepository, classes + "/META-INF/MANIFEST.MF");
         log.debug("Lookup plugin descriptor in '{}'", manifestFile);
         if (!manifestFile.exists()) {
             throw new PluginException("Cannot find '" + manifestFile + "' file");
         }
 
-    	FileInputStream input = null;
-		try {
-			input = new FileInputStream(manifestFile);
-		} catch (FileNotFoundException e) {
-			// not happening
-		}
+        FileInputStream input = null;
+        try {
+            input = new FileInputStream(manifestFile);
+        } catch (FileNotFoundException e) {
+            // not happening
+        }
 
-    	Manifest manifest = null;
+        Manifest manifest = null;
         try {
             manifest = new Manifest(input);
         } catch (IOException e) {
             throw new PluginException(e.getMessage(), e);
         } finally {
             try {
-				input.close();
-			} catch (IOException e) {
-				throw new PluginException(e.getMessage(), e);
-			}
+                input.close();
+            } catch (IOException e) {
+                throw new PluginException(e.getMessage(), e);
+            }
         }
 
+        return manifest;
+    }
+
+    protected PluginDescriptor createPluginDescriptor(Manifest manifest) {
         PluginDescriptor pluginDescriptor = new PluginDescriptor();
 
         // TODO validate !!!
-        Attributes attrs = manifest.getMainAttributes();
-        String id = attrs.getValue("Plugin-Id");
-        if (StringUtils.isEmpty(id)) {
-        	throw new PluginException("Plugin-Id cannot be empty");
-        }
+        Attributes attributes = manifest.getMainAttributes();
+        String id = attributes.getValue("Plugin-Id");
         pluginDescriptor.setPluginId(id);
 
-        String description = attrs.getValue("Plugin-Description");
+        String description = attributes.getValue("Plugin-Description");
         if (StringUtils.isEmpty(description)) {
-        	pluginDescriptor.setPluginDescription("");
+            pluginDescriptor.setPluginDescription("");
         } else {
             pluginDescriptor.setPluginDescription(description);
         }
 
-        String clazz = attrs.getValue("Plugin-Class");
-        if (StringUtils.isEmpty(clazz)) {
-        	throw new PluginException("Plugin-Class cannot be empty");
-        }
+        String clazz = attributes.getValue("Plugin-Class");
         pluginDescriptor.setPluginClass(clazz);
 
-        String version = attrs.getValue("Plugin-Version");
-        if (StringUtils.isEmpty(version)) {
-        	throw new PluginException("Plugin-Version cannot be empty");
-        }
+        String version = attributes.getValue("Plugin-Version");
         pluginDescriptor.setPluginVersion(Version.valueOf(version));
 
-        String provider = attrs.getValue("Plugin-Provider");
+        String provider = attributes.getValue("Plugin-Provider");
         pluginDescriptor.setProvider(provider);
-        String dependencies = attrs.getValue("Plugin-Dependencies");
+        String dependencies = attributes.getValue("Plugin-Dependencies");
         pluginDescriptor.setDependencies(dependencies);
 
-        String requires = attrs.getValue("Plugin-Requires");
+        String requires = attributes.getValue("Plugin-Requires");
         if (StringUtils.isNotEmpty(requires)) {
-        	pluginDescriptor.setRequires(requires);
+            pluginDescriptor.setRequires(requires);
         }
 
-		return pluginDescriptor;
-	}
+        return pluginDescriptor;
+    }
+
+    protected void validatePluginDescriptor(PluginDescriptor pluginDescriptor) throws PluginException {
+        if (StringUtils.isEmpty(pluginDescriptor.getPluginId())) {
+            throw new PluginException("Plugin-Id cannot be empty");
+        }
+        if (StringUtils.isEmpty(pluginDescriptor.getPluginClass())) {
+            throw new PluginException("Plugin-Class cannot be empty");
+        }
+    }
 
 }
diff --git a/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java b/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java
index 609987b..a6d35c9 100644
--- a/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java
+++ b/pf4j/src/main/java/ro/fortsoft/pf4j/PropertiesPluginDescriptorFinder.java
@@ -13,6 +13,10 @@
 package ro.fortsoft.pf4j;
 
 import com.github.zafarkhaja.semver.Version;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import ro.fortsoft.pf4j.util.StringUtils;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -20,11 +24,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.Properties;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import ro.fortsoft.pf4j.util.StringUtils;
-
 /**
  * Find a plugin descriptor in a properties file (in plugin repository).
  *
@@ -48,59 +47,74 @@ public class PropertiesPluginDescriptorFinder implements PluginDescriptorFinder
 
 	@Override
 	public PluginDescriptor find(File pluginRepository) throws PluginException {
+        Properties properties = readProperties(pluginRepository);
+
+        PluginDescriptor pluginDescriptor = createPluginDescriptor(properties);
+        validatePluginDescriptor(pluginDescriptor);
+
+        return pluginDescriptor;
+	}
+
+    protected Properties readProperties(File pluginRepository) throws PluginException {
         File propertiesFile = new File(pluginRepository, propertiesFileName);
         log.debug("Lookup plugin descriptor in '{}'", propertiesFile);
         if (!propertiesFile.exists()) {
             throw new PluginException("Cannot find '" + propertiesFile + "' file");
         }
 
-    	InputStream input = null;
-		try {
-			input = new FileInputStream(propertiesFile);
-		} catch (FileNotFoundException e) {
-			// not happening
-		}
+        InputStream input = null;
+        try {
+            input = new FileInputStream(propertiesFile);
+        } catch (FileNotFoundException e) {
+            // not happening
+        }
 
-    	Properties properties = new Properties();
+        Properties properties = new Properties();
         try {
-        	properties.load(input);
+            properties.load(input);
         } catch (IOException e) {
             throw new PluginException(e.getMessage(), e);
         } finally {
             try {
-				input.close();
-			} catch (IOException e) {
-				throw new PluginException(e.getMessage(), e);
-			}
+                input.close();
+            } catch (IOException e) {
+                throw new PluginException(e.getMessage(), e);
+            }
         }
 
+        return properties;
+    }
+
+    protected PluginDescriptor createPluginDescriptor(Properties properties) {
         PluginDescriptor pluginDescriptor = new PluginDescriptor();
 
         // TODO validate !!!
         String id = properties.getProperty("plugin.id");
-        if (StringUtils.isEmpty(id)) {
-        	throw new PluginException("plugin.id cannot be empty");
-        }
         pluginDescriptor.setPluginId(id);
 
         String clazz = properties.getProperty("plugin.class");
-        if (StringUtils.isEmpty(clazz)) {
-        	throw new PluginException("plugin.class cannot be empty");
-        }
         pluginDescriptor.setPluginClass(clazz);
 
         String version = properties.getProperty("plugin.version");
-        if (StringUtils.isEmpty(version)) {
-        	throw new PluginException("plugin.version cannot be empty");
-        }
         pluginDescriptor.setPluginVersion(Version.valueOf(version));
 
         String provider = properties.getProperty("plugin.provider");
         pluginDescriptor.setProvider(provider);
+
         String dependencies = properties.getProperty("plugin.dependencies");
         pluginDescriptor.setDependencies(dependencies);
 
-		return pluginDescriptor;
-	}
+        return pluginDescriptor;
+    }
+
+    protected void validatePluginDescriptor(PluginDescriptor pluginDescriptor) throws PluginException {
+        if (StringUtils.isEmpty(pluginDescriptor.getPluginId())) {
+            throw new PluginException("plugin.id cannot be empty");
+
+        }
+        if (StringUtils.isEmpty(pluginDescriptor.getPluginClass())) {
+            throw new PluginException("plugin.class cannot be empty");
+        }
+    }
 
 }