Issue #4628 - Add support for conditional module dependencies in jett…

…y-start. Updated after review. Renamed isRequiredModule() -> isConditionalModule() and inverted expressions that were using the method. Signed-off-by: Simone Bordet <[email protected]>
jetty · Mar 19, 2020 · 3fcc2ed · 3fcc2ed
1 parent dbaf0bf
commit 3fcc2ed
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 19 deletions.
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
@@ -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;
     }

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
@@ -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);