Skip to content

Commit 561f78e

Browse files
gae-java-botgae-java-bot
committed
Some refactoring suggestions, and usage of Jetty 12.1.1 code internally.
PiperOrigin-RevId: 818694090 Change-Id: I272e7e4c05476ea934fce7ff1ea4b0ddcddadeb8
1 parent cf2bdf5 commit 561f78e

17 files changed

Lines changed: 1297 additions & 83 deletions

File tree

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/http/JettyHttpApiHostClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public void onContent(Response response, ByteBuffer byteBuffer) {
165165
}
166166

167167
private void abortBecauseTooLarge(Response response) {
168-
response.abort(new ApiProxy.ResponseTooLargeException(null, null));
168+
var unused = response.abort(new ApiProxy.ResponseTooLargeException(null, null));
169169
// This exception will be replaced with a proper one in onComplete().
170170
}
171171

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/AppVersionHandlerFactory.java‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@
1515
*/
1616
package com.google.apphosting.runtime.jetty;
1717

18+
import java.lang.reflect.InvocationTargetException;
1819
import com.google.apphosting.runtime.AppVersion;
19-
import com.google.apphosting.runtime.jetty.ee11.EE11AppVersionHandlerFactory;
20-
import com.google.apphosting.runtime.jetty.ee8.EE8AppVersionHandlerFactory;
2120
import org.eclipse.jetty.server.Handler;
2221
import org.eclipse.jetty.server.Server;
2322

@@ -39,16 +38,34 @@ static EEVersion getEEVersion() {
3938
}
4039
}
4140

42-
static AppVersionHandlerFactory newInstance(Server server, String serverInfo) {
43-
switch (getEEVersion()) {
44-
case EE11:
45-
return new EE11AppVersionHandlerFactory(server, serverInfo);
46-
case EE8:
47-
return new EE8AppVersionHandlerFactory(server, serverInfo);
48-
default:
49-
throw new IllegalStateException("Unknown EE version: " + getEEVersion());
41+
static AppVersionHandlerFactory newInstance(Server server, String serverInfo) {
42+
String appVersionHandlerFactoryNameClassName = getAppVersionHandlerFactoryNameClassName();
43+
try {
44+
return Class.forName(appVersionHandlerFactoryNameClassName)
45+
.asSubclass(AppVersionHandlerFactory.class)
46+
.getConstructor(Server.class, String.class)
47+
.newInstance(server, serverInfo);
48+
} catch (ClassNotFoundException
49+
| IllegalAccessException
50+
| IllegalArgumentException
51+
| InstantiationException
52+
| NoSuchMethodException
53+
| SecurityException
54+
| InvocationTargetException ex) {
55+
throw new IllegalStateException(
56+
"Unable to create instance of " + appVersionHandlerFactoryNameClassName, ex);
57+
} catch (ClassCastException cce) {
58+
throw new IllegalStateException("Not a subtype of " + AppVersionHandlerFactory.class.getName(), cce);
5059
}
5160
}
5261

62+
static String getAppVersionHandlerFactoryNameClassName() {
63+
return switch (getEEVersion()) {
64+
case EE11 -> "com.google.apphosting.runtime.jetty.ee11.EE11AppVersionHandlerFactory";
65+
case EE8 -> "com.google.apphosting.runtime.jetty.ee8.EE8AppVersionHandlerFactory";
66+
default -> throw new IllegalStateException("Unknown EE version: " + getEEVersion());
67+
};
68+
}
69+
5370
Handler createHandler(AppVersion appVersion) throws Exception;
5471
}

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/JettyServletEngineAdapter.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public void run(Runnable runnable) {
176176
EvaluationRuntimeServerInterface evaluationRuntimeServerInterface =
177177
Objects.requireNonNull(runtimeOptions.evaluationRuntimeServerInterface());
178178
evaluationRuntimeServerInterface.addAppVersion(context, appinfo);
179-
context.getResponse();
179+
var unused = context.getResponse();
180180
appVersionKey = AppVersionKey.fromAppInfo(appinfo);
181181
} catch (Exception e) {
182182
throw new IllegalStateException(e);
@@ -262,8 +262,11 @@ public void serviceRequest(UPRequest upRequest, MutableUpResponse upResponse) th
262262
}
263263

264264
DelegateRpcExchange rpcExchange = new DelegateRpcExchange(upRequest, upResponse);
265-
rpcExchange.setAttribute(AppEngineConstants.APP_VERSION_KEY_REQUEST_ATTR, appVersionKey);
266-
rpcExchange.setAttribute(AppEngineConstants.ENVIRONMENT_ATTR, ApiProxy.getCurrentEnvironment());
265+
var unusedApiVersionKey =
266+
rpcExchange.setAttribute(AppEngineConstants.APP_VERSION_KEY_REQUEST_ATTR, appVersionKey);
267+
var unusedEnvironment =
268+
rpcExchange.setAttribute(
269+
AppEngineConstants.ENVIRONMENT_ATTR, ApiProxy.getCurrentEnvironment());
267270
rpcConnector.service(rpcExchange);
268271
try {
269272
rpcExchange.awaitResponse();

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/delegate/impl/DelegateRpcExchange.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public DelegateRpcExchange(RuntimePb.UPRequest request, MutableUpResponse respon
6767
final boolean skipAdmin = hasSkipAdminCheck(request);
6868
// Translate the X-Google-Internal-SkipAdminCheck to a servlet attribute.
6969
if (skipAdmin) {
70-
setAttribute(SKIP_ADMIN_CHECK_ATTR, true);
70+
var unused = setAttribute(SKIP_ADMIN_CHECK_ATTR, true);
7171

7272
// N.B.: If SkipAdminCheck is set, we're actually lying
7373
// to Jetty here to tell it that HTTPS is in use when it may not

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ public boolean removeEventListener(EventListener listener) {
246246
@Override
247247
public void doStart() throws Exception {
248248
super.doStart();
249-
addEventListener(new TransactionCleanupListener(getClassLoader()));
249+
var unused = addEventListener(new TransactionCleanupListener(getClassLoader()));
250250
}
251251

252252
@Override

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/EE8AppVersionHandlerFactory.java‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.eclipse.jetty.ee8.annotations.AnnotationConfiguration;
3838
import org.eclipse.jetty.ee8.nested.ContextHandler;
3939
import org.eclipse.jetty.ee8.nested.Dispatcher;
40+
import org.eclipse.jetty.ee8.nested.Request;
4041
import org.eclipse.jetty.ee8.quickstart.QuickStartConfiguration;
4142
import org.eclipse.jetty.ee8.servlet.ErrorPageErrorHandler;
4243
import org.eclipse.jetty.ee8.webapp.FragmentConfiguration;
@@ -197,34 +198,32 @@ private org.eclipse.jetty.server.Handler doCreateHandler(AppVersion appVersion)
197198
.setAsyncPersistence(sessionsConfig.isAsyncPersistence())
198199
.setServletContextHandler(context);
199200

200-
SessionManagerHandler.create(builder.build());
201+
var unused = SessionManagerHandler.create(builder.build());
201202
// Pass the AppVersion on to any of our servlets (e.g. ResourceFileServlet).
202203
context.setAttribute(AppEngineConstants.APP_VERSION_CONTEXT_ATTR, appVersion);
203204

204205
if (Boolean.getBoolean(HTTP_CONNECTOR_MODE)) {
205-
context.addEventListener(
206-
new ContextHandler.ContextScopeListener() {
207-
@Override
208-
public void enterScope(
209-
ContextHandler.APIContext context,
210-
org.eclipse.jetty.ee8.nested.Request request,
211-
Object reason) {
212-
if (request != null) {
213-
ApiProxy.Environment environment =
214-
(ApiProxy.Environment)
215-
request.getAttribute(AppEngineConstants.ENVIRONMENT_ATTR);
216-
if (environment != null) {
217-
ApiProxy.setEnvironmentForCurrentThread(environment);
206+
var unusedEventListener =
207+
context.addEventListener(
208+
new ContextHandler.ContextScopeListener() {
209+
@Override
210+
public void enterScope(
211+
ContextHandler.APIContext context, Request request, Object reason) {
212+
if (request != null) {
213+
ApiProxy.Environment environment =
214+
(ApiProxy.Environment)
215+
request.getAttribute(AppEngineConstants.ENVIRONMENT_ATTR);
216+
if (environment != null) {
217+
ApiProxy.setEnvironmentForCurrentThread(environment);
218+
}
219+
}
218220
}
219-
}
220-
}
221-
222-
@Override
223-
public void exitScope(
224-
ContextHandler.APIContext context, org.eclipse.jetty.ee8.nested.Request request) {
225-
ApiProxy.clearEnvironmentForCurrentThread();
226-
}
227-
});
221+
222+
@Override
223+
public void exitScope(ContextHandler.APIContext context, Request request) {
224+
ApiProxy.clearEnvironmentForCurrentThread();
225+
}
226+
});
228227
}
229228

230229
return context.get();
@@ -244,7 +243,7 @@ private static class NullErrorHandler extends ErrorPageErrorHandler {
244243
@Override
245244
public void handle(
246245
String target,
247-
org.eclipse.jetty.ee8.nested.Request baseRequest,
246+
Request baseRequest,
248247
HttpServletRequest request,
249248
HttpServletResponse response)
250249
throws IOException, ServletException {

‎runtime/runtime_impl_jetty121/src/main/java/com/google/apphosting/runtime/jetty/ee8/FileSender.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.apphosting.utils.config.AppYaml;
2121
import com.google.common.base.Strings;
2222
import java.io.IOException;
23+
import java.io.InputStream;
2324
import java.io.OutputStream;
2425
import java.util.Map;
2526
import java.util.Optional;
@@ -36,9 +37,27 @@
3637
public class FileSender {
3738

3839
private final AppYaml appYaml;
40+
private final OutputWriter outputWriter;
41+
42+
public interface OutputWriter {
43+
void writeTo(InputStream in, OutputStream out, long contentLength) throws IOException;
44+
}
45+
46+
public static class IoOutputWriter implements OutputWriter {
47+
@Override
48+
public void writeTo(InputStream in, OutputStream out, long contentLength) throws IOException {
49+
IO.copy(in, out, contentLength);
50+
}
51+
}
3952

4053
public FileSender(AppYaml appYaml) {
4154
this.appYaml = appYaml;
55+
this.outputWriter = new IoOutputWriter();
56+
}
57+
58+
public FileSender(AppYaml appYaml, OutputWriter outputWriter) {
59+
this.appYaml = appYaml;
60+
this.outputWriter = outputWriter;
4261
}
4362

4463
/** Writes or includes the specified resource. */
@@ -61,7 +80,11 @@ public void sendData(
6180
} catch (IllegalStateException e) {
6281
out = new WriterOutputStream(response.getWriter());
6382
}
64-
IO.copy(resource.newInputStream(), out, contentLength);
83+
this.outputWriter.writeTo(resource.newInputStream(), out, contentLength);
84+
}
85+
86+
public void writeTo(InputStream in, OutputStream out, long contentLength) throws IOException {
87+
IO.copy(in, out, contentLength);
6588
}
6689

6790
/** Writes the headers that should accompany the specified resource. */
@@ -85,8 +108,7 @@ private void writeHeaders(
85108
}
86109
}
87110

88-
response.setDateHeader(
89-
HttpHeader.LAST_MODIFIED.asString(), resource.lastModified().toEpochMilli());
111+
response.setDateHeader(HttpHeader.LAST_MODIFIED.asString(), resource.lastModified().toEpochMilli());
90112
if (appYaml != null) {
91113
// Add user specific static headers
92114
Optional<AppYaml.Handler> maybeHandler =

‎runtime/runtime_impl_jetty121/src/test/java/com/google/apphosting/runtime/grpc/FakeApiProxyImplFactory.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Copyright 2021 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.apphosting.runtime.grpc;
18+
19+
import com.google.apphosting.base.protos.AppinfoPb.AppInfo;
20+
import com.google.apphosting.base.protos.RuntimePb;
21+
import com.google.apphosting.runtime.ApiDeadlineOracle;
22+
import com.google.apphosting.runtime.ApiProxyImpl;
23+
import com.google.apphosting.runtime.AppVersion;
24+
import com.google.apphosting.runtime.MutableUpResponse;
25+
import com.google.apphosting.runtime.TraceWriter;
26+
import com.google.apphosting.runtime.anyrpc.APIHostClientInterface;
27+
import com.google.apphosting.runtime.timer.CpuRatioTimer;
28+
import com.google.apphosting.runtime.timer.TimerFactory;
29+
import java.time.Duration;
30+
import java.util.ArrayList;
31+
import java.util.List;
32+
import java.util.concurrent.Future;
33+
import java.util.concurrent.Semaphore;
34+
35+
/**
36+
* Support for creating ApiProxyImpl instances.
37+
*
38+
*/
39+
public class FakeApiProxyImplFactory {
40+
public static ApiProxyImpl newApiProxyImpl(APIHostClientInterface apiHostClient) {
41+
return ApiProxyImpl.builder()
42+
.setApiHost(apiHostClient)
43+
.setDeadlineOracle(
44+
new ApiDeadlineOracle.Builder()
45+
.initDeadlineMap(30, "", 30, "")
46+
.initOfflineDeadlineMap(30, "", 30, "")
47+
.build())
48+
.setByteCountBeforeFlushing(8192)
49+
.setMaxLogFlushTime(Duration.ofSeconds(5))
50+
.build();
51+
}
52+
53+
private static AppVersion fakeAppVersion() {
54+
return AppVersion.builder()
55+
.setAppInfo(AppInfo.getDefaultInstance())
56+
.setPublicRoot("")
57+
.build();
58+
}
59+
60+
private static CpuRatioTimer fakeCpuRatioTimer() {
61+
ThreadGroup myThreadGroup = Thread.currentThread().getThreadGroup();
62+
long fakeCyclesPerSecond = 1_000_000_000L;
63+
return new TimerFactory(fakeCyclesPerSecond).getCpuRatioTimer(myThreadGroup);
64+
}
65+
66+
public static ApiProxyImpl.EnvironmentImpl fakeEnvironment(
67+
ApiProxyImpl apiProxyImpl, String securityTicket) {
68+
return fakeEnvironment(apiProxyImpl, securityTicket, null);
69+
}
70+
71+
public static ApiProxyImpl.EnvironmentImpl fakeEnvironment(
72+
ApiProxyImpl apiProxyImpl, String securityTicket, TraceWriter traceWriter) {
73+
Semaphore outstandingApiRpcSemaphore = new Semaphore(1);
74+
List<Future<?>> asyncFutures = new ArrayList<>();
75+
RuntimePb.UPRequest request =
76+
RuntimePb.UPRequest.newBuilder().setSecurityTicket(securityTicket).buildPartial();
77+
return apiProxyImpl.createEnvironment(
78+
fakeAppVersion(),
79+
request,
80+
new MutableUpResponse(),
81+
traceWriter,
82+
fakeCpuRatioTimer(),
83+
null,
84+
asyncFutures,
85+
outstandingApiRpcSemaphore,
86+
null,
87+
null,
88+
Long.MAX_VALUE);
89+
}
90+
}

0 commit comments

Comments
 (0)