mirror of
https://github.com/google/nomulus.git
synced 2025-06-27 23:03:34 +02:00
Fix the TestServer filter support added in []
In the previous CL I added filter support for the test server, but even though I could verify that filters were being run when debugging, in practice the side effects of the filters (notably, ObjectifyFilter clearing the session cache) were somehow not present in tests (and therefore causing new as-yet unsubmitted tests that rely on proper session caching to break). Investigating further, the way TestServer works is that it creates a wrapper Servlet for each route, and in that wrapper just pushes a future onto a queue and waits on it. The actual target servlet is run within the queue, not within the wrapper servlet's context. I had added filters to the *wrapper* servlet, which meant that even though they were invoked before adding the task to the queue, they were not invoked in the process of actually running the task. In this CL I pushed the filters into the task itself, just like the target servlet. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=144123637
This commit is contained in:
parent
e981bad0b3
commit
0859cde790
2 changed files with 41 additions and 30 deletions
|
@ -15,18 +15,23 @@
|
||||||
package google.registry.server;
|
package google.registry.server;
|
||||||
|
|
||||||
import static com.google.common.base.Preconditions.checkNotNull;
|
import static com.google.common.base.Preconditions.checkNotNull;
|
||||||
import static com.google.common.base.Suppliers.memoize;
|
import static google.registry.util.TypeUtils.instantiate;
|
||||||
|
|
||||||
import com.google.common.base.Supplier;
|
|
||||||
import com.google.common.base.Throwables;
|
import com.google.common.base.Throwables;
|
||||||
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.util.concurrent.Uninterruptibles;
|
import com.google.common.util.concurrent.Uninterruptibles;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
import java.util.Iterator;
|
||||||
import java.util.Queue;
|
import java.util.Queue;
|
||||||
import java.util.concurrent.Callable;
|
import java.util.concurrent.Callable;
|
||||||
import java.util.concurrent.ExecutionException;
|
import java.util.concurrent.ExecutionException;
|
||||||
import java.util.concurrent.FutureTask;
|
import java.util.concurrent.FutureTask;
|
||||||
import javax.annotation.Nullable;
|
import javax.annotation.Nullable;
|
||||||
|
import javax.servlet.Filter;
|
||||||
|
import javax.servlet.FilterChain;
|
||||||
import javax.servlet.ServletException;
|
import javax.servlet.ServletException;
|
||||||
|
import javax.servlet.ServletRequest;
|
||||||
|
import javax.servlet.ServletResponse;
|
||||||
import javax.servlet.http.HttpServlet;
|
import javax.servlet.http.HttpServlet;
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
import javax.servlet.http.HttpServletResponse;
|
import javax.servlet.http.HttpServletResponse;
|
||||||
|
@ -34,17 +39,23 @@ import javax.servlet.http.HttpServletResponse;
|
||||||
/**
|
/**
|
||||||
* Servlet that wraps a servlet and delegates request execution to a queue.
|
* Servlet that wraps a servlet and delegates request execution to a queue.
|
||||||
*
|
*
|
||||||
|
* <p>The actual invocation of the delegate does not happen within this servlet's lifecycle.
|
||||||
|
* Therefore, the task on the queue must manually invoke filters within the queue task.
|
||||||
|
*
|
||||||
* @see TestServer
|
* @see TestServer
|
||||||
*/
|
*/
|
||||||
public final class ServletWrapperDelegatorServlet extends HttpServlet {
|
public final class ServletWrapperDelegatorServlet extends HttpServlet {
|
||||||
|
|
||||||
private final Queue<FutureTask<Void>> requestQueue;
|
private final Queue<FutureTask<Void>> requestQueue;
|
||||||
private final Supplier<HttpServlet> servlet;
|
private final Class<? extends HttpServlet> servletClass;
|
||||||
|
private final ImmutableList<Class<? extends Filter>> filterClasses;
|
||||||
|
|
||||||
ServletWrapperDelegatorServlet(
|
ServletWrapperDelegatorServlet(
|
||||||
Class<? extends HttpServlet> servletClass,
|
Class<? extends HttpServlet> servletClass,
|
||||||
|
ImmutableList<Class<? extends Filter>> filterClasses,
|
||||||
Queue<FutureTask<Void>> requestQueue) {
|
Queue<FutureTask<Void>> requestQueue) {
|
||||||
this.servlet = lazilyInstantiate(checkNotNull(servletClass, "servletClass"));
|
this.servletClass = servletClass;
|
||||||
|
this.filterClasses = filterClasses;
|
||||||
this.requestQueue = checkNotNull(requestQueue, "requestQueue");
|
this.requestQueue = checkNotNull(requestQueue, "requestQueue");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -55,7 +66,20 @@ public final class ServletWrapperDelegatorServlet extends HttpServlet {
|
||||||
@Nullable
|
@Nullable
|
||||||
@Override
|
@Override
|
||||||
public Void call() throws ServletException, IOException {
|
public Void call() throws ServletException, IOException {
|
||||||
servlet.get().service(req, rsp);
|
// Simulate the full filter chain with the servlet at the end.
|
||||||
|
final Iterator<Class<? extends Filter>> filtersIter = filterClasses.iterator();
|
||||||
|
FilterChain filterChain =
|
||||||
|
new FilterChain() {
|
||||||
|
@Override
|
||||||
|
public void doFilter(ServletRequest request, ServletResponse response)
|
||||||
|
throws IOException, ServletException {
|
||||||
|
if (filtersIter.hasNext()) {
|
||||||
|
instantiate(filtersIter.next()).doFilter(request, response, this);
|
||||||
|
} else {
|
||||||
|
instantiate(servletClass).service(request, response);
|
||||||
|
}
|
||||||
|
}};
|
||||||
|
filterChain.doFilter(req, rsp);
|
||||||
return null;
|
return null;
|
||||||
}});
|
}});
|
||||||
requestQueue.add(task);
|
requestQueue.add(task);
|
||||||
|
@ -67,16 +91,4 @@ public final class ServletWrapperDelegatorServlet extends HttpServlet {
|
||||||
throw Throwables.propagate(e.getCause());
|
throw Throwables.propagate(e.getCause());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static <T> Supplier<T> lazilyInstantiate(final Class<? extends T> clazz) {
|
|
||||||
return memoize(new Supplier<T>() {
|
|
||||||
@Override
|
|
||||||
public T get() {
|
|
||||||
try {
|
|
||||||
return clazz.newInstance();
|
|
||||||
} catch (InstantiationException | IllegalAccessException e) {
|
|
||||||
throw new RuntimeException(e);
|
|
||||||
}
|
|
||||||
}});
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -15,11 +15,12 @@
|
||||||
package google.registry.server;
|
package google.registry.server;
|
||||||
|
|
||||||
import static com.google.common.base.Preconditions.checkArgument;
|
import static com.google.common.base.Preconditions.checkArgument;
|
||||||
|
import static com.google.common.util.concurrent.Runnables.doNothing;
|
||||||
import static google.registry.util.NetworkUtils.getCanonicalHostName;
|
import static google.registry.util.NetworkUtils.getCanonicalHostName;
|
||||||
|
|
||||||
import com.google.common.base.Throwables;
|
import com.google.common.base.Throwables;
|
||||||
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.net.HostAndPort;
|
import com.google.common.net.HostAndPort;
|
||||||
import com.google.common.util.concurrent.Callables;
|
|
||||||
import com.google.common.util.concurrent.SimpleTimeLimiter;
|
import com.google.common.util.concurrent.SimpleTimeLimiter;
|
||||||
import java.net.MalformedURLException;
|
import java.net.MalformedURLException;
|
||||||
import java.net.URL;
|
import java.net.URL;
|
||||||
|
@ -34,7 +35,6 @@ import javax.annotation.Nullable;
|
||||||
import javax.servlet.Filter;
|
import javax.servlet.Filter;
|
||||||
import javax.servlet.http.HttpServlet;
|
import javax.servlet.http.HttpServlet;
|
||||||
import org.mortbay.jetty.Connector;
|
import org.mortbay.jetty.Connector;
|
||||||
import org.mortbay.jetty.Handler;
|
|
||||||
import org.mortbay.jetty.Server;
|
import org.mortbay.jetty.Server;
|
||||||
import org.mortbay.jetty.bio.SocketConnector;
|
import org.mortbay.jetty.bio.SocketConnector;
|
||||||
import org.mortbay.jetty.servlet.Context;
|
import org.mortbay.jetty.servlet.Context;
|
||||||
|
@ -85,8 +85,8 @@ public final class TestServer {
|
||||||
public TestServer(
|
public TestServer(
|
||||||
HostAndPort address,
|
HostAndPort address,
|
||||||
Map<String, Path> runfiles,
|
Map<String, Path> runfiles,
|
||||||
Iterable<Route> routes,
|
ImmutableList<Route> routes,
|
||||||
Iterable<Class<? extends Filter>> filters) {
|
ImmutableList<Class<? extends Filter>> filters) {
|
||||||
urlAddress = createUrlAddress(address);
|
urlAddress = createUrlAddress(address);
|
||||||
server.addConnector(createConnector(address));
|
server.addConnector(createConnector(address));
|
||||||
server.addHandler(createHandler(runfiles, routes, filters));
|
server.addHandler(createHandler(runfiles, routes, filters));
|
||||||
|
@ -120,7 +120,7 @@ public final class TestServer {
|
||||||
* main event loop, for post-request processing.
|
* main event loop, for post-request processing.
|
||||||
*/
|
*/
|
||||||
public void ping() {
|
public void ping() {
|
||||||
requestQueue.add(new FutureTask<>(Callables.<Void>returning(null)));
|
requestQueue.add(new FutureTask<Void>(doNothing(), null));
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Stops the HTTP server. */
|
/** Stops the HTTP server. */
|
||||||
|
@ -151,8 +151,8 @@ public final class TestServer {
|
||||||
|
|
||||||
private Context createHandler(
|
private Context createHandler(
|
||||||
Map<String, Path> runfiles,
|
Map<String, Path> runfiles,
|
||||||
Iterable<Route> routes,
|
ImmutableList<Route> routes,
|
||||||
Iterable<Class<? extends Filter>> filters) {
|
ImmutableList<Class<? extends Filter>> filters) {
|
||||||
Context context = new Context(server, CONTEXT_PATH, Context.SESSIONS);
|
Context context = new Context(server, CONTEXT_PATH, Context.SESSIONS);
|
||||||
context.addServlet(new ServletHolder(HealthzServlet.class), "/healthz");
|
context.addServlet(new ServletHolder(HealthzServlet.class), "/healthz");
|
||||||
for (Map.Entry<String, Path> runfile : runfiles.entrySet()) {
|
for (Map.Entry<String, Path> runfile : runfiles.entrySet()) {
|
||||||
|
@ -161,10 +161,8 @@ public final class TestServer {
|
||||||
runfile.getKey());
|
runfile.getKey());
|
||||||
}
|
}
|
||||||
for (Route route : routes) {
|
for (Route route : routes) {
|
||||||
context.addServlet(new ServletHolder(wrapServlet(route.servletClass())), route.path());
|
context.addServlet(
|
||||||
}
|
new ServletHolder(wrapServlet(route.servletClass(), filters)), route.path());
|
||||||
for (Class<? extends Filter> filter : filters) {
|
|
||||||
context.addFilter(filter, "/*", Handler.REQUEST);
|
|
||||||
}
|
}
|
||||||
ServletHolder holder = new ServletHolder(DefaultServlet.class);
|
ServletHolder holder = new ServletHolder(DefaultServlet.class);
|
||||||
holder.setInitParameter("aliases", "1");
|
holder.setInitParameter("aliases", "1");
|
||||||
|
@ -172,8 +170,9 @@ public final class TestServer {
|
||||||
return context;
|
return context;
|
||||||
}
|
}
|
||||||
|
|
||||||
private HttpServlet wrapServlet(Class<? extends HttpServlet> servletClass) {
|
private HttpServlet wrapServlet(
|
||||||
return new ServletWrapperDelegatorServlet(servletClass, requestQueue);
|
Class<? extends HttpServlet> servletClass, ImmutableList<Class<? extends Filter>> filters) {
|
||||||
|
return new ServletWrapperDelegatorServlet(servletClass, filters, requestQueue);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static Connector createConnector(HostAndPort address) {
|
private static Connector createConnector(HostAndPort address) {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue