-
Notifications
You must be signed in to change notification settings - Fork 20
feat: support name filter on ListStores API #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 WalkthroughAdds an optional exact-name filter to ListStores. Updates Java API signatures, client wiring, options object, docs, and tests to accept and forward a new String Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Client as OpenFgaClient
participant Api as OpenFgaApi
participant HTTP as HTTP Client
participant Service as OpenFGA
Caller->>Client: listStores(options: pageSize?, continuationToken?, name?)
Client->>Api: listStores(pageSize, continuationToken, name, overrides?)
Api->>HTTP: GET /stores?page_size&continuation_token&name?
HTTP->>Service: Request (name query present if provided)
Service-->>HTTP: 200 OK (ListStoresResponse)
HTTP-->>Api: ApiResponse<ListStoresResponse>
Api-->>Client: CompletableFuture<ApiResponse<...>>
Client-->>Caller: Future resolves with stores (filtered if name provided)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java
Show resolved
Hide resolved
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (36.33%) 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 #237 +/- ##
============================================
+ Coverage 36.30% 36.33% +0.03%
- Complexity 1138 1140 +2
============================================
Files 187 187
Lines 7170 7174 +4
Branches 822 822
============================================
+ Hits 2603 2607 +4
Misses 4462 4462
Partials 105 105 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2932711 to
a3c371b
Compare
a3c371b to
d5fb4a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java (1)
144-168: Pipeline failure is expected per past review.The test logic correctly:
- Creates multiple stores with different names
- Applies an exact name filter via
ClientListStoresOptions- Asserts that only the target store is returned
Per the past review comment by ewanharris, this test failure during
createStorewill be resolved once #234 is merged and the OpenFGA version is updated.src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java (1)
127-149: Pipeline failure is expected per past review.The test correctly:
- Creates multiple stores with different names
- Filters by exact name using the third parameter
- Validates that only one store is returned
Per the past review comment by ewanharris (line 128), this test failure during
createStorewill be resolved once #234 is merged and the OpenFGA version is updated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/OpenFgaApi.md(6 hunks)src/main/java/dev/openfga/sdk/api/OpenFgaApi.java(1 hunks)src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java(2 hunks)src/main/java/dev/openfga/sdk/api/configuration/ClientListStoresOptions.java(2 hunks)src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java(3 hunks)src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java(1 hunks)src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java(7 hunks)src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
src/main/java/dev/openfga/sdk/api/client/model/ClientListStoresResponse.java (1)
ClientListStoresResponse(20-45)
🪛 GitHub Actions: Java CI with Gradle
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java
[error] 371-371: FgaApiValidationError: createStore caused integration test failure during listStoresWithNameFilter (OpenFgaApiIntegrationTest).
src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java
[error] 1-1: Task ':integration' failed. Command: './gradlew build test-integration' (or equivalent). Failure: dev.openfga.sdk.errors.FgaApiValidationError: createStore during OpenFgaApiIntegrationTest.listStoresWithNameFilter (integration test)
🔇 Additional comments (7)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (2)
87-87: LGTM!The addition of the third
nullparameter for the name filter is correct and maintains backward compatibility.
97-98: LGTM!The implementation correctly passes the name filter from options to the API layer, maintaining consistency with other optional parameters.
src/main/java/dev/openfga/sdk/api/configuration/ClientListStoresOptions.java (1)
21-21: LGTM!The new
namefield with its fluent setter and getter follows the established pattern for other optional parameters in this class. The implementation is clean and consistent.Also applies to: 51-58
src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java (3)
17-17: LGTM!The import additions and mock setup changes properly support the new API signature with
ArgumentMatchers.any()for flexible stubbing of theConfigurationOverrideparameter.Also applies to: 35-35, 79-81
104-104: LGTM!Correctly updated the existing test to pass
nullas the third parameter for the name filter, maintaining backward compatibility testing.
116-165: LGTM!The three new test methods comprehensively cover:
- Basic name filtering (lines 116-140)
- Name-only parameter usage (lines 142-165)
- ConfigurationOverride with name parameter (lines 167-191)
All tests properly verify URL construction with the name query parameter and validate the filtered response.
src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java (1)
87-87: LGTM!The existing tests correctly updated to pass
nullas the third parameter for the name filter, maintaining compatibility with the new API signature.Also applies to: 118-118
d5fb4a6 to
d860f91
Compare
Description
Generates the SDK to include the changes to support passing the
namefilter to theListStoresAPIReferences
Supersedes #195
Partly generated from openfga/sdk-generator#586
Closes #157
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Tests