Fix #10411 default environment (#10415)

Implemented a simpler default environment algorithm where an application that does not specify an environment is always attempted in the default.

Updated documentation.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>

documentation/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-jetty.adoc

@@ -37,7 +37,7 @@ A simple Jetty context XML file, for example named `wiki.xml` is the following:
 <2> Specifies the web application `contextPath`, which may be different from the `+*.war+` file name.
 <3> Specifies the file system path of the `+*.war+` file.
 
-The Jetty content XML file must be accompanied by a `+*.properties+` file that specifies the xref:og-deploy[environment] to use for the deployment:
+The Jetty content XML file may be accompanied by a `+*.properties+` file that specifies the xref:og-deploy[environment] to use for the deployment:
 
 .wiki.properties
 [source,properties,subs=attributes]

documentation/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-rules.adoc

@@ -40,29 +40,34 @@ This allows the XML file to reference the `+*.war+` file and avoid that the web
 [[og-deploy-rules-environment]]
 ===== Environment Resolution
 
-A web application is always deployed to a specific environment.
+A web application is always deployed to a specific environment, which is either configured for the deployed application or set to the default environment.
 
-If you enabled only one specific deployer module, for example `{ee-current}-deploy`, then the web applications and the Jetty context XML files in `$JETTY_BASE/webapps` will be deployed to the `{ee-current}` environment.
+If only a single specific deployer module is enabled, for example `{ee-current}-deploy`, then it is the default environment and applications will be deployed to it without any additional configuration.
 
-You can enable simultaneously multiple deployer modules if you need to deploy multiple web applications each to a specific environment.
+If multiple deployer modules are enabled, then the default environment is:
 
-For example, you have an `old-ee9.war` web application that you want to deploy to the Jakarta EE 9 environment, and a `new-{ee-current}.war` web application that you want to deploy to the Jakarta {ee-current-caps} environment.
-First, you must enable both the `ee9-deploy` and the `{ee-current}-deploy` modules.
-Then, you add a `+*.properties+` file with the same name of the web application, in the example above `$JETTY_BASE/webapps/old-ee9.properties`, with the following content:
+* The most recent Jakarta EE environment of the `{ee-all}-deploy` modules that are enabled.
+* Otherwise, the `core` environment, if the `core-deploy` module is enabled.
+* Otherwise, no deployer environment has been enabled, and therefore no application can be deployed.
 
-.old-ee9.properties
+For example, if `core-deploy`, `ee9-deploy` and the `{ee-current}-deploy` modules are enabled, then `{ee-current}` is the default environment, to which applications will be deployed unless otherwise configured (see below).
+
+To configure a specific environment for an application, you add a `+*.properties+` file with the same name of the web application.
+For example, an application deployed to `$JETTY_BASE/webapps/my-ee9-app.war` is configured with the file `$JETTY_BASE/webapps/my-ee9-app.properties`, with the following content:
+
+.my-ee9-app.properties
 [source,properties]
 ----
 environment=ee9
 ----
 
-In case of simultaneous multiple environments, it is good practice to always specify the `+*.properties+` file for your web applications.
+In case of simultaneous multiple deployer environments, it is good practice to always specify the `+*.properties+` file for your web applications.
 
 [CAUTION]
 ====
-If you do *not* specify the `+*.properties+` file for your web applications, then the deployer for the most recent EE version will be used.
+If you do *not* specify the `+*.properties+` file for your web applications, then the deployer for the default environment will be used.
 
-For example, if you have enabled the EE deployer Jetty module for all EE versions, and you deploy an EE 9 web application _without_ the `+*.properties+` file, then it will be deployed by the {ee-current-caps} deployer, with unspecified results.
+For example, if you have enabled the deployer Jetty module for all Jakarta EE versions, and you deploy an EE 9 web application _without_ the `+*.properties+` file, then it will be deployed by the {ee-current-caps} deployer, with unspecified results.
 
 This unspecified deployment may not work as the EE 9 web application may use APIs that have been removed in {ee-current-caps}, causing an error at runtime.
 ====

jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java

@@ -121,26 +121,23 @@ void setLifeCycleNode(Node node)
 
     private final AutoLock _lock = new AutoLock();
     private Throwable _onStartupErrors;
-    private final List<AppProvider> _providers = new ArrayList<AppProvider>();
+    private final List<AppProvider> _providers = new ArrayList<>();
     private final AppLifeCycle _lifecycle = new AppLifeCycle();
     private final Queue<AppEntry> _apps = new ConcurrentLinkedQueue<AppEntry>();
     private ContextHandlerCollection _contexts;
     private boolean _useStandardBindings = true;
     private String _defaultLifeCycleGoal = AppLifeCycle.STARTED;
 
     /**
-     * Get the default {@link Environment} name for deployed applications.
-     * @return The name of environment known to the {@link AppProvider}s returned from
-     * {@link #getAppProviders()} that matches {@link Deployable#EE_ENVIRONMENT_NAME}.
-     * If multiple names match, then the maximal name, according to {@link Deployable#EE_ENVIRONMENT_COMPARATOR}
-     * is returned.
+     * Get the default {@link Environment} name for deployed applications, which is
+     * the maximal name when using the {@link Deployable#ENVIRONMENT_COMPARATOR}.
+     * @return The default {@link Environment} name or null.
      */
     public String getDefaultEnvironmentName()
     {
         return _providers.stream()
             .map(AppProvider::getEnvironmentName)
-            .filter(Deployable.EE_ENVIRONMENT_NAME)
-            .max(Deployable.EE_ENVIRONMENT_COMPARATOR)
+            .max(Deployable.ENVIRONMENT_COMPARATOR)
             .orElse(null);
     }
 

jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java

@@ -30,7 +30,6 @@
 import org.eclipse.jetty.deploy.AppProvider;
 import org.eclipse.jetty.deploy.DeploymentManager;
 import org.eclipse.jetty.server.Deployable;
-import org.eclipse.jetty.util.FileID;
 import org.eclipse.jetty.util.Scanner;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
@@ -148,33 +147,15 @@ protected App createApp(Path path)
         if (LOG.isDebugEnabled())
             LOG.debug("{} creating {}", this, app);
 
-        String environmentName = app.getEnvironmentName();
-
         String defaultEnvironmentName = _deploymentManager.getDefaultEnvironmentName();
-        if (StringUtil.isBlank(environmentName))
+
+        String environmentName = app.getEnvironmentName();
+        if (StringUtil.isBlank(environmentName) && StringUtil.isNotBlank(defaultEnvironmentName))
         {
-            // We may be able to default the environmentName
-            String basename = FileID.getBasename(path);
-            boolean isWebapp = FileID.isWebArchive(path) ||
-                Files.isDirectory(path) && Files.exists(path.resolve("WEB-INF")) ||
-                FileID.isXml(path) && (
-                    Files.exists(path.getParent().resolve(basename + ".war")) ||
-                        Files.exists(path.getParent().resolve(basename + ".WAR")) ||
-                        Files.exists(path.getParent().resolve(basename + "/WEB-INF")));
-            boolean coreProvider = _deploymentManager.hasAppProviderFor(Environment.CORE.getName());
-
-            // TODO review these heuristics... or even if we should have them at all
-            if (isWebapp || (Files.isDirectory(path) && defaultEnvironmentName != null))
-                environmentName = defaultEnvironmentName;
-            else if (coreProvider)
-                environmentName = Environment.CORE.getName();
-
-            if (StringUtil.isNotBlank(environmentName))
-            {
-                app.getProperties().put(Deployable.ENVIRONMENT, environmentName);
-                if (LOG.isDebugEnabled())
-                    LOG.debug("{} default environment for {}", this, app);
-            }
+            environmentName = defaultEnvironmentName;
+            app.getProperties().put(Deployable.ENVIRONMENT, environmentName);
+            if (LOG.isDebugEnabled())
+                LOG.debug("{} default environment for {}", this, app);
         }
 
         if (StringUtil.isNotBlank(environmentName))

jetty-core/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/DeploymentManagerTest.java

@@ -18,6 +18,7 @@
 import java.util.Set;
 
 import org.eclipse.jetty.deploy.test.XmlConfiguredJetty;
+import org.eclipse.jetty.server.Deployable;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
@@ -27,6 +28,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -117,7 +119,7 @@ public String getEnvironmentName()
             @Override
             public String getEnvironmentName()
             {
-                return "ee12";
+                return "ee10";
             }
         });
         assertThat(depman.getDefaultEnvironmentName(), is("ee12"));
@@ -128,10 +130,29 @@ public String getEnvironmentName()
             @Override
             public String getEnvironmentName()
             {
-                return "ee12";
+                return "somethingElse";
             }
         });
         assertThat(depman.getDefaultEnvironmentName(), is("ee12"));
+
+        Environment.ensure("other");
+        depman.addAppProvider(new MockAppProvider()
+        {
+            @Override
+            public String getEnvironmentName()
+            {
+                return "other";
+            }
+        });
+
+        assertThat(depman.getAppProviders().stream().map(AppProvider::getEnvironmentName).sorted(Deployable.ENVIRONMENT_COMPARATOR).toList(),
+            contains(
+                "other",
+                "somethingElse",
+                "ee7",
+                "ee10",
+                "ee12"
+                ));
     }
 
     @Test

jetty-core/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/MockAppProvider.java

@@ -84,4 +84,10 @@ public ContextHandler createContextHandler(App app)
 
         return contextHandler;
     }
+
+    @Override
+    public String toString()
+    {
+        return String.format("MockAppProvider@%x:%s", hashCode(), getEnvironmentName());
+    }
 }

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Deployable.java

@@ -15,7 +15,6 @@
 
 import java.util.Comparator;
 import java.util.Map;
-import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -25,22 +24,31 @@
  */
 public interface Deployable
 {
-    Pattern EE_ENVIRONMENT_NAME_PATTERN = Pattern.compile("ee(\\d*)");
+    Pattern EE_ENVIRONMENT_NAME_PATTERN = Pattern.compile("ee(\\d+)");
 
-    Predicate<String> EE_ENVIRONMENT_NAME = s -> EE_ENVIRONMENT_NAME_PATTERN.matcher(s).matches();
-
-    Comparator<String> EE_ENVIRONMENT_COMPARATOR = (e1, e2) ->
+    /**
+     * A comparator that ranks names matching EE_ENVIRONMENT_NAME_PATTERN higher than other names,
+     * EE names are compared by EE number, otherwise simple name comparison is used.
+     */
+    Comparator<String> ENVIRONMENT_COMPARATOR = (e1, e2) ->
     {
         Matcher m1 = EE_ENVIRONMENT_NAME_PATTERN.matcher(e1);
         Matcher m2 = EE_ENVIRONMENT_NAME_PATTERN.matcher(e2);
 
-        if (m1.matches() && m2.matches())
+        if (m1.matches())
         {
-            int n1 = Integer.parseInt(m1.group(1));
-            int n2 = Integer.parseInt(m2.group(1));
-            return Integer.compare(n1, n2);
+            if (m2.matches())
+            {
+                int n1 = Integer.parseInt(m1.group(1));
+                int n2 = Integer.parseInt(m2.group(1));
+                return Integer.compare(n1, n2);
+            }
+            return 1;
         }
-        return 0;
+        if (m2.matches())
+            return -1;
+
+        return e1.compareTo(e2);
     };
 
     String ATTRIBUTE_PREFIX = "jetty.deploy.attribute.";
@@ -49,7 +57,7 @@ public interface Deployable
     String CONTAINER_SCAN_JARS = "jetty.deploy.containerScanJarPattern";
     String CONTEXT_PATH = "jetty.deploy.contextPath";
     String DEFAULTS_DESCRIPTOR = "jetty.deploy.defaultsDescriptor";
-    String ENVIRONMENT = "environment"; // TODO should this have jetty.deploy.
+    String ENVIRONMENT = "environment";
     String EXTRACT_WARS = "jetty.deploy.extractWars";
     String PARENT_LOADER_PRIORITY = "jetty.deploy.parentLoaderPriority";
     String SCI_EXCLUSION_PATTERN = "jetty.deploy.servletContainerInitializerExclusionPattern";