Skip to content

Commit b090f2a

Browse files
Copilothsluoyz
andcommitted
Remove reflection dependency and support generic RoleManager
- Added getRoleGraph() method to RoleManager interface with default implementation - Implemented getRoleGraph() in DefaultRoleManager with defensive copy - Removed reflection-based buildGraph() method from DefaultDetector - Removed instanceof check to support any RoleManager implementation - Updated test to reflect new behavior (now accepts any RoleManager) Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
1 parent 77291ae commit b090f2a

File tree

4 files changed

+36
-41
lines changed

4 files changed

+36
-41
lines changed

src/main/java/org/casbin/jcasbin/detector/DefaultDetector.java

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package org.casbin.jcasbin.detector;
1616

17-
import org.casbin.jcasbin.rbac.DefaultRoleManager;
1817
import org.casbin.jcasbin.rbac.RoleManager;
1918

2019
import java.util.*;
@@ -32,15 +31,8 @@ public class DefaultDetector implements Detector {
3231
*/
3332
@Override
3433
public String check(RoleManager rm) {
35-
if (!(rm instanceof DefaultRoleManager)) {
36-
throw new IllegalArgumentException("DefaultDetector only supports DefaultRoleManager");
37-
}
38-
39-
DefaultRoleManager drm = (DefaultRoleManager) rm;
40-
4134
// Build adjacency list from the role manager
42-
// Using local data structures to avoid sharing references with RoleManager's internal state
43-
Map<String, List<String>> graph = buildGraph(drm);
35+
Map<String, List<String>> graph = rm.getRoleGraph();
4436

4537
// Perform DFS to detect cycles
4638
Set<String> visited = new HashSet<>();
@@ -59,35 +51,6 @@ public String check(RoleManager rm) {
5951
return null;
6052
}
6153

62-
/**
63-
* Builds a directed graph (adjacency list) from the DefaultRoleManager.
64-
* Each role points to the roles it inherits (its parent roles).
65-
*/
66-
private Map<String, List<String>> buildGraph(DefaultRoleManager drm) {
67-
Map<String, List<String>> graph = new HashMap<>();
68-
69-
try {
70-
// Use reflection to access the package-private allRoles field
71-
java.lang.reflect.Field allRolesField = DefaultRoleManager.class.getDeclaredField("allRoles");
72-
allRolesField.setAccessible(true);
73-
@SuppressWarnings("unchecked")
74-
Map<String, ?> allRoles = (Map<String, ?>) allRolesField.get(drm);
75-
76-
// Iterate through all roles and get their parent roles
77-
for (String roleName : allRoles.keySet()) {
78-
List<String> parentRoles = drm.getRoles(roleName);
79-
graph.put(roleName, new ArrayList<>(parentRoles));
80-
}
81-
} catch (NoSuchFieldException e) {
82-
throw new RuntimeException("Failed to access 'allRoles' field in DefaultRoleManager via reflection. " +
83-
"The field may have been renamed or removed.", e);
84-
} catch (IllegalAccessException e) {
85-
throw new RuntimeException("Failed to access 'allRoles' field in DefaultRoleManager via reflection. " +
86-
"Permission denied to access the field.", e);
87-
}
88-
89-
return graph;
90-
}
9154

9255
/**
9356
* Performs depth-first search to detect cycles in the graph using an iterative approach.

src/main/java/org/casbin/jcasbin/rbac/DefaultRoleManager.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,4 +268,21 @@ public String toString() {
268268
public void printRoles() {
269269
Util.logPrintfInfo("{}", this);
270270
}
271+
272+
/**
273+
* getRoleGraph returns the dependency graph (adjacency list) of roles.
274+
* Each key represents a role name, and the corresponding value is a list of role names
275+
* that the key role inherits from (parent roles).
276+
*
277+
* @return a defensive copy of the role inheritance graph
278+
*/
279+
@Override
280+
public Map<String, List<String>> getRoleGraph() {
281+
Map<String, List<String>> graph = new HashMap<>();
282+
for (String roleName : allRoles.keySet()) {
283+
List<String> parentRoles = getRoles(roleName);
284+
graph.put(roleName, new ArrayList<>(parentRoles));
285+
}
286+
return graph;
287+
}
271288
}

src/main/java/org/casbin/jcasbin/rbac/RoleManager.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
package org.casbin.jcasbin.rbac;
1616

17+
import java.util.Collections;
1718
import java.util.List;
19+
import java.util.Map;
1820

1921
public interface RoleManager {
2022
/**
@@ -75,4 +77,15 @@ public interface RoleManager {
7577
* printRoles prints all the roles to log.
7678
*/
7779
void printRoles();
80+
81+
/**
82+
* getRoleGraph returns the dependency graph (adjacency list) of roles.
83+
* Each key represents a role name, and the corresponding value is a list of role names
84+
* that the key role inherits from (parent roles).
85+
*
86+
* @return a map representing the role inheritance graph, or an empty map if not supported.
87+
*/
88+
default Map<String, List<String>> getRoleGraph() {
89+
return Collections.emptyMap();
90+
}
7891
}

src/test/java/org/casbin/jcasbin/main/DefaultDetectorTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,10 @@ public void testLargeGraphWithCycle() {
214214
assertTrue("Result should contain 'Cycle detected'", result.contains("Cycle detected:"));
215215
}
216216

217-
@Test(expected = IllegalArgumentException.class)
217+
@Test
218218
public void testUnsupportedRoleManager() {
219-
// Test with a RoleManager that is not DefaultRoleManager
219+
// Test with a RoleManager that uses the default getRoleGraph() implementation
220+
// The default implementation returns an empty map, so no cycles should be detected
220221
RoleManager rm = new RoleManager() {
221222
@Override
222223
public void clear() {}
@@ -247,7 +248,8 @@ public void printRoles() {}
247248
};
248249

249250
Detector detector = new DefaultDetector();
250-
detector.check(rm); // Should throw IllegalArgumentException
251+
String result = detector.check(rm);
252+
assertNull("Expected no cycle with default getRoleGraph() implementation", result);
251253
}
252254

253255
@Test

0 commit comments

Comments
 (0)