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 <gregw@webtide.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>

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]
+----
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
+<Configure id="Server" class="com.acme.server.CustomJettyServer">
+</Configure>
+----
+
+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]

jetty-server/src/main/config/modules/https.mod

@@ -12,7 +12,7 @@ ssl
 [depend]
 ssl
 
-[optional]
+[after]
 alpn
 http2
 http-forwarded

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

jetty-start/src/main/java/org/eclipse/jetty/start/Module.java

@@ -154,11 +154,16 @@ public class Module implements Comparable<Module>
     private final List<String> _depends = new ArrayList<>();
 
     /**
-     * Optional dependencies from {@code [optional]} section are structural in nature.
+     * Module names from {@code [before]} section
      */
-    private final Set<String> _optional = new HashSet<>();
+    private final Set<String> _before = new HashSet<>();
 
-    public Module(BaseHome basehome, Path path) throws FileNotFoundException, IOException
+    /**
+     * Module names from {@code [after]} section
+     */
+    private final Set<String> _after = new HashSet<>();
+
+    public Module(BaseHome basehome, Path path) throws IOException
     {
         super();
         _path = path;
@@ -237,9 +242,12 @@ public void expandDependencies(Props props)
         List<String> 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<String> 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<String> getProvides()
         return new HashSet<>(_provides);
     }
 
+    public Set<String> getBefore()
+    {
+        return Set.copyOf(_before);
+    }
+
+    public Set<String> getAfter()
+    {
+        return Set.copyOf(_after);
+    }
+
+    /**
+     * @return the module names in the [after] section
+     * @deprecated use {@link #getAfter()} instead
+     */
+    @Deprecated
     public Set<String> getOptional()
     {
-        return new HashSet<>(_optional);
+        return getAfter();
     }
 
     public List<String> getDescription()

jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java

@@ -245,9 +245,13 @@ private void writeRelationships(PrintWriter out, Iterable<Module> 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);
             }
         }
     }

jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java

@@ -121,12 +121,22 @@ public void showModules(List<String> 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<Module> getEnabled()
 
                 Set<Module> 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<Module> getSortedAll()
 
                 Set<Module> 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);

jetty-start/src/test/resources/dist-home/modules/base.mod

@@ -1,4 +1,4 @@
-[optional]
+[after]
 optional
 
 [lib]

jetty-start/src/test/resources/dist-home/modules/main.mod

@@ -6,7 +6,7 @@ Example of a module
 [depend]
 base
 
-[optional]
+[after]
 optional
 
 [lib]

jetty-start/src/test/resources/usecases/alternate/modules/noDftOptionA.mod

@@ -1,7 +1,7 @@
 [provides]
 noDft
 
-[optional]
+[after]
 default
 
 [ini]

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

jetty-start/src/test/resources/usecases/ordered.3.cmdline.txt

@@ -0,0 +1,2 @@
+--module=dependent,alternateA,before
+

jetty-start/src/test/resources/usecases/ordered/modules/before.mod

@@ -0,0 +1,9 @@
+
+[Depends]
+alternate
+
+[Before]
+dependent
+
+[xml]
+etc/before.xml

jetty-start/src/test/resources/usecases/transientWithIniTemplate/modules/transient.mod

@@ -1,5 +1,5 @@
 [xml]
 etc/t.xml
 
-[optional]
+[after]
 main

jetty-start/src/test/resources/usecases/transientWithoutIniTemplate/modules/transient.mod

@@ -1,7 +1,7 @@
 [xml]
 etc/t.xml
 
-[optional]
+[after]
 main
 
 [ini]

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 = "" +
+                "<?xml version=\"1.0\"?>" +
+                "<!DOCTYPE Configure PUBLIC \"-//Jetty//Configure//EN\" \"https://www.eclipse.org/jetty/configure_10_0.dtd\">" +
+                "<Configure id=\"sslConnector\" class=\"org.eclipse.jetty.server.ServerConnector\">" +
+                "  <Call name=\"addIfAbsentConnectionFactory\">" +
+                "    <Arg>" +
+                "      <New class=\"org.eclipse.jetty.server.SslConnectionFactory\">" +
+                "        <Arg name=\"next\">" + nextProtocol + "</Arg>" +
+                "        <Arg name=\"sslContextFactory\"><Ref refid=\"sslContextFactory\"/></Arg>" +
+                "      </New>" +
+                "    </Arg>" +
+                "  </Call>" +
+                "  <Call name=\"addConnectionFactory\">" +
+                "    <Arg>" +
+                "      <New class=\"org.eclipse.jetty.fcgi.server.ServerFCGIConnectionFactory\">" +
+                "        <Arg><Ref refid=\"sslHttpConfig\" /></Arg>" +
+                "      </New>" +
+                "    </Arg>" +
+                "  </Call>" +
+                "</Configure>";
+            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")));
+                }
+            }
+        }
+    }
 }