WIP

Author: gregw

invoker/core/src/main/java/com/google/cloud/functions/invoker/HttpFunctionExecutor.java

@@ -68,13 +68,13 @@ public boolean handle(Request request, Response response, Callback callback) thr
     try {
       Thread.currentThread().setContextClassLoader(function.getClass().getClassLoader());
       function.service(reqImpl, respImpl);
+      respImpl.close();
       callback.succeeded();
     } catch (Throwable t) {
       logger.log(Level.SEVERE, "Failed to execute " + function.getClass().getName(), t);
       Response.writeError(request, response, callback, t);
     } finally {
       Thread.currentThread().setContextClassLoader(oldContextLoader);
-      respImpl.flush();
     }
     return true;
   }

invoker/core/src/main/java/com/google/cloud/functions/invoker/TypedFunctionExecutor.java

@@ -105,7 +105,7 @@ public boolean handle(Request req, Response res, Callback callback) throws Excep
     try {
       Thread.currentThread().setContextClassLoader(function.getClass().getClassLoader());
       handleRequest(reqImpl, resImpl);
-      resImpl.flush();
+      resImpl.close();
       callback.succeeded();
     } catch (Throwable t) {
       Response.writeError(req, res, callback, t);

invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java

@@ -81,7 +81,6 @@ public Map<String, List<String>> getQueryParameters() {
 
   @Override
   public Map<String, HttpPart> getParts() {
-
     // TODO initiate reading the parts asynchronously before invocation
     String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE);
     if (Type.MULTIPART_FORM_DATA.is(MimeTypes.getContentTypeWithoutCharset(contentType))) {
@@ -115,15 +114,31 @@ public Optional<String> getCharacterEncoding() {
     return Optional.ofNullable(charset == null ? null : charset.name());
   }
 
+  private InputStream inputStream;
+  private BufferedReader reader;
+
   @Override
   public InputStream getInputStream() throws IOException {
-    return Content.Source.asInputStream(request);
+    if (reader != null) {
+      throw new IllegalStateException("getReader() already called");
+    }
+    if (inputStream == null) {
+      inputStream = Content.Source.asInputStream(request);
+    }
+    return inputStream;
   }
 
   @Override
   public BufferedReader getReader() throws IOException {
-    return new BufferedReader(new InputStreamReader(getInputStream(),
-        getCharacterEncoding().orElse(StandardCharsets.UTF_8.name())));
+    if (reader == null) {
+      if (inputStream != null) {
+        throw new IllegalStateException("getInputStream already called");
+      }
+      inputStream = Content.Source.asInputStream(request);
+      reader = new BufferedReader(new InputStreamReader(getInputStream(),
+          getCharacterEncoding().orElse(StandardCharsets.UTF_8.name())));
+    }
+    return reader;
   }
 
   @Override

invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java

@@ -17,6 +17,7 @@
 import static java.util.stream.Collectors.toMap;
 
 import com.google.cloud.functions.HttpResponse;
+import java.io.BufferedOutputStream;
 import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -32,7 +33,7 @@
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.io.Content;
-import org.eclipse.jetty.io.AbstractOutputStreamWriter;
+import org.eclipse.jetty.io.WriteThroughWriter;
 import org.eclipse.jetty.server.Response;
 
 public class HttpResponseImpl implements HttpResponse {
@@ -83,41 +84,46 @@ private static <T> List<T> list(Collection<T> collection) {
     return (collection instanceof List<?>) ? (List<T>) collection : new ArrayList<>(collection);
   }
 
+  private OutputStream outputStream;
   private BufferedWriter writer;
 
   @Override
-  public OutputStream getOutputStream() throws IOException {
+  public OutputStream getOutputStream() {
     if (writer != null) {
       throw new IllegalStateException("getWriter called");
+    } else if (outputStream == null) {
+      // TODO use BufferedSink when it is available
+      outputStream = new BufferedOutputStream(Content.Sink.asOutputStream(response));
     }
-    return Content.Sink.asOutputStream(response);
+    return outputStream;
   }
 
   @Override
   public synchronized BufferedWriter getWriter() throws IOException {
     if (writer == null) {
+      if (outputStream != null) {
+        throw new IllegalStateException("getOutputStream called");
+      }
       String contentType = getContentType().orElse(null);
       Charset charset = Objects.requireNonNullElse(
           response.getRequest().getContext().getMimeTypes().getCharset(contentType),
           StandardCharsets.UTF_8);
-      writer = new BufferedWriter(AbstractOutputStreamWriter.newWriter(getOutputStream(), charset));
+      // TODO should we buffer in the input stream rather than as characters
+      outputStream = Content.Sink.asOutputStream(response);
+      writer = new BufferedWriter(WriteThroughWriter.newWriter(getOutputStream(), charset));
     }
     return writer;
   }
 
-  public void flush() {
+  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.
-      try {
-        getOutputStream().flush();
-      } catch (IllegalStateException e) {
-        getWriter().flush();
+      if (writer != null) {
+        writer.close();
+      } else if (outputStream != null) {
+        outputStream.close();
       }
     } catch (IOException e) {
-      // Too bad, can't flush.
+      // Too bad, can't close.
     }
   }
 }