From f5ffb5e7b08e6473474a5da54ec79db16f54beba Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 25 Feb 2025 20:23:58 -0600 Subject: [PATCH] Change default SCL instance, and allow selecting from service loader The default is now to ask a service loader for a ServletContainerLauncher if there is a single entry available, and failing that to use the static server - no servlets will be available. ServletContainerLaunchers registered as a provider for the service loader must provide a name that matches a specific regex. Those names may now also be used when specifying a particular server name rather than the fully qualified class name. The JettyLauncher will be kept for at least another version to ensure that projects can easily migrate to newer versions. This patch also moves `setBaseRequestLogLevel` to ServletContainerLauncher so other implementations can use it to log appropriately based on how they were started. Fixes #10057 --- .../core/ext/ServletContainerLauncher.java | 30 +++++- dev/core/src/com/google/gwt/dev/DevMode.java | 98 ++++++++++++------- .../gwt/dev/shell/jetty/JettyLauncher.java | 34 ++----- .../gwt/dev/shell/jetty/SslConfiguration.java | 5 +- 4 files changed, 102 insertions(+), 65 deletions(-) diff --git a/dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java b/dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java index 9c7602a09f..14148090de 100644 --- a/dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java +++ b/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. + *

+ * 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. - * + *

* 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. + *

+ * 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. *

diff --git a/dev/core/src/com/google/gwt/dev/DevMode.java b/dev/core/src/com/google/gwt/dev/DevMode.java index 2d1cbc0cba..8469d964e1 100644 --- a/dev/core/src/com/google/gwt/dev/DevMode.java +++ b/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,16 +125,32 @@ 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 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 @@ -139,7 +158,15 @@ 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 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)) { diff --git a/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java b/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java index c5d3cbcccd..78001f61a8 100644 --- a/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java +++ b/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 diff --git a/dev/core/src/com/google/gwt/dev/shell/jetty/SslConfiguration.java b/dev/core/src/com/google/gwt/dev/shell/jetty/SslConfiguration.java index 3d6da8ef1a..3eefbcb0b3 100644 --- a/dev/core/src/com/google/gwt/dev/shell/jetty/SslConfiguration.java +++ b/dev/core/src/com/google/gwt/dev/shell/jetty/SslConfiguration.java @@ -54,7 +54,7 @@ public static Optional 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 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(); } }