feat: APIExecutor for calling raw arbitrary endpoints #273
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis PR introduces the API Executor feature, enabling direct HTTP access to OpenFGA endpoints not yet wrapped by the SDK. It adds ApiExecutor and ApiExecutorRequestBuilder classes, updates HttpRequestAttempt to handle String responses, exposes an apiExecutor() method in OpenFgaClient, and provides comprehensive examples, tests, and documentation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant OpenFgaClient
participant ApiExecutor
participant ApiExecutorRequestBuilder
participant HttpRequestAttempt
participant Server as OpenFGA Server
Client->>OpenFgaClient: apiExecutor()
OpenFgaClient-->>Client: ApiExecutor instance
Client->>ApiExecutorRequestBuilder: builder(method, path)
ApiExecutorRequestBuilder-->>Client: builder instance
Client->>ApiExecutorRequestBuilder: pathParam(key, value)
Client->>ApiExecutorRequestBuilder: queryParam(key, value)
Client->>ApiExecutorRequestBuilder: header(key, value)
Client->>ApiExecutorRequestBuilder: body(data)
Client->>ApiExecutorRequestBuilder: build()
ApiExecutorRequestBuilder-->>Client: request builder
Client->>ApiExecutor: send(requestBuilder, responseType)
ApiExecutor->>ApiExecutor: buildCompletePath()
ApiExecutor->>ApiExecutor: buildHttpRequest()
ApiExecutor->>HttpRequestAttempt: execute(httpRequest)
HttpRequestAttempt->>Server: HTTP request
Server-->>HttpRequestAttempt: HTTP response
HttpRequestAttempt->>HttpRequestAttempt: deserializeResponse()
HttpRequestAttempt-->>ApiExecutor: ApiResponse<T>
ApiExecutor-->>Client: CompletableFuture<ApiResponse<T>>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (37.68%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #273 +/- ##
============================================
+ Coverage 37.06% 37.68% +0.62%
- Complexity 1200 1238 +38
============================================
Files 194 196 +2
Lines 7504 7606 +102
Branches 865 881 +16
============================================
+ Hits 2781 2866 +85
- Misses 4593 4601 +8
- Partials 130 139 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Related Documentation Checked 5 published document(s) in 0 knowledge base(s). No updates required. How did I do? Any feedback? |
There was a problem hiding this comment.
Pull request overview
This PR adds a Raw API feature that enables developers to call arbitrary OpenFGA endpoints not yet wrapped by the SDK's typed methods. The raw API maintains all SDK configuration including authentication, retry logic, error handling, and telemetry while providing flexible access to any endpoint.
Changes:
- Added Raw API functionality with
RawRequestBuilderfor constructing requests andRawApifor execution - Updated
HttpRequestAttemptto support raw string responses - Added comprehensive unit and integration tests
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.java | Builder pattern for constructing HTTP requests with path/query params, headers, and body |
| src/main/java/dev/openfga/sdk/api/client/RawApi.java | Executes raw HTTP requests using SDK's internal HTTP client and configuration |
| src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java | Adds raw() method to expose Raw API functionality |
| src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java | Modified to return raw string responses when String.class is requested |
| src/test/java/dev/openfga/sdk/api/client/RawApiTest.java | Comprehensive unit tests using WireMock |
| src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java | Integration tests using OpenFGA container |
| docs/RawApi.md | Complete API documentation with examples |
| examples/raw-api/ | Working example demonstrating all Raw API features |
| README.md | Main documentation updated with Raw API usage examples |
| examples/streamed-list-objects/build.gradle | Version bump to 0.9.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@docs/RawApi.md`:
- Around line 59-67: The docs show using a terminal .build() call on
RawRequestBuilder but the class uses fluent chaining and returns this; remove
the .build() invocation and pass the RawRequestBuilder instance directly to
send() (i.e., update examples using RawRequestBuilder so they call
send(rawRequestBuilder) or equivalent), ensuring consistency with the other
examples that omit .build() and referencing RawRequestBuilder and the send()
usage in the ApiResponse examples.
In `@examples/raw-api/gradle/wrapper/gradle-wrapper.properties`:
- Around line 1-7: Update the Gradle wrapper distribution URL to use Gradle
8.2.1 by changing the distributionUrl value from
"https://services.gradle.org/distributions/gradle-8.6-bin.zip" to
"https://services.gradle.org/distributions/gradle-8.2.1-bin.zip", leaving the
existing security settings (validateDistributionUrl=true and
networkTimeout=10000) and other wrapper properties (distributionBase,
distributionPath, zipStoreBase, zipStorePath) unchanged.
In `@examples/raw-api/README.md`:
- Around line 21-32: The README uses a hardcoded absolute path
"/Users/anurag/openfga/java-sdk"; change that to a relative or generic
instruction so it works for all users—replace the line `cd
/Users/anurag/openfga/java-sdk` with guidance to "cd to the SDK root directory"
or a relative path like "../.." and update the step to run the build (e.g.,
"./gradlew build") from the SDK root; ensure the README clearly states "from the
SDK root directory" or similar instead of the developer-specific absolute path.
- Around line 59-67: The snippet incorrectly calls a non-existent terminal
method .build() on RawRequestBuilder; remove the trailing .build() and use the
fluent builder instance directly (e.g., assign the result of
RawRequestBuilder.builder(...).pathParam(...).queryParam(...).body(...).header(...)
to the request variable) so no .build() invocation remains and the variable type
matches RawRequestBuilder.
In `@src/main/java/dev/openfga/sdk/api/client/RawApi.java`:
- Around line 150-155: In RawApi (the block that handles request bodies), don't
call String.getBytes() without a charset; explicitly encode string bodies as
UTF-8 (e.g., use StandardCharsets.UTF_8) when setting bodyBytes for the String
branch, and add the necessary import for StandardCharsets if missing so the
request payload encoding is deterministic and consistent with HTTP/JSON
expectations.
In
`@src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java`:
- Around line 442-456: In writeTupleUsingRawRequest, the requestBody currently
puts "writes" as a list of objects containing "tuple_key"; change it to put
"writes" as a map with the "tuple_keys" array instead (i.e. set
requestBody.put("writes", Map.of("tuple_keys", List.of(tupleKey)))); update the
structure before building the RawRequestBuilder so the body matches the OpenFGA
Write API, then keep using RawRequestBuilder and fga.raw().send(...) as-is.
🧹 Nitpick comments (15)
examples/streamed-list-objects/build.gradle (1)
20-20: Version bump looks good.The SDK version update from 0.9.3 to 0.9.4 is correct and consistent with the Raw API feature introduced in this PR.
Optional: Consider extracting the SDK version to a Gradle variable.
To reduce maintenance burden when bumping versions, you could extract the version number:
♻️ Optional refactor
ext { + openfgaSdkVersion = "0.9.4" jacksonVersion = "2.18.2" }dependencies { // Use local build of SDK - implementation files('../../build/libs/openfga-sdk-0.9.4.jar') + implementation files("../../build/libs/openfga-sdk-${openfgaSdkVersion}.jar")This would allow updating the version in one place and would apply consistently across all example modules.
examples/raw-api/Makefile (2)
1-2: Addallto the.PHONYdeclaration.The
alltarget on line 2 should be included in the.PHONYdeclaration to prevent issues if a file namedallever exists in the directory.Suggested fix
-.PHONY: build run run-openfga +.PHONY: all build run run-openfga
5-5: Consider pinning to a specific OpenFGA version.Using
latestforopenfga_versioncan cause reproducibility issues in examples when the upstream image changes. Consider pinning to a specific version that matches the SDK's tested compatibility.src/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.java (2)
7-19: Minor documentation accuracy issue.The Javadoc states that "Path and query parameters are automatically URL-encoded" (line 10), but the encoding is actually performed in
RawApi.buildCompletePath(), not in this builder class. While this is functionally correct from the user's perspective, consider clarifying that encoding happens at request execution time.
72-77: Consider documenting or reconsidering silent null handling.The
pathParam,queryParam, andheadermethods silently ignore calls when either key or value is null. While this provides lenient handling, it could mask bugs where the caller expects a parameter to be set but accidentally passes null.Consider either:
- Documenting this behavior in the Javadoc
- Throwing
IllegalArgumentExceptionfor null keys (values could remain optional)src/main/java/dev/openfga/sdk/api/client/RawApi.java (1)
101-109: Consider type-safe store ID access.The
instanceofcheck and cast pattern could be avoided if the constructor requiredClientConfigurationinstead ofConfiguration, sinceRawApiis typically accessed viaOpenFgaClient.raw()which has aClientConfiguration. However, the current approach offers flexibility ifRawApineeds to work with other configuration types in the future.examples/raw-api/src/main/java/dev/openfga/sdk/example/RawApiExample.java (2)
88-97: Consider removing the hardcoded fallback store ID.The hardcoded fallback ID
"01YCP46JKYM8FJCQ37NMBYHE5X"is unlikely to exist in the user's environment. If both listing and creating stores fail, subsequent examples will also fail. Consider propagating the error or exiting gracefully instead.Suggested approach
} catch (Exception e) { System.err.println("✗ Error: " + e.getMessage()); - // Create a store on error - try { - return createStoreForExamples(fgaClient); - } catch (Exception ex) { - return "01YCP46JKYM8FJCQ37NMBYHE5X"; // fallback - } + System.err.println("Unable to list or create stores. Please ensure OpenFGA server is running."); + throw e; }
199-205: Add null check forgetCause()for defensive coding.While
ExecutionExceptiontypically has a cause, adding a null check prevents potentialNullPointerExceptionin edge cases.Suggested fix
} catch (Exception e) { // Expected error - demonstrates proper error handling System.out.println("✓ Error handled correctly:"); System.out.println(" Message: " + e.getMessage()); - System.out.println(" Type: " + e.getCause().getClass().getSimpleName()); + Throwable cause = e.getCause(); + System.out.println(" Type: " + (cause != null ? cause.getClass().getSimpleName() : "Unknown")); }examples/raw-api/build.gradle (2)
18-20: Hardcoded SDK version may cause maintenance issues.The SDK version
0.9.4is hardcoded in the JAR path. When the SDK version is bumped, this file will need manual updates or the example will fail to build.Consider using a dynamic approach or at least documenting this dependency clearly:
💡 Suggested improvement
+ext { + sdkVersion = "0.9.4" + jacksonVersion = "2.18.2" +} + dependencies { // Use local build of SDK - implementation files('../../build/libs/openfga-sdk-0.9.4.jar') + // NOTE: Update sdkVersion when SDK version changes + implementation files("../../build/libs/openfga-sdk-${sdkVersion}.jar")
40-42: Duplicate comment block.The comment block explaining spotless usage appears twice (lines 40-42 and 61-63). The second occurrence can be removed since the
fmttask is self-explanatory.🧹 Suggested cleanup
} -// Use spotless plugin to automatically format code, remove unused import, etc -// To apply changes directly to the file, run `gradlew spotlessApply` -// Ref: https://github.com/diffplug/spotless/tree/main/plugin-gradle tasks.register('fmt') { dependsOn 'spotlessApply' }Also applies to: 61-63
src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java (4)
31-31: UnusedObjectMapperinstance.The
ObjectMapperis declared but never used in this test class. Either remove it or use it where JSON parsing/assertions could benefit from it.
36-37: Remove debug logging system property from test setup.Setting
HttpRequestAttempt.debug-loggingin tests can cause noisy output in CI/CD pipelines and is typically not needed for integration tests. If debugging is needed, consider using a test profile or making it conditional.🧹 Suggested cleanup
`@BeforeEach` public void initializeApi() throws Exception { - System.setProperty("HttpRequestAttempt.debug-logging", "enable"); - ClientConfiguration apiConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()); fga = new OpenFgaClient(apiConfig); }
253-255: Empty continuation token query parameter.Passing an empty string for
continuation_tokenis unusual. Typically, you would either omit the parameter entirely on the first request or pass a non-empty token from a previous response. The server may interpret an empty string differently than an absent parameter.Consider removing this parameter for the initial request:
💡 Suggested fix
RawRequestBuilder request = RawRequestBuilder.builder("GET", "/stores/{store_id}/authorization-models") - .queryParam("page_size", "10") - .queryParam("continuation_token", ""); + .queryParam("page_size", "10");
350-361: Prefer asserting on specific exception type rather than catching genericException.The current pattern catches all exceptions and just asserts non-null. This could mask unexpected failures. Consider using
assertThrowswith the expected exception type for clearer failure messages and stricter validation.💡 Suggested improvement
- // Should throw an exception - try { - fga.raw().send(request, GetStoreResponse.class).get(); - fail("Should have thrown an exception for non-existent store"); - } catch (Exception e) { - // Expected - verify it's some kind of error (ExecutionException wrapping an FgaError) - assertNotNull(e, "Exception should not be null"); - System.out.println("✓ Successfully handled error for non-existent store"); - System.out.println(" Error type: " + e.getClass().getSimpleName()); - if (e.getCause() != null) { - System.out.println(" Cause: " + e.getCause().getClass().getSimpleName()); - } - } + // Should throw ExecutionException wrapping FgaError + ExecutionException exception = assertThrows( + ExecutionException.class, + () -> fga.raw().send(request, GetStoreResponse.class).get()); + + assertInstanceOf(dev.openfga.sdk.errors.FgaError.class, exception.getCause(), + "Expected FgaError for non-existent store");src/test/java/dev/openfga/sdk/api/client/RawApiTest.java (1)
159-164: UnusedmockResponsevariable.The
ExperimentalResponse mockResponseis instantiated but never used since the mock response is built directly as a JSON string. Remove the unused variable.🧹 Suggested cleanup
`@Test` public void rawApi_canSendGetRequestWithTypedResponse() throws Exception { // Setup mock server - ExperimentalResponse mockResponse = new ExperimentalResponse(true, 42, "Success"); stubFor(get(urlEqualTo("/stores/" + DEFAULT_STORE_ID + "/experimental-feature"))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
README.mddocs/RawApi.mdexamples/README.mdexamples/raw-api/Makefileexamples/raw-api/README.mdexamples/raw-api/build.gradleexamples/raw-api/gradle.propertiesexamples/raw-api/gradle/wrapper/gradle-wrapper.propertiesexamples/raw-api/gradlewexamples/raw-api/gradlew.batexamples/raw-api/settings.gradleexamples/raw-api/src/main/java/dev/openfga/sdk/example/RawApiExample.javaexamples/streamed-list-objects/build.gradlesrc/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.javasrc/main/java/dev/openfga/sdk/api/client/OpenFgaClient.javasrc/main/java/dev/openfga/sdk/api/client/RawApi.javasrc/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.javasrc/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.javasrc/test/java/dev/openfga/sdk/api/client/RawApiTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T19:43:18.975Z
Learnt from: jimmyjames
Repo: openfga/java-sdk PR: 211
File: examples/opentelemetry/src/main/java/dev/openfga/sdk/example/opentelemetry/OpenTelemetryExample.java:8-10
Timestamp: 2025-08-19T19:43:18.975Z
Learning: The examples/opentelemetry/src/main/java/dev/openfga/sdk/example/opentelemetry/OpenTelemetryExample.java file will be generated from mustache templates, so the auto-generation header is correct and should be retained.
Applied to files:
examples/README.mdexamples/raw-api/README.mdexamples/raw-api/src/main/java/dev/openfga/sdk/example/RawApiExample.javaexamples/raw-api/build.gradle
🧬 Code graph analysis (2)
examples/raw-api/src/main/java/dev/openfga/sdk/example/RawApiExample.java (2)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
OpenFgaClient(23-1495)src/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.java (1)
RawRequestBuilder(20-145)
src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java (3)
src/main/java/dev/openfga/sdk/errors/FgaError.java (1)
com(50-84)src/main/java/dev/openfga/sdk/api/configuration/ClientConfiguration.java (1)
ClientConfiguration(8-137)src/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.java (1)
RawRequestBuilder(20-145)
🪛 checkmake (0.2.2)
examples/raw-api/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 2-2: Target "all" should be declared PHONY.
(phonydeclared)
🔇 Additional comments (24)
examples/raw-api/settings.gradle (1)
1-1: LGTM!Standard Gradle settings file with appropriate project naming.
src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java (1)
219-224: LGTM!The String.class short-circuit for raw response handling is clean and correctly placed after the Void.class check. The
@SuppressWarnings("unchecked")is justified since the type is verified by theclazz == String.classcondition.Note: If
response.body()returnsnull, this will propagate a null value, which appears consistent with how the Raw API is intended to work for empty responses.examples/raw-api/gradle.properties (1)
1-1: LGTM!Simple property file providing the default language configuration for the Gradle build.
examples/raw-api/Makefile (1)
8-16: LGTM!The build, run, and run-openfga targets are well-structured. The
&&chaining inrun-openfgacorrectly ensures the container only runs if the pull succeeds.examples/raw-api/gradlew.bat (1)
1-89: LGTM - Standard Gradle wrapper script.This is a standard Gradle-generated Windows batch wrapper script. The implementation follows established Gradle conventions for Java discovery, classpath setup, and wrapper execution.
examples/raw-api/gradlew (1)
1-234: LGTM - Standard Gradle wrapper script.This is a standard Gradle-generated POSIX shell wrapper script with proper handling for symlinks, various Unix-like environments (Cygwin, MSYS, Darwin, AIX), and POSIX shell compatibility requirements.
src/main/java/dev/openfga/sdk/api/client/RawRequestBuilder.java (1)
118-145: LGTM - Well-designed accessors with proper encapsulation.The package-private accessors appropriately hide implementation details while the defensive copies for map getters prevent external mutation of internal state.
src/main/java/dev/openfga/sdk/api/client/RawApi.java (2)
162-165: Verify header override behavior is intentional.Custom headers from
RawRequestBuilderare added after SDK-managed headers (set inApiClient.requestBuilder), which means they could override standard headers likeContent-TypeorAuthorization. If this is intentional for advanced use cases, consider documenting this behavior. If not, you may want to validate or restrict which headers users can set.
57-95: LGTM - Well-structured async request handling.The
sendmethods properly validate inputs, delegate appropriately, and useCompletableFuture.failedFuturefor proper async error propagation. The method name format provides useful context for debugging and tracing.examples/README.md (1)
12-18: LGTM - Clear documentation for new examples.The new sections appropriately describe the streaming and raw API examples, following the existing documentation pattern. The description correctly highlights the key benefit of the Raw API feature (maintaining SDK configuration for endpoints not yet wrapped by the SDK).
docs/RawApi.md (2)
1-155: Well-structured documentation for the Raw API feature.The documentation provides a comprehensive guide covering quick start, API reference, multiple usage examples, error handling, and migration guidance. The structure is clear and will help users adopt the Raw API effectively.
136-142: No action required. The documentation claim is accurate: the{store_id}placeholder is indeed auto-replaced from the client configuration when not provided via.pathParam(). This behavior is implemented in RawApi (lines 101-109) and matches the documented behavior.src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
49-67: LGTM - Clean implementation of the raw() accessor method.The method provides a straightforward way to access the Raw API functionality. The Javadoc is comprehensive with a useful example. Creating a new
RawApiinstance per call is acceptable if the object is lightweight and stateless.README.md (3)
49-49: LGTM - TOC updated with the new section.
750-778: Good fixes for code examples.The fixes correct the variable reference and remove the stray parenthesis in the correlationId value.
1171-1252: Comprehensive Raw API documentation added to README.The new section provides clear guidance on using the Raw API with practical examples for both raw JSON and typed responses. The documentation aligns well with the rest of the README structure.
examples/raw-api/src/main/java/dev/openfga/sdk/example/RawApiExample.java (1)
13-57: Well-structured example demonstrating Raw API capabilities.The example effectively showcases different Raw API use cases: typed responses, raw JSON, query parameters, custom headers, and error handling. The class documentation clearly notes the use of blocking
.get()calls for simplicity with guidance on async patterns for production use.examples/raw-api/build.gradle (1)
1-67: Overall structure looks good.The build file follows the pattern of other example projects in the repository, with appropriate dependencies for the Raw API example including Jackson for JSON handling and OpenTelemetry as required by the SDK.
src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java (1)
43-72: Comprehensive test coverage for Raw API functionality.The integration tests thoroughly cover the key Raw API use cases:
- List/create/get stores with typed and raw responses
- Path parameter substitution (explicit and automatic)
- Query parameters and custom headers
- Authorization model operations
- Pagination support
The tests serve as excellent documentation for the Raw API feature.
Also applies to: 74-101, 135-160, 162-185, 187-237, 314-338, 364-391
src/test/java/dev/openfga/sdk/api/client/RawApiTest.java (5)
31-48: Test DTO uses public fields.While acceptable for test DTOs, using public fields for
ExperimentalResponseis unconventional compared to the SDK's model classes that use getters. This is fine for test isolation purposes.
60-64: Thorough RawRequestBuilder validation tests.Excellent coverage of builder edge cases:
- Null/empty method and path validation
- Invalid HTTP method rejection
- All valid HTTP methods accepted (including case-insensitivity)
- Path params, query params, headers, and body handling
Also applies to: 66-154
156-337: Comprehensive WireMock-based API tests.The unit tests effectively validate:
- Typed and raw JSON responses
- Request body serialization with JSON path matching
- Query parameter encoding
- Custom header propagation
- HTTP 4xx/5xx error handling mapped to
FgaErrorThe verification assertions ensure both response handling and request construction are correct.
339-365: Path parameter URL encoding test.Good test for special characters in path parameters. The test correctly verifies that
"store with spaces"is encoded to"store%20with%20spaces"in the URL.
367-378: Null argument validation tests.These tests ensure the Raw API properly rejects null builders and response types with
IllegalArgumentException, providing clear fail-fast behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/test-integration/java/dev/openfga/sdk/api/client/RawApiIntegrationTest.java
Outdated
Show resolved
Hide resolved
SoulPancake
changed the title
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/client/ApiExecutorRequestBuilder.java
Outdated
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/ApiExecutor.md`:
- Around line 53-56: The doc snippet uses the wrong type name; replace the usage
of RawApi with the correct ApiExecutor type everywhere this example appears
(e.g., change the variable declaration that uses RawApi to ApiExecutor and keep
the call to client.apiExecutor() unchanged) so the documented type matches the
implementation/class name ApiExecutor.
In
`@examples/api-executor/src/main/java/dev/openfga/sdk/example/ApiExecutorExample.java`:
- Around line 193-209: In errorHandlingExample, the catch block assumes
e.getCause() is non-null which can cause an NPE; update the catch to safely
handle a null cause when printing the error type (e.g., if e.getCause() != null
print e.getCause().getClass().getSimpleName() otherwise print
e.getClass().getSimpleName() or "UnknownCause"), and ensure the Message and Type
lines still print the original exception information without risking a
null-pointer.
In `@src/main/java/dev/openfga/sdk/api/client/ApiExecutor.java`:
- Around line 15-27: Update the Javadoc example to use the correct API executor
accessor: replace the typed-response call that uses client.raw() with
client.apiExecutor() so the example matches the raw JSON example and the
ApiExecutor flow; locate the example block referencing
ApiExecutorRequestBuilder, ApiResponse and the send method and change the
client.raw() invocation to client.apiExecutor() to ensure consistency.
In `@src/main/java/dev/openfga/sdk/api/client/ApiExecutorRequestBuilder.java`:
- Around line 12-19: The Javadoc example uses the removed RawRequestBuilder
class; update the example to reference the current ApiExecutorRequestBuilder API
by replacing occurrences of RawRequestBuilder with ApiExecutorRequestBuilder and
adjust the example variable/type (e.g.,
ApiExecutorRequestBuilder.builder("POST",
"/stores/{store_id}/endpoint").pathParam("store_id",
storeId).queryParam("limit", "50").body(requestObject).build();) so the example
matches the actual ApiExecutorRequestBuilder builder API and method chain.
In
`@src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java`:
- Around line 28-41: The test sets the JVM property
"HttpRequestAttempt.debug-logging" in initializeApi() but never restores it;
capture the previous value at start of initializeApi() (e.g., store in a private
String previousDebugLogging field), set the property as you do, and add an
`@AfterEach` method (e.g., restoreDebugLogging or cleanupDebugLogging) that
restores the property to the saved previous value (use System.setProperty if
previous != null, otherwise remove it with System.clearProperty) so the global
state is not leaked to other tests; reference initializeApi(), the OpenFgaClient
setup, and the "HttpRequestAttempt.debug-logging" property when making the
changes.
🧹 Nitpick comments (5)
examples/api-executor/Makefile (1)
1-2: Addallto.PHONYdeclaration.The
alltarget on line 2 should be declared as PHONY to ensure it always runs regardless of file existence.Suggested fix
-.PHONY: build run run-openfga +.PHONY: all build run run-openfga all: buildexamples/api-executor/build.gradle (2)
18-20: Hardcoded SDK version may become stale.The local SDK jar path contains a hardcoded version
0.9.4. This will require manual updates when the SDK version changes.Consider using a variable or dynamic version resolution:
+def sdkVersion = "0.9.4" + dependencies { // Use local build of SDK - implementation files('../../build/libs/openfga-sdk-0.9.4.jar') + implementation files("../../build/libs/openfga-sdk-${sdkVersion}.jar")Alternatively, document that this needs to be updated with SDK releases.
61-66: Remove duplicate comment block.Lines 61-63 duplicate the spotless documentation comment already present at lines 40-42.
Suggested fix
-// Use spotless plugin to automatically format code, remove unused import, etc -// To apply changes directly to the file, run `gradlew spotlessApply` -// Ref: https://github.com/diffplug/spotless/tree/main/plugin-gradle tasks.register('fmt') { dependsOn 'spotlessApply' }docs/ApiExecutor.md (1)
112-119: MissingpathParamfor{store_id}placeholder.The Custom Headers example uses
/stores/{store_id}/actionbut doesn't include a.pathParam("store_id", ...)call. While the notes mention auto-replacement, explicitly showing it would be clearer for users.Suggested fix
### Custom Headers ```java ApiExecutorRequestBuilder.builder("POST", "/stores/{store_id}/action") + .pathParam("store_id", storeId) .header("X-Request-ID", UUID.randomUUID().toString()) .header("X-Idempotency-Key", "key-123") .body(data) .build();</details> </blockquote></details> <details> <summary>src/main/java/dev/openfga/sdk/api/client/ApiExecutor.java (1)</summary><blockquote> `82-90`: **Fail fast when path placeholders remain unresolved.** If `{store_id}` (or any placeholder) survives replacement, the request will hit a literal path and likely 404. Consider rejecting early with a clearer error. <details> <summary>💡 Suggested guard</summary> ```diff String completePath = buildCompletePath(requestBuilder); + if (completePath.contains("{") || completePath.contains("}")) { + throw new FgaInvalidParameterException( + "Unresolved path parameters in request path: " + completePath); + } HttpRequest httpRequest = buildHttpRequest(requestBuilder, completePath);
examples/api-executor/src/main/java/dev/openfga/sdk/example/ApiExecutorExample.java
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java
Show resolved
Hide resolved
Description
solves #249
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.