diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java new file mode 100644 index 00000000000..9054ff5fada --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java @@ -0,0 +1,36 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.api.command.admin.acl; + +import org.apache.cloudstack.acl.Role; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.response.RoleResponse; + +public abstract class RoleCmd extends BaseCmd { + + protected void setupResponse(final Role role) { + final RoleResponse response = new RoleResponse(); + response.setId(role.getUuid()); + response.setRoleName(role.getName()); + response.setRoleType(role.getRoleType()); + response.setDescription(role.getDescription()); + response.setResponseName(getCommandName()); + response.setObjectName("role"); + setResponseObject(response); + } +} diff --git a/api/src/org/apache/cloudstack/acl/RoleService.java b/api/src/org/apache/cloudstack/acl/RoleService.java index 98d170b5853..721bef08c20 100644 --- a/api/src/org/apache/cloudstack/acl/RoleService.java +++ b/api/src/org/apache/cloudstack/acl/RoleService.java @@ -31,7 +31,7 @@ public interface RoleService { boolean isEnabled(); Role findRole(final Long id); Role createRole(final String name, final RoleType roleType, final String description); - boolean updateRole(final Role role, final String name, final RoleType roleType, final String description); + Role updateRole(final Role role, final String name, final RoleType roleType, final String description); boolean deleteRole(final Role role); RolePermission findRolePermission(final Long id); diff --git a/api/src/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java b/api/src/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java index 994573d1796..87f0288dd01 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java @@ -34,7 +34,7 @@ import org.apache.cloudstack.context.CallContext; requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {RoleType.Admin}) -public class CreateRoleCmd extends BaseCmd { +public class CreateRoleCmd extends RoleCmd { public static final String APINAME = "createRole"; ///////////////////////////////////////////////////// @@ -83,16 +83,6 @@ public class CreateRoleCmd extends BaseCmd { return Account.ACCOUNT_ID_SYSTEM; } - private void setupResponse(final Role role) { - final RoleResponse response = new RoleResponse(); - response.setId(role.getUuid()); - response.setRoleName(role.getName()); - response.setRoleType(role.getRoleType()); - response.setResponseName(getCommandName()); - response.setObjectName("role"); - setResponseObject(response); - } - @Override public void execute() { CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription()); diff --git a/api/src/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java b/api/src/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java index e17fc6f5714..f9519aeec5a 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java @@ -22,21 +22,20 @@ import com.google.common.base.Strings; import org.apache.cloudstack.acl.Role; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiArgValidator; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; -import org.apache.cloudstack.api.ApiArgValidator; import org.apache.cloudstack.api.response.RoleResponse; -import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.cloudstack.context.CallContext; -@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = SuccessResponse.class, +@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {RoleType.Admin}) -public class UpdateRoleCmd extends BaseCmd { +public class UpdateRoleCmd extends RoleCmd { public static final String APINAME = "updateRole"; ///////////////////////////////////////////////////// @@ -100,9 +99,7 @@ public class UpdateRoleCmd extends BaseCmd { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided"); } CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription()); - boolean result = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription()); - SuccessResponse response = new SuccessResponse(getCommandName()); - response.setSuccess(result); - setResponseObject(response); + role = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription()); + setupResponse(role); } } diff --git a/api/test/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java b/api/test/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java new file mode 100644 index 00000000000..c0bd390c196 --- /dev/null +++ b/api/test/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java @@ -0,0 +1,70 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.api.command.test; + +import junit.framework.TestCase; +import org.apache.cloudstack.acl.Role; +import org.apache.cloudstack.acl.RoleService; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.command.admin.acl.UpdateRoleCmd; +import org.apache.cloudstack.api.response.RoleResponse; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.mockito.Mockito.when; + + +public class UpdateRoleCmdTest extends TestCase{ + + private UpdateRoleCmd updateRoleCmd; + private RoleService roleService; + private Role role; + + @Override + @Before + public void setUp() { + roleService = Mockito.spy(RoleService.class); + updateRoleCmd = new UpdateRoleCmd(); + ReflectionTestUtils.setField(updateRoleCmd,"roleService",roleService); + ReflectionTestUtils.setField(updateRoleCmd,"roleId",1L); + ReflectionTestUtils.setField(updateRoleCmd,"roleName","user"); + ReflectionTestUtils.setField(updateRoleCmd,"roleType", "User"); + ReflectionTestUtils.setField(updateRoleCmd,"roleDescription","Description Initial"); + role = Mockito.mock(Role.class); + } + + @Test + public void testUpdateSuccess() { + when(roleService.findRole(updateRoleCmd.getRoleId())).thenReturn(role); + when(role.getId()).thenReturn(1L); + when(role.getUuid()).thenReturn("12345-abcgdkajd"); + when(role.getDescription()).thenReturn("Defualt user"); + when(role.getName()).thenReturn("User"); + when(role.getRoleType()).thenReturn(RoleType.User); + when(roleService.updateRole(role,updateRoleCmd.getRoleName(),updateRoleCmd.getRoleType(),updateRoleCmd.getRoleDescription())).thenReturn(role); + when(role.getId()).thenReturn(1L); + when(role.getDescription()).thenReturn("Description Initial"); + when(role.getName()).thenReturn("User"); + updateRoleCmd.execute(); + RoleResponse response = (RoleResponse) updateRoleCmd.getResponseObject(); + assertEquals((String)ReflectionTestUtils.getField(response, "roleName"),role.getName()); + assertEquals((String)ReflectionTestUtils.getField(response, "roleDescription"),role.getDescription()); + } +} diff --git a/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java index 5557aff71e0..b1e13313117 100644 --- a/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java +++ b/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java @@ -119,11 +119,9 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu @Override @ActionEvent(eventType = EventTypes.EVENT_ROLE_UPDATE, eventDescription = "updating Role") - public boolean updateRole(final Role role, final String name, final RoleType roleType, final String description) { + public Role updateRole(final Role role, final String name, final RoleType roleType, final String description) { checkCallerAccess(); - if (role == null) { - return false; - } + if (roleType != null && roleType == RoleType.Unknown) { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unknown is not a valid role type"); } @@ -145,7 +143,9 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu if (!Strings.isNullOrEmpty(description)) { roleVO.setDescription(description); } - return roleDao.update(role.getId(), roleVO); + + roleDao.update(role.getId(), roleVO); + return role; } @Override diff --git a/test/integration/smoke/test_dynamicroles.py b/test/integration/smoke/test_dynamicroles.py index 5d105f6a427..61e1d199741 100644 --- a/test/integration/smoke/test_dynamicroles.py +++ b/test/integration/smoke/test_dynamicroles.py @@ -209,7 +209,8 @@ class TestDynamicRoles(cloudstackTestCase): """ self.account.delete(self.apiclient) new_role_name = self.getRandomRoleName() - self.role.update(self.apiclient, name=new_role_name, type='Admin') + new_role_description = "Fake role description created after update" + self.role.update(self.apiclient, name=new_role_name, type='Admin', description=new_role_description) update_role = Role.list(self.apiclient, id=self.role.id)[0] self.assertEqual( update_role.name, @@ -221,7 +222,11 @@ class TestDynamicRoles(cloudstackTestCase): 'Admin', msg="Role type does not match updated role type" ) - + self.assertEqual( + update_role.description, + new_role_description, + msg="Role description does not match updated role description" + ) @attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False) def test_role_lifecycle_update_role_inuse(self):