improve PluginDescriptorFinder implementations (see #70)

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,6 +41,15 @@ public class ManifestPluginDescriptorFinder implements PluginDescriptorFinder {
 
 	@Override
 	public PluginDescriptor find(File pluginRepository) throws PluginException {
+        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");
@@ -70,41 +78,36 @@ public class ManifestPluginDescriptorFinder implements PluginDescriptorFinder {
             }
         }
 
+        return manifest;
+    }
+
+    protected PluginDescriptor createPluginDescriptor(Manifest manifest) {
         PluginDescriptor pluginDescriptor = new PluginDescriptor();
 
         // TODO validate !!!
-        Attributes attrs = manifest.getMainAttributes();
+        Attributes attributes = manifest.getMainAttributes();
-        String id = attrs.getValue("Plugin-Id");
+        String id = attributes.getValue("Plugin-Id");
-        if (StringUtils.isEmpty(id)) {
-        	throw new PluginException("Plugin-Id cannot be empty");
-        }
         pluginDescriptor.setPluginId(id);
 
-        String description = attrs.getValue("Plugin-Description");
+        String description = attributes.getValue("Plugin-Description");
         if (StringUtils.isEmpty(description)) {
             pluginDescriptor.setPluginDescription("");
         } else {
             pluginDescriptor.setPluginDescription(description);
         }
 
-        String clazz = attrs.getValue("Plugin-Class");
+        String clazz = attributes.getValue("Plugin-Class");
-        if (StringUtils.isEmpty(clazz)) {
-        	throw new PluginException("Plugin-Class cannot be empty");
-        }
         pluginDescriptor.setPluginClass(clazz);
 
-        String version = attrs.getValue("Plugin-Version");
+        String version = attributes.getValue("Plugin-Version");
-        if (StringUtils.isEmpty(version)) {
-        	throw new PluginException("Plugin-Version cannot be empty");
-        }
         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);
         }
@@ -112,4 +115,13 @@ public class ManifestPluginDescriptorFinder implements PluginDescriptorFinder {
         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");
+        }
+    }
+
 }

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,6 +47,15 @@ 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()) {
@@ -74,33 +82,39 @@ public class PropertiesPluginDescriptorFinder implements PluginDescriptorFinder
             }
         }
 
+        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;
     }
 
+    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");
+        }
+    }
+
 }