Refactor RelationshipResolver to use generic relationship types#14
Refactor RelationshipResolver to use generic relationship types#14
Conversation
Make the relationship type in RelationshipResolver and RebacPolicy generic instead of hardcoded to &str. This allows using enums or other domain-specific types for compile-time safety — a typo like "contibutor" becomes a compile error rather than a silent permission failure. Add an enum-based relationship example to rebac_policy.rs demonstrating the pattern. Co-authored-by: Steel <52361520+SteelAlloy@users.noreply.github.com> Closes #10
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e71597e24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR refactors RelationshipResolver and RebacPolicy to use a generic relationship type parameter (Re) instead of hardcoded &str, enabling compile-time safety when using enums or other domain-specific types for relationships. This addresses issue #10 by allowing strongly-typed relationship definitions.
Changes:
- Added generic type parameter
RetoRelationshipResolvertrait andRebacPolicystruct - Added
fmt::Displaybound onRefor human-readable policy evaluation messages - Updated all existing implementations to use
Stringas the relationship type - Added comprehensive enum-based relationship example demonstrating type-safe alternative to strings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/lib.rs | Adds generic Re type parameter to RelationshipResolver trait and RebacPolicy, updates documentation and tests to use String explicitly |
| examples/rebac_policy.rs | Updates string-based examples to use .to_string() and adds new enum-based relationship example with Display implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document that Re must implement Display (used for human-readable policy evaluation reasons). Fix enum example output that said "viewer" when Bob is actually a contributor.
- Move Display requirement docs from RelationshipResolver trait (where it is not enforced) to RebacPolicy struct (where the bound lives) - Remove redundant Send + Sync bounds on RG in Policy impl (already required by RelationshipResolver supertrait) - Add unit test for enum-based relationship types - Fix enum example: test all three users, correct comment to say "only owners can edit", add assertions
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
RelationshipResolverandRebacPolicygeneric (Re) instead of hardcoded to&strfmt::Displaybound onRefor human-readable policy evaluation messagesrebac_policy.rsBased on @SteelAlloy's work in #11. Rebased on main (which now includes the
#![warn(missing_docs)]changes from #13) and added the enum example requested in review.Breaking change:
RelationshipResolver<S, R>becomesRelationshipResolver<S, R, Re>, andRebacPolicygains an additional type parameter.Closes #10
Test plan
cargo fmt --all— no changescargo clippy --all-targets --all-features -- -D warnings— zero warningscargo doc --no-deps— zero warningscargo test --all-targets --all-features— all 65 tests passcargo test --doc— all 9 doc tests passcargo run --example rebac_policy— runs successfully including new enum section