Merge "Fix "single_alias" action for CLI commands"

manilaclient/shell.py

@@ -55,10 +55,21 @@ class AllowOnlyOneAliasAtATimeAction(argparse.Action):
     def __call__(self, parser, namespace, values, option_string=None):
         # NOTE(vponomaryov): this method is redefinition of
         # argparse.Action.__call__ interface
-        if getattr(namespace, self.dest) is not None:
+
+        if not hasattr(self, 'calls'):
+            self.calls = {}
+
+        if self.dest not in self.calls:
+            self.calls[self.dest] = set()
+
+        local_values = sorted(values) if isinstance(values, list) else values
+        self.calls[self.dest].add(six.text_type(local_values))
+
+        if len(self.calls[self.dest]) == 1:
+            setattr(namespace, self.dest, local_values)
+        else:
             msg = "Only one alias is allowed at a time."
             raise argparse.ArgumentError(self, msg)
-        setattr(namespace, self.dest, values)
 
 
 class ManilaClientArgumentParser(argparse.ArgumentParser):

manilaclient/tests/unit/test_shell.py

@@ -17,13 +17,16 @@ import ddt
 import fixtures
 import mock
 from six import moves
+from tempest.lib.cli import output_parser
 from testtools import matchers
 
 import manilaclient
 from manilaclient.common import constants
 from manilaclient import exceptions
+from manilaclient.openstack.common import cliutils
 from manilaclient import shell
 from manilaclient.tests.unit import utils
+from manilaclient.tests.unit.v2 import fakes
 
 
 @ddt.ddt
@@ -221,3 +224,146 @@ class OpenstackManilaShellTest(utils.TestCase):
 
         for expected_arg in expected_args:
             self.assertIn(expected_arg, help_text)
+
+
+class CustomOpenStackManilaShell(shell.OpenStackManilaShell):
+
+    @staticmethod
+    @cliutils.arg(
+        '--default-is-none',
+        '--default_is_none',
+        type=str,
+        metavar='<redefined_metavar>',
+        action='single_alias',
+        help='Default value is None and metavar set.',
+        default=None)
+    def do_foo(cs, args):
+        cliutils.print_dict({'key': args.default_is_none})
+
+    @staticmethod
+    @cliutils.arg(
+        '--default-is-not-none',
+        '--default_is_not_none',
+        type=str,
+        action='single_alias',
+        help='Default value is not None and metavar not set.',
+        default='bar')
+    def do_bar(cs, args):
+        cliutils.print_dict({'key': args.default_is_not_none})
+
+    @staticmethod
+    @cliutils.arg(
+        '--list-like',
+        '--list_like',
+        nargs='*',
+        action='single_alias',
+        help='Default value is None, metavar not set and result is list.',
+        default=None)
+    def do_quuz(cs, args):
+        cliutils.print_dict({'key': args.list_like})
+
+
+@ddt.ddt
+class AllowOnlyOneAliasAtATimeActionTest(utils.TestCase):
+    FAKE_ENV = {
+        'OS_USERNAME': 'username',
+        'OS_PASSWORD': 'password',
+        'OS_TENANT_NAME': 'tenant_name',
+        'OS_AUTH_URL': 'http://no.where',
+    }
+
+    def setUp(self):
+        super(self.__class__, self).setUp()
+        for k, v in self.FAKE_ENV.items():
+            self.useFixture(fixtures.EnvironmentVariable(k, v))
+        self.mock_object(
+            shell.client, 'get_client_class',
+            mock.Mock(return_value=fakes.FakeClient))
+
+    def shell_discover_client(self,
+                              current_client,
+                              os_api_version,
+                              os_endpoint_type,
+                              os_service_type,
+                              client_args):
+        return current_client, manilaclient.API_MAX_VERSION
+
+    def shell(self, argstr):
+        orig = sys.stdout
+        try:
+            sys.stdout = moves.StringIO()
+            _shell = CustomOpenStackManilaShell()
+            _shell._discover_client = self.shell_discover_client
+            _shell.main(argstr.split())
+        except SystemExit:
+            exc_type, exc_value, exc_traceback = sys.exc_info()
+            self.assertEqual(exc_value.code, 0)
+        finally:
+            out = sys.stdout.getvalue()
+            sys.stdout.close()
+            sys.stdout = orig
+
+        return out
+
+    @ddt.data(
+        ('--default-is-none foo', 'foo'),
+        ('--default-is-none foo --default-is-none foo', 'foo'),
+        ('--default-is-none foo --default_is_none foo', 'foo'),
+        ('--default_is_none None', 'None'),
+    )
+    @ddt.unpack
+    def test_foo_success(self, options_str, expected_result):
+        output = self.shell('foo %s' % options_str)
+        parsed_output = output_parser.details(output)
+        self.assertEqual({'key': expected_result}, parsed_output)
+
+    @ddt.data(
+        '--default-is-none foo --default-is-none bar',
+        '--default-is-none foo --default_is_none bar',
+        '--default-is-none foo --default_is_none FOO',
+    )
+    def test_foo_error(self, options_str):
+        self.assertRaises(
+            matchers.MismatchError, self.shell, 'foo %s' % options_str)
+
+    @ddt.data(
+        ('--default-is-not-none bar', 'bar'),
+        ('--default_is_not_none bar --default-is-not-none bar', 'bar'),
+        ('--default_is_not_none bar --default_is_not_none bar', 'bar'),
+        ('--default-is-not-none not_bar', 'not_bar'),
+        ('--default_is_not_none None', 'None'),
+    )
+    @ddt.unpack
+    def test_bar_success(self, options_str, expected_result):
+        output = self.shell('bar %s' % options_str)
+        parsed_output = output_parser.details(output)
+        self.assertEqual({'key': expected_result}, parsed_output)
+
+    @ddt.data(
+        '--default-is-not-none foo --default-is-not-none bar',
+        '--default-is-not-none foo --default_is_not_none bar',
+        '--default-is-not-none bar --default_is_not_none BAR',
+    )
+    def test_bar_error(self, options_str):
+        self.assertRaises(
+            matchers.MismatchError, self.shell, 'bar %s' % options_str)
+
+    @ddt.data(
+        ('--list-like q=w', "['q=w']"),
+        ('--list-like q=w --list_like q=w', "['q=w']"),
+        ('--list-like q=w e=r t=y --list_like e=r t=y q=w',
+         "['e=r', 'q=w', 't=y']"),
+        ('--list_like q=w e=r t=y', "['e=r', 'q=w', 't=y']"),
+    )
+    @ddt.unpack
+    def test_quuz_success(self, options_str, expected_result):
+        output = self.shell('quuz %s' % options_str)
+        parsed_output = output_parser.details(output)
+        self.assertEqual({'key': expected_result}, parsed_output)
+
+    @ddt.data(
+        '--list-like q=w --list_like e=r t=y',
+    )
+    def test_quuz_error(self, options_str):
+        self.assertRaises(
+            matchers.MismatchError, self.shell, 'quuz %s' % options_str)

manilaclient/v2/shell.py

@@ -401,9 +401,11 @@ def do_quota_defaults(cs, args):
     help='New value for the "snapshot_gigabytes" quota.')
 @cliutils.arg(
     '--share-networks',
+    '--share_networks',
     metavar='<share-networks>',
     type=int,
     default=None,
+    action='single_alias',
     help='New value for the "share_networks" quota.')
 @cliutils.arg(
     '--force',
@@ -529,7 +531,9 @@ def do_rate_limits(cs, args):
     help='Share size in GiB.')
 @cliutils.arg(
     '--snapshot-id',
+    '--snapshot_id',
     metavar='<snapshot-id>',
+    action='single_alias',
     help='Optional snapshot ID to create the share from. (Default=None)',
     default=None)
 @cliutils.arg(
@@ -546,7 +550,9 @@ def do_rate_limits(cs, args):
     default=None)
 @cliutils.arg(
     '--share-network',
+    '--share_network',
     metavar='<network-info>',
+    action='single_alias',
     help='Optional network info ID or name.',
     default=None)
 @cliutils.arg(
@@ -624,6 +630,7 @@ def do_create(cs, args):
     metavar='<True|False>',
     choices=['True', 'False'],
     required=False,
+    action='single_alias',
     help='Enables or disables generic host-based force-migration, which '
          'bypasses driver optimizations. Default=False.',
     default=False)
@@ -705,6 +712,7 @@ def do_migration_cancel(cs, args):
     '--state',
     metavar='<task_state>',
     default='migration_error',
+    action='single_alias',
     help=('Indicate which task state to assign the share. Options include '
           'migration_starting, migration_in_progress, migration_completing, '
           'migration_success, migration_error, migration_cancelled, '
@@ -1082,6 +1090,7 @@ def do_show(cs, args):
     type=str,
     default=None,
     choices=['rw', 'ro'],
+    action='single_alias',
     help='Share access level ("rw" and "ro" access levels are supported). '
          'Defaults to rw.')
 def do_access_allow(cs, args):
@@ -1177,7 +1186,7 @@ def do_access_list(cs, args):
          'was used for share creation. OPTIONAL: Default=None',
     default=None)
 @cliutils.arg(
-    '--share-type', '--volume-type'
+    '--share-type', '--volume-type',
     '--share_type', '--share-type-id', '--volume-type-id',  # aliases
     '--share-type_id', '--share_type-id', '--share_type_id',  # aliases
     '--volume_type', '--volume_type_id',
@@ -3311,6 +3320,7 @@ def do_share_replica_list(cs, args):
     '--share_network',
     metavar='<network-info>',
     default=None,
+    action='single_alias',
     help='Optional network info ID or name.')
 @api_versions.wraps("2.11")
 @api_versions.experimental_api
@@ -3418,6 +3428,7 @@ def do_share_replica_reset_state(cs, args):
     '--state',  # alias for user sanity
     metavar='<replica_state>',
     default='out_of_sync',
+    action='single_alias',
     help=('Indicate which replica_state to assign the replica. Options '
           'include in_sync, out_of_sync, active, error. If no '
           'state is provided, out_of_sync will be used.'))