Merge "Allow known options after unknown ones in list and update command"

9 files changed, 136 insertions, 76 deletions

diff --git a/quantumclient/quantum/v2_0/__init__.py b/quantumclient/quantum/v2_0/__init__.py
index dde2a69..cba5ca1 100644
--- a/quantumclient/quantum/v2_0/__init__.py
+++ b/quantumclient/quantum/v2_0/__init__.py
@@ -98,6 +98,18 @@ def add_extra_argument(parser, name, _help):
                      '[--key2 [type=int|bool|...] value ...]')
 
 
+def is_number(s):
+    try:
+        float(s)  # for int, long and float
+    except ValueError:
+        try:
+            complex(s)  # for complex
+        except ValueError:
+            return False
+
+    return True
+
+
 def parse_args_to_dict(values_specs):
     '''It is used to analyze the extra command options to command.
 
@@ -156,7 +168,8 @@ def parse_args_to_dict(values_specs):
             _list_flag = True
             continue
         if not _item.startswith('--'):
-            if not current_item or '=' in current_item:
+            if (not current_item or '=' in current_item or
+                _item.startswith('-') and not is_number(_item)):
                 raise exceptions.CommandError(
                     "Invalid values_specs %s" % ' '.join(values_specs))
             _value_number += 1
@@ -318,8 +331,6 @@ class UpdateCommand(QuantumCommand):
         parser.add_argument(
             'id', metavar=self.resource,
             help='ID or name of %s to update' % self.resource)
-        add_extra_argument(parser, 'value_specs',
-                           'new values for the %s' % self.resource)
         self.add_known_arguments(parser)
         return parser
 
@@ -327,11 +338,14 @@ class UpdateCommand(QuantumCommand):
         self.log.debug('run(%s)' % parsed_args)
         quantum_client = self.get_client()
         quantum_client.format = parsed_args.request_format
-        value_specs = parsed_args.value_specs
-        dict_args = self.args2body(parsed_args).get(self.resource, {})
-        dict_specs = parse_args_to_dict(value_specs)
-        body = {self.resource: dict(dict_args.items() +
-                                    dict_specs.items())}
+        _extra_values = parse_args_to_dict(self.values_specs)
+        _merge_args(self, parsed_args, _extra_values,
+                    self.values_specs)
+        body = self.args2body(parsed_args)
+        if self.resource in body:
+            body[self.resource].update(_extra_values)
+        else:
+            body[self.resource] = _extra_values
         if not body[self.resource]:
             raise exceptions.CommandError(
                 "Must specify new values to update %s" % self.resource)
@@ -400,30 +414,29 @@ class ListCommand(QuantumCommand, lister.Lister):
     def get_parser(self, prog_name):
         parser = super(ListCommand, self).get_parser(prog_name)
         add_show_list_common_argument(parser)
-        add_extra_argument(parser, 'filter_specs', 'filters options')
         return parser
 
-    def retrieve_list(self, parsed_args):
-        """Retrieve a list of resources from Quantum server"""
-        quantum_client = self.get_client()
-        search_opts = parse_args_to_dict(parsed_args.filter_specs)
-        self.log.debug('search options: %s', search_opts)
-        quantum_client.format = parsed_args.request_format
+    def args2search_opts(self, parsed_args):
+        search_opts = {}
         fields = parsed_args.fields
-        extra_fields = search_opts.get('fields', [])
-        if extra_fields:
-            if isinstance(extra_fields, list):
-                fields.extend(extra_fields)
-            else:
-                fields.append(extra_fields)
-        if fields:
+        if parsed_args.fields:
             search_opts.update({'fields': fields})
         if parsed_args.show_details:
             search_opts.update({'verbose': 'True'})
+        return search_opts
+
+    def retrieve_list(self, parsed_args):
+        """Retrieve a list of resources from Quantum server"""
+        quantum_client = self.get_client()
+        quantum_client.format = parsed_args.request_format
+        _extra_values = parse_args_to_dict(self.values_specs)
+        _merge_args(self, parsed_args, _extra_values,
+                    self.values_specs)
+        search_opts = self.args2search_opts(parsed_args)
+        search_opts.update(_extra_values)
         obj_lister = getattr(quantum_client,
                              "list_%ss" % self.resource)
         data = obj_lister(**search_opts)
-
         collection = self.resource + "s"
         return data.get(collection, [])
 
diff --git a/quantumclient/quantum/v2_0/network.py b/quantumclient/quantum/v2_0/network.py
index b714ae5..dfc0171 100644
--- a/quantumclient/quantum/v2_0/network.py
+++ b/quantumclient/quantum/v2_0/network.py
@@ -60,9 +60,9 @@ class ListExternalNetwork(ListNetwork):
     log = logging.getLogger(__name__ + '.ListExternalNetwork')
 
     def retrieve_list(self, parsed_args):
-        if '--' not in parsed_args.filter_specs:
-            parsed_args.filter_specs.append('--')
-        parsed_args.filter_specs.append('--router:external=True')
+        external = '--router:external=True'
+        if external not in self.values_specs:
+            self.values_specs.append('--router:external=True')
         return super(ListExternalNetwork, self).retrieve_list(parsed_args)
 
 
diff --git a/quantumclient/quantum/v2_0/port.py b/quantumclient/quantum/v2_0/port.py
index 76468b7..753df0e 100644
--- a/quantumclient/quantum/v2_0/port.py
+++ b/quantumclient/quantum/v2_0/port.py
@@ -52,13 +52,10 @@ class ListRouterPort(ListCommand):
     list_columns = ['id', 'name', 'mac_address', 'fixed_ips']
 
     def get_parser(self, prog_name):
-        parser = super(ListCommand, self).get_parser(prog_name)
-        quantumv20.add_show_list_common_argument(parser)
+        parser = super(ListRouterPort, self).get_parser(prog_name)
         parser.add_argument(
             'id', metavar='router',
             help='ID or name of router to look up')
-        quantumv20.add_extra_argument(parser, 'filter_specs',
-                                      'filters options')
         return parser
 
     def get_data(self, parsed_args):
@@ -66,7 +63,7 @@ class ListRouterPort(ListCommand):
         quantum_client.format = parsed_args.request_format
         _id = quantumv20.find_resourceid_by_name_or_id(
             quantum_client, 'router', parsed_args.id)
-        parsed_args.filter_specs.append('--device_id=%s' % _id)
+        self.values_specs.append('--device_id=%s' % _id)
         return super(ListRouterPort, self).get_data(parsed_args)
 
 
diff --git a/quantumclient/shell.py b/quantumclient/shell.py
index 534cf63..4b22e98 100644
--- a/quantumclient/shell.py
+++ b/quantumclient/shell.py
@@ -37,6 +37,19 @@ VERSION = '2.0'
 QUANTUM_API_VERSION = '2.0'
 
 
+def run_command(cmd, cmd_parser, sub_argv):
+    _argv = sub_argv
+    index = -1
+    values_specs = []
+    if '--' in sub_argv:
+        index = sub_argv.index('--')
+        _argv = sub_argv[:index]
+        values_specs = sub_argv[index:]
+    known_args, _values_specs = cmd_parser.parse_known_args(_argv)
+    cmd.values_specs = (index == -1 and _values_specs or values_specs)
+    return cmd.run(known_args)
+
+
 def env(*_vars, **kwargs):
     """Search for the first defined of possibly many env vars
 
@@ -447,9 +460,7 @@ class QuantumShell(App):
                          else ' '.join([self.NAME, cmd_name])
                          )
             cmd_parser = cmd.get_parser(full_name)
-            known_args, values_specs = cmd_parser.parse_known_args(sub_argv)
-            cmd.values_specs = values_specs
-            result = cmd.run(known_args)
+            return run_command(cmd, cmd_parser, sub_argv)
         except Exception as err:
             if self.options.debug:
                 self.log.exception(err)
diff --git a/quantumclient/tests/unit/test_casual_args.py b/quantumclient/tests/unit/test_casual_args.py
index 150edec..fcfff93 100644
--- a/quantumclient/tests/unit/test_casual_args.py
+++ b/quantumclient/tests/unit/test_casual_args.py
@@ -53,6 +53,17 @@ class CLITestArgs(testtools.TestCase):
         self.assertRaises(exceptions.CommandError,
                           quantumV20.parse_args_to_dict, _specs)
 
+    def test_badarg_with_minus(self):
+        _specs = ['--arg1', 'value1', '-D']
+        self.assertRaises(exceptions.CommandError,
+                          quantumV20.parse_args_to_dict, _specs)
+
+    def test_goodarg_with_minus_number(self):
+        _specs = ['--arg1', 'value1', '-1', '-1.0']
+        _mydict = quantumV20.parse_args_to_dict(_specs)
+        self.assertEqual(['value1', '-1', '-1.0'],
+                         _mydict['arg1'])
+
     def test_badarg_duplicate(self):
         _specs = ['--tag=t', '--arg1', 'value1', '--arg1', 'value1']
         self.assertRaises(exceptions.CommandError,
diff --git a/quantumclient/tests/unit/test_cli20.py b/quantumclient/tests/unit/test_cli20.py
index 1c1dfca..d2abaf9 100644
--- a/quantumclient/tests/unit/test_cli20.py
+++ b/quantumclient/tests/unit/test_cli20.py
@@ -15,15 +15,13 @@
 #
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
-import sys
-
 import fixtures
 import mox
 from mox import Comparator
 from mox import ContainsKeyValue
 import testtools
 
-from quantumclient.quantum import v2_0 as quantumv20
+from quantumclient import shell
 from quantumclient.v2_0.client import Client
 
 
@@ -63,6 +61,31 @@ def end_url(path, query=None):
     return query and _url_str + "?" + query or _url_str
 
 
+class MyUrlComparator(Comparator):
+    def __init__(self, lhs, client):
+        self.lhs = lhs
+        self.client = client
+
+    def equals(self, rhs):
+        return str(self) == rhs
+
+    def __str__(self):
+        if self.client and self.client.format != FORMAT:
+            lhs_parts = self.lhs.split("?", 1)
+            if len(lhs_parts) == 2:
+                lhs = ("%s%s?%s" % (lhs_parts[0][:-4],
+                                    self.client.format,
+                                    lhs_parts[1]))
+            else:
+                lhs = ("%s%s" % (lhs_parts[0][:-4],
+                                 self.client.format))
+            return lhs
+        return self.lhs
+
+    def __repr__(self):
+        return str(self)
+
+
 class MyComparator(Comparator):
     def __init__(self, lhs, client):
         self.lhs = lhs
@@ -172,9 +195,7 @@ class CLITestV20Base(testtools.TestCase):
                                                         resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser('create_' + resource)
-        known_args, values_specs = cmd_parser.parse_known_args(args)
-        cmd.values_specs = values_specs
-        cmd.run(known_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -197,9 +218,7 @@ class CLITestV20Base(testtools.TestCase):
                                      TOKEN)).AndReturn((MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources_collection)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
 
@@ -244,15 +263,13 @@ class CLITestV20Base(testtools.TestCase):
                 query = "fields=" + field
         path = getattr(self.client, resources + "_path")
         self.client.httpclient.request(
-            end_url(path, query), 'GET',
+            MyUrlComparator(end_url(path, query), self.client), 'GET',
             body=None,
             headers=ContainsKeyValue('X-Auth-Token',
                                      TOKEN)).AndReturn((MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -265,15 +282,13 @@ class CLITestV20Base(testtools.TestCase):
         body = {resource: extrafields}
         path = getattr(self.client, resource + "_path")
         self.client.httpclient.request(
-            end_url(path % myid), 'PUT',
+            MyUrlComparator(end_url(path % myid), self.client), 'PUT',
             body=MyComparator(body, self.client),
             headers=ContainsKeyValue('X-Auth-Token',
                                      TOKEN)).AndReturn((MyResp(204), None))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("update_" + resource)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -296,9 +311,7 @@ class CLITestV20Base(testtools.TestCase):
                                      TOKEN)).AndReturn((MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("show_" + resource)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -317,9 +330,7 @@ class CLITestV20Base(testtools.TestCase):
                                      TOKEN)).AndReturn((MyResp(204), None))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("delete_" + resource)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -338,10 +349,8 @@ class CLITestV20Base(testtools.TestCase):
             headers=ContainsKeyValue('X-Auth-Token',
                                      TOKEN)).AndReturn((MyResp(204), None))
         self.mox.ReplayAll()
-        cmd_parser = cmd.get_parser("update_" + resource)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        cmd_parser = cmd.get_parser("delete_" + resource)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
diff --git a/quantumclient/tests/unit/test_cli20_network.py b/quantumclient/tests/unit/test_cli20_network.py
index 8eebcfe..6bc9ebd 100644
--- a/quantumclient/tests/unit/test_cli20_network.py
+++ b/quantumclient/tests/unit/test_cli20_network.py
@@ -28,6 +28,7 @@ from quantumclient.quantum.v2_0.network import ListExternalNetwork
 from quantumclient.quantum.v2_0.network import ListNetwork
 from quantumclient.quantum.v2_0.network import ShowNetwork
 from quantumclient.quantum.v2_0.network import UpdateNetwork
+from quantumclient import shell
 from quantumclient.tests.unit import test_cli20
 from quantumclient.tests.unit.test_cli20 import CLITestV20Base
 from quantumclient.tests.unit.test_cli20 import MyApp
@@ -120,9 +121,7 @@ class CLITestV20Network(CLITestV20Base):
                     (test_cli20.MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -276,10 +275,7 @@ class CLITestV20Network(CLITestV20Base):
                     (test_cli20.MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources)
-
-        parsed_args = cmd_parser.parse_args(args)
-
-        cmd.run(parsed_args)
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
@@ -343,10 +339,7 @@ class CLITestV20Network(CLITestV20Base):
                                     (test_cli20.MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
-
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
diff --git a/quantumclient/tests/unit/test_cli20_port.py b/quantumclient/tests/unit/test_cli20_port.py
index c4d9dad..faec572 100644
--- a/quantumclient/tests/unit/test_cli20_port.py
+++ b/quantumclient/tests/unit/test_cli20_port.py
@@ -19,6 +19,7 @@ import sys
 
 from mox import ContainsKeyValue
 
+from quantumclient import shell
 from quantumclient.quantum.v2_0.port import CreatePort
 from quantumclient.quantum.v2_0.port import DeletePort
 from quantumclient.quantum.v2_0.port import ListPort
@@ -213,10 +214,7 @@ class CLITestV20Port(CLITestV20Base):
                                     (test_cli20.MyResp(200), resstr))
         self.mox.ReplayAll()
         cmd_parser = cmd.get_parser("list_" + resources)
-
-        parsed_args = cmd_parser.parse_args(args)
-        cmd.run(parsed_args)
-
+        shell.run_command(cmd, cmd_parser, args)
         self.mox.VerifyAll()
         self.mox.UnsetStubs()
         _str = self.fake_stdout.make_string()
diff --git a/quantumclient/tests/unit/test_cli20_subnet.py b/quantumclient/tests/unit/test_cli20_subnet.py
index 997718b..9b9650e 100644
--- a/quantumclient/tests/unit/test_cli20_subnet.py
+++ b/quantumclient/tests/unit/test_cli20_subnet.py
@@ -312,6 +312,12 @@ class CLITestV20Subnet(CLITestV20Base):
         cmd = ListSubnet(MyApp(sys.stdout), None)
         self._test_list_resources(resources, cmd, tags=['a', 'b'])
 
+    def test_list_subnets_known_option_after_unknown(self):
+        """List subnets: -- --tags a b --request-format xml."""
+        resources = "subnets"
+        cmd = ListSubnet(MyApp(sys.stdout), None)
+        self._test_list_resources(resources, cmd, tags=['a', 'b'])
+
     def test_list_subnets_detail_tags(self):
         """List subnets: -D -- --tags a b."""
         resources = "subnets"
@@ -335,6 +341,28 @@ class CLITestV20Subnet(CLITestV20Base):
                                    {'name': 'myname', 'tags': ['a', 'b'], }
                                    )
 
+    def test_update_subnet_known_option_before_id(self):
+        """Update subnet: --request-format json myid --name myname."""
+        # --request-format xml is known option
+        resource = 'subnet'
+        cmd = UpdateSubnet(MyApp(sys.stdout), None)
+        self._test_update_resource(resource, cmd, 'myid',
+                                   ['--request-format', 'json',
+                                    'myid', '--name', 'myname'],
+                                   {'name': 'myname', }
+                                   )
+
+    def test_update_subnet_known_option_after_id(self):
+        """Update subnet: myid --name myname --request-format json."""
+        # --request-format xml is known option
+        resource = 'subnet'
+        cmd = UpdateSubnet(MyApp(sys.stdout), None)
+        self._test_update_resource(resource, cmd, 'myid',
+                                   ['myid', '--name', 'myname',
+                                    '--request-format', 'json'],
+                                   {'name': 'myname', }
+                                   )
+
     def test_show_subnet(self):
         """Show subnet: --fields id --fields name myid."""
         resource = 'subnet'