Skip to content

Commit

Permalink
Merge branch 'bare-plugins'
Browse files Browse the repository at this point in the history
This topic branch fixes the problem where bare .class plugins were no
longer picked up by a legacy environment when noPluginClassLoader() was
called (which is now the default for ImageJ2/Fiji).

The problem was reported by lots of people and fixed during the opening
game of the world cup 2014. Brazil won.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Jun 13, 2014
2 parents e81bc9b + 8d28f05 commit 48a1cfe
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 20 deletions.
9 changes: 8 additions & 1 deletion src/main/java/net/imagej/patcher/EssentialLegacyHooks.java
Expand Up @@ -115,7 +115,7 @@ void addThisLoadersClasspath() {
/**
* Intended for sole use with patches in {@link ij.IJ}. DO NOT USE.
*/
public static ClassLoader missingSubdirs(final ClassLoader loader) {
public static ClassLoader missingSubdirs(final ClassLoader loader, final boolean addPluginsDir) {
if (loader == null || !(loader instanceof URLClassLoader)) return null;
final URLClassLoader classLoader = (URLClassLoader) loader;
final Set<File> directories = new HashSet<File>();
Expand All @@ -126,6 +126,13 @@ public static ClassLoader missingSubdirs(final ClassLoader loader) {
directories.add(dir);
}
final Set<File> missing = new HashSet<File>();
if (addPluginsDir) {
final File plugins = new File(IJ.getDirectory("plugins"));
if (!directories.contains(plugins)) {
directories.add(plugins);
missing.add(plugins);
}
}
try {
for (final File dir : new HashSet<File>(directories)) {
missingSubdirectories(dir, directories, missing);
Expand Down
16 changes: 15 additions & 1 deletion src/main/java/net/imagej/patcher/LegacyExtensions.java
Expand Up @@ -782,8 +782,22 @@ static void noPluginClassLoader(final CodeHacker hacker) {
final String runUserPlugInSig = "static java.lang.Object runUserPlugIn(java.lang.String commandName, java.lang.String className, java.lang.String arg, boolean createNewLoader)";
hacker.insertAtTopOfExceptionHandlers("ij.IJ", runUserPlugInSig, "java.lang.NoClassDefFoundError",
"java.lang.ClassLoader loader = " + LegacyInjector.ESSENTIAL_LEGACY_HOOKS_CLASS +
" .missingSubdirs(getClassLoader());" +
" .missingSubdirs(getClassLoader(), false);" +
"if (loader != null) _classLoader = loader;");
hacker.replaceCallInMethod("ij.IJ", runUserPlugInSig,
"java.lang.ClassLoader", "loadClass",
"try {" +
" $_ = $proceed($$);" +
"} catch (java.lang.ClassNotFoundException e) {" +
" java.lang.ClassLoader loader = " + LegacyInjector.ESSENTIAL_LEGACY_HOOKS_CLASS +
" .missingSubdirs(getClassLoader(), true);" +
" if (loader == null) {" +
" throw e;" +
" } else {" +
" _classLoader = loader;" +
" $_ = loader.loadClass($1);" +
" }" +
"}");
}

static void disableRefreshMenus(final CodeHacker hacker) {
Expand Down
108 changes: 108 additions & 0 deletions src/test/java/net/imagej/patcher/BarePluginsTest.java
@@ -0,0 +1,108 @@
/*
* #%L
* ImageJ software for multidimensional image processing and analysis.
* %%
* Copyright (C) 2009 - 2014 Board of Regents of the University of
* Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck
* Institute of Molecular Cell Biology and Genetics.
* %%
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
* #L%
*/

package net.imagej.patcher;

import static net.imagej.patcher.TestUtils.invokeStatic;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.scijava.test.TestUtils.createTemporaryDirectory;

import java.io.File;

import javassist.ClassClassPath;
import javassist.ClassPool;
import javassist.CtClass;
import javassist.CtField;
import javassist.CtMethod;

import org.junit.Test;

/**
* This unit test verifies that bare {@code .class} plugins are picked up correctly.
*
* @author Johannes Schindelin
*/
public class BarePluginsTest {

static {
try {
LegacyInjector.preinit();
}
catch (NoClassDefFoundError e) {
// ignore: LegacyInjector not in *this* classpath
}
catch (Throwable t) {
t.printStackTrace();
throw new RuntimeException("Got exception (see error log)");
}
}

@Test
public void testBarePlugin() throws Exception {
// generate simple plugin
final File tmp = createTemporaryDirectory("bare-plugins-");
final File plugins = new File(tmp, "plugins");
assertTrue(plugins.mkdir());

final ClassPool pool = new ClassPool();
pool.appendClassPath(new ClassClassPath(getClass()));
final CtClass clazz = pool.makeClass("Bare_Test_Plugin");
clazz.addField(CtField.make("public static java.lang.String value;", clazz));
clazz.addInterface(pool.get("ij.plugin.PlugIn"));
clazz.addMethod(CtMethod.make("public void run(java.lang.String arg) {" +
" value = \"bare \" + ij.Macro.getOptions().trim();" +
"}",
clazz));
clazz.addMethod(CtMethod.make("public static java.lang.String get() {" +
" return value;" +
"}",
clazz));
clazz.writeFile(plugins.getPath());

final String pluginsDir = System.getProperty("plugins.dir");
try {
System.setProperty("plugins.dir", plugins.getPath());
final LegacyEnvironment ij1 = new LegacyEnvironment(null, true);
ij1.noPluginClassLoader();
final String message = "Yep, that's the bare plugin alright!";
ij1.run("Bare Test Plugin", message);
final ClassLoader loader = invokeStatic(ij1.getClassLoader(), "ij.IJ", "getClassLoader");
assertEquals("bare " + message, invokeStatic(loader, "Bare_Test_Plugin", "get"));
} finally {
if (pluginsDir == null) {
System.clearProperty("plugins.dir");
} else {
System.setProperty("plugins.dir", pluginsDir);
}
}
}
}
18 changes: 10 additions & 8 deletions src/test/java/net/imagej/patcher/ExtraPluginDirsTest.java
Expand Up @@ -31,6 +31,8 @@

package net.imagej.patcher;

import static net.imagej.patcher.TestUtils.getTestEnvironment;
import static net.imagej.patcher.TestUtils.makeJar;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -90,13 +92,13 @@ public void before() throws IOException {
@Test
public void findsExtraPluginDir() throws Exception {
final File jarFile = new File(tmpDir, "Set_Property.jar");
TestUtils.makeJar(jarFile, Set_Property.class.getName());
makeJar(jarFile, Set_Property.class.getName());
assertTrue(jarFile.getAbsolutePath() + " exists", jarFile.exists());
System.setProperty("ij1.plugin.dirs", tmpDir.getAbsolutePath());

final String key = "random-" + Math.random();
System.setProperty(key, "321");
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment(true, true);
final LegacyEnvironment ij1 = getTestEnvironment(true, true);
ij1.run("Set Property", "key=" + key + " value=123");
assertEquals("123", System.getProperty(key));
}
Expand All @@ -107,17 +109,17 @@ public void knowsAboutJarsDirectory() throws Exception {
assertTrue(pluginsDir.mkdirs());
final File jarsDir = new File(tmpDir, "jars");
assertTrue(jarsDir.mkdirs());
LegacyEnvironment ij1 = TestUtils.getTestEnvironment();
LegacyEnvironment ij1 = getTestEnvironment();
final String helperClassName = TestUtils.class.getName();
try {
assertNull(ij1.runPlugIn(helperClassName, null));
} catch (Throwable t) {
/* all okay, we did not find the class */
}
final File jarFile = new File(jarsDir, "helper.jar");
TestUtils.makeJar(jarFile, helperClassName);
makeJar(jarFile, helperClassName);
System.setProperty("plugins.dir", pluginsDir.getAbsolutePath());
ij1 = TestUtils.getTestEnvironment();
ij1 = getTestEnvironment();
try {
assertNotNull(ij1.runPlugIn(helperClassName, null));
} catch (Throwable t) {
Expand Down Expand Up @@ -157,7 +159,7 @@ public void extraDirectory() throws Exception {
final String property = "ij.patcher.test." + Math.random();
System.clearProperty(property);
assertNull(System.getProperty(property));
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment();
final LegacyEnvironment ij1 = getTestEnvironment();
ij1.addPluginClasspath(tmpDir);
ij1.run(menuLabel, "property=" + property);
assertEquals(tmpDir.toURI().toURL().toString() + path, System.getProperty(property));
Expand All @@ -169,11 +171,11 @@ public void correctSubmenu() throws Exception {
final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
try {
final File jarFile = new File(tmpDir, "Submenu_Test.jar");
TestUtils.makeJar(jarFile, "Submenu_Test");
makeJar(jarFile, "Submenu_Test");
assertTrue(jarFile.getAbsolutePath() + " exists", jarFile.exists());
System.setProperty("ij1.plugin.dirs", tmpDir.getAbsolutePath());

final LegacyEnvironment ij1 = TestUtils.getTestEnvironment(false, true);
final LegacyEnvironment ij1 = getTestEnvironment(false, true);
final Class<?> imagej = ij1.getClassLoader().loadClass(ImageJ.class.getName());
imagej.getConstructor(Applet.class, Integer.TYPE).newInstance(null, ImageJ.NO_SHOW);
ij1.run("Submenu Test", "menupath=[Plugins>Submenu Test] class=Submenu_Test");
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/net/imagej/patcher/FatJarTest.java
Expand Up @@ -31,6 +31,7 @@

package net.imagej.patcher;

import static net.imagej.patcher.TestUtils.getTestEnvironment;
import static org.scijava.test.TestUtils.createTemporaryDirectory;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -133,7 +134,7 @@ public void testFatJars() throws Exception {

try {
System.setProperty("plugins.dir", tmp.getAbsolutePath());
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment();
final LegacyEnvironment ij1 = getTestEnvironment();
assertEquals(expect, ij1.runPlugIn(getClass().getName(), "").toString());
}
finally {
Expand Down
10 changes: 6 additions & 4 deletions src/test/java/net/imagej/patcher/HeadlessCompletenessTest.java
Expand Up @@ -31,14 +31,16 @@

package net.imagej.patcher;

import static org.scijava.test.TestUtils.createTemporaryDirectory;
import static net.imagej.patcher.TestUtils.construct;
import static net.imagej.patcher.TestUtils.getTestEnvironment;
import static net.imagej.patcher.TestUtils.invoke;
import static net.imagej.patcher.TestUtils.invokeStatic;
import static net.imagej.patcher.TestUtils.makeJar;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import static org.scijava.test.TestUtils.createTemporaryDirectory;
import ij.ImageJ;

import java.awt.Frame;
Expand Down Expand Up @@ -193,9 +195,9 @@ public void testMenuStructure() throws Exception {
assumeTrue(!GraphicsEnvironment.isHeadless());

final File jarFile = new File(tmpDir, "Set_Property.jar");
TestUtils.makeJar(jarFile, Set_Property.class.getName());
makeJar(jarFile, Set_Property.class.getName());

final LegacyEnvironment headlessIJ1 = TestUtils.getTestEnvironment();
final LegacyEnvironment headlessIJ1 = getTestEnvironment();
headlessIJ1.addPluginClasspath(jarFile);
headlessIJ1.runMacro("", "");
final Map<String, String> menuItems =
Expand All @@ -204,7 +206,7 @@ public void testMenuStructure() throws Exception {
assertTrue("does not have 'Set Property'", menuItems
.containsKey("Plugins>Set Property"));

final LegacyEnvironment ij1 = TestUtils.getTestEnvironment(false, false);
final LegacyEnvironment ij1 = getTestEnvironment(false, false);
ij1.addPluginClasspath(jarFile);
final Frame ij1Frame =
construct(ij1.getClassLoader(), "ij.ImageJ", ImageJ.NO_SHOW);
Expand Down
12 changes: 7 additions & 5 deletions src/test/java/net/imagej/patcher/HeadlessEnvironmentTest.java
Expand Up @@ -32,7 +32,9 @@
package net.imagej.patcher;

import static net.imagej.patcher.TestUtils.construct;
import static net.imagej.patcher.TestUtils.getTestEnvironment;
import static net.imagej.patcher.TestUtils.invoke;
import static net.imagej.patcher.TestUtils.makeJar;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -87,7 +89,7 @@ public void restoreThreadName() {

@Test
public void testMacro() throws Exception {
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment();
final LegacyEnvironment ij1 = getTestEnvironment();
final String propertyName = "headless.test.property" + Math.random();
final String propertyValue = "Hello, world!";
System.setProperty(propertyName, "(unset)");
Expand All @@ -106,7 +108,7 @@ public void testEncapsulation() throws Exception {
Thread.currentThread().setName("Run$_" + threadName);
Macro.setOptions("(unset)");
assertEquals("(unset) ", Macro.getOptions());
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment();
final LegacyEnvironment ij1 = getTestEnvironment();
ij1.runMacro("call(\"ij.Macro.setOptions\", \"Hello, world!\");",
null);
assertEquals("(unset) ", Macro.getOptions());
Expand Down Expand Up @@ -154,7 +156,7 @@ public void testIJInit() throws Exception {
public void testWithoutPluginClassLoader() throws Exception {
tmpDir = createTemporaryDirectory("class-loader-");
final File jarFile = new File(tmpDir, "Set_Property.jar");
TestUtils.makeJar(jarFile, Set_Property.class.getName());
makeJar(jarFile, Set_Property.class.getName());

// make a new class loader with the plugin and the classes
// required to make a new LegacyEnvironment in it
Expand Down Expand Up @@ -185,7 +187,7 @@ public void testWithoutPluginClassLoader() throws Exception {

@Test
public void testNonOverriddenMethods() throws Exception {
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment(true, false);
final LegacyEnvironment ij1 = getTestEnvironment(true, false);
final ClassLoader loader = ij1.getClassLoader();
final Object result;
final Thread thread = Thread.currentThread();
Expand All @@ -208,7 +210,7 @@ private static boolean runExampleDialogPlugin(final boolean patchHeadless) throw
}

private static boolean runExamplePlugin(final boolean patchHeadless, final String arg, final String macroOptions, final String expectedValue) throws Exception {
final LegacyEnvironment ij1 = TestUtils.getTestEnvironment(patchHeadless, false);
final LegacyEnvironment ij1 = getTestEnvironment(patchHeadless, false);
ij1.addPluginClasspath(HeadlessEnvironmentTest.class.getClassLoader());
try {
ij1.setMacroOptions(macroOptions);
Expand Down

0 comments on commit 48a1cfe

Please sign in to comment.