Fix formatting of the flavor properties

Do not stringify flavor properties to allow proper output formatting to
json/yaml/etc

Change-Id: I9f4c42acb85b726af87123134dd19de98fe95074

3 files changed, 86 insertions, 42 deletions

diff --git a/openstackclient/compute/v2/flavor.py b/openstackclient/compute/v2/flavor.py
index 805e919e..00431b7b 100644
--- a/openstackclient/compute/v2/flavor.py
+++ b/openstackclient/compute/v2/flavor.py
@@ -18,6 +18,7 @@
 import logging
 
 from novaclient import api_versions
+from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -30,6 +31,31 @@ from openstackclient.identity import common as identity_common
 LOG = logging.getLogger(__name__)
 
 
+_formatters = {
+    'extra_specs': format_columns.DictColumn,
+    # Unless we finish switch to use SDK resources this need to be doubled this
+    # way
+    'properties': format_columns.DictColumn,
+    'Properties': format_columns.DictColumn
+}
+
+
+def _get_flavor_columns(item):
+    # To maintain backwards compatibility we need to rename sdk props to
+    # whatever OSC was using before
+    column_map = {
+        'extra_specs': 'properties',
+        'ephemeral': 'OS-FLV-EXT-DATA:ephemeral',
+        'is_disabled': 'OS-FLV-DISABLED:disabled',
+        'is_public': 'os-flavor-access:is_public'
+
+    }
+    hidden_columns = ['links', 'location']
+
+    return utils.get_osc_show_columns_for_sdk_resource(
+        item, column_map, hidden_columns)
+
+
 def _find_flavor(compute_client, flavor):
     try:
         return compute_client.flavors.get(flavor)
@@ -191,10 +217,16 @@ class CreateFlavor(command.ShowOne):
                 LOG.error(_("Failed to set flavor property: %s"), e)
 
         flavor_info = flavor._info.copy()
-        flavor_info.pop("links")
-        flavor_info['properties'] = utils.format_dict(flavor.get_keys())
+        flavor_info['properties'] = flavor.get_keys()
 
-        return zip(*sorted(flavor_info.items()))
+        display_columns, columns = _get_flavor_columns(flavor_info)
+        data = utils.get_dict_properties(
+            flavor_info, columns,
+            formatters=_formatters,
+            mixed_case_fields=['OS-FLV-DISABLED:disabled',
+                               'OS-FLV-EXT-DATA:ephemeral'])
+
+        return (display_columns, data)
 
 
 class DeleteFlavor(command.Command):
@@ -309,7 +341,7 @@ class ListFlavor(command.Lister):
 
         return (column_headers,
                 (utils.get_item_properties(
-                    s, columns, formatters={'Properties': utils.format_dict},
+                    s, columns, formatters=_formatters,
                 ) for s in data))
 
 
@@ -428,11 +460,8 @@ class ShowFlavor(command.ShowOne):
             try:
                 flavor_access = compute_client.flavor_access.list(
                     flavor=resource_flavor.id)
-                projects = [utils.get_field(access, 'tenant_id')
-                            for access in flavor_access]
-                # TODO(Huanxuan Ao): This format case can be removed after
-                # patch https://review.opendev.org/#/c/330223/ merged.
-                access_projects = utils.format_list(projects)
+                access_projects = [utils.get_field(access, 'tenant_id')
+                                   for access in flavor_access]
             except Exception as e:
                 msg = _("Failed to get access projects list "
                         "for flavor '%(flavor)s': %(e)s")
@@ -442,11 +471,17 @@ class ShowFlavor(command.ShowOne):
         flavor.update({
             'access_project_ids': access_projects
         })
-        flavor.pop("links", None)
 
-        flavor['properties'] = utils.format_dict(resource_flavor.get_keys())
+        flavor['properties'] = resource_flavor.get_keys()
+
+        display_columns, columns = _get_flavor_columns(flavor)
+        data = utils.get_dict_properties(
+            flavor, columns,
+            formatters=_formatters,
+            mixed_case_fields=['OS-FLV-DISABLED:disabled',
+                               'OS-FLV-EXT-DATA:ephemeral'])
 
-        return zip(*sorted(flavor.items()))
+        return (display_columns, data)
 
 
 class UnsetFlavor(command.Command):
diff --git a/openstackclient/tests/functional/compute/v2/test_flavor.py b/openstackclient/tests/functional/compute/v2/test_flavor.py
index c274adf2..162d4287 100644
--- a/openstackclient/tests/functional/compute/v2/test_flavor.py
+++ b/openstackclient/tests/functional/compute/v2/test_flavor.py
@@ -115,8 +115,8 @@ class FlavorTests(base.TestCase):
         self.assertFalse(
             cmd_output["os-flavor-access:is_public"],
         )
-        self.assertEqual(
-            "a='b2', b='d2'",
+        self.assertDictEqual(
+            {"a": "b2", "b": "d2"},
             cmd_output["properties"],
         )
 
@@ -133,12 +133,18 @@ class FlavorTests(base.TestCase):
             "flavor list -f json " +
             "--long"
         ))
-        col_name = [x["Name"] for x in cmd_output]
-        col_properties = [x['Properties'] for x in cmd_output]
-        self.assertIn(name1, col_name)
-        self.assertIn("a='b', c='d'", col_properties)
-        self.assertNotIn(name2, col_name)
-        self.assertNotIn("b2', b='d2'", col_properties)
+        # We have list of complex json objects
+        # Iterate through the list setting flags
+        found_expected = False
+        for rec in cmd_output:
+            if rec['Name'] == name1:
+                found_expected = True
+                self.assertEqual('b', rec['Properties']['a'])
+                self.assertEqual('d', rec['Properties']['c'])
+            elif rec['Name'] == name2:
+                # We should have not seen private flavor
+                self.assertFalse(True)
+        self.assertTrue(found_expected)
 
         # Test list --public
         cmd_output = json.loads(self.openstack(
@@ -201,8 +207,8 @@ class FlavorTests(base.TestCase):
         self.assertFalse(
             cmd_output["os-flavor-access:is_public"],
         )
-        self.assertEqual(
-            "a='first', b='second'",
+        self.assertDictEqual(
+            {"a": "first", "b": "second"},
             cmd_output["properties"],
         )
 
@@ -223,9 +229,14 @@ class FlavorTests(base.TestCase):
             cmd_output["id"],
         )
         self.assertEqual(
-            "a='third and 10', b='second', g='fourth'",
-            cmd_output['properties'],
-        )
+            'third and 10',
+            cmd_output['properties']['a'])
+        self.assertEqual(
+            'second',
+            cmd_output['properties']['b'])
+        self.assertEqual(
+            'fourth',
+            cmd_output['properties']['g'])
 
         raw_output = self.openstack(
             "flavor unset " +
@@ -238,7 +249,5 @@ class FlavorTests(base.TestCase):
             "flavor show -f json " +
             name1
         ))
-        self.assertEqual(
-            "a='third and 10', g='fourth'",
-            cmd_output["properties"],
-        )
+
+        self.assertNotIn('b', cmd_output['properties'])
diff --git a/openstackclient/tests/unit/compute/v2/test_flavor.py b/openstackclient/tests/unit/compute/v2/test_flavor.py
index 4732cc82..2828d74e 100644
--- a/openstackclient/tests/unit/compute/v2/test_flavor.py
+++ b/openstackclient/tests/unit/compute/v2/test_flavor.py
@@ -17,8 +17,8 @@ from unittest import mock
 from unittest.mock import call
 
 import novaclient
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
-from osc_lib import utils
 
 from openstackclient.compute.v2 import flavor
 from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes
@@ -70,7 +70,7 @@ class TestFlavorCreate(TestFlavor):
         flavor.id,
         flavor.name,
         flavor.is_public,
-        utils.format_dict(flavor.properties),
+        format_columns.DictColumn(flavor.properties),
         flavor.ram,
         flavor.rxtx_factor,
         flavor.swap,
@@ -111,7 +111,7 @@ class TestFlavorCreate(TestFlavor):
         self.flavors_mock.create.assert_called_once_with(*default_args)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_flavor_create_all_options(self):
 
@@ -165,7 +165,7 @@ class TestFlavorCreate(TestFlavor):
         self.flavor.get_keys.assert_called_once_with()
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_flavor_create_other_options(self):
 
@@ -226,7 +226,7 @@ class TestFlavorCreate(TestFlavor):
             {'key1': 'value1', 'key2': 'value2'})
         self.flavor.get_keys.assert_called_with()
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_public_flavor_create_with_project(self):
         arglist = [
@@ -300,7 +300,7 @@ class TestFlavorCreate(TestFlavor):
         self.flavors_mock.create.assert_called_once_with(*args)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_flavor_create_with_description_api_older(self):
         arglist = [
@@ -429,7 +429,7 @@ class TestFlavorList(TestFlavor):
     data_long = (data[0] + (
         flavors[0].swap,
         flavors[0].rxtx_factor,
-        u'property=\'value\''
+        format_columns.DictColumn(flavors[0].properties)
     ), )
 
     def setUp(self):
@@ -583,7 +583,7 @@ class TestFlavorList(TestFlavor):
         )
 
         self.assertEqual(self.columns_long, columns)
-        self.assertEqual(tuple(self.data_long), tuple(data))
+        self.assertListItemEqual(self.data_long, tuple(data))
 
 
 class TestFlavorSet(TestFlavor):
@@ -817,7 +817,7 @@ class TestFlavorShow(TestFlavor):
         flavor.id,
         flavor.name,
         flavor.is_public,
-        utils.format_dict(flavor.get_keys()),
+        format_columns.DictColumn(flavor.get_keys()),
         flavor.ram,
         flavor.rxtx_factor,
         flavor.swap,
@@ -854,7 +854,7 @@ class TestFlavorShow(TestFlavor):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_private_flavor_show(self):
         private_flavor = compute_fakes.FakeFlavor.create_one_flavor(
@@ -874,13 +874,13 @@ class TestFlavorShow(TestFlavor):
         data_with_project = (
             private_flavor.disabled,
             private_flavor.ephemeral,
-            self.flavor_access.tenant_id,
+            [self.flavor_access.tenant_id],
             private_flavor.description,
             private_flavor.disk,
             private_flavor.id,
             private_flavor.name,
             private_flavor.is_public,
-            utils.format_dict(private_flavor.get_keys()),
+            format_columns.DictColumn(private_flavor.get_keys()),
             private_flavor.ram,
             private_flavor.rxtx_factor,
             private_flavor.swap,
@@ -894,7 +894,7 @@ class TestFlavorShow(TestFlavor):
         self.flavor_access_mock.list.assert_called_with(
             flavor=private_flavor.id)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(data_with_project, data)
+        self.assertItemEqual(data_with_project, data)
 
 
 class TestFlavorUnset(TestFlavor):