fix(java-sdk): fix passing model ID in BatchCheck - #555
Conversation
rhamzeh
changed the title
|
""" WalkthroughThe changes update the Java SDK template to ensure the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OpenFgaClient
participant API
Client->>OpenFgaClient: batchCheck(items, options)
OpenFgaClient->>OpenFgaClient: Build BatchCheckRequest
Note right of OpenFgaClient: Set checks, consistency, authorizationModelId
OpenFgaClient->>API: batchCheck(BatchCheckRequest)
API-->>OpenFgaClient: BatchCheckResponse
OpenFgaClient-->>Client: ClientBatchCheckResponse
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
config/clients/java/template/src/test/api/client/OpenFgaClientTest.java.mustache (2)
2032-2039: Loosen JSON-body coupling – prefer semantic matchers over rawString.format.The test hard-codes the exact JSON produced by the SDK (including field order and explicit
nullvalues).
Any innocuous re-ordering of fields or a serialization setting that omitsnulls will make this test fail even though the client behaviour is still correct.Consider switching to a body matcher that asserts only the relevant semantics, e.g.:
mockHttpClient.onPost(postUrl) .withBody(allOf( matchingJsonPath("$.checks[0].tuple_key.user", equalTo(DEFAULT_USER)), matchingJsonPath("$.authorization_model_id", equalTo(DEFAULT_AUTH_MODEL_ID)), matchingJsonPath("$.consistency", equalTo(ConsistencyPreference.MINIMIZE_LATENCY.name())) )) .doReturn(200, "{\"result\":{}}");This keeps the test resilient to unrelated JSON formatting changes.
2052-2058: Minor assertion gap – verify HTTP-layer interaction count.After invoking
fga.batchCheck, you assert the request body and the emptiness of the result map, but you do not assert the call count onmockHttpClient. Adding:mockHttpClient.verify().post(postUrl).called(1);would mirror the pattern used in neighbouring tests and guard against accidental retries.
config/clients/java/template/src/main/api/client/OpenFgaClient.java.mustache (1)
698-702: Minor naming nit – avoid parameter shadowing for clarity.Using
requestboth as the lambda parameter and in outer scopes is slightly confusing. Renaming improves readability and prevents accidental misuse.-Consumer<List<BatchCheckItem>> singleBatchCheckRequest = request -> call(() -> { - BatchCheckRequest body = new BatchCheckRequest().checks(request); +Consumer<List<BatchCheckItem>> singleBatchCheckRequest = batchItems -> call(() -> { + BatchCheckRequest body = new BatchCheckRequest().checks(batchItems);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
config/clients/java/template/libraries/native/api.mustache(1 hunks)config/clients/java/template/src/main/api/client/OpenFgaClient.java.mustache(1 hunks)config/clients/java/template/src/test/api/client/OpenFgaClientTest.java.mustache(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (go)
- GitHub Check: build-and-test-java-sdk
- GitHub Check: build-and-test-go-sdk
- GitHub Check: build-and-test-dotnet-sdk
- GitHub Check: build-and-test-python-sdk
- GitHub Check: build-and-test-js-sdk
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
config/clients/java/template/libraries/native/api.mustache (1)
268-272: ```shell
#!/bin/bash
set -eSearch for any earlier occurrences of putting the model ID into telemetryAttributes
rg -n "FGA_CLIENT_REQUEST_MODEL_ID" config/clients/java/template/libraries/native/api.mustache
Locate the buildTelemetryAttributes method definition to see initial population logic
rg -n "buildTelemetryAttributes" -n 20 config/clients/java/template/libraries/native/api.mustache
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
bringing the openfga/sdk-generator#555 from the generator
* fix(client): fix passing model ID in BatchCheck bringing the openfga/sdk-generator#555 from the generator * chore(docs): fix typo in README source: openfga/sdk-generator#561 * chore: remove unnecessary if condition source: openfga/sdk-generator#546 --------- Co-authored-by: Andrés Aguiar <andres.aguiar@gmail.com> Co-authored-by: AlessandroMinoccheri <aminoccheri@inpost.it>
3 participants
Summary
Testing
make test-client-java(fails: docker not found)https://chatgpt.com/codex/tasks/task_e_6846b74224688322925a10d153f3fca5
Summary by CodeRabbit
New Features
Tests
Closes openfga/java-sdk#177