Merge pull request #4097 from eclipse/jetty-10.0.x-ConfigurationCleanup

Configuration cleanup

jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyWebAppContext.java

@@ -42,10 +42,10 @@ import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
-import org.eclipse.jetty.util.resource.PathResource;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.resource.ResourceCollection;
 import org.eclipse.jetty.webapp.Configuration;
+import org.eclipse.jetty.webapp.Configurations;
 import org.eclipse.jetty.webapp.MetaInfConfiguration;
 import org.eclipse.jetty.webapp.WebAppContext;
 
@@ -310,22 +310,27 @@ public class JettyWebAppContext extends WebAppContext
     }
 
     @Override
-    protected void loadConfigurations()
+    protected Configurations newConfigurations()
     {
-        super.loadConfigurations();
+        Configurations configurations = super.newConfigurations();
-        try
+        if (getJettyEnvXml() != null)
         {
-            // inject configurations with config from maven plugin
+            try
-            for (Configuration c : getWebAppConfigurations())
             {
-                if (c instanceof EnvConfiguration && getJettyEnvXml() != null)
+                // inject configurations with config from maven plugin
-                    ((EnvConfiguration)c).setJettyEnvResource(new PathResource(new File(getJettyEnvXml())));
+                for (Configuration c : configurations)
+                {
+                    if (c instanceof EnvConfiguration)
+                        ((EnvConfiguration)c).setJettyEnvResource(Resource.newResource(getJettyEnvXml()));
+                }
+            }
+            catch (IOException e)
+            {
+                throw new RuntimeException(e);
             }
         }
-        catch (Exception e)
+
-        {
+        return configurations;
-            throw new RuntimeException(e);
-        }
     }
 
     @Override

jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartConfiguration.java

@@ -85,7 +85,7 @@ public class QuickStartConfiguration extends AbstractConfiguration
 
     public QuickStartConfiguration()
     {
-        super(false);
+        super(true);
         addDependencies(WebInfConfiguration.class);
         addDependents(WebXmlConfiguration.class);
     }
@@ -208,7 +208,7 @@ public class QuickStartConfiguration extends AbstractConfiguration
     {
         LOG.info("Quickstarting {}", context);
         _quickStart = true;
-        context.setConfigurations(context.getWebAppConfigurations().stream()
+        context.setConfigurations(context.getConfigurations().stream()
             .filter(c -> !__replacedConfigurations.contains(c.replaces()) && !__replacedConfigurations.contains(c.getClass()))
             .collect(Collectors.toList()).toArray(new Configuration[]{}));
         context.getMetaData().setWebXml((Resource)context.getAttribute(QUICKSTART_WEB_XML));

jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartGeneratorConfiguration.java

@@ -93,7 +93,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration
 
     public QuickStartGeneratorConfiguration(boolean abort)
     {
-        super(true);
+        super(false);
         _count = 0;
         _abort = abort;
     }

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/AbstractConfiguration.java

@@ -26,7 +26,7 @@ import java.util.stream.Collectors;
 
 public class AbstractConfiguration implements Configuration
 {
-    private final boolean _disabledByDefault;
+    private final boolean _enabledByDefault;
     private final List<String> _after = new ArrayList<>();
     private final List<String> _beforeThis = new ArrayList<>();
     private final ClassMatcher _system = new ClassMatcher();
@@ -34,12 +34,12 @@ public class AbstractConfiguration implements Configuration
 
     protected AbstractConfiguration()
     {
-        this(false);
+        this(true);
     }
 
-    protected AbstractConfiguration(boolean disabledByDefault)
+    protected AbstractConfiguration(boolean enabledByDefault)
     {
-        _disabledByDefault = disabledByDefault;
+        _enabledByDefault = enabledByDefault;
     }
 
     /**
@@ -196,9 +196,9 @@ public class AbstractConfiguration implements Configuration
     }
 
     @Override
-    public boolean isDisabledByDefault()
+    public boolean isEnabledByDefault()
     {
-        return _disabledByDefault;
+        return _enabledByDefault;
     }
 
     @Override

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configuration.java

@@ -34,12 +34,12 @@ import org.eclipse.jetty.util.TopologicalSort;
  * </p>
  * <p>Configuration instances are discovered by the {@link Configurations} class using either the
  * {@link ServiceLoader} mechanism or by an explicit call to {@link Configurations#setKnown(String...)}.
- * By default, all Configurations that do not implement the {@link #isDisabledByDefault()} interface
+ * By default, all Configurations that do not return false from {@link #isEnabledByDefault()}
  * are applied to all {@link WebAppContext}s within the JVM.  However a Server wide default {@link Configurations}
  * collection may also be defined with {@link Configurations#setServerDefault(org.eclipse.jetty.server.Server)}.
  * Furthermore, each individual Context may have its Configurations list explicitly set and/or amended with
  * {@link WebAppContext#setConfigurations(Configuration[])}, {@link WebAppContext#addConfiguration(Configuration...)}
- * or {@link WebAppContext#getWebAppConfigurations()}.
+ * or {@link WebAppContext#getConfigurations()}.
  * </p>
  * <p>Since Jetty-9.4, Configurations are self ordering using the {@link #getDependencies()} and
  * {@link #getDependents()} methods for a {@link TopologicalSort} initiated by {@link Configurations#sort()}
@@ -171,9 +171,9 @@ public interface Configuration
     void destroy(WebAppContext context) throws Exception;
 
     /**
-     * @return true if configuration is disabled by default
+     * @return true if configuration is enabled by default
      */
-    boolean isDisabledByDefault();
+    boolean isEnabledByDefault();
 
     /**
      * @return true if configuration should be aborted

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java

@@ -209,7 +209,7 @@ public class Configurations extends AbstractList<Configuration> implements Dumpa
         if (configurations == null)
         {
             configurations = new Configurations(Configurations.getKnown().stream()
-                .filter(c -> !c.isDisabledByDefault())
+                .filter(c -> c.isEnabledByDefault())
                 .map(c -> c.getClass().getName())
                 .toArray(String[]::new));
         }
@@ -279,6 +279,27 @@ public class Configurations extends AbstractList<Configuration> implements Dumpa
         }
     }
 
+    public <T> T get(Class<? extends T> configClass)
+    {
+        for (Configuration configuration : _configurations)
+        {
+            if (configClass.isAssignableFrom(configuration.getClass()))
+                return (T)configuration;
+        }
+        return null;
+    }
+
+    public <T> List<T> getConfigurations(Class<? extends T> configClass)
+    {
+        List<T> list = new ArrayList<>();
+        for (Configuration configuration : _configurations)
+        {
+            if (configClass.isAssignableFrom(configuration.getClass()))
+                list.add((T)configuration);
+        }
+        return list;
+    }
+
     public void clear()
     {
         _configurations.clear();

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java

@@ -95,7 +95,7 @@ import org.eclipse.jetty.util.resource.ResourceCollection;
  * <ul>
  * <li>Add all Server class inclusions from all known configurations {@link Configurations#getKnown()}</li>
  * <li>{@link #loadConfigurations()}, which uses either explicitly set Configurations or takes the server
- * default (which is all known non {@link Configuration#isDisabledByDefault()} Configurations.</li>
+ * default (which is all known {@link Configuration#isEnabledByDefault()} Configurations.</li>
  * <li>Sort the configurations using {@link TopologicalSort} in {@link Configurations#sort()}.</li>
  * <li>Add all Server class exclusions from this webapps {@link Configurations}</li>
  * <li>Add all System classes inclusions and exclusions for this webapps {@link Configurations}</li>
@@ -183,10 +183,10 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         "org.eclipse.jetty."                // hide jetty classes
     );
 
-    private final Configurations _configurations = new Configurations();
     private final ClassMatcher _systemClasses = new ClassMatcher(__dftSystemClasses);
     private final ClassMatcher _serverClasses = new ClassMatcher(__dftServerClasses);
 
+    private Configurations _configurations;
     private String _defaultsDescriptor = WEB_DEFAULTS_XML;
     private String _descriptor = null;
     private final List<String> _overrideDescriptors = new ArrayList<>();
@@ -570,18 +570,21 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
     {
         // Prepare for configuration
         MultiException mx = new MultiException();
-        for (Configuration configuration : _configurations)
+        if (_configurations != null)
         {
-            try
+            for (Configuration configuration : _configurations)
             {
-                configuration.destroy(this);
+                try
-            }
+                {
-            catch (Exception e)
+                    configuration.destroy(this);
-            {
+                }
-                mx.add(e);
+                catch (Exception e)
+                {
+                    mx.add(e);
+                }
             }
         }
-        _configurations.clear();
+        _configurations = null;
         super.destroy();
         mx.ifExceptionThrowRuntime();
     }
@@ -615,10 +618,9 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
     /**
      * @return Returns the configurations.
      */
-    public Configurations getWebAppConfigurations()
+    public Configurations getConfigurations()
     {
-        if (_configurations.size() == 0)
+        loadConfigurations();
-            loadConfigurations();
         return _configurations;
     }
 
@@ -885,10 +887,18 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
     protected void loadConfigurations()
     {
         //if the configuration instances have been set explicitly, use them
-        if (!_configurations.isEmpty())
+        if (_configurations != null)
             return;
+        if (isStarted())
+            throw new IllegalStateException();
+        _configurations = newConfigurations();
+    }
 
-        _configurations.add(Configurations.getServerDefault(getServer()).toArray());
+    protected Configurations newConfigurations()
+    {
+        Configurations configurations = new Configurations();
+        configurations.add(Configurations.getServerDefault(getServer()).toArray());
+        return configurations;
     }
 
     @Override
@@ -958,8 +968,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
      */
     public void setConfigurationClasses(String[] configurations)
     {
-        if (isStarted())
+        if (_configurations == null)
-            throw new IllegalStateException();
+            _configurations = new Configurations();
         _configurations.set(configurations);
     }
 
@@ -973,15 +983,13 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
      */
     public void setConfigurations(Configuration[] configurations)
     {
-        if (isStarted())
+        if (_configurations == null)
-            throw new IllegalStateException();
+            _configurations = new Configurations();
         _configurations.set(configurations);
     }
 
     public void addConfiguration(Configuration... configuration)
     {
-        if (isStarted())
-            throw new IllegalStateException();
         loadConfigurations();
         _configurations.add(configuration);
     }
@@ -989,12 +997,19 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
     public <T> T getConfiguration(Class<? extends T> configClass)
     {
         loadConfigurations();
-        for (Configuration configuration : _configurations)
+        return _configurations.get(configClass);
-        {
+    }
-            if (configClass.isAssignableFrom(configuration.getClass()))
+
-                return (T)configuration;
+    public void removeConfiguration(Configuration... configurations)
-        }
+    {
-        return null;
+        if (_configurations != null)
+            _configurations.remove(configurations);
+    }
+
+    public void removeConfiguration(Class<? extends Configuration>... configurations)
+    {
+        if (_configurations != null)
+            _configurations.remove(configurations);
     }
 
     /**

jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java

@@ -100,7 +100,7 @@ public class WebAppContextTest
     {
         Configurations.cleanKnown();
         String[] known_and_enabled = Configurations.getKnown().stream()
-            .filter(c -> !c.isDisabledByDefault())
+            .filter(c -> c.isEnabledByDefault())
             .map(c -> c.getClass().getName())
             .toArray(String[]::new);
 
@@ -108,7 +108,7 @@ public class WebAppContextTest
 
         //test if no classnames set, its the defaults
         WebAppContext wac = new WebAppContext();
-        assertThat(wac.getWebAppConfigurations().stream()
+        assertThat(wac.getConfigurations().stream()
                 .map(c -> c.getClass().getName())
                 .collect(Collectors.toList()),
             Matchers.containsInAnyOrder(known_and_enabled));
@@ -126,7 +126,7 @@ public class WebAppContextTest
         Configurations.cleanKnown();
         WebAppContext wac = new WebAppContext();
         wac.setServer(new Server());
-        assertThat(wac.getWebAppConfigurations().stream().map(c -> c.getClass().getName()).collect(Collectors.toList()),
+        assertThat(wac.getConfigurations().stream().map(c -> c.getClass().getName()).collect(Collectors.toList()),
             Matchers.contains(
                 "org.eclipse.jetty.webapp.JmxConfiguration",
                 "org.eclipse.jetty.webapp.WebInfConfiguration",
@@ -144,14 +144,14 @@ public class WebAppContextTest
         Configuration[] configs = {new WebInfConfiguration()};
         WebAppContext wac = new WebAppContext();
         wac.setConfigurations(configs);
-        assertThat(wac.getWebAppConfigurations(), Matchers.contains(configs));
+        assertThat(wac.getConfigurations(), Matchers.contains(configs));
 
         //test that explicit config instances override any from server
         String[] classNames = {"x.y.z"};
         Server server = new Server();
         server.setAttribute(Configuration.ATTR, classNames);
         wac.setServer(server);
-        assertThat(wac.getWebAppConfigurations(), Matchers.contains(configs));
+        assertThat(wac.getConfigurations(), Matchers.contains(configs));
     }
 
     @Test

tests/test-integration/src/test/java/org/eclipse/jetty/test/DeploymentErrorTest.java

@@ -193,7 +193,7 @@ public class DeploymentErrorTest
         assertThat("ContextHandler.isAvailable", context.isAvailable(), is(false));
         WebAppContext webapp = (WebAppContext)context;
         TrackedConfiguration trackedConfiguration = null;
-        for (Configuration webappConfig : webapp.getWebAppConfigurations())
+        for (Configuration webappConfig : webapp.getConfigurations())
         {
             if (webappConfig instanceof TrackedConfiguration)
                 trackedConfiguration = (TrackedConfiguration)webappConfig;
@@ -239,7 +239,7 @@ public class DeploymentErrorTest
         assertThat("ContextHandler.isAvailable", context.isAvailable(), is(false));
         WebAppContext webapp = (WebAppContext)context;
         TrackedConfiguration trackedConfiguration = null;
-        for (Configuration webappConfig : webapp.getWebAppConfigurations())
+        for (Configuration webappConfig : webapp.getConfigurations())
         {
             if (webappConfig instanceof TrackedConfiguration)
                 trackedConfiguration = (TrackedConfiguration)webappConfig;
@@ -285,7 +285,7 @@ public class DeploymentErrorTest
         assertThat("ContextHandler.isAvailable", context.isAvailable(), is(false));
         WebAppContext webapp = (WebAppContext)context;
         TrackedConfiguration trackedConfiguration = null;
-        for (Configuration webappConfig : webapp.getWebAppConfigurations())
+        for (Configuration webappConfig : webapp.getConfigurations())
         {
             if (webappConfig instanceof TrackedConfiguration)
                 trackedConfiguration = (TrackedConfiguration)webappConfig;