Merge "Improve readability of vnflcm show command" into stable/ussuri

Display show command data in human_readable format for better
readability for complex attributes of vnf instance like
'vimConnectionInfo', 'instantiatedVnfInfo', '_links'.

Please checkout below link to see the difference in vnflcm show
response data before and after resolving the issue.
http://paste.openstack.org/show/792866

Change-Id: I52684618f7042f5566ea3bed7255d17d2fd61df2

.gitreview

@@ -2,3 +2,4 @@
 host=review.opendev.org
 port=29418
 project=openstack/python-tackerclient.git
+defaultbranch=stable/ussuri

tackerclient/osc/utils.py

@@ -22,6 +22,7 @@ to this module. They should go to tackerclient.osc.v1.utils.
 
 import operator
 
+from cliff import columns as cliff_columns
 from keystoneclient import exceptions as identity_exc
 from keystoneclient.v3 import domains
 from keystoneclient.v3 import projects
@@ -205,3 +206,9 @@ def _find_identity_resource(identity_client_manager, name_or_id,
 
 
 # The above are borrowed from openstackclient.identity.common.
+
+
+class FormatComplexDataColumn(cliff_columns.FormattableColumn):
+
+    def human_readable(self):
+        return format_dict_with_indention(self._value)

tackerclient/osc/v1/vnflcm/vnflcm.py

@@ -18,7 +18,6 @@ import logging
 import os
 import time
 
-from osc_lib.cli import format_columns
 from osc_lib.command import command
 from osc_lib import utils
 
@@ -52,6 +51,10 @@ EXTRA_WAITING_TIME = 10
 
 SLEEP_TIME = 1
 
+formatters = {'vimConnectionInfo': tacker_osc_utils.FormatComplexDataColumn,
+              'instantiatedVnfInfo': tacker_osc_utils.FormatComplexDataColumn,
+              '_links': tacker_osc_utils.FormatComplexDataColumn}
+
 
 def _get_columns(vnflcm_obj, action=None):
     column_map = {
@@ -132,9 +135,9 @@ class CreateVnfLcm(command.ShowOne):
                 print((_('VNF Instance %(id)s is created and instantiation'
                          ' request has been accepted.') % {'id': vnf['id']}))
         display_columns, columns = _get_columns(vnf)
-        data = utils.get_item_properties(
-            sdk_utils.DictModel(vnf),
-            columns, mixed_case_fields=_mixed_case_fields)
+        data = utils.get_item_properties(sdk_utils.DictModel(vnf),
+                                         columns, formatters=formatters,
+                                         mixed_case_fields=_mixed_case_fields)
         return (display_columns, data)
 
 
@@ -156,7 +159,7 @@ class ShowVnfLcm(command.ShowOne):
         data = utils.get_item_properties(
             sdk_utils.DictModel(obj),
             columns, mixed_case_fields=_mixed_case_fields,
-            formatters={'instantiatedVnfInfo': format_columns.DictColumn})
+            formatters=formatters)
         return (display_columns, data)
 
 

tackerclient/osc/v1/vnfpkgm/vnf_package.py

@@ -29,6 +29,12 @@ from tackerclient.osc import utils as tacker_osc_utils
 LOG = logging.getLogger(__name__)
 
 
+formatters = {'softwareImages': tacker_osc_utils.FormatComplexDataColumn,
+              'checksum': tacker_osc_utils.FormatComplexDataColumn,
+              '_links': tacker_osc_utils.FormatComplexDataColumn,
+              'userDefinedData': tacker_osc_utils.FormatComplexDataColumn}
+
+
 _mixed_case_fields = ('usageState', 'onboardingState', 'operationalState',
                       'vnfProductName', 'softwareImages', 'userDefinedData',
                       'vnfdId', 'vnfdVersion', 'vnfSoftwareVersion',
@@ -87,7 +93,8 @@ class CreateVnfPackage(command.ShowOne):
         display_columns, columns = _get_columns(vnf_package)
         data = utils.get_item_properties(
             sdk_utils.DictModel(vnf_package),
-            columns, mixed_case_fields=_mixed_case_fields)
+            columns, formatters=formatters,
+            mixed_case_fields=_mixed_case_fields)
         return (display_columns, data)
 
 
@@ -136,12 +143,12 @@ class ListVnfPackage(command.Lister):
             lambda x, y: x + (' ' if y.isupper() else '') + y, field).title()
 
     def get_attributes(self, extra_fields=None, all_fields=False,
-                       exclude_fields=None):
+                       exclude_fields=None, exclude_default=False):
         fields = ['id', 'vnfProductName', 'onboardingState',
-                  'usageState', 'operationalState']
+                  'usageState', 'operationalState', '_links']
         complex_fields = ['checksum', 'softwareImages', 'userDefinedData']
         simple_fields = ['vnfdVersion', 'vnfProvider', 'vnfSoftwareVersion',
-                         'vnfdId', '_links']
+                         'vnfdId']
 
         if extra_fields:
             fields.extend(extra_fields)
@@ -153,6 +160,9 @@ class ListVnfPackage(command.Lister):
             fields.extend(complex_fields)
             fields.extend(simple_fields)
 
+        if exclude_default:
+            fields.extend(simple_fields)
+
         attrs = []
         for field in fields:
             if field == '_links':
@@ -168,6 +178,7 @@ class ListVnfPackage(command.Lister):
         extra_fields = []
         exclude_fields = []
         all_fields = False
+        exclude_default = False
         if parsed_args.filter:
             _params['filter'] = parsed_args.filter
         if parsed_args.fields:
@@ -181,6 +192,7 @@ class ListVnfPackage(command.Lister):
             exclude_fields.extend(fields)
         if parsed_args.exclude_default:
             _params['exclude_default'] = None
+            exclude_default = True
         if parsed_args.all_fields:
             _params['all_fields'] = None
             all_fields = True
@@ -188,11 +200,12 @@ class ListVnfPackage(command.Lister):
         client = self.app.client_manager.tackerclient
         data = client.list_vnf_packages(**_params)
         headers, columns = tacker_osc_utils.get_column_definitions(
-            self.get_attributes(extra_fields, all_fields, exclude_fields),
-            long_listing=True)
+            self.get_attributes(extra_fields, all_fields, exclude_fields,
+                                exclude_default), long_listing=True)
         return (headers,
                 (utils.get_dict_properties(
-                    s, columns, mixed_case_fields=_mixed_case_fields,
+                    s, columns, formatters=formatters,
+                    mixed_case_fields=_mixed_case_fields,
                 ) for s in data['vnf_packages']))
 
 
@@ -215,7 +228,8 @@ class ShowVnfPackage(command.ShowOne):
         display_columns, columns = _get_columns(vnf_package)
         data = utils.get_item_properties(
             sdk_utils.DictModel(vnf_package),
-            columns, mixed_case_fields=_mixed_case_fields)
+            columns, formatters=formatters,
+            mixed_case_fields=_mixed_case_fields)
         return (display_columns, data)
 
 
@@ -452,5 +466,6 @@ class UpdateVnfPackage(command.ShowOne):
         display_columns, columns = self.get_columns(updated_values)
         data = utils.get_item_properties(
             sdk_utils.DictModel(updated_values),
-            columns, mixed_case_fields=_mixed_case_fields)
+            columns, formatters=formatters,
+            mixed_case_fields=_mixed_case_fields)
         return (display_columns, data)

tackerclient/tests/unit/osc/base.py

@@ -17,6 +17,8 @@ import mock
 from requests_mock.contrib import fixture as requests_mock_fixture
 import testtools
 
+from cliff import columns as cliff_columns
+
 
 class FixturedTestCase(testtools.TestCase):
     client_fixture_class = None
@@ -51,6 +53,20 @@ class FixturedTestCase(testtools.TestCase):
                 msg = 'method %s should not have been called' % m
             self.fail(msg)
 
+    def assertListItemsEqual(self, expected, actual):
+        """Assertion based on human_readable values of list items"""
+
+        self.assertEqual(len(expected), len(actual))
+        for col_expected, col_actual in zip(expected, actual):
+            if isinstance(col_actual, tuple):
+                self.assertListItemsEqual(col_expected, col_actual)
+            elif isinstance(col_expected, cliff_columns.FormattableColumn):
+                self.assertIsInstance(col_actual, col_expected.__class__)
+                self.assertEqual(col_expected.human_readable(),
+                                 col_actual.human_readable())
+            else:
+                self.assertEqual(col_expected, col_actual)
+
 
 class ParserException(Exception):
     pass

tackerclient/tests/unit/osc/v1/test_vnf_package.py

@@ -104,8 +104,9 @@ class TestCreateVnfPackage(TestVnfPackage):
 
         columns, data = (self.create_vnf_package.take_action(parsed_args))
         self.assertCountEqual(_get_columns_vnf_package(), columns)
-        self.assertCountEqual(vnf_package_fakes.get_vnf_package_data(json),
-                              data)
+        headers, attributes = vnf_package._get_columns(json)
+        self.assertListItemsEqual(vnf_package_fakes.get_vnf_package_data(
+            json, columns=attributes), data)
 
 
 @ddt.ddt
@@ -122,12 +123,12 @@ class TestListVnfPackage(TestVnfPackage):
             count=3, onboarded_vnf_package=onboarded_vnf_package)
 
     def get_list_columns(self, all_fields=False, exclude_fields=None,
-                         extra_fields=None):
+                         extra_fields=None, exclude_default=False):
 
         columns = ['Id', 'Vnf Product Name', 'Onboarding State', 'Usage State',
-                   'Operational State']
+                   'Operational State', 'Links']
         complex_columns = ['Checksum', 'Software Images', 'User Defined Data']
-        simple_columns = ['Vnfd Version', 'Vnf Provider', 'Vnfd Id', 'Links',
+        simple_columns = ['Vnfd Version', 'Vnf Provider', 'Vnfd Id',
                           'Vnf Software Version']
 
         if extra_fields:
@@ -140,6 +141,9 @@ class TestListVnfPackage(TestVnfPackage):
             columns.extend(complex_columns)
             columns.extend(simple_columns)
 
+        if exclude_default:
+            columns.extend(simple_columns)
+
         return columns
 
     def _get_mock_response_for_list_vnf_packages(
@@ -163,7 +167,7 @@ class TestListVnfPackage(TestVnfPackage):
             expected_data.append(vnf_package_fakes.get_vnf_package_data(
                 vnf_package_obj, columns=columns, list_action=True))
         self.assertCountEqual(self.get_list_columns(), actual_columns)
-        self.assertCountEqual(expected_data, list(data))
+        self.assertListItemsEqual(expected_data, list(data))
 
     @ddt.data('all_fields', 'exclude_default')
     def test_take_action(self, arg):
@@ -177,7 +181,7 @@ class TestListVnfPackage(TestVnfPackage):
 
         actual_columns, data = self.list_vnf_package.take_action(parsed_args)
         expected_data = []
-        kwargs = {arg: True} if arg == 'all_fields' else {}
+        kwargs = {arg: True}
         headers, columns = tacker_osc_utils.get_column_definitions(
             self.list_vnf_package.get_attributes(**kwargs), long_listing=True)
 
@@ -186,7 +190,7 @@ class TestListVnfPackage(TestVnfPackage):
                 vnf_package_obj, columns=columns, list_action=True, **kwargs))
 
         self.assertCountEqual(self.get_list_columns(**kwargs), actual_columns)
-        self.assertCountEqual(expected_data, list(data))
+        self.assertListItemsEqual(expected_data, list(data))
 
     def test_take_action_with_exclude_fields(self):
         parsed_args = self.check_parser(
@@ -222,7 +226,7 @@ class TestListVnfPackage(TestVnfPackage):
             exclude_fields=['Software Images', 'Checksum',
                             'User Defined Data'])
         self.assertCountEqual(expected_columns, actual_columns)
-        self.assertCountEqual(expected_data, list(data))
+        self.assertListItemsEqual(expected_data, list(data))
 
     @ddt.data((['--all_fields', '--fields', 'softwareImages'],
                [('all_fields', True), ('fields', 'softwareImages')]),
@@ -254,7 +258,8 @@ class TestListVnfPackage(TestVnfPackage):
         expected_data = []
         headers, columns = tacker_osc_utils.get_column_definitions(
             self.list_vnf_package.get_attributes(
-                extra_fields=['softwareImages', 'checksum']),
+                extra_fields=['softwareImages', 'checksum'],
+                exclude_default=True),
             long_listing=True)
 
         for vnf_package_obj in updated_vnf_packages['vnf_packages']:
@@ -263,9 +268,10 @@ class TestListVnfPackage(TestVnfPackage):
                 exclude_default=True))
 
         self.assertCountEqual(self.get_list_columns(
-            extra_fields=['Software Images', 'Checksum']),
+            extra_fields=['Software Images', 'Checksum'],
+            exclude_default=True),
             actual_columns)
-        self.assertCountEqual(expected_data, list(data))
+        self.assertListItemsEqual(expected_data, list(data))
 
 
 @ddt.ddt
@@ -288,12 +294,13 @@ class TestShowVnfPackage(TestVnfPackage):
         self.requests_mock.register_uri('GET', url, json=vnf_package_obj,
                                         headers=self.header)
         columns, data = (self.show_vnf_package.take_action(parsed_args))
-        self.assertEqual(sorted(_get_columns_vnf_package(
-            vnf_package_obj=vnf_package_obj, action='show')), sorted(columns))
         self.assertCountEqual(_get_columns_vnf_package(
             vnf_package_obj=vnf_package_obj, action='show'), columns)
-        self.assertCountEqual(
-            vnf_package_fakes.get_vnf_package_data(vnf_package_obj), data)
+
+        headers, attributes = vnf_package._get_columns(vnf_package_obj)
+        self.assertListItemsEqual(
+            vnf_package_fakes.get_vnf_package_data(vnf_package_obj,
+                                                   columns=attributes), data)
 
     def test_show_no_options(self):
         self.assertRaises(base.ParserException, self.check_parser,
@@ -536,7 +543,7 @@ class TestUpdateVnfPackage(TestVnfPackage):
         columns, data = self.update_vnf_package.take_action(parsed_args)
         self.assertItemsEqual(_get_columns_vnf_package(
             vnf_package_obj=fake_response, action='update'), columns)
-        self.assertItemsEqual(
+        self.assertListItemsEqual(
             vnf_package_fakes.get_vnf_package_data(fake_response), data)
 
     def test_update_no_options(self):

tackerclient/tests/unit/osc/v1/test_vnflcm.py

@@ -105,7 +105,9 @@ class TestCreateVnfLcm(TestVnfLcm):
                 json={}, headers=self.header)
 
         sys.stdout = buffer = StringIO()
-        columns, data = (self.create_vnf_lcm.take_action(parsed_args))
+        actual_columns, data = (self.create_vnf_lcm.take_action(parsed_args))
+
+        headers, attributes = vnflcm._get_columns(json)
 
         expected_message = (
             'VNF Instance ' + json['id'] + ' is created and instantiation '
@@ -114,9 +116,9 @@ class TestCreateVnfLcm(TestVnfLcm):
             self.assertEqual(expected_message, buffer.getvalue().strip())
 
         self.assertItemsEqual(_get_columns_vnflcm(),
-                              columns)
-        self.assertItemsEqual(vnflcm_fakes.get_vnflcm_data(json),
-                              data)
+                              actual_columns)
+        self.assertListItemsEqual(vnflcm_fakes.get_vnflcm_data(
+            json, columns=attributes), data)
 
 
 class TestShowVnfLcm(TestVnfLcm):
@@ -145,6 +147,10 @@ class TestShowVnfLcm(TestVnfLcm):
         columns, data = (self.show_vnf_lcm.take_action(parsed_args))
         self.assertItemsEqual(_get_columns_vnflcm(action='show'),
                               columns)
+        headers, attributes = vnflcm._get_columns(vnf_instance, action='show')
+        self.assertListItemsEqual(
+            vnflcm_fakes.get_vnflcm_data(vnf_instance, columns=attributes),
+            data)
 
 
 class TestListVnfLcm(TestVnfLcm):

tackerclient/tests/unit/osc/v1/vnf_package_fakes.py

@@ -15,6 +15,8 @@
 
 from oslo_utils import uuidutils
 
+from tackerclient.osc import utils as tacker_osc_utils
+
 
 def vnf_package_obj(attrs=None, onboarded_state=False):
     """Create a fake vnf package.
@@ -34,7 +36,7 @@ def vnf_package_obj(attrs=None, onboarded_state=False):
                         "onboardingState": "CREATED",
                         "operationalState": "DISABLED",
                         "usageState": "NOT_IN_USE",
-                        "userDefinedData": None}
+                        "userDefinedData": {'key': 'value'}}
 
     if onboarded_state:
         fake_vnf_package = {"id": "60a6ac16-b50d-4e92-964b-b3cf98c7cf5c",
@@ -70,7 +72,7 @@ def vnf_package_obj(attrs=None, onboarded_state=False):
                             "onboardingState": "ONBOARDED",
                             "operationalState": "ENABLED",
                             "usageState": "IN_USE",
-                            "userDefinedData": None,
+                            "userDefinedData": {'key': 'value'},
                             "_links": {
                                 "self": {
                                     "href": "string"
@@ -88,10 +90,10 @@ def vnf_package_obj(attrs=None, onboarded_state=False):
     return fake_vnf_package
 
 
-def get_vnf_package_data(vnf_package, **kwargs):
+def get_vnf_package_data(vnf_package_obj, **kwargs):
     """Get the vnf package data from a FakeVnfPackage dict object.
 
-    :param vnf_package:
+    :param vnf_package_obj:
         A FakeVnfPackage dict object
     :return:
         A list which may include the following values:
@@ -99,19 +101,27 @@ def get_vnf_package_data(vnf_package, **kwargs):
         'vnfd': {'href': 'string'}}, '60a6ac16-b50d-4e92-964b-b3cf98c7cf5c',
         'CREATED', 'DISABLED', 'NOT_IN_USE', {'Test_key': 'Test_value'}]
     """
+    complex_attributes = ['softwareImages', 'checksum', '_links',
+                          'userDefinedData']
+    for attribute in complex_attributes:
+        if vnf_package_obj.get(attribute):
+            vnf_package_obj.update(
+                {attribute: tacker_osc_utils.FormatComplexDataColumn(
+                    vnf_package_obj[attribute])})
 
     if kwargs.get('list_action'):
         # In case of List VNF packages we get empty string as data for
         # 'vnfProductName' if onboardingState is CREATED. Hence to match
         # up with actual data we are adding here empty string.
-        if not vnf_package.get('vnfProductName'):
-            vnf_package['vnfProductName'] = ''
+        if not vnf_package_obj.get('vnfProductName'):
+            vnf_package_obj['vnfProductName'] = ''
 
-    # return the list of data as per column order
     if kwargs.get('columns'):
-        return tuple([vnf_package[key] for key in kwargs.get('columns')])
+        # return the list of data as per column order
+        return tuple([vnf_package_obj[key] for key in kwargs.get('columns')])
 
-    return tuple([vnf_package[key] for key in sorted(vnf_package.keys())])
+    return tuple([vnf_package_obj[key] for key in sorted(
+        vnf_package_obj.keys())])
 
 
 def create_vnf_packages(count=2, onboarded_vnf_package=False):

tackerclient/tests/unit/osc/v1/vnflcm_fakes.py

@@ -16,6 +16,8 @@
 from oslo_utils.fixture import uuidsentinel
 from oslo_utils import uuidutils
 
+from tackerclient.osc import utils as tacker_osc_utils
+
 
 def vnf_instance_response(attrs=None, instantiation_state='NOT_INSTANTIATED'):
     """Create a fake vnf instance.
@@ -117,6 +119,12 @@ def get_vnflcm_data(vnf_instance, list_action="/?originalUrl=https%3A%2F%2Fgit.openstack.org%2FFalse%2C%2520columns%3DNone)%3A%253C%2Fcode">
     :return:
         A tuple object sorted based on the name of the columns.
     """
+    complex_attributes = ['vimConnectionInfo', 'instantiatedVnfInfo', '_links']
+    for attribute in complex_attributes:
+        if vnf_instance.get(attribute):
+            vnf_instance.update(
+                {attribute: tacker_osc_utils.FormatComplexDataColumn(
+                    vnf_instance[attribute])})
 
     if list_action:
         for item in ['vnfInstanceDescription', 'vnfdVersion']:
@@ -126,7 +134,8 @@ def get_vnflcm_data(vnf_instance, list_action="/?originalUrl=https%3A%2F%2Fgit.openstack.org%2FFalse%2C%2520columns%3DNone)%3A%253C%2Fcode">
     if columns:
         return tuple([vnf_instance[key] for key in columns])
 
-    return tuple([vnf_instance[key] for key in sorted(vnf_instance.keys())])
+    return tuple([vnf_instance[key] for key in sorted(
+        vnf_instance.keys())])
 
 
 def create_vnf_instances(count=2):

tox.ini

@@ -13,7 +13,7 @@ setenv = VIRTUAL_ENV={envdir}
 usedevelop = True
 install_command = pip install {opts} {packages}
 deps =
-  -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}
+  -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/ussuri}
   -r{toxinidir}/requirements.txt
   -r{toxinidir}/test-requirements.txt
 commands = stestr run --slowest {posargs}
@@ -31,7 +31,7 @@ commands = sphinx-build -W -b html doc/source doc/build/html
 
 [testenv:releasenotes]
 deps =
-  -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}
+  -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/ussuri}
   -r{toxinidir}/doc/requirements.txt
 commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html