fix: migrate transaction docs and examples to transactions() API - #373
Conversation
Move the write mode-selection Javadoc in OpenFgaClient and the example projects off the double-negative disableTransactions(boolean) to the affirmative transactions()/isTransactionsEnabled() API. Also switch the internal write mode branch to !isTransactionsEnabled() so the SDK no longer calls its own soon-to-be-deprecated method. Behavior is unchanged. Test call sites that still use disableTransactions are intentionally left for the deprecation issue, which will migrate them and keep one back-compat test under @SuppressWarnings. Refs #368
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe write path now uses ChangesTransaction API migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (38.69%) 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 #373 +/- ##
=========================================
Coverage 38.69% 38.69%
+ Complexity 1290 1289 -1
=========================================
Files 198 198
Lines 7707 7707
Branches 900 900
=========================================
Hits 2982 2982
Misses 4579 4579
Partials 146 146 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s write transaction documentation, examples, and internal branching logic to use the affirmative transactions(boolean) / isTransactionsEnabled() API (introduced in #352) instead of the double-negative disableTransactions(...) methods that are slated for deprecation.
Changes:
- Updated
OpenFgaClient.write(...)Javadoc to describe transactional/non-transactional behavior in terms ofisTransactionsEnabled(). - Switched the internal mode-selection branch from
options.disableTransactions()to!options.isTransactionsEnabled(). - Migrated example call sites from
.disableTransactions(true)to.transactions(false).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java | Updates write-mode docs and internal mode-selection to use isTransactionsEnabled() rather than disableTransactions(). |
| examples/basic-examples/src/main/java/dev/openfga/sdk/example/Example1.java | Updates the Java basic example to use .transactions(false) instead of .disableTransactions(true). |
| examples/basic-examples/src/main/kotlin/dev/openfga/sdk/example/KotlinExample1.kt | Updates the Kotlin basic example to use .transactions(false) instead of .disableTransactions(true). |
| src/test-integration/java/dev/openfga/sdk/example/Example1.java | Updates the integration example to use .transactions(false) instead of .disableTransactions(true). |
Suppressed comments (1)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java:469
- The code treats a null ClientWriteOptions as the default (transactions enabled), but the Javadoc wording implies callers always have a non-null options instance. Consider documenting the actual condition to avoid confusion and accidental null dereferences in user code examples.
* <h3>Transactional Mode (default)</h3>
* <p>When {@code options.isTransactionsEnabled()} is true (the default):</p>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback. The write(request) overload takes no options, so describing the mode as options.isTransactionsEnabled() implied a call on an absent or null options reference. Reword both overloads to key the mode off the transactions() setter and state that transactional mode is the default when no options are passed. Docs only, no behavior change. Refs #368
Summary
Moves the write transaction docs and examples off the double-negative
disableTransactions(boolean)to the affirmativetransactions(boolean)/isTransactionsEnabled()API added in #352. Both APIs coexist today; this stops the SDK's own docs, examples, and internal code from teaching or using the method slated for deprecation.Changes
OpenFgaClient.java: thewritemode-selection Javadoc (both overloads) now describes behavior viaisTransactionsEnabled()instead ofdisableTransactions().OpenFgaClient.java: the internal write-mode branch changed fromoptions.disableTransactions()to!options.isTransactionsEnabled(), so the SDK no longer calls its own soon-to-be-deprecated method..disableTransactions(true)to.transactions(false):examples/basic-examples(Java and Kotlin) andsrc/test-integration.Behavior is unchanged.
transactions(false)is exactly equivalent to the priordisableTransactions(true), and!isTransactionsEnabled()equals the priordisableTransactions().Out of scope
OpenFgaClientTest,OpenFgaClientHeadersTest, andOpenFgaClientWriteResponseHeadersTeststill calldisableTransactions(...). These are migrated in the deprecation issue ((4 of 5) Deprecate disableTransactions(boolean)/disableTransactions() in favour of transactions()/isTransactionsEnabled() #369), which will move most of them totransactions(...)and keep one back-compat test under@SuppressWarnings("deprecation"). This ordering is what keeps the build warning-free once deprecation lands.README.mdwrite examples: generated from openfga/sdk-generator and already updated via chore(sync): sync with generator #372.@Deprecatedto the old methods: separate issue ((4 of 5) Deprecate disableTransactions(boolean)/disableTransactions() in favour of transactions()/isTransactionsEnabled() #369), gated on this change shipping in a release.Verification
Changes verified by inspection:
transactions(boolean)andisTransactionsEnabled()exist onClientWriteOptions, and the logic is a direct equivalence swap. A local compile could not run here because dependency resolution against the private Artifactory mirror returns 401; CI performs the authoritative compile and test.Why now
Step 1 of retiring
disableTransactions. Review on #352 flagged that the SDK's own docs and examples still teach the double-negative method. The old methods cannot be deprecated until the docs and examples stop teaching them, and the deprecation stays blocked until this change ships in a release.Closes #368
Summary by CodeRabbit
transactions(false)option.