Skip to content

Commit ad6d364

Browse files
shwstpprdhslove
shwstppr
authored and
dhslove
committed
api,server,ui: improve listing public ip for associate (apache#11591)
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 70e81a0 commit ad6d364

6 files changed

Lines changed: 146 additions & 82 deletions

File tree

‎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
@@ -909,6 +909,7 @@
909909
import com.cloud.user.dao.SSHKeyPairDao;
910910
import com.cloud.user.dao.UserDao;
911911
import com.cloud.user.dao.UserDataDao;
912+
import com.cloud.utils.EnumUtils;
912913
import com.cloud.utils.NumbersUtil;
913914
import com.cloud.utils.Pair;
914915
import com.cloud.utils.PasswordGenerator;
@@ -5156,6 +5157,22 @@ public Pair<List<? extends ConfigurationGroup>, Integer> listConfigurationGroups
51565157
return new Pair<>(result.first(), result.second());
51575158
}
51585159

5160+
protected List<IpAddress.State> getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) {
5161+
final String statesStr = cmd.getState();
5162+
final List<IpAddress.State> states = new ArrayList<>();
5163+
if (StringUtils.isBlank(statesStr)) {
5164+
return states;
5165+
}
5166+
for (String s : StringUtils.split(statesStr, ",")) {
5167+
IpAddress.State state = EnumUtils.getEnumIgnoreCase(IpAddress.State.class, s.trim());
5168+
if (state == null) {
5169+
throw new InvalidParameterValueException("Invalid state: " + s);
5170+
}
5171+
states.add(state);
5172+
}
5173+
return states;
5174+
}
5175+
51595176
@Override
51605177
public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) {
51615178
final Long associatedNetworkId = cmd.getAssociatedNetworkId();
@@ -5166,20 +5183,20 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
51665183
final Long networkId = cmd.getNetworkId();
51675184
final Long vpcId = cmd.getVpcId();
51685185

5169-
final String state = cmd.getState();
5186+
final List<IpAddress.State> states = getStatesForIpAddressSearch(cmd);
51705187
Boolean isAllocated = cmd.isAllocatedOnly();
51715188
if (isAllocated == null) {
5172-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
5189+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
51735190
isAllocated = Boolean.FALSE;
51745191
} else {
51755192
isAllocated = Boolean.TRUE; // default
51765193
}
51775194
} else {
5178-
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
5195+
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
51795196
if (isAllocated) {
51805197
throw new InvalidParameterValueException("Conflict: allocatedonly is true but state is Free");
51815198
}
5182-
} else if (state != null && state.equalsIgnoreCase(IpAddress.State.Allocated.name())) {
5199+
} else if (states.contains(IpAddress.State.Allocated)) {
51835200
isAllocated = Boolean.TRUE;
51845201
}
51855202
}
@@ -5258,10 +5275,8 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
52585275
Boolean isRecursive = cmd.isRecursive();
52595276
final List<Long> permittedAccounts = new ArrayList<>();
52605277
ListProjectResourcesCriteria listProjectResourcesCriteria = null;
5261-
Boolean isAllocatedOrReserved = false;
5262-
if (isAllocated || IpAddress.State.Reserved.name().equalsIgnoreCase(state)) {
5263-
isAllocatedOrReserved = true;
5264-
}
5278+
boolean isAllocatedOrReserved = isAllocated ||
5279+
(states.size() == 1 && IpAddress.State.Reserved.equals(states.get(0)));
52655280
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
52665281
final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(),
52675282
null);
@@ -5275,7 +5290,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
52755290
buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated);
52765291

52775292
SearchCriteria<IPAddressVO> sc = sb.create();
5278-
setParameters(sc, cmd, vlanType, isAllocated);
5293+
setParameters(sc, cmd, vlanType, isAllocated, states);
52795294

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

53465361
SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
5347-
setParameters(searchCriteria, cmd, vlanType, false);
5362+
setParameters(searchCriteria, cmd, vlanType, false, states);
53485363
searchCriteria.setParameters("state", IpAddress.State.Free.name());
53495364
addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
53505365
}
@@ -5357,7 +5372,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
53575372
sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN);
53585373

53595374
SearchCriteria<IPAddressVO> sc2 = sb2.create();
5360-
setParameters(sc2, cmd, vlanType, isAllocated);
5375+
setParameters(sc2, cmd, vlanType, isAllocated, states);
53615376
sc2.setParameters("ids", freeAddrIds.toArray());
53625377
_publicIpAddressDao.buildQuarantineSearchCriteria(sc2);
53635378
addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free
@@ -5387,7 +5402,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
53875402
sb.and("isSourceNat", sb.entity().isSourceNat(), SearchCriteria.Op.EQ);
53885403
sb.and("isStaticNat", sb.entity().isOneToOneNat(), SearchCriteria.Op.EQ);
53895404
sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ);
5390-
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
5405+
sb.and("state", sb.entity().getState(), SearchCriteria.Op.IN);
53915406
sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ);
53925407
sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ);
53935408

@@ -5429,7 +5444,8 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
54295444
}
54305445
}
54315446

5432-
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
5447+
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType,
5448+
Boolean isAllocated, List<IpAddress.State> states) {
54335449
final Object keyword = cmd.getKeyword();
54345450
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
54355451
final Long sourceNetworkId = cmd.getNetworkId();
@@ -5441,7 +5457,6 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
54415457
final Boolean sourceNat = cmd.isSourceNat();
54425458
final Boolean staticNat = cmd.isStaticNat();
54435459
final Boolean forDisplay = cmd.getDisplay();
5444-
final String state = cmd.getState();
54455460
final Boolean forSystemVms = cmd.getForSystemVMs();
54465461
final boolean forProvider = cmd.isForProvider();
54475462
final Map<String, String> tags = cmd.getTags();
@@ -5498,13 +5513,14 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
54985513
sc.setParameters("display", forDisplay);
54995514
}
55005515

5501-
if (state != null) {
5502-
sc.setParameters("state", state);
5516+
if (CollectionUtils.isNotEmpty(states)) {
5517+
sc.setParameters("state", states.toArray());
55035518
} else if (isAllocated != null && isAllocated) {
55045519
sc.setParameters("state", IpAddress.State.Allocated);
55055520
}
55065521

5507-
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) {
5522+
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() &&
5523+
states.contains(IpAddress.State.Free)) {
55085524
sc.setParameters(FOR_SYSTEMVMS, false);
55095525
} else {
55105526
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)