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

feat: support AutoSave in built-in FileAdapter #391

Merged
merged 5 commits into from
Mar 18, 2024

Conversation

imp2002
Copy link
Member

@imp2002 imp2002 commented Mar 13, 2024

No description provided.

@casbin-bot
Copy link
Member

@tangyang9464 please review

@casbin-bot casbin-bot requested a review from tangyang9464 March 13, 2024 07:08
@imp2002
Copy link
Member Author

imp2002 commented Mar 13, 2024

Because the previous unit tests defaulted to a File Adapter that didn't implement addPolicy() and removePolicy(), ignoring the impact of autoSave, implementing addPolicy() and removePolicy() now results in inconsistent expect. Therefore, it is necessary to modify the previous tests.

For example:
image
Following the implementation of File Adapter's addPolicy() method, the policy will change when autoSave is enabled.

@imp2002 imp2002 marked this pull request as draft March 13, 2024 07:46
@hsluoyz
Copy link
Member

hsluoyz commented Mar 13, 2024

@imp2002 you can keep that existing test in AutoSave OFF mode. Then add a new test case which enables AutoSave

@imp2002 imp2002 force-pushed the feat/csv_adapter_autosave branch from dec5076 to d1e5f28 Compare March 17, 2024 14:44
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2024

Codecov Report

Attention: Patch coverage is 52.38095% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 69.04%. Comparing base (c56af29) to head (1afb96a).

Files Patch % Lines
...sbin/jcasbin/persist/file_adapter/FileAdapter.java 52.38% 7 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
- Coverage   69.38%   69.04%   -0.34%     
==========================================
  Files          53       53              
  Lines        2401     2420      +19     
  Branches      423      426       +3     
==========================================
+ Hits         1666     1671       +5     
- Misses        621      634      +13     
- Partials      114      115       +1     

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

@imp2002 imp2002 marked this pull request as ready for review March 17, 2024 14:46
@hsluoyz hsluoyz changed the title feat: implement FileAdapter addPolicy(), removePolicy() feat: support AutoSave in built-in FileAdapter Mar 17, 2024
@hsluoyz hsluoyz merged commit c7d1c21 into casbin:master Mar 18, 2024
4 of 5 checks passed
Copy link

🎉 This PR is included in version 1.54.0 🎉

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