-
Notifications
You must be signed in to change notification settings - Fork 23
Add name filter support to ListStores #195
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 WalkthroughThe changes add support for filtering stores by name in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as OpenFgaClient
participant Api as OpenFgaApi
participant Server as OpenFGA Server
Client->>Api: listStores(pageSize, continuationToken, name, override)
Api->>Server: GET /stores?page_size=...&continuation_token=...&name=...
Server-->>Api: ListStoresResponse (filtered by name)
Api-->>Client: ApiResponse<ListStoresResponse>
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related issues
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
hi @varkart, thanks for the PR! Generally looking good, a couple things we need:
|
c54882a to
d6211f3
Compare
hi @jimmyjames |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (35.21%) 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 #195 +/- ##
============================================
+ Coverage 33.73% 35.21% +1.47%
- Complexity 1005 1073 +68
============================================
Files 182 187 +5
Lines 6900 7093 +193
Branches 778 803 +25
============================================
+ Hits 2328 2498 +170
- Misses 4467 4485 +18
- Partials 105 110 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add name parameter to ClientListStoresOptions with fluent API - Extend OpenFgaApi.listStores() method signatures with backward compatibility - Update OpenFgaClient to pass name parameter from options to API - Add comprehensive unit and integration tests - Update OpenFGA container version for test compatibility - Maintain full backward compatibility Resolves openfga#157
- Add name parameter to ClientListStoresOptions - Extend OpenFgaApi.listStores() method signatures - Update OpenFgaClient to pass name parameter - Add comprehensive unit and integration tests - Maintain full backward compatibility Resolves openfga#157
|
Thanks for this contribution @varkart! We unfortunately had someone already make this change in the sdk-generator repo, but there's some extra tests in this PR that I'd like to carry over. I'm going to do the following:
Thanks again for this contribution and sorry about the double work situation that happened! |
Add name filter support to ListStores
Resolves #157
Description
What problem is being solved?
The OpenFGA API supports filtering stores by name (introduced in openfga/api#211), but the Java SDK doesn't expose
this functionality. Users currently cannot filter stores by name when calling
listStores(), requiring them toretrieve all stores and filter client-side.
How is it being solved?
Added optional
nameparameter support to the existingListStoresfunctionality by:ClientListStoresOptionsclass with anamefieldOpenFgaApi.listStores()that accept the name parameterOpenFgaClientto pass the name parameter from options to the APIpathWithParamsutility to properly construct query parametersWhat changes are made to solve it?
namefield with fluent setter/getter methodslistStoresmethods acceptingnameparameter while maintaining backwardcompatibility
getName()from options to the API calland Client levels
Usage examples:
References
namefilter toListStores#157Review Checklist
ull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork).
openfga.dev [Provide a link to any relevant PRs in the references section
above]
mainSummary by CodeRabbit
New Features
Tests