Skip to content

Commit 66ec265

Browse files
chore(implementation)!: use Jetty-12 core without servlets
Port the invoker to upgrade to Eclipse Jetty-12 version 12. Specifically using the new core APIs of Eclipse Jetty-12 that allow the overhead of a Servlet container to be avoided. BREAKING CHANGE: use Java 17 or above, as required by Eclipse Jetty-12. Signed-off-by: Lachlan Roberts <[email protected]>
1 parent 42ec133 commit 66ec265

File tree

8 files changed

+96
-41
lines changed

8 files changed

+96
-41
lines changed

invoker/core/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
<maven.compiler.source>17</maven.compiler.source>
2424
<maven.compiler.target>17</maven.compiler.target>
2525
<cloudevents.sdk.version>4.0.1</cloudevents.sdk.version>
26-
<jetty.version>12.1.1</jetty.version>
26+
<jetty.version>12.1.3</jetty.version>
2727
</properties>
2828

2929
<licenses>

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,7 @@ public boolean handle(Request req, Response res, Callback callback) throws Excep
345345
callback.succeeded();
346346
} catch (Throwable t) {
347347
logger.log(Level.SEVERE, "Failed to execute " + functionExecutor.functionName(), t);
348-
res.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500);
349-
callback.succeeded();
348+
Response.writeError(req, res, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, null);
350349
} finally {
351350
executionIdUtil.removeExecutionId();
352351
}

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

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,16 @@
2121
import java.io.InputStreamReader;
2222
import java.nio.charset.Charset;
2323
import java.nio.charset.StandardCharsets;
24-
import java.util.ArrayList;
2524
import java.util.Collections;
2625
import java.util.HashMap;
2726
import java.util.List;
2827
import java.util.Map;
2928
import java.util.Objects;
3029
import java.util.Optional;
31-
import java.util.TreeMap;
32-
import org.eclipse.jetty.http.*;
30+
import org.eclipse.jetty.http.HttpHeader;
31+
import org.eclipse.jetty.http.MimeTypes;
3332
import org.eclipse.jetty.http.MultiPart.Part;
33+
import org.eclipse.jetty.http.MultiPartFormData;
3434
import org.eclipse.jetty.io.Content;
3535
import org.eclipse.jetty.server.Request;
3636
import org.eclipse.jetty.util.Fields;
@@ -144,15 +144,7 @@ public BufferedReader getReader() throws IOException {
144144

145145
@Override
146146
public Map<String, List<String>> getHeaders() {
147-
return toStringListMap(request.getHeaders());
148-
}
149-
150-
static Map<String, List<String>> toStringListMap(HttpFields headers) {
151-
Map<String, List<String>> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
152-
for (HttpField field : headers) {
153-
map.computeIfAbsent(field.getName(), key -> new ArrayList<>()).add(field.getValue());
154-
}
155-
return map;
147+
return HttpUtil.toStringListMap(request.getHeaders());
156148
}
157149

158150
private static class HttpPartImpl implements HttpPart {
@@ -186,8 +178,7 @@ public Optional<String> getCharacterEncoding() {
186178

187179
@Override
188180
public InputStream getInputStream() throws IOException {
189-
// TODO: update with createContentSource when https://github.com/jetty/jetty.project/pull/13610 is released.
190-
Content.Source contentSource = part.newContentSource(null, 0, -1);
181+
Content.Source contentSource = part.createContentSource();
191182
return Content.Source.asInputStream(contentSource);
192183
}
193184

@@ -202,7 +193,7 @@ public BufferedReader getReader() throws IOException {
202193

203194
@Override
204195
public Map<String, List<String>> getHeaders() {
205-
return HttpRequestImpl.toStringListMap(part.getHeaders());
196+
return HttpUtil.toStringListMap(part.getHeaders());
206197
}
207198

208199
@Override

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,15 @@ public void appendHeader(String key, String value) {
7777

7878
@Override
7979
public Map<String, List<String>> getHeaders() {
80-
return HttpRequestImpl.toStringListMap(response.getHeaders());
80+
return HttpUtil.toStringListMap(response.getHeaders());
8181
}
8282

8383
@Override
8484
public OutputStream getOutputStream() {
8585
if (writer != null) {
8686
throw new IllegalStateException("getWriter called");
87-
} else if (outputStream == null) {
87+
}
88+
if (outputStream == null) {
8889
Request request = response.getRequest();
8990
int outputBufferSize = request.getConnectionMetaData().getHttpConfiguration()
9091
.getOutputBufferSize();
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.cloud.functions.invoker.http;
16+
17+
import java.util.ArrayList;
18+
import java.util.List;
19+
import java.util.Map;
20+
import java.util.TreeMap;
21+
import org.eclipse.jetty.http.HttpField;
22+
import org.eclipse.jetty.http.HttpFields;
23+
24+
class HttpUtil {
25+
public static Map<String, List<String>> toStringListMap(HttpFields headers) {
26+
Map<String, List<String>> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
27+
for (HttpField field : headers) {
28+
map.computeIfAbsent(field.getName(), key -> new ArrayList<>()).add(field.getValue());
29+
}
30+
return map;
31+
}
32+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 Google LLC
1+
// Copyright 2025 Google LLC
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.

invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,13 @@ abstract static class TestCase {
173173

174174
abstract int expectedResponseCode();
175175

176+
/**
177+
* Expected response headers map, header name -> value.
178+
* Value "*" asserts the header is present with any value.
179+
* Value "-" asserts the header is not present.
180+
*
181+
* @return the expected response headers for this test case.
182+
*/
176183
abstract Optional<Map<String, String>> expectedResponseHeaders();
177184

178185
abstract Optional<String> expectedResponseText();
@@ -269,10 +276,39 @@ public void helloWorld() throws Exception {
269276
ROBOTS_TXT_TEST_CASE));
270277
}
271278

279+
@Test
280+
public void timeoutHttpSuccess() throws Exception {
281+
testFunction(
282+
SignatureType.HTTP,
283+
fullTarget("TimeoutHttp"),
284+
ImmutableList.of(),
285+
ImmutableList.of(
286+
TestCase.builder()
287+
.setExpectedResponseText("finished\n")
288+
.setExpectedResponseText(Optional.empty())
289+
.build()),
290+
ImmutableMap.of("CLOUD_RUN_TIMEOUT_SECONDS", "3"));
291+
}
292+
293+
@Test
294+
public void timeoutHttpTimesOut() throws Exception {
295+
testFunction(
296+
SignatureType.HTTP,
297+
fullTarget("TimeoutHttp"),
298+
ImmutableList.of(),
299+
ImmutableList.of(
300+
TestCase.builder()
301+
.setExpectedResponseCode(408)
302+
.setExpectedResponseText(Optional.empty())
303+
.build()),
304+
ImmutableMap.of("CLOUD_RUN_TIMEOUT_SECONDS", "1"));
305+
}
306+
272307
@Test
273308
public void bufferedWrites() throws Exception {
274-
// This test checks that writes are buffered and are written
275-
// in an efficient way with known content-length if possible.
309+
// This test checks that writes are buffered, and are written
310+
// in an efficient way with known content-length if possible
311+
// instead of doing a chunked response.
276312
testHttpFunction(
277313
fullTarget("BufferedWrites"),
278314
ImmutableList.of(
@@ -798,26 +834,26 @@ private void testFunction(
798834
throws Exception {
799835
ServerProcess serverProcess =
800836
startServer(signatureType, target, extraArgs, environmentVariables);
837+
HttpClient httpClient = new HttpClient();
801838
try {
802-
HttpClient httpClient = new HttpClient();
803839
httpClient.start();
804840
for (TestCase testCase : testCases) {
805841
testCase.snoopFile().ifPresent(File::delete);
806842
String uri = "http://localhost:" + serverPort + testCase.url();
807843
Request request = httpClient.POST(uri);
808844

809-
request.headers(m -> {
810-
testCase.httpContentType().ifPresent(contentType -> m.put(HttpHeader.CONTENT_TYPE, contentType));
811-
testCase.httpHeaders().forEach(m::put);
845+
request.headers(headers -> {
846+
testCase.httpContentType().ifPresent(contentType -> headers.put(HttpHeader.CONTENT_TYPE, contentType));
847+
testCase.httpHeaders().forEach(headers::put);
812848
});
813849
request.body(testCase.requestContent());
814850
ContentResponse response = request.send();
815851
expect
816852
.withMessage("Response to %s is %s %s", uri, response.getStatus(), response.getReason())
817853
.that(response.getStatus())
818854
.isEqualTo(testCase.expectedResponseCode());
819-
testCase.expectedResponseHeaders().ifPresent(map -> {
820-
for (Map.Entry<String, String> entry : map.entrySet()) {
855+
testCase.expectedResponseHeaders().ifPresent(expectedResponseHeaders -> {
856+
for (Map.Entry<String, String> entry : expectedResponseHeaders.entrySet()) {
821857
if ("*".equals(entry.getValue())) {
822858
expect.that(response.getHeaders().getFieldNamesCollection()).contains(entry.getKey());
823859
} else if ("-".equals(entry.getValue())) {
@@ -839,6 +875,7 @@ private void testFunction(
839875
}
840876
} finally {
841877
serverProcess.close();
878+
httpClient.stop();
842879
}
843880
for (TestCase testCase : testCases) {
844881
testCase

invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@
4040
import org.eclipse.jetty.client.HttpClient;
4141
import org.eclipse.jetty.client.MultiPartRequestContent;
4242
import org.eclipse.jetty.client.StringRequestContent;
43-
import org.eclipse.jetty.http.*;
43+
import org.eclipse.jetty.http.HttpFields;
44+
import org.eclipse.jetty.http.HttpHeader;
45+
import org.eclipse.jetty.http.HttpStatus;
4446
import org.eclipse.jetty.http.HttpStatus.Code;
47+
import org.eclipse.jetty.http.MultiPart;
48+
import org.eclipse.jetty.http.MultiPartConfig;
4549
import org.eclipse.jetty.server.Handler;
4650
import org.eclipse.jetty.server.Request;
4751
import org.eclipse.jetty.server.Response;
@@ -309,16 +313,10 @@ private HttpRequestHandler(
309313
@Override
310314
public boolean handle(Request request, Response response, Callback callback) {
311315
try {
312-
if (!HttpMethod.POST.is(request.getMethod())) {
313-
response.setStatus(HttpStatus.METHOD_NOT_ALLOWED_405);
314-
callback.succeeded();
315-
return true;
316-
}
317-
318316
testReference.get().test(new HttpRequestImpl(request));
319317
} catch (Throwable t) {
320-
t.printStackTrace();
321318
exceptionReference.set(t);
319+
Response.writeError(request, response, callback, t);
322320
}
323321
callback.succeeded();
324322
return true;
@@ -397,9 +395,6 @@ private HttpResponseHandler(
397395

398396
@Override
399397
public boolean handle(Request request, Response response, Callback callback) {
400-
if (!HttpMethod.POST.is(request.getMethod())) {
401-
return false;
402-
}
403398
try {
404399
testReference.get().test(new HttpResponseImpl(response));
405400
callback.succeeded();
@@ -462,10 +457,10 @@ private void httpResponseEffects(
462457
response -> response.setStatusCode(HttpStatus.IM_A_TEAPOT_418),
463458
response -> assertThat(response.getStatus()).isEqualTo(HttpStatus.IM_A_TEAPOT_418)),
464459
responseTest(
465-
// reason string cannot be set by application
466460
response -> response.setStatusCode(HttpStatus.IM_A_TEAPOT_418, "Je suis une théière"),
467461
response -> {
468462
assertThat(response.getStatus()).isEqualTo(HttpStatus.IM_A_TEAPOT_418);
463+
// Reason string cannot be set by the application.
469464
assertThat(response.getReason()).isEqualTo(Code.IM_A_TEAPOT.getMessage());
470465
}),
471466
responseTest(

0 commit comments

Comments
 (0)