Skip to content

Commit

Permalink
Log deprecation warning when using built-in JettyLauncher
Browse files Browse the repository at this point in the history
This patch has two approaches to check if the JettyLauncher is being
used, it checks for any configured filters/servlets (other than the
defaults provided by Jetty) in order to log a warning on startup, and
it checks for non-404 responses to catch other resources loaded after
startup.

No warning is logged for tests.

Fixes #9863
  • Loading branch information
niloc132 committed Nov 15, 2023
1 parent be37473 commit dad1634
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
43 changes: 43 additions & 0 deletions dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,41 @@
import java.util.Enumeration;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;

/**
* A {@link ServletContainerLauncher} for an embedded Jetty server.
*/
public class JettyLauncher extends ServletContainerLauncher {
private static final AtomicBoolean hasLoggedDeprecationWarning = new AtomicBoolean(false);

/**
* Disable warning about JettyLauncher being deprecated when running tests, as this only is
* intended to apply to DevMode - The test runner can be updated at the same time as any other
* changes are made to JettyLauncher.
*/
public static void suppressDeprecationWarningForTests() {
hasLoggedDeprecationWarning.set(true);
}

/**
* Warns, only once, that JettyLauncher is deprecated for removal. Call when it is clear that
* the developer is actively deploying code to, or loading resources from, the embedded Jetty
* instance.
*
* @param log existing TreeLogger to append warning to
*/
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 " +
"ServletContainerLauncher to continue running your application server from " +
"DevMode.");
}
}

/**
* Log jetty requests/responses to TreeLogger.
Expand All @@ -93,6 +123,10 @@ public void log(Request request, Response response) {
// Copied from NCSARequestLog
status = 404;
}
if (status != 404) {
// Ignore 404 errors, log the first other call to the server if we haven't logged yet
maybeLogDeprecationWarning(logger);
}
TreeLogger.Type logStatus, logHeaders;
if (status >= 500) {
logStatus = TreeLogger.ERROR;
Expand Down Expand Up @@ -566,6 +600,15 @@ private WebAppContextWithReload(TreeLogger logger, String webApp,
protected void doStart() throws Exception {
setClassLoader(new WebAppClassLoaderExtension());
super.doStart();

// After start, warn if a servlet/filter was configured other than those provided by Jetty
boolean hasNonJettyFiltersOrServlets = Stream.concat(
getServletContext().getServletRegistrations().values().stream(),
getServletContext().getFilterRegistrations().values().stream()
).anyMatch(r -> !r.getClassName().startsWith("org.eclipse.jetty"));
if (hasNonJettyFiltersOrServlets) {
maybeLogDeprecationWarning(logger);
}
}

@Override
Expand Down
1 change: 1 addition & 0 deletions user/src/com/google/gwt/junit/JUnitShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ public String[] getDefaultArgs() {
* ----- Options from DevMode -------
*/
// Hard code the server.
JettyLauncher.suppressDeprecationWarningForTests();
options.setServletContainerLauncher(shell.new MyJettyLauncher());
// DISABLE: ArgHandlerStartupURLs
registerHandler(new ArgHandlerWarDir(options) {
Expand Down

0 comments on commit dad1634

Please sign in to comment.