From c50b68aeb71f3361f917136086a74b4f29f89067 Mon Sep 17 00:00:00 2001 From: hinerm Date: Fri, 19 Jul 2024 13:42:39 -0500 Subject: [PATCH 1/3] Remove IJ1Helper param from DefaultLegacyHooks The LegacyService has a setter for its IJ1Helper. Tracking it separately in the DefaultLegacyHooks creates an opportunity for skew between its helper and the LegacyService's helper. The LegacyService has an accessor for its IJ1Helper instance so it is not necessary to cache it in the DefaultLegacyHooks. See https://github.com/imagej/imagej-legacy/issues/305 --- pom.xml | 2 +- .../net/imagej/legacy/DefaultLegacyHooks.java | 17 ++++++----------- .../java/net/imagej/legacy/LegacyService.java | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/pom.xml b/pom.xml index 028545db..5ff7c970 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ net.imagej imagej-legacy - 1.2.3-SNAPSHOT + 2.0.0-SNAPSHOT ImageJ Legacy Bridge The legacy component enables backward compatibility with the original version of ImageJ (1.x). It contains the code necessary to translate between ImageJ and ImageJ2 images, so that ImageJ plugins can be executed faithfully. diff --git a/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java b/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java index 633b9721..1e7d6f52 100644 --- a/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java +++ b/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java @@ -85,7 +85,6 @@ public class DefaultLegacyHooks extends LegacyHooks { private final LegacyService legacyService; - private final IJ1Helper helper; private LogService log; private LegacyEditor editor; @@ -97,14 +96,7 @@ public class DefaultLegacyHooks extends LegacyHooks { private BufferedWriter logFileWriter; public DefaultLegacyHooks(final LegacyService legacyService) { - this(legacyService, legacyService.getIJ1Helper()); - } - - public DefaultLegacyHooks(final LegacyService legacyService, - final IJ1Helper helper) - { this.legacyService = legacyService; - this.helper = helper; } @Override @@ -147,6 +139,8 @@ public Object interceptRunPlugIn(final String className, final String arg) { return legacyService == null ? null : legacyService.getContext(); } + IJ1Helper helper = legacyService.getIJ1Helper(); + // Intercept IJ1 commands if (helper != null) { // intercept ij.plugins.Commands @@ -357,7 +351,7 @@ public void addMenuItem(final String menuPath, final String command) { final String mPath = m.group(1); final String label = m.group(2); final String plugin = m.group(3); - helper.addCommand(mPath, label, plugin); + legacyService.getIJ1Helper().addCommand(mPath, label, plugin); } } } @@ -501,7 +495,7 @@ public boolean interceptCloseAllWindows() { final Window win = windows[w]; // Skip the ImageJ 1.x main window - if (win == null || win == helper.getIJ()) { + if (win == null || win == legacyService.getIJ1Helper().getIJ()) { continue; } @@ -550,6 +544,7 @@ public boolean interceptCloseAllWindows() { @Override public void interceptImageWindowClose(final Object window) { final Frame w = (Frame)window; + final IJ1Helper helper = legacyService.getIJ1Helper(); // When quitting, IJ1 doesn't dispose closing ImageWindows. // If the quit is later canceled this would leave orphaned windows. // Thus we queue any closed windows for disposal. @@ -569,7 +564,7 @@ public boolean disposing() { // within its ij.ImageJ#run() method, which is typically, but not always, // called on a separate thread by ij.ImageJ#quit(). The question is: did // the shutdown originate from an IJ1 code path, or a SciJava one? - if (helper.isDisposing()) { + if (legacyService.getIJ1Helper().isDisposing()) { // NB: ImageJ1 is in the process of a hard shutdown via an API call on // the SciJava level. It was probably either LegacyService#dispose() or // LegacyUI#dispose(), either of which triggers IJ1Helper#dispose(). diff --git a/src/main/java/net/imagej/legacy/LegacyService.java b/src/main/java/net/imagej/legacy/LegacyService.java index 4421347a..e12f7547 100644 --- a/src/main/java/net/imagej/legacy/LegacyService.java +++ b/src/main/java/net/imagej/legacy/LegacyService.java @@ -457,7 +457,7 @@ public void initialize() { final ClassLoader loader = Context.getClassLoader(); ij1Helper = new IJ1Helper(this); LegacyInjector.installHooks(loader, // - new DefaultLegacyHooks(this, ij1Helper)); + new DefaultLegacyHooks(this)); instance = this; // Initialize ImageJ 1.x, if needed. From ecadba9e265dc652c590adfbea56600818275c90 Mon Sep 17 00:00:00 2001 From: hinerm Date: Fri, 19 Jul 2024 13:44:45 -0500 Subject: [PATCH 2/3] Add MacroRecorderPostprocessor in test In my Windows environment (command line and IntelliJ) the MacroRecorderPostprocessor was not being registered as a postprocessor. Doing this seems necessary for the test to pass. --- .../imagej/legacy/plugin/MacroRecorderPostprocessorTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/java/net/imagej/legacy/plugin/MacroRecorderPostprocessorTest.java b/src/test/java/net/imagej/legacy/plugin/MacroRecorderPostprocessorTest.java index 23669b41..d2a32aba 100644 --- a/src/test/java/net/imagej/legacy/plugin/MacroRecorderPostprocessorTest.java +++ b/src/test/java/net/imagej/legacy/plugin/MacroRecorderPostprocessorTest.java @@ -44,6 +44,7 @@ import org.scijava.Context; import org.scijava.module.Module; import org.scijava.module.process.AbstractPreprocessorPlugin; +import org.scijava.module.process.PostprocessorPlugin; import org.scijava.module.process.PreprocessorPlugin; import org.scijava.plugin.PluginInfo; import org.scijava.plugin.PluginService; @@ -86,6 +87,9 @@ public void testParametersRecorded() throws InterruptedException, context.service(PluginService.class).addPlugin(new PluginInfo<>( MockInputHarvester.class, PreprocessorPlugin.class)); + context.service(PluginService.class).addPlugin(new PluginInfo<>( + MacroRecorderPostprocessor.class, PostprocessorPlugin.class)); + // NB: Override the IJ1Helper to remember which parameters get recorded. final EideticIJ1Helper ij1Helper = new EideticIJ1Helper(); legacyService.setIJ1Helper(ij1Helper); From ba93c4e6f8b58f4c6fb0ab5f0e267c4984f2b5a5 Mon Sep 17 00:00:00 2001 From: hinerm Date: Mon, 22 Jul 2024 10:21:03 -0500 Subject: [PATCH 3/3] Factor out IJ1Helper convenience method In case we want to do null checking in the future. --- .../net/imagej/legacy/DefaultLegacyHooks.java | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java b/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java index 1e7d6f52..d305a002 100644 --- a/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java +++ b/src/main/java/net/imagej/legacy/DefaultLegacyHooks.java @@ -139,7 +139,7 @@ public Object interceptRunPlugIn(final String className, final String arg) { return legacyService == null ? null : legacyService.getContext(); } - IJ1Helper helper = legacyService.getIJ1Helper(); + IJ1Helper helper = helper(); // Intercept IJ1 commands if (helper != null) { @@ -335,6 +335,7 @@ public void addMenuItem(final String menuPath, final String command) { final Pattern pattern = Pattern.compile( "^\\s*([^,]*),\\s*\"([^\"]*)\",\\s*([^\\s]*(\\(.*\\))?)\\s*"); final ClassLoader cl = Context.getClassLoader(); + final IJ1Helper helper = helper(); try { final Enumeration pluginsConfigs = cl.getResources("plugins.config"); while (pluginsConfigs.hasMoreElements()) { @@ -351,7 +352,7 @@ public void addMenuItem(final String menuPath, final String command) { final String mPath = m.group(1); final String label = m.group(2); final String plugin = m.group(3); - legacyService.getIJ1Helper().addCommand(mPath, label, plugin); + helper.addCommand(mPath, label, plugin); } } } @@ -495,7 +496,7 @@ public boolean interceptCloseAllWindows() { final Window win = windows[w]; // Skip the ImageJ 1.x main window - if (win == null || win == legacyService.getIJ1Helper().getIJ()) { + if (win == null || win == helper().getIJ()) { continue; } @@ -544,7 +545,7 @@ public boolean interceptCloseAllWindows() { @Override public void interceptImageWindowClose(final Object window) { final Frame w = (Frame)window; - final IJ1Helper helper = legacyService.getIJ1Helper(); + final IJ1Helper helper = helper(); // When quitting, IJ1 doesn't dispose closing ImageWindows. // If the quit is later canceled this would leave orphaned windows. // Thus we queue any closed windows for disposal. @@ -564,7 +565,7 @@ public boolean disposing() { // within its ij.ImageJ#run() method, which is typically, but not always, // called on a separate thread by ij.ImageJ#quit(). The question is: did // the shutdown originate from an IJ1 code path, or a SciJava one? - if (legacyService.getIJ1Helper().isDisposing()) { + if (helper().isDisposing()) { // NB: ImageJ1 is in the process of a hard shutdown via an API call on // the SciJava level. It was probably either LegacyService#dispose() or // LegacyUI#dispose(), either of which triggers IJ1Helper#dispose(). @@ -581,6 +582,17 @@ public boolean disposing() { // -- Helper methods -- + /** + * Convenience method for accessing the attached {@link LegacyService}'s + * {@link IJ1Helper}. + */ + private IJ1Helper helper() { + // NB: although there is a setter for the IJ1Helper, it is documented as + // "non-API" and thus the helper should never be null. If this changes at + // some point we should do null checking here. + return legacyService.getIJ1Helper(); + } + /** * Determines whether a file is binary or text. *