Copilot commented on code in PR #687:
URL: https://github.com/apache/ranger/pull/687#discussion_r2380159511
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceACLs.java:
##########
@@ -117,46 +117,73 @@ public void finalizeAcls() {
public void setUserAccessInfo(String userName, String accessType, Integer
access, RangerPolicy policy) {
Map<String, AccessResult> userAccessInfo =
userACLs.computeIfAbsent(userName, k -> new HashMap<>());
+ AccessResult existingResult =
userAccessInfo.get(accessType);
- AccessResult accessResult = userAccessInfo.get(accessType);
-
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
-
- userAccessInfo.put(accessType, accessResult);
+ if (existingResult == null) {
+ userAccessInfo.put(accessType, new AccessResult(access, policy));
} else if (!ACCESS_CONDITIONAL.equals(access)) {
- accessResult.setResult(access);
- accessResult.setPolicy(policy);
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
}
}
public void setGroupAccessInfo(String groupName, String accessType,
Integer access, RangerPolicy policy) {
Map<String, AccessResult> groupAccessInfo =
groupACLs.computeIfAbsent(groupName, k -> new HashMap<>());
+ AccessResult existingResult =
groupAccessInfo.get(accessType);
- AccessResult accessResult = groupAccessInfo.get(accessType);
-
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
-
- groupAccessInfo.put(accessType, accessResult);
+ if (existingResult == null) {
+ groupAccessInfo.put(accessType, new AccessResult(access, policy));
} else if (!ACCESS_CONDITIONAL.equals(access)) {
- accessResult.setResult(access);
- accessResult.setPolicy(policy);
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
}
}
public void setRoleAccessInfo(String roleName, String accessType, Integer
access, RangerPolicy policy) {
Map<String, AccessResult> roleAccessInfo =
roleACLs.computeIfAbsent(roleName, k -> new HashMap<>());
+ AccessResult existingResult =
roleAccessInfo.get(accessType);
- AccessResult accessResult = roleAccessInfo.get(accessType);
+ if (existingResult == null) {
+ roleAccessInfo.put(accessType, new AccessResult(access, policy));
+ } else if (!ACCESS_CONDITIONAL.equals(access)) {
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
+ }
+ }
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
+ public void setUserAccessInfo(String userName, String accessType,
AccessResult accessResult) {
+ Map<String, AccessResult> userAccessInfo =
userACLs.computeIfAbsent(userName, k -> new HashMap<>());
+ AccessResult existingResult =
userAccessInfo.get(accessType);
+ if (existingResult == null) {
+ userAccessInfo.put(accessType, accessResult);
+ } else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
+ existingResult.setResult(accessResult.getResult());
+ existingResult.setPolicy(accessResult.getPolicy());
+ }
+ }
+
+ public void setGroupAccessInfo(String userName, String accessType,
AccessResult accessResult) {
+ Map<String, AccessResult> groupAccessInfo =
groupACLs.computeIfAbsent(userName, k -> new HashMap<>());
Review Comment:
The parameter name should be `groupName` instead of `userName` for the group
access method. This is inconsistent with the existing method signature and
could cause confusion.
```suggestion
public void setGroupAccessInfo(String groupName, String accessType,
AccessResult accessResult) {
Map<String, AccessResult> groupAccessInfo =
groupACLs.computeIfAbsent(groupName, k -> new HashMap<>());
```
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceACLs.java:
##########
@@ -117,46 +117,73 @@ public void finalizeAcls() {
public void setUserAccessInfo(String userName, String accessType, Integer
access, RangerPolicy policy) {
Map<String, AccessResult> userAccessInfo =
userACLs.computeIfAbsent(userName, k -> new HashMap<>());
+ AccessResult existingResult =
userAccessInfo.get(accessType);
- AccessResult accessResult = userAccessInfo.get(accessType);
-
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
-
- userAccessInfo.put(accessType, accessResult);
+ if (existingResult == null) {
+ userAccessInfo.put(accessType, new AccessResult(access, policy));
} else if (!ACCESS_CONDITIONAL.equals(access)) {
- accessResult.setResult(access);
- accessResult.setPolicy(policy);
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
}
}
public void setGroupAccessInfo(String groupName, String accessType,
Integer access, RangerPolicy policy) {
Map<String, AccessResult> groupAccessInfo =
groupACLs.computeIfAbsent(groupName, k -> new HashMap<>());
+ AccessResult existingResult =
groupAccessInfo.get(accessType);
- AccessResult accessResult = groupAccessInfo.get(accessType);
-
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
-
- groupAccessInfo.put(accessType, accessResult);
+ if (existingResult == null) {
+ groupAccessInfo.put(accessType, new AccessResult(access, policy));
} else if (!ACCESS_CONDITIONAL.equals(access)) {
- accessResult.setResult(access);
- accessResult.setPolicy(policy);
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
}
}
public void setRoleAccessInfo(String roleName, String accessType, Integer
access, RangerPolicy policy) {
Map<String, AccessResult> roleAccessInfo =
roleACLs.computeIfAbsent(roleName, k -> new HashMap<>());
+ AccessResult existingResult =
roleAccessInfo.get(accessType);
- AccessResult accessResult = roleAccessInfo.get(accessType);
+ if (existingResult == null) {
+ roleAccessInfo.put(accessType, new AccessResult(access, policy));
+ } else if (!ACCESS_CONDITIONAL.equals(access)) {
+ existingResult.setResult(access);
+ existingResult.setPolicy(policy);
+ }
+ }
- if (accessResult == null) {
- accessResult = new AccessResult(access, policy);
+ public void setUserAccessInfo(String userName, String accessType,
AccessResult accessResult) {
+ Map<String, AccessResult> userAccessInfo =
userACLs.computeIfAbsent(userName, k -> new HashMap<>());
+ AccessResult existingResult =
userAccessInfo.get(accessType);
+ if (existingResult == null) {
+ userAccessInfo.put(accessType, accessResult);
+ } else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
+ existingResult.setResult(accessResult.getResult());
+ existingResult.setPolicy(accessResult.getPolicy());
+ }
+ }
+
+ public void setGroupAccessInfo(String userName, String accessType,
AccessResult accessResult) {
+ Map<String, AccessResult> groupAccessInfo =
groupACLs.computeIfAbsent(userName, k -> new HashMap<>());
+ AccessResult existingResult =
groupAccessInfo.get(accessType);
+
+ if (existingResult == null) {
+ groupAccessInfo.put(accessType, accessResult);
+ } else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
+ existingResult.setResult(accessResult.getResult());
+ existingResult.setPolicy(accessResult.getPolicy());
+ }
+ }
+
+ public void setRoleAccessInfo(String userName, String accessType,
AccessResult accessResult) {
+ Map<String, AccessResult> roleAccessInfo =
roleACLs.computeIfAbsent(userName, k -> new HashMap<>());
Review Comment:
The parameter name should be `roleName` instead of `userName` for the role
access method. This is inconsistent with the existing method signature and
could cause confusion.
```suggestion
public void setRoleAccessInfo(String roleName, String accessType,
AccessResult accessResult) {
Map<String, AccessResult> roleAccessInfo =
roleACLs.computeIfAbsent(roleName, k -> new HashMap<>());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
