Improve module deprecation (#12547)

Improved messages used on more modules
jetty · Nov 20, 2024 · 8788709 · 8788709
1 parent c5e3282
commit 8788709
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 41 deletions.
diff --git a/jetty-core/jetty-server/src/main/config/modules/customrequestlog.mod b/jetty-core/jetty-server/src/main/config/modules/customrequestlog.mod
diff --git a/jetty-core/jetty-server/src/main/config/modules/delay-until-content.mod b/jetty-core/jetty-server/src/main/config/modules/delay-until-content.mod
@@ -6,6 +6,9 @@ For form data and multipart, the handling is delayed until the entire request bo
 been asynchronously read. For all other content types, the delay is for up to a configurable
 number of content bytes.
 
+[deprecated]
+Use 'eager-content' module instead.
+
 [tags]
 server
 

diff --git a/jetty-core/jetty-server/src/main/config/modules/eager-content.mod b/jetty-core/jetty-server/src/main/config/modules/eager-content.mod
@@ -13,8 +13,8 @@ server
 
 [after]
 compression
-cross-origin
 gzip
+cross-origin
 rewrite
 size-limit
 

diff --git a/jetty-core/jetty-server/src/main/config/modules/qos.mod b/jetty-core/jetty-server/src/main/config/modules/qos.mod
@@ -5,10 +5,6 @@ to limit the number of concurrent requests, for resource management.
 [tags]
 server
 
-[before]
-compression
-gzip
-
 [depends]
 server
 

diff --git a/jetty-core/jetty-server/src/main/config/modules/thread-limit.mod b/jetty-core/jetty-server/src/main/config/modules/thread-limit.mod
@@ -5,10 +5,6 @@ the number of requests per IP address, for denial-of-service protection.
 [tags]
 server
 
-[before]
-compression
-gzip
-
 [depends]
 server
 

diff --git a/jetty-core/jetty-server/src/main/config/modules/threadlimit.mod b/jetty-core/jetty-server/src/main/config/modules/threadlimit.mod
@@ -1,5 +1,9 @@
 [description]
-DEPRECATED - use the thread-limit module instead.
+Installs ThreadLimitHandler at the root of the `Handler` tree, to limit
+the number of requests per IP address, for denial-of-service protection.
+
+[deprecated]
+Use 'thread-limit' module instead.
 
 [depend]
 thread-limit
diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java
@@ -186,12 +186,12 @@ public Module(BaseHome basehome, Path path) throws IOException
         }
         if (m < 0)
             throw new IllegalArgumentException("Module not contained within modules directory: " + basehome.toShortForm(path));
-        String n = path.getName(m + 1).toString();
+        StringBuilder n = new StringBuilder(path.getName(m + 1).toString());
         for (int i = m + 2; i < path.getNameCount(); i++)
         {
-            n = n + "/" + path.getName(i).toString();
+            n.append("/").append(path.getName(i));
         }
-        Matcher matcher = MOD_NAME.matcher(n);
+        Matcher matcher = MOD_NAME.matcher(n.toString());
         if (!matcher.matches())
             throw new IllegalArgumentException("Module filename must have .mod extension: " + basehome.toShortForm(path));
         _name = matcher.group(1);
@@ -248,7 +248,7 @@ public boolean equals(Object obj)
 
     public void expandDependencies(Props props)
     {
-        Function<String, String> expander = d -> props.expand(d);
+        Function<String, String> expander = props::expand;
 
         List<String> tmp = _depends.stream().map(expander).collect(Collectors.toList());
         _depends.clear();
@@ -329,7 +329,7 @@ public int hashCode()
 
     public boolean hasLicense()
     {
-        return (_license != null) && (_license.size() > 0);
+        return !_license.isEmpty();
     }
 
     /**
@@ -383,7 +383,7 @@ public void process(BaseHome basehome) throws FileNotFoundException, IOException
                 else
                 {
                     // blank lines and comments are valid for ini-template section
-                    if ((line.length() == 0) || line.startsWith("#"))
+                    if ((line.isEmpty()) || line.startsWith("#"))
                     {
                         // Remember ini comments and whitespace (empty lines)
                         // for the [ini-template] section
@@ -556,6 +556,11 @@ public List<String> getDepends()
         return new ArrayList<>(_depends);
     }
 
+    public boolean isDeprecated()
+    {
+        return !_deprecated.isEmpty();
+    }
+
     public List<String> getDeprecated()
     {
         return List.copyOf(_deprecated);

diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java
@@ -97,7 +97,7 @@ public void showModules(PrintStream out, List<String> modules)
             String label;
             Set<String> provides = module.getProvides();
             provides.remove(module.getName());
-            out.printf("%n     Module: %s %s%n", module.getName(), provides.size() > 0 ? provides : "");
+            out.printf("%n     Module: %s %s%n", module.getName(), !provides.isEmpty() ? provides : "");
             for (String description : module.getDescription())
             {
                 out.printf("           : %s%n", description);
@@ -182,12 +182,13 @@ public void listModules(PrintStream out, List<String> tags)
 
         tags = new ArrayList<>(tags);
 
-        boolean wild = tags.contains("*");
+        boolean wild = tags.remove("*");
+        boolean showDeprecated = tags.remove("deprecated") || wild;
+
         Set<String> included = new HashSet<>();
-        if (wild)
-            tags.remove("*");
-        else
+        if (!wild)
             tags.stream().filter(t -> !t.startsWith("-")).forEach(included::add);
+
         Set<String> excluded = new HashSet<>();
         tags.stream().filter(t -> t.startsWith("-")).map(t -> t.substring(1)).forEach(excluded::add);
         if (!included.contains("internal"))
@@ -199,12 +200,14 @@ public void listModules(PrintStream out, List<String> tags)
         Optional<Integer> max = _modules.stream().filter(filter).map(Module::getName).map(String::length).max(Integer::compareTo);
         if (max.isEmpty())
             return;
-        String format = "%" + max.get() + "s - %s%n";
+        String format = "%" + max.get() + "s - %s%s%n";
 
         Comparator<Module> comparator = wild ? Comparator.comparing(Module::getName) : Module::compareTo;
         AtomicReference<String> tag = new AtomicReference<>();
         _modules.stream().filter(filter).sorted(comparator).forEach(module ->
         {
+            if (module.isDeprecated() && !showDeprecated)
+                return;
             if (!wild && !module.getPrimaryTag().equals(tag.get()))
             {
                 tag.set(module.getPrimaryTag());
@@ -213,7 +216,7 @@ public void listModules(PrintStream out, List<String> tags)
             }
 
             List<String> description = module.getDescription();
-            out.printf(format, module.getName(), description != null && description.size() > 0 ? description.get(0) : "");
+            out.printf(format, module.getName(), module.isDeprecated() ? "DEPRECATED " : "", description != null && !description.isEmpty() ? description.get(0) : "");
         });
     }
 
@@ -229,7 +232,7 @@ public void listEnabled(PrintStream out)
         {
             String index = (i++) + ")";
             String name = module.getName();
-            if (!module.getDeprecated().isEmpty())
+            if (module.isDeprecated())
                 name += " (deprecated)";
             for (String s : module.getEnableSources())
             {
@@ -424,10 +427,9 @@ private void enable(Set<String> newlyEnabled, Module module, String enabledFrom,
             return;
         }
 
-        List<String> deprecated = module.getDeprecated();
-        if (!deprecated.isEmpty())
+        if (module.isDeprecated())
         {
-            String reason = deprecated.stream().collect(Collectors.joining(System.lineSeparator()));
+            String reason = module.getDeprecated().stream().collect(Collectors.joining(System.lineSeparator()));
             StartLog.warn(reason);
         }
 
@@ -635,15 +637,15 @@ public void checkEnabledModules()
                     Set<Module> providers = getAvailableProviders(d);
                     if (providers.stream().noneMatch(Module::isEnabled))
                     {
-                        if (unsatisfied.length() > 0)
+                        if (!unsatisfied.isEmpty())
                             unsatisfied.append(',');
                         unsatisfied.append(m.getName());
                         StartLog.error("Module [%s] requires a module providing [%s] from one of %s%n", m.getName(), d, providers);
                     }
                 });
         });
 
-        if (unsatisfied.length() > 0)
+        if (!unsatisfied.isEmpty())
             throw new UsageException(-1, "Unsatisfied module dependencies: " + unsatisfied);
     }
 }
diff --git a/...ribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/...ribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java
@@ -1177,11 +1177,20 @@ public void testDeprecatedModule() throws Exception
         );
         Files.write(deprecatedModule, lines, StandardOpenOption.CREATE);
 
+        try (JettyHomeTester.Run listConfigRun = distribution.start(List.of("--list-modules")))
+        {
+            assertTrue(listConfigRun.awaitFor(START_TIMEOUT, TimeUnit.SECONDS));
+            assertEquals(0, listConfigRun.getExitValue());
+
+            assertTrue(listConfigRun.getLogs().stream().noneMatch(log -> log.contains("DEPRECATED")));
+        }
+
         try (JettyHomeTester.Run listConfigRun = distribution.start(List.of("--list-modules=deprecated")))
         {
             assertTrue(listConfigRun.awaitFor(START_TIMEOUT, TimeUnit.SECONDS));
             assertEquals(0, listConfigRun.getExitValue());
 
+            assertTrue(listConfigRun.getLogs().stream().anyMatch(log -> log.contains("DEPRECATED")));
             assertTrue(listConfigRun.getLogs().stream().anyMatch(log -> log.contains(description)));
         }