chore: make streamed list objects client layer pvt #266
Conversation
WalkthroughThe PR temporarily disables the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
Documentation Updates 1 document(s) were updated by changes in this PR: OpenFGA's Space How did I do? Any feedback? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #266 +/- ##
============================================
- Coverage 37.06% 36.59% -0.47%
+ Complexity 1200 1182 -18
============================================
Files 194 194
Lines 7504 7504
Branches 865 865
============================================
- Hits 2781 2746 -35
- Misses 4593 4633 +40
+ Partials 130 125 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR makes the streamedListObjects API private to prevent SDK clients from using it until it's ready for public release. The feature was recently added but needs to remain internal for now.
Key Changes:
- Changed three overloaded
streamedListObjectsmethods frompublictoprivateinOpenFgaClient.java - Commented out all tests in
StreamedListObjectsTest.javawith a note explaining the API is not yet public - Commented out the integration test in
OpenFgaClientIntegrationTest.java - Commented out the example code in
StreamedListObjectsExample.javaand updated its README with a notice
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java | Changed visibility of three streamedListObjects method overloads from public to private |
| src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java | Commented out entire test class with note about API not being publicly available yet |
| src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java | Commented out streamedListObjects integration test and removed unused import |
| examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java | Commented out example code with note about temporary unavailability |
| examples/streamed-list-objects/README.md | Added notice at the top indicating the example is temporarily disabled |
💡 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
1134-1141: Breaking change: Public OpenFgaClient methods made private will break example code and integration tests.Making
streamedListObjectsmethods private inOpenFgaClientis a breaking change. The example code atexamples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java:141and integration tests atsrc/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java(lines 425, 441, 458) actively call these methods. This change will cause compilation failures in the repository itself.The
StreamedListObjectsApiclass provides public streaming methods, but they require different parameters (storeIdandListObjectsRequestdirectly), whereas theOpenFgaClientmethods accept higher-levelClientListObjectsRequestandClientStreamedListObjectsOptionstypes. These are convenience wrappers, not redundant implementations.Either:
- Update the example and integration tests to use the new API shape if that's intentional
- Keep these methods public if they represent the intended public API
- Provide a clear migration path and deprecation timeline before making them private
♻️ Duplicate comments (2)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (2)
1174-1183: Breaking change: Second overload also made private.This overload with options support is subject to the same breaking change concerns as the first overload. See the previous comment on lines 1134-1141 for detailed recommendations.
1214-1266: Breaking change: Third overload also made private.This final overload with error handling support completes the pattern of making all
streamedListObjectsvariants private. The same breaking change concerns apply as noted in the comment on lines 1134-1141.Positive observation: The implementation remains intact and functional, which will make it easier to re-expose this API in the future when ready.
🧹 Nitpick comments (1)
src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java (1)
1-429: Entire test suite appropriately disabled.With the
streamedListObjectsAPI now private, these tests cannot compile. The note clearly explains this is temporary until the API is publicly available.For production code, commented-out code is typically removed in favor of relying on version control history. However, given the explicit temporary nature and planned re-enablement, the current approach is acceptable.
Optional: Consider whether deleting these tests entirely would be cleaner, since Git preserves the full history. This avoids maintaining large commented blocks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/streamed-list-objects/README.md(1 hunks)examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java(2 hunks)src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java(3 hunks)src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java(2 hunks)src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java(1 hunks)
🧰 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/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java
🪛 Gitleaks (8.30.0)
src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java
[high] 35-35: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (java)
- GitHub Check: Test and Build OpenFGA (17)
- GitHub Check: Test and Build OpenFGA (21)
- GitHub Check: Test and Build OpenFGA (11)
- GitHub Check: Analyze (java)
🔇 Additional comments (4)
examples/streamed-list-objects/README.md (1)
3-5: Clean approach to temporarily disable the example.The HTML comment wrapper preserves the documentation while clearly indicating the example is disabled. This aligns well with making the API private in the main client class.
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java (1)
375-469: Test appropriately disabled for private API.Since the
streamedListObjectsmethods are now private, this test would no longer compile. The block comment approach preserves the test for future re-enablement.Consider whether these tests should be deleted entirely rather than commented out, as version control preserves history. However, given the temporary nature indicated in the notes, the current approach is reasonable.
src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java (1)
35-35: Static analysis false positive (informational only).Gitleaks flagged this line as a potential API key, but
DEFAULT_STORE_IDis a test fixture constant, not a real credential. Since this entire file is currently commented out, this is not an active concern. When re-enabled, no action is needed.examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java (1)
1-199: Example appropriately disabled with clear documentation.The note explains the temporary nature, and the block comment preserves the example code for future re-enablement. This aligns with making the
streamedListObjectsAPI private.Based on learnings, if example files are generated from mustache templates, ensure the templates are also updated to reflect this temporary disablement to avoid regenerating active example code.
ewanharris
left a comment
ewanharris
left a comment
There was a problem hiding this comment.
I know that the OpenFgaApi class isn't meant to be public, but do we want to make that private too?
src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java
Outdated
Show resolved
Hide resolved
@ewanharris In the API layer? |
|
Oh yeah good point, it would just lead to more difficulty in the OpenFgaClient code I guess. Given we don't really document/recommend and AFAIK we treat it as unstable anyways I guess we're fine to leave it |
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java
Show resolved
Hide resolved
Description
making the client layer private so this cannot be used by SDK client
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.