Copilot commented on code in PR #12622:
URL: https://github.com/apache/cloudstack/pull/12622#discussion_r2785836775
##########
engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java:
##########
@@ -600,7 +600,9 @@ public boolean equals(Object obj) {
return true;
}
- return NetUtils.isNetworkAWithinNetworkB(cidr, that.cidr);
+ return NetUtils.isNetworkAWithinNetworkB(
+
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr),
+
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr));
Review Comment:
`equals` is using `NetUtils.isNetworkAWithinNetworkB(...)`, which is not
symmetric (A within B does not imply B within A). That means `NetworkVO.equals`
can violate the Java `equals` contract, and it is also inconsistent with
`hashCode()` (which is based only on `id`). Consider switching
`equals`/`hashCode` to be `id`-based like other VOs, or (if CIDR-based equality
is truly intended) make the comparison symmetric and update `hashCode`
accordingly (e.g., based on the same normalized CIDR(s) and trafficType).
##########
engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java:
##########
@@ -600,7 +600,9 @@ public boolean equals(Object obj) {
return true;
}
- return NetUtils.isNetworkAWithinNetworkB(cidr, that.cidr);
+ return NetUtils.isNetworkAWithinNetworkB(
+
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr),
+
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr));
Review Comment:
Minor maintainability: `getFirstValueFromCommaSeparatedString(...)` is
computed twice inline. Assign the normalized CIDRs to local variables before
calling `isNetworkAWithinNetworkB` to avoid repeated splitting/trimming and
make debugging/logging easier.
```suggestion
final String normalizedCidr =
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr);
final String normalizedThatCidr =
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr);
return NetUtils.isNetworkAWithinNetworkB(normalizedCidr,
normalizedThatCidr);
```
--
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]
