Skip to content

Conversation

@JohnnyTank0208
Copy link

@JohnnyTank0208 JohnnyTank0208 commented Oct 9, 2025

Fix: #461

Add two methods content that addNamedGroupingPoliciesEx() and addGroupingPoliciesEx() . At the same time,unit tests were conducted and update pom central-publishing-maven-plugin to appropriate version(0.4.0).

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2025

CLA assistant check
All committers have signed the CLA.

@hsluoyz hsluoyz requested a review from Copilot October 11, 2025 17:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements two new methods addNamedGroupingPoliciesEx() and addGroupingPoliciesEx() that enhance existing policy management functionality by allowing partial success when adding multiple rules. Unlike the original methods that fail completely if any rule already exists, these "Ex" variants will skip duplicate rules and add only the new ones.

Key changes:

  • Added implementation of addNamedGroupingPoliciesEx() and addGroupingPoliciesEx() methods in both ManagementEnforcer and SyncedEnforcer
  • Created comprehensive unit tests for the new methods including edge cases with duplicate rules
  • Downgraded central-publishing-maven-plugin version from 0.6.0 to 0.4.0

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main/java/org/casbin/jcasbin/main/ManagementEnforcer.java Implements the core logic for addNamedGroupingPoliciesEx and addGroupingPoliciesEx methods
src/main/java/org/casbin/jcasbin/main/SyncedEnforcer.java Adds synchronized wrapper methods for the new Ex variants to maintain thread safety
src/test/java/org/casbin/jcasbin/main/AddNamedGroupingPoliciesExTest.java Comprehensive test suite covering all variants of the new methods
src/test/java/org/casbin/jcasbin/main/ManagementAPIUnitTest.java Extends existing test to include coverage of addNamedGroupingPoliciesEx
pom.xml Downgrades central-publishing-maven-plugin version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

* @return succeeds or not.
*/
public boolean addNamedGroupingPoliciesEx(String ptype, List<List<String>> rules) {
return addPolicies("g", ptype, rules, true);
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method passes 'g' as the first parameter to addPolicies, but this appears inconsistent with the ptype parameter. Consider adding a comment explaining why 'g' is hardcoded here or verify if ptype should be used instead.

Copilot uses AI. Check for mistakes.
@hsluoyz
Copy link
Member

hsluoyz commented Oct 11, 2025

Replaced by: #463

@hsluoyz hsluoyz closed this Oct 11, 2025
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.

[feature] sync new APIs like AddNamedGroupingPoliciesEx() from Go Casbin to jCasbin

3 participants