Copilot commented on code in PR #10212:
URL: https://github.com/apache/cloudstack/pull/10212#discussion_r2393747957
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4691,23 +4691,75 @@ protected void
verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hyp
}
}
- private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<?
extends Network> networkList) {
- // Check that hostName is unique in the network domain
- Map<String, List<Long>> ntwkDomains = new HashMap<String,
List<Long>>();
+ private List<NetworkVO>
getNetworksWithSameNetworkDomainInDomains(List<NetworkVO> networkList, boolean
checkSubDomains) {
+ List<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
Review Comment:
Consider using `Collectors.toSet()` instead of `Collectors.toList()` since
the variable is named `uniqueNtwkDomains` and you likely want to eliminate
duplicates to avoid unnecessary database queries.
```suggestion
Set<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toSet());
```
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4691,23 +4691,75 @@ protected void
verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hyp
}
}
- private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<?
extends Network> networkList) {
- // Check that hostName is unique in the network domain
- Map<String, List<Long>> ntwkDomains = new HashMap<String,
List<Long>>();
+ private List<NetworkVO>
getNetworksWithSameNetworkDomainInDomains(List<NetworkVO> networkList, boolean
checkSubDomains) {
+ List<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ List<Long> domainIdList = new ArrayList<>();
for (Network network : networkList) {
+ domainIdList.add(network.getDomainId());
+ }
+ Set<Long> finalDomainIdList = new HashSet<>(domainIdList);
+ if (checkSubDomains) {
+ for (Long domainId : domainIdList) {
+ DomainVO domain = _domainDao.findById(domainId);
+ List<Long> childDomainIds =
_domainDao.getDomainChildrenIds(domain.getPath());
+ finalDomainIdList.addAll(childDomainIds);
+ }
+ }
+ return _networkDao.listByNetworkDomainsAndDomainIds(uniqueNtwkDomains,
finalDomainIdList.stream().collect(Collectors.toList()));
Review Comment:
The `finalDomainIdList` is already a Set, so calling
`stream().collect(Collectors.toList())` is unnecessary. You can pass `new
ArrayList<>(finalDomainIdList)` directly or modify the DAO method to accept
Set<Long>.
```suggestion
return
_networkDao.listByNetworkDomainsAndDomainIds(uniqueNtwkDomains, new
ArrayList<>(finalDomainIdList));
```
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4691,23 +4691,75 @@ protected void
verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hyp
}
}
- private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<?
extends Network> networkList) {
- // Check that hostName is unique in the network domain
- Map<String, List<Long>> ntwkDomains = new HashMap<String,
List<Long>>();
+ private List<NetworkVO>
getNetworksWithSameNetworkDomainInDomains(List<NetworkVO> networkList, boolean
checkSubDomains) {
+ List<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ List<Long> domainIdList = new ArrayList<>();
for (Network network : networkList) {
+ domainIdList.add(network.getDomainId());
+ }
+ Set<Long> finalDomainIdList = new HashSet<>(domainIdList);
+ if (checkSubDomains) {
+ for (Long domainId : domainIdList) {
+ DomainVO domain = _domainDao.findById(domainId);
+ List<Long> childDomainIds =
_domainDao.getDomainChildrenIds(domain.getPath());
+ finalDomainIdList.addAll(childDomainIds);
+ }
+ }
+ return _networkDao.listByNetworkDomainsAndDomainIds(uniqueNtwkDomains,
finalDomainIdList.stream().collect(Collectors.toList()));
+ }
+
+ private List<NetworkVO> getNetworksForCheckUniqueHostName(List<NetworkVO>
networkList) {
+ List<NetworkVO> finalNetworkList;
+ List<String> uniqueNtwkDomains;
+ switch (VmDistinctHostNameScope.value()) {
+ case "global":
+ uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ finalNetworkList =
_networkDao.listByNetworkDomains(uniqueNtwkDomains);
+ break;
+ case "domain":
+ finalNetworkList =
getNetworksWithSameNetworkDomainInDomains(networkList, false);
+ break;
+ case "subdomain":
+ finalNetworkList =
getNetworksWithSameNetworkDomainInDomains(networkList, true);
+ break;
+ case "account":
+ uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ List<Long> accountIds =
networkList.stream().map(Network::getAccountId).collect(Collectors.toList());
+ finalNetworkList =
_networkDao.listByNetworkDomainsAndAccountIds(uniqueNtwkDomains, accountIds);
+ break;
+ default:
+ Set<Long> vpcIds =
networkList.stream().map(Network::getVpcId).filter(Objects::nonNull).collect(Collectors.toSet());
+ finalNetworkList = new ArrayList<>(networkList);
+ for (Long vpcId : vpcIds) {
+ finalNetworkList.addAll(_networkDao.listByVpc(vpcId));
+ }
+ break;
+ }
+ return finalNetworkList;
+ }
+
+ private Map<String, Set<Long>>
getNetworkIdPerNetworkDomain(List<NetworkVO> networkList) {
+ Map<String, Set<Long>> ntwkDomains = new HashMap<>();
+
+ List<NetworkVO> updatedNetworkList =
getNetworksForCheckUniqueHostName(networkList);
+ for (Network network : updatedNetworkList) {
String ntwkDomain = network.getNetworkDomain();
+ Set<Long> ntwkIds;
if (!ntwkDomains.containsKey(ntwkDomain)) {
- List<Long> ntwkIds = new ArrayList<Long>();
- ntwkIds.add(network.getId());
- ntwkDomains.put(ntwkDomain, ntwkIds);
+ ntwkIds = new HashSet<>();
} else {
- List<Long> ntwkIds = ntwkDomains.get(ntwkDomain);
- ntwkIds.add(network.getId());
- ntwkDomains.put(ntwkDomain, ntwkIds);
+ ntwkIds = ntwkDomains.get(ntwkDomain);
}
+ ntwkIds.add(network.getId());
+ ntwkDomains.put(ntwkDomain, ntwkIds);
}
+ return ntwkDomains;
+ }
- for (Entry<String, List<Long>> ntwkDomain : ntwkDomains.entrySet()) {
+ private void checkIfHostNameUniqueInNtwkDomain(String hostName,
List<NetworkVO> networkList) {
+ // Check that hostName is unique
Review Comment:
[nitpick] The method parameter type changed from `List<? extends Network>`
to `List<NetworkVO>`, but the method name and comment still reference
'NtwkDomain' (network domain). Consider updating the method name to
`checkIfHostNameUniqueInScope` to better reflect its expanded functionality.
```suggestion
/**
* Checks that the given hostName is unique within the relevant scope
(global, domain, subdomain, account, or VPC),
* as determined by configuration, across the provided list of networks.
*/
private void checkIfHostNameUniqueInScope(String hostName,
List<NetworkVO> networkList) {
```
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4691,23 +4691,75 @@ protected void
verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hyp
}
}
- private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<?
extends Network> networkList) {
- // Check that hostName is unique in the network domain
- Map<String, List<Long>> ntwkDomains = new HashMap<String,
List<Long>>();
+ private List<NetworkVO>
getNetworksWithSameNetworkDomainInDomains(List<NetworkVO> networkList, boolean
checkSubDomains) {
+ List<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ List<Long> domainIdList = new ArrayList<>();
for (Network network : networkList) {
+ domainIdList.add(network.getDomainId());
+ }
+ Set<Long> finalDomainIdList = new HashSet<>(domainIdList);
+ if (checkSubDomains) {
+ for (Long domainId : domainIdList) {
+ DomainVO domain = _domainDao.findById(domainId);
+ List<Long> childDomainIds =
_domainDao.getDomainChildrenIds(domain.getPath());
+ finalDomainIdList.addAll(childDomainIds);
+ }
+ }
+ return _networkDao.listByNetworkDomainsAndDomainIds(uniqueNtwkDomains,
finalDomainIdList.stream().collect(Collectors.toList()));
+ }
+
+ private List<NetworkVO> getNetworksForCheckUniqueHostName(List<NetworkVO>
networkList) {
+ List<NetworkVO> finalNetworkList;
+ List<String> uniqueNtwkDomains;
+ switch (VmDistinctHostNameScope.value()) {
+ case "global":
+ uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
Review Comment:
The same stream operation
`networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList())`
is duplicated in the 'global' and 'account' cases. Extract this to a variable
at the method level to avoid code duplication.
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4691,23 +4691,75 @@ protected void
verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hyp
}
}
- private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<?
extends Network> networkList) {
- // Check that hostName is unique in the network domain
- Map<String, List<Long>> ntwkDomains = new HashMap<String,
List<Long>>();
+ private List<NetworkVO>
getNetworksWithSameNetworkDomainInDomains(List<NetworkVO> networkList, boolean
checkSubDomains) {
+ List<String> uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ List<Long> domainIdList = new ArrayList<>();
for (Network network : networkList) {
+ domainIdList.add(network.getDomainId());
+ }
+ Set<Long> finalDomainIdList = new HashSet<>(domainIdList);
+ if (checkSubDomains) {
+ for (Long domainId : domainIdList) {
+ DomainVO domain = _domainDao.findById(domainId);
+ List<Long> childDomainIds =
_domainDao.getDomainChildrenIds(domain.getPath());
+ finalDomainIdList.addAll(childDomainIds);
+ }
+ }
+ return _networkDao.listByNetworkDomainsAndDomainIds(uniqueNtwkDomains,
finalDomainIdList.stream().collect(Collectors.toList()));
+ }
+
+ private List<NetworkVO> getNetworksForCheckUniqueHostName(List<NetworkVO>
networkList) {
+ List<NetworkVO> finalNetworkList;
+ List<String> uniqueNtwkDomains;
+ switch (VmDistinctHostNameScope.value()) {
+ case "global":
+ uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
+ finalNetworkList =
_networkDao.listByNetworkDomains(uniqueNtwkDomains);
+ break;
+ case "domain":
+ finalNetworkList =
getNetworksWithSameNetworkDomainInDomains(networkList, false);
+ break;
+ case "subdomain":
+ finalNetworkList =
getNetworksWithSameNetworkDomainInDomains(networkList, true);
+ break;
+ case "account":
+ uniqueNtwkDomains =
networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList());
Review Comment:
The same stream operation
`networkList.stream().map(NetworkVO::getNetworkDomain).collect(Collectors.toList())`
is duplicated in the 'global' and 'account' cases. Extract this to a variable
at the method level to avoid code duplication.
--
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]
