From 303e031235b687abb2dce2919580bdbb2ac78958 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 24 Mar 2021 23:31:36 +1100 Subject: [PATCH] Added before dependents to Modules (#6080) Added `before` section to a module to control ordering rename `options` section to `after` Signed-off-by: Greg Wilkins Co-authored-by: Simone Bordet --- .../operations-guide/modules/modules.adoc | 49 ++++++++-- .../src/main/config/modules/https.mod | 2 +- .../src/main/config/modules/server.mod | 2 +- .../java/org/eclipse/jetty/start/Module.java | 43 +++++++-- .../jetty/start/ModuleGraphWriter.java | 8 +- .../java/org/eclipse/jetty/start/Modules.java | 38 ++++++-- .../test/resources/dist-home/modules/base.mod | 2 +- .../test/resources/dist-home/modules/main.mod | 2 +- .../alternate/modules/noDftOptionA.mod | 2 +- .../resources/usecases/ordered.3.assert.txt | 4 + .../resources/usecases/ordered.3.cmdline.txt | 2 + .../resources/usecases/ordered/etc/before.xml | 0 .../usecases/ordered/modules/before.mod | 9 ++ .../modules/transient.mod | 2 +- .../modules/transient.mod | 2 +- .../tests/distribution/DistributionTests.java | 91 +++++++++++++++++++ 16 files changed, 228 insertions(+), 30 deletions(-) create mode 100644 jetty-start/src/test/resources/usecases/ordered.3.assert.txt create mode 100644 jetty-start/src/test/resources/usecases/ordered.3.cmdline.txt create mode 100644 jetty-start/src/test/resources/usecases/ordered/etc/before.xml create mode 100644 jetty-start/src/test/resources/usecases/ordered/modules/before.mod diff --git a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc index 64f515622bf8..67c414717672 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc @@ -198,16 +198,51 @@ Circular dependencies are not allowed. The order of the enabled modules is used to determine the processing of the configuration, for example the order of processing of the xref:og-modules-directive-files[+[files]+] section, the order of processing of XML files defined in the xref:og-modules-directive-xml[+[xml]+] section, etc. -[[og-modules-directive-optional]] -===== [optional] +[[og-modules-directive-after]] +===== [after] -A list of module names that this module depends on, if they are explicitly enabled. +This directive indicates that this module is ordered after the listed module names, if they are enabled. -For example, the module `https` has an optional dependency on `http2`. -Enabling the `https` modules _does not_ enable the `http2` module. +For example, module `https` is `[after]` module `http2`. +Enabling the `https` module _does not_ enable the `http2` module. -However, if the `http2` module is explicitly enabled, then the `https` module depends on it, and therefore the `http2` module is xref:og-modules-directive-depends[sorted] _before_ the `https` module. -In this way, you are guaranteed that the `http2` module is processed before the `https` module. +However, if the `http2` module is enabled (explicitly or transitively), then the `https` module is xref:og-modules-directive-depends[sorted] _after_ the `http2` module. +In this way, you are guaranteed that the `https` module is processed after the `http2` module. + +[[og-modules-directive-before]] +===== [before] + +This directive indicates that this module is ordered before the listed module names, if they are enabled. + +One use of this directive is to create a prerequisite module without the need to modify the `depends` directive of an existing module. +For example, to create a custom `org.eclipse.jetty.server.Server` subclass instance to be used by the standard `server` module, without modifying the existing `server.mod` file nor the `jetty.xml` file that it uses. This can be achieved by creating the `custom-server` xref:og-modules-custom[Jetty custom module]: + +.custom-server.mod +---- +[description] +This module creates a custom Server subclass instance. + +[before] +server + +[xml] +etc/custom-server.xml +---- + +The `custom-server.xml` file is the following: + +.custom-server.xml +[source,xml] +---- + + + + +---- + +The presence of the `[before]` directive in `custom-server.mod` causes the processing of the `custom-server.xml` file to happen before the processing of the standard `jetty.xml` file referenced by the standard `server.mod` Jetty module. + +Thus, the instance assigned to the `Server` identifier is your custom `com.acme.server.CustomJettyServer` instance from the `custom-server.xml` file; this instance is then used while processing the `jetty.xml` file. [[og-modules-directive-files]] ===== [files] diff --git a/jetty-server/src/main/config/modules/https.mod b/jetty-server/src/main/config/modules/https.mod index 23cd62fac5fa..943191df3094 100644 --- a/jetty-server/src/main/config/modules/https.mod +++ b/jetty-server/src/main/config/modules/https.mod @@ -12,7 +12,7 @@ ssl [depend] ssl -[optional] +[after] alpn http2 http-forwarded diff --git a/jetty-server/src/main/config/modules/server.mod b/jetty-server/src/main/config/modules/server.mod index abf0c66f9311..f9559177fde1 100644 --- a/jetty-server/src/main/config/modules/server.mod +++ b/jetty-server/src/main/config/modules/server.mod @@ -3,7 +3,7 @@ Enables and configures the Jetty server. This module does not enable any network protocol support. To enable a specific network protocol such as HTTP/1.1, you must enable the correspondent Jetty module. -[optional] +[after] jvm ext resources 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 ab6ff17531ad..dd7c21a73756 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 @@ -154,11 +154,16 @@ public class Module implements Comparable private final List _depends = new ArrayList<>(); /** - * Optional dependencies from {@code [optional]} section are structural in nature. + * Module names from {@code [before]} section */ - private final Set _optional = new HashSet<>(); + private final Set _before = new HashSet<>(); - public Module(BaseHome basehome, Path path) throws FileNotFoundException, IOException + /** + * Module names from {@code [after]} section + */ + private final Set _after = new HashSet<>(); + + public Module(BaseHome basehome, Path path) throws IOException { super(); _path = path; @@ -237,9 +242,12 @@ public void expandDependencies(Props props) List tmp = _depends.stream().map(expander).collect(Collectors.toList()); _depends.clear(); _depends.addAll(tmp); - tmp = _optional.stream().map(expander).collect(Collectors.toList()); - _optional.clear(); - _optional.addAll(tmp); + tmp = _after.stream().map(expander).collect(Collectors.toList()); + _after.clear(); + _after.addAll(tmp); + tmp = _before.stream().map(expander).collect(Collectors.toList()); + _before.clear(); + _before.addAll(tmp); } public List getDefaultConfig() @@ -426,8 +434,12 @@ public void process(BaseHome basehome) throws FileNotFoundException, IOException case "PROVIDES": _provides.add(line); break; + case "BEFORE": + _before.add(line); + break; case "OPTIONAL": - _optional.add(line); + case "AFTER": + _after.add(line); break; case "EXEC": _jvmArgs.add(line); @@ -509,9 +521,24 @@ public Set getProvides() return new HashSet<>(_provides); } + public Set getBefore() + { + return Set.copyOf(_before); + } + + public Set getAfter() + { + return Set.copyOf(_after); + } + + /** + * @return the module names in the [after] section + * @deprecated use {@link #getAfter()} instead + */ + @Deprecated public Set getOptional() { - return new HashSet<>(_optional); + return getAfter(); } public List getDescription() 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 a348887eeaef..665a5849f728 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 @@ -245,9 +245,13 @@ private void writeRelationships(PrintWriter out, Iterable modules, List< depends = Module.normalizeModuleName(depends); out.printf(" \"%s\" -> \"%s\";%n", module.getName(), depends); } - for (String optional : module.getOptional()) + for (String before : module.getBefore()) { - out.printf(" \"%s\" => \"%s\";%n", module.getName(), optional); + out.printf(" \"%s\" << \"%s\";%n", module.getName(), before); + } + for (String after : module.getAfter()) + { + out.printf(" \"%s\" >> \"%s\";%n", module.getName(), after); } } } 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 ce48de8e416a..17bc231e8d58 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 @@ -121,12 +121,22 @@ public void showModules(List modules) } System.out.println(); } - if (!module.getOptional().isEmpty()) + if (!module.getBefore().isEmpty()) { - label = " Optional: %s"; - for (String parent : module.getOptional()) + label = " Before: %s"; + for (String before : module.getBefore()) { - System.out.printf(label, parent); + System.out.printf(label, before); + label = ", %s"; + } + System.out.println(); + } + if (!module.getAfter().isEmpty()) + { + label = " After: %s"; + for (String after : module.getAfter()) + { + System.out.printf(label, after); label = ", %s"; } System.out.println(); @@ -310,14 +320,22 @@ public List getEnabled() Set provided = _provided.get(name); if (provided != null) + { for (Module p : provided) { if (p.isEnabled()) sort.addDependency(module, p); } + } }; module.getDepends().forEach(add); - module.getOptional().forEach(add); + module.getAfter().forEach(add); + module.getBefore().forEach(name -> + { + Module before = _names.get(name); + if (before != null && before.isEnabled()) + sort.addDependency(before, module); + }); } sort.sort(enabled); @@ -339,13 +357,21 @@ public List getSortedAll() Set provided = _provided.get(name); if (provided != null) + { for (Module p : provided) { sort.addDependency(module, p); } + } }; module.getDepends().forEach(add); - module.getOptional().forEach(add); + module.getAfter().forEach(add); + module.getBefore().forEach(name -> + { + Module before = _names.get(name); + if (before != null) + sort.addDependency(before, module); + }); } sort.sort(all); diff --git a/jetty-start/src/test/resources/dist-home/modules/base.mod b/jetty-start/src/test/resources/dist-home/modules/base.mod index e927af45f14f..0a8f3a34dc56 100644 --- a/jetty-start/src/test/resources/dist-home/modules/base.mod +++ b/jetty-start/src/test/resources/dist-home/modules/base.mod @@ -1,4 +1,4 @@ -[optional] +[after] optional [lib] diff --git a/jetty-start/src/test/resources/dist-home/modules/main.mod b/jetty-start/src/test/resources/dist-home/modules/main.mod index 7b041ac74558..8fd93990e8a5 100644 --- a/jetty-start/src/test/resources/dist-home/modules/main.mod +++ b/jetty-start/src/test/resources/dist-home/modules/main.mod @@ -6,7 +6,7 @@ Example of a module [depend] base -[optional] +[after] optional [lib] diff --git a/jetty-start/src/test/resources/usecases/alternate/modules/noDftOptionA.mod b/jetty-start/src/test/resources/usecases/alternate/modules/noDftOptionA.mod index cfa7551c2532..47325b365803 100644 --- a/jetty-start/src/test/resources/usecases/alternate/modules/noDftOptionA.mod +++ b/jetty-start/src/test/resources/usecases/alternate/modules/noDftOptionA.mod @@ -1,7 +1,7 @@ [provides] noDft -[optional] +[after] default [ini] diff --git a/jetty-start/src/test/resources/usecases/ordered.3.assert.txt b/jetty-start/src/test/resources/usecases/ordered.3.assert.txt new file mode 100644 index 000000000000..14040c9cb0ec --- /dev/null +++ b/jetty-start/src/test/resources/usecases/ordered.3.assert.txt @@ -0,0 +1,4 @@ +## The XMLs we expect (order is important) +XML|${jetty.base}/etc/alternateA.xml +XML|${jetty.base}/etc/before.xml +XML|${jetty.base}/etc/dependent.xml diff --git a/jetty-start/src/test/resources/usecases/ordered.3.cmdline.txt b/jetty-start/src/test/resources/usecases/ordered.3.cmdline.txt new file mode 100644 index 000000000000..229ce9049d46 --- /dev/null +++ b/jetty-start/src/test/resources/usecases/ordered.3.cmdline.txt @@ -0,0 +1,2 @@ +--module=dependent,alternateA,before + diff --git a/jetty-start/src/test/resources/usecases/ordered/etc/before.xml b/jetty-start/src/test/resources/usecases/ordered/etc/before.xml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/jetty-start/src/test/resources/usecases/ordered/modules/before.mod b/jetty-start/src/test/resources/usecases/ordered/modules/before.mod new file mode 100644 index 000000000000..57d0aaedadcd --- /dev/null +++ b/jetty-start/src/test/resources/usecases/ordered/modules/before.mod @@ -0,0 +1,9 @@ + +[Depends] +alternate + +[Before] +dependent + +[xml] +etc/before.xml \ No newline at end of file diff --git a/jetty-start/src/test/resources/usecases/transientWithIniTemplate/modules/transient.mod b/jetty-start/src/test/resources/usecases/transientWithIniTemplate/modules/transient.mod index 05fb78407ea7..16b7210f50eb 100644 --- a/jetty-start/src/test/resources/usecases/transientWithIniTemplate/modules/transient.mod +++ b/jetty-start/src/test/resources/usecases/transientWithIniTemplate/modules/transient.mod @@ -1,5 +1,5 @@ [xml] etc/t.xml -[optional] +[after] main diff --git a/jetty-start/src/test/resources/usecases/transientWithoutIniTemplate/modules/transient.mod b/jetty-start/src/test/resources/usecases/transientWithoutIniTemplate/modules/transient.mod index 57a3e004817a..e4876194e3b4 100644 --- a/jetty-start/src/test/resources/usecases/transientWithoutIniTemplate/modules/transient.mod +++ b/jetty-start/src/test/resources/usecases/transientWithoutIniTemplate/modules/transient.mod @@ -1,7 +1,7 @@ [xml] etc/t.xml -[optional] +[after] main [ini] diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index b99ddccfa6ad..0924c38af361 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -768,4 +768,95 @@ public void testJavaUtilLoggingBridge() throws Exception } } } + + @Test + public void testBeforeDirectiveInModule() throws Exception + { + String jettyVersion = System.getProperty("jettyVersion"); + JettyHomeTester distribution = JettyHomeTester.Builder.newInstance() + .jettyVersion(jettyVersion) + .mavenLocalRepository(System.getProperty("mavenRepoPath")) + .build(); + + String[] args1 = { + "--add-modules=https,test-keystore" + }; + + try (JettyHomeTester.Run run1 = distribution.start(args1)) + { + assertTrue(run1.awaitFor(10, TimeUnit.SECONDS)); + assertEquals(0, run1.getExitValue()); + + Path jettyBase = run1.getConfig().getJettyBase(); + + Path jettyBaseEtc = jettyBase.resolve("etc"); + Files.createDirectories(jettyBaseEtc); + Path sslPatchXML = jettyBaseEtc.resolve("ssl-patch.xml"); + String nextProtocol = "fcgi/1.0"; + String xml = "" + + "" + + "" + + "" + + " " + + " " + + " " + + " " + nextProtocol + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + Files.write(sslPatchXML, List.of(xml), StandardOpenOption.CREATE); + + Path jettyBaseModules = jettyBase.resolve("modules"); + Files.createDirectories(jettyBaseModules); + Path sslPatchModule = jettyBaseModules.resolve("ssl-patch.mod"); + String module = "" + + "[depends]\n" + + "fcgi\n" + + "\n" + + "[before]\n" + + "https\n" + + "http2\n" + // http2 is not explicitly enabled. + "\n" + + "[after]\n" + + "ssl\n" + + "\n" + + "[xml]\n" + + "etc/ssl-patch.xml\n"; + Files.write(sslPatchModule, List.of(module), StandardOpenOption.CREATE); + + String[] args2 = { + "--add-modules=ssl-patch" + }; + + try (JettyHomeTester.Run run2 = distribution.start(args2)) + { + assertTrue(run2.awaitFor(10, TimeUnit.SECONDS)); + assertEquals(0, run2.getExitValue()); + + int port = distribution.freePort(); + try (JettyHomeTester.Run run3 = distribution.start("jetty.http.port=" + port)) + { + assertTrue(run3.awaitConsoleLogsFor("Started Server@", 10, TimeUnit.SECONDS)); + + // Check for the protocol order: fcgi must be after ssl and before http. + assertTrue(run3.getLogs().stream() + .anyMatch(log -> log.contains("(ssl, fcgi/1.0, http/1.1)"))); + + // Protocol "h2" must not be enabled because the + // http2 Jetty module was not explicitly enabled. + assertFalse(run3.getLogs().stream() + .anyMatch(log -> log.contains("h2"))); + } + } + } + } }