Skip to content

Commit ca9dceb

Browse files
davidjumanidavidjumani
committed
Adding pagination for listRoles
1 parent 6b67bca commit ca9dceb

6 files changed

Lines changed: 87 additions & 35 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/acl/RoleService.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
import java.util.List;
2121

22+
import com.cloud.utils.Pair;
23+
2224
import org.apache.cloudstack.acl.RolePermission.Permission;
2325
import org.apache.cloudstack.framework.config.ConfigKey;
2426

@@ -65,16 +67,22 @@ public interface RoleService {
6567
*/
6668
List<Role> listRoles();
6769

70+
Pair<List<Role>, Integer> listRoles(Long startIndex, Long limit);
71+
6872
/**
6973
* Find all roles that have the giving {@link String} as part of their name.
7074
* If the user calling the method is not a 'root admin', roles of type {@link RoleType#Admin} wil lbe removed of the returned list.
7175
*/
7276
List<Role> findRolesByName(String name);
7377

78+
Pair<List<Role>, Integer> findRolesByName(String name, Long startIndex, Long limit);
79+
7480
/**
7581
* Find all roles by {@link RoleType}. If the role type is {@link RoleType#Admin}, the calling account must be a root admin, otherwise we return an empty list.
7682
*/
7783
List<Role> findRolesByType(RoleType roleType);
7884

85+
Pair<List<Role>, Integer> findRolesByType(RoleType roleType, Long startIndex, Long limit);
86+
7987
List<RolePermission> findAllPermissionsBy(Long roleId);
8088
}

‎api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,19 @@
2525
import org.apache.cloudstack.acl.RoleType;
2626
import org.apache.cloudstack.api.APICommand;
2727
import org.apache.cloudstack.api.ApiConstants;
28-
import org.apache.cloudstack.api.BaseCmd;
28+
import org.apache.cloudstack.api.BaseListCmd;
2929
import org.apache.cloudstack.api.Parameter;
3030
import org.apache.cloudstack.api.response.ListResponse;
3131
import org.apache.cloudstack.api.response.RoleResponse;
3232
import org.apache.commons.lang3.StringUtils;
3333

3434
import com.cloud.user.Account;
35+
import com.cloud.utils.Pair;
3536
import com.google.common.base.Strings;
3637

3738
@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
38-
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
39-
public class ListRolesCmd extends BaseCmd {
39+
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin })
40+
public class ListRolesCmd extends BaseListCmd {
4041
public static final String APINAME = "listRoles";
4142

4243
/////////////////////////////////////////////////////
@@ -77,18 +78,18 @@ public RoleType getRoleType() {
7778

7879
@Override
7980
public String getCommandName() {
80-
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
81+
return APINAME.toLowerCase() + BaseListCmd.RESPONSE_SUFFIX;
8182
}
8283

8384
@Override
8485
public long getEntityOwnerId() {
8586
return Account.ACCOUNT_ID_SYSTEM;
8687
}
8788

88-
private void setupResponse(final List<Role> roles) {
89+
private void setupResponse(final Pair<List<Role>, Integer> roles) {
8990
final ListResponse<RoleResponse> response = new ListResponse<>();
9091
final List<RoleResponse> roleResponses = new ArrayList<>();
91-
for (final Role role : roles) {
92+
for (final Role role : roles.first()) {
9293
if (role == null) {
9394
continue;
9495
}
@@ -100,22 +101,22 @@ private void setupResponse(final List<Role> roles) {
100101
roleResponse.setObjectName("role");
101102
roleResponses.add(roleResponse);
102103
}
103-
response.setResponses(roleResponses);
104+
response.setResponses(roleResponses, roles.second());
104105
response.setResponseName(getCommandName());
105106
setResponseObject(response);
106107
}
107108

108109
@Override
109110
public void execute() {
110-
List<Role> roles;
111+
Pair<List<Role>, Integer> roles;
111112
if (getId() != null && getId() > 0L) {
112-
roles = Collections.singletonList(roleService.findRole(getId()));
113+
roles = new Pair<List<Role>, Integer>(Collections.singletonList(roleService.findRole(getId())), 1);
113114
} else if (StringUtils.isNotBlank(getName())) {
114-
roles = roleService.findRolesByName(getName());
115+
roles = roleService.findRolesByName(getName(), getStartIndex(), getPageSizeVal());
115116
} else if (getRoleType() != null) {
116-
roles = roleService.findRolesByType(getRoleType());
117+
roles = roleService.findRolesByType(getRoleType(), getStartIndex(), getPageSizeVal());
117118
} else {
118-
roles = roleService.listRoles();
119+
roles = roleService.listRoles(getStartIndex(), getPageSizeVal());
119120
}
120121
setupResponse(roles);
121122
}

‎engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.apache.cloudstack.acl.dao;
1919

20+
import com.cloud.utils.Pair;
2021
import com.cloud.utils.db.GenericDao;
2122
import org.apache.cloudstack.acl.RoleType;
2223
import org.apache.cloudstack.acl.RoleVO;
@@ -25,5 +26,10 @@
2526

2627
public interface RoleDao extends GenericDao<RoleVO, Long> {
2728
List<RoleVO> findAllByName(String roleName);
29+
30+
Pair<List<RoleVO>, Integer> findAllByName(final String roleName, Long offset, Long limit);
31+
2832
List<RoleVO> findAllByRoleType(RoleType type);
33+
34+
Pair<List<RoleVO>, Integer> findAllByRoleType(RoleType type, Long offset, Long limit);
2935
}

‎engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.cloudstack.acl.dao;
1919

20+
import com.cloud.utils.Pair;
21+
import com.cloud.utils.db.Filter;
2022
import com.cloud.utils.db.GenericDaoBase;
2123
import com.cloud.utils.db.SearchBuilder;
2224
import com.cloud.utils.db.SearchCriteria;
@@ -45,15 +47,26 @@ public RoleDaoImpl() {
4547

4648
@Override
4749
public List<RoleVO> findAllByName(final String roleName) {
50+
return findAllByName(roleName, null, null).first();
51+
}
52+
53+
@Override
54+
public Pair<List<RoleVO>, Integer> findAllByName(final String roleName, Long offset, Long limit) {
4855
SearchCriteria<RoleVO> sc = RoleByNameSearch.create();
4956
sc.setParameters("roleName", "%" + roleName + "%");
50-
return listBy(sc);
57+
Pair<List<RoleVO>, Integer> result = searchAndCount(sc, new Filter(RoleVO.class, "id", true, offset, limit));
58+
System.out.println("Result : " + result);
59+
return result;
5160
}
5261

5362
@Override
5463
public List<RoleVO> findAllByRoleType(final RoleType type) {
64+
return findAllByRoleType(type, null, null).first();
65+
}
66+
67+
public Pair<List<RoleVO>, Integer> findAllByRoleType(final RoleType type, Long offset, Long limit) {
5568
SearchCriteria<RoleVO> sc = RoleByTypeSearch.create();
5669
sc.setParameters("roleType", type);
57-
return listBy(sc);
70+
return searchAndCount(sc, new Filter(RoleVO.class, "id", true, offset, limit));
5871
}
5972
}

‎server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@
4949
import com.cloud.user.AccountManager;
5050
import com.cloud.user.dao.AccountDao;
5151
import com.cloud.utils.ListUtils;
52+
import com.cloud.utils.Pair;
5253
import com.cloud.utils.component.ManagerBase;
5354
import com.cloud.utils.component.PluggableService;
55+
import com.cloud.utils.db.Filter;
5456
import com.cloud.utils.db.Transaction;
5557
import com.cloud.utils.db.TransactionCallback;
5658
import com.cloud.utils.db.TransactionStatus;
@@ -244,49 +246,62 @@ public boolean deleteRolePermission(final RolePermission rolePermission) {
244246

245247
@Override
246248
public List<Role> findRolesByName(String name) {
247-
List<? extends Role> roles = null;
249+
return findRolesByName(name, null, null).first();
250+
}
251+
252+
@Override
253+
public Pair<List<Role>, Integer> findRolesByName(String name, Long startIndex, Long limit) {
248254
if (StringUtils.isNotBlank(name)) {
249-
roles = roleDao.findAllByName(name);
255+
Pair<List<RoleVO>, Integer> data = roleDao.findAllByName(name, startIndex, limit);
256+
int removed = removeRootAdminRolesIfNeeded(data.first());
257+
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
250258
}
251-
removeRootAdminRolesIfNeeded(roles);
252-
return ListUtils.toListOfInterface(roles);
259+
return new Pair<List<Role>, Integer>(new ArrayList<Role>(), 0);
253260
}
254261

255262
/**
256263
* Removes roles of the given list that have the type '{@link RoleType#Admin}' if the user calling the method is not a 'root admin'.
257264
* The actual removal is executed via {@link #removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root admin', we do nothing here.
258265
*/
259-
protected void removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
266+
protected int removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
260267
Account account = getCurrentAccount();
261268
if (!accountManager.isRootAdmin(account.getId())) {
262-
removeRootAdminRoles(roles);
269+
return removeRootAdminRoles(roles);
263270
}
271+
return 0;
264272
}
265273

266274
/**
267275
* Remove all roles that have the {@link RoleType#Admin}.
268276
*/
269-
protected void removeRootAdminRoles(List<? extends Role> roles) {
277+
protected int removeRootAdminRoles(List<? extends Role> roles) {
270278
if (CollectionUtils.isEmpty(roles)) {
271-
return;
279+
return 0;
272280
}
273281
Iterator<? extends Role> rolesIterator = roles.iterator();
282+
int count = 0;
274283
while (rolesIterator.hasNext()) {
275284
Role role = rolesIterator.next();
276285
if (RoleType.Admin == role.getRoleType()) {
286+
count++;
277287
rolesIterator.remove();
278288
}
279289
}
280-
290+
return count;
281291
}
282292

283293
@Override
284294
public List<Role> findRolesByType(RoleType roleType) {
295+
return findRolesByType(roleType, null, null).first();
296+
}
297+
298+
@Override
299+
public Pair<List<Role>, Integer> findRolesByType(RoleType roleType, Long startIndex, Long limit) {
285300
if (roleType == null || RoleType.Admin == roleType && !accountManager.isRootAdmin(getCurrentAccount().getId())) {
286-
return Collections.emptyList();
301+
return new Pair<List<Role>, Integer>(Collections.emptyList(), 0);
287302
}
288-
List<? extends Role> roles = roleDao.findAllByRoleType(roleType);
289-
return ListUtils.toListOfInterface(roles);
303+
Pair<List<RoleVO>, Integer> data = roleDao.findAllByRoleType(roleType, startIndex, limit);
304+
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second()));
290305
}
291306

292307
@Override
@@ -296,6 +311,14 @@ public List<Role> listRoles() {
296311
return ListUtils.toListOfInterface(roles);
297312
}
298313

314+
@Override
315+
public Pair<List<Role>, Integer> listRoles(Long startIndex, Long limit) {
316+
Pair<List<RoleVO>, Integer> data = roleDao.searchAndCount(null,
317+
new Filter(RoleVO.class, "id", Boolean.TRUE, startIndex, limit));
318+
int removed = removeRootAdminRolesIfNeeded(data.first());
319+
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
320+
}
321+
299322
@Override
300323
public List<RolePermission> findAllPermissionsBy(final Long roleId) {
301324
List<? extends RolePermission> permissions = rolePermissionsDao.findAllByRoleIdSorted(roleId);

‎server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import com.cloud.user.Account;
3636
import com.cloud.user.AccountManager;
37+
import com.cloud.utils.Pair;
3738

3839
@RunWith(MockitoJUnitRunner.class)
3940
public class RoleManagerImplTest {
@@ -163,12 +164,12 @@ public void findRolesByNameTestBlankRoleName() {
163164
@Test
164165
public void findRolesByNameTest() {
165166
String roleName = "roleName";
166-
ArrayList<Role> toBeReturned = new ArrayList<>();
167-
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByName(roleName);
167+
List<Role> roles = new ArrayList<>();
168+
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 0);
169+
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByName(roleName, null, null);
168170

169171
roleManagerImpl.findRolesByName(roleName);
170-
171-
Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(toBeReturned);
172+
Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
172173
}
173174

174175
@Test
@@ -248,12 +249,12 @@ public void findRolesByTypeTestAdminRoleRootAdminUser() {
248249

249250
List<Role> roles = new ArrayList<>();
250251
roles.add(Mockito.mock(Role.class));
251-
Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.Admin);
252+
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 1);
253+
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByRoleType(RoleType.Admin, null, null);
252254
List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin);
253255

254256
Assert.assertEquals(1, returnedRoles.size());
255257
Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong());
256-
Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
257258
}
258259

259260
@Test
@@ -263,12 +264,12 @@ public void findRolesByTypeTestNonAdminRoleRootAdminUser() {
263264

264265
List<Role> roles = new ArrayList<>();
265266
roles.add(Mockito.mock(Role.class));
266-
Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.User);
267+
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 1);
268+
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByRoleType(RoleType.User, null, null);
267269
List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.User);
268270

269271
Assert.assertEquals(1, returnedRoles.size());
270272
Mockito.verify(accountManagerMock, Mockito.times(0)).isRootAdmin(Mockito.anyLong());
271-
Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
272273
}
273274

274275
@Test
@@ -277,7 +278,7 @@ public void listRolesTest() {
277278
roles.add(Mockito.mock(Role.class));
278279

279280
Mockito.doReturn(roles).when(roleDaoMock).listAll();
280-
Mockito.doNothing().when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
281+
Mockito.doReturn(0).when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
281282

282283
List<Role> returnedRoles = roleManagerImpl.listRoles();
283284

0 commit comments

Comments
 (0)