From ef5a7caf85bd6169701371da67029457abdaf47f Mon Sep 17 00:00:00 2001
From: Huanxuan Ao <huanxuan.ao@easystack.cn>
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 <group-domain>]
[--user-domain <user-domain>]
<group>
- <user>
+ <user> [<user> ...]
.. option:: --group-domain <group-domain>
@@ -38,7 +38,8 @@ Add user to group
.. describe:: <user>
- User to add to <group> (name or ID)
+ User(s) to add to <group> (name or ID)
+ (repeat option to add multiple users)
group contains user
-------------------
@@ -172,7 +173,7 @@ Remove user from group
[--group-domain <group-domain>]
[--user-domain <user-domain>]
<group>
- <user>
+ <user> [<user> ...]
.. option:: --group-domain <group-domain>
@@ -194,7 +195,8 @@ Remove user from group
.. describe:: <user>
- User to remove from <group> (name or ID)
+ User(s) to remove from <group> (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='<user>',
- help=_('User to add to <group> (name or ID)'),
+ nargs='+',
+ help=_('User(s) to add to <group> (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='<user>',
- help=_('User to remove from <group> (name or ID)'),
+ nargs='+',
+ help=_('User(s) to remove from <group> (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`]