Change default SCL instance, and allow selecting from service loader

dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java

@@ -17,6 +17,7 @@
 
 import java.io.File;
 import java.net.BindException;
+import java.util.regex.Pattern;
 
 /**
  * Defines the service provider interface for launching servlet containers that
@@ -25,8 +26,21 @@
  * Subclasses should be careful about calling any methods defined on this class
  * or else they risk failing when used with a version of GWT that did not have
  * those methods.
+ * <p>
+ * As of GWT 2.13, launcher implementations can be discovered by a service loader. Launchers that
+ * specify a name can be selected by the user via the {@code -server} argument to DevMode using
+ * that name instead of their fully qualified class name. Additionally, if only one launcher type
+ * is present on the classpath, it will be used automatically without the need to specify it. As a
+ * result, names should be unique, and projects may wish to take care to avoid allowing more than
+ * one provider at a time on the classpath.
  */
 public abstract class ServletContainerLauncher {
+  /**
+   * Allowed names for ServletContainerLauncher instances, to be able to be used with a
+   * ServiceLoader. If not registered as a service, the "-server" argument can be used with the
+   * class's fully qualified name, and the name property need not follow this pattern.
+   */
+  public static final Pattern SERVICE_NAME_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9_$.]+");
   /*
    * NOTE: Any new methods must have default implementations, and any users of
    * this class must be prepared to handle LinkageErrors when calling new
@@ -42,17 +56,19 @@ public byte[] getIconBytes() {
   }
 
   /**
-   * @return a short human-readable name of this servlet container, or null
-   *     if no name should be displayed.
+   * @return a short human-readable name of this servlet container, or null if no name
+   *     should be displayed. Must match {@link #SERVICE_NAME_PATTERN} to be used when
+   *     loading this server by name.
    */
   public String getName() {
-    return "Web Server";
+    return "Default Web Server";
   }
+
   /**
    * Return true if this servlet container launcher is configured for secure
    * operation (ie, HTTPS).  This value is only queried after arguments, if any,
    * have been processed.
-   *
+   * <p>
    * The default implementation just returns false.
    *
    * @return true if HTTPS is in use
@@ -76,6 +92,17 @@ public boolean processArguments(TreeLogger logger, String arguments) {
     return false;
   }
 
+  /**
+   * Specifies the default log level. Presently DevMode (and JUnitShell) will set this to TRACE
+   * when using a RemoteUI implementation, INFO for other implementations.
+   * <p>
+   * Default implementation does nothing, subclasses are encouraged to use this to configure their
+   * own logggers.
+   */
+  public void setBaseRequestLogLevel(TreeLogger.Type baseLogLevel) {
+    // Do nothing by default.
+  }
+
   /**
    * Set the bind address for the web server socket.
    * <p>

dev/core/src/com/google/gwt/dev/DevMode.java

@@ -26,8 +26,8 @@
 import com.google.gwt.dev.shell.BrowserListener;
 import com.google.gwt.dev.shell.CodeServerListener;
 import com.google.gwt.dev.shell.OophmSessionHandler;
+import com.google.gwt.dev.shell.StaticResourceServer;
 import com.google.gwt.dev.shell.SuperDevListener;
-import com.google.gwt.dev.shell.jetty.JettyLauncher;
 import com.google.gwt.dev.ui.RestartServerCallback;
 import com.google.gwt.dev.ui.RestartServerEvent;
 import com.google.gwt.dev.util.InstalledHelpInfo;
@@ -57,12 +57,15 @@
 import java.io.File;
 import java.io.FilenameFilter;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.net.BindException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.ServiceLoader;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 /**
  * The main executable class for the hosted mode shell. NOTE: the public API for
@@ -123,30 +126,49 @@ public boolean setFlag(boolean value) {
   }
 
   /**
-   * Handles the -server command line flag.
+   * Handles the -server command line flag. If unspecified, tries to find a single SCL defined in
+   * the service loader, or else defaults to StaticResourceServer.
    */
   protected static class ArgHandlerServer extends ArgHandlerString {
 
-    private static final String DEFAULT_SCL = JettyLauncher.class.getName();
-
-    private HostedModeOptions options;
+    private static final String DEFAULT_SCL = StaticResourceServer.class.getName();
 
+    private final HostedModeOptions options;
+    private final Map<String, ServletContainerLauncher> registered;
     public ArgHandlerServer(HostedModeOptions options) {
       this.options = options;
+      registered = ServiceLoader.load(ServletContainerLauncher.class).stream()
+          .map(ServiceLoader.Provider::get)
+          .filter(scl -> {
+            if (!ServletContainerLauncher.SERVICE_NAME_PATTERN.matcher(scl.getName()).matches()) {
+              System.err.println("Server class '" + scl.getClass().getName() +
+                  "' has an invalid name '" + scl.getName() +
+                  "'. To be used from the service loader, this name must match " +
+                  ServletContainerLauncher.SERVICE_NAME_PATTERN.pattern() + ". Skipping.");
+              return false;
+            }
+            return true;
+          })
+          .collect(Collectors.toMap(
+              ServletContainerLauncher::getName,
+              scl -> scl));
     }
 
     @Override
     public String[] getDefaultArgs() {
       if (options.isNoServer()) {
         return null;
       } else {
-        return new String[] {getTag(), DEFAULT_SCL};
+        // Use the default SCL
+        return new String[] { getTag(), "" };
       }
     }
 
     @Override
     public String getPurpose() {
-      return "Specify a different embedded web server to run (must implement ServletContainerLauncher)";
+      return "Specify a different embedded web server to run (must implement " +
+          "ServletContainerLauncher). May be specified by fully qualified class name, or if " +
+          "provided by a ServiceLoader, by the service name.";
     }
 
     @Override
@@ -163,38 +185,63 @@ public String[] getTagArgs() {
     public boolean setString(String arg) {
       // Supercedes -noserver.
       options.setNoServer(false);
-      String sclClassName;
-      String sclArgs;
+      String sclName;
       int idx = arg.indexOf(':');
       if (idx >= 0) {
-        sclArgs = arg.substring(idx + 1);
-        sclClassName = arg.substring(0, idx);
+        options.setServletContainerLauncherArgs(arg.substring(idx + 1));
+        sclName = arg.substring(0, idx);
       } else {
-        sclArgs = null;
-        sclClassName = arg;
+        sclName = arg;
       }
-      if (sclClassName.length() == 0) {
-        sclClassName = DEFAULT_SCL;
+      if (sclName.isEmpty()) {
+        if (registered.size() == 1) {
+          // Exactly one registered SCL, use it as the default, by fully qualified class name
+          sclName = registered.values().iterator().next().getClass().getName();
+        } else {
+          if (!registered.isEmpty()) {
+            System.err.println("Multiple server classes found in the service loader, but none " +
+                "specified on the command line");
+            for (String s : registered.keySet()) {
+              System.err.println("  * " + s + "- " +
+                  registered.get(s).getClass().getName());
+            }
+          }
+          sclName = DEFAULT_SCL;
+        }
       }
+      // Try to load the class by name
       Throwable t;
       try {
         Class<?> clazz =
-            Class.forName(sclClassName, true, Thread.currentThread().getContextClassLoader());
+            Class.forName(sclName, true, Thread.currentThread().getContextClassLoader());
         Class<? extends ServletContainerLauncher> sclClass =
             clazz.asSubclass(ServletContainerLauncher.class);
-        options.setServletContainerLauncher(sclClass.newInstance());
-        options.setServletContainerLauncherArgs(sclArgs);
+        options.setServletContainerLauncher(sclClass.getDeclaredConstructor().newInstance());
         return true;
-      } catch (ClassCastException e) {
-        t = e;
-      } catch (ClassNotFoundException e) {
-        t = e;
-      } catch (InstantiationException e) {
-        t = e;
-      } catch (IllegalAccessException e) {
+      } catch (ClassCastException | ClassNotFoundException | InstantiationException |
+               IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+        // Don't log any error until we've tried the service loader too
         t = e;
       }
-      System.err.println("Unable to load server class '" + sclClassName + "'");
+
+      if (registered.containsKey(sclName)) {
+        options.setServletContainerLauncher(registered.get(sclName));
+        return true;
+      }
+      System.err.println("Failed to find a server class with name '" + sclName +
+          "' in the service loader:");
+      if (registered.isEmpty()) {
+        System.err.println("No server classes found in the service loader.");
+      } else {
+        System.err.println("Available server classes:");
+        for (ServletContainerLauncher servletContainerLauncher : registered.values()) {
+          System.err.println("  * " + servletContainerLauncher.getName() + " - " +
+              servletContainerLauncher.getClass().getName());
+        }
+      }
+
+      System.err.println("Unable to load server class '" + sclName +
+          "' by fully qualified name or from the service loader");
       t.printStackTrace();
       return false;
     }
@@ -626,14 +673,7 @@ protected int doStartUpServer() {
         ui.setWebServerSecure(serverLogger);
       }
 
-      /*
-       * TODO: This is a hack to pass the base log level to the SCL. We'll have
-       * to figure out a better way to do this for SCLs in general.
-       */
-      if (scl instanceof JettyLauncher) {
-        JettyLauncher jetty = (JettyLauncher) scl;
-        jetty.setBaseRequestLogLevel(getBaseLogLevelForUI());
-      }
+      scl.setBaseRequestLogLevel(getBaseLogLevelForUI());
       scl.setBindAddress(options.getBindAddress());
 
       if (serverLogger.isLoggable(TreeLogger.TRACE)) {

dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java

@@ -89,10 +89,9 @@ public static void suppressDeprecationWarningForTests() {
    */
   private static void maybeLogDeprecationWarning(TreeLogger log) {
     if (hasLoggedDeprecationWarning.compareAndSet(false, true)) {
-      log.log(TreeLogger.Type.WARN, "DevMode will default to -noserver in a future release, and " +
-              "JettyLauncher may be removed or changed. Please consider running your own " +
-              "application server and either passing -noserver to DevMode or migrating to " +
-              "CodeServer. Alternatively, consider implementing your own " +
+      log.log(TreeLogger.Type.WARN, "JettyLauncher is deprecated for removal. Please consider" +
+              "running your own application server and either passing -noserver to DevMode or " +
+              "migrating to CodeServer. Alternatively, consider implementing your own " +
               "ServletContainerLauncher to continue running your application server from " +
               "DevMode.");
     }
@@ -548,12 +547,9 @@ private static void setupConnector(ServerConnector connector,
 
   private SslConfiguration sslConfig = new SslConfiguration(ClientAuthType.NONE, null, null, false);
 
-  private final Object privateInstanceLock = new Object();
-
-
   @Override
   public String getName() {
-    return "Jetty";
+    return "DeprecatedJettyLauncher";
   }
 
   @Override
@@ -574,15 +570,9 @@ public boolean processArguments(TreeLogger logger, String arguments) {
     return true;
   }
 
-  /*
-   * TODO: This is a hack to pass the base log level to the SCL. We'll have to
-   * figure out a better way to do this for SCLs in general. Please do not
-   * depend on this method, as it is subject to change.
-   */
+  @Override
   public void setBaseRequestLogLevel(TreeLogger.Type baseLogLevel) {
-    synchronized (privateInstanceLock) {
-      this.baseLogLevel = baseLogLevel;
-    }
+    this.baseLogLevel = baseLogLevel;
   }
 
   @Override
@@ -639,7 +629,7 @@ public ServletContainer start(TreeLogger logger, int port, File appRootDir)
     wac.setSecurityHandler(new ConstraintSecurityHandler());
 
     RequestLogHandler logHandler = new RequestLogHandler();
-    logHandler.setRequestLog(new JettyRequestLogger(logger, getBaseLogLevel()));
+    logHandler.setRequestLog(new JettyRequestLogger(logger, this.baseLogLevel));
     logHandler.setHandler(wac);
     server.setHandler(logHandler);
     server.start();
@@ -730,16 +720,6 @@ private void checkStartParams(TreeLogger logger, int port, File appRootDir) {
     }
   }
 
-  /*
-   * TODO: This is a hack to pass the base log level to the SCL. We'll have to
-   * figure out a better way to do this for SCLs in general.
-   */
-  private TreeLogger.Type getBaseLogLevel() {
-    synchronized (privateInstanceLock) {
-      return this.baseLogLevel;
-    }
-  }
-
   /**
    * This is a modified version of JreMemoryLeakPreventionListener.java found
    * in the Apache Tomcat project at

dev/core/src/com/google/gwt/dev/shell/jetty/SslConfiguration.java

@@ -56,7 +56,7 @@ public static Optional<SslConfiguration> parseArgs(String[] args, TreeLogger log
       }
       if ("ssl".equals(tag)) {
         useSsl = true;
-        URL keyStoreUrl = JettyLauncher.class.getResource("localhost.keystore");
+        URL keyStoreUrl = SslConfiguration.class.getResource("localhost.keystore");
         if (keyStoreUrl == null) {
           logger.log(TreeLogger.ERROR, "Default GWT keystore not found");
           return Optional.empty();
@@ -88,8 +88,7 @@ public static Optional<SslConfiguration> parseArgs(String[] args, TreeLogger log
                   + value + "'");
         }
       } else {
-        logger.log(TreeLogger.ERROR, "Unexpected argument to "
-                + JettyLauncher.class.getSimpleName() + ": " + arg);
+        logger.log(TreeLogger.ERROR, "Unexpected SSL argument: " + arg);
         return Optional.empty();
       }
     }

dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerWarDir.java

@@ -45,6 +45,11 @@ public String getTag() {
     return "-war";
   }
 
+  @Override
+  public String[] getTags() {
+    return new String[] {getTag(), "-launcherDir"};
+  }
+
   @Override
   public void setDir(File dir) {
     option.setWarDir(dir);

samples/common.ant.xml

@@ -179,6 +179,7 @@
             <sysproperty key="gwt.devjar" value="../../gwt-dev.jar" />
             <arg value="-XnoEclipse" />
             <arg value="-overwrite" />
+            <arg value="-useLegacyJetty" />
             <arg value="-out" />
             <arg file="${samples.scripts}/${sample.upper}" />
             <arg value="com.google.gwt.sample.${sample.lower}.${sample.upper}" />