Merge "Refactor role list subcommand for identity v3 api"

5 files changed, 302 insertions, 333 deletions

diff --git a/openstackclient/identity/v3/group.py b/openstackclient/identity/v3/group.py
index f51129c9..c5a44017 100644
--- a/openstackclient/identity/v3/group.py
+++ b/openstackclient/identity/v3/group.py
@@ -177,23 +177,6 @@ class ListGroup(lister.Lister):
             help='Name or ID of group to list [required with --role]',
         )
         parser.add_argument(
-            '--role',
-            action='store_true',
-            default=False,
-            help='List the roles assigned to <group>',
-        )
-        domain_or_project = parser.add_mutually_exclusive_group()
-        domain_or_project.add_argument(
-            '--domain',
-            metavar='<domain>',
-            help='Filter list by <domain> [Only valid with --role]',
-        )
-        domain_or_project.add_argument(
-            '--project',
-            metavar='<project>',
-            help='Filter list by <project> [Only valid with --role]',
-        )
-        parser.add_argument(
             '--long',
             action='store_true',
             default=False,
@@ -205,65 +188,12 @@ class ListGroup(lister.Lister):
         self.log.debug('take_action(%s)', parsed_args)
         identity_client = self.app.client_manager.identity
 
-        if parsed_args.role:
-            # List roles belonging to group
-
-            # Group is required here, bail if it is not supplied
-            if not parsed_args.group:
-                sys.stderr.write('Error: Group must be specified')
-                # TODO(dtroyer): This lists the commands...I want it to
-                # show the help for _this_ command.
-                self.app.DeferredHelpAction(
-                    self.app.parser,
-                    self.app.parser,
-                    None,
-                    None,
-                )
-                return ([], [])
-
-            group = utils.find_resource(
-                identity_client.groups,
-                parsed_args.group,
-            )
-
-            if parsed_args.domain:
-                columns = ('ID', 'Name', 'Domain', 'Group')
-                domain = utils.find_resource(
-                    identity_client.domains,
-                    parsed_args.domain,
-                )
-                data = identity_client.roles.list(
-                    group=group,
-                    domain=domain,
-                )
-                for group_role in data:
-                    group_role.group = group.name
-                    group_role.domain = domain.name
-            elif parsed_args.project:
-                columns = ('ID', 'Name', 'Project', 'Group')
-                project = utils.find_resource(
-                    identity_client.projects,
-                    parsed_args.project,
-                )
-                data = identity_client.roles.list(
-                    group=group,
-                    project=project,
-                )
-                for group_role in data:
-                    group_role.group = group.name
-                    group_role.project = project.name
-            else:
-                # TODO(dtroyer): raise exception here, this really is an error
-                sys.stderr.write("Error: Must specify --domain or --project "
-                                 "with --role\n")
-                return ([], [])
+        # List groups
+        if parsed_args.long:
+            columns = ('ID', 'Name', 'Domain ID', 'Description')
         else:
-            # List groups
-            if parsed_args.long:
-                columns = ('ID', 'Name', 'Domain ID', 'Description')
-            else:
-                columns = ('ID', 'Name')
-            data = identity_client.groups.list()
+            columns = ('ID', 'Name')
+        data = identity_client.groups.list()
 
         return (columns,
                 (utils.get_item_properties(
diff --git a/openstackclient/identity/v3/role.py b/openstackclient/identity/v3/role.py
index 69c0aa6a..2aabc00c 100644
--- a/openstackclient/identity/v3/role.py
+++ b/openstackclient/identity/v3/role.py
@@ -192,10 +192,110 @@ class ListRole(lister.Lister):
 
     log = logging.getLogger(__name__ + '.ListRole')
 
+    def get_parser(self, prog_name):
+        parser = super(ListRole, self).get_parser(prog_name)
+        domain_or_project = parser.add_mutually_exclusive_group()
+        domain_or_project.add_argument(
+            '--domain',
+            metavar='<domain>',
+            help='Filter role list by <domain>',
+        )
+        domain_or_project.add_argument(
+            '--project',
+            metavar='<project>',
+            help='Filter role list by <project>',
+        )
+        user_or_group = parser.add_mutually_exclusive_group()
+        user_or_group.add_argument(
+            '--user',
+            metavar='<user>',
+            help='Name or ID of user to list roles asssigned to',
+        )
+        user_or_group.add_argument(
+            '--group',
+            metavar='<group>',
+            help='Name or ID of group to list roles asssigned to',
+        )
+        return parser
+
     def take_action(self, parsed_args):
-        self.log.debug('take_action(%s)', parsed_args)
-        columns = ('ID', 'Name')
-        data = self.app.client_manager.identity.roles.list()
+        self.log.debug('take_action(%s)' % parsed_args)
+        identity_client = self.app.client_manager.identity
+
+        if parsed_args.user:
+            user = utils.find_resource(
+                identity_client.users,
+                parsed_args.user,
+            )
+        elif parsed_args.group:
+            group = utils.find_resource(
+                identity_client.groups,
+                parsed_args.group,
+            )
+
+        if parsed_args.domain:
+            domain = utils.find_resource(
+                identity_client.domains,
+                parsed_args.domain,
+            )
+        elif parsed_args.project:
+            project = utils.find_resource(
+                identity_client.projects,
+                parsed_args.project,
+            )
+
+        # no user or group specified, list all roles in the system
+        if not parsed_args.user and not parsed_args.group:
+            columns = ('ID', 'Name')
+            data = identity_client.roles.list()
+        elif parsed_args.user and parsed_args.domain:
+            columns = ('ID', 'Name', 'Domain', 'User')
+            data = identity_client.roles.list(
+                user=user,
+                domain=domain,
+            )
+            for user_role in data:
+                user_role.user = user.name
+                user_role.domain = domain.name
+        elif parsed_args.user and parsed_args.project:
+            columns = ('ID', 'Name', 'Project', 'User')
+            data = identity_client.roles.list(
+                user=user,
+                project=project,
+            )
+            for user_role in data:
+                user_role.user = user.name
+                user_role.project = project.name
+        elif parsed_args.user:
+            columns = ('ID', 'Name')
+            data = identity_client.roles.list(
+                user=user,
+                domain='default',
+            )
+        elif parsed_args.group and parsed_args.domain:
+            columns = ('ID', 'Name', 'Domain', 'Group')
+            data = identity_client.roles.list(
+                group=group,
+                domain=domain,
+            )
+            for group_role in data:
+                group_role.group = group.name
+                group_role.domain = domain.name
+        elif parsed_args.group and parsed_args.project:
+            columns = ('ID', 'Name', 'Project', 'Group')
+            data = identity_client.roles.list(
+                group=group,
+                project=project,
+            )
+            for group_role in data:
+                group_role.group = group.name
+                group_role.project = project.name
+        else:
+            sys.stderr.write("Error: If a user or group is specified, either "
+                             "--domain or --project must also be specified to "
+                             "list role grants.\n")
+            return ([], [])
+
         return (columns,
                 (utils.get_item_properties(
                     s, columns,
diff --git a/openstackclient/identity/v3/user.py b/openstackclient/identity/v3/user.py
index e628e884..c4adb225 100644
--- a/openstackclient/identity/v3/user.py
+++ b/openstackclient/identity/v3/user.py
@@ -17,7 +17,6 @@
 
 import logging
 import six
-import sys
 
 from cliff import command
 from cliff import lister
@@ -162,23 +161,6 @@ class ListUser(lister.Lister):
             help='Name or ID of user to list [required with --role]',
         )
         parser.add_argument(
-            '--role',
-            action='store_true',
-            default=False,
-            help='List the roles assigned to <user>',
-        )
-        domain_or_project = parser.add_mutually_exclusive_group()
-        domain_or_project.add_argument(
-            '--domain',
-            metavar='<domain>',
-            help='Filter list by <domain> [Only valid with --role]',
-        )
-        domain_or_project.add_argument(
-            '--project',
-            metavar='<project>',
-            help='Filter list by <project> [Only valid with --role]',
-        )
-        parser.add_argument(
             '--long',
             action='store_true',
             default=False,
@@ -187,70 +169,15 @@ class ListUser(lister.Lister):
         return parser
 
     def take_action(self, parsed_args):
-        self.log.debug('take_action(%s)', parsed_args)
-        identity_client = self.app.client_manager.identity
+        self.log.debug('take_action(%s)' % parsed_args)
 
-        if parsed_args.role:
-            # List roles belonging to user
-
-            # User is required here, bail if it is not supplied
-            if not parsed_args.user:
-                sys.stderr.write('Error: User must be specified')
-                return ([], [])
-
-            user = utils.find_resource(
-                identity_client.users,
-                parsed_args.user,
-            )
-
-            # List a user's roles
-            if not parsed_args.domain and not parsed_args.project:
-                columns = ('ID', 'Name')
-                data = identity_client.roles.list(
-                    user=user,
-                    domain='default',
-                )
-            # List a user's roles on a domain
-            elif parsed_args.user and parsed_args.domain:
-                columns = ('ID', 'Name', 'Domain', 'User')
-                domain = utils.find_resource(
-                    identity_client.domains,
-                    parsed_args.domain,
-                )
-                data = identity_client.roles.list(
-                    user=user,
-                    domain=domain,
-                )
-                for user_role in data:
-                    user_role.user = user.name
-                    user_role.domain = domain.name
-            # List a user's roles on a project
-            elif parsed_args.user and parsed_args.project:
-                columns = ('ID', 'Name', 'Project', 'User')
-                project = utils.find_resource(
-                    identity_client.projects,
-                    parsed_args.project,
-                )
-                data = identity_client.roles.list(
-                    user=user,
-                    project=project,
-                )
-                for user_role in data:
-                    user_role.user = user.name
-                    user_role.project = project.name
-            else:
-                # TODO(dtroyer): raise exception here, this really is an error
-                sys.stderr.write("Error: Must specify --domain or --project "
-                                 "with --role\n")
-                return ([], [])
+        # List users
+        if parsed_args.long:
+            columns = ('ID', 'Name', 'Project Id', 'Domain Id',
+                       'Description', 'Email', 'Enabled')
         else:
-            # List users
-            if parsed_args.long:
-                columns = ('ID', 'Name', 'Project Id', 'Domain Id',
-                           'Description', 'Email', 'Enabled')
-            else:
-                columns = ('ID', 'Name')
-            data = self.app.client_manager.identity.users.list()
+            columns = ('ID', 'Name')
+        data = self.app.client_manager.identity.users.list()
 
         return (columns,
                 (utils.get_item_properties(
diff --git a/openstackclient/tests/identity/v3/test_role.py b/openstackclient/tests/identity/v3/test_role.py
index 0c0551e1..fa02ecb9 100644
--- a/openstackclient/tests/identity/v3/test_role.py
+++ b/openstackclient/tests/identity/v3/test_role.py
@@ -296,6 +296,27 @@ class TestRoleList(TestRole):
             ),
         ]
 
+        self.domains_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(identity_fakes.DOMAIN),
+            loaded=True,
+        )
+        self.projects_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(identity_fakes.PROJECT),
+            loaded=True,
+        )
+        self.users_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(identity_fakes.USER),
+            loaded=True,
+        )
+        self.groups_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(identity_fakes.GROUP),
+            loaded=True,
+        )
+
         # Get the command object to test
         self.cmd = role.ListRole(self.app, None)
 
@@ -317,6 +338,172 @@ class TestRoleList(TestRole):
         ), )
         self.assertEqual(tuple(data), datalist)
 
+    def test_user_list_user(self):
+        arglist = [
+            '--user', identity_fakes.user_id,
+        ]
+        verifylist = [
+            ('user', identity_fakes.user_id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = {
+            'domain': 'default',
+            'user': self.users_mock.get(),
+        }
+        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
+        self.roles_mock.list.assert_called_with(
+            **kwargs
+        )
+
+        collist = ('ID', 'Name')
+        self.assertEqual(columns, collist)
+        datalist = ((
+            identity_fakes.role_id,
+            identity_fakes.role_name,
+        ), )
+        self.assertEqual(tuple(data), datalist)
+
+    def test_role_list_domain_user(self):
+        arglist = [
+            '--domain', identity_fakes.domain_name,
+            '--user', identity_fakes.user_id,
+        ]
+        verifylist = [
+            ('domain', identity_fakes.domain_name),
+            ('user', identity_fakes.user_id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = {
+            'domain': self.domains_mock.get(),
+            'user': self.users_mock.get(),
+        }
+        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
+        self.roles_mock.list.assert_called_with(
+            **kwargs
+        )
+
+        collist = ('ID', 'Name', 'Domain', 'User')
+        self.assertEqual(columns, collist)
+        datalist = ((
+            identity_fakes.role_id,
+            identity_fakes.role_name,
+            identity_fakes.domain_name,
+            identity_fakes.user_name,
+        ), )
+        self.assertEqual(tuple(data), datalist)
+
+    def test_role_list_domain_group(self):
+        arglist = [
+            '--domain', identity_fakes.domain_name,
+            '--group', identity_fakes.group_id,
+        ]
+        verifylist = [
+            ('domain', identity_fakes.domain_name),
+            ('group', identity_fakes.group_id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = {
+            'domain': self.domains_mock.get(),
+            'group': self.groups_mock.get(),
+        }
+        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
+        self.roles_mock.list.assert_called_with(
+            **kwargs
+        )
+
+        collist = ('ID', 'Name', 'Domain', 'Group')
+        self.assertEqual(columns, collist)
+        datalist = ((
+            identity_fakes.role_id,
+            identity_fakes.role_name,
+            identity_fakes.domain_name,
+            identity_fakes.group_name,
+        ), )
+        self.assertEqual(tuple(data), datalist)
+
+    def test_role_list_project_user(self):
+        arglist = [
+            '--project', identity_fakes.project_name,
+            '--user', identity_fakes.user_id,
+        ]
+        verifylist = [
+            ('project', identity_fakes.project_name),
+            ('user', identity_fakes.user_id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = {
+            'project': self.projects_mock.get(),
+            'user': self.users_mock.get(),
+        }
+        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
+        self.roles_mock.list.assert_called_with(
+            **kwargs
+        )
+
+        collist = ('ID', 'Name', 'Project', 'User')
+        self.assertEqual(columns, collist)
+        datalist = ((
+            identity_fakes.role_id,
+            identity_fakes.role_name,
+            identity_fakes.project_name,
+            identity_fakes.user_name,
+        ), )
+        self.assertEqual(tuple(data), datalist)
+
+    def test_role_list_project_group(self):
+        arglist = [
+            '--project', identity_fakes.project_name,
+            '--group', identity_fakes.group_id,
+        ]
+        verifylist = [
+            ('project', identity_fakes.project_name),
+            ('group', identity_fakes.group_id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = {
+            'project': self.projects_mock.get(),
+            'group': self.groups_mock.get(),
+        }
+        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
+        self.roles_mock.list.assert_called_with(
+            **kwargs
+        )
+
+        collist = ('ID', 'Name', 'Project', 'Group')
+        self.assertEqual(columns, collist)
+        datalist = ((
+            identity_fakes.role_id,
+            identity_fakes.role_name,
+            identity_fakes.project_name,
+            identity_fakes.group_name,
+        ), )
+        self.assertEqual(tuple(data), datalist)
+
 
 class TestRoleRemove(TestRole):
 
diff --git a/openstackclient/tests/identity/v3/test_user.py b/openstackclient/tests/identity/v3/test_user.py
index 093d919b..b9c060d7 100644
--- a/openstackclient/tests/identity/v3/test_user.py
+++ b/openstackclient/tests/identity/v3/test_user.py
@@ -476,33 +476,6 @@ class TestUserList(TestUser):
     def setUp(self):
         super(TestUserList, self).setUp()
 
-        self.domains_mock.get.return_value = fakes.FakeResource(
-            None,
-            copy.deepcopy(identity_fakes.DOMAIN),
-            loaded=True,
-        )
-
-        self.projects_mock.get.return_value = fakes.FakeResource(
-            None,
-            copy.deepcopy(identity_fakes.PROJECT),
-            loaded=True,
-        )
-        self.projects_mock.list.return_value = [
-            fakes.FakeResource(
-                None,
-                copy.deepcopy(identity_fakes.PROJECT),
-                loaded=True,
-            ),
-        ]
-
-        self.roles_mock.list.return_value = [
-            fakes.FakeResource(
-                None,
-                copy.deepcopy(identity_fakes.ROLE),
-                loaded=True,
-            ),
-        ]
-
         self.users_mock.get.return_value = fakes.FakeResource(
             None,
             copy.deepcopy(identity_fakes.USER),
@@ -537,154 +510,6 @@ class TestUserList(TestUser):
         ), )
         self.assertEqual(tuple(data), datalist)
 
-    def test_user_list_project(self):
-        arglist = [
-            '--project', identity_fakes.project_id,
-        ]
-        verifylist = [
-            ('project', identity_fakes.project_id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        self.users_mock.list.assert_called_with()
-
-        collist = ('ID', 'Name')
-        self.assertEqual(columns, collist)
-        datalist = ((
-            identity_fakes.user_id,
-            identity_fakes.user_name,
-        ), )
-        self.assertEqual(tuple(data), datalist)
-
-    def test_user_list_domain(self):
-        arglist = [
-            '--domain', identity_fakes.domain_id,
-        ]
-        verifylist = [
-            ('domain', identity_fakes.domain_id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        self.users_mock.list.assert_called_with()
-
-        collist = ('ID', 'Name')
-        self.assertEqual(columns, collist)
-        datalist = ((
-            identity_fakes.user_id,
-            identity_fakes.user_name,
-        ), )
-        self.assertEqual(tuple(data), datalist)
-
-    def test_user_list_role_user(self):
-        arglist = [
-            '--role',
-            identity_fakes.user_id,
-        ]
-        verifylist = [
-            ('role', True),
-            ('user', identity_fakes.user_id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'domain': 'default',
-            'user': self.users_mock.get(),
-        }
-        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
-        self.roles_mock.list.assert_called_with(
-            **kwargs
-        )
-
-        collist = ('ID', 'Name')
-        self.assertEqual(columns, collist)
-        datalist = ((
-            identity_fakes.role_id,
-            identity_fakes.role_name,
-        ), )
-        self.assertEqual(tuple(data), datalist)
-
-    def test_user_list_role_domain(self):
-        arglist = [
-            '--domain', identity_fakes.domain_name,
-            '--role',
-            identity_fakes.user_id,
-        ]
-        verifylist = [
-            ('domain', identity_fakes.domain_name),
-            ('role', True),
-            ('user', identity_fakes.user_id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'domain': self.domains_mock.get(),
-            'user': self.users_mock.get(),
-        }
-        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
-        self.roles_mock.list.assert_called_with(
-            **kwargs
-        )
-
-        collist = ('ID', 'Name', 'Domain', 'User')
-        self.assertEqual(columns, collist)
-        datalist = ((
-            identity_fakes.role_id,
-            identity_fakes.role_name,
-            identity_fakes.domain_name,
-            identity_fakes.user_name,
-        ), )
-        self.assertEqual(tuple(data), datalist)
-
-    def test_user_list_role_project(self):
-        arglist = [
-            '--project', identity_fakes.project_name,
-            '--role',
-            identity_fakes.user_id,
-        ]
-        verifylist = [
-            ('project', identity_fakes.project_name),
-            ('role', True),
-            ('user', identity_fakes.user_id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'project': self.projects_mock.get(),
-            'user': self.users_mock.get(),
-        }
-        # RoleManager.list(user=, group=, domain=, project=, **kwargs)
-        self.roles_mock.list.assert_called_with(
-            **kwargs
-        )
-
-        collist = ('ID', 'Name', 'Project', 'User')
-        self.assertEqual(columns, collist)
-        datalist = ((
-            identity_fakes.role_id,
-            identity_fakes.role_name,
-            identity_fakes.project_name,
-            identity_fakes.user_name,
-        ), )
-        self.assertEqual(tuple(data), datalist)
-
     def test_user_list_long(self):
         arglist = [
             '--long',