Skip to content

Commit e0c9da6

Browse files
ctruedenctrueden
committed
Merge branch 'overlapping-script-dirs'
This branch improves the script finder logic: - Adds API to specify a menu prefix for each base script directory. - Adds each script only once, even if present in script directories. - Adds unit tests for these features.
2 parents 96d0932 + 265fd11 commit e0c9da6

File tree

4 files changed

+153
-25
lines changed

4 files changed

+153
-25
lines changed

‎src/main/java/org/scijava/script/DefaultScriptService.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.scijava.Context;
5353
import org.scijava.Gateway;
5454
import org.scijava.InstantiableException;
55+
import org.scijava.MenuPath;
5556
import org.scijava.Priority;
5657
import org.scijava.command.CommandService;
5758
import org.scijava.log.LogService;
@@ -98,6 +99,9 @@ public class DefaultScriptService extends
9899
/** List of directories to scan for scripts. */
99100
private ArrayList<File> scriptDirs;
100101

102+
/** Menu prefix to use for each script directory, if any. */
103+
private HashMap<File, MenuPath> menuPrefixes;
104+
101105
/** Index of available scripts, by script <em>file</em>. */
102106
private HashMap<File, ScriptInfo> scripts;
103107

@@ -133,11 +137,24 @@ public List<File> getScriptDirectories() {
133137
return Collections.unmodifiableList(scriptDirs());
134138
}
135139

140+
@Override
141+
public MenuPath getMenuPrefix(final File scriptDirectory) {
142+
return menuPrefixes().get(scriptDirectory);
143+
}
144+
136145
@Override
137146
public void addScriptDirectory(final File scriptDirectory) {
138147
scriptDirs().add(scriptDirectory);
139148
}
140149

150+
@Override
151+
public void addScriptDirectory(final File scriptDirectory,
152+
final MenuPath menuPrefix)
153+
{
154+
scriptDirs().add(scriptDirectory);
155+
menuPrefixes().put(scriptDirectory, menuPrefix);
156+
}
157+
141158
@Override
142159
public void removeScriptDirectory(final File scriptDirectory) {
143160
scriptDirs().remove(scriptDirectory);
@@ -283,6 +300,12 @@ private List<File> scriptDirs() {
283300
return scriptDirs;
284301
}
285302

303+
/** Gets {@link #menuPrefixes}, initializing if needed. */
304+
private HashMap<File, MenuPath> menuPrefixes() {
305+
if (menuPrefixes == null) initMenuPrefixes();
306+
return menuPrefixes;
307+
}
308+
286309
/** Gets {@link #scripts}, initializing if needed. */
287310
private HashMap<File, ScriptInfo> scripts() {
288311
if (scripts == null) initScripts();
@@ -338,6 +361,12 @@ private synchronized void initScriptDirs() {
338361
scriptDirs = dirs;
339362
}
340363

364+
/** Initializes {@link #menuPrefixes}. */
365+
private synchronized void initMenuPrefixes() {
366+
if (menuPrefixes != null) return;
367+
menuPrefixes = new HashMap<File, MenuPath>();
368+
}
369+
341370
/** Initializes {@link #scripts}. */
342371
private synchronized void initScripts() {
343372
if (scripts != null) return; // already initialized

‎src/main/java/org/scijava/script/ScriptFinder.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
package org.scijava.script;
3333

3434
import java.io.File;
35+
import java.util.HashSet;
3536
import java.util.List;
37+
import java.util.Set;
3638

3739
import org.scijava.AbstractContextual;
3840
import org.scijava.MenuEntry;
@@ -78,18 +80,24 @@ public void findScripts(final List<ScriptInfo> scripts) {
7880

7981
int scriptCount = 0;
8082

83+
final HashSet<File> scriptFiles = new HashSet<File>();
8184
for (final File directory : directories) {
8285
if (!directory.exists()) {
8386
log.warn("Ignoring non-existent scripts directory: " +
8487
directory.getAbsolutePath());
8588
continue;
8689
}
87-
scriptCount += discoverScripts(scripts, directory, new MenuPath());
90+
final MenuPath prefix = scriptService.getMenuPrefix(directory);
91+
final MenuPath menuPath = prefix == null ? new MenuPath() : prefix;
92+
scriptCount +=
93+
discoverScripts(scripts, scriptFiles, directory, menuPath);
8894
}
8995

9096
log.info("Found " + scriptCount + " scripts");
9197
}
9298

99+
// -- Helper methods --
100+
93101
/**
94102
* Looks through a directory, discovering and adding scripts.
95103
*
@@ -98,7 +106,7 @@ public void findScripts(final List<ScriptInfo> scripts) {
98106
* @param menuPath The menu path, which must not be {@code null}.
99107
*/
100108
private int discoverScripts(final List<ScriptInfo> scripts,
101-
final File directory, final MenuPath menuPath)
109+
final Set<File> scriptFiles, final File directory, final MenuPath menuPath)
102110
{
103111
final File[] fileList = directory.listFiles();
104112
if (fileList == null) return 0; // directory does not exist
@@ -107,10 +115,12 @@ private int discoverScripts(final List<ScriptInfo> scripts,
107115
final boolean isTopLevel = menuPath.size() == 0;
108116

109117
for (final File file : fileList) {
118+
if (scriptFiles.contains(file)) continue; // script already added
119+
110120
final String name = file.getName().replace('_', ' ');
111121
if (file.isDirectory()) {
112122
// recurse into subdirectory
113-
discoverScripts(scripts, file, subMenuPath(menuPath, name));
123+
discoverScripts(scripts, scriptFiles, file, subMenuPath(menuPath, name));
114124
}
115125
else if (isTopLevel) {
116126
// ignore scripts in toplevel script directories
@@ -121,15 +131,14 @@ else if (scriptService.canHandleFile(file)) {
121131
final int dot = name.lastIndexOf('.');
122132
final String noExt = dot <= 0 ? name : name.substring(0, dot);
123133
scripts.add(createEntry(file, subMenuPath(menuPath, noExt)));
134+
scriptFiles.add(file);
124135
scriptCount++;
125136
}
126137
}
127138

128139
return scriptCount;
129140
}
130141

131-
// -- Helper methods --
132-
133142
private MenuPath
134143
subMenuPath(final MenuPath menuPath, final String subMenuName)
135144
{

‎src/main/java/org/scijava/script/ScriptService.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import javax.script.ScriptEngineFactory;
4444
import javax.script.ScriptException;
4545

46+
import org.scijava.MenuPath;
4647
import org.scijava.module.process.PostprocessorPlugin;
4748
import org.scijava.module.process.PreprocessorPlugin;
4849
import org.scijava.plugin.Plugin;
@@ -95,9 +96,20 @@ public interface ScriptService extends SingletonService<ScriptLanguage>,
9596
/** Gets the base directories to scan for scripts. */
9697
List<File> getScriptDirectories();
9798

99+
/**
100+
* Gets the menu path prefix for the given script directory, or null if none.
101+
*/
102+
MenuPath getMenuPrefix(File scriptDirectory);
103+
98104
/** Adds a base directory to scan for scripts. */
99105
void addScriptDirectory(File scriptDirectory);
100106

107+
/**
108+
* Adds a base directory to scan for scripts, placing discovered scripts
109+
* beneath the given menu path prefix.
110+
*/
111+
void addScriptDirectory(File scriptDirectory, final MenuPath menuPrefix);
112+
101113
/** Removes a base directory to scan for scripts. */
102114
void removeScriptDirectory(File scriptDirectory);
103115

‎src/test/java/org/scijava/script/ScriptFinderTest.java‎

Lines changed: 98 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.io.IOException;
3838
import java.util.ArrayList;
3939
import java.util.Arrays;
40+
import java.util.Collections;
4041
import java.util.List;
4142

4243
import javax.script.ScriptEngine;
@@ -45,8 +46,10 @@
4546
import org.junit.BeforeClass;
4647
import org.junit.Test;
4748
import org.scijava.Context;
49+
import org.scijava.MenuPath;
4850
import org.scijava.plugin.Plugin;
4951
import org.scijava.test.TestUtils;
52+
import org.scijava.util.AppUtils;
5053
import org.scijava.util.FileUtils;
5154

5255
/**
@@ -64,10 +67,10 @@ public class ScriptFinderTest {
6467
public static void setUp() throws IOException {
6568
scriptsDir = TestUtils.createTemporaryDirectory("script-finder-");
6669
TestUtils.createPath(scriptsDir, "ignored.foo");
67-
TestUtils.createPath(scriptsDir, "Plugins/quick.foo");
68-
TestUtils.createPath(scriptsDir, "Plugins/brown.foo");
69-
TestUtils.createPath(scriptsDir, "Plugins/fox.foo");
70-
TestUtils.createPath(scriptsDir, "Plugins/The_Lazy_Dog.foo");
70+
TestUtils.createPath(scriptsDir, "Scripts/quick.foo");
71+
TestUtils.createPath(scriptsDir, "Scripts/brown.foo");
72+
TestUtils.createPath(scriptsDir, "Scripts/fox.foo");
73+
TestUtils.createPath(scriptsDir, "Scripts/The_Lazy_Dog.foo");
7174
TestUtils.createPath(scriptsDir, "Math/add.foo");
7275
TestUtils.createPath(scriptsDir, "Math/subtract.foo");
7376
TestUtils.createPath(scriptsDir, "Math/multiply.foo");
@@ -86,30 +89,105 @@ public static void tearDown() {
8689

8790
@Test
8891
public void testFindScripts() {
89-
final Context context = new Context(ScriptService.class);
90-
final ScriptService scriptService = context.service(ScriptService.class);
92+
final ScriptService scriptService = createScriptService();
9193
scriptService.addScriptDirectory(scriptsDir);
9294

93-
final ScriptFinder scriptFinder = new ScriptFinder(scriptService);
95+
final ArrayList<ScriptInfo> scripts = findScripts(scriptService);
9496

95-
final ArrayList<ScriptInfo> scripts = new ArrayList<ScriptInfo>();
96-
scriptFinder.findScripts(scripts);
9797
assertEquals(11, scripts.size());
98-
assertMenuPath("Math > add", scripts, 0);
99-
assertMenuPath("Math > divide", scripts, 1);
100-
assertMenuPath("Math > multiply", scripts, 2);
101-
assertMenuPath("Math > subtract", scripts, 3);
102-
assertMenuPath("Math > Trig > cos", scripts, 4);
103-
assertMenuPath("Math > Trig > sin", scripts, 5);
104-
assertMenuPath("Math > Trig > tan", scripts, 6);
105-
assertMenuPath("Plugins > brown", scripts, 7);
106-
assertMenuPath("Plugins > fox", scripts, 8);
107-
assertMenuPath("Plugins > quick", scripts, 9);
108-
assertMenuPath("Plugins > The Lazy Dog", scripts, 10);
98+
assertMenuPath("Scripts > The Lazy Dog", scripts, 0);
99+
assertMenuPath("Math > add", scripts, 1);
100+
assertMenuPath("Scripts > brown", scripts, 2);
101+
assertMenuPath("Math > Trig > cos", scripts, 3);
102+
assertMenuPath("Math > divide", scripts, 4);
103+
assertMenuPath("Scripts > fox", scripts, 5);
104+
assertMenuPath("Math > multiply", scripts, 6);
105+
assertMenuPath("Scripts > quick", scripts, 7);
106+
assertMenuPath("Math > Trig > sin", scripts, 8);
107+
assertMenuPath("Math > subtract", scripts, 9);
108+
assertMenuPath("Math > Trig > tan", scripts, 10);
109+
}
110+
111+
/**
112+
* Tests that menu prefixes work as expected when
113+
* {@link ScriptService#addScriptDirectory(File, org.scijava.MenuPath)} is
114+
* called.
115+
*/
116+
@Test
117+
public void testMenuPrefixes() {
118+
final ScriptService scriptService = createScriptService();
119+
120+
final MenuPath menuPrefix = new MenuPath("Foo > Bar");
121+
assertEquals(2, menuPrefix.size());
122+
assertEquals("Bar", menuPrefix.getLeaf().getName());
123+
scriptService.addScriptDirectory(scriptsDir, menuPrefix);
124+
125+
final ArrayList<ScriptInfo> scripts = findScripts(scriptService);
126+
127+
assertEquals(12, scripts.size());
128+
assertMenuPath("Foo > Bar > Scripts > The Lazy Dog", scripts, 0);
129+
assertMenuPath("Foo > Bar > Math > add", scripts, 1);
130+
assertMenuPath("Foo > Bar > Scripts > brown", scripts, 2);
131+
assertMenuPath("Foo > Bar > Math > Trig > cos", scripts, 3);
132+
assertMenuPath("Foo > Bar > Math > divide", scripts, 4);
133+
assertMenuPath("Foo > Bar > Scripts > fox", scripts, 5);
134+
assertMenuPath("Foo > Bar > ignored", scripts, 6);
135+
assertMenuPath("Foo > Bar > Math > multiply", scripts, 7);
136+
assertMenuPath("Foo > Bar > Scripts > quick", scripts, 8);
137+
assertMenuPath("Foo > Bar > Math > Trig > sin", scripts, 9);
138+
assertMenuPath("Foo > Bar > Math > subtract", scripts, 10);
139+
assertMenuPath("Foo > Bar > Math > Trig > tan", scripts, 11);
140+
}
141+
142+
/**
143+
* Tests that scripts are discovered only once when present in multiple base
144+
* directories.
145+
*/
146+
@Test
147+
public void testOverlappingDirectories() {
148+
final ScriptService scriptService = createScriptService();
149+
150+
// Scripts -> Plugins
151+
scriptService.addScriptDirectory(new File(scriptsDir, "Scripts"),
152+
new MenuPath("Plugins"));
153+
// everything else "in place"
154+
scriptService.addScriptDirectory(scriptsDir);
155+
156+
final ArrayList<ScriptInfo> scripts = findScripts(scriptService);
157+
158+
assertEquals(11, scripts.size());
159+
assertMenuPath("Plugins > The Lazy Dog", scripts, 0);
160+
assertMenuPath("Math > add", scripts, 1);
161+
assertMenuPath("Plugins > brown", scripts, 2);
162+
assertMenuPath("Math > Trig > cos", scripts, 3);
163+
assertMenuPath("Math > divide", scripts, 4);
164+
assertMenuPath("Plugins > fox", scripts, 5);
165+
assertMenuPath("Math > multiply", scripts, 6);
166+
assertMenuPath("Plugins > quick", scripts, 7);
167+
assertMenuPath("Math > Trig > sin", scripts, 8);
168+
assertMenuPath("Math > subtract", scripts, 9);
169+
assertMenuPath("Math > Trig > tan", scripts, 10);
109170
}
110171

111172
// -- Helper methods --
112173

174+
private ScriptService createScriptService() {
175+
final Context context = new Context(ScriptService.class);
176+
final ScriptService scriptService = context.service(ScriptService.class);
177+
final File defaultScriptsDir =
178+
new File(AppUtils.getBaseDirectory(ScriptFinder.class), "scripts");
179+
scriptService.removeScriptDirectory(defaultScriptsDir);
180+
return scriptService;
181+
}
182+
183+
private ArrayList<ScriptInfo> findScripts(final ScriptService scriptService) {
184+
final ScriptFinder scriptFinder = new ScriptFinder(scriptService);
185+
final ArrayList<ScriptInfo> scripts = new ArrayList<ScriptInfo>();
186+
scriptFinder.findScripts(scripts);
187+
Collections.sort(scripts);
188+
return scripts;
189+
}
190+
113191
private void assertMenuPath(final String menuString,
114192
final ArrayList<ScriptInfo> scripts, final int i)
115193
{

0 commit comments

Comments
 (0)