Use cliff formattable columns in identity commands

Partial-Bug: #1687955
Partially implement blueprint osc-formattable-columns

Change-Id: Ia13314a012b3a7363ffb24a13c79c6ecdff1ed7b

11 files changed, 140 insertions, 87 deletions

diff --git a/openstackclient/identity/v2_0/catalog.py b/openstackclient/identity/v2_0/catalog.py
index 993bdd53..5d1e3062 100644
--- a/openstackclient/identity/v2_0/catalog.py
+++ b/openstackclient/identity/v2_0/catalog.py
@@ -15,6 +15,7 @@
 
 import logging
 
+from cliff import columns as cliff_columns
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -26,20 +27,21 @@ from openstackclient.i18n import _
 LOG = logging.getLogger(__name__)
 
 
-def _format_endpoints(eps=None):
-    if not eps:
-        return ""
-    ret = ''
-    for index, ep in enumerate(eps):
-        region = eps[index].get('region')
-        if region is None:
-            region = '<none>'
-        ret += region + '\n'
-        for endpoint_type in ['publicURL', 'internalURL', 'adminURL']:
-            url = eps[index].get(endpoint_type)
-            if url:
-                ret += "  %s: %s\n" % (endpoint_type, url)
-    return ret
+class EndpointsColumn(cliff_columns.FormattableColumn):
+    def human_readable(self):
+        if not self._value:
+            return ""
+        ret = ''
+        for ep in self._value:
+            region = ep.get('region')
+            if region is None:
+                region = '<none>'
+            ret += region + '\n'
+            for endpoint_type in ['publicURL', 'internalURL', 'adminURL']:
+                url = ep.get(endpoint_type)
+                if url:
+                    ret += "  %s: %s\n" % (endpoint_type, url)
+        return ret
 
 
 class ListCatalog(command.Lister):
@@ -60,7 +62,7 @@ class ListCatalog(command.Lister):
                 (utils.get_dict_properties(
                     s, columns,
                     formatters={
-                        'Endpoints': _format_endpoints,
+                        'Endpoints': EndpointsColumn,
                     },
                 ) for s in data))
 
@@ -91,7 +93,7 @@ class ShowCatalog(command.ShowOne):
             if (service.get('name') == parsed_args.service or
                     service.get('type') == parsed_args.service):
                 data = service
-                data['endpoints'] = _format_endpoints(data['endpoints'])
+                data['endpoints'] = EndpointsColumn(data['endpoints'])
                 if 'endpoints_links' in data:
                     data.pop('endpoints_links')
                 break
diff --git a/openstackclient/identity/v2_0/project.py b/openstackclient/identity/v2_0/project.py
index 04d422ec..df57574b 100644
--- a/openstackclient/identity/v2_0/project.py
+++ b/openstackclient/identity/v2_0/project.py
@@ -18,6 +18,7 @@
 import logging
 
 from keystoneauth1 import exceptions as ks_exc
+from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -297,7 +298,7 @@ class ShowProject(command.ShowOne):
                 if v is not None:
                     properties[k] = v
 
-        info['properties'] = utils.format_dict(properties)
+        info['properties'] = format_columns.DictColumn(properties)
         return zip(*sorted(six.iteritems(info)))
 
 
diff --git a/openstackclient/identity/v2_0/user.py b/openstackclient/identity/v2_0/user.py
index 2a3dde6b..0675877b 100644
--- a/openstackclient/identity/v2_0/user.py
+++ b/openstackclient/identity/v2_0/user.py
@@ -15,8 +15,10 @@
 
 """Identity v2.0 User action implementations"""
 
+import functools
 import logging
 
+from cliff import columns as cliff_columns
 from keystoneauth1 import exceptions as ks_exc
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -29,6 +31,31 @@ from openstackclient.i18n import _
 LOG = logging.getLogger(__name__)
 
 
+class ProjectColumn(cliff_columns.FormattableColumn):
+    """Formattable column for project column.
+
+    Unlike the parent FormattableColumn class, the initializer of the
+    class takes project_cache as the second argument.
+    osc_lib.utils.get_item_properties instantiate cliff FormattableColumn
+    object with a single parameter "column value", so you need to pass
+    a partially initialized class like
+    ``functools.partial(ProjectColumn, project_cache)``.
+    """
+
+    def __init__(self, value, project_cache=None):
+        super(ProjectColumn, self).__init__(value)
+        self.project_cache = project_cache or {}
+
+    def human_readable(self):
+        project = self._value
+        if not project:
+            return ""
+        if project in self.project_cache.keys():
+            return self.project_cache[project].name
+        else:
+            return project
+
+
 class CreateUser(command.ShowOne):
     _description = _("Create new user")
 
@@ -187,15 +214,7 @@ class ListUser(command.Lister):
 
     def take_action(self, parsed_args):
         identity_client = self.app.client_manager.identity
-
-        def _format_project(project):
-            if not project:
-                return ""
-            if project in project_cache.keys():
-                return project_cache[project].name
-            else:
-                return project
-
+        formatters = {}
         project = None
         if parsed_args.project:
             project = utils.find_resource(
@@ -227,6 +246,8 @@ class ListUser(command.Lister):
             except Exception:
                 # Just forget it if there's any trouble
                 pass
+            formatters['tenantId'] = functools.partial(
+                ProjectColumn, project_cache=project_cache)
         else:
             columns = column_headers = ('ID', 'Name')
         data = identity_client.users.list(tenant_id=project)
@@ -251,7 +272,7 @@ class ListUser(command.Lister):
                 (utils.get_item_properties(
                     s, columns,
                     mixed_case_fields=('tenantId',),
-                    formatters={'tenantId': _format_project},
+                    formatters=formatters,
                 ) for s in data))
 
 
diff --git a/openstackclient/identity/v3/catalog.py b/openstackclient/identity/v3/catalog.py
index 28f4fada..59430c4c 100644
--- a/openstackclient/identity/v3/catalog.py
+++ b/openstackclient/identity/v3/catalog.py
@@ -15,6 +15,7 @@
 
 import logging
 
+from cliff import columns as cliff_columns
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -26,15 +27,16 @@ from openstackclient.i18n import _
 LOG = logging.getLogger(__name__)
 
 
-def _format_endpoints(eps=None):
-    if not eps:
-        return ""
-    ret = ''
-    for ep in eps:
-        region = ep.get('region_id') or ep.get('region') or '<none>'
-        ret += region + '\n'
-        ret += "  %s: %s\n" % (ep['interface'], ep['url'])
-    return ret
+class EndpointsColumn(cliff_columns.FormattableColumn):
+    def human_readable(self):
+        if not self._value:
+            return ""
+        ret = ''
+        for ep in self._value:
+            region = ep.get('region_id') or ep.get('region') or '<none>'
+            ret += region + '\n'
+            ret += "  %s: %s\n" % (ep['interface'], ep['url'])
+        return ret
 
 
 class ListCatalog(command.Lister):
@@ -55,7 +57,7 @@ class ListCatalog(command.Lister):
                 (utils.get_dict_properties(
                     s, columns,
                     formatters={
-                        'Endpoints': _format_endpoints,
+                        'Endpoints': EndpointsColumn,
                     },
                 ) for s in data))
 
@@ -86,7 +88,7 @@ class ShowCatalog(command.ShowOne):
             if (service.get('name') == parsed_args.service or
                     service.get('type') == parsed_args.service):
                 data = dict(service)
-                data['endpoints'] = _format_endpoints(data['endpoints'])
+                data['endpoints'] = EndpointsColumn(data['endpoints'])
                 if 'links' in data:
                     data.pop('links')
                 break
diff --git a/openstackclient/identity/v3/identity_provider.py b/openstackclient/identity/v3/identity_provider.py
index d8951d31..b3315182 100644
--- a/openstackclient/identity/v3/identity_provider.py
+++ b/openstackclient/identity/v3/identity_provider.py
@@ -15,6 +15,7 @@
 
 import logging
 
+from osc_lib.cli import format_columns
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -103,7 +104,7 @@ class CreateIdentityProvider(command.ShowOne):
             enabled=parsed_args.enabled)
 
         idp._info.pop('links', None)
-        remote_ids = utils.format_list(idp._info.pop('remote_ids', []))
+        remote_ids = format_columns.ListColumn(idp._info.pop('remote_ids', []))
         idp._info['remote_ids'] = remote_ids
         return zip(*sorted(six.iteritems(idp._info)))
 
@@ -245,6 +246,6 @@ class ShowIdentityProvider(command.ShowOne):
             id=parsed_args.identity_provider)
 
         idp._info.pop('links', None)
-        remote_ids = utils.format_list(idp._info.pop('remote_ids', []))
+        remote_ids = format_columns.ListColumn(idp._info.pop('remote_ids', []))
         idp._info['remote_ids'] = remote_ids
         return zip(*sorted(six.iteritems(idp._info)))
diff --git a/openstackclient/tests/unit/identity/v2_0/test_catalog.py b/openstackclient/tests/unit/identity/v2_0/test_catalog.py
index 65af58a2..362dec08 100644
--- a/openstackclient/tests/unit/identity/v2_0/test_catalog.py
+++ b/openstackclient/tests/unit/identity/v2_0/test_catalog.py
@@ -71,17 +71,9 @@ class TestCatalogList(TestCatalog):
         datalist = ((
             'supernova',
             'compute',
-            'one\n  publicURL: https://public.one.example.com\n  '
-            'internalURL: https://internal.one.example.com\n  '
-            'adminURL: https://admin.one.example.com\n'
-            'two\n  publicURL: https://public.two.example.com\n  '
-            'internalURL: https://internal.two.example.com\n  '
-            'adminURL: https://admin.two.example.com\n'
-            '<none>\n  publicURL: https://public.none.example.com\n  '
-            'internalURL: https://internal.none.example.com\n  '
-            'adminURL: https://admin.none.example.com\n',
+            catalog.EndpointsColumn(self.service_catalog['endpoints']),
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
     def test_catalog_list_with_endpoint_url(self):
         attr = {
@@ -121,11 +113,9 @@ class TestCatalogList(TestCatalog):
         datalist = ((
             'supernova',
             'compute',
-            'one\n  publicURL: https://public.one.example.com\n'
-            'two\n  publicURL: https://public.two.example.com\n  '
-            'internalURL: https://internal.two.example.com\n'
+            catalog.EndpointsColumn(service_catalog['endpoints']),
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
 
 class TestCatalogShow(TestCatalog):
@@ -160,6 +150,18 @@ class TestCatalogShow(TestCatalog):
         collist = ('endpoints', 'id', 'name', 'type')
         self.assertEqual(collist, columns)
         datalist = (
+            catalog.EndpointsColumn(self.service_catalog['endpoints']),
+            self.service_catalog.id,
+            'supernova',
+            'compute',
+        )
+        self.assertItemEqual(datalist, data)
+
+
+class TestFormatColumns(TestCatalog):
+    def test_endpoints_column_human_readabale(self):
+        col = catalog.EndpointsColumn(self.service_catalog['endpoints'])
+        self.assertEqual(
             'one\n  publicURL: https://public.one.example.com\n  '
             'internalURL: https://internal.one.example.com\n  '
             'adminURL: https://admin.one.example.com\n'
@@ -169,8 +171,23 @@ class TestCatalogShow(TestCatalog):
             '<none>\n  publicURL: https://public.none.example.com\n  '
             'internalURL: https://internal.none.example.com\n  '
             'adminURL: https://admin.none.example.com\n',
-            self.service_catalog.id,
-            'supernova',
-            'compute',
-        )
-        self.assertEqual(datalist, data)
+            col.human_readable())
+
+    def test_endpoints_column_human_readable_with_partial_endpoint_urls(self):
+        endpoints = [
+            {
+                'region': 'one',
+                'publicURL': 'https://public.one.example.com',
+            },
+            {
+                'region': 'two',
+                'publicURL': 'https://public.two.example.com',
+                'internalURL': 'https://internal.two.example.com',
+            },
+        ]
+        col = catalog.EndpointsColumn(endpoints)
+        self.assertEqual(
+            'one\n  publicURL: https://public.one.example.com\n'
+            'two\n  publicURL: https://public.two.example.com\n  '
+            'internalURL: https://internal.two.example.com\n',
+            col.human_readable())
diff --git a/openstackclient/tests/unit/identity/v2_0/test_project.py b/openstackclient/tests/unit/identity/v2_0/test_project.py
index c726f2a6..7af7b394 100644
--- a/openstackclient/tests/unit/identity/v2_0/test_project.py
+++ b/openstackclient/tests/unit/identity/v2_0/test_project.py
@@ -16,6 +16,7 @@
 import mock
 
 from keystoneauth1 import exceptions as ks_exc
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils
 
@@ -640,9 +641,9 @@ class TestProjectShow(TestProject):
             True,
             self.fake_proj_show.id,
             self.fake_proj_show.name,
-            '',
+            format_columns.DictColumn({}),
         )
-        self.assertEqual(datalist, data)
+        self.assertItemEqual(datalist, data)
 
 
 class TestProjectUnset(TestProject):
diff --git a/openstackclient/tests/unit/identity/v2_0/test_user.py b/openstackclient/tests/unit/identity/v2_0/test_user.py
index a8b9497e..0a0d4b36 100644
--- a/openstackclient/tests/unit/identity/v2_0/test_user.py
+++ b/openstackclient/tests/unit/identity/v2_0/test_user.py
@@ -482,7 +482,7 @@ class TestUserList(TestUser):
         self.users_mock.list.assert_called_with(tenant_id=None)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_user_list_project(self):
         arglist = [
@@ -502,7 +502,7 @@ class TestUserList(TestUser):
         self.users_mock.list.assert_called_with(tenant_id=project_id)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_user_list_long(self):
         arglist = [
@@ -525,11 +525,13 @@ class TestUserList(TestUser):
         datalist = ((
             self.fake_user_l.id,
             self.fake_user_l.name,
-            self.fake_project_l.name,
+            user.ProjectColumn(
+                self.fake_project_l.id,
+                {self.fake_project_l.id: self.fake_project_l}),
             self.fake_user_l.email,
             True,
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
 
 class TestUserSet(TestUser):
@@ -817,4 +819,4 @@ class TestUserShow(TestUser):
             self.fake_user.name,
             self.fake_project.id,
         )
-        self.assertEqual(datalist, data)
+        self.assertItemEqual(datalist, data)
diff --git a/openstackclient/tests/unit/identity/v3/fakes.py b/openstackclient/tests/unit/identity/v3/fakes.py
index 27ee9fd0..5caf156b 100644
--- a/openstackclient/tests/unit/identity/v3/fakes.py
+++ b/openstackclient/tests/unit/identity/v3/fakes.py
@@ -20,6 +20,7 @@ import uuid
 from keystoneauth1 import access
 from keystoneauth1 import fixture
 import mock
+from osc_lib.cli import format_columns
 
 from openstackclient.tests.unit import fakes
 from openstackclient.tests.unit import utils
@@ -300,7 +301,7 @@ TOKEN_WITH_DOMAIN_ID = {
 idp_id = 'test_idp'
 idp_description = 'super exciting IdP description'
 idp_remote_ids = ['entity1', 'entity2']
-formatted_idp_remote_ids = 'entity1, entity2'
+formatted_idp_remote_ids = format_columns.ListColumn(idp_remote_ids)
 
 IDENTITY_PROVIDER = {
     'id': idp_id,
diff --git a/openstackclient/tests/unit/identity/v3/test_catalog.py b/openstackclient/tests/unit/identity/v3/test_catalog.py
index 53008e8c..ba076dbd 100644
--- a/openstackclient/tests/unit/identity/v3/test_catalog.py
+++ b/openstackclient/tests/unit/identity/v3/test_catalog.py
@@ -91,12 +91,9 @@ class TestCatalogList(TestCatalog):
         datalist = ((
             'supernova',
             'compute',
-            'onlyone\n  public: https://public.example.com\n'
-            'onlyone\n  admin: https://admin.example.com\n'
-            '<none>\n  internal: https://internal.example.com\n'
-            '<none>\n  none: https://none.example.com\n',
+            catalog.EndpointsColumn(self.fake_service['endpoints']),
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
 
 class TestCatalogShow(TestCatalog):
@@ -131,12 +128,20 @@ class TestCatalogShow(TestCatalog):
         collist = ('endpoints', 'id', 'name', 'type')
         self.assertEqual(collist, columns)
         datalist = (
-            'onlyone\n  public: https://public.example.com\nonlyone\n'
-            '  admin: https://admin.example.com\n'
-            '<none>\n  internal: https://internal.example.com\n'
-            '<none>\n  none: https://none.example.com\n',
+            catalog.EndpointsColumn(self.fake_service['endpoints']),
             'qwertyuiop',
             'supernova',
             'compute',
         )
-        self.assertEqual(datalist, data)
+        self.assertItemEqual(datalist, data)
+
+
+class TestFormatColumns(TestCatalog):
+    def test_endpoints_column_human_readabale(self):
+        col = catalog.EndpointsColumn(self.fake_service['endpoints'])
+        self.assertEqual(
+            'onlyone\n  public: https://public.example.com\n'
+            'onlyone\n  admin: https://admin.example.com\n'
+            '<none>\n  internal: https://internal.example.com\n'
+            '<none>\n  none: https://none.example.com\n',
+            col.human_readable())
diff --git a/openstackclient/tests/unit/identity/v3/test_identity_provider.py b/openstackclient/tests/unit/identity/v3/test_identity_provider.py
index dc82ab74..0163c6c8 100644
--- a/openstackclient/tests/unit/identity/v3/test_identity_provider.py
+++ b/openstackclient/tests/unit/identity/v3/test_identity_provider.py
@@ -90,7 +90,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_description(self):
         arglist = [
@@ -118,7 +118,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_remote_id(self):
         arglist = [
@@ -146,7 +146,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_remote_ids_multiple(self):
         arglist = [
@@ -175,7 +175,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_remote_ids_file(self):
         arglist = [
@@ -208,7 +208,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_disabled(self):
 
@@ -251,7 +251,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
             identity_fakes.idp_id,
             identity_fakes.formatted_idp_remote_ids
         )
-        self.assertEqual(datalist, data)
+        self.assertItemEqual(datalist, data)
 
     def test_create_identity_provider_domain_name(self):
         arglist = [
@@ -279,7 +279,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
     def test_create_identity_provider_domain_id(self):
         arglist = [
@@ -307,7 +307,7 @@ class TestIdentityProviderCreate(TestIdentityProvider):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertItemEqual(self.datalist, data)
 
 
 class TestIdentityProviderDelete(TestIdentityProvider):
@@ -383,7 +383,7 @@ class TestIdentityProviderList(TestIdentityProvider):
             identity_fakes.domain_id,
             identity_fakes.idp_description,
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
 
 class TestIdentityProviderSet(TestIdentityProvider):
@@ -668,4 +668,4 @@ class TestIdentityProviderShow(TestIdentityProvider):
             identity_fakes.idp_id,
             identity_fakes.formatted_idp_remote_ids
         )
-        self.assertEqual(datalist, data)
+        self.assertItemEqual(datalist, data)