Merge pull request #4633 from eclipse/jetty-9.4.x-4628-start-module-n…

…on-required

Issue #4628 - Ensuring checkEnabledModules is conditional dependency aware

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

@@ -195,14 +195,14 @@ public Module(BaseHome basehome, Path path) throws IOException
         process(basehome);
     }
 
-    public static boolean isRequiredDependency(String depends)
+    public static boolean isConditionalDependency(String depends)
     {
-        return (depends != null) && (depends.charAt(0) != '?');
+        return (depends != null) && (depends.charAt(0) == '?');
     }
 
     public static String normalizeModuleName(String name)
     {
-        if (!isRequiredDependency(name))
+        if (isConditionalDependency(name))
             return name.substring(1);
         return name;
     }

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

@@ -91,8 +91,8 @@ public void dump(List<String> tags)
         _modules.stream()
             .filter(m ->
             {
-                boolean included = all || m.getTags().stream().anyMatch(t -> include.contains(t));
-                boolean excluded = m.getTags().stream().anyMatch(t -> exclude.contains(t));
+                boolean included = all || m.getTags().stream().anyMatch(include::contains);
+                boolean excluded = m.getTags().stream().anyMatch(exclude::contains);
                 return included && !excluded;
             })
             .sorted()
@@ -135,9 +135,9 @@ public void dump(List<String> tags)
                     {
                         parent = Module.normalizeModuleName(parent);
                         System.out.printf(label, parent);
-                        if (!Module.isRequiredDependency(parent))
+                        if (Module.isConditionalDependency(parent))
                         {
-                            System.out.print(" [not-required]");
+                            System.out.print(" [conditional]");
                         }
                         label = ", %s";
                     }
@@ -219,11 +219,7 @@ private Module registerModule(Path file)
             Module module = new Module(_baseHome, file);
             _modules.add(module);
             _names.put(module.getName(), module);
-            module.getProvides().forEach(n ->
-            {
-                _provided.computeIfAbsent(n, k -> new HashSet<Module>()).add(module);
-            });
-
+            module.getProvides().forEach(n -> _provided.computeIfAbsent(n, k -> new HashSet<>()).add(module));
             return module;
         }
         catch (Error | RuntimeException t)
@@ -258,7 +254,7 @@ public String toString()
 
     public List<Module> getEnabled()
     {
-        List<Module> enabled = _modules.stream().filter(m -> m.isEnabled()).collect(Collectors.toList());
+        List<Module> enabled = _modules.stream().filter(Module::isEnabled).collect(Collectors.toList());
 
         TopologicalSort<Module> sort = new TopologicalSort<>();
         for (Module module : enabled)
@@ -360,7 +356,7 @@ private void enable(Set<String> newlyEnabled, Module module, String enabledFrom,
         StartLog.debug("Enabled module %s depends on %s", module.getName(), module.getDepends());
         for (String dependsOnRaw : module.getDepends())
         {
-            boolean isRequired = Module.isRequiredDependency(dependsOnRaw);
+            boolean isConditional = Module.isConditionalDependency(dependsOnRaw);
             // Final to allow lambda's below to use name
             final String dependentModule = Module.normalizeModuleName(dependsOnRaw);
 
@@ -376,7 +372,7 @@ private void enable(Set<String> newlyEnabled, Module module, String enabledFrom,
                 if (dependentModule.contains("/"))
                 {
                     Path file = _baseHome.getPath("modules/" + dependentModule + ".mod");
-                    if (isRequired || Files.exists(file))
+                    if (!isConditional || Files.exists(file))
                     {
                         registerModule(file).expandDependencies(_args.getProperties());
                         providers = _provided.get(dependentModule);
@@ -387,10 +383,10 @@ private void enable(Set<String> newlyEnabled, Module module, String enabledFrom,
                         continue;
                     }
                 }
-                // is this a non-required module
-                if (!isRequired)
+                // is this a conditional module
+                if (isConditional)
                 {
-                    StartLog.debug("Skipping non-required module [%s]: doesn't exist", dependentModule);
+                    StartLog.debug("Skipping conditional module [%s]: it does not exist", dependentModule);
                     continue;
                 }
                 // throw an exception (not a dynamic module and a required dependency)
@@ -488,7 +484,7 @@ public void checkEnabledModules()
         {
             // Check dependencies
             m.getDepends().stream()
-                .filter(Module::isRequiredDependency)
+                .filter(depends -> !Module.isConditionalDependency(depends))
                 .forEach(d ->
                 {
                     Set<Module> providers = getAvailableProviders(d);

jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java

@@ -234,6 +234,7 @@ public void testResolveNotRequiredModuleNotFound() throws IOException
 
         // Collect active module list
         List<Module> active = modules.getEnabled();
+        modules.checkEnabledModules();
 
         // Assert names are correct, and in the right order
         List<String> expectedNames = new ArrayList<>();
@@ -282,6 +283,7 @@ public void testResolveNotRequiredModuleFound() throws IOException
 
         // Collect active module list
         List<Module> active = modules.getEnabled();
+        modules.checkEnabledModules();
 
         // Assert names are correct, and in the right order
         List<String> expectedNames = new ArrayList<>();
@@ -331,6 +333,7 @@ public void testResolveNotRequiredModuleFoundDynamic() throws IOException
 
         // Collect active module list
         List<Module> active = modules.getEnabled();
+        modules.checkEnabledModules();
 
         // Assert names are correct, and in the right order
         List<String> expectedNames = new ArrayList<>();