Hello,
This fixes https://fedorahosted.org/freeipa/ticket/3963 for master. I'll make a slightly less invasive patch for 3.x.


Basically, the version argument is now expected for all commands (including e.g. ping & env), and also when calling the commands in_server. If it's not given, a message is returned, so this should should only really affect tests that check the output strictly.

--
PetrĀ³
From 35830bbce76d24c6cd94b0b72251ff471884e8e2 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 6 Jun 2014 14:21:16 +0200
Subject: [PATCH] ipalib.frontend: Do API version check before converting
 arguments

This results in the proper message being shown if the client sends
an option the server doesn't have yet.

It also adds the check to commands that override run() but not __call__,
such as `ipa ping`, and to commands run on the server. Adjust tests
for these changes.

https://fedorahosted.org/freeipa/ticket/3963
---
 ipalib/__init__.py                            |  4 ++--
 ipalib/frontend.py                            | 34 +++++++++++++--------------
 ipatests/test_xmlrpc/test_automount_plugin.py |  8 ++++---
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 220a83ddc4559601a9f56b5fbc81119a231484ee..2818e457a8c81758abf61dc90ed869205f98149e 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -140,7 +140,7 @@ class my_command(Command):
 >>> api = create_api()
 >>> api.register(my_command)
 >>> api.finalize()
->>> api.Command.my_command() # Call your command
+>>> api.Command.my_command(version=u'2.47') # Call your command
 {'result': 'My run() method was called!'}
 
 When `frontend.Command.__call__()` is called, it first validates any arguments
@@ -359,7 +359,7 @@ class my_command(Command):
 
 And yet we can call ``my_command()``:
 
->>> api.Command.my_command()
+>>> api.Command.my_command(version=u'2.47')
 {'result': 'Just my_command.forward() getting called here.'}
 
 
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 66be7939f9e4b9b45ae48d59d78a57b8a92f8bdd..a9a3dfeaac2ab7047c036ea7bbed7655dd93be56 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -419,6 +419,11 @@ def __call__(self, *args, **options):
         XML-RPC and the executed an the nearest IPA server.
         """
         self.ensure_finalized()
+        version_provided = 'version' in options
+        if version_provided:
+            self.verify_client_version(unicode(options['version']))
+        else:
+            options['version'] = API_VERSION
         params = self.args_options_2_params(*args, **options)
         self.debug(
             'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
@@ -429,11 +434,13 @@ def __call__(self, *args, **options):
         self.debug(
             '%s(%s)', self.name, ', '.join(self._repr_iter(**params))
         )
-        if not self.api.env.in_server and 'version' not in params:
-            params['version'] = API_VERSION
         self.validate(**params)
         (args, options) = self.params_2_args_options(**params)
         ret = self.run(*args, **options)
+        if not version_provided and isinstance(ret, dict):
+            messages.add_message(
+                API_VERSION, ret,
+                messages.VersionMissing(server_version=API_VERSION))
         if (
             isinstance(ret, dict)
             and 'summary' in self.output
@@ -441,7 +448,7 @@ def __call__(self, *args, **options):
         ):
             ret['summary'] = self.get_summary_default(ret)
         if self.use_output_validation and (self.output or ret is not None):
-            self.validate_output(ret, options.get('version', API_VERSION))
+            self.validate_output(ret, options['version'])
         return ret
 
     def soft_validate(self, values):
@@ -744,17 +751,7 @@ def run(self, *args, **options):
         performs is executed remotely.
         """
         if self.api.env.in_server:
-            version_provided = 'version' in options
-            if version_provided:
-                self.verify_client_version(options['version'])
-            else:
-                options['version'] = API_VERSION
-            result = self.execute(*args, **options)
-            if not version_provided:
-                messages.add_message(
-                    API_VERSION, result,
-                    messages.VersionMissing(server_version=API_VERSION))
-            return result
+            return self.execute(*args, **options)
         return self.forward(*args, **options)
 
     def execute(self, *args, **kw):
@@ -1318,15 +1315,18 @@ class Method(Attribute, Command):
 
     >>> list(api.Method)
     ['user_add']
-    >>> api.Method.user_add() # Will call user_add.run()
+    >>> api.Method.user_add(version=u'2.88')  # Will call user_add.run()
     {'result': 'Added the user!'}
 
+    (The "version" argument is the API version to use.
+    The current API version can be found in ipalib.version.API_VERSION.)
+
     Second, because `Method` is a subclass of `Command`, the ``user_add``
     plugin can also be accessed through the ``api.Command`` namespace:
 
     >>> list(api.Command)
     ['user_add']
-    >>> api.Command.user_add() # Will call user_add.run()
+    >>> api.Command.user_add(version=u'2.88') # Will call user_add.run()
     {'result': 'Added the user!'}
 
     And third, ``user_add`` can be accessed as an attribute on the ``user``
@@ -1336,7 +1336,7 @@ class Method(Attribute, Command):
     ['user']
     >>> list(api.Object.user.methods)
     ['add']
-    >>> api.Object.user.methods.add() # Will call user_add.run()
+    >>> api.Object.user.methods.add(version=u'2.88') # Will call user_add.run()
     {'result': 'Added the user!'}
 
     The `Attribute` base class implements the naming convention for the
diff --git a/ipatests/test_xmlrpc/test_automount_plugin.py b/ipatests/test_xmlrpc/test_automount_plugin.py
index 0f272f7bb5b49b28886a28619de0a46e8f4e3645..6f1dcfd7dcc4ad034ffe2940b8f8a7e8bc7a8a51 100644
--- a/ipatests/test_xmlrpc/test_automount_plugin.py
+++ b/ipatests/test_xmlrpc/test_automount_plugin.py
@@ -52,7 +52,7 @@ def check_tofiles(self):
 
         mock_ui = MockTextui()
         command = api.Command['automountlocation_tofiles']
-        command.output_for_cli(mock_ui, res, self.locname)
+        command.output_for_cli(mock_ui, res, self.locname, version=u'2.88')
         expected_output = self.tofiles_output
         assert_deepequal(expected_output, u'\n'.join(mock_ui))
 
@@ -88,7 +88,8 @@ def check_import_roundtrip(self):
             # Feed the files to automountlocation_import & check
             master_file = u'%s/auto.master' % conf_directory
             automountlocation_import = api.Command['automountlocation_import']
-            res = automountlocation_import(self.locname, master_file)
+            res = automountlocation_import(self.locname, master_file,
+                                           version=u'2.88')
             assert_deepequal(dict(
                 result=dict(
                     keys=lambda k: k,
@@ -234,7 +235,8 @@ def test_a2_automountmap_tofiles(self):
         """
         Test the `automountlocation_tofiles` command.
         """
-        res = api.Command['automountlocation_tofiles'](self.locname)
+        res = api.Command['automountlocation_tofiles'](self.locname,
+                                                       version=u'2.88')
         assert_deepequal(dict(
             result=dict(
                 keys={'auto.direct': ()},
-- 
1.9.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to