Skip to content

Commit 51299af

Browse files
committed
fix: address batchCheck review feedback
- clientBatchCheck: use handle instead of handleAsync so the trivial response construction runs inline, avoiding thread-per-task churn on <=2 vCPU hosts where the common pool falls back to per-task threads and reintroduces the churn this change removes - batchCheck: route response-processing errors through the failure reference so a malformed 200 (e.g. null result) no longer aborts the remaining batches in a lane; all batches are attempted, then the returned future fails, matching the request-failure path - tests: add regression coverage that response-processing failures fail the returned future and do not skip remaining batches
1 parent f989a91 commit 51299af

2 files changed

Lines changed: 58 additions & 1 deletion

File tree

src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,7 @@ public CompletableFuture<List<ClientBatchCheckClientResponse>> clientBatchCheck(
940940

941941
Function<ClientCheckRequest, CompletableFuture<Void>> singleClientCheckRequest =
942942
request -> call(() -> this.check(request, clientCheckOptions))
943-
.handleAsync(ClientBatchCheckClientResponse.asyncHandler(request))
943+
.handle(ClientBatchCheckClientResponse.asyncHandler(request))
944944
.thenAccept(responses::add);
945945

946946
return executeInLanes(requests, singleClientCheckRequest, maxParallelRequests)
@@ -1068,6 +1068,10 @@ public CompletableFuture<ClientBatchCheckResponse> batchCheck(
10681068
});
10691069
responses.addAll(batchResults);
10701070
return null;
1071+
})
1072+
.exceptionally(processingFailure -> {
1073+
failure.compareAndSet(null, processingFailure);
1074+
return null;
10711075
});
10721076

10731077
return executeInLanes(batchedChecks, singleBatchCheckRequest, maxParallelRequests)

src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,6 +2095,59 @@ public void batchCheckMethodsRejectNonPositiveMaxParallelRequests() {
20952095
}
20962096
}
20972097

2098+
@Test
2099+
public void batchCheckFailsWhenResponseProcessingFails() throws Exception {
2100+
// Given: a successful HTTP response whose body is missing the result payload
2101+
String batchCheckUrl = String.format("%s/stores/%s/batch-check", FgaConstants.TEST_API_URL, DEFAULT_STORE_ID);
2102+
mockHttpClient.onPost(batchCheckUrl).doReturn(200, "{\"result\": null}");
2103+
2104+
ClientBatchCheckItem batchItem = new ClientBatchCheckItem()
2105+
._object(DEFAULT_OBJECT)
2106+
.relation(DEFAULT_RELATION)
2107+
.user(DEFAULT_USER)
2108+
.correlationId("cor-1");
2109+
2110+
// When
2111+
var future = fga.batchCheck(new ClientBatchCheckRequest().checks(List.of(batchItem)));
2112+
2113+
// Then: a response-processing error must fail the returned future rather than be silently dropped
2114+
var exception = assertThrows(ExecutionException.class, () -> future.get(5, TimeUnit.SECONDS));
2115+
assertNotNull(exception.getCause());
2116+
}
2117+
2118+
@Test
2119+
public void batchCheckAttemptsRemainingBatchesWhenResponseProcessingFails() throws Exception {
2120+
// Given: 60 checks form two sub-batches with the default maxBatchSize of 50, sent one lane at a time
2121+
var pending = new CopyOnWriteArrayList<CompletableFuture<HttpResponse<String>>>();
2122+
var fga = clientBackedByPendingResponses(pending);
2123+
List<ClientBatchCheckItem> checks = IntStream.range(0, 60)
2124+
.mapToObj(i -> new ClientBatchCheckItem()
2125+
._object(DEFAULT_OBJECT)
2126+
.relation(DEFAULT_RELATION)
2127+
.user(DEFAULT_USER)
2128+
.correlationId("cor-" + i))
2129+
.collect(Collectors.toList());
2130+
var options = new ClientBatchCheckOptions().maxParallelRequests(1);
2131+
2132+
var future = assertTimeoutPreemptively(
2133+
Duration.ofSeconds(5), () -> fga.batchCheck(new ClientBatchCheckRequest().checks(checks), options));
2134+
2135+
assertFalse(future.isDone());
2136+
Thread.sleep(200);
2137+
assertEquals(1, pending.size());
2138+
2139+
// The first sub-batch returns a malformed 200 (null result) that fails during response processing
2140+
pending.get(0).complete(fakeResponse("{\"result\": null}"));
2141+
2142+
// The lane must still send the remaining sub-batch instead of aborting early
2143+
awaitSize(pending, 2);
2144+
pending.get(1).complete(fakeResponse("{\"result\": {}}"));
2145+
2146+
// Then: every batch was attempted, and the returned future fails with the captured processing error
2147+
var exception = assertThrows(ExecutionException.class, () -> future.get(5, TimeUnit.SECONDS));
2148+
assertNotNull(exception.getCause());
2149+
}
2150+
20982151
private OpenFgaClient clientBackedByPendingResponses(List<CompletableFuture<HttpResponse<String>>> pending)
20992152
throws Exception {
21002153
HttpClient pendingClient = mock(HttpClient.class);

0 commit comments

Comments
 (0)