From 1ee8c8ad3afbd4f0e4f1f78a790f35f85ef90217 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 2 Mar 2020 10:13:29 -0600 Subject: [PATCH 1/4] Issue #4628 - Non-Required Module Dependency Support Signed-off-by: Joakim Erdfelt --- .../java/org/eclipse/jetty/start/Module.java | 17 +- .../jetty/start/ModuleGraphWriter.java | 1 + .../java/org/eclipse/jetty/start/Modules.java | 54 +++++-- .../org/eclipse/jetty/start/ModulesTest.java | 148 ++++++++++++++++++ .../non-required-deps/modules/bar-dive.mod | 2 + .../non-required-deps/modules/bar.mod | 6 + .../non-required-deps/modules/foo.mod | 1 + .../modules/impls/bar-dynamic.mod | 2 + 8 files changed, 211 insertions(+), 20 deletions(-) create mode 100644 jetty-start/src/test/resources/non-required-deps/modules/bar-dive.mod create mode 100644 jetty-start/src/test/resources/non-required-deps/modules/bar.mod create mode 100644 jetty-start/src/test/resources/non-required-deps/modules/foo.mod create mode 100644 jetty-start/src/test/resources/non-required-deps/modules/impls/bar-dynamic.mod 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 From f2a4c6b6150938c03022f29f3fa25bed5b7451f3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 2 Mar 2020 10:18:46 -0600 Subject: [PATCH 2/4] Issue #4620 - Better support for alt PrintStream in StdErrLog Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/util/log/StdErrLog.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/log/StdErrLog.java b/jetty-util/src/main/java/org/eclipse/jetty/util/log/StdErrLog.java index efc8d3780ca..cade8de869c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/log/StdErrLog.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/log/StdErrLog.java @@ -135,7 +135,8 @@ public class StdErrLog extends AbstractLogger private int _level; // Level that this Logger was configured as (remembered in special case of .setDebugEnabled()) private int _configuredLevel; - private PrintStream _stderr = System.err; + // The alternate stream to print to (if set) + private PrintStream _altStream; private boolean _source; // Print the long form names, otherwise use abbreviated private boolean _printLongNames = __long; @@ -389,16 +390,14 @@ public class StdErrLog extends AbstractLogger this._level = level; } + /** + * The alternate stream to use for STDERR. + * + * @param stream the stream of choice, or {@code null} to use {@link System#err} + */ public void setStdErrStream(PrintStream stream) { - if (stream == null) - { - this._stderr = System.err; - } - else - { - this._stderr = stream; - } + this._altStream = stream; } @Override @@ -442,7 +441,14 @@ public class StdErrLog extends AbstractLogger private void println(StringBuilder builder) { - _stderr.println(builder); + if (_altStream != null) + _altStream.println(builder); + else + { + // We always use the PrintStream stored in System.err, + // just in case someone has replaced it with a call to System.setErr(PrintStream) + System.err.println(builder); + } } private void format(StringBuilder builder, String level, String msg, Object... inArgs) @@ -648,7 +654,7 @@ public class StdErrLog extends AbstractLogger StdErrLog logger = new StdErrLog(fullname); // Preserve configuration for new loggers configuration logger.setPrintLongNames(_printLongNames); - logger._stderr = this._stderr; + logger._altStream = this._altStream; // Force the child to have any programmatic configuration if (_level != _configuredLevel) From 3a775423038524463d3cea008f6a7ddb6a5eda22 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 5 Mar 2020 09:10:12 +1000 Subject: [PATCH 3/4] upgrade shade plugin and align version (#4639) Signed-off-by: olivier lamy --- jetty-websocket/websocket-client/pom.xml | 1 - pom.xml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/jetty-websocket/websocket-client/pom.xml b/jetty-websocket/websocket-client/pom.xml index e90c32a58cc..19042689ad8 100644 --- a/jetty-websocket/websocket-client/pom.xml +++ b/jetty-websocket/websocket-client/pom.xml @@ -104,7 +104,6 @@ org.apache.maven.plugins maven-shade-plugin - 2.4.3 package diff --git a/pom.xml b/pom.xml index dc8feb9356f..07ea30698eb 100644 --- a/pom.xml +++ b/pom.xml @@ -640,7 +640,7 @@ org.apache.maven.plugins maven-shade-plugin - 3.1.1 + 3.2.2 org.apache.maven.plugins From 67eb9b35311dfd67b149204105a4f980772fa755 Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Sun, 8 Mar 2020 21:10:39 +1000 Subject: [PATCH 4/4] use same version as in pom Signed-off-by: olivier lamy --- .../main/config/modules/session-store-hazelcast-embedded.mod | 2 +- .../main/config/modules/session-store-hazelcast-remote.mod | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-embedded.mod b/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-embedded.mod index 374493bc509..3d8ba5a1174 100644 --- a/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-embedded.mod +++ b/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-embedded.mod @@ -13,7 +13,7 @@ session-store sessions [files] -maven://com.hazelcast/hazelcast/3.9.4|lib/hazelcast/hazelcast-3.9.4.jar +maven://com.hazelcast/hazelcast/3.12.6|lib/hazelcast/hazelcast-3.12.6.jar [xml] etc/sessions/hazelcast/default.xml diff --git a/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-remote.mod b/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-remote.mod index 3796bef5924..e3d67a3d387 100644 --- a/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-remote.mod +++ b/jetty-hazelcast/src/main/config/modules/session-store-hazelcast-remote.mod @@ -13,8 +13,8 @@ session-store sessions [files] -maven://com.hazelcast/hazelcast/3.9.4|lib/hazelcast/hazelcast-3.9.4.jar -maven://com.hazelcast/hazelcast-client/3.9.4|lib/hazelcast/hazelcast-client-3.9.4.jar +maven://com.hazelcast/hazelcast/3.12.6|lib/hazelcast/hazelcast-3.12.6.jar +maven://com.hazelcast/hazelcast-client/3.12.6|lib/hazelcast/hazelcast-client-3.12.6.jar [xml] etc/sessions/hazelcast/remote.xml