Refactor Extension show and list command

1.keep the column display order consist in extension list with
  and without "--long" option.
2.rework for network extentsion list, openstacksdk return object,
  so the logic should be same with other service.
3.add some unit test cases, like: extension list --network --long,
  extension list --network --compute, to cover regular use cases.
4.raise exact exception when network extension don't exist, avoid
  internal TypeError in "extension show" commands.

Change-Id: I2e23ced80d8da8aa1106b22472db850367b351ce
Closes-Bug: #1689233

Author: ankur-odl

openstackclient/common/extension.py

@@ -15,7 +15,6 @@
 
 """Extension action implementations"""
 
-import itertools
 import logging
 
 from osc_lib.command import command
@@ -66,8 +65,8 @@ def get_parser(self, prog_name):
 
     def take_action(self, parsed_args):
         if parsed_args.long:
-            columns = ('Name', 'Namespace', 'Description',
-                       'Alias', 'Updated', 'Links')
+            columns = ('Name', 'Alias', 'Description',
+                       'Namespace', 'Updated', 'Links')
         else:
             columns = ('Name', 'Alias', 'Description')
 
@@ -104,35 +103,22 @@ def take_action(self, parsed_args):
                             "Block Storage API")
                 LOG.warning(message)
 
-        # Resource classes for the above
+        if parsed_args.network or show_all:
+            network_client = self.app.client_manager.network
+            try:
+                data += network_client.extensions()
+            except Exception:
+                message = _("Failed to retrieve extensions list "
+                            "from Network API")
+                LOG.warning(message)
+
         extension_tuples = (
             utils.get_item_properties(
                 s,
                 columns,
-                formatters={},
             ) for s in data
         )
 
-        # Dictionaries for the below
-        if parsed_args.network or show_all:
-            network_client = self.app.client_manager.network
-            try:
-                data = network_client.extensions()
-                dict_tuples = (
-                    utils.get_item_properties(
-                        s,
-                        columns,
-                        formatters={},
-                    ) for s in data
-                )
-                extension_tuples = itertools.chain(
-                    extension_tuples,
-                    dict_tuples
-                )
-            except Exception:
-                message = _("Extensions list not supported by Network API")
-                LOG.warning(message)
-
         return (columns, extension_tuples)
 
 
@@ -152,14 +138,7 @@ def get_parser(self, prog_name):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
-        columns = ('Alias', 'Description', 'Links', 'Name',
-                   'Namespace', 'Updated')
         ext = str(parsed_args.extension)
-        obj = client.find_extension(ext)
-        dict_tuples = (utils.get_item_properties(
-                       obj,
-                       columns,
-                       formatters={},)
-                       )
-
-        return columns, dict_tuples
+        obj = client.find_extension(ext, ignore_missing=False).to_dict()
+
+        return zip(*sorted(obj.items()))

openstackclient/tests/functional/common/test_extension.py

@@ -15,6 +15,8 @@
 
 import json
 
+from tempest.lib import exceptions as tempest_exc
+
 from openstackclient.tests.functional import base
 
 
@@ -23,7 +25,6 @@ class ExtensionTests(base.TestCase):
 
     @classmethod
     def setUpClass(cls):
-        # super(NetworkTests, cls).setUp()
         cls.haz_network = base.is_service_enabled('network')
 
     def test_extension_list_compute(self):
@@ -38,6 +39,18 @@ def test_extension_list_compute(self):
             name_list,
         )
 
+    def test_extension_list_volume(self):
+        """Test volume extension list"""
+        json_output = json.loads(self.openstack(
+            'extension list -f json ' +
+            '--volume'
+        ))
+        name_list = [item.get('Name') for item in json_output]
+        self.assertIn(
+            'TypesManage',
+            name_list,
+        )
+
     def test_extension_list_network(self):
         """Test network extension list"""
         if not self.haz_network:
@@ -79,5 +92,19 @@ def test_extension_show_network(self):
         ))
         self.assertEqual(
             name,
-            json_output.get('Alias'),
+            json_output.get('alias'),
         )
+
+    def test_extension_show_not_exist(self):
+        """Test extension show with not existed name"""
+        if not self.haz_network:
+            self.skipTest("No Network service present")
+
+        name = 'not_existed_ext'
+        try:
+            self.openstack('extension show ' + name)
+        except tempest_exc.CommandFailed as e:
+            self.assertIn('ResourceNotFound', str(e))
+            self.assertIn(name, str(e))
+        else:
+            self.fail('CommandFailed should be raised')

openstackclient/tests/unit/common/test_extension.py

@@ -67,7 +67,7 @@ def setUp(self):
 class TestExtensionList(TestExtension):
 
     columns = ('Name', 'Alias', 'Description')
-    long_columns = ('Name', 'Namespace', 'Description', 'Alias', 'Updated',
+    long_columns = ('Name', 'Alias', 'Description', 'Namespace', 'Updated',
                     'Links')
 
     volume_extension = volume_fakes.FakeExtension.create_one_extension()
@@ -145,33 +145,33 @@ def test_extension_list_long(self):
         datalist = (
             (
                 self.identity_extension.name,
-                self.identity_extension.namespace,
-                self.identity_extension.description,
                 self.identity_extension.alias,
+                self.identity_extension.description,
+                self.identity_extension.namespace,
                 self.identity_extension.updated,
                 self.identity_extension.links,
             ),
             (
                 self.compute_extension.name,
-                self.compute_extension.namespace,
-                self.compute_extension.description,
                 self.compute_extension.alias,
+                self.compute_extension.description,
+                self.compute_extension.namespace,
                 self.compute_extension.updated,
                 self.compute_extension.links,
             ),
             (
                 self.volume_extension.name,
-                self.volume_extension.namespace,
-                self.volume_extension.description,
                 self.volume_extension.alias,
+                self.volume_extension.description,
+                self.volume_extension.namespace,
                 self.volume_extension.updated,
                 self.volume_extension.links,
             ),
             (
                 self.network_extension.name,
-                self.network_extension.namespace,
-                self.network_extension.description,
                 self.network_extension.alias,
+                self.network_extension.description,
+                self.network_extension.namespace,
                 self.network_extension.updated,
                 self.network_extension.links,
             ),
@@ -214,6 +214,27 @@ def test_extension_list_network(self):
         self._test_extension_list_helper(arglist, verifylist, datalist)
         self.network_extensions_mock.assert_called_with()
 
+    def test_extension_list_network_with_long(self):
+        arglist = [
+            '--network',
+            '--long',
+        ]
+        verifylist = [
+            ('network', True),
+            ('long', True),
+        ]
+        datalist = ((
+            self.network_extension.name,
+            self.network_extension.alias,
+            self.network_extension.description,
+            self.network_extension.namespace,
+            self.network_extension.updated,
+            self.network_extension.links,
+        ), )
+        self._test_extension_list_helper(arglist, verifylist, datalist,
+                                         long=True)
+        self.network_extensions_mock.assert_called_with()
+
     def test_extension_list_compute(self):
         arglist = [
             '--compute',
@@ -229,6 +250,31 @@ def test_extension_list_compute(self):
         self._test_extension_list_helper(arglist, verifylist, datalist)
         self.compute_extensions_mock.show_all.assert_called_with()
 
+    def test_extension_list_compute_and_network(self):
+        arglist = [
+            '--compute',
+            '--network',
+        ]
+        verifylist = [
+            ('compute', True),
+            ('network', True),
+        ]
+        datalist = (
+            (
+                self.compute_extension.name,
+                self.compute_extension.alias,
+                self.compute_extension.description,
+            ),
+            (
+                self.network_extension.name,
+                self.network_extension.alias,
+                self.network_extension.description,
+            ),
+        )
+        self._test_extension_list_helper(arglist, verifylist, datalist)
+        self.compute_extensions_mock.show_all.assert_called_with()
+        self.network_extensions_mock.assert_called_with()
+
     def test_extension_list_volume(self):
         arglist = [
             '--volume',
@@ -251,12 +297,12 @@ class TestExtensionShow(TestExtension):
     )
 
     columns = (
-        'Alias',
-        'Description',
-        'Links',
-        'Name',
-        'Namespace',
-        'Updated'
+        'alias',
+        'description',
+        'links',
+        'name',
+        'namespace',
+        'updated',
     )
 
     data = (
@@ -296,7 +342,7 @@ def test_show_all_options(self):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.app.client_manager.network.find_extension.assert_called_with(
-            self.extension_details.alias)
+            self.extension_details.alias, ignore_missing=False)
 
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)

releasenotes/notes/bug-1689233-c3f98e159c75374e.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Raise exact exception when extension don't exist in ``extension show``
+    command, and keep the column display order consist in ``extension list``
+    with and without ``--long`` option.
+    [Bug `1689233 <https://bugs.launchpad.net/python-openstackclient/+bug/1689233>`_]