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
This commit is contained in:
Huanxuan Ao
parent
49e866174f
commit
ef5a7caf85
@ -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
|
||||
---------
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
@ -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."""
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -0,0 +1,5 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
Add support to add/remove multi users by ``group add/remove user`` command.
|
||||
[Blueprint :oscbp:`support-multi-add-remove`]
|
||||
Loading…
Reference in New Issue
Block a user