chore: tidy up ClientBatchOptions and ClientListRetationsOptions by rhamzeh · Pull Request #128 · openfga/dotnet-sdk

rhamzeh · 2025-10-01T17:33:30Z

Description

What problem is being solved?

How is it being solved?

What changes are made to solve it?

References

Review Checklist

I have clicked on "allow edits by maintainers".
I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
The correct base branch is being used, if not main
I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

Refactor
- ListRelations now accepts dedicated ListRelationsOptions for clearer, purpose-built configuration. Update integrations accordingly.
Bug Fixes
- Corrected equality and hash code behavior for batch check responses, enabling reliable comparisons and use in collections.
- Validation methods for related response types no longer throw and now behave consistently.

coderabbitai · 2025-10-01T17:33:39Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Updated ListRelations to accept IClientListRelationsOptions instead of IClientBatchCheckOptions. Implemented equality, hashing, and no-op validation for batch check response models. Added IClientListRelationsOptions interface and updated ClientListRelationsOptions to implement it.

Changes

Cohort / File(s)	Summary of Changes
Client API: ListRelations options type `src/OpenFga.Sdk/Client/Client.cs`	Changed ListRelations signature to use `IClientListRelationsOptions?` instead of `IClientBatchCheckOptions?`. No other parameters or return types changed.
Batch check response semantics `src/OpenFga.Sdk/Client/Model/ClientBatchCheckResponse.cs`	Implemented `Equals`, `GetHashCode`, and `Validate` for `BatchCheckSingleResponse` and `ClientBatchCheckClientResponse`. Error equality now based on reference or message. Validation methods now yield no results instead of throwing. Hash codes include relevant fields and response collection.
ListRelations options interface `src/OpenFga.Sdk/Client/Model/ClientListRelationsOptions.cs`	Added `IClientListRelationsOptions : IClientBatchCheckOptions`. Updated `ClientListRelationsOptions` to implement `IClientListRelationsOptions` (no field/behavior changes shown).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title inaccurately refers to “ClientBatchOptions” and contains a typo in “ClientListRetationsOptions,” which do not match the actual types and file names changed and therefore does not clearly or correctly summarize the main changes in the pull request.	Please revise the title to accurately reference the updated interfaces and classes (for example, IClientBatchCheckOptions and IClientListRelationsOptions) without typos and to succinctly summarize the core change to the ListRelations method signature and related cleanup.
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov-commenter · 2025-10-01T17:34:22Z

Codecov Report

❌ Patch coverage is 14.81481% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.61%. Comparing base (0b57734) to head (1aa2d58).

Files with missing lines	Patch %	Lines
...enFga.Sdk/Client/Model/ClientBatchCheckResponse.cs	14.81%	23 Missing ⚠️

❌ Your project status has failed because the head coverage (31.61%) 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     #128      +/-   ##
==========================================
- Coverage   31.69%   31.61%   -0.09%     
==========================================
  Files         135      135              
  Lines        6862     6886      +24     
  Branches      900      905       +5     
==========================================
+ Hits         2175     2177       +2     
- Misses       4499     4520      +21     
- Partials      188      189       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/OpenFga.Sdk/Client/Model/ClientBatchCheckResponse.cs (2)
64-65: Consider exception type in equality check.

Comparing exceptions solely by message can lead to false positives—two different exception types with identical messages would be considered equal. Consider including exception type in the comparison:
 (ReferenceEquals(Error, other.Error) ||
- (Error != null && other.Error != null && Error.Message == other.Error.Message));
+ (Error != null && other.Error != null && 
+  Error.GetType() == other.Error.GetType() && 
+  Error.Message == other.Error.Message));
87-89: Align hash code with equality logic.

If the Equals method is updated to include exception type (per previous comment), this hash code should also incorporate Error.GetType():
 if (Error != null) {
-    hashCode = (hashCode * 9923) + Error.Message.GetHashCode();
+    hashCode = (hashCode * 9923) + Error.GetType().GetHashCode();
+    hashCode = (hashCode * 9923) + Error.Message.GetHashCode();
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b57734 and 0a52192.

📒 Files selected for processing (3)

src/OpenFga.Sdk/Client/Client.cs (1 hunks)
src/OpenFga.Sdk/Client/Model/ClientBatchCheckResponse.cs (4 hunks)
src/OpenFga.Sdk/Client/Model/ClientListRelationsOptions.cs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/OpenFga.Sdk/Client/Model/ClientBatchCheckResponse.cs (3)

src/OpenFga.Sdk/Client/Model/ClientExpandRequest.cs (2)

IEnumerable (81-83)

GetHashCode (92-110)

src/OpenFga.Sdk/Client/Model/ClientCheckRequest.cs (2)

IEnumerable (98-100)

GetHashCode (109-135)

src/OpenFga.Sdk/Client/Model/ClientWriteResponse.cs (4)

IEnumerable (69-71)

IEnumerable (138-140)

GetHashCode (81-99)

GetHashCode (149-163)

⏰ 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). (1)

GitHub Check: Analyze (csharp)

🔇 Additional comments (6)

src/OpenFga.Sdk/Client/Model/ClientListRelationsOptions.cs (2)

18-19: LGTM! Good type safety improvement.

The new IClientListRelationsOptions interface provides type-specific options for ListRelations, improving API clarity while maintaining compatibility through inheritance from IClientBatchCheckOptions.

21-21: LGTM! Consistent with the new interface.

The class now correctly implements IClientListRelationsOptions, aligning with the type hierarchy change.

src/OpenFga.Sdk/Client/Client.cs (1)

391-393: LGTM! Type-safe signature improvement.

The method signature now correctly accepts IClientListRelationsOptions, making the API more type-safe and semantically clear. Since IClientListRelationsOptions extends IClientBatchCheckOptions, the downstream BatchCheck call at line 411 remains valid.

src/OpenFga.Sdk/Client/Model/ClientBatchCheckResponse.cs (3)

76-78: LGTM! Proper validation implementation.

The no-op validation implementation is correct for a type with no validation requirements.

134-136: LGTM! Proper validation implementation.

The no-op validation is appropriate for this response type.

144-154: LGTM! Correct collection hash code.

The hash code properly accumulates values from the Responses collection with appropriate null handling.

rhamzeh requested a review from a team as a code owner October 1, 2025 17:33

coderabbitai bot reviewed Oct 1, 2025

View reviewed changes

chore: tidy up ClientBatchOptions and ClientListRetationsOptions

1aa2d58

rhamzeh force-pushed the fix/clientbatchoptions branch from 0a52192 to 1aa2d58 Compare October 1, 2025 18:05

evansims approved these changes Oct 1, 2025

View reviewed changes

rhamzeh added this pull request to the merge queue Oct 1, 2025

Merged via the queue into main with commit f79d9de Oct 1, 2025
18 checks passed

rhamzeh deleted the fix/clientbatchoptions branch October 1, 2025 18:47

coderabbitai bot mentioned this pull request Oct 30, 2025

feat: Implement server-side BatchCheck using /batch-check endpoint #150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: tidy up ClientBatchOptions and ClientListRetationsOptions#128

chore: tidy up ClientBatchOptions and ClientListRetationsOptions#128
rhamzeh merged 1 commit intomainfrom
fix/clientbatchoptions

rhamzeh commented Oct 1, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 1, 2025 •

edited

Loading

Review skipped

Uh oh!

codecov-commenter commented Oct 1, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

rhamzeh commented Oct 1, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What problem is being solved?

How is it being solved?

What changes are made to solve it?

References

Review Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

codecov-commenter commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

rhamzeh commented Oct 1, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 1, 2025 •

edited

Loading

codecov-commenter commented Oct 1, 2025 •

edited

Loading