Skip to content

Commit 38ccbfb

Browse files
Dingane Hlalukuyadvr
Dingane Hlaluku
authored and
yadvr
committed
CLOUDSTACK-9663: updateRole cmd to return updated role as JSON (#2406)
This fixes updateRole to return a role response, like other update APIs. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
1 parent 8949efe commit 38ccbfb

7 files changed

Lines changed: 125 additions & 27 deletions

File tree

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
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+
18+
package org.apache.cloudstack.api.command.admin.acl;
19+
20+
import org.apache.cloudstack.acl.Role;
21+
import org.apache.cloudstack.api.BaseCmd;
22+
import org.apache.cloudstack.api.response.RoleResponse;
23+
24+
public abstract class RoleCmd extends BaseCmd {
25+
26+
protected void setupResponse(final Role role) {
27+
final RoleResponse response = new RoleResponse();
28+
response.setId(role.getUuid());
29+
response.setRoleName(role.getName());
30+
response.setRoleType(role.getRoleType());
31+
response.setDescription(role.getDescription());
32+
response.setResponseName(getCommandName());
33+
response.setObjectName("role");
34+
setResponseObject(response);
35+
}
36+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface RoleService {
3131
boolean isEnabled();
3232
Role findRole(final Long id);
3333
Role createRole(final String name, final RoleType roleType, final String description);
34-
boolean updateRole(final Role role, final String name, final RoleType roleType, final String description);
34+
Role updateRole(final Role role, final String name, final RoleType roleType, final String description);
3535
boolean deleteRole(final Role role);
3636

3737
RolePermission findRolePermission(final Long id);

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
3535
since = "4.9.0",
3636
authorized = {RoleType.Admin})
37-
public class CreateRoleCmd extends BaseCmd {
37+
public class CreateRoleCmd extends RoleCmd {
3838
public static final String APINAME = "createRole";
3939

4040
/////////////////////////////////////////////////////
@@ -83,16 +83,6 @@ public long getEntityOwnerId() {
8383
return Account.ACCOUNT_ID_SYSTEM;
8484
}
8585

86-
private void setupResponse(final Role role) {
87-
final RoleResponse response = new RoleResponse();
88-
response.setId(role.getUuid());
89-
response.setRoleName(role.getName());
90-
response.setRoleType(role.getRoleType());
91-
response.setResponseName(getCommandName());
92-
response.setObjectName("role");
93-
setResponseObject(response);
94-
}
95-
9686
@Override
9787
public void execute() {
9888
CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription());

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,20 @@
2222
import org.apache.cloudstack.acl.Role;
2323
import org.apache.cloudstack.acl.RoleType;
2424
import org.apache.cloudstack.api.APICommand;
25+
import org.apache.cloudstack.api.ApiArgValidator;
2526
import org.apache.cloudstack.api.ApiConstants;
2627
import org.apache.cloudstack.api.ApiErrorCode;
2728
import org.apache.cloudstack.api.BaseCmd;
2829
import org.apache.cloudstack.api.Parameter;
2930
import org.apache.cloudstack.api.ServerApiException;
30-
import org.apache.cloudstack.api.ApiArgValidator;
3131
import org.apache.cloudstack.api.response.RoleResponse;
32-
import org.apache.cloudstack.api.response.SuccessResponse;
3332
import org.apache.cloudstack.context.CallContext;
3433

35-
@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = SuccessResponse.class,
34+
@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = RoleResponse.class,
3635
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
3736
since = "4.9.0",
3837
authorized = {RoleType.Admin})
39-
public class UpdateRoleCmd extends BaseCmd {
38+
public class UpdateRoleCmd extends RoleCmd {
4039
public static final String APINAME = "updateRole";
4140

4241
/////////////////////////////////////////////////////
@@ -100,9 +99,7 @@ public void execute() {
10099
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided");
101100
}
102101
CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription());
103-
boolean result = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription());
104-
SuccessResponse response = new SuccessResponse(getCommandName());
105-
response.setSuccess(result);
106-
setResponseObject(response);
102+
role = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription());
103+
setupResponse(role);
107104
}
108105
}

‎api/test/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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+
18+
package org.apache.cloudstack.api.command.test;
19+
20+
import junit.framework.TestCase;
21+
import org.apache.cloudstack.acl.Role;
22+
import org.apache.cloudstack.acl.RoleService;
23+
import org.apache.cloudstack.acl.RoleType;
24+
import org.apache.cloudstack.api.command.admin.acl.UpdateRoleCmd;
25+
import org.apache.cloudstack.api.response.RoleResponse;
26+
import org.junit.Before;
27+
import org.junit.Test;
28+
import org.mockito.Mockito;
29+
import org.springframework.test.util.ReflectionTestUtils;
30+
31+
import static org.mockito.Mockito.when;
32+
33+
34+
public class UpdateRoleCmdTest extends TestCase{
35+
36+
private UpdateRoleCmd updateRoleCmd;
37+
private RoleService roleService;
38+
private Role role;
39+
40+
@Override
41+
@Before
42+
public void setUp() {
43+
roleService = Mockito.spy(RoleService.class);
44+
updateRoleCmd = new UpdateRoleCmd();
45+
ReflectionTestUtils.setField(updateRoleCmd,"roleService",roleService);
46+
ReflectionTestUtils.setField(updateRoleCmd,"roleId",1L);
47+
ReflectionTestUtils.setField(updateRoleCmd,"roleName","user");
48+
ReflectionTestUtils.setField(updateRoleCmd,"roleType", "User");
49+
ReflectionTestUtils.setField(updateRoleCmd,"roleDescription","Description Initial");
50+
role = Mockito.mock(Role.class);
51+
}
52+
53+
@Test
54+
public void testUpdateSuccess() {
55+
when(roleService.findRole(updateRoleCmd.getRoleId())).thenReturn(role);
56+
when(role.getId()).thenReturn(1L);
57+
when(role.getUuid()).thenReturn("12345-abcgdkajd");
58+
when(role.getDescription()).thenReturn("Defualt user");
59+
when(role.getName()).thenReturn("User");
60+
when(role.getRoleType()).thenReturn(RoleType.User);
61+
when(roleService.updateRole(role,updateRoleCmd.getRoleName(),updateRoleCmd.getRoleType(),updateRoleCmd.getRoleDescription())).thenReturn(role);
62+
when(role.getId()).thenReturn(1L);
63+
when(role.getDescription()).thenReturn("Description Initial");
64+
when(role.getName()).thenReturn("User");
65+
updateRoleCmd.execute();
66+
RoleResponse response = (RoleResponse) updateRoleCmd.getResponseObject();
67+
assertEquals((String)ReflectionTestUtils.getField(response, "roleName"),role.getName());
68+
assertEquals((String)ReflectionTestUtils.getField(response, "roleDescription"),role.getDescription());
69+
}
70+
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,9 @@ public RoleVO doInTransaction(TransactionStatus status) {
119119

120120
@Override
121121
@ActionEvent(eventType = EventTypes.EVENT_ROLE_UPDATE, eventDescription = "updating Role")
122-
public boolean updateRole(final Role role, final String name, final RoleType roleType, final String description) {
122+
public Role updateRole(final Role role, final String name, final RoleType roleType, final String description) {
123123
checkCallerAccess();
124-
if (role == null) {
125-
return false;
126-
}
124+
127125
if (roleType != null && roleType == RoleType.Unknown) {
128126
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unknown is not a valid role type");
129127
}
@@ -145,7 +143,9 @@ public boolean updateRole(final Role role, final String name, final RoleType rol
145143
if (!Strings.isNullOrEmpty(description)) {
146144
roleVO.setDescription(description);
147145
}
148-
return roleDao.update(role.getId(), roleVO);
146+
147+
roleDao.update(role.getId(), roleVO);
148+
return role;
149149
}
150150

151151
@Override

‎test/integration/smoke/test_dynamicroles.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ def test_role_lifecycle_update(self):
209209
"""
210210
self.account.delete(self.apiclient)
211211
new_role_name = self.getRandomRoleName()
212-
self.role.update(self.apiclient, name=new_role_name, type='Admin')
212+
new_role_description = "Fake role description created after update"
213+
self.role.update(self.apiclient, name=new_role_name, type='Admin', description=new_role_description)
213214
update_role = Role.list(self.apiclient, id=self.role.id)[0]
214215
self.assertEqual(
215216
update_role.name,
@@ -221,7 +222,11 @@ def test_role_lifecycle_update(self):
221222
'Admin',
222223
msg="Role type does not match updated role type"
223224
)
224-
225+
self.assertEqual(
226+
update_role.description,
227+
new_role_description,
228+
msg="Role description does not match updated role description"
229+
)
225230

226231
@attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)
227232
def test_role_lifecycle_update_role_inuse(self):

0 commit comments

Comments
 (0)