Copilot commented on code in PR #13022:
URL: https://github.com/apache/cloudstack/pull/13022#discussion_r3153097633
##########
server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java:
##########
@@ -168,9 +168,9 @@ public ProjectRole findProjectRole(Long roleId, Long
projectId) {
@Override
public List<ProjectRole> findProjectRoles(Long projectId, String keyword) {
- if (projectId == null || projectId < 1L ||
projectDao.findById(projectId) == null) {
- logger.warn("Invalid project ID provided");
- return null;
+ if (projectId == null) {
+ logger.warn("Invalid project ID provided; thus, an empty list is
being returned.");
+ return Collections.emptyList();
}
return ListUtils.toListOfInterface(projRoleDao.findAllRoles(projectId,
keyword));
Review Comment:
`findProjectRoles` no longer validates `projectId` values other than null
(it used to reject `< 1` and non-existent/removed projects). This makes its
behavior inconsistent with `findProjectRole` and can return roles for a removed
project ID. Consider restoring the previous validation (`projectId < 1L ||
projectDao.findById(projectId) == null`) but returning
`Collections.emptyList()` instead of `null` to avoid NPEs while keeping the
guardrail.
--
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]
