Skip to content

Commit f384872

Browse files
shwstpprshwstppr
committed
fix and tests
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent e80e892 commit f384872

File tree

7 files changed

+196
-12
lines changed

7 files changed

+196
-12
lines changed

‎engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ public List<Long> listByUserdataIdsNotAccount(List<Long> userdataIds, long accou
870870
return Collections.emptyList();
871871
}
872872
GenericSearchBuilder<VMTemplateVO, Long> sb = createSearchBuilder(Long.class);
873-
sb.selectFields(userDataSearch.entity().getId());
873+
sb.selectFields(sb.entity().getId());
874874
sb.and("userDataId", sb.entity().getUserDataId(), SearchCriteria.Op.EQ);
875875
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
876876
sb.and("accountId", sb.entity().getAccountId(), SearchCriteria.Op.NEQ);

‎engine/schema/src/main/java/com/cloud/user/dao/UserDataDao.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ public interface UserDataDao extends GenericDao<UserDataVO, Long> {
2727

2828
public UserDataVO findByName(long accountId, long domainId, String name);
2929

30-
List<UserDataVO> listByAccountId(long accountId);
30+
List<Long> listIdsByAccountId(long accountId);
31+
3132
int removeByAccountId(long accountId);
3233

3334
}

‎engine/schema/src/main/java/com/cloud/user/dao/UserDataDaoImpl.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
// under the License.
1717
package com.cloud.user.dao;
1818

19+
import java.util.List;
20+
1921
import com.cloud.user.UserDataVO;
2022
import com.cloud.utils.db.GenericDaoBase;
23+
import com.cloud.utils.db.GenericSearchBuilder;
2124
import com.cloud.utils.db.SearchBuilder;
2225
import com.cloud.utils.db.SearchCriteria;
2326
import org.springframework.stereotype.Component;
@@ -64,6 +67,18 @@ public UserDataVO findByName(long accountId, long domainId, String name) {
6467
return findOneBy(sc);
6568
}
6669

70+
@Override
71+
public List<Long> listIdsByAccountId(long accountId) {
72+
GenericSearchBuilder<UserDataVO, Long> sb = createSearchBuilder(Long.class);
73+
sb.selectFields(sb.entity().getId());
74+
sb.and("accountId", sb.entity().getAccountId(), SearchCriteria.Op.EQ);
75+
sb.done();
76+
SearchCriteria<Long> sc = sb.create();
77+
sc.setParameters("accountId", accountId);
78+
return customSearch(sc, null);
79+
}
80+
81+
6782
@Override
6883
public int removeByAccountId(long accountId) {
6984
SearchCriteria<UserDataVO> sc = userdataSearch.create();

‎engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.junit.Assert.assertEquals;
2020
import static org.junit.Assert.assertNotNull;
2121
import static org.junit.Assert.assertNull;
22+
import static org.junit.Assert.assertTrue;
2223
import static org.mockito.ArgumentMatchers.any;
2324
import static org.mockito.Mockito.doReturn;
2425
import static org.mockito.Mockito.mock;
@@ -47,6 +48,7 @@
4748
import com.cloud.storage.VMTemplateVO;
4849
import com.cloud.utils.Pair;
4950
import com.cloud.utils.db.Filter;
51+
import com.cloud.utils.db.GenericSearchBuilder;
5052
import com.cloud.utils.db.SearchBuilder;
5153
import com.cloud.utils.db.SearchCriteria;
5254

@@ -207,4 +209,49 @@ public void testFindSystemVMReadyTemplate() {
207209
VMTemplateVO readyTemplate = templateDao.findSystemVMReadyTemplate(zoneId, Hypervisor.HypervisorType.KVM, CPU.CPUArch.arm64.getType());
208210
Assert.assertEquals(CPU.CPUArch.arm64, readyTemplate.getArch());
209211
}
212+
213+
@Test
214+
public void listByUserdataIdsNotAccount_ReturnsEmptyListWhenUserdataIdsIsEmpty() {
215+
List<Long> result = templateDao.listByUserdataIdsNotAccount(Collections.emptyList(), 1L);
216+
assertNotNull(result);
217+
assertTrue(result.isEmpty());
218+
}
219+
220+
@Test
221+
public void listByUserdataIdsNotAccount_ReturnsMatchingTemplates() {
222+
List<Long> userdataIds = Arrays.asList(1L, 2L);
223+
long accountId = 3L;
224+
225+
GenericSearchBuilder<VMTemplateVO, Long> sb = mock(GenericSearchBuilder.class);
226+
when(sb.entity()).thenReturn(mock(VMTemplateVO.class));
227+
SearchCriteria<Long> sc = mock(SearchCriteria.class);
228+
doReturn(sb).when(templateDao).createSearchBuilder(Long.class);
229+
doReturn(sc).when(sb).create();
230+
doReturn(Arrays.asList(10L, 20L)).when(templateDao).customSearch(sc, null);
231+
232+
List<Long> result = templateDao.listByUserdataIdsNotAccount(userdataIds, accountId);
233+
234+
assertNotNull(result);
235+
assertEquals(2, result.size());
236+
assertTrue(result.contains(10L));
237+
assertTrue(result.contains(20L));
238+
}
239+
240+
@Test
241+
public void listByUserdataIdsNotAccount_ReturnsEmptyListWhenNoMatchingTemplates() {
242+
List<Long> userdataIds = Arrays.asList(1L, 2L);
243+
long accountId = 3L;
244+
245+
GenericSearchBuilder<VMTemplateVO, Long> sb = mock(GenericSearchBuilder.class);
246+
when(sb.entity()).thenReturn(mock(VMTemplateVO.class));
247+
SearchCriteria<Long> sc = mock(SearchCriteria.class);
248+
doReturn(sb).when(templateDao).createSearchBuilder(Long.class);
249+
doReturn(sc).when(sb).create();
250+
doReturn(Collections.emptyList()).when(templateDao).customSearch(sc, null);
251+
252+
List<Long> result = templateDao.listByUserdataIdsNotAccount(userdataIds, accountId);
253+
254+
assertNotNull(result);
255+
assertTrue(result.isEmpty());
256+
}
210257
}

‎engine/schema/src/test/java/com/cloud/user/dao/UserDataDaoImplTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.user.dao;
18+
19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNotNull;
21+
import static org.junit.Assert.assertTrue;
22+
import static org.mockito.Mockito.doReturn;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
25+
26+
import java.util.Arrays;
27+
import java.util.Collections;
28+
import java.util.List;
29+
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.mockito.InjectMocks;
33+
import org.mockito.Spy;
34+
import org.mockito.junit.MockitoJUnitRunner;
35+
36+
import com.cloud.user.UserDataVO;
37+
import com.cloud.utils.db.GenericSearchBuilder;
38+
import com.cloud.utils.db.SearchCriteria;
39+
40+
@RunWith(MockitoJUnitRunner.class)
41+
public class UserDataDaoImplTest {
42+
43+
@Spy
44+
@InjectMocks
45+
UserDataDaoImpl userDataDaoImpl;
46+
47+
@Test
48+
public void listIdsByAccountId_ReturnsEmptyListWhenNoIdsFound() {
49+
long accountId = 1L;
50+
51+
GenericSearchBuilder<UserDataVO, Long> sb = mock(GenericSearchBuilder.class);
52+
when(sb.entity()).thenReturn(mock(UserDataVO.class));
53+
SearchCriteria<Long> sc = mock(SearchCriteria.class);
54+
doReturn(sb).when(userDataDaoImpl).createSearchBuilder(Long.class);
55+
doReturn(sc).when(sb).create();
56+
doReturn(Collections.emptyList()).when(userDataDaoImpl).customSearch(sc, null);
57+
58+
List<Long> result = userDataDaoImpl.listIdsByAccountId(accountId);
59+
60+
assertNotNull(result);
61+
assertTrue(result.isEmpty());
62+
}
63+
64+
@Test
65+
public void listIdsByAccountId_ReturnsListOfIdsWhenFound() {
66+
long accountId = 1L;
67+
68+
GenericSearchBuilder<UserDataVO, Long> sb = mock(GenericSearchBuilder.class);
69+
when(sb.entity()).thenReturn(mock(UserDataVO.class));
70+
SearchCriteria<Long> sc = mock(SearchCriteria.class);
71+
doReturn(sb).when(userDataDaoImpl).createSearchBuilder(Long.class);
72+
doReturn(sc).when(sb).create();
73+
doReturn(Arrays.asList(10L, 20L)).when(userDataDaoImpl).customSearch(sc, null);
74+
75+
List<Long> result = userDataDaoImpl.listIdsByAccountId(accountId);
76+
77+
assertNotNull(result);
78+
assertEquals(2, result.size());
79+
assertTrue(result.contains(10L));
80+
assertTrue(result.contains(20L));
81+
}
82+
}

‎server/src/main/java/com/cloud/user/AccountManagerImpl.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -476,15 +476,16 @@ public void setQuerySelectors(List<QuerySelector> querySelectors) {
476476
}
477477

478478
protected void deleteUserDataForAccount(long accountId) {
479-
List<UserDataVO> userdataList = userDataDao.listByAccountId(accountId);
480-
if (CollectionUtils.isNotEmpty(userdataList)) {
481-
List<Long> conflictingTemplateIds = _templateDao.listByUserdataIdsNotAccount(userdataList
482-
.stream()
483-
.map(UserDataVO::getId)
484-
.collect(Collectors.toList()), accountId);
485-
if (CollectionUtils.isNotEmpty(conflictingTemplateIds)) {
486-
throw new CloudRuntimeException("User data owned by account linked to templates not owned by the account");
487-
}
479+
List<Long> userdataIdsList = userDataDao.listIdsByAccountId(accountId);
480+
if (CollectionUtils.isEmpty(userdataIdsList)) {
481+
return;
482+
}
483+
List<Long> conflictingTemplateIds = _templateDao.listByUserdataIdsNotAccount(userdataIdsList, accountId);
484+
if (CollectionUtils.isNotEmpty(conflictingTemplateIds)) {
485+
logger.warn("User data IDs {} owned by account ID {} cannot be deleted as some of them are " +
486+
"linked to templates {} not owned by the account.", userdataIdsList, accountId,
487+
conflictingTemplateIds);
488+
throw new CloudRuntimeException("User data owned by account linked to templates not owned by the account");
488489
}
489490
userDataDao.removeByAccountId(accountId);
490491
}

‎server/src/test/java/com/cloud/user/AccountManagerImplTest.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.net.UnknownHostException;
2424
import java.util.ArrayList;
2525
import java.util.Arrays;
26+
import java.util.Collections;
2627
import java.util.HashMap;
2728
import java.util.List;
2829
import java.util.Map;
@@ -198,7 +199,7 @@ public void deleteUserAccount() {
198199
sshkeyList.add(sshkey);
199200
Mockito.when(_sshKeyPairDao.listKeyPairs(Mockito.anyLong(), Mockito.anyLong())).thenReturn(sshkeyList);
200201
Mockito.when(_sshKeyPairDao.remove(Mockito.anyLong())).thenReturn(true);
201-
Mockito.when(userDataDao.removeByAccountId(Mockito.anyLong())).thenReturn(222);
202+
Mockito.doNothing().when(accountManagerImpl).deleteUserDataForAccount(Mockito.anyLong());
202203
Mockito.doNothing().when(accountManagerImpl).deleteWebhooksForAccount(Mockito.anyLong());
203204
Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations((Account) any());
204205

@@ -1589,4 +1590,41 @@ public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() {
15891590

15901591
accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock);
15911592
}
1593+
1594+
@Test
1595+
public void deleteUserDataForAccountWhenNoUserDataExists() {
1596+
long accountId = 1L;
1597+
Mockito.when(userDataDao.listIdsByAccountId(accountId)).thenReturn(Collections.emptyList());
1598+
1599+
accountManagerImpl.deleteUserDataForAccount(accountId);
1600+
1601+
Mockito.verify(userDataDao, Mockito.times(1)).listIdsByAccountId(accountId);
1602+
Mockito.verify(userDataDao, Mockito.times(0)).removeByAccountId(accountId);
1603+
Mockito.verifyNoInteractions(_templateDao);
1604+
}
1605+
1606+
@Test
1607+
public void deleteUserDataForAccountWhenNoConflictingTemplatesExist() {
1608+
long accountId = 1L;
1609+
List<Long> userdataIds = List.of(101L, 102L);
1610+
Mockito.when(userDataDao.listIdsByAccountId(accountId)).thenReturn(userdataIds);
1611+
Mockito.when(_templateDao.listByUserdataIdsNotAccount(userdataIds, accountId)).thenReturn(Collections.emptyList());
1612+
1613+
accountManagerImpl.deleteUserDataForAccount(accountId);
1614+
1615+
Mockito.verify(userDataDao, Mockito.times(1)).listIdsByAccountId(accountId);
1616+
Mockito.verify(_templateDao, Mockito.times(1)).listByUserdataIdsNotAccount(userdataIds, accountId);
1617+
Mockito.verify(userDataDao, Mockito.times(1)).removeByAccountId(accountId);
1618+
}
1619+
1620+
@Test(expected = CloudRuntimeException.class)
1621+
public void deleteUserDataForAccountWhenConflictingTemplatesExist() {
1622+
long accountId = 1L;
1623+
List<Long> userdataIds = List.of(101L, 102L);
1624+
List<Long> conflictingTemplateIds = List.of(201L, 202L);
1625+
Mockito.when(userDataDao.listIdsByAccountId(accountId)).thenReturn(userdataIds);
1626+
Mockito.when(_templateDao.listByUserdataIdsNotAccount(userdataIds, accountId)).thenReturn(conflictingTemplateIds);
1627+
1628+
accountManagerImpl.deleteUserDataForAccount(accountId);
1629+
}
15921630
}

0 commit comments

Comments
 (0)