From ef5a7caf85bd6169701371da67029457abdaf47f Mon Sep 17 00:00:00 2001 From: Huanxuan Ao Date: Fri, 17 Mar 2017 13:28:49 +0800 Subject: [PATCH] Support to add/remove multi users for "group add/remove user" Similar delete commands in OSC, we can also support add/remove multi users for one specified group, this review implement it. Change-Id: I8ccf99d4ee83a18778fa3ff5c0a42bc7c6ff21fb Implements: bp support-multi-add-remove --- doc/source/command-objects/group.rst | 10 +- openstackclient/identity/v3/group.py | 68 ++++++--- .../tests/unit/identity/v3/fakes.py | 38 +++++ .../tests/unit/identity/v3/test_group.py | 131 +++++++++++++----- ...ort-multi-add-remove-9516f72cfacea11a.yaml | 5 + 5 files changed, 193 insertions(+), 59 deletions(-) create mode 100644 releasenotes/notes/bp-support-multi-add-remove-9516f72cfacea11a.yaml diff --git a/doc/source/command-objects/group.rst b/doc/source/command-objects/group.rst index 3c3199cfeb..ac938efdc2 100644 --- a/doc/source/command-objects/group.rst +++ b/doc/source/command-objects/group.rst @@ -16,7 +16,7 @@ Add user to group [--group-domain ] [--user-domain ] - + [ ...] .. option:: --group-domain @@ -38,7 +38,8 @@ Add user to group .. describe:: - User to add to (name or ID) + User(s) to add to (name or ID) + (repeat option to add multiple users) group contains user ------------------- @@ -172,7 +173,7 @@ Remove user from group [--group-domain ] [--user-domain ] - + [ ...] .. option:: --group-domain @@ -194,7 +195,8 @@ Remove user from group .. describe:: - User to remove from (name or ID) + User(s) to remove from (name or ID) + (repeat option to remove multiple users) group set --------- diff --git a/openstackclient/identity/v3/group.py b/openstackclient/identity/v3/group.py index b5f5d8ad88..39c8547c4e 100644 --- a/openstackclient/identity/v3/group.py +++ b/openstackclient/identity/v3/group.py @@ -44,7 +44,9 @@ class AddUserToGroup(command.Command): parser.add_argument( 'user', metavar='', - help=_('User to add to (name or ID)'), + nargs='+', + help=_('User(s) to add to (name or ID) ' + '(repeat option to add multiple users)'), ) common.add_group_domain_option_to_parser(parser) common.add_user_domain_option_to_parser(parser) @@ -53,20 +55,32 @@ class AddUserToGroup(command.Command): def take_action(self, parsed_args): identity_client = self.app.client_manager.identity - user_id = common.find_user(identity_client, - parsed_args.user, - parsed_args.user_domain).id group_id = common.find_group(identity_client, parsed_args.group, parsed_args.group_domain).id - try: - identity_client.users.add_to_group(user_id, group_id) - except Exception as e: - msg = _("%(user)s not added to group %(group)s: %(e)s") % { - 'user': parsed_args.user, + result = 0 + for i in parsed_args.user: + try: + user_id = common.find_user(identity_client, + i, + parsed_args.user_domain).id + identity_client.users.add_to_group(user_id, group_id) + except Exception as e: + result += 1 + msg = _("%(user)s not added to group %(group)s: %(e)s") % { + 'user': i, + 'group': parsed_args.group, + 'e': e, + } + LOG.error(msg) + if result > 0: + total = len(parsed_args.user) + msg = (_("%(result)s of %(total)s users not added to group " + "%(group)s.")) % { + 'result': result, + 'total': total, 'group': parsed_args.group, - 'e': e, } raise exceptions.CommandError(msg) @@ -286,7 +300,9 @@ class RemoveUserFromGroup(command.Command): parser.add_argument( 'user', metavar='', - help=_('User to remove from (name or ID)'), + nargs='+', + help=_('User(s) to remove from (name or ID) ' + '(repeat option to remove multiple users)'), ) common.add_group_domain_option_to_parser(parser) common.add_user_domain_option_to_parser(parser) @@ -295,20 +311,32 @@ class RemoveUserFromGroup(command.Command): def take_action(self, parsed_args): identity_client = self.app.client_manager.identity - user_id = common.find_user(identity_client, - parsed_args.user, - parsed_args.user_domain).id group_id = common.find_group(identity_client, parsed_args.group, parsed_args.group_domain).id - try: - identity_client.users.remove_from_group(user_id, group_id) - except Exception as e: - msg = _("%(user)s not removed from group %(group)s: %(e)s") % { - 'user': parsed_args.user, + result = 0 + for i in parsed_args.user: + try: + user_id = common.find_user(identity_client, + i, + parsed_args.user_domain).id + identity_client.users.remove_from_group(user_id, group_id) + except Exception as e: + result += 1 + msg = _("%(user)s not removed from group %(group)s: %(e)s") % { + 'user': i, + 'group': parsed_args.group, + 'e': e, + } + LOG.error(msg) + if result > 0: + total = len(parsed_args.user) + msg = (_("%(result)s of %(total)s users not removed from group " + "%(group)s.")) % { + 'result': result, + 'total': total, 'group': parsed_args.group, - 'e': e, } raise exceptions.CommandError(msg) diff --git a/openstackclient/tests/unit/identity/v3/fakes.py b/openstackclient/tests/unit/identity/v3/fakes.py index 01b5dede0f..139d90d5b8 100644 --- a/openstackclient/tests/unit/identity/v3/fakes.py +++ b/openstackclient/tests/unit/identity/v3/fakes.py @@ -770,6 +770,44 @@ class FakeUser(object): loaded=True) return user + @staticmethod + def create_users(attrs=None, count=2): + """Create multiple fake users. + + :param Dictionary attrs: + A dictionary with all attributes + :param int count: + The number of users to fake + :return: + A list of FakeResource objects faking the users + """ + users = [] + for i in range(0, count): + user = FakeUser.create_one_user(attrs) + users.append(user) + + return users + + @staticmethod + def get_users(users=None, count=2): + """Get an iterable MagicMock object with a list of faked users. + + If users list is provided, then initialize the Mock object with + the list. Otherwise create one. + + :param List users: + A list of FakeResource objects faking users + :param Integer count: + The number of users to be faked + :return + An iterable Mock object with side_effect set to a list of faked + users + """ + if users is None: + users = FakeUser.create_users(count) + + return mock.Mock(side_effect=users) + class FakeGroup(object): """Fake one or more group.""" diff --git a/openstackclient/tests/unit/identity/v3/test_group.py b/openstackclient/tests/unit/identity/v3/test_group.py index 5870e1dbc3..81722631b3 100644 --- a/openstackclient/tests/unit/identity/v3/test_group.py +++ b/openstackclient/tests/unit/identity/v3/test_group.py @@ -42,47 +42,78 @@ class TestGroup(identity_fakes.TestIdentityv3): class TestGroupAddUser(TestGroup): - group = identity_fakes.FakeGroup.create_one_group() - user = identity_fakes.FakeUser.create_one_user() + _group = identity_fakes.FakeGroup.create_one_group() + users = identity_fakes.FakeUser.create_users(count=2) def setUp(self): super(TestGroupAddUser, self).setUp() - self.groups_mock.get.return_value = self.group - self.users_mock.get.return_value = self.user + self.groups_mock.get.return_value = self._group + self.users_mock.get = ( + identity_fakes.FakeUser.get_users(self.users)) self.users_mock.add_to_group.return_value = None self.cmd = group.AddUserToGroup(self.app, None) def test_group_add_user(self): arglist = [ - self.group.name, - self.user.name, + self._group.name, + self.users[0].name, ] verifylist = [ - ('group', self.group.name), - ('user', self.user.name), + ('group', self._group.name), + ('user', [self.users[0].name]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) self.users_mock.add_to_group.assert_called_once_with( - self.user.id, self.group.id) + self.users[0].id, self._group.id) self.assertIsNone(result) - def test_group_add_user_with_error(self): - self.users_mock.add_to_group.side_effect = exceptions.CommandError() + def test_group_add_multi_users(self): arglist = [ - self.group.name, - self.user.name, + self._group.name, + self.users[0].name, + self.users[1].name, ] verifylist = [ - ('group', self.group.name), - ('user', self.user.name), + ('group', self._group.name), + ('user', [self.users[0].name, self.users[1].name]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.assertRaises(exceptions.CommandError, - self.cmd.take_action, parsed_args) + + result = self.cmd.take_action(parsed_args) + calls = [call(self.users[0].id, self._group.id), + call(self.users[1].id, self._group.id)] + self.users_mock.add_to_group.assert_has_calls(calls) + self.assertIsNone(result) + + @mock.patch.object(group.LOG, 'error') + def test_group_add_user_with_error(self, mock_error): + self.users_mock.add_to_group.side_effect = [ + exceptions.CommandError(), None] + arglist = [ + self._group.name, + self.users[0].name, + self.users[1].name, + ] + verifylist = [ + ('group', self._group.name), + ('user', [self.users[0].name, self.users[1].name]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + msg = "1 of 2 users not added to group %s." % self._group.name + self.assertEqual(msg, str(e)) + msg = ("%(user)s not added to group %(group)s: ") % { + 'user': self.users[0].name, + 'group': self._group.name, + } + mock_error.assert_called_once_with(msg) class TestGroupCheckUser(TestGroup): @@ -463,48 +494,78 @@ class TestGroupList(TestGroup): class TestGroupRemoveUser(TestGroup): - group = identity_fakes.FakeGroup.create_one_group() - user = identity_fakes.FakeUser.create_one_user() + _group = identity_fakes.FakeGroup.create_one_group() + users = identity_fakes.FakeUser.create_users(count=2) def setUp(self): super(TestGroupRemoveUser, self).setUp() - self.groups_mock.get.return_value = self.group - self.users_mock.get.return_value = self.user + self.groups_mock.get.return_value = self._group + self.users_mock.get = ( + identity_fakes.FakeUser.get_users(self.users)) self.users_mock.remove_from_group.return_value = None self.cmd = group.RemoveUserFromGroup(self.app, None) def test_group_remove_user(self): arglist = [ - self.group.id, - self.user.id, + self._group.id, + self.users[0].id, ] verifylist = [ - ('group', self.group.id), - ('user', self.user.id), + ('group', self._group.id), + ('user', [self.users[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) self.users_mock.remove_from_group.assert_called_once_with( - self.user.id, self.group.id) + self.users[0].id, self._group.id) self.assertIsNone(result) - def test_group_remove_user_with_error(self): - self.users_mock.remove_from_group.side_effect = ( - exceptions.CommandError()) + def test_group_remove_multi_users(self): arglist = [ - self.group.id, - self.user.id, + self._group.name, + self.users[0].name, + self.users[1].name, ] verifylist = [ - ('group', self.group.id), - ('user', self.user.id), + ('group', self._group.name), + ('user', [self.users[0].name, self.users[1].name]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.assertRaises(exceptions.CommandError, - self.cmd.take_action, parsed_args) + + result = self.cmd.take_action(parsed_args) + calls = [call(self.users[0].id, self._group.id), + call(self.users[1].id, self._group.id)] + self.users_mock.remove_from_group.assert_has_calls(calls) + self.assertIsNone(result) + + @mock.patch.object(group.LOG, 'error') + def test_group_remove_user_with_error(self, mock_error): + self.users_mock.remove_from_group.side_effect = [ + exceptions.CommandError(), None] + arglist = [ + self._group.id, + self.users[0].id, + self.users[1].id, + ] + verifylist = [ + ('group', self._group.id), + ('user', [self.users[0].id, self.users[1].id]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + msg = "1 of 2 users not removed from group %s." % self._group.id + self.assertEqual(msg, str(e)) + msg = ("%(user)s not removed from group %(group)s: ") % { + 'user': self.users[0].id, + 'group': self._group.id, + } + mock_error.assert_called_once_with(msg) class TestGroupSet(TestGroup): diff --git a/releasenotes/notes/bp-support-multi-add-remove-9516f72cfacea11a.yaml b/releasenotes/notes/bp-support-multi-add-remove-9516f72cfacea11a.yaml new file mode 100644 index 0000000000..83a7c03baa --- /dev/null +++ b/releasenotes/notes/bp-support-multi-add-remove-9516f72cfacea11a.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Add support to add/remove multi users by ``group add/remove user`` command. + [Blueprint :oscbp:`support-multi-add-remove`]