Skip to content

Commit

Permalink
Merge pull request #1138 from codeborne/refactor-loading-plugins
Browse files Browse the repository at this point in the history 
Make it possible to load plugins from single `play.plugins` file
  • Loading branch information
@asolntsev
asolntsev authored Jun 1, 2017
2 parents bd21d52 + cf0d378 commit 1094543
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 77 deletions.
11 changes: 11 additions & 0 deletions documentation/manual/configuration.textile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,17 @@ Default: none.
h2(#play). Play run-time


h3(#play.plugins.descriptor). play.plugins.descriptor

File to load play plugins from.
* If given, play with load plugins ONLY from this file. @since play 1.5
* If not given, play will find ALL files "play.plugins" from classpath and load plugins from them (this is the default behaviour).

bc. play.plugins.descriptor=conf/my-play-app.plugins

Default: none.


h3(#play.bytecodeCache). play.bytecodeCache

Used to disable the bytecode cache in @dev@ mode; has no effect in @prod@ mode.
Expand Down
80 changes: 26 additions & 54 deletions framework/src/play/plugins/PluginCollection.java
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,14 @@
package play.plugins;

import java.io.BufferedReader;
import java.io.File;
import java.io.InputStreamReader;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.net.URL;
import java.util.AbstractCollection;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.*;

import play.Logger;
import play.Play;
Expand All @@ -39,6 +28,9 @@
import play.test.TestEngine;
import play.vfs.VirtualFile;

import static java.util.Collections.emptyList;
import static java.util.Objects.hash;

/**
* Class handling all plugins used by Play.
*
Expand Down Expand Up @@ -113,7 +105,7 @@ private LoadingPluginInfo(String name, int index, URL url) {

@Override
public String toString() {
return "LoadingPluginInfo{" + "name='" + name + '\'' + ", index=" + index + ", url=" + url + '}';
return String.format("LoadingPluginInfo{name='%s', index=%s, url=%s}", name, index, url);
}

@Override
Expand All @@ -136,58 +128,26 @@ public boolean equals(Object o) {
return false;

LoadingPluginInfo that = (LoadingPluginInfo) o;

if (index != that.index)
return false;
if (name != null ? !name.equals(that.name) : that.name != null)
return false;

return true;
return Objects.equals(index, that.index) && Objects.equals(name, that.name);
}

@Override
public int hashCode() {
int result = name != null ? name.hashCode() : 0;
result = 31 * result + index;
return result;
return hash(name, index);
}
}

/**
* Enable found plugins
*/
public void loadPlugins() {
Logger.trace("Loading plugins");
// Play! plugins
Enumeration<URL> urls = null;
try {
urls = Play.classloader.getResources(play_plugins_resourceName);
} catch (Exception e) {
Logger.error("Error loading play.plugins", e);
return;
}
List<URL> urls = loadPlayPluginDescriptors();

// First we build one big SortedSet of all plugins to load (sorted based
// on index)
// 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.
// Data structure is a SortedSet instead of a List to 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
// I can see loading the same plugin with different indexes, but I can't
// think of a reasonable use case for
// loading the same plugin multiple times at the same priority.
SortedSet<LoadingPluginInfo> pluginsToLoad = new TreeSet<>();
while (urls != null && urls.hasMoreElements()) {
URL url = urls.nextElement();
for (URL url : urls) {
Logger.trace("Found one plugins descriptor, %s", url);
try {
BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), "utf-8"));
try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), "utf-8"))) {
String line;
while ((line = reader.readLine()) != null) {
if (line.trim().length() == 0) {
Expand All @@ -200,7 +160,6 @@ public void loadPlugins() {
} catch (Exception e) {
Logger.error(e, "Error interpreting %s", url);
}

}

for (LoadingPluginInfo info : pluginsToLoad) {
Expand All @@ -217,7 +176,7 @@ public void loadPlugins() {
Logger.error(ex, "Error loading plugin %s", info.toString());
}
}
// Mow we must call onLoad for all plugins - and we must detect if a
// Now we must call onLoad for all plugins - and we must detect if a
// plugin
// disables another plugin the old way, by removing it from
// Play.plugins.
Expand All @@ -231,7 +190,20 @@ public void loadPlugins() {

// Must update Play.plugins-list one last time
updatePlayPluginsList();
}

List<URL> loadPlayPluginDescriptors() {
try {
String playPluginsDescriptor = Play.configuration.getProperty("play.plugins.descriptor");
if (playPluginsDescriptor != null) {
return Collections.singletonList(new File(playPluginsDescriptor).toURI().toURL());
}
return Collections.list(Play.classloader.getResources(play_plugins_resourceName));
}
catch (Exception e) {
Logger.error(e, "Error loading play.plugins");
return emptyList();
}
}

/**
Expand Down
72 changes: 49 additions & 23 deletions framework/test-src/play/plugins/PluginCollectionTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package play.plugins;

import java.util.Arrays;
import java.util.Collection;

import org.junit.Before;
import org.junit.Test;

import play.*;
import play.data.parsing.TempFilePlugin;
import play.data.validation.ValidationPlugin;
Expand All @@ -16,19 +12,19 @@
import play.jobs.JobsPlugin;
import play.libs.WS;
import play.test.TestEngine;

import java.io.File;
import java.util.Collection;

import static java.util.Arrays.asList;
import static org.fest.assertions.Assertions.assertThat;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

/**
* Created by IntelliJ IDEA.
* User: mortenkjetland
* Date: 3/3/11
* Time: 12:14 AM
* To change this template use File | Settings | File Templates.
*/
public class PluginCollectionTest {

@Before
public void init(){
public void init() {
new PlayBuilder().build();
}

Expand All @@ -55,7 +51,7 @@ public void verifyLoading() {
@Test
public void verifyLoadingFromFilesWithBlankLines() throws Exception {
//create custom PluginCollection that fakes that TestPlugin is application plugin
PluginCollection pc = new PluginCollection(){
PluginCollection pc = new PluginCollection() {
@Override
protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {
//return true only if This is our TestPlugin
Expand All @@ -76,10 +72,40 @@ protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {

}

/**
* 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
*/
@Test
public void skipsDuplicatePlugins() {
PluginCollection pc = spy(new PluginCollection());
when(pc.loadPlayPluginDescriptors()).thenReturn(asList(
getClass().getResource("custom-play.plugins"),
getClass().getResource("custom-play.plugins.duplicate"))
);
pc.loadPlugins();
assertThat(pc.getAllPlugins()).containsExactly(
pc.getPluginInstance(CorePlugin.class),
pc.getPluginInstance(TestPlugin.class));
}

@Test
public void canLoadPlayPluginsFromASingleDescriptor() throws Exception {
Play.configuration.setProperty("play.plugins.descriptor", "test-src/play/plugins/custom-play.plugins");
PluginCollection pc = new PluginCollection();
assertThat(pc.loadPlayPluginDescriptors()).containsExactly(
new File("test-src/play/plugins/custom-play.plugins").toURI().toURL()
);
}

@Test
public void verifyReloading() throws Exception{
public void verifyReloading() throws Exception {
//create custom PluginCollection that fakes that TestPlugin is application plugin
PluginCollection pc = new PluginCollection(){
PluginCollection pc = new PluginCollection() {
@Override
protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {
//return true only if This is our TestPlugin
Expand Down Expand Up @@ -113,7 +139,7 @@ protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {

@SuppressWarnings({"deprecation"})
@Test
public void verifyUpdatePlayPluginsList(){
public void verifyUpdatePlayPluginsList() {
assertThat(Play.plugins).isEmpty();

PluginCollection pc = new PluginCollection();
Expand All @@ -126,7 +152,7 @@ public void verifyUpdatePlayPluginsList(){

@SuppressWarnings({"deprecation"})
@Test
public void verifyThatDisabelingPluginsTheOldWayStillWorks(){
public void verifyThatDisablingPluginsTheOldWayStillWorks() {
PluginCollection pc = new PluginCollection();


Expand Down Expand Up @@ -173,8 +199,8 @@ class LegacyPlugin extends PlayPlugin {
public void onLoad() {
//find TestPlugin in Play.plugins-list and remove it to disable it
PlayPlugin pluginToRemove = null;
for( PlayPlugin pp : Play.plugins){
if( pp.getClass().equals( TestPlugin.class)){
for (PlayPlugin pp : Play.plugins) {
if ( pp.getClass().equals( TestPlugin.class)) {
pluginToRemove = pp;
break;
}
Expand All @@ -188,25 +214,25 @@ class PluginWithTests extends PlayPlugin {

@Override
public Collection<Class> getUnitTests() {
return Arrays.asList(new Class[]{PluginUnit.class});
return asList(new Class[]{PluginUnit.class});
}

@Override
public Collection<Class> getFunctionalTests() {
return Arrays.asList(new Class[]{PluginFunc.class});
return asList(new Class[]{PluginFunc.class});
}
}

class PluginWithTests2 extends PlayPlugin {

@Override
public Collection<Class> getUnitTests() {
return Arrays.asList(new Class[]{PluginUnit2.class});
return asList(new Class[]{PluginUnit2.class});
}

@Override
public Collection<Class> getFunctionalTests() {
return Arrays.asList(new Class[]{PluginFunc2.class});
return asList(new Class[]{PluginFunc2.class});
}
}

Expand Down
2 changes: 2 additions & 0 deletions framework/test-src/play/plugins/custom-play.plugins.duplicate
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
0:play.CorePlugin
1:play.plugins.TestPlugin

0 comments on commit 1094543

Please sign in to comment.