diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java index a4d1097a5df..fbd4bfc39f1 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.start; import java.io.BufferedReader; import java.io.BufferedWriter; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintWriter; import java.nio.charset.StandardCharsets; @@ -154,12 +153,12 @@ public class Module implements Comparable private final List _license = new ArrayList<>(); /** - * Dependencies + * Dependencies from {@code [depends]} section */ private final List _depends = new ArrayList<>(); /** - * Optional + * Optional dependencies from {@code [optional]} section are structural in nature. */ private final Set _optional = new HashSet<>(); @@ -196,6 +195,18 @@ public class Module implements Comparable process(basehome); } + public static boolean isRequiredDependency(String depends) + { + return (depends != null) && (depends.charAt(0) != '?'); + } + + public static String normalizeModuleName(String name) + { + if (!isRequiredDependency(name)) + return name.substring(1); + return name; + } + public String getName() { return _name; diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java b/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java index b671490597a..8fa82380500 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java @@ -247,6 +247,7 @@ public class ModuleGraphWriter { for (String depends : module.getDepends()) { + depends = Module.normalizeModuleName(depends); out.printf(" \"%s\" -> \"%s\";%n", module.getName(), depends); } for (String optional : module.getOptional()) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java index f8c4dc37b94..bc33e279828 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.start; import java.io.FileInputStream; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; @@ -132,7 +133,12 @@ public class Modules implements Iterable label = " Depend: %s"; for (String parent : module.getDepends()) { + parent = Module.normalizeModuleName(parent); System.out.printf(label, parent); + if (!Module.isRequiredDependency(parent)) + { + System.out.print(" [not-required]"); + } label = ", %s"; } System.out.println(); @@ -352,45 +358,59 @@ public class Modules implements Iterable // Process module dependencies (always processed as may be dynamic) StartLog.debug("Enabled module %s depends on %s", module.getName(), module.getDepends()); - for (String dependsOn : module.getDepends()) + for (String dependsOnRaw : module.getDepends()) { - // Look for modules that provide that dependency - Set providers = getAvailableProviders(dependsOn); + boolean isRequired = Module.isRequiredDependency(dependsOnRaw); + // Final to allow lambda's below to use name + final String dependentModule = Module.normalizeModuleName(dependsOnRaw); - StartLog.debug("Module %s depends on %s provided by %s", module, dependsOn, providers); + // Look for modules that provide that dependency + Set providers = getAvailableProviders(dependentModule); + + StartLog.debug("Module %s depends on %s provided by %s", module, dependentModule, providers); // If there are no known providers of the module if (providers.isEmpty()) { // look for a dynamic module - if (dependsOn.contains("/")) + if (dependentModule.contains("/")) { - Path file = _baseHome.getPath("modules/" + dependsOn + ".mod"); - registerModule(file).expandDependencies(_args.getProperties()); - providers = _provided.get(dependsOn); - if (providers == null || providers.isEmpty()) - throw new UsageException("Module %s does not provide %s", _baseHome.toShortForm(file), dependsOn); + Path file = _baseHome.getPath("modules/" + dependentModule + ".mod"); + if (isRequired || Files.exists(file)) + { + registerModule(file).expandDependencies(_args.getProperties()); + providers = _provided.get(dependentModule); + if (providers == null || providers.isEmpty()) + throw new UsageException("Module %s does not provide %s", _baseHome.toShortForm(file), dependentModule); - enable(newlyEnabled, providers.stream().findFirst().get(), "dynamic dependency of " + module.getName(), true); + enable(newlyEnabled, providers.stream().findFirst().get(), "dynamic dependency of " + module.getName(), true); + continue; + } + } + // is this a non-required module + if (!isRequired) + { + StartLog.debug("Skipping non-required module [%s]: doesn't exist", dependentModule); continue; } - throw new UsageException("No module found to provide %s for %s", dependsOn, module); + // throw an exception (not a dynamic module and a required dependency) + throw new UsageException("No module found to provide %s for %s", dependentModule, module); } // If a provider is already enabled, then add a transitive enable - if (providers.stream().filter(Module::isEnabled).count() > 0) - providers.stream().filter(m -> m.isEnabled() && !m.equals(module)).forEach(m -> enable(newlyEnabled, m, "transitive provider of " + dependsOn + " for " + module.getName(), true)); + if (providers.stream().anyMatch(Module::isEnabled)) + providers.stream().filter(m -> m.isEnabled() && !m.equals(module)).forEach(m -> enable(newlyEnabled, m, "transitive provider of " + dependentModule + " for " + module.getName(), true)); else { // Is there an obvious default? Optional dftProvider = (providers.size() == 1) ? providers.stream().findFirst() - : providers.stream().filter(m -> m.getName().equals(dependsOn)).findFirst(); + : providers.stream().filter(m -> m.getName().equals(dependentModule)).findFirst(); if (dftProvider.isPresent()) - enable(newlyEnabled, dftProvider.get(), "transitive provider of " + dependsOn + " for " + module.getName(), true); + enable(newlyEnabled, dftProvider.get(), "transitive provider of " + dependentModule + " for " + module.getName(), true); else if (StartLog.isDebugEnabled()) - StartLog.debug("Module %s requires a %s implementation from one of %s", module, dependsOn, providers); + StartLog.debug("Module %s requires a %s implementation from one of %s", module, dependentModule, providers); } } } diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java index bb6c8638214..461351ceea5 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java @@ -36,6 +36,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; @ExtendWith(WorkDirExtension.class) public class ModulesTest @@ -202,6 +204,152 @@ public class ModulesTest assertThat("Resolved XMLs: " + actualXmls, actualXmls, contains(expectedXmls.toArray())); } + @Test + public void testResolveNotRequiredModuleNotFound() throws IOException + { + // Test Env + File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps"); + File baseDir = testdir.getEmptyPathDir().toFile(); + String[] cmdLine = new String[]{"bar.type=cannot-find-me"}; + + // Configuration + CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine); + ConfigSources config = new ConfigSources(); + config.add(cmdLineSource); + config.add(new JettyHomeConfigSource(homeDir.toPath())); + config.add(new JettyBaseConfigSource(baseDir.toPath())); + + // Initialize + BaseHome basehome = new BaseHome(config); + + StartArgs args = new StartArgs(basehome); + args.parse(config); + + // Test Modules + Modules modules = new Modules(basehome, args); + modules.registerAll(); + + // Enable module + modules.enable("bar", TEST_SOURCE); + + // Collect active module list + List active = modules.getEnabled(); + + // Assert names are correct, and in the right order + List expectedNames = new ArrayList<>(); + expectedNames.add("foo"); + expectedNames.add("bar"); + + List actualNames = new ArrayList<>(); + for (Module actual : active) + { + actualNames.add(actual.getName()); + } + + assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray())); + + Props props = args.getProperties(); + assertThat(props.getString("bar.name"), is(nullValue())); + } + + @Test + public void testResolveNotRequiredModuleFound() throws IOException + { + // Test Env + File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps"); + File baseDir = testdir.getEmptyPathDir().toFile(); + String[] cmdLine = new String[]{"bar.type=dive"}; + + // Configuration + CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine); + ConfigSources config = new ConfigSources(); + config.add(cmdLineSource); + config.add(new JettyHomeConfigSource(homeDir.toPath())); + config.add(new JettyBaseConfigSource(baseDir.toPath())); + + // Initialize + BaseHome basehome = new BaseHome(config); + + StartArgs args = new StartArgs(basehome); + args.parse(config); + + // Test Modules + Modules modules = new Modules(basehome, args); + modules.registerAll(); + + // Enable module + modules.enable("bar", TEST_SOURCE); + + // Collect active module list + List active = modules.getEnabled(); + + // Assert names are correct, and in the right order + List expectedNames = new ArrayList<>(); + expectedNames.add("foo"); + expectedNames.add("bar"); + expectedNames.add("bar-dive"); + + List actualNames = new ArrayList<>(); + for (Module actual : active) + { + actualNames.add(actual.getName()); + } + + assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray())); + + Props props = args.getProperties(); + assertThat(props.getString("bar.name"), is("dive")); + } + + @Test + public void testResolveNotRequiredModuleFoundDynamic() throws IOException + { + // Test Env + File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps"); + File baseDir = testdir.getEmptyPathDir().toFile(); + String[] cmdLine = new String[]{"bar.type=dynamic"}; + + // Configuration + CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine); + ConfigSources config = new ConfigSources(); + config.add(cmdLineSource); + config.add(new JettyHomeConfigSource(homeDir.toPath())); + config.add(new JettyBaseConfigSource(baseDir.toPath())); + + // Initialize + BaseHome basehome = new BaseHome(config); + + StartArgs args = new StartArgs(basehome); + args.parse(config); + + // Test Modules + Modules modules = new Modules(basehome, args); + modules.registerAll(); + + // Enable module + modules.enable("bar", TEST_SOURCE); + + // Collect active module list + List active = modules.getEnabled(); + + // Assert names are correct, and in the right order + List expectedNames = new ArrayList<>(); + expectedNames.add("foo"); + expectedNames.add("bar"); + expectedNames.add("impls/bar-dynamic"); + + List actualNames = new ArrayList<>(); + for (Module actual : active) + { + actualNames.add(actual.getName()); + } + + assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray())); + + Props props = args.getProperties(); + assertThat(props.getString("bar.name"), is("dynamic")); + } + private List normalizeLibs(List active) { List libs = new ArrayList<>(); diff --git a/jetty-start/src/test/resources/non-required-deps/modules/bar-dive.mod b/jetty-start/src/test/resources/non-required-deps/modules/bar-dive.mod new file mode 100644 index 00000000000..03e6e66414c --- /dev/null +++ b/jetty-start/src/test/resources/non-required-deps/modules/bar-dive.mod @@ -0,0 +1,2 @@ +[ini] +bar.name=dive \ No newline at end of file diff --git a/jetty-start/src/test/resources/non-required-deps/modules/bar.mod b/jetty-start/src/test/resources/non-required-deps/modules/bar.mod new file mode 100644 index 00000000000..debf4d33799 --- /dev/null +++ b/jetty-start/src/test/resources/non-required-deps/modules/bar.mod @@ -0,0 +1,6 @@ +# Top level mod + +[depends] +foo +?bar-${bar.type} +?impls/bar-${bar.type} \ No newline at end of file diff --git a/jetty-start/src/test/resources/non-required-deps/modules/foo.mod b/jetty-start/src/test/resources/non-required-deps/modules/foo.mod new file mode 100644 index 00000000000..05bbd0f2d91 --- /dev/null +++ b/jetty-start/src/test/resources/non-required-deps/modules/foo.mod @@ -0,0 +1 @@ +# nothing here \ No newline at end of file diff --git a/jetty-start/src/test/resources/non-required-deps/modules/impls/bar-dynamic.mod b/jetty-start/src/test/resources/non-required-deps/modules/impls/bar-dynamic.mod new file mode 100644 index 00000000000..66d12f46563 --- /dev/null +++ b/jetty-start/src/test/resources/non-required-deps/modules/impls/bar-dynamic.mod @@ -0,0 +1,2 @@ +[ini] +bar.name=dynamic \ No newline at end of file