Skip to content

Commit 9ec8cc4

Browse files
shwstpprshwstppr
authored
api,server,ui: improve listing public ip for associate (apache#11591)
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent dba889e commit 9ec8cc4

File tree

6 files changed

+146
-82
lines changed

6 files changed

+146
-82
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: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,7 @@
823823
import com.cloud.user.dao.SSHKeyPairDao;
824824
import com.cloud.user.dao.UserDao;
825825
import com.cloud.user.dao.UserDataDao;
826+
import com.cloud.utils.EnumUtils;
826827
import com.cloud.utils.NumbersUtil;
827828
import com.cloud.utils.Pair;
828829
import com.cloud.utils.PasswordGenerator;
@@ -2419,6 +2420,22 @@ public Pair<List<? extends ConfigurationGroup>, Integer> listConfigurationGroups
24192420
return new Pair<>(result.first(), result.second());
24202421
}
24212422

2423+
protected List<IpAddress.State> getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) {
2424+
final String statesStr = cmd.getState();
2425+
final List<IpAddress.State> states = new ArrayList<>();
2426+
if (StringUtils.isBlank(statesStr)) {
2427+
return states;
2428+
}
2429+
for (String s : StringUtils.split(statesStr, ",")) {
2430+
IpAddress.State state = EnumUtils.getEnumIgnoreCase(IpAddress.State.class, s.trim());
2431+
if (state == null) {
2432+
throw new InvalidParameterValueException("Invalid state: " + s);
2433+
}
2434+
states.add(state);
2435+
}
2436+
return states;
2437+
}
2438+
24222439
@Override
24232440
public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) {
24242441
final Long associatedNetworkId = cmd.getAssociatedNetworkId();
@@ -2429,20 +2446,20 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
24292446
final Long networkId = cmd.getNetworkId();
24302447
final Long vpcId = cmd.getVpcId();
24312448

2432-
final String state = cmd.getState();
2449+
final List<IpAddress.State> states = getStatesForIpAddressSearch(cmd);
24332450
Boolean isAllocated = cmd.isAllocatedOnly();
24342451
if (isAllocated == null) {
2435-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
2452+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
24362453
isAllocated = Boolean.FALSE;
24372454
} else {
24382455
isAllocated = Boolean.TRUE; // default
24392456
}
24402457
} else {
2441-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
2458+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
24422459
if (isAllocated) {
24432460
throw new InvalidParameterValueException("Conflict: allocatedonly is true but state is Free");
24442461
}
2445-
} else if (state != null && state.equalsIgnoreCase(IpAddress.State.Allocated.name())) {
2462+
} else if (states.contains(IpAddress.State.Allocated)) {
24462463
isAllocated = Boolean.TRUE;
24472464
}
24482465
}
@@ -2521,10 +2538,8 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
25212538
Boolean isRecursive = cmd.isRecursive();
25222539
final List<Long> permittedAccounts = new ArrayList<>();
25232540
ListProjectResourcesCriteria listProjectResourcesCriteria = null;
2524-
boolean isAllocatedOrReserved = false;
2525-
if (isAllocated || IpAddress.State.Reserved.name().equalsIgnoreCase(state)) {
2526-
isAllocatedOrReserved = true;
2527-
}
2541+
boolean isAllocatedOrReserved = isAllocated ||
2542+
(states.size() == 1 && IpAddress.State.Reserved.equals(states.get(0)));
25282543
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
25292544
final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(),
25302545
null);
@@ -2538,7 +2553,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
25382553
buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated);
25392554

25402555
SearchCriteria<IPAddressVO> sc = sb.create();
2541-
setParameters(sc, cmd, vlanType, isAllocated);
2556+
setParameters(sc, cmd, vlanType, isAllocated, states);
25422557

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

26082623
SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
2609-
setParameters(searchCriteria, cmd, vlanType, false);
2624+
setParameters(searchCriteria, cmd, vlanType, false, states);
26102625
searchCriteria.setParameters("state", IpAddress.State.Free.name());
26112626
addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
26122627
}
@@ -2619,7 +2634,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
26192634
sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN);
26202635

26212636
SearchCriteria<IPAddressVO> sc2 = sb2.create();
2622-
setParameters(sc2, cmd, vlanType, isAllocated);
2637+
setParameters(sc2, cmd, vlanType, isAllocated, states);
26232638
sc2.setParameters("ids", freeAddrIds.toArray());
26242639
_publicIpAddressDao.buildQuarantineSearchCriteria(sc2);
26252640
addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free
@@ -2649,7 +2664,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
26492664
sb.and("isSourceNat", sb.entity().isSourceNat(), SearchCriteria.Op.EQ);
26502665
sb.and("isStaticNat", sb.entity().isOneToOneNat(), SearchCriteria.Op.EQ);
26512666
sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ);
2652-
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
2667+
sb.and("state", sb.entity().getState(), SearchCriteria.Op.IN);
26532668
sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ);
26542669
sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ);
26552670

@@ -2692,7 +2707,8 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
26922707
}
26932708
}
26942709

2695-
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
2710+
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType,
2711+
Boolean isAllocated, List<IpAddress.State> states) {
26962712
final Object keyword = cmd.getKeyword();
26972713
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
26982714
final Long sourceNetworkId = cmd.getNetworkId();
@@ -2703,7 +2719,6 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
27032719
final Boolean sourceNat = cmd.isSourceNat();
27042720
final Boolean staticNat = cmd.isStaticNat();
27052721
final Boolean forDisplay = cmd.getDisplay();
2706-
final String state = cmd.getState();
27072722
final Boolean forSystemVms = cmd.getForSystemVMs();
27082723
final boolean forProvider = cmd.isForProvider();
27092724
final Map<String, String> tags = cmd.getTags();
@@ -2760,13 +2775,14 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
27602775
sc.setParameters("display", forDisplay);
27612776
}
27622777

2763-
if (state != null) {
2764-
sc.setParameters("state", state);
2778+
if (CollectionUtils.isNotEmpty(states)) {
2779+
sc.setParameters("state", states.toArray());
27652780
} else if (isAllocated != null && isAllocated) {
27662781
sc.setParameters("state", IpAddress.State.Allocated);
27672782
}
27682783

2769-
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) {
2784+
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() &&
2785+
states.contains(IpAddress.State.Free)) {
27702786
sc.setParameters(FOR_SYSTEMVMS, false);
27712787
} else {
27722788
sc.setParameters(FOR_SYSTEMVMS, forSystemVms);

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

Lines changed: 56 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

@@ -1033,4 +1034,49 @@ public void testGetExternalVmConsole() {
10331034
Assert.assertNotNull(spy.getExternalVmConsole(virtualMachine, host));
10341035
Mockito.verify(extensionManager).getInstanceConsole(virtualMachine, host);
10351036
}
1037+
1038+
@Test
1039+
public void getStatesForIpAddressSearchReturnsValidStates() {
1040+
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
1041+
Mockito.when(cmd.getState()).thenReturn("Allocated ,free");
1042+
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
1043+
Assert.assertEquals(2, result.size());
1044+
Assert.assertTrue(result.contains(IpAddress.State.Allocated));
1045+
Assert.assertTrue(result.contains(IpAddress.State.Free));
1046+
}
1047+
1048+
@Test
1049+
public void getStatesForIpAddressSearchReturnsEmptyListForNullState() {
1050+
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
1051+
Mockito.when(cmd.getState()).thenReturn(null);
1052+
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
1053+
Assert.assertTrue(result.isEmpty());
1054+
}
1055+
1056+
@Test
1057+
public void getStatesForIpAddressSearchReturnsEmptyListForBlankState() {
1058+
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
1059+
Mockito.when(cmd.getState()).thenReturn(" ");
1060+
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
1061+
Assert.assertTrue(result.isEmpty());
1062+
}
1063+
1064+
@Test(expected = InvalidParameterValueException.class)
1065+
public void getStatesForIpAddressSearchThrowsExceptionForInvalidState() {
1066+
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
1067+
Mockito.when(cmd.getState()).thenReturn("InvalidState");
1068+
spy.getStatesForIpAddressSearch(cmd);
1069+
}
1070+
1071+
@Test
1072+
public void getStatesForIpAddressSearchHandlesMixedValidAndInvalidStates() {
1073+
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
1074+
Mockito.when(cmd.getState()).thenReturn("Allocated,InvalidState");
1075+
try {
1076+
spy.getStatesForIpAddressSearch(cmd);
1077+
Assert.fail("Expected InvalidParameterValueException to be thrown");
1078+
} catch (InvalidParameterValueException e) {
1079+
Assert.assertEquals("Invalid state: InvalidState", e.getMessage());
1080+
}
1081+
}
10361082
}

‎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)