Skip to content

Commit 8e414a6

Browse files
shwstpprshwstppr
committed
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 <[email protected]>
1 parent f67b738 commit 8e414a6

File tree

5 files changed

+92
-80
lines changed

5 files changed

+92
-80
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public class ListPublicIpAddressesCmd extends BaseListRetrieveOnlyResourceCountC
5353
@Parameter(name = ApiConstants.ALLOCATED_ONLY, type = CommandType.BOOLEAN, description = "limits search results to allocated public IP addresses")
5454
private Boolean allocatedOnly;
5555

56-
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state")
56+
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state. A comma-separated list of states can be passed")
5757
private String state;
5858

5959
@Parameter(name = ApiConstants.FOR_VIRTUAL_NETWORK, type = CommandType.BOOLEAN, description = "the virtual network for the IP address")

‎server/src/main/java/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,19 +2422,29 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
24222422
final Long vpcId = cmd.getVpcId();
24232423

24242424
final String state = cmd.getState();
2425+
List<IpAddress.State> states = new ArrayList<>();
2426+
if (StringUtils.isNotBlank(state)) {
2427+
for (String s : StringUtils.split(state, ",")) {
2428+
try {
2429+
states.add(IpAddress.State.valueOf(s));
2430+
} catch (IllegalArgumentException e) {
2431+
throw new InvalidParameterValueException("Invalid state: " + s);
2432+
}
2433+
}
2434+
}
24252435
Boolean isAllocated = cmd.isAllocatedOnly();
24262436
if (isAllocated == null) {
2427-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
2437+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
24282438
isAllocated = Boolean.FALSE;
24292439
} else {
24302440
isAllocated = Boolean.TRUE; // default
24312441
}
24322442
} else {
2433-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
2443+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
24342444
if (isAllocated) {
24352445
throw new InvalidParameterValueException("Conflict: allocatedonly is true but state is Free");
24362446
}
2437-
} else if (state != null && state.equalsIgnoreCase(IpAddress.State.Allocated.name())) {
2447+
} else if (states.contains(IpAddress.State.Allocated)) {
24382448
isAllocated = Boolean.TRUE;
24392449
}
24402450
}
@@ -2513,10 +2523,8 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
25132523
Boolean isRecursive = cmd.isRecursive();
25142524
final List<Long> permittedAccounts = new ArrayList<>();
25152525
ListProjectResourcesCriteria listProjectResourcesCriteria = null;
2516-
boolean isAllocatedOrReserved = false;
2517-
if (isAllocated || IpAddress.State.Reserved.name().equalsIgnoreCase(state)) {
2518-
isAllocatedOrReserved = true;
2519-
}
2526+
boolean isAllocatedOrReserved = isAllocated ||
2527+
(states.size() == 1 && IpAddress.State.Reserved.equals(states.get(0)));
25202528
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
25212529
final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(),
25222530
null);
@@ -2530,7 +2538,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
25302538
buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated);
25312539

25322540
SearchCriteria<IPAddressVO> sc = sb.create();
2533-
setParameters(sc, cmd, vlanType, isAllocated);
2541+
setParameters(sc, cmd, vlanType, isAllocated, states);
25342542

25352543
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
25362544
_accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
@@ -2598,7 +2606,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
25982606
buildParameters(searchBuilder, cmd, false);
25992607

26002608
SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
2601-
setParameters(searchCriteria, cmd, vlanType, false);
2609+
setParameters(searchCriteria, cmd, vlanType, false, states);
26022610
searchCriteria.setParameters("state", IpAddress.State.Free.name());
26032611
addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
26042612
}
@@ -2611,7 +2619,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
26112619
sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN);
26122620

26132621
SearchCriteria<IPAddressVO> sc2 = sb2.create();
2614-
setParameters(sc2, cmd, vlanType, isAllocated);
2622+
setParameters(sc2, cmd, vlanType, isAllocated, states);
26152623
sc2.setParameters("ids", freeAddrIds.toArray());
26162624
_publicIpAddressDao.buildQuarantineSearchCriteria(sc2);
26172625
addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free
@@ -2641,7 +2649,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
26412649
sb.and("isSourceNat", sb.entity().isSourceNat(), SearchCriteria.Op.EQ);
26422650
sb.and("isStaticNat", sb.entity().isOneToOneNat(), SearchCriteria.Op.EQ);
26432651
sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ);
2644-
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
2652+
sb.and("state", sb.entity().getState(), SearchCriteria.Op.IN);
26452653
sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ);
26462654
sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ);
26472655

@@ -2684,7 +2692,8 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
26842692
}
26852693
}
26862694

2687-
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
2695+
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType,
2696+
Boolean isAllocated, List<IpAddress.State> states) {
26882697
final Object keyword = cmd.getKeyword();
26892698
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
26902699
final Long sourceNetworkId = cmd.getNetworkId();
@@ -2695,7 +2704,6 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
26952704
final Boolean sourceNat = cmd.isSourceNat();
26962705
final Boolean staticNat = cmd.isStaticNat();
26972706
final Boolean forDisplay = cmd.getDisplay();
2698-
final String state = cmd.getState();
26992707
final Boolean forSystemVms = cmd.getForSystemVMs();
27002708
final boolean forProvider = cmd.isForProvider();
27012709
final Map<String, String> tags = cmd.getTags();
@@ -2752,13 +2760,14 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
27522760
sc.setParameters("display", forDisplay);
27532761
}
27542762

2755-
if (state != null) {
2756-
sc.setParameters("state", state);
2763+
if (CollectionUtils.isNotEmpty(states)) {
2764+
sc.setParameters("state", states.toArray());
27572765
} else if (isAllocated != null && isAllocated) {
27582766
sc.setParameters("state", IpAddress.State.Allocated);
27592767
}
27602768

2761-
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) {
2769+
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() &&
2770+
states.contains(IpAddress.State.Free)) {
27622771
sc.setParameters(FOR_SYSTEMVMS, false);
27632772
} else {
27642773
sc.setParameters(FOR_SYSTEMVMS, forSystemVms);

‎server/src/test/java/com/cloud/server/ManagementServerImplTest.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.lang.reflect.Field;
2727
import java.util.ArrayList;
2828
import java.util.Arrays;
29+
import java.util.Collections;
2930
import java.util.List;
3031

3132
import org.apache.cloudstack.annotation.dao.AnnotationDao;
@@ -258,14 +259,14 @@ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsTrue() throws Ill
258259
Mockito.when(cmd.getId()).thenReturn(null);
259260
Mockito.when(cmd.isSourceNat()).thenReturn(null);
260261
Mockito.when(cmd.isStaticNat()).thenReturn(null);
261-
Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
262262
Mockito.when(cmd.getTags()).thenReturn(null);
263-
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
263+
List<IpAddress.State> states = Collections.singletonList(IpAddress.State.Free);
264+
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states);
264265

265266
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
266267
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
267268
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
268-
Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
269+
Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray());
269270
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
270271
}
271272

@@ -281,14 +282,14 @@ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsFalse() throws No
281282
Mockito.when(cmd.getId()).thenReturn(null);
282283
Mockito.when(cmd.isSourceNat()).thenReturn(null);
283284
Mockito.when(cmd.isStaticNat()).thenReturn(null);
284-
Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
285285
Mockito.when(cmd.getTags()).thenReturn(null);
286-
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
286+
List<IpAddress.State> states = Collections.singletonList(IpAddress.State.Free);
287+
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states);
287288

288289
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
289290
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
290291
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
291-
Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
292+
Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray());
292293
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
293294
}
294295

@@ -304,13 +305,13 @@ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsFalse() throws No
304305
Mockito.when(cmd.getId()).thenReturn(null);
305306
Mockito.when(cmd.isSourceNat()).thenReturn(null);
306307
Mockito.when(cmd.isStaticNat()).thenReturn(null);
307-
Mockito.when(cmd.getState()).thenReturn(null);
308308
Mockito.when(cmd.getTags()).thenReturn(null);
309-
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
309+
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList());
310310

311311
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
312312
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
313313
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
314+
Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated);
314315
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
315316
}
316317

@@ -326,13 +327,13 @@ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsTrue() throws NoS
326327
Mockito.when(cmd.getId()).thenReturn(null);
327328
Mockito.when(cmd.isSourceNat()).thenReturn(null);
328329
Mockito.when(cmd.isStaticNat()).thenReturn(null);
329-
Mockito.when(cmd.getState()).thenReturn(null);
330330
Mockito.when(cmd.getTags()).thenReturn(null);
331-
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
331+
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList());
332332

333333
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
334334
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
335335
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
336+
Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated);
336337
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
337338
}
338339

‎ui/src/components/widgets/InfiniteScrollSelect.vue‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
- defaultOption (Object, optional): Preselected object to include initially
4444
- showIcon (Boolean, optional): Whether to show icon for the options. Default is true
4545
- defaultIcon (String, optional): Icon to be shown when there is no resource icon for the option. Default is 'cloud-outlined'
46+
- autoSelectFirstOption (Boolean, optional): Whether to automatically select the first option when options are loaded. Default is false
4647
4748
Events:
4849
- @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 @@
8182
<resource-icon v-if="option.icon && option.icon.base64image" :image="option.icon.base64image" size="1x" style="margin-right: 5px"/>
8283
<render-icon v-else :icon="defaultIcon" style="margin-right: 5px" />
8384
</span>
84-
<span>{{ option[optionLabelKey] }}</span>
85+
<span>{{ optionLabelFn ? optionLabelFn(option) : option[optionLabelKey] }}</span>
8586
</span>
8687
</a-select-option>
8788
</a-select>
@@ -120,6 +121,10 @@ export default {
120121
type: String,
121122
default: 'name'
122123
},
124+
optionLabelFn: {
125+
type: Function,
126+
default: null
127+
},
123128
defaultOption: {
124129
type: Object,
125130
default: null
@@ -135,6 +140,10 @@ export default {
135140
pageSize: {
136141
type: Number,
137142
default: null
143+
},
144+
autoSelectFirstOption: {
145+
type: Boolean,
146+
default: false
138147
}
139148
},
140149
data () {
@@ -147,11 +156,12 @@ export default {
147156
searchTimer: null,
148157
scrollHandlerAttached: false,
149158
preselectedOptionValue: null,
150-
successiveFetches: 0
159+
successiveFetches: 0,
160+
canSelectFirstOption: false
151161
}
152162
},
153163
created () {
154-
this.addDefaultOptionIfNeeded(true)
164+
this.addDefaultOptionIfNeeded()
155165
},
156166
mounted () {
157167
this.preselectedOptionValue = this.$attrs.value
@@ -208,6 +218,7 @@ export default {
208218
}).catch(error => {
209219
this.$notifyError(error)
210220
}).finally(() => {
221+
this.canSelectFirstOption = true
211222
if (this.successiveFetches === 0) {
212223
this.loading = false
213224
}
@@ -218,6 +229,12 @@ export default {
218229
(Array.isArray(this.preselectedOptionValue) && this.preselectedOptionValue.length === 0) ||
219230
this.successiveFetches >= this.maxSuccessiveFetches) {
220231
this.resetPreselectedOptionValue()
232+
if (!this.canSelectFirstOption && this.autoSelectFirstOption && this.options.length > 0) {
233+
this.$nextTick(() => {
234+
this.preselectedOptionValue = this.options[0][this.optionValueKey]
235+
this.onChange(this.preselectedOptionValue)
236+
})
237+
}
221238
return
222239
}
223240
const matchValue = Array.isArray(this.preselectedOptionValue) ? this.preselectedOptionValue[0] : this.preselectedOptionValue
@@ -239,6 +256,7 @@ export default {
239256
},
240257
addDefaultOptionIfNeeded () {
241258
if (this.defaultOption) {
259+
this.canSelectFirstOption = true
242260
this.options.push(this.defaultOption)
243261
}
244262
},

0 commit comments

Comments
 (0)