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
@@ -46,13 +60,14 @@ public byte[] getIconBytes() {
    *     if no name should be displayed.
    */
   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 +91,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;
@@ -56,12 +56,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
@@ -122,24 +125,48 @@ 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};
+        if (registered.size() == 1) {
+          // Exactly one registered SCL, use it as the default, by fully qualified class name
+          return new String[] {
+              getTag(),
+              registered.values().iterator().next().getClass().getName()
+          };
+        }
+        // Use the default SCL
+        return new String[] { getTag(), DEFAULT_SCL };
       }
     }
 
@@ -162,38 +189,50 @@ 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()) {
+        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;
     }
@@ -618,14 +657,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.");
     }
@@ -543,12 +542,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
@@ -569,15 +565,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
@@ -634,7 +624,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();
@@ -725,16 +715,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

@@ -54,7 +54,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();
@@ -85,8 +85,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();
       }
     }