From c5afee964eee0cdf81a4f22fd78a6838a3da7537 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Oct 04 2017 08:09:18 +0000
Subject: cli: simplify parsing of arbitrary types


Add the 'constructor' type to IPAOption to allow parsing arbitrary
types.

When using this type, supply the 'constructor' attribute with the
constructor of the type.  The checker for the 'constructor' type
attempts to construct the data, returning if successful else raising
OptionValueError.

The 'knob' interface remains unchanged but now accepts arbitrary
constructors.

This feature subsumes the '_option_callback' mechanism, which has
been refactored away.

This feature also subsumes the "dn" type in IPAOption, but this
refactor is deferred.

Part of: https://pagure.io/freeipa/issue/6858

Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>

---

diff --git a/ipapython/config.py b/ipapython/config.py
index 6e53472..8393e0d 100644
--- a/ipapython/config.py
+++ b/ipapython/config.py
@@ -79,16 +79,28 @@ def check_dn_option(option, opt, value):
     except Exception as e:
         raise OptionValueError("option %s: invalid DN: %s" % (opt, e))
 
+
+def check_constructor(option, opt, value):
+    con = option.constructor
+    assert con is not None, "Oops! Developer forgot to set 'constructor' kwarg"
+    try:
+        return con(value)
+    except Exception as e:
+        raise OptionValueError("option {} invalid: {}".format(opt, e))
+
+
 class IPAOption(Option):
     """
     optparse.Option subclass with support of options labeled as
     security-sensitive such as passwords.
     """
-    ATTRS = Option.ATTRS + ["sensitive"]
-    TYPES = Option.TYPES + ("ip", "dn")
+    ATTRS = Option.ATTRS + ["sensitive", "constructor"]
+    TYPES = Option.TYPES + ("ip", "dn", "constructor")
     TYPE_CHECKER = copy(Option.TYPE_CHECKER)
     TYPE_CHECKER["ip"] = check_ip_option
     TYPE_CHECKER["dn"] = check_dn_option
+    TYPE_CHECKER["constructor"] = check_constructor
+
 
 class IPAOptionParser(OptionParser):
     """
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 9dff308..1cac24d 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -8,7 +8,6 @@ Command line support.
 
 import collections
 import enum
-import functools
 import logging
 import optparse  # pylint: disable=deprecated-module
 import signal
@@ -105,17 +104,6 @@ def uninstall_tool(configurable_class, command_name, log_file_name,
     )
 
 
-def _option_callback(action, option, opt_str, value, parser, opt_type):
-    try:
-        value = opt_type(value)
-    except ValueError as e:
-        raise optparse.OptionValueError(
-            "option {0}: {1}".format(opt_str, e))
-
-    option.take_action(
-        action, option.dest, opt_str, value, parser.values, parser)
-
-
 class ConfigureTool(admintool.AdminTool):
     configurable_class = None
     debug_option = False
@@ -186,24 +174,16 @@ class ConfigureTool(admintool.AdminTool):
                 kwargs['metavar'] = "{{{0}}}".format(
                                                 ",".join(kwargs['choices']))
             else:
-                kwargs['nargs'] = 1
-                kwargs['callback_args'] = (knob_scalar_type,)
+                kwargs['type'] = 'constructor'
+                kwargs['constructor'] = knob_scalar_type
             kwargs['dest'] = name
             if issubclass(knob_type, list):
-                if 'type' not in kwargs:
-                    kwargs['action'] = 'callback'
-                    kwargs['callback'] = (
-                        functools.partial(_option_callback, 'append'))
-                elif kwargs['type'] is None:
+                if kwargs['type'] is None:
                     kwargs['action'] = 'append_const'
                 else:
                     kwargs['action'] = 'append'
             else:
-                if 'type' not in kwargs:
-                    kwargs['action'] = 'callback'
-                    kwargs['callback'] = (
-                        functools.partial(_option_callback, 'store'))
-                elif kwargs['type'] is None:
+                if kwargs['type'] is None:
                     kwargs['action'] = 'store_const'
                 else:
                     kwargs['action'] = 'store'