-
Notifications
You must be signed in to change notification settings - Fork 66
chore(implementation): use Jetty-12.1 core without servlets #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6a9b6f9
42ec133
66ec265
cff88d1
712daf1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,25 +30,32 @@ | |
| import io.cloudevents.http.HttpMessageFactory; | ||
| import java.io.BufferedReader; | ||
| import java.io.IOException; | ||
| import java.io.InputStreamReader; | ||
| import java.io.Reader; | ||
| import java.lang.reflect.Type; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.time.OffsetDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.TreeMap; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import javax.servlet.http.HttpServlet; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import org.eclipse.jetty.http.HttpField; | ||
| import org.eclipse.jetty.http.HttpHeader; | ||
| import org.eclipse.jetty.http.HttpStatus; | ||
| import org.eclipse.jetty.io.Content; | ||
| import org.eclipse.jetty.server.Handler; | ||
| import org.eclipse.jetty.server.Request; | ||
| import org.eclipse.jetty.server.Response; | ||
| import org.eclipse.jetty.util.Callback; | ||
|
|
||
| /** Executes the user's background function. */ | ||
| public final class BackgroundFunctionExecutor extends HttpServlet { | ||
| public final class BackgroundFunctionExecutor extends Handler.Abstract { | ||
| private static final Logger logger = Logger.getLogger("com.google.cloud.functions.invoker"); | ||
|
|
||
| private final FunctionExecutor<?> functionExecutor; | ||
|
|
@@ -177,8 +184,13 @@ static Optional<Type> backgroundFunctionTypeArgument( | |
| .findFirst(); | ||
| } | ||
|
|
||
| private static Event parseLegacyEvent(HttpServletRequest req) throws IOException { | ||
| try (BufferedReader bodyReader = req.getReader()) { | ||
| private static Event parseLegacyEvent(Request req) throws IOException { | ||
| try (BufferedReader bodyReader = | ||
| new BufferedReader( | ||
| new InputStreamReader( | ||
| Content.Source.asInputStream(req), | ||
| Objects.requireNonNullElse( | ||
| Request.getCharset(req), StandardCharsets.ISO_8859_1)))) { | ||
| return parseLegacyEvent(bodyReader); | ||
| } | ||
| } | ||
|
|
@@ -225,7 +237,7 @@ private static Context contextFromCloudEvent(CloudEvent cloudEvent) { | |
| * for the various triggers. CloudEvents are ones that follow the standards defined by <a | ||
| * href="https://cloudevents.io">cloudevents.io</a>. | ||
| * | ||
| * @param <CloudEventDataT> the type to be used in the {@link Unmarshallers} call when | ||
| * @param <CloudEventDataT> the type to be used in the {code Unmarshallers} call when | ||
| * unmarshalling this event, if it is a CloudEvent. | ||
| */ | ||
| private abstract static class FunctionExecutor<CloudEventDataT> { | ||
|
|
@@ -322,23 +334,25 @@ 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 { | ||
| String contentType = req.getContentType(); | ||
| public boolean handle(Request req, Response res, Callback callback) throws Exception { | ||
| String contentType = req.getHeaders().get(HttpHeader.CONTENT_TYPE); | ||
| try { | ||
| executionIdUtil.storeExecutionId(req); | ||
| if ((contentType != null && contentType.startsWith("application/cloudevents+json")) | ||
| || req.getHeader("ce-specversion") != null) { | ||
| || req.getHeaders().get("ce-specversion") != null) { | ||
| serviceCloudEvent(req); | ||
| } else { | ||
| serviceLegacyEvent(req); | ||
| } | ||
| res.setStatus(HttpServletResponse.SC_OK); | ||
| res.setStatus(HttpStatus.OK_200); | ||
| callback.succeeded(); | ||
| } catch (Throwable t) { | ||
| res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); | ||
| logger.log(Level.SEVERE, "Failed to execute " + functionExecutor.functionName(), t); | ||
| Response.writeError(req, res, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, null); | ||
| } finally { | ||
| executionIdUtil.removeExecutionId(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| private enum CloudEventKind { | ||
|
|
@@ -352,10 +366,14 @@ private enum CloudEventKind { | |
| * @param <CloudEventT> a fake type parameter, which corresponds to the type parameter of {@link | ||
| * FunctionExecutor}. | ||
| */ | ||
| private <CloudEventT> void serviceCloudEvent(HttpServletRequest req) throws Exception { | ||
| private <CloudEventT> void serviceCloudEvent(Request req) throws Exception { | ||
| @SuppressWarnings("unchecked") | ||
| FunctionExecutor<CloudEventT> executor = (FunctionExecutor<CloudEventT>) functionExecutor; | ||
| byte[] body = req.getInputStream().readAllBytes(); | ||
|
|
||
| // Read the entire request body into a byte array. | ||
| // TODO: this method is deprecated for removal, use the method introduced by | ||
| // https://github.com/jetty/jetty.project/pull/13939 when it is released. | ||
| byte[] body = Content.Source.asByteArrayAsync(req, -1).get(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain this array manipulation in a comment? why the '-1'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This But this is simply reading the entire content of the request into a |
||
| MessageReader reader = HttpMessageFactory.createReaderFromMultimap(headerMap(req), body); | ||
| // It's important not to set the context ClassLoader earlier, because MessageUtils will use | ||
| // ServiceLoader.load(EventFormat.class) to find a handler to deserialize a binary CloudEvent | ||
|
|
@@ -369,17 +387,17 @@ private <CloudEventT> void serviceCloudEvent(HttpServletRequest req) throws Exce | |
| // https://github.com/cloudevents/sdk-java/pull/259. | ||
| } | ||
|
|
||
| private static Map<String, List<String>> headerMap(HttpServletRequest req) { | ||
| private static Map<String, List<String>> headerMap(Request req) { | ||
| Map<String, List<String>> headerMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | ||
| for (String header : Collections.list(req.getHeaderNames())) { | ||
| for (String value : Collections.list(req.getHeaders(header))) { | ||
| headerMap.computeIfAbsent(header, unused -> new ArrayList<>()).add(value); | ||
| } | ||
| for (HttpField field : req.getHeaders()) { | ||
| headerMap | ||
| .computeIfAbsent(field.getName(), unused -> new ArrayList<>()) | ||
| .addAll(field.getValueList()); | ||
| } | ||
| return headerMap; | ||
| } | ||
|
|
||
| private void serviceLegacyEvent(HttpServletRequest req) throws Exception { | ||
| private void serviceLegacyEvent(Request req) throws Exception { | ||
| Event event = parseLegacyEvent(req); | ||
| runWithContextClassLoader(() -> functionExecutor.serviceLegacyEvent(event)); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment here or at the top of the method explaining the return value? It seems confusing to me at face value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is specified in the javadoc of
handleIn this case we are always handling the request so we should always return true.