shwstppr commented on a change in pull request #4675:
URL: https://github.com/apache/cloudstack/pull/4675#discussion_r596611620
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -2004,42 +2023,101 @@ private void
abstractDataStoreClustersList(List<StoragePool> storagePools, List<
@Override
public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final
ListPublicIpAddressesCmd cmd) {
- final Object keyword = cmd.getKeyword();
- final Long physicalNetworkId = cmd.getPhysicalNetworkId();
final Long associatedNetworkId = cmd.getAssociatedNetworkId();
- final Long sourceNetworkId = cmd.getNetworkId();
final Long zone = cmd.getZoneId();
- final String address = cmd.getIpAddress();
final Long vlan = cmd.getVlanId();
final Boolean forVirtualNetwork = cmd.isForVirtualNetwork();
- final Boolean forLoadBalancing = cmd.isForLoadBalancing();
final Long ipId = cmd.getId();
- final Boolean sourceNat = cmd.isSourceNat();
- final Boolean staticNat = cmd.isStaticNat();
+ final Long networkId = cmd.getNetworkId();
final Long vpcId = cmd.getVpcId();
- final Boolean forDisplay = cmd.getDisplay();
- final Map<String, String> tags = cmd.getTags();
final String state = cmd.getState();
Boolean isAllocated = cmd.isAllocatedOnly();
if (isAllocated == null) {
- isAllocated = Boolean.TRUE;
-
- if (state != null) {
+ if (state != null &&
state.equalsIgnoreCase(IpAddress.State.Free.name())) {
isAllocated = Boolean.FALSE;
+ } else {
+ isAllocated = Boolean.TRUE; // default
+ }
+ } else {
+ if (state != null &&
state.equalsIgnoreCase(IpAddress.State.Free.name())) {
+ if (isAllocated) {
+ throw new InvalidParameterValueException("Conflict:
allocatedonly is true but state is Free");
+ }
+ } else if (state != null &&
state.equalsIgnoreCase(IpAddress.State.Allocated.name())) {
+ isAllocated = Boolean.TRUE;
Review comment:
@ravening what if API was passed `allocatedonly=False` and
`state=Allocated`? Shouldn't we raise exception like above case
(allocatedonly=true & state=Free)
This block can be simplified (or maybe complicated :smile:) with something
like,
```
if (Boolean.TRUE.equals(isAllocated) &&
IpAddress.State.Free.name().equals(state)) {
throw new InvalidParameterValueException("Conflict:
allocatedonly is true but state is Free");
}
if (Boolean.FALSE.equals(isAllocated) &&
IpAddress.State.Allocated.name().equals(state)) {
throw new InvalidParameterValueException("Conflict:
allocatedonly is false but state is Allocated");
}
if (isAllocated == null) {
isAllocated = !IpAddress.State.Free.name().equals(state);
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
