Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use rtype to get request tokens in CoreEnforcer #355

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

pzet123
Copy link
Contributor

@pzet123 pzet123 commented Aug 15, 2023

From my understanding, the CoreEnforcer class was previously using the number of tokens within different request types to determine which request type that user has passed in. This causes issues when you have multiple request types with the same number of tokens.

I've changed this so that the rtype is used to determine from which request type the tokens should be retrieved, similarly to how the policy tokens are retrieved.

The message being passed into CasbinMatcherException was also incorrect as it referenced rvals but was really getting length values from pvals

Some minor spelling mistakes fixed too.

@casbin-bot
Copy link
Member

@tangyang9464 @imp2002 please review

@codecov-commenter
Copy link

Codecov Report

Merging #355 (40023da) into master (cf4dd31) will decrease coverage by 0.22%.
Report is 1 commits behind head on master.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
- Coverage   69.58%   69.36%   -0.22%     
==========================================
  Files          49       49              
  Lines        2229     2236       +7     
  Branches      399      398       -1     
==========================================
  Hits         1551     1551              
- Misses        567      574       +7     
  Partials      111      111              
Files Changed Coverage Δ
...ain/java/org/casbin/jcasbin/main/CoreEnforcer.java 77.05% <60.00%> (-0.27%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hsluoyz hsluoyz merged commit 8c50f92 into casbin:master Aug 15, 2023
1 check passed
@github-actions
Copy link

🎉 This PR is included in version 1.37.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants