diff --git a/invoker/core/pom.xml b/invoker/core/pom.xml index dbf10306..712d1b91 100644 --- a/invoker/core/pom.xml +++ b/invoker/core/pom.xml @@ -46,11 +46,6 @@ functions-framework-api 1.1.0 - - javax.servlet - javax.servlet-api - 4.0.1 - io.cloudevents cloudevents-core diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/BackgroundFunctionExecutor.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/BackgroundFunctionExecutor.java index 98b9bc8a..3e860389 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/BackgroundFunctionExecutor.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/BackgroundFunctionExecutor.java @@ -42,12 +42,14 @@ import java.util.TreeMap; import java.util.logging.Level; import java.util.logging.Logger; -import javax.servlet.http.HttpServlet; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; /** Executes the user's background function. */ -public final class BackgroundFunctionExecutor extends HttpServlet { +public final class BackgroundFunctionExecutor extends AbstractHandler { private static final Logger logger = Logger.getLogger("com.google.cloud.functions.invoker"); private final FunctionExecutor functionExecutor; @@ -223,7 +225,7 @@ private static Context contextFromCloudEvent(CloudEvent cloudEvent) { * for the various triggers. CloudEvents are ones that follow the standards defined by cloudevents.io. * - * @param the type to be used in the {@link Unmarshallers} call when + * @param the type to be used in the {code Unmarshallers} call when * unmarshalling this event, if it is a CloudEvent. */ private abstract static class FunctionExecutor { @@ -320,7 +322,9 @@ void serviceCloudEvent(CloudEvent cloudEvent) throws Exception { /** Executes the user's background function. This can handle all HTTP methods. */ @Override - public void service(HttpServletRequest req, HttpServletResponse res) throws IOException { + public void handle(String s, Request baseRequest, HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + baseRequest.setHandled(true); String contentType = req.getContentType(); try { if ((contentType != null && contentType.startsWith("application/cloudevents+json")) diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java index 401e22a2..c6cdef2a 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java @@ -17,14 +17,17 @@ import com.google.cloud.functions.HttpFunction; import com.google.cloud.functions.invoker.http.HttpRequestImpl; import com.google.cloud.functions.invoker.http.HttpResponseImpl; +import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; -import javax.servlet.http.HttpServlet; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; /** Executes the user's method. */ -public class HttpFunctionExecutor extends HttpServlet { +public class HttpFunctionExecutor extends AbstractHandler { private static final Logger logger = Logger.getLogger("com.google.cloud.functions.invoker"); private final HttpFunction function; @@ -62,8 +65,9 @@ public static HttpFunctionExecutor forClass(Class functionClass) { } /** Executes the user's method, can handle all HTTP type methods. */ - @Override - public void service(HttpServletRequest req, HttpServletResponse res) { + public void handle(String s, Request baseRequest, HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + baseRequest.setHandled(true); HttpRequestImpl reqImpl = new HttpRequestImpl(req); HttpResponseImpl respImpl = new HttpResponseImpl(res); ClassLoader oldContextLoader = Thread.currentThread().getContextClassLoader(); @@ -75,7 +79,7 @@ public void service(HttpServletRequest req, HttpServletResponse res) { res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } finally { Thread.currentThread().setContextClassLoader(oldContextLoader); - respImpl.flush(); + respImpl.close(); } } } diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java index a6edfc32..99ac993b 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java @@ -10,16 +10,20 @@ import com.google.gson.GsonBuilder; import java.io.BufferedReader; import java.io.BufferedWriter; +import java.io.IOException; import java.lang.reflect.Type; import java.util.Arrays; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; -import javax.servlet.http.HttpServlet; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; -public class TypedFunctionExecutor extends HttpServlet { +public class TypedFunctionExecutor extends AbstractHandler { private static final String APPLY_METHOD = "apply"; private static final Logger logger = Logger.getLogger("com.google.cloud.functions.invoker"); @@ -94,7 +98,9 @@ static Optional handlerTypeArgument(Class> f /** Executes the user's method, can handle all HTTP type methods. */ @Override - public void service(HttpServletRequest req, HttpServletResponse res) { + public void handle(String s, Request baseRequest, HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + baseRequest.setHandled(true); HttpRequestImpl reqImpl = new HttpRequestImpl(req); HttpResponseImpl resImpl = new HttpResponseImpl(res); ClassLoader oldContextClassLoader = Thread.currentThread().getContextClassLoader(); @@ -104,7 +110,7 @@ public void service(HttpServletRequest req, HttpServletResponse res) { handleRequest(reqImpl, resImpl); } finally { Thread.currentThread().setContextClassLoader(oldContextClassLoader); - resImpl.flush(); + resImpl.close(); } } @@ -114,7 +120,7 @@ private void handleRequest(HttpRequest req, HttpResponse res) { reqObj = format.deserialize(req, argType); } catch (Throwable t) { logger.log(Level.SEVERE, "Failed to parse request for " + function.getClass().getName(), t); - res.setStatusCode(HttpServletResponse.SC_BAD_REQUEST); + res.setStatusCode(HttpStatus.BAD_REQUEST_400); return; } @@ -123,7 +129,7 @@ private void handleRequest(HttpRequest req, HttpResponse res) { resObj = function.apply(reqObj); } catch (Throwable t) { logger.log(Level.SEVERE, "Failed to execute " + function.getClass().getName(), t); - res.setStatusCode(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + res.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); return; } @@ -132,7 +138,7 @@ private void handleRequest(HttpRequest req, HttpResponse res) { } catch (Throwable t) { logger.log( Level.SEVERE, "Failed to serialize response for " + function.getClass().getName(), t); - res.setStatusCode(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + res.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); return; } } @@ -147,7 +153,7 @@ private static class GsonWireFormat implements TypedFunction.WireFormat { @Override public void serialize(Object object, HttpResponse response) throws Exception { if (object == null) { - response.setStatusCode(HttpServletResponse.SC_NO_CONTENT); + response.setStatusCode(HttpStatus.NO_CONTENT_204); return; } try (BufferedWriter bodyWriter = response.getWriter()) { diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java index 2119645a..dce4e8e1 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java @@ -33,12 +33,14 @@ import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.Part; public class HttpRequestImpl implements HttpRequest { private final HttpServletRequest request; + private final MultipartConfigElement multipartConfigElement = new MultipartConfigElement(""); public HttpRequestImpl(HttpServletRequest request) { this.request = request; @@ -81,6 +83,7 @@ public Map getParts() { throw new IllegalStateException("Content-Type must be multipart/form-data: " + contentType); } try { + request.setAttribute("org.eclipse.jetty.multipartConfig", multipartConfigElement); return request.getParts().stream().collect(toMap(Part::getName, HttpPartImpl::new)); } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java index c02246f0..5fab86b7 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java @@ -20,6 +20,7 @@ import java.io.BufferedWriter; import java.io.IOException; import java.io.OutputStream; +import java.io.Writer; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -27,6 +28,7 @@ import java.util.Optional; import java.util.TreeMap; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.util.IO; public class HttpResponseImpl implements HttpResponse { private final HttpServletResponse response; @@ -43,7 +45,7 @@ public void setStatusCode(int code) { @Override @SuppressWarnings("deprecation") public void setStatusCode(int code, String message) { - response.setStatus(code, message); + response.setStatus(code); } @Override @@ -86,32 +88,92 @@ public OutputStream getOutputStream() throws IOException { @Override public synchronized BufferedWriter getWriter() throws IOException { if (writer == null) { - // Unfortunately this means that we get two intermediate objects between the object we return - // and the underlying Writer that response.getWriter() wraps. We could try accessing the - // PrintWriter.out field via reflection, but that sort of access to non-public fields of - // platform classes is now frowned on and may draw warnings or even fail in subsequent - // versions. We could instead wrap the OutputStream, but that would require us to deduce the - // appropriate Charset, using logic like this: - // https://github.com/eclipse/jetty.project/blob/923ec38adf/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java#L731 - // We may end up doing that if performance is an issue. - writer = new BufferedWriter(response.getWriter()); + writer = new NonBufferedWriter(response.getWriter()); } return writer; } - public void flush() { + /** Close the response, flushing all content. */ + public void close() { try { - // We can't use HttpServletResponse.flushBuffer() because we wrap the - // PrintWriter returned by HttpServletResponse in our own BufferedWriter - // to match our API. So we have to flush whichever of getWriter() or - // getOutputStream() works. + IO.close(getOutputStream()); + } catch (IllegalStateException | IOException e) { try { - getOutputStream().flush(); - } catch (IllegalStateException e) { - getWriter().flush(); + IO.close(getWriter()); + } catch (IOException ioe) { + // Too bad, can't close. } - } catch (IOException e) { - // Too bad, can't flush. + } + } + + /** + * A {@link BufferedWriter} that does not buffer. It is generally more efficient to buffer at the + * lower level, since frequently total content is smaller than a single buffer and the lower level + * buffer can turn a close into a last write that will avoid chunking the response if at all + * possible. However, {@link BufferedWriter} is in the API for {@link HttpResponse}, so we must + * return a writer of that type. + */ + private static class NonBufferedWriter extends BufferedWriter { + private final Writer writer; + + public NonBufferedWriter(Writer out) { + super(out, 1); + writer = out; + } + + @Override + public void write(int c) throws IOException { + writer.write(c); + } + + @Override + public void write(char[] cbuf) throws IOException { + writer.write(cbuf); + } + + @Override + public void write(char[] cbuf, int off, int len) throws IOException { + writer.write(cbuf, off, len); + } + + @Override + public void write(String str) throws IOException { + writer.write(str); + } + + @Override + public void write(String str, int off, int len) throws IOException { + writer.write(str, off, len); + } + + @Override + public Writer append(CharSequence csq) throws IOException { + return writer.append(csq); + } + + @Override + public Writer append(CharSequence csq, int start, int end) throws IOException { + return writer.append(csq, start, end); + } + + @Override + public Writer append(char c) throws IOException { + return writer.append(c); + } + + @Override + public void flush() throws IOException { + writer.flush(); + } + + @Override + public void close() throws IOException { + writer.close(); + } + + @Override + public void newLine() throws IOException { + writer.write(System.lineSeparator()); } } } diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java index 892d6038..67d188e3 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java @@ -44,23 +44,19 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Stream; -import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; -import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.thread.QueuedThreadPool; /** @@ -87,7 +83,7 @@ public class Invoker { // if we arrange for them to be formatted using StackDriver's "structured // logging" JSON format. Remove the JDK's standard logger and replace it with // the JSON one. - for (Handler handler : rootLogger.getHandlers()) { + for (java.util.logging.Handler handler : rootLogger.getHandlers()) { rootLogger.removeHandler(handler); } rootLogger.addHandler(new JsonLogHandler(System.out, false)); @@ -233,17 +229,6 @@ ClassLoader getFunctionClassLoader() { return functionClassLoader; } - /** - * This will start the server and wait (join) for function calls. To start the server inside a - * unit or integration test, use {@link #startTestServer()} instead. - * - * @see #stopServer() - * @throws Exception - */ - public void startServer() throws Exception { - startServer(true); - } - /** * This will start the server and return. * @@ -270,12 +255,23 @@ public void startServer() throws Exception { * } * * @see #stopServer() - * @throws Exception + * @throws Exception If there was a problem starting the server */ public void startTestServer() throws Exception { startServer(false); } + /** + * This will start the server and wait (join) for function calls. To start the server inside a + * unit or integration test, use {@link #startTestServer()} instead. + * + * @see #stopServer() + * @throws Exception If there was a problem starting the server + */ + public void startServer() throws Exception { + startServer(true); + } + private void startServer(boolean join) throws Exception { if (server != null) { throw new IllegalStateException("Server already started"); @@ -285,32 +281,30 @@ private void startServer(boolean join) throws Exception { server = new Server(pool); ServerConnector connector = new ServerConnector(server); connector.setPort(port); + connector.setReuseAddress(true); server.setConnectors(new Connector[] {connector}); - - ServletContextHandler servletContextHandler = new ServletContextHandler(); - servletContextHandler.setContextPath("/"); - server.setHandler(NotFoundHandler.forServlet(servletContextHandler)); + server.setHandler(new NotFoundHandler()); Class functionClass = loadFunctionClass(); - HttpServlet servlet; + Handler handler; if (functionSignatureType == null) { - servlet = servletForDeducedSignatureType(functionClass); + handler = handlerForDeducedSignatureType(functionClass); } else { switch (functionSignatureType) { case "http": if (TypedFunction.class.isAssignableFrom(functionClass)) { - servlet = TypedFunctionExecutor.forClass(functionClass); + handler = TypedFunctionExecutor.forClass(functionClass); } else { - servlet = HttpFunctionExecutor.forClass(functionClass); + handler = HttpFunctionExecutor.forClass(functionClass); } break; case "event": case "cloudevent": - servlet = BackgroundFunctionExecutor.forClass(functionClass); + handler = BackgroundFunctionExecutor.forClass(functionClass); break; case "typed": - servlet = TypedFunctionExecutor.forClass(functionClass); + handler = TypedFunctionExecutor.forClass(functionClass); break; default: String error = @@ -321,10 +315,12 @@ private void startServer(boolean join) throws Exception { throw new RuntimeException(error); } } - ServletHolder servletHolder = new ServletHolder(servlet); - servletHolder.getRegistration().setMultipartConfig(new MultipartConfigElement("")); - servletContextHandler.addServlet(servletHolder, "/*"); + HandlerWrapper tail = server; + while (tail.getHandler() != null && tail.getHandler() instanceof HandlerWrapper) { + tail = (HandlerWrapper) tail.getHandler(); + } + tail.setHandler(handler); server.start(); logServerInfo(); if (join) { @@ -371,7 +367,7 @@ private Class loadFunctionClass() throws ClassNotFoundException { } } - private HttpServlet servletForDeducedSignatureType(Class functionClass) { + private Handler handlerForDeducedSignatureType(Class functionClass) { if (HttpFunction.class.isAssignableFrom(functionClass)) { return HttpFunctionExecutor.forClass(functionClass); } @@ -451,16 +447,11 @@ private static boolean isGcf() { /** * Wrapper that intercepts requests for {@code /favicon.ico} and {@code /robots.txt} and causes - * them to produce a 404 status. Otherwise they would be sent to the function code, like any other - * URL, meaning that someone testing their function by using a browser as an HTTP client can see - * two requests, one for {@code /favicon.ico} and one for {@code /} (or whatever). + * them to produce a 404 status. Otherwise, they would be sent to the function code, like any + * other URL, meaning that someone testing their function by using a browser as an HTTP client can + * see two requests, one for {@code /favicon.ico} and one for {@code /} (or whatever). */ private static class NotFoundHandler extends HandlerWrapper { - static NotFoundHandler forServlet(ServletContextHandler servletHandler) { - NotFoundHandler handler = new NotFoundHandler(); - handler.setHandler(servletHandler); - return handler; - } private static final Set NOT_FOUND_PATHS = new HashSet<>(Arrays.asList("/favicon.ico", "/robots.txt")); diff --git a/invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java b/invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java index 3f3de837..d82fdf64 100644 --- a/invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java +++ b/invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java @@ -165,6 +165,8 @@ abstract static class TestCase { abstract int expectedResponseCode(); + abstract Optional> expectedResponseHeaders(); + abstract Optional expectedResponseText(); abstract Optional expectedJson(); @@ -184,6 +186,7 @@ static Builder builder() { .setUrl("/") .setRequestText("") .setExpectedResponseCode(HttpStatus.OK_200) + .setExpectedResponseHeaders(ImmutableMap.of()) .setExpectedResponseText("") .setHttpContentType("text/plain") .setHttpHeaders(ImmutableMap.of()); @@ -202,6 +205,8 @@ Builder setRequestText(String text) { abstract Builder setExpectedResponseCode(int x); + abstract Builder setExpectedResponseHeaders(Map x); + abstract Builder setExpectedResponseText(String x); abstract Builder setExpectedResponseText(Optional x); @@ -247,17 +252,49 @@ public void helloWorld() throws Exception { testHttpFunction( fullTarget("HelloWorld"), ImmutableList.of( - TestCase.builder().setExpectedResponseText("hello\n").build(), + TestCase.builder() + .setExpectedResponseHeaders(ImmutableMap.of("Content-Length", "*")) + .setExpectedResponseText("hello\n") + .build(), FAVICON_TEST_CASE, ROBOTS_TXT_TEST_CASE)); } + @Test + public void bufferedWrites() throws Exception { + // This test checks that writes are buffered and are written + // in an efficient way with known content-length if possible. + testHttpFunction( + fullTarget("BufferedWrites"), + ImmutableList.of( + TestCase.builder() + .setUrl("/target?writes=2") + .setExpectedResponseText("write 0\nwrite 1\n") + .setExpectedResponseHeaders( + ImmutableMap.of( + "x-write-0", "true", + "x-write-1", "true", + "x-written", "true", + "Content-Length", "16")) + .build(), + TestCase.builder() + .setUrl("/target?writes=2&flush=true") + .setExpectedResponseText("write 0\nwrite 1\n") + .setExpectedResponseHeaders( + ImmutableMap.of( + "x-write-0", "true", + "x-write-1", "true", + "x-written", "-", + "Transfer-Encoding", "chunked")) + .build())); + } + @Test public void exceptionHttp() throws Exception { String exceptionExpectedOutput = "\"severity\": \"ERROR\", \"logging.googleapis.com/sourceLocation\": {\"file\":" + " \"com/google/cloud/functions/invoker/HttpFunctionExecutor.java\", \"method\":" - + " \"service\"}, \"message\": \"Failed to execute" + + " \"handle\"}, \"message\": \"Failed to execute" + " com.google.cloud.functions.invoker.testfunctions.ExceptionHttp\\n" + "java.lang.RuntimeException: exception thrown for test"; testHttpFunction( @@ -274,7 +311,7 @@ public void exceptionBackground() throws Exception { String exceptionExpectedOutput = "\"severity\": \"ERROR\", \"logging.googleapis.com/sourceLocation\": {\"file\":" + " \"com/google/cloud/functions/invoker/BackgroundFunctionExecutor.java\", \"method\":" - + " \"service\"}, \"message\": \"Failed to execute" + + " \"handle\"}, \"message\": \"Failed to execute" + " com.google.cloud.functions.invoker.testfunctions.ExceptionBackground\\n" + "java.lang.RuntimeException: exception thrown for test"; @@ -345,6 +382,9 @@ public void stackDriverLogging() throws Exception { + "\"method\": \"service\"}, " + "\"message\": \"oops\\njava.lang.Exception: disaster\\n" + " at com.google.cloud.functions.invoker.testfunctions.Log.service(Log.java:"; + + // TODO this test case is suspected of being flaky, as occasionally the final log entry + // is missing TestCase exceptionTestCase = TestCase.builder() .setUrl("/?message=oops&level=severe&exception=disaster") @@ -549,7 +589,7 @@ public void multipart() throws Exception { fullTarget("Multipart"), ImmutableList.of( TestCase.builder() - .setHttpContentType(Optional.empty()) + .setHttpContentType(multiPartProvider.getContentType()) .setRequestContent(multiPartProvider) .setExpectedResponseText(expectedResponse) .build())); @@ -694,6 +734,26 @@ private void testFunction( .withMessage("Response to %s is %s %s", uri, response.getStatus(), response.getReason()) .that(response.getStatus()) .isEqualTo(testCase.expectedResponseCode()); + testCase + .expectedResponseHeaders() + .ifPresent( + map -> { + for (Map.Entry entry : map.entrySet()) { + if ("*".equals(entry.getValue())) { + expect + .that(response.getHeaders().getFieldNamesCollection()) + .contains(entry.getKey()); + } else if ("-".equals(entry.getValue())) { + expect + .that(response.getHeaders().getFieldNamesCollection()) + .doesNotContain(entry.getKey()); + } else { + expect + .that(response.getHeaders().getValuesList(entry.getKey())) + .contains(entry.getValue()); + } + } + }); testCase .expectedResponseText() .ifPresent(text -> expect.that(response.getContentAsString()).isEqualTo(text)); diff --git a/invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java b/invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java index e52ec62a..df23a350 100644 --- a/invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java +++ b/invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java @@ -35,8 +35,7 @@ import java.util.TreeMap; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; -import javax.servlet.MultipartConfigElement; -import javax.servlet.http.HttpServlet; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.HttpClient; @@ -48,9 +47,10 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpStatus.Code; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.BeforeClass; import org.junit.Test; @@ -82,21 +82,16 @@ public static void allocateServerPort() throws IOException { } /** - * Wrapper class that allows us to start a Jetty server with a single servlet for {@code /*} - * within a try-with-resources statement. The servlet will be configured to support multipart + * Wrapper class that allows us to start a Jetty server with a single handler for {@code /*} + * within a try-with-resources statement. The handler will be configured to support multipart * requests. */ private static class SimpleServer implements AutoCloseable { private final Server server; - SimpleServer(HttpServlet servlet) throws Exception { + SimpleServer(Handler handler) throws Exception { this.server = new Server(serverPort); - ServletContextHandler context = new ServletContextHandler(); - context.setContextPath("/"); - server.setHandler(context); - ServletHolder servletHolder = new ServletHolder(servlet); - servletHolder.getRegistration().setMultipartConfig(new MultipartConfigElement("tiddly")); - context.addServlet(servletHolder, "/*"); + server.setHandler(handler); server.start(); } @@ -113,16 +108,16 @@ private interface HttpRequestTest { /** * Tests methods on the {@link HttpRequest} object while the request is being serviced. We are not - * guaranteed that the underlying {@link HttpServletRequest} object will still be valid when the - * request completes, and in fact in Jetty it isn't. So we perform the checks in the context of - * the servlet, and report any exception back to the test method. + * guaranteed that the underlying {@link Request} object will still be valid when the request + * completes, and in fact in Jetty it isn't. So we perform the checks in the context of the + * handler, and report any exception back to the test method. */ @Test public void httpRequestMethods() throws Exception { AtomicReference testReference = new AtomicReference<>(); AtomicReference exceptionReference = new AtomicReference<>(); - HttpRequestServlet testServlet = new HttpRequestServlet(testReference, exceptionReference); - try (SimpleServer server = new SimpleServer(testServlet)) { + HttpRequestHandler testHandler = new HttpRequestHandler(testReference, exceptionReference); + try (SimpleServer server = new SimpleServer(testHandler)) { httpRequestMethods(testReference, exceptionReference); } } @@ -223,8 +218,8 @@ public void emptyRequest() throws Exception { }; AtomicReference exceptionReference = new AtomicReference<>(); AtomicReference testReference = new AtomicReference<>(test); - HttpRequestServlet testServlet = new HttpRequestServlet(testReference, exceptionReference); - try (SimpleServer server = new SimpleServer(testServlet)) { + HttpRequestHandler testHandler = new HttpRequestHandler(testReference, exceptionReference); + try (SimpleServer server = new SimpleServer(testHandler)) { ContentResponse response = httpClient.POST(uri).send(); assertThat(response.getStatus()).isEqualTo(HttpStatus.OK_200); throwIfNotNull(exceptionReference.get()); @@ -240,7 +235,7 @@ private void validateReader(BufferedReader reader) { public void multiPartRequest() throws Exception { AtomicReference testReference = new AtomicReference<>(); AtomicReference exceptionReference = new AtomicReference<>(); - HttpRequestServlet testServlet = new HttpRequestServlet(testReference, exceptionReference); + HttpRequestHandler testHandler = new HttpRequestHandler(testReference, exceptionReference); HttpClient httpClient = new HttpClient(); httpClient.start(); String uri = "http://localhost:" + serverPort + "/"; @@ -283,7 +278,7 @@ public void multiPartRequest() throws Exception { assertThat(bytes).isEqualTo(RANDOM_BYTES); } }; - try (SimpleServer server = new SimpleServer(testServlet)) { + try (SimpleServer server = new SimpleServer(testHandler)) { testReference.set(test); Request request = httpClient.POST(uri).header("foo", "oof").content(multiPart); ContentResponse response = request.send(); @@ -292,11 +287,11 @@ public void multiPartRequest() throws Exception { } } - private static class HttpRequestServlet extends HttpServlet { + private static class HttpRequestHandler extends AbstractHandler { private final AtomicReference testReference; private final AtomicReference exceptionReference; - private HttpRequestServlet( + private HttpRequestHandler( AtomicReference testReference, AtomicReference exceptionReference) { this.testReference = testReference; @@ -304,7 +299,13 @@ private HttpRequestServlet( } @Override - protected void doPost(HttpServletRequest req, HttpServletResponse resp) { + public void handle( + String s, + org.eclipse.jetty.server.Request baseRequest, + HttpServletRequest req, + HttpServletResponse res) + throws IOException, ServletException { + baseRequest.setHandled(true); try { testReference.get().test(new HttpRequestImpl(req)); } catch (Throwable t) { @@ -327,8 +328,8 @@ private interface HttpResponseTest { public void httpResponseSetAndGet() throws Exception { AtomicReference testReference = new AtomicReference<>(); AtomicReference exceptionReference = new AtomicReference<>(); - HttpResponseServlet testServlet = new HttpResponseServlet(testReference, exceptionReference); - try (SimpleServer server = new SimpleServer(testServlet)) { + HttpResponseHandler testHandler = new HttpResponseHandler(testReference, exceptionReference); + try (SimpleServer server = new SimpleServer(testHandler)) { httpResponseSetAndGet(testReference, exceptionReference); } } @@ -350,8 +351,7 @@ private void httpResponseSetAndGet( .containsAtLeast("Content-Type", Arrays.asList("application/octet-stream")); }, response -> { - Map> initialHeaders = response.getHeaders(); - // The servlet spec says this should be empty, but actually we get a Date header here. + // The fields are initialized with a Date header as per the HTTP RFCs. // So we just check that we can add our own headers. response.appendHeader("foo", "bar"); response.appendHeader("wibbly", "wobbly"); @@ -373,11 +373,11 @@ private void httpResponseSetAndGet( } } - private static class HttpResponseServlet extends HttpServlet { + private static class HttpResponseHandler extends AbstractHandler { private final AtomicReference testReference; private final AtomicReference exceptionReference; - private HttpResponseServlet( + private HttpResponseHandler( AtomicReference testReference, AtomicReference exceptionReference) { this.testReference = testReference; @@ -385,8 +385,13 @@ private HttpResponseServlet( } @Override - protected void doPost(HttpServletRequest req, HttpServletResponse resp) { + public void handle( + String s, + org.eclipse.jetty.server.Request baseRequest, + HttpServletRequest req, + HttpServletResponse resp) { try { + baseRequest.setHandled(true); testReference.get().test(new HttpResponseImpl(resp)); } catch (Throwable t) { exceptionReference.set(t); @@ -417,15 +422,15 @@ private static ResponseTest responseTest( /** * Tests that operations on the {@link HttpResponse} have the appropriate effect on the HTTP * response that ends up being sent. Here, for each check, we have two operations: the operation - * on the {@link HttpResponse}, which happens inside the servlet, and the operation to check the + * on the {@link HttpResponse}, which happens inside the handler, and the operation to check the * HTTP result, which happens in the client thread. */ @Test public void httpResponseEffects() throws Exception { AtomicReference testReference = new AtomicReference<>(); AtomicReference exceptionReference = new AtomicReference<>(); - HttpResponseServlet testServlet = new HttpResponseServlet(testReference, exceptionReference); - try (SimpleServer server = new SimpleServer(testServlet)) { + HttpResponseHandler testHandler = new HttpResponseHandler(testReference, exceptionReference); + try (SimpleServer server = new SimpleServer(testHandler)) { httpResponseEffects(testReference, exceptionReference); } } @@ -445,10 +450,11 @@ private void httpResponseEffects( response -> response.setStatusCode(HttpStatus.IM_A_TEAPOT_418), response -> assertThat(response.getStatus()).isEqualTo(HttpStatus.IM_A_TEAPOT_418)), responseTest( + // reason string cannot be set by application response -> response.setStatusCode(HttpStatus.IM_A_TEAPOT_418, "Je suis une théière"), response -> { assertThat(response.getStatus()).isEqualTo(HttpStatus.IM_A_TEAPOT_418); - assertThat(response.getReason()).isEqualTo("Je suis une théière"); + assertThat(response.getReason()).isEqualTo(Code.IM_A_TEAPOT.getMessage()); }), responseTest( response -> response.setContentType("application/noddy"), diff --git a/invoker/core/src/test/java/com/google/cloud/functions/invoker/testfunctions/BufferedWrites.java b/invoker/core/src/test/java/com/google/cloud/functions/invoker/testfunctions/BufferedWrites.java new file mode 100644 index 00000000..a7989a74 --- /dev/null +++ b/invoker/core/src/test/java/com/google/cloud/functions/invoker/testfunctions/BufferedWrites.java @@ -0,0 +1,27 @@ +package com.google.cloud.functions.invoker.testfunctions; + +import com.google.cloud.functions.HttpFunction; +import com.google.cloud.functions.HttpRequest; +import com.google.cloud.functions.HttpResponse; +import java.io.BufferedWriter; +import java.util.List; +import java.util.Map; + +public class BufferedWrites implements HttpFunction { + @Override + public void service(HttpRequest request, HttpResponse response) throws Exception { + Map> queryParameters = request.getQueryParameters(); + int writes = Integer.parseInt(request.getFirstQueryParameter("writes").orElse("0")); + boolean flush = Boolean.parseBoolean(request.getFirstQueryParameter("flush").orElse("false")); + + BufferedWriter writer = response.getWriter(); + for (int i = 0; i < writes; i++) { + response.appendHeader("x-write-" + i, "true"); + writer.write("write " + i + "\n"); + } + if (flush) { + writer.flush(); + } + response.appendHeader("x-written", "true"); + } +}