Skip to content

Commit

Permalink
test(client): add tests on non-transactional write failure path
Browse files Browse the repository at this point in the history
  • Loading branch information
booniepepper committed Nov 20, 2023
1 parent e33b91d commit 4a350af
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 23 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/semgrep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ jobs:
image: returntocorp/semgrep
if: (github.actor != 'dependabot[bot]' && github.actor != 'snyk-bot')
steps:
- uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.2
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
fetch-depth: 0
- run: semgrep ci
env:
SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }}
4 changes: 4 additions & 0 deletions .openapi-generator/FILES
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ src/main/java/dev/openfga/sdk/api/auth/OAuth2Client.java
src/main/java/dev/openfga/sdk/api/client/ApiClient.java
src/main/java/dev/openfga/sdk/api/client/ApiResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientAssertion.java
src/main/java/dev/openfga/sdk/api/client/ClientBatchCheckResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientCheckRequest.java
src/main/java/dev/openfga/sdk/api/client/ClientCheckResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientCreateStoreResponse.java
Expand All @@ -93,6 +94,7 @@ src/main/java/dev/openfga/sdk/api/client/ClientGetStoreResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientListObjectsRequest.java
src/main/java/dev/openfga/sdk/api/client/ClientListObjectsResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientListRelationsRequest.java
src/main/java/dev/openfga/sdk/api/client/ClientListRelationsResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientListStoresResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientReadAssertionsResponse.java
src/main/java/dev/openfga/sdk/api/client/ClientReadAuthorizationModelResponse.java
Expand All @@ -109,11 +111,13 @@ src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java
src/main/java/dev/openfga/sdk/api/configuration/ApiToken.java
src/main/java/dev/openfga/sdk/api/configuration/BaseConfiguration.java
src/main/java/dev/openfga/sdk/api/configuration/ClientBatchCheckOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientCheckOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientConfiguration.java
src/main/java/dev/openfga/sdk/api/configuration/ClientCredentials.java
src/main/java/dev/openfga/sdk/api/configuration/ClientExpandOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientListObjectsOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientListRelationsOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientListStoresOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientReadAssertionsOptions.java
src/main/java/dev/openfga/sdk/api/configuration/ClientReadAuthorizationModelOptions.java
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
/*
* OpenFGA
* A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar.
*
* The version of the OpenAPI document: 0.1
* Contact: [email protected]
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/

package dev.openfga.sdk.api.client;

import java.util.List;
Expand Down
33 changes: 19 additions & 14 deletions src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,16 @@ public CompletableFuture<ClientWriteResponse> write(ClientWriteRequest request,
configuration.assertValid();
String storeId = configuration.getStoreIdChecked();

if (options != null && options.enableTransactions()) {
if (options != null && !options.disableTransactions()) {
return writeTransactions(storeId, request, options);
} else {
return writeOneTransaction(storeId, request, options);
}

return writeNonTransaction(storeId, request, options);
}

private CompletableFuture<ClientWriteResponse> writeOneTransaction(
private CompletableFuture<ClientWriteResponse> writeNonTransaction(
String storeId, ClientWriteRequest request, ClientWriteOptions options) {

WriteRequest body = new WriteRequest();

ClientTupleKey.asTupleKeys(request.getWrites()).ifPresent(body::writes);
Expand All @@ -306,37 +307,41 @@ private CompletableFuture<ClientWriteResponse> writeTransactions(

int chunkSize = options.getTransactionChunkSize();

var writeTransactions =
chunksOf(chunkSize, request.getWrites()).stream().map(ClientWriteRequest::ofWrites);
var deleteTransactions =
chunksOf(chunkSize, request.getDeletes()).stream().map(ClientWriteRequest::ofDeletes);
var writeTransactions = chunksOf(chunkSize, request.getWrites()).map(ClientWriteRequest::ofWrites);
var deleteTransactions = chunksOf(chunkSize, request.getDeletes()).map(ClientWriteRequest::ofDeletes);

var transactions = Stream.concat(writeTransactions, deleteTransactions).collect(Collectors.toList());
var futureResponse = this.writeOneTransaction(storeId, transactions.get(0), options);
var futureResponse = this.writeNonTransaction(storeId, transactions.get(0), options);

for (int i = 1; i < transactions.size(); i++) {
final int index = i; // Must be final in this scope for closure.

// The resulting completable future of this chain will result in either:
// 1. The first exception thrown in a failed completion. Other thenApply() will not be evaluated.
// 1. The first exception thrown in a failed completion. Other thenCompose() will not be evaluated.
// 2. The final successful ClientWriteResponse.
futureResponse.thenApply(_response -> this.writeOneTransaction(storeId, transactions.get(index), options));
futureResponse = futureResponse.thenCompose(
_response -> this.writeNonTransaction(storeId, transactions.get(index), options));
}

return futureResponse;
}

private <T> List<List<T>> chunksOf(int chunkSize, List<T> list) {
private <T> Stream<List<T>> chunksOf(int chunkSize, List<T> list) {
if (list == null || list.isEmpty()) {
return Stream.empty();
}

int nChunks = (int) Math.ceil(list.size() / (double) chunkSize);

int finalEndExclusive = list.size();
List<List<T>> chunks = new ArrayList<>();
Stream.Builder<List<T>> chunks = Stream.builder();

for (int i = 0; i < nChunks; i++) {
List<T> chunk = list.subList(i * chunkSize, Math.min((i + 1) * chunkSize, finalEndExclusive));
chunks.add(chunk);
}
return chunks;

return chunks.build();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

public class ClientWriteOptions {
private String authorizationModelId;
private Boolean enableTransactions;
private Boolean disableTransactions = false;
private int transactionChunkSize;

public ClientWriteOptions authorizationModelId(String authorizationModelId) {
Expand All @@ -26,13 +26,13 @@ public String getAuthorizationModelId() {
return authorizationModelId;
}

public ClientWriteOptions enableTransactions(boolean enableTransactions) {
this.enableTransactions = enableTransactions;
public ClientWriteOptions disableTransactions(boolean disableTransactions) {
this.disableTransactions = disableTransactions;
return this;
}

public boolean enableTransactions() {
return enableTransactions != null && enableTransactions;
public boolean disableTransactions() {
return disableTransactions != null && disableTransactions;
}

public ClientWriteOptions transactionChunkSize(int transactionChunkSize) {
Expand Down
62 changes: 59 additions & 3 deletions src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@
import java.time.Duration;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -53,8 +55,9 @@ public class OpenFgaClientTest {
@BeforeEach
public void beforeEachTest() throws Exception {
mockHttpClient = new HttpClientMock();
// mockHttpClient.debugOn(); // Uncomment when debugging HTTP requests.

HttpClient.Builder mockHttpClientBuilder = mock(HttpClient.Builder.class);
var mockHttpClientBuilder = mock(HttpClient.Builder.class);
when(mockHttpClientBuilder.executor(any())).thenReturn(mockHttpClientBuilder);
when(mockHttpClientBuilder.build()).thenReturn(mockHttpClient);

Expand All @@ -67,7 +70,7 @@ public void beforeEachTest() throws Exception {
.maxRetries(DEFAULT_MAX_RETRIES)
.minimumRetryDelay(DEFAULT_RETRY_DELAY);

ApiClient mockApiClient = mock(ApiClient.class);
var mockApiClient = mock(ApiClient.class);
when(mockApiClient.getHttpClient()).thenReturn(mockHttpClient);
when(mockApiClient.getObjectMapper()).thenReturn(new ObjectMapper());
when(mockApiClient.getHttpClientBuilder()).thenReturn(mockHttpClientBuilder);
Expand Down Expand Up @@ -1118,7 +1121,7 @@ public void writeTest_transactions() throws Exception {
.writes(List.of(tuple, tuple, tuple, tuple, tuple))
.deletes(List.of(tuple, tuple, tuple, tuple, tuple));
ClientWriteOptions options =
new ClientWriteOptions().enableTransactions(true).transactionChunkSize(2);
new ClientWriteOptions().disableTransactions(false).transactionChunkSize(2);

// When
var response = fga.write(request, options).get();
Expand All @@ -1131,6 +1134,58 @@ public void writeTest_transactions() throws Exception {
assertEquals(200, response.getStatusCode());
}

@Test
public void writeTest_transactionsWithFailure() throws Exception {
// Given
String postPath = "https://localhost/stores/01YCP46JKYM8FJCQ37NMBYHE5X/write";
String firstUser = "user:first";
String failedUser = "user:SECOND";
String skippedUser = "user:third";
Function<String, String> writeBody = user -> String.format(
"{\"writes\":{\"tuple_keys\":[{\"object\":\"%s\",\"relation\":\"%s\",\"user\":\"%s\"}]},\"deletes\":null,\"authorization_model_id\":\"%s\"}",
DEFAULT_OBJECT, DEFAULT_RELATION, user, DEFAULT_AUTH_MODEL_ID);
mockHttpClient
.onPost(postPath)
.withBody(isOneOf(writeBody.apply(firstUser), writeBody.apply(skippedUser)))
.doReturn(200, EMPTY_RESPONSE_BODY);
mockHttpClient
.onPost(postPath)
.withBody(is(writeBody.apply(failedUser)))
.doReturn(400, "{\"code\":\"validation_error\",\"message\":\"Generic validation error\"}");
ClientWriteRequest request = new ClientWriteRequest()
.writes(Stream.of(firstUser, failedUser, skippedUser)
.map(user -> new ClientTupleKey()
._object(DEFAULT_OBJECT)
.relation(DEFAULT_RELATION)
.user(user))
.collect(Collectors.toList()));
ClientWriteOptions options =
new ClientWriteOptions().disableTransactions(false).transactionChunkSize(1);

// When
var execException = assertThrows(
ExecutionException.class, () -> fga.write(request, options).get());

// Then
mockHttpClient
.verify()
.post(postPath)
.withBody(is(writeBody.apply(firstUser)))
.called(1);
mockHttpClient
.verify()
.post(postPath)
.withBody(is(writeBody.apply(failedUser)))
.called(1);
mockHttpClient
.verify()
.post(postPath)
.withBody(is(writeBody.apply(skippedUser)))
.called(0);
var exception = assertInstanceOf(FgaApiValidationError.class, execException.getCause());
assertEquals(400, exception.getStatusCode());
}

@Test
public void writeTuplesTest() throws Exception {
// Given
Expand Down Expand Up @@ -1713,6 +1768,7 @@ public void listObjects_500() throws Exception {
assertEquals(
"{\"code\":\"internal_error\",\"message\":\"Internal Server Error\"}", exception.getResponseData());
}

/**
* Check whether a user is authorized to access an object.
*/
Expand Down

0 comments on commit 4a350af

Please sign in to comment.