Skip to content

Commit c19c4e3

Browse files
ludochrenovate-botgae-java-botlachlan-roberts
authored
Fix/issue 462 (#469)
* Hardcode several JavaRuntimeParams values. PiperOrigin-RevId: 860637485 Change-Id: Ibbd87902f4ef73e3620180ad870dc8200a40b417 * Modernize Java syntax to JDK17 level in App Engine code base. PiperOrigin-RevId: 861206703 Change-Id: If6a63e7bae3d33e833cf752ee8313d88b9dffa86 * Update all non-major dependencies * Refactor: Use lambda expressions and Character.valueOf. PiperOrigin-RevId: 862378286 Change-Id: I140e95ffda132265f442bc1d222c587331e367d6 * Update all non-major dependencies * Refactor AppEngineWebXmlInitialParse to allow mocking environment variables and add a test. New Test Permutations We added new test cases to AppEngineWebXmlInitialParseTest.java to ensure full coverage of Runtime, Jetty, and Jakarta EE version combinations: testJava17WithExperimentEnableJetty12: Verifies that when the environment variable EXPERIMENT_ENABLE_JETTY12_FOR_JAVA is set to true for the java17 runtime (simulated via envProvider), the configuration correctly defaults to appengine.use.EE8=true (Jetty 12.0 in EE8 mode) instead of the standard Jetty 9.4 legacy mode. testHttpConnectorExperiment: Verifies that when EXPERIMENT_ENABLE_HTTP_CONNECTOR_FOR_JAVA is set to true, the system properties appengine.use.HttpConnector and appengine.ignore.cancelerror are automatically enabled. testJava17WithJetty121: Verifies that for the java17 runtime, enabling appengine.use.jetty121 alone does not inadvertently enable Jakarta EE 8, 10, or 11. It confirms that the configuration remains valid with only the Jetty 12.1 flag set. testJava17WithEE8AndJetty121: Verifies that for java17, explicitly enabling both appengine.use.EE8 and appengine.use.jetty121 correctly sets both properties to true, supporting the legacy EE8 environment on the newer Jetty infrastructure. testJava21WithEE10Explicit: Confirms that explicitly setting appengine.use.EE10 for java21 (which is the default) works as expected, ensuring EE10 is true and jetty121 remains false (mapping to Jetty 12.0). testJava21WithEE8AndJetty121: Verifies that for java21, combining appengine.use.EE8 with appengine.use.jetty121 correctly enables both flags. Environment Variable Emulation To reliably test logic dependent on environment variables (such as EXPERIMENT_ENABLE_JETTY12_FOR_JAVA or EXPERIMENT_ENABLE_HTTP_CONNECTOR_FOR_JAVA) without modifying the actual system environment or affecting other tests: The AppEngineWebXmlInitialParse class now accepts a custom envProvider (a UnaryOperator<String>) via the package-private setEnvProvider method. By default, this provider delegates to System::getenv. In tests, we inject a lambda function (e.g., key -> "true") to simulate specific environment variables being set. This allows us to verify that the parser correctly activates experimental features or defaults based on the environment. testSystemPropertyOverrideRuntime: This test sets the GAE_RUNTIME system property to "java21" (while the XML specifies "java17") and verifies that the runtime logic correctly switches to "java21" behavior (defaulting to EE10). This kills the mutant where runtimeId re-fetching from system properties (line 178) was removed, as the test would fail if runtimeId remained "java17". testSystemPropertyOverrideRuntimeWithWhitespace: This test sets GAE_RUNTIME to " java21 " (with whitespace) and asserts that the code correctly handles it as "java21" (setting EE10 to true). This kills the mutant where the trim() call (lines 179-181) was removed, as the test would fail if the whitespace prevented the "java21" logic from triggering. testLogEE8: This test captures the logs generated by AppEngineWebXmlInitialParse and asserts that "appengine.use.EE8=true" is logged when EE8 is enabled. This kills the mutant where the logging logic for EE8 (lines 249-251) was removed. PiperOrigin-RevId: 862787419 Change-Id: I1bc08a2ee06faa1018f0f225e8ca90f358cc094d * Update all non-major dependencies * Refactor StringBuilder usage to use append() calls. PiperOrigin-RevId: 863263787 Change-Id: I5b7805c536e69fd544fc71edadd0d799e3e5c145 * Update dependency org.apache.maven.plugins:maven-compiler-plugin to v3.15.0 * Update Jetty versions and servlet API jar. Update Jetty 12.0.x to 12.0.32 and Jetty 12.1.x to 12.1.6. Also, update the Jetty servlet API jar from 4.0.6 to 4.0.9 in the Jetty121EE8Sdk configuration. PiperOrigin-RevId: 864150501 Change-Id: Idc20a1e9abda7b10f9a0e695ad979d3e6cbaa7a2 * temporary fix to AppEngineAnnotationConfiguration for issue #462 Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com> --------- Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com> Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: GAE Java Team <gae-java-bot@google.com> Co-authored-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
1 parent 820ce92 commit c19c4e3

132 files changed

Lines changed: 1740 additions & 3731 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

api/src/main/java/com/google/appengine/api/blobstore/BlobstoreServiceImpl.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,10 @@ public void delete(BlobKey... blobKeys) {
164164
try {
165165
ApiProxy.makeSyncCall(PACKAGE, "DeleteBlob", request.build().toByteArray());
166166
} catch (ApiProxy.ApplicationException ex) {
167-
switch (BlobstoreServiceError.ErrorCode.forNumber(ex.getApplicationError())) {
168-
case INTERNAL_ERROR:
169-
throw new BlobstoreFailureException("An internal blobstore error occurred.");
170-
default:
171-
throw new BlobstoreFailureException("An unexpected error occurred.", ex);
172-
}
167+
throw switch (BlobstoreServiceError.ErrorCode.forNumber(ex.getApplicationError())) {
168+
case INTERNAL_ERROR -> new BlobstoreFailureException("An internal blobstore error occurred.");
169+
default -> new BlobstoreFailureException("An unexpected error occurred.", ex);
170+
};
173171
}
174172
}
175173

api/src/main/java/com/google/appengine/api/datastore/AsyncDatastoreServiceImpl.java

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,11 @@ protected TransactionImpl.InternalTransaction doBeginTransaction(TransactionOpti
253253
}
254254
}
255255
if (options.transactionMode() != null) {
256-
switch (options.transactionMode()) {
257-
case READ_ONLY:
258-
request.setMode(TransactionMode.READ_ONLY);
259-
break;
260-
case READ_WRITE:
261-
request.setMode(TransactionMode.READ_WRITE);
262-
break;
263-
default:
264-
throw new AssertionError("Unrecognized transaction mode: " + options.transactionMode());
265-
}
256+
request.setMode(switch (options.transactionMode()) {
257+
case READ_ONLY -> TransactionMode.READ_ONLY;
258+
case READ_WRITE -> TransactionMode.READ_WRITE;
259+
default -> throw new AssertionError("Unrecognized transaction mode: " + options.transactionMode());
260+
});
266261
}
267262

268263
Future<DatastoreV3Pb.Transaction> future =
@@ -620,21 +615,11 @@ protected Map<Index, IndexState> wrap(CompositeIndices indices) throws Exception
620615
for (CompositeIndex ci : indices.getIndexList()) {
621616
Index index = IndexTranslator.convertFromPb(ci);
622617
switch (ci.getState()) {
623-
case DELETED:
624-
answer.put(index, IndexState.DELETING);
625-
break;
626-
case ERROR:
627-
answer.put(index, IndexState.ERROR);
628-
break;
629-
case READ_WRITE:
630-
answer.put(index, IndexState.SERVING);
631-
break;
632-
case WRITE_ONLY:
633-
answer.put(index, IndexState.BUILDING);
634-
break;
635-
default:
636-
logger.log(Level.WARNING, "Unrecognized index state for " + index);
637-
break;
618+
case DELETED -> answer.put(index, IndexState.DELETING);
619+
case ERROR -> answer.put(index, IndexState.ERROR);
620+
case READ_WRITE -> answer.put(index, IndexState.SERVING);
621+
case WRITE_ONLY -> answer.put(index, IndexState.BUILDING);
622+
default -> logger.log(Level.WARNING, "Unrecognized index state for {0}", index);
638623
}
639624
}
640625
return answer;

api/src/main/java/com/google/appengine/api/datastore/BaseQueryResultsSource.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,7 @@ public BaseQueryResultsSource(
129129
this.offset = fetchOptions.getOffset() != null ? fetchOptions.getOffset() : 0;
130130
this.txn = txn;
131131
this.query = query;
132-
this.currentTransactionProvider =
133-
new CurrentTransactionProvider() {
134-
@Override
135-
public Transaction getCurrentTransaction(Transaction defaultValue) {
136-
return txn;
137-
}
138-
};
132+
this.currentTransactionProvider = defaultValue -> txn;
139133
this.initialQueryResultFuture = initialQueryResultFuture;
140134
this.skippedResults = 0;
141135
}

api/src/main/java/com/google/appengine/api/datastore/Blob.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ public int hashCode() {
6262
/** Two {@code Blob} objects are considered equal if their contained bytes match exactly. */
6363
@Override
6464
public boolean equals(@Nullable Object object) {
65-
if (object instanceof Blob) {
66-
Blob key = (Blob) object;
65+
if (object instanceof Blob key) {
6766
return Arrays.equals(bytes, key.bytes);
6867
}
6968
return false;

api/src/main/java/com/google/appengine/api/datastore/CloudDatastoreV1ClientImpl.java

Lines changed: 29 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
2323
import com.google.api.client.googleapis.compute.ComputeCredential;
2424
import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport;
25-
import com.google.api.client.http.HttpRequest;
26-
import com.google.api.client.http.HttpRequestInitializer;
2725
import com.google.api.client.json.gson.GsonFactory;
2826
import com.google.api.client.util.ExponentialBackOff;
2927
import com.google.auto.value.AutoValue;
@@ -59,6 +57,7 @@
5957
import java.util.concurrent.Executors;
6058
import java.util.concurrent.Future;
6159
import java.util.logging.Level;
60+
import java.util.logging.LogRecord;
6261
import java.util.logging.Logger;
6362
import org.jspecify.annotations.Nullable;
6463

@@ -126,67 +125,31 @@ static synchronized CloudDatastoreV1ClientImpl create(DatastoreServiceConfig con
126125

127126
@Override
128127
public Future<BeginTransactionResponse> beginTransaction(final BeginTransactionRequest req) {
129-
return makeCall(
130-
new Callable<BeginTransactionResponse>() {
131-
@Override
132-
public BeginTransactionResponse call() throws DatastoreException {
133-
return datastore.beginTransaction(req);
134-
}
135-
});
128+
return makeCall(() -> datastore.beginTransaction(req));
136129
}
137130

138131
@Override
139132
public Future<RollbackResponse> rollback(final RollbackRequest req) {
140-
return makeCall(
141-
new Callable<RollbackResponse>() {
142-
@Override
143-
public RollbackResponse call() throws DatastoreException {
144-
return datastore.rollback(req);
145-
}
146-
});
133+
return makeCall(() -> datastore.rollback(req));
147134
}
148135

149136
@Override
150137
public Future<RunQueryResponse> runQuery(final RunQueryRequest req) {
151-
return makeCall(
152-
new Callable<RunQueryResponse>() {
153-
@Override
154-
public RunQueryResponse call() throws DatastoreException {
155-
return datastore.runQuery(req);
156-
}
157-
});
138+
return makeCall(() -> datastore.runQuery(req));
158139
}
159140

160141
@Override
161142
public Future<LookupResponse> lookup(final LookupRequest req) {
162-
return makeCall(
163-
new Callable<LookupResponse>() {
164-
@Override
165-
public LookupResponse call() throws DatastoreException {
166-
return datastore.lookup(req);
167-
}
168-
});
143+
return makeCall(() -> datastore.lookup(req));
169144
}
170145

171146
@Override
172147
public Future<AllocateIdsResponse> allocateIds(final AllocateIdsRequest req) {
173-
return makeCall(
174-
new Callable<AllocateIdsResponse>() {
175-
@Override
176-
public AllocateIdsResponse call() throws DatastoreException {
177-
return datastore.allocateIds(req);
178-
}
179-
});
148+
return makeCall(() -> datastore.allocateIds(req));
180149
}
181150

182151
private Future<CommitResponse> commit(final CommitRequest req) {
183-
return makeCall(
184-
new Callable<CommitResponse>() {
185-
@Override
186-
public CommitResponse call() throws DatastoreException {
187-
return datastore.commit(req);
188-
}
189-
});
152+
return makeCall(() -> datastore.commit(req));
190153
}
191154

192155
@Override
@@ -222,10 +185,11 @@ public T call() throws Exception {
222185
return callable.call();
223186
} catch (Exception e) {
224187
if (isRetryable(e) && remainingTries > 0) {
225-
logger.log(
226-
Level.FINE,
227-
String.format("Caught retryable exception; %d tries remaining", remainingTries),
228-
e);
188+
LogRecord lr =
189+
new LogRecord(Level.FINE, "Caught retryable exception; {0} tries remaining");
190+
lr.setParameters(new Object[] {remainingTries});
191+
lr.setThrown(e);
192+
logger.log(lr);
229193
Thread.sleep(backoff.nextBackOffMillis());
230194
} else {
231195
throw e;
@@ -249,21 +213,17 @@ private <T extends Message> Future<T> makeCall(final Callable<T> oneAttempt) {
249213
? new Exception()
250214
: null;
251215
return executor.submit(
252-
new Callable<T>() {
253-
@Override
254-
public T call() throws Exception {
255-
try {
256-
return new RetryingCallable<>(oneAttempt, maxRetries).call();
257-
} catch (DatastoreException e) {
258-
String message =
259-
stackTraceCapturer != null
260-
? String.format(
261-
"%s%nstack trace when async call was initiated: <%n%s>",
262-
e.getMessage(), Throwables.getStackTraceAsString(stackTraceCapturer))
263-
: String.format(
264-
"%s%n(stack trace capture for async call is disabled)", e.getMessage());
265-
throw DatastoreApiHelper.createV1Exception(e.getCode(), message, e);
266-
}
216+
() -> {
217+
try {
218+
return new RetryingCallable<>(oneAttempt, maxRetries).call();
219+
} catch (DatastoreException e) {
220+
String message =
221+
stackTraceCapturer != null
222+
? e.getMessage()
223+
+ "\nstack trace when async call was initiated: <\n"
224+
+ Throwables.getStackTraceAsString(stackTraceCapturer)
225+
: e.getMessage() + "\n(stack trace capture for async call is disabled)";
226+
throw DatastoreApiHelper.createV1Exception(e.getCode(), message, e);
267227
}
268228
});
269229
}
@@ -275,13 +235,10 @@ private static DatastoreOptions createDatastoreOptions(
275235
setProjectEndpoint(projectId, options);
276236
options.credential(getCredential());
277237
options.initializer(
278-
new HttpRequestInitializer() {
279-
@Override
280-
public void initialize(HttpRequest request) throws IOException {
281-
request.setConnectTimeout(httpConnectTimeoutMillis);
282-
if (config.getDeadline() != null) {
283-
request.setReadTimeout((int) (config.getDeadline() * 1000));
284-
}
238+
request -> {
239+
request.setConnectTimeout(httpConnectTimeoutMillis);
240+
if (config.getDeadline() != null) {
241+
request.setReadTimeout((int) (config.getDeadline() * 1000));
285242
}
286243
});
287244
return options.build();
@@ -298,8 +255,7 @@ private static Credential getCredential() throws GeneralSecurityException, IOExc
298255
if (privateKeyFile != null) {
299256
logger.log(
300257
Level.INFO,
301-
"Service account and private key file were provided. "
302-
+ "Using service account credential.");
258+
"Service account and private key file were provided. Using service account credential.");
303259
return getServiceAccountCredentialBuilder(serviceAccount)
304260
.setServiceAccountPrivateKeyFromP12File(new File(privateKeyFile))
305261
.build();
@@ -308,8 +264,7 @@ private static Credential getCredential() throws GeneralSecurityException, IOExc
308264
if (privateKey != null) {
309265
logger.log(
310266
Level.INFO,
311-
"Service account and private key were provided. "
312-
+ "Using service account credential.");
267+
"Service account and private key were provided. Using service account credential.");
313268
return getServiceAccountCredentialBuilder(serviceAccount)
314269
.setServiceAccountPrivateKey(privateKey)
315270
.build();

api/src/main/java/com/google/appengine/api/datastore/CompositeIndexManager.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,7 @@ protected String generateXmlForIndex(Index index, IndexSource source) {
145145

146146
/** We compare {@link Property Properties} by comparing their names. */
147147
private static final Comparator<Property> PROPERTY_NAME_COMPARATOR =
148-
new Comparator<Property>() {
149-
@Override
150-
public int compare(Property o1, Property o2) {
151-
return o1.getName().compareTo(o2.getName());
152-
}
153-
};
148+
(o1, o2) -> o1.getName().compareTo(o2.getName());
154149

155150
private List<Property> getRecommendedIndexProps(IndexComponentsOnlyQuery query) {
156151
// Construct the list of index properties

api/src/main/java/com/google/appengine/api/datastore/DataTypeTranslator.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,6 @@ public RawValue getValue(Value propertyValue) {
11131113
}
11141114

11151115
@Override
1116-
@ SuppressWarnings("PatternMatchingInstanceof")
11171116
public @Nullable Comparable<?> asComparable(Object value) {
11181117
Object value2 = ((RawValue) value).getValue();
11191118
// All possible values except byte[] are already comparable.
@@ -2027,10 +2026,7 @@ public int compareTo(ComparableByteArray other) {
20272026

20282027
@Override
20292028
public boolean equals(@Nullable Object obj) {
2030-
if (obj == null) {
2031-
return false;
2032-
}
2033-
return Arrays.equals(bytes, ((ComparableByteArray) obj).bytes);
2029+
return obj instanceof ComparableByteArray other && Arrays.equals(bytes, other.bytes);
20342030
}
20352031

20362032
@Override

api/src/main/java/com/google/appengine/api/datastore/DatastoreApiHelper.java

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -63,59 +63,41 @@ public static RuntimeException translateError(ApiProxy.ApplicationException exce
6363
if (errorCode == null) {
6464
return new DatastoreFailureException(exception.getErrorDetail());
6565
}
66-
switch (errorCode) {
67-
case BAD_REQUEST:
68-
return new IllegalArgumentException(exception.getErrorDetail());
69-
70-
case CONCURRENT_TRANSACTION:
71-
return new ConcurrentModificationException(exception.getErrorDetail());
72-
73-
case NEED_INDEX:
74-
return new DatastoreNeedIndexException(exception.getErrorDetail());
75-
76-
case TIMEOUT:
77-
case BIGTABLE_ERROR:
78-
return new DatastoreTimeoutException(exception.getErrorDetail());
79-
80-
case COMMITTED_BUT_STILL_APPLYING:
81-
return new CommittedButStillApplyingException(exception.getErrorDetail());
82-
83-
case RESOURCE_EXHAUSTED:
84-
return new ApiProxy.OverQuotaException(exception.getErrorDetail(), (Throwable) null);
85-
86-
case INTERNAL_ERROR:
87-
default:
88-
return new DatastoreFailureException(exception.getErrorDetail());
89-
}
66+
return switch (errorCode) {
67+
case BAD_REQUEST -> new IllegalArgumentException(exception.getErrorDetail());
68+
case CONCURRENT_TRANSACTION -> new ConcurrentModificationException(exception.getErrorDetail());
69+
case NEED_INDEX -> new DatastoreNeedIndexException(exception.getErrorDetail());
70+
case TIMEOUT, BIGTABLE_ERROR -> new DatastoreTimeoutException(exception.getErrorDetail());
71+
case COMMITTED_BUT_STILL_APPLYING -> new CommittedButStillApplyingException(
72+
exception.getErrorDetail());
73+
case RESOURCE_EXHAUSTED -> new ApiProxy.OverQuotaException(
74+
exception.getErrorDetail(), (Throwable) null);
75+
default -> new DatastoreFailureException(exception.getErrorDetail());
76+
};
9077
}
9178

9279
static RuntimeException createV1Exception(Code code, String message, Throwable cause) {
9380
if (code == null) {
9481
return new DatastoreFailureException(message, cause);
9582
}
96-
switch (code) {
97-
case ABORTED:
98-
return new ConcurrentModificationException(message, cause);
99-
case FAILED_PRECONDITION:
83+
return switch (code) {
84+
case ABORTED -> new ConcurrentModificationException(message, cause);
85+
case FAILED_PRECONDITION -> {
10086
if (message.contains("The Cloud Datastore API is not enabled for the project")) {
101-
return new DatastoreFailureException(message, cause);
87+
yield new DatastoreFailureException(message, cause);
10288
}
10389
// Could also indicate ErrorCode.SAFE_TIME_TOO_OLD.
104-
return new DatastoreNeedIndexException(message, cause);
105-
case DEADLINE_EXCEEDED:
106-
return new DatastoreTimeoutException(message, cause);
107-
case INVALID_ARGUMENT:
108-
case PERMISSION_DENIED:
109-
return new IllegalArgumentException(message, cause);
110-
case UNAVAILABLE:
111-
return new ApiProxy.RPCFailedException(message, cause);
112-
case RESOURCE_EXHAUSTED:
113-
return new ApiProxy.OverQuotaException(message, cause);
114-
case INTERNAL:
115-
// Could also indicate ErrorCode.COMMITTED_BUT_STILL_APPLYING.
116-
default:
117-
return new DatastoreFailureException(message, cause);
118-
}
90+
yield new DatastoreNeedIndexException(message, cause);
91+
}
92+
case DEADLINE_EXCEEDED -> new DatastoreTimeoutException(message, cause);
93+
case INVALID_ARGUMENT, PERMISSION_DENIED -> new IllegalArgumentException(message, cause);
94+
case UNAVAILABLE -> new ApiProxy.RPCFailedException(message, cause);
95+
case RESOURCE_EXHAUSTED -> new ApiProxy.OverQuotaException(message, cause);
96+
case INTERNAL ->
97+
// Could also indicate ErrorCode.COMMITTED_BUT_STILL_APPLYING.
98+
new DatastoreFailureException(message, cause);
99+
default -> new DatastoreFailureException(message, cause);
100+
};
119101
}
120102

121103
static <T extends Message, S extends Message.Builder> Future<T> makeAsyncCall(
@@ -162,8 +144,8 @@ protected T wrap(byte[] responseBytes) throws InvalidProtocolBufferException {
162144

163145
@Override
164146
protected Throwable convertException(Throwable cause) {
165-
if (cause instanceof ApiProxy.ApplicationException) {
166-
return translateError((ApiProxy.ApplicationException) cause);
147+
if (cause instanceof ApiProxy.ApplicationException applicationException) {
148+
return translateError(applicationException);
167149
}
168150
return cause;
169151
}

0 commit comments

Comments
 (0)