Skip to content

fix: preserve conditional roles when registering name matching - #539

Merged
hsluoyz merged 1 commit into
masterfrom
fix/conditional-role-matching
Oct 26, 2026
Merged

hsluoyz merged 1 commit into
masterfrom
fix/conditional-role-matching

Conversation

@tisonkun

Copy link
Copy Markdown
Member

Registering a role-name matcher through addNamedMatchingFunc throws ClassCastException for conditional role managers. Registering it directly rebuilds the graph without condition functions or parameters, so a previously denied request can become allowed.

Dispatch to the supported role-manager implementations and rebuild conditional role graphs from explicit links while retaining all condition metadata, including conditions registered before their links. Regression tests exercise named roles with and without domains, repeated matcher registration, matcher replacement, parameter updates, and effective pattern matching.

Validation:

  • Java 8 (Zulu 8u482), Maven 3.9.12: mvn -B -ntp clean verify cobertura:cobertura -Papache-release -Drevision=1.100.0 -Dgpg.skip=true passed all 350 tests with zero failures, errors, or skips, including conformance inputs pinned to apache/casbin@c35c93d29063b8bc3bc2ef30b09943d344a289f4.
  • All five new regression cases fail on the original implementation and pass with the fix.
  • Apache RAT 0.18 and all 11 release-verification Python tests pass.

Cobertura still emits the existing source-parser diagnostics while calculating complexity; the build exits successfully.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.92%. Comparing base (38766ce) to head (e4356b7).

Files with missing lines Patch % Lines
...ain/java/org/casbin/jcasbin/main/CoreEnforcer.java 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #539      +/-   ##
==========================================
+ Coverage   70.47%   70.92%   +0.45%     
==========================================
  Files          66       66              
  Lines        3356     3357       +1     
  Branches      617      616       -1     
==========================================
+ Hits         2365     2381      +16     
+ Misses        794      781      -13     
+ Partials      197      195       -2     

☔ View full report in Codecov by Harness.
📢 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.

@tisonkun
tisonkun added this pull request to stack #541 October 26, 2026 06:11
@hsluoyz
hsluoyz self-requested a review October 26, 2026 13:14
@hsluoyz
hsluoyz merged commit 0ce7f6d into master Oct 26, 2026
1 check passed
@tisonkun
tisonkun deleted the fix/conditional-role-matching branch October 27, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants