#198 verify that reading plugins descriptors from classpath is possible

framework/src/play/ClasspathResource.java

@@ -5,26 +5,48 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URL;
+import java.util.List;
 import java.util.regex.Pattern;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Collections.list;
+import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
 
 public class ClasspathResource {
   private static final Pattern RE_JAR_FILE_NAME = Pattern.compile("file:(.+\\.jar)!.+");
+  private final String fileName;
   private final URL url;
 
   public static ClasspathResource file(String fileName) {
-    return new ClasspathResource(Thread.currentThread().getContextClassLoader().getResource(fileName));
+    return new ClasspathResource(fileName, Thread.currentThread().getContextClassLoader().getResource(fileName));
   }
 
-  ClasspathResource(URL url) {
-    this.url = url;
+  public static List<ClasspathResource> files(String fileName) {
+    try {
+      return list(Thread.currentThread().getContextClassLoader().getResources(fileName)).stream()
+        .map(url -> new ClasspathResource(fileName, url))
+        .collect(toList());
+    }
+    catch (IOException e) {
+      throw new IllegalArgumentException("Failed to read files " + fileName);
+    }
+  }
+
+  ClasspathResource(String fileName, URL url) {
+    this.fileName = fileName;
+    this.url = requireNonNull(url, () -> "File not found in classpath: " + fileName);
+  }
+
+  public URL url() {
+    return url;
   }
 
   @Override
   public String toString() {
-    return "File " + url;
+    return fileName;
   }
 
   public String content() {
@@ -35,6 +57,15 @@ public String content() {
     }
   }
 
+  public List<String> lines() {
+    try (InputStream in = url.openStream()) {
+      return IOUtils.readLines(in, UTF_8);
+    }
+    catch (IOException e) {
+      throw new RuntimeException("Failed to read file " + url, e);
+    }
+  }
+
   public boolean isModifiedAfter(long timestamp) {
     switch (url.getProtocol()) {
       case "file": {

framework/src/play/plugins/PluginCollection.java

@@ -2,6 +2,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import play.ClasspathResource;
 import play.Play;
 import play.PlayPlugin;
 import play.data.binding.RootParamNode;
@@ -16,21 +17,18 @@
 import play.templates.Template;
 import play.vfs.VirtualFile;
 
+import javax.annotation.CheckReturnValue;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.ParametersAreNonnullByDefault;
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStreamReader;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
-import java.net.URL;
 import java.util.*;
 import java.util.stream.Stream;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Collections.list;
+import static java.lang.Integer.parseInt;
+import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toList;
 
 /**
@@ -62,67 +60,58 @@ public PluginCollection(SortedSet<PluginDescriptor> pluginsToLoad) {
      */
     public void loadPlugins() {
         logger.trace("Loading plugins");
-        List<URL> urls = getPlayPluginFileUrls();
-
-        // First we build one big SortedSet of all plugins to load (sorted based on index)
-        // This must be done to make sure the enhancing is happening
-        // when loading plugins using other classes that must be enhanced.
-        SortedSet<PluginDescriptor> pluginsToLoad = new TreeSet<>();
-        for (URL url : urls) {
-            logger.trace("Found one plugins descriptor, {}", url);
-            try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), UTF_8))) {
-                String line;
-                while ((line = reader.readLine()) != null) {
-                    if (line.trim().isEmpty()) {
-                        continue;
-                    }
-                    String[] lineParts = line.split(":");
-                    PluginDescriptor info = new PluginDescriptor(lineParts[1].trim(), Integer.parseInt(lineParts[0]), url);
-                    pluginsToLoad.add(info);
-                }
-            } catch (IOException e) {
-                throw new IllegalArgumentException("Failed to read plugins descriptor " + url, e);
-            }
-        }
+        List<ClasspathResource> pluginsFiles = getPlayPluginFiles();
+
+        // Sort plugins by index
+        SortedSet<PluginDescriptor> pluginsToLoad = pluginsFiles.stream()
+          .map(this::readPluginsDescriptor)
+          .flatMap(List::stream)
+          .collect(toCollection(TreeSet::new));
 
         loadPlugins(pluginsToLoad);
     }
 
-    public void loadPlugins(SortedSet<PluginDescriptor> pluginsToLoad) {
+    private List<PluginDescriptor> readPluginsDescriptor(ClasspathResource pluginsFile) {
+        logger.trace("Found one plugins descriptor, {}", pluginsFile);
+        return pluginsFile.lines().stream()
+          .filter(line -> !line.trim().isEmpty())
+          .map(line -> toPluginDescriptor(pluginsFile, line))
+          .collect(toList());
+    }
+
+    @Nonnull
+    @CheckReturnValue
+    private PluginDescriptor toPluginDescriptor(ClasspathResource pluginsFile, String line) {
+        String[] lineParts = line.split(":");
+        return new PluginDescriptor(lineParts[1].trim(), parseInt(lineParts[0]), pluginsFile.url());
+    }
+
+    private void loadPlugins(SortedSet<PluginDescriptor> pluginsToLoad) {
         for (PluginDescriptor info : pluginsToLoad) {
             logger.trace("Loading plugin {}", info.name);
             PlayPlugin plugin = Injector.getBeanOfType(info.name);
             plugin.index = info.index;
             addPlugin(plugin);
             logger.trace("Plugin {} loaded", plugin);
         }
-        // Now we call onLoad for all plugins and we detect if a plugin disables another plugin the
+        // Now we call onLoad for all plugins, and we detect if a plugin disables another plugin the
         // old way, by removing it from Play.plugins.
         getEnabledPlugins().forEach(plugin -> {
             logger.trace("Initializing plugin {}", plugin);
             plugin.onLoad();
         });
     }
 
-    List<URL> getPlayPluginFileUrls() {
+    List<ClasspathResource> getPlayPluginFiles() {
         String[] pluginsDescriptorFilenames = Play.configuration.getProperty("play.plugins.descriptor", "play.plugins").split(",");
-        List<URL> pluginDescriptors = Arrays.stream(pluginsDescriptorFilenames)
-          .map(f -> getResources(f))
+        List<ClasspathResource> pluginDescriptors = Arrays.stream(pluginsDescriptorFilenames)
+          .map(fileName -> ClasspathResource.files(fileName))
           .flatMap(List::stream)
           .collect(toList());
         logger.info("Found plugin descriptors: {}", pluginDescriptors);
         return pluginDescriptors;
     }
 
-    private List<URL> getResources(String f) {
-        try {
-            return list(Thread.currentThread().getContextClassLoader().getResources(f));
-        }
-        catch (IOException e) {
-            throw new IllegalArgumentException("Failed to read plugins from " + f);
-        }
-    }
-
     /**
      * Adds one plugin and enables it
      * 
@@ -145,6 +134,7 @@ protected synchronized void addPlugin(PlayPlugin plugin) {
      * @return PlayPlugin
      */
     @Nullable
+    @SuppressWarnings("unchecked")
     public <T extends PlayPlugin> T getPluginInstance(Class<T> pluginClazz) {
         return getAllPlugins()
           .filter(p -> pluginClazz.isInstance(p))

framework/test/play/ClasspathResourceTest.java

@@ -10,7 +10,7 @@
 class ClasspathResourceTest {
   @Test
   void extractsNameOfJarFile_from_classpathURL() throws MalformedURLException {
-    assertThat(new ClasspathResource(new URL("jar:file:/Users/toomas/replay/build/libs/app-1.2.3.jar!/routes.yml")).getJarFilePath())
+    assertThat(new ClasspathResource("routes.yml", new URL("jar:file:/Users/toomas/replay/build/libs/app-1.2.3.jar!/routes.yml")).getJarFilePath())
       .isEqualTo("/Users/toomas/replay/build/libs/app-1.2.3.jar");
   }
 }

framework/test/play/plugins/PluginCollectionTest.java

@@ -19,9 +19,10 @@
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.when;
+import static play.ClasspathResource.file;
 
 public class PluginCollectionTest {
 
@@ -67,15 +68,15 @@ public void verifyLoadingFromFilesWithBlankLines() {
     /**
      * Avoid including the same class+index twice.
      *
-     * This happened in the past under a range of circumstances, including: 1. Class path on NTFS or other case
-     * insensitive file system includes play.plugins directory 2x (C:/myproject/conf;c:/myproject/conf) 2.
-     * https://play.lighthouseapp.com/projects/57987/tickets/176-app-playplugins-loaded-twice-conf-on-2-classpaths
+     * This happened in the past under a range of circumstances, including: 
+     * 1. Class path on NTFS or other case-insensitive file system includes "play.plugins" directory 2x (C:/myproject/conf;c:/myproject/conf)
+     * 2. https://play.lighthouseapp.com/projects/57987/tickets/176-app-playplugins-loaded-twice-conf-on-2-classpaths
      */
     @Test
     public void skipsDuplicatePlugins() {
         PluginCollection pc = spy(new PluginCollection());
-        when(pc.getPlayPluginFileUrls())
-                .thenReturn(asList(getClass().getResource("custom-play.plugins"), getClass().getResource("custom-play.plugins.duplicate")));
+        doReturn(asList(file("play/plugins/custom-play.plugins"), file("play/plugins/custom-play.plugins.duplicate")))
+          .when(pc).getPlayPluginFiles();
         pc.loadPlugins();
         assertThat(pc.getAllPlugins()).containsExactly(pc.getPluginInstance(PlayStatusPlugin.class), pc.getPluginInstance(TestPlugin.class));
     }
@@ -84,17 +85,17 @@ public void skipsDuplicatePlugins() {
     public void canLoadPlayPluginsFromASingleDescriptor() {
         Play.configuration.setProperty("play.plugins.descriptor", "play/plugins/custom-play.plugins");
         PluginCollection pc = new PluginCollection();
-        assertThat(pc.getPlayPluginFileUrls().size()).isEqualTo(1);
-        assertThat(pc.getPlayPluginFileUrls().get(0).toString()).endsWith("play/plugins/custom-play.plugins");
+        assertThat(pc.getPlayPluginFiles().size()).isEqualTo(1);
+        assertThat(pc.getPlayPluginFiles().get(0).toString()).endsWith("play/plugins/custom-play.plugins");
     }
 
     @Test
     public void canLoadPlayPluginsFromMultipleDescriptors() {
         Play.configuration.setProperty("play.plugins.descriptor", "play/plugins/custom-play.plugins,play.plugins.sample");
         PluginCollection pc = new PluginCollection();
-        assertThat(pc.getPlayPluginFileUrls().size()).isEqualTo(2);
-        assertThat(pc.getPlayPluginFileUrls().get(0).toString()).endsWith("play/plugins/custom-play.plugins");
-        assertThat(pc.getPlayPluginFileUrls().get(1).toString()).endsWith("play.plugins.sample");
+        assertThat(pc.getPlayPluginFiles().size()).isEqualTo(2);
+        assertThat(pc.getPlayPluginFiles().get(0).toString()).endsWith("play/plugins/custom-play.plugins");
+        assertThat(pc.getPlayPluginFiles().get(1).toString()).endsWith("play.plugins.sample");
     }
 
     @Test