Copilot commented on code in PR #11767:
URL: https://github.com/apache/cloudstack/pull/11767#discussion_r2393692681
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -2813,13 +2813,17 @@ private void
addDomainNetworksByDomainPathToSearch(SearchCriteria<NetworkVO> add
SearchCriteria<NetworkVO> domainSC = sb.create();
domainSC.addAnd("id", SearchCriteria.Op.IN, networkIds.toArray());
domainSC.addAnd("aclType", SearchCriteria.Op.EQ,
ACLType.Domain.toString());
- addProjectNetworksConditionToSearch(domainSC, true);
+ addProjectNetworksConditionToSearch(domainSC, true, null);
additionalSC.addOr("id", SearchCriteria.Op.SC, domainSC);
}
}
Review Comment:
The method signature change from private to protected and adding a Long
parameter is a breaking change for subclasses. Consider maintaining backward
compatibility by overloading the method instead of changing the existing
signature.
```suggestion
/**
* Original method signature for backward compatibility.
*/
private void
addProjectNetworksConditionToSearch(SearchCriteria<NetworkVO> sc, boolean
skipProjectNetworks) {
sc.getJoin("account").addAnd("type", skipProjectNetworks ? Op.NEQ :
Op.EQ, Account.Type.PROJECT);
sc.addAnd("id", Op.SC, sc.getJoin("account"));
}
/**
* Overloaded method with additional projectId parameter.
*/
```
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -2813,13 +2813,17 @@ private void
addDomainNetworksByDomainPathToSearch(SearchCriteria<NetworkVO> add
SearchCriteria<NetworkVO> domainSC = sb.create();
domainSC.addAnd("id", SearchCriteria.Op.IN, networkIds.toArray());
domainSC.addAnd("aclType", SearchCriteria.Op.EQ,
ACLType.Domain.toString());
- addProjectNetworksConditionToSearch(domainSC, true);
+ addProjectNetworksConditionToSearch(domainSC, true, null);
additionalSC.addOr("id", SearchCriteria.Op.SC, domainSC);
}
}
- private void addProjectNetworksConditionToSearch(SearchCriteria<NetworkVO>
sc, boolean skipProjectNetworks) {
- sc.getJoin("account").addAnd("type", skipProjectNetworks ? Op.NEQ :
Op.EQ, Account.Type.PROJECT);
+ protected void
addProjectNetworksConditionToSearch(SearchCriteria<NetworkVO> sc, boolean
skipProjectNetworks, Long projectId) {
+ if (!skipProjectNetworks && projectId == -1) {
Review Comment:
The comparison `projectId == -1` will fail if projectId is null. Since the
method signature accepts `Long projectId` (which can be null), this should use
`Long.valueOf(-1L).equals(projectId)` or add a null check first.
```suggestion
if (!skipProjectNetworks && Long.valueOf(-1L).equals(projectId)) {
```
--
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]
