Skip to content

Commit fc4ebaa

Browse files
alegueleresalegueleres
authored
fix: DistributedEnforcer addPolicySelf and InternalEnforcer to use dispatcher in methods (casbin#282)
* fix: DistributedEnforcer.addPolicySelf was not inserting new rules * fix: Adjusted InternalEnforcer methods to use dispatcher when dispatcher is configured. * fix: fix codebeat issues * fix: codebeat issues * fix: code review suggestion to change CoreEnforcer.mustUseDispatcher to be protected instead of public.
1 parent e777aec commit fc4ebaa

File tree

5 files changed

+187
-23
lines changed

5 files changed

+187
-23
lines changed

‎src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java

+4
Original file line numberDiff line numberDiff line change
@@ -709,4 +709,8 @@ public boolean isAutoNotifyDispatcher() {
709709
public void setAutoNotifyDispatcher(boolean autoNotifyDispatcher) {
710710
this.autoNotifyDispatcher = autoNotifyDispatcher;
711711
}
712+
713+
protected boolean mustUseDispatcher() {
714+
return this.dispatcher != null && this.autoNotifyDispatcher;
715+
}
712716
}

‎src/main/java/org/casbin/jcasbin/main/DistributedEnforcer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public DistributedEnforcer(String modelPath, String policyFile, boolean enableLo
110110
public List<List<String>> addPolicySelf(BooleanSupplier shouldPersist, String sec, String ptype, List<List<String>> rules) {
111111
List<List<String>> noExistsPolicy = new ArrayList<>();
112112
for (List<String> rule : rules) {
113-
if (this.model.hasPolicy(sec, ptype, rule)) {
113+
if (!this.model.hasPolicy(sec, ptype, rule)) {
114114
noExistsPolicy.add(rule);
115115
}
116116
}

‎src/main/java/org/casbin/jcasbin/main/InternalEnforcer.java

+46-21
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.util.ArrayList;
2626
import java.util.List;
2727

28+
import static java.util.Collections.singletonList;
29+
2830
/**
2931
* InternalEnforcer = CoreEnforcer + Internal API.
3032
*/
@@ -33,6 +35,11 @@ class InternalEnforcer extends CoreEnforcer {
3335
* addPolicy adds a rule to the current policy.
3436
*/
3537
boolean addPolicy(String sec, String ptype, List<String> rule) {
38+
if (mustUseDispatcher()) {
39+
dispatcher.addPolicies(sec, ptype, singletonList(rule));
40+
return true;
41+
}
42+
3643
if (model.hasPolicy(sec, ptype, rule)) {
3744
return false;
3845
}
@@ -50,11 +57,7 @@ boolean addPolicy(String sec, String ptype, List<String> rule) {
5057

5158
model.addPolicy(sec, ptype, rule);
5259

53-
if (sec.equals("g")) {
54-
List<List<String>> rules = new ArrayList<>();
55-
rules.add(rule);
56-
buildIncrementalRoleLinks(Model.PolicyOperations.POLICY_ADD, ptype, rules);
57-
}
60+
buildIncrementalRoleLinks(sec, ptype, singletonList(rule), Model.PolicyOperations.POLICY_ADD);
5861

5962
if (watcher != null && autoNotifyWatcher) {
6063
if (watcher instanceof WatcherEx) {
@@ -71,6 +74,11 @@ boolean addPolicy(String sec, String ptype, List<String> rule) {
7174
* addPolicies adds rules to the current policy.
7275
*/
7376
boolean addPolicies(String sec, String ptype, List<List<String>> rules) {
77+
if (mustUseDispatcher()) {
78+
dispatcher.addPolicies(sec, ptype, rules);
79+
return true;
80+
}
81+
7482
if (model.hasPolicies(sec, ptype, rules)) {
7583
return false;
7684
}
@@ -90,9 +98,7 @@ boolean addPolicies(String sec, String ptype, List<List<String>> rules) {
9098

9199
model.addPolicies(sec, ptype, rules);
92100

93-
if (sec.equals("g")) {
94-
buildIncrementalRoleLinks(Model.PolicyOperations.POLICY_ADD, ptype, rules);
95-
}
101+
buildIncrementalRoleLinks(sec, ptype, rules, Model.PolicyOperations.POLICY_ADD);
96102

97103
if (watcher != null && autoNotifyWatcher) {
98104
watcher.update();
@@ -115,6 +121,11 @@ public void buildIncrementalRoleLinks(Model.PolicyOperations op, String ptype, L
115121
* removePolicy removes a rule from the current policy.
116122
*/
117123
boolean removePolicy(String sec, String ptype, List<String> rule) {
124+
if (mustUseDispatcher()) {
125+
dispatcher.removePolicies(sec, ptype, singletonList(rule));
126+
return true;
127+
}
128+
118129
if (adapter != null && autoSave) {
119130
try {
120131
adapter.removePolicy(sec, ptype, rule);
@@ -132,11 +143,7 @@ boolean removePolicy(String sec, String ptype, List<String> rule) {
132143
return false;
133144
}
134145

135-
if (sec.equals("g")) {
136-
List<List<String>> rules = new ArrayList<>();
137-
rules.add(rule);
138-
buildIncrementalRoleLinks(Model.PolicyOperations.POLICY_REMOVE, ptype, rules);
139-
}
146+
buildIncrementalRoleLinks(sec, ptype, singletonList(rule), Model.PolicyOperations.POLICY_REMOVE);
140147

141148
if (watcher != null && autoNotifyWatcher) {
142149
if (watcher instanceof WatcherEx) {
@@ -159,7 +166,7 @@ boolean removePolicy(String sec, String ptype, List<String> rule) {
159166
* @return succeeds or not.
160167
*/
161168
boolean updatePolicy(String sec, String ptype, List<String> oldRule, List<String> newRule) {
162-
if (dispatcher != null && autoNotifyDispatcher) {
169+
if (mustUseDispatcher()) {
163170
dispatcher.updatePolicy(sec, ptype, oldRule, newRule);
164171
return true;
165172
}
@@ -225,6 +232,11 @@ boolean updatePolicy(String sec, String ptype, List<String> oldRule, List<String
225232
* removePolicies removes rules from the current policy.
226233
*/
227234
boolean removePolicies(String sec, String ptype, List<List<String>> rules) {
235+
if (mustUseDispatcher()) {
236+
dispatcher.removePolicies(sec, ptype, rules);
237+
return true;
238+
}
239+
228240
if (!model.hasPolicies(sec, ptype, rules)) {
229241
return false;
230242
}
@@ -248,9 +260,7 @@ boolean removePolicies(String sec, String ptype, List<List<String>> rules) {
248260
return false;
249261
}
250262

251-
if (sec.equals("g")) {
252-
buildIncrementalRoleLinks(Model.PolicyOperations.POLICY_REMOVE, ptype, rules);
253-
}
263+
buildIncrementalRoleLinks(sec, ptype, rules, Model.PolicyOperations.POLICY_REMOVE);
254264

255265
if (watcher != null && autoNotifyWatcher) {
256266
// error intentionally ignored
@@ -264,8 +274,13 @@ boolean removePolicies(String sec, String ptype, List<List<String>> rules) {
264274
* removeFilteredPolicy removes rules based on field filters from the current policy.
265275
*/
266276
boolean removeFilteredPolicy(String sec, String ptype, int fieldIndex, String... fieldValues) {
277+
if (mustUseDispatcher()) {
278+
dispatcher.removeFilteredPolicy(sec, ptype, fieldIndex, fieldValues);
279+
return true;
280+
}
281+
267282
if (fieldValues == null || fieldValues.length == 0) {
268-
Util.logPrint("Invaild fieldValues parameter");
283+
Util.logPrint("Invalid fieldValues parameter");
269284
return false;
270285
}
271286

@@ -287,9 +302,7 @@ boolean removeFilteredPolicy(String sec, String ptype, int fieldIndex, String...
287302
return false;
288303
}
289304

290-
if (sec.equals("g")) {
291-
buildIncrementalRoleLinks(Model.PolicyOperations.POLICY_REMOVE, ptype, effects);
292-
}
305+
buildIncrementalRoleLinks(sec, ptype, effects, Model.PolicyOperations.POLICY_REMOVE);
293306

294307
if (watcher != null && autoNotifyWatcher) {
295308
// error intentionally ignored
@@ -315,4 +328,16 @@ int getDomainIndex(String ptype) {
315328
}
316329
return index;
317330
}
331+
332+
private void buildIncrementalRoleLinks(
333+
final String sec,
334+
final String ptype,
335+
final List<List<String>> rules,
336+
final Model.PolicyOperations operation
337+
) {
338+
if ("g".equals(sec)) {
339+
buildIncrementalRoleLinks(operation, ptype, rules);
340+
}
341+
}
342+
318343
}

‎src/test/java/org/casbin/jcasbin/main/InternalEnforcerWithDispatcherTest.java

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
package org.casbin.jcasbin.main;
2+
3+
import org.casbin.jcasbin.persist.Dispatcher;
4+
import org.junit.Before;
5+
import org.junit.Test;
6+
import sun.reflect.generics.reflectiveObjects.NotImplementedException;
7+
8+
import java.util.List;
9+
10+
import static java.util.Arrays.asList;
11+
import static java.util.Collections.singletonList;
12+
import static org.junit.Assert.assertArrayEquals;
13+
import static org.junit.Assert.assertEquals;
14+
import static org.junit.Assert.assertTrue;
15+
16+
public class InternalEnforcerWithDispatcherTest {
17+
18+
private final static String SEC = "expected-sec";
19+
20+
private final static String PTYPE = "expected-ptype";
21+
22+
private final static List<String> RULE = asList("expected-new-rule-1", "expected-new-rule-2");
23+
24+
private final static List<String> OLD_RULE = asList("expected-old-rule-1", "expected-old-rule-2");
25+
26+
private final static int FIELD_INDEX = 0;
27+
28+
private final static String[] FIELD_VALUES = new String[1];
29+
30+
private InternalEnforcer enforcer;
31+
32+
@Before
33+
public void setUp() {
34+
this.enforcer = new InternalEnforcer();
35+
this.enforcer.setDispatcher(new CustomDispatcher());
36+
this.enforcer.setAutoNotifyDispatcher(true);
37+
}
38+
39+
@Test
40+
public void testAddPolicy() {
41+
boolean result = enforcer.addPolicy(SEC, PTYPE, RULE);
42+
assertTrue(result);
43+
}
44+
45+
@Test
46+
public void testAddPolicies() {
47+
boolean result = enforcer.addPolicies(SEC, PTYPE, singletonList(RULE));
48+
assertTrue(result);
49+
}
50+
51+
@Test
52+
public void testRemovePolicy() {
53+
boolean result = enforcer.removePolicy(SEC, PTYPE, RULE);
54+
assertTrue(result);
55+
}
56+
57+
@Test
58+
public void testRemovePolicies() {
59+
boolean result = enforcer.removePolicies(SEC, PTYPE, singletonList(RULE));
60+
assertTrue(result);
61+
}
62+
63+
@Test
64+
public void testRemoveFilteredPolicy() {
65+
boolean result = enforcer.removeFilteredPolicy(SEC, PTYPE, FIELD_INDEX, FIELD_VALUES);
66+
assertTrue(result);
67+
}
68+
69+
@Test
70+
public void testUpdatePolicy() {
71+
boolean result = enforcer.updatePolicy(SEC, PTYPE, OLD_RULE, RULE);
72+
assertTrue(result);
73+
}
74+
75+
private static class CustomDispatcher implements Dispatcher {
76+
77+
@Override
78+
public void addPolicies(
79+
final String sec,
80+
final String ptype,
81+
final List<List<String>> rules
82+
) {
83+
assertEquals(SEC, sec);
84+
assertEquals(PTYPE, ptype);
85+
assertEquals(singletonList(RULE), rules);
86+
}
87+
88+
@Override
89+
public void removePolicies(
90+
final String sec,
91+
final String ptype,
92+
final List<List<String>> rules
93+
) {
94+
assertEquals(SEC, sec);
95+
assertEquals(PTYPE, ptype);
96+
assertEquals(singletonList(RULE), rules);
97+
}
98+
99+
@Override
100+
public void removeFilteredPolicy(
101+
final String sec,
102+
final String ptype,
103+
final int fieldIndex,
104+
final String... fieldValues
105+
) {
106+
assertEquals(SEC, sec);
107+
assertEquals(PTYPE, ptype);
108+
assertEquals(FIELD_INDEX, fieldIndex);
109+
assertArrayEquals(FIELD_VALUES, fieldValues);
110+
}
111+
112+
@Override
113+
public void clearPolicy() {
114+
throw new NotImplementedException();
115+
}
116+
117+
@Override
118+
public void updatePolicy(
119+
final String sec,
120+
final String ptype,
121+
final List<String> oldRule,
122+
final List<String> newRule
123+
) {
124+
assertEquals(SEC, sec);
125+
assertEquals(PTYPE, ptype);
126+
assertEquals(OLD_RULE, oldRule);
127+
assertEquals(RULE, newRule);
128+
}
129+
}
130+
}

‎src/test/java/org/casbin/jcasbin/main/SyncedDistributedAPIUnitTest.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ public void testDistributedAPI() {
3030
asList("bob", "data2", "write"),
3131
asList("data2_admin", "data2", "read"),
3232
asList("data2_admin", "data2", "write")));
33-
de.addPolicySelf(() -> false, "g", "g", asList(asList("alice", "data2_admin")));
33+
de.addPolicySelf(() -> false, "g", "g", asList(
34+
asList("alice", "data2_admin"),
35+
asList("new_user", "data2_admin")
36+
));
3437

3538
testEnforce(de, "alice", "data1", "read", true);
3639
testEnforce(de, "alice", "data1", "write", false);
@@ -40,6 +43,8 @@ public void testDistributedAPI() {
4043
testEnforce(de, "data2_admin", "data2", "write", true);
4144
testEnforce(de, "alice", "data2", "read", true);
4245
testEnforce(de, "alice", "data2", "write", true);
46+
testEnforce(de, "new_user", "data2", "write", true);
47+
testEnforce(de, "new_user", "data2", "read", true);
4348

4449
de.updatePolicySelf(() -> false, "p", "p", asList("alice", "data1", "read"), asList("alice", "data1", "write"));
4550
de.updatePolicySelf(() -> false, "g", "g", asList("alice", "data2_admin"), asList("tom", "alice"));

0 commit comments

Comments
 (0)