Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to load plugins from single play.plugins file #1138

Merged
merged 3 commits into from
Jun 1, 2017
Merged

Make it possible to load plugins from single play.plugins file #1138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
40aa815
cleanup code
asolntsev May 9, 2017
02a75fb
convert long comment to a test (aka executable documentation!)
asolntsev May 9, 2017
cf0d378
#1137 can load play plugins from a single descriptor
asolntsev May 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions documentation/manual/configuration.textile
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
0:play.CorePlugin
1:play.plugins.TestPlugin