From 8e414a66d8a17fe8add9cd919f045f602a0c5b4a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 5 Sep 2025 22:25:08 +0530 Subject: [PATCH 1/6] api,server,ui: improve listing public ip for associate Currently, acquire or associate IP action in the UI will load only 500 addresses. ALso, it will load all 500 of them together. This PR uses InfiniteScrollSelect to iteratively load more addresses. To facilitate retrieving both Free and Reserved addresses api and server change has been made for the listPublicIpAddresses API to pass a comma-separated list of states for state parameter. Signed-off-by: Abhishek Kumar --- .../address/ListPublicIpAddressesCmd.java | 2 +- .../cloud/server/ManagementServerImpl.java | 41 +++++---- .../server/ManagementServerImplTest.java | 21 ++--- .../widgets/InfiniteScrollSelect.vue | 24 +++++- ui/src/views/network/IpAddressesTab.vue | 84 ++++++++----------- 5 files changed, 92 insertions(+), 80 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java index 357f0c83ed74..57fd733d51d2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java @@ -53,7 +53,7 @@ public class ListPublicIpAddressesCmd extends BaseListRetrieveOnlyResourceCountC @Parameter(name = ApiConstants.ALLOCATED_ONLY, type = CommandType.BOOLEAN, description = "limits search results to allocated public IP addresses") private Boolean allocatedOnly; - @Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state") + @Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state. A comma-separated list of states can be passed") private String state; @Parameter(name = ApiConstants.FOR_VIRTUAL_NETWORK, type = CommandType.BOOLEAN, description = "the virtual network for the IP address") diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 1b96cd4ef15b..07f84162bdb6 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -2422,19 +2422,29 @@ public Pair, Integer> searchForIPAddresses(final ListP final Long vpcId = cmd.getVpcId(); final String state = cmd.getState(); + List states = new ArrayList<>(); + if (StringUtils.isNotBlank(state)) { + for (String s : StringUtils.split(state, ",")) { + try { + states.add(IpAddress.State.valueOf(s)); + } catch (IllegalArgumentException e) { + throw new InvalidParameterValueException("Invalid state: " + s); + } + } + } Boolean isAllocated = cmd.isAllocatedOnly(); if (isAllocated == null) { - if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) { + if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) { isAllocated = Boolean.FALSE; } else { isAllocated = Boolean.TRUE; // default } } else { - if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) { + if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) { if (isAllocated) { throw new InvalidParameterValueException("Conflict: allocatedonly is true but state is Free"); } - } else if (state != null && state.equalsIgnoreCase(IpAddress.State.Allocated.name())) { + } else if (states.contains(IpAddress.State.Allocated)) { isAllocated = Boolean.TRUE; } } @@ -2513,10 +2523,8 @@ public Pair, Integer> searchForIPAddresses(final ListP Boolean isRecursive = cmd.isRecursive(); final List permittedAccounts = new ArrayList<>(); ListProjectResourcesCriteria listProjectResourcesCriteria = null; - boolean isAllocatedOrReserved = false; - if (isAllocated || IpAddress.State.Reserved.name().equalsIgnoreCase(state)) { - isAllocatedOrReserved = true; - } + boolean isAllocatedOrReserved = isAllocated || + (states.size() == 1 && IpAddress.State.Reserved.equals(states.get(0))); if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) { final Ternary domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(), null); @@ -2530,7 +2538,7 @@ public Pair, Integer> searchForIPAddresses(final ListP buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated); SearchCriteria sc = sb.create(); - setParameters(sc, cmd, vlanType, isAllocated); + setParameters(sc, cmd, vlanType, isAllocated, states); if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) { _accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); @@ -2598,7 +2606,7 @@ public Pair, Integer> searchForIPAddresses(final ListP buildParameters(searchBuilder, cmd, false); SearchCriteria searchCriteria = searchBuilder.create(); - setParameters(searchCriteria, cmd, vlanType, false); + setParameters(searchCriteria, cmd, vlanType, false, states); searchCriteria.setParameters("state", IpAddress.State.Free.name()); addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network } @@ -2611,7 +2619,7 @@ public Pair, Integer> searchForIPAddresses(final ListP sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN); SearchCriteria sc2 = sb2.create(); - setParameters(sc2, cmd, vlanType, isAllocated); + setParameters(sc2, cmd, vlanType, isAllocated, states); sc2.setParameters("ids", freeAddrIds.toArray()); _publicIpAddressDao.buildQuarantineSearchCriteria(sc2); addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free @@ -2641,7 +2649,7 @@ private void buildParameters(final SearchBuilder sb, final ListPubl sb.and("isSourceNat", sb.entity().isSourceNat(), SearchCriteria.Op.EQ); sb.and("isStaticNat", sb.entity().isOneToOneNat(), SearchCriteria.Op.EQ); sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ); - sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + sb.and("state", sb.entity().getState(), SearchCriteria.Op.IN); sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ); sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ); @@ -2684,7 +2692,8 @@ private void buildParameters(final SearchBuilder sb, final ListPubl } } - protected void setParameters(SearchCriteria sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) { + protected void setParameters(SearchCriteria sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, + Boolean isAllocated, List states) { final Object keyword = cmd.getKeyword(); final Long physicalNetworkId = cmd.getPhysicalNetworkId(); final Long sourceNetworkId = cmd.getNetworkId(); @@ -2695,7 +2704,6 @@ protected void setParameters(SearchCriteria sc, final ListPublicIpA final Boolean sourceNat = cmd.isSourceNat(); final Boolean staticNat = cmd.isStaticNat(); final Boolean forDisplay = cmd.getDisplay(); - final String state = cmd.getState(); final Boolean forSystemVms = cmd.getForSystemVMs(); final boolean forProvider = cmd.isForProvider(); final Map tags = cmd.getTags(); @@ -2752,13 +2760,14 @@ protected void setParameters(SearchCriteria sc, final ListPublicIpA sc.setParameters("display", forDisplay); } - if (state != null) { - sc.setParameters("state", state); + if (CollectionUtils.isNotEmpty(states)) { + sc.setParameters("state", states.toArray()); } else if (isAllocated != null && isAllocated) { sc.setParameters("state", IpAddress.State.Allocated); } - if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) { + if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && + states.contains(IpAddress.State.Free)) { sc.setParameters(FOR_SYSTEMVMS, false); } else { sc.setParameters(FOR_SYSTEMVMS, forSystemVms); diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index ebced92f8fe9..1def79cd5f54 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -26,6 +26,7 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -258,14 +259,14 @@ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsTrue() throws Ill Mockito.when(cmd.getId()).thenReturn(null); Mockito.when(cmd.isSourceNat()).thenReturn(null); Mockito.when(cmd.isStaticNat()).thenReturn(null); - Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name()); Mockito.when(cmd.getTags()).thenReturn(null); - spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE); + List states = Collections.singletonList(IpAddress.State.Free); + spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states); Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork); Mockito.verify(sc, Mockito.times(1)).setParameters("display", false); Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L); - Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free"); + Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray()); Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false); } @@ -281,14 +282,14 @@ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsFalse() throws No Mockito.when(cmd.getId()).thenReturn(null); Mockito.when(cmd.isSourceNat()).thenReturn(null); Mockito.when(cmd.isStaticNat()).thenReturn(null); - Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name()); Mockito.when(cmd.getTags()).thenReturn(null); - spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE); + List states = Collections.singletonList(IpAddress.State.Free); + spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states); Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork); Mockito.verify(sc, Mockito.times(1)).setParameters("display", false); Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L); - Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free"); + Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray()); Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false); } @@ -304,13 +305,13 @@ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsFalse() throws No Mockito.when(cmd.getId()).thenReturn(null); Mockito.when(cmd.isSourceNat()).thenReturn(null); Mockito.when(cmd.isStaticNat()).thenReturn(null); - Mockito.when(cmd.getState()).thenReturn(null); Mockito.when(cmd.getTags()).thenReturn(null); - spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE); + spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList()); Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork); Mockito.verify(sc, Mockito.times(1)).setParameters("display", false); Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L); + Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated); Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false); } @@ -326,13 +327,13 @@ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsTrue() throws NoS Mockito.when(cmd.getId()).thenReturn(null); Mockito.when(cmd.isSourceNat()).thenReturn(null); Mockito.when(cmd.isStaticNat()).thenReturn(null); - Mockito.when(cmd.getState()).thenReturn(null); Mockito.when(cmd.getTags()).thenReturn(null); - spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE); + spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList()); Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork); Mockito.verify(sc, Mockito.times(1)).setParameters("display", false); Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L); + Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated); Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false); } diff --git a/ui/src/components/widgets/InfiniteScrollSelect.vue b/ui/src/components/widgets/InfiniteScrollSelect.vue index 608eeebf1332..122feafb2a00 100644 --- a/ui/src/components/widgets/InfiniteScrollSelect.vue +++ b/ui/src/components/widgets/InfiniteScrollSelect.vue @@ -43,6 +43,7 @@ - defaultOption (Object, optional): Preselected object to include initially - showIcon (Boolean, optional): Whether to show icon for the options. Default is true - defaultIcon (String, optional): Icon to be shown when there is no resource icon for the option. Default is 'cloud-outlined' + - autoSelectFirstOption (Boolean, optional): Whether to automatically select the first option when options are loaded. Default is false Events: - @change-option-value (Function): Emits the selected option value(s) when value(s) changes. Do not use @change as it will give warnings and may not work @@ -81,7 +82,7 @@ - {{ option[optionLabelKey] }} + {{ optionLabelFn ? optionLabelFn(option) : option[optionLabelKey] }} @@ -120,6 +121,10 @@ export default { type: String, default: 'name' }, + optionLabelFn: { + type: Function, + default: null + }, defaultOption: { type: Object, default: null @@ -135,6 +140,10 @@ export default { pageSize: { type: Number, default: null + }, + autoSelectFirstOption: { + type: Boolean, + default: false } }, data () { @@ -147,11 +156,12 @@ export default { searchTimer: null, scrollHandlerAttached: false, preselectedOptionValue: null, - successiveFetches: 0 + successiveFetches: 0, + canSelectFirstOption: false } }, created () { - this.addDefaultOptionIfNeeded(true) + this.addDefaultOptionIfNeeded() }, mounted () { this.preselectedOptionValue = this.$attrs.value @@ -208,6 +218,7 @@ export default { }).catch(error => { this.$notifyError(error) }).finally(() => { + this.canSelectFirstOption = true if (this.successiveFetches === 0) { this.loading = false } @@ -218,6 +229,12 @@ export default { (Array.isArray(this.preselectedOptionValue) && this.preselectedOptionValue.length === 0) || this.successiveFetches >= this.maxSuccessiveFetches) { this.resetPreselectedOptionValue() + if (!this.canSelectFirstOption && this.autoSelectFirstOption && this.options.length > 0) { + this.$nextTick(() => { + this.preselectedOptionValue = this.options[0][this.optionValueKey] + this.onChange(this.preselectedOptionValue) + }) + } return } const matchValue = Array.isArray(this.preselectedOptionValue) ? this.preselectedOptionValue[0] : this.preselectedOptionValue @@ -239,6 +256,7 @@ export default { }, addDefaultOptionIfNeeded () { if (this.defaultOption) { + this.canSelectFirstOption = true this.options.push(this.defaultOption) } }, diff --git a/ui/src/views/network/IpAddressesTab.vue b/ui/src/views/network/IpAddressesTab.vue index ee9d1eff42bc..37ebc29f19d9 100644 --- a/ui/src/views/network/IpAddressesTab.vue +++ b/ui/src/views/network/IpAddressesTab.vue @@ -148,20 +148,17 @@ - - {{ ip.ipaddress }} ({{ ip.state }}) - + api="listPublicIpAddresses" + :apiParams="listApiParamsForAssociate" + resourceType="publicipaddress" + optionValueKey="ipaddress" + :optionLabelFn="ip => ip.ipaddress + ' (' + ip.state + ')'" + defaultIcon="environment-outlined" + :autoSelectFirstOption="true" + @change-option-value="(ip) => acquireIp = ip" />
{{ $t('label.cancel') }} @@ -212,13 +209,15 @@ import Status from '@/components/widgets/Status' import TooltipButton from '@/components/widgets/TooltipButton' import BulkActionView from '@/components/view/BulkActionView' import eventBus from '@/config/eventBus' +import InfiniteScrollSelect from '@/components/widgets/InfiniteScrollSelect' export default { name: 'IpAddressesTab', components: { Status, TooltipButton, - BulkActionView + BulkActionView, + InfiniteScrollSelect }, props: { resource: { @@ -281,7 +280,6 @@ export default { showAcquireIp: false, acquireLoading: false, acquireIp: null, - listPublicIpAddress: [], changeSourceNat: false, zoneExtNetProvider: '' } @@ -302,6 +300,26 @@ export default { } }, inject: ['parentFetchData'], + computed: { + listApiParams () { + const params = { + zoneid: this.resource.zoneid, + domainid: this.resource.domainid, + account: this.resource.account, + forvirtualnetwork: true, + allocatedonly: false + } + if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) { + params.forprovider = true + } + return params + }, + listApiParamsForAssociate () { + const params = this.listApiParams + params.state = 'Free,Reserved' + return params + } + }, methods: { fetchData () { const params = { @@ -344,19 +362,9 @@ export default { }).catch(reject) }) }, - fetchListPublicIpAddress () { + fetchListPublicIpAddress (state) { return new Promise((resolve, reject) => { - const params = { - zoneid: this.resource.zoneid, - domainid: this.resource.domainid, - account: this.resource.account, - forvirtualnetwork: true, - allocatedonly: false - } - if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) { - params.forprovider = true - } - getAPI('listPublicIpAddresses', params).then(json => { + getAPI('listPublicIpAddresses', this.listApiParams).then(json => { const listPublicIps = json.listpublicipaddressesresponse.publicipaddress || [] resolve(listPublicIps) }).catch(reject) @@ -554,30 +562,6 @@ export default { }, async onShowAcquireIp () { this.showAcquireIp = true - this.acquireLoading = true - this.listPublicIpAddress = [] - - try { - const listPublicIpAddress = await this.fetchListPublicIpAddress() - listPublicIpAddress.forEach(item => { - if (item.state === 'Free' || item.state === 'Reserved') { - this.listPublicIpAddress.push({ - ipaddress: item.ipaddress, - state: item.state - }) - } - }) - this.listPublicIpAddress.sort(function (a, b) { - if (a.ipaddress < b.ipaddress) { return -1 } - if (a.ipaddress > b.ipaddress) { return 1 } - return 0 - }) - this.acquireIp = this.listPublicIpAddress && this.listPublicIpAddress.length > 0 ? this.listPublicIpAddress[0].ipaddress : null - this.acquireLoading = false - } catch (e) { - this.acquireLoading = false - this.$notifyError(e) - } }, onCloseModal () { this.showAcquireIp = false From 0e2fec1985a9cad7293a20bd27033471b53d2a2a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 10 Oct 2025 10:04:23 +0530 Subject: [PATCH 2/6] refactor Signed-off-by: Abhishek Kumar --- .../cloud/server/ManagementServerImpl.java | 27 ++++++----- .../server/ManagementServerImplTest.java | 45 +++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 07f84162bdb6..e5ecebea28dc 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -2411,18 +2411,9 @@ public Pair, Integer> listConfigurationGroups return new Pair<>(result.first(), result.second()); } - @Override - public Pair, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) { - final Long associatedNetworkId = cmd.getAssociatedNetworkId(); - final Long zone = cmd.getZoneId(); - final Long vlan = cmd.getVlanId(); - final Boolean forVirtualNetwork = cmd.isForVirtualNetwork(); - final Long ipId = cmd.getId(); - final Long networkId = cmd.getNetworkId(); - final Long vpcId = cmd.getVpcId(); - + protected List getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) { final String state = cmd.getState(); - List states = new ArrayList<>(); + final List states = new ArrayList<>(); if (StringUtils.isNotBlank(state)) { for (String s : StringUtils.split(state, ",")) { try { @@ -2432,6 +2423,20 @@ public Pair, Integer> searchForIPAddresses(final ListP } } } + return states; + } + + @Override + public Pair, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) { + final Long associatedNetworkId = cmd.getAssociatedNetworkId(); + final Long zone = cmd.getZoneId(); + final Long vlan = cmd.getVlanId(); + final Boolean forVirtualNetwork = cmd.isForVirtualNetwork(); + final Long ipId = cmd.getId(); + final Long networkId = cmd.getNetworkId(); + final Long vpcId = cmd.getVpcId(); + + final List states = getStatesForIpAddressSearch(cmd); Boolean isAllocated = cmd.isAllocatedOnly(); if (isAllocated == null) { if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) { diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index 1def79cd5f54..327b0a5b5576 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -1034,4 +1034,49 @@ public void testGetExternalVmConsole() { Assert.assertNotNull(spy.getExternalVmConsole(virtualMachine, host)); Mockito.verify(extensionManager).getInstanceConsole(virtualMachine, host); } + + @Test + public void getStatesForIpAddressSearchReturnsValidStates() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn("Allocated,Free"); + List result = spy.getStatesForIpAddressSearch(cmd); + Assert.assertEquals(2, result.size()); + Assert.assertTrue(result.contains(IpAddress.State.Allocated)); + Assert.assertTrue(result.contains(IpAddress.State.Free)); + } + + @Test + public void getStatesForIpAddressSearchReturnsEmptyListForNullState() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn(null); + List result = spy.getStatesForIpAddressSearch(cmd); + Assert.assertTrue(result.isEmpty()); + } + + @Test + public void getStatesForIpAddressSearchReturnsEmptyListForBlankState() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn(" "); + List result = spy.getStatesForIpAddressSearch(cmd); + Assert.assertTrue(result.isEmpty()); + } + + @Test(expected = InvalidParameterValueException.class) + public void getStatesForIpAddressSearchThrowsExceptionForInvalidState() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn("InvalidState"); + spy.getStatesForIpAddressSearch(cmd); + } + + @Test + public void getStatesForIpAddressSearchHandlesMixedValidAndInvalidStates() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn("Allocated,InvalidState"); + try { + spy.getStatesForIpAddressSearch(cmd); + Assert.fail("Expected InvalidParameterValueException to be thrown"); + } catch (InvalidParameterValueException e) { + Assert.assertEquals("Invalid state: InvalidState", e.getMessage()); + } + } } From cb1c0aae6df041ba25330f9585cada832e16c5e0 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 10 Oct 2025 18:17:43 +0530 Subject: [PATCH 3/6] change Signed-off-by: Abhishek Kumar --- .../com/cloud/server/ManagementServerImpl.java | 18 ++++++++++-------- .../cloud/server/ManagementServerImplTest.java | 2 +- .../main/java/com/cloud/utils/EnumUtils.java | 12 ++++++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index e5ecebea28dc..5dfc84945c59 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -822,6 +822,7 @@ import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.user.dao.UserDao; import com.cloud.user.dao.UserDataDao; +import com.cloud.utils.EnumUtils; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.PasswordGenerator; @@ -2412,16 +2413,17 @@ public Pair, Integer> listConfigurationGroups } protected List getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) { - final String state = cmd.getState(); + final String statesStr = cmd.getState(); final List states = new ArrayList<>(); - if (StringUtils.isNotBlank(state)) { - for (String s : StringUtils.split(state, ",")) { - try { - states.add(IpAddress.State.valueOf(s)); - } catch (IllegalArgumentException e) { - throw new InvalidParameterValueException("Invalid state: " + s); - } + if (StringUtils.isBlank(statesStr)) { + return states; + } + for (String s : StringUtils.split(statesStr, ",")) { + IpAddress.State state = EnumUtils.getEnumIgnoreCase(IpAddress.State.class, s.trim()); + if (state == null) { + throw new InvalidParameterValueException("Invalid state: " + s); } + states.add(state); } return states; } diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index 327b0a5b5576..2f52df982b7a 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -1038,7 +1038,7 @@ public void testGetExternalVmConsole() { @Test public void getStatesForIpAddressSearchReturnsValidStates() { ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); - Mockito.when(cmd.getState()).thenReturn("Allocated,Free"); + Mockito.when(cmd.getState()).thenReturn("Allocated ,free"); List result = spy.getStatesForIpAddressSearch(cmd); Assert.assertEquals(2, result.size()); Assert.assertTrue(result.contains(IpAddress.State.Allocated)); diff --git a/utils/src/main/java/com/cloud/utils/EnumUtils.java b/utils/src/main/java/com/cloud/utils/EnumUtils.java index 02b6a25b8955..f62beb7c5de3 100644 --- a/utils/src/main/java/com/cloud/utils/EnumUtils.java +++ b/utils/src/main/java/com/cloud/utils/EnumUtils.java @@ -55,4 +55,16 @@ public static > T fromString(Class clz, String value) { } return null; } + + public static > T getEnumIgnoreCase(Class enumClass, String name) { + if (enumClass == null || name == null) { + return null; + } + for (T constant : enumClass.getEnumConstants()) { + if (constant.name().equalsIgnoreCase(name.trim())) { + return constant; + } + } + return null; + } } From 42e8a73dd03b666f1d828a309dc73237022baf68 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 13 Oct 2025 16:39:15 +0530 Subject: [PATCH 4/6] user commons Signed-off-by: Abhishek Kumar --- .../java/com/cloud/server/ManagementServerImpl.java | 2 +- utils/src/main/java/com/cloud/utils/EnumUtils.java | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 5dfc84945c59..c536c2f83b90 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -659,6 +659,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -822,7 +823,6 @@ import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.user.dao.UserDao; import com.cloud.user.dao.UserDataDao; -import com.cloud.utils.EnumUtils; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.PasswordGenerator; diff --git a/utils/src/main/java/com/cloud/utils/EnumUtils.java b/utils/src/main/java/com/cloud/utils/EnumUtils.java index f62beb7c5de3..02b6a25b8955 100644 --- a/utils/src/main/java/com/cloud/utils/EnumUtils.java +++ b/utils/src/main/java/com/cloud/utils/EnumUtils.java @@ -55,16 +55,4 @@ public static > T fromString(Class clz, String value) { } return null; } - - public static > T getEnumIgnoreCase(Class enumClass, String name) { - if (enumClass == null || name == null) { - return null; - } - for (T constant : enumClass.getEnumConstants()) { - if (constant.name().equalsIgnoreCase(name.trim())) { - return constant; - } - } - return null; - } } From 89eadcbf684b50050f22382aaea00e34512e753c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 13 Nov 2025 21:58:45 +0530 Subject: [PATCH 5/6] wrap commons EnumUtils Signed-off-by: Abhishek Kumar --- .../main/java/com/cloud/server/ManagementServerImpl.java | 2 +- utils/src/main/java/com/cloud/utils/EnumUtils.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index d31cac22ba80..e6032662e926 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -659,7 +659,6 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; -import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -824,6 +823,7 @@ import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.user.dao.UserDao; import com.cloud.user.dao.UserDataDao; +import com.cloud.utils.EnumUtils; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.PasswordGenerator; diff --git a/utils/src/main/java/com/cloud/utils/EnumUtils.java b/utils/src/main/java/com/cloud/utils/EnumUtils.java index 02b6a25b8955..c5cf01679ec1 100644 --- a/utils/src/main/java/com/cloud/utils/EnumUtils.java +++ b/utils/src/main/java/com/cloud/utils/EnumUtils.java @@ -55,4 +55,12 @@ public static > T fromString(Class clz, String value) { } return null; } + + public static > T getEnumIgnoreCase(final Class enumClass, final String enumName) { + return org.apache.commons.lang3.EnumUtils.getEnumIgnoreCase(enumClass, enumName); + } + + public static > T getEnumIgnoreCase(final Class enumClass, final String enumName, T defaultValue) { + return org.apache.commons.lang3.EnumUtils.getEnumIgnoreCase(enumClass, enumName, defaultValue); + } } From 136b6ac39795e9eb4a91a01a1a185d29feab215f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Nov 2025 09:28:33 +0530 Subject: [PATCH 6/6] change Signed-off-by: Abhishek Kumar --- utils/src/main/java/com/cloud/utils/EnumUtils.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/utils/src/main/java/com/cloud/utils/EnumUtils.java b/utils/src/main/java/com/cloud/utils/EnumUtils.java index c5cf01679ec1..380b595a0ad1 100644 --- a/utils/src/main/java/com/cloud/utils/EnumUtils.java +++ b/utils/src/main/java/com/cloud/utils/EnumUtils.java @@ -19,7 +19,7 @@ package com.cloud.utils; -public class EnumUtils { +public class EnumUtils extends org.apache.commons.lang3.EnumUtils { public static String listValues(Enum[] enums) { StringBuilder b = new StringBuilder("["); @@ -55,12 +55,4 @@ public static > T fromString(Class clz, String value) { } return null; } - - public static > T getEnumIgnoreCase(final Class enumClass, final String enumName) { - return org.apache.commons.lang3.EnumUtils.getEnumIgnoreCase(enumClass, enumName); - } - - public static > T getEnumIgnoreCase(final Class enumClass, final String enumName, T defaultValue) { - return org.apache.commons.lang3.EnumUtils.getEnumIgnoreCase(enumClass, enumName, defaultValue); - } }