On 27.3.2012 17:41, Martin Kosek wrote:
On Tue, 2012-03-27 at 16:42 +0200, Martin Kosek wrote:
On Tue, 2012-03-27 at 16:30 +0200, Jan Cholasta wrote:
On 27.3.2012 16:00, Martin Kosek wrote:
On Thu, 2012-03-15 at 14:57 +0100, Jan Cholasta wrote:
On 15.3.2012 14:20, Petr Viktorin wrote:
On 03/15/2012 12:05 PM, Jan Cholasta wrote:
On 15.3.2012 11:36, Jan Cholasta wrote:
(this is a continuation of
<http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html>)




Hi,

the attached patches fix<https://fedorahosted.org/freeipa/ticket/1847>
and<https://fedorahosted.org/freeipa/ticket/2245>:

[PATCH] Fix the procedure for getting default values of command
parameters.

The parameters used in default_from of other parameters are now properly
validated before the default_from is called.

[PATCH] Change parameters to use only default_from for dynamic default
values.

Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for proper
parameter validation, as there is no way to tell which parameters to
validate prior to calling create_default, because create_default does
not provide information about which parameters are used for generating
the default value.

Honza


Forgot to remove one FIXME bit in dns.py. Update patch attached.

Honza


   >   diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
   >   index a10960a..61c645d 100644
   >   --- a/ipalib/plugins/dns.py
   >   +++ b/ipalib/plugins/dns.py
   >   @@ -1528,7 +1528,7 @@ class dnszone(LDAPObject):
   >   label=_('SOA serial'),
   >   doc=_('SOA record serial number'),
   >   minvalue=1,
   >   - create_default=_create_zone_serial,
   >   + default_from=_create_zone_serial,
   >   autofill=True,
   >   ),
   >   Int('idnssoarefresh',
   >   diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py
   >   index b26f7e9..9bee314 100644
   >   --- a/ipalib/plugins/passwd.py
   >   +++ b/ipalib/plugins/passwd.py
   >   @@ -69,7 +69,7 @@ class passwd(Command):
   >   label=_('User name'),
   >   primary_key=True,
   >   autofill=True,
   >   - create_default=lambda **kw: util.get_current_principal(),
   >   + default_from=lambda: util.get_current_principal(),
   >   normalizer=lambda value: normalize_principal(value),
   >   ),
   >   Password('password',


This is just a minor nitpick, but I'd like to know if there's a reason
behind it: why are you sometimes using lambda and sometimes not?

I use lambda as a protective measure against accidents caused by adding
optional arguments to the functions used. _create_zone_serial is an
exception to that rule, because it is private to the dns plugin.


The patch works well here, but I think I'm not the one to ack it.


Honza


The patch looks OK, I found just minor issues.

1) We may want to add some check for wildcards (**kw) in default_from, I
guess it would mess with your dependency solver. Some nice error would
warn developers that they are doing something bad.

Added the check.


2) Patch 47.4 needs minor rebasing

Done.


Martin


Updated patches attached.

Honza


I think you squashed the change with an incorrect commit. The check
should be rather included in patch 44.

I beg to differ, patch 44 changes the way parameter defaults are generated, which is not affected by this change at all. The check is there just to warn developers early if they use default_from in a wrong way (it wouldn't have worked even without the check). This could have happened in patch 47, if I had done a bad job replacing create_default with default_from, so the check belongs there (or into a separate patch).


Martin

I just noticed it breaks one unit test:

======================================================================
ERROR: Test the `ipalib.parameters.DefaultFrom.__init__` method.
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
     self.test(*self.arg)
   File "/home/mkosek/freeipa/tests/test_ipalib/test_parameters.py", line 52, 
in test_init
     o = self.cls(callback, *keys)
   File "/home/mkosek/freeipa/ipalib/parameters.py", line 201, in __init__
     raise ValueError("callback: variable-length argument list not allowed")
ValueError: callback: variable-length argument list not allowed

----------------------------------------------------------------------

Fixed.


I also think it would be useful to have one test specifically for this
new check.

Added.


Martin


Updated patches attached.

Honza

--
Jan Cholasta
>From 0ac41ae8536caaff63e7e84f71affdf5fc9c0b5c Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 16 Jan 2012 09:21:50 -0500
Subject: [PATCH 2/2] Fix the procedure for getting default values of command
 parameters.

The parameters used in default_from of other parameters are now
properly validated before the default_from is called.

ticket 1847
---
 ipalib/cli.py         |    8 ++--
 ipalib/frontend.py    |   79 ++++++++++++++++++++++++++++++++++++++++---------
 ipalib/plugins/dns.py |   22 ++++----------
 3 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index ea320cf..f72cca5 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -48,7 +48,7 @@ import plugable
 import util
 from errors import (PublicError, CommandError, HelpError, InternalError,
         NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError,
-        PromptFailed)
+        PromptFailed, ConversionError)
 from constants import CLI_TAB
 from parameters import Password, Bytes, File, Str, StrEnum
 from text import _
@@ -1152,7 +1152,7 @@ class cli(backend.Executioner):
             if (param.required and param.name not in kw) or \
                 (param.alwaysask and honor_alwaysask) or self.env.prompt_all:
                 if param.autofill:
-                    kw[param.name] = param.get_default(**kw)
+                    kw[param.name] = cmd.get_default_of(param.name, **kw)
                 if param.name in kw and kw[param.name] is not None:
                     continue
                 if param.password:
@@ -1160,7 +1160,7 @@ class cli(backend.Executioner):
                         param.label, param.confirm
                     )
                 else:
-                    default = param.get_default(**kw)
+                    default = cmd.get_default_of(param.name, **kw)
                     error = None
                     while True:
                         if error is not None:
@@ -1172,7 +1172,7 @@ class cli(backend.Executioner):
                             if value is not None:
                                 kw[param.name] = value
                             break
-                        except ValidationError, e:
+                        except (ValidationError, ConversionError), e:
                             error = e.error
             elif param.password and kw.get(param.name, False) is True:
                 kw[param.name] = self.Backend.textui.prompt_password(
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 47c72d1..f665963 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -396,6 +396,7 @@ class Command(HasParam):
     args = Plugin.finalize_attr('args')
     options = Plugin.finalize_attr('options')
     params = Plugin.finalize_attr('params')
+    params_by_default = Plugin.finalize_attr('params_by_default')
     obj = None
 
     use_output_validation = True
@@ -419,11 +420,7 @@ class Command(HasParam):
         self.debug(
             'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
         )
-        while True:
-            default = self.get_default(**params)
-            if len(default) == 0:
-                break
-            params.update(default)
+        params.update(self.get_default(**params))
         params = self.normalize(**params)
         params = self.convert(**params)
         self.debug(
@@ -648,17 +645,53 @@ class Command(HasParam):
         >>> c.get_default(color=u'Yellow')
         {}
         """
-        return dict(self.__get_default_iter(kw))
+        params = [p.name for p in self.params() if p.name not in kw and (p.required or p.autofill)]
+        return dict(self.__get_default_iter(params, kw))
 
-    def __get_default_iter(self, kw):
+    def get_default_of(self, name, **kw):
         """
-        Generator method used by `Command.get_default`.
+        Return default value for parameter `name`.
         """
-        for param in self.params():
-            if param.name in kw:
-                continue
-            if param.required or param.autofill:
-                default = param.get_default(**kw)
+        default = dict(self.__get_default_iter([name], kw))
+        return default.get(name)
+
+    def __get_default_iter(self, params, kw):
+        """
+        Generator method used by `Command.get_default` and `Command.get_default_of`.
+        """
+        # Find out what additional parameters are needed to dynamically create
+        # the default values with default_from.
+        dep = set()
+        for param in reversed(self.params_by_default):
+            if param.name in params or param.name in dep:
+                if param.default_from is None:
+                    continue
+                for name in param.default_from.keys:
+                    dep.add(name)
+
+        for param in self.params_by_default():
+            default = None
+            hasdefault = False
+            if param.name in dep:
+                if param.name in kw:
+                    # Parameter is specified, convert and validate the value.
+                    kw[param.name] = param(kw[param.name], **kw)
+                else:
+                    # Parameter is not specified, use default value. Convert
+                    # and validate the value, it might not be returned so
+                    # there's no guarantee it will be converted and validated
+                    # later.
+                    default = param(None, **kw)
+                    if default is not None:
+                        kw[param.name] = default
+                    hasdefault = True
+            if param.name in params:
+                if not hasdefault:
+                    # Default value is not available from the previous step,
+                    # get it now. At this point it is certain that the value
+                    # will be returned, so let the caller care about conversion
+                    # and validation.
+                    default = param.get_default(**kw)
                 if default is not None:
                     yield (param.name, default)
 
@@ -753,6 +786,7 @@ class Command(HasParam):
         else:
             self.max_args = None
         self._create_param_namespace('options')
+        params_nosort = tuple(self.args()) + tuple(self.options()) #pylint: disable=E1102
         def get_key(p):
             if p.required:
                 if p.sortorder < 0:
@@ -762,9 +796,26 @@ class Command(HasParam):
                 return 1
             return 2
         self.params = NameSpace(
-            sorted(tuple(self.args()) + tuple(self.options()), key=get_key),    #pylint: disable=E1102
+            sorted(params_nosort, key=get_key),
             sort=False
         )
+        # Sort params so that the ones with default_from come after the ones
+        # that the default_from might depend on and save the result in
+        # params_by_default namespace.
+        params = []
+        for i in params_nosort:
+            pos = len(params)
+            for j in params_nosort:
+                if j.default_from is None:
+                    continue
+                if i.name not in j.default_from.keys:
+                    continue
+                try:
+                    pos = min(pos, params.index(j))
+                except ValueError:
+                    pass
+            params.insert(pos, i)
+        self.params_by_default = NameSpace(params, sort=False)
         self.output = NameSpace(self._iter_output(), sort=False)
         self._create_param_namespace('output_params')
         super(Command, self)._on_finalize()
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 72e7822..b7069fb 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1694,13 +1694,6 @@ class dnszone_add(LDAPCreate):
         ),
     )
 
-    def args_options_2_params(self, *args, **options):
-        # FIXME: Check if name_from_ip is valid. The framework should do that,
-        #        but it does not. Before it's fixed, this should suffice.
-        if 'name_from_ip' in options:
-            self.obj.params['name_from_ip'](unicode(options['name_from_ip']))
-        return super(dnszone_add, self).args_options_2_params(*args, **options)
-
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         if not dns_container_exists(self.api.Backend.ldap2):
             raise errors.NotFound(reason=_('DNS is not configured'))
@@ -1747,13 +1740,6 @@ api.register(dnszone_del)
 class dnszone_mod(LDAPUpdate):
     __doc__ = _('Modify DNS zone (SOA record).')
 
-    def args_options_2_params(self, *args, **options):
-        # FIXME: Check if name_from_ip is valid. The framework should do that,
-        #        but it does not. Before it's fixed, this should suffice.
-        if 'name_from_ip' in options:
-            self.obj.params['name_from_ip'](unicode(options['name_from_ip']))
-        return super(dnszone_mod, self).args_options_2_params(*args, **options)
-
 api.register(dnszone_mod)
 
 
@@ -1761,8 +1747,12 @@ class dnszone_find(LDAPSearch):
     __doc__ = _('Search for DNS zones (SOA records).')
 
     def args_options_2_params(self, *args, **options):
-        # FIXME: Check if name_from_ip is valid. The framework should do that,
-        #        but it does not. Before it's fixed, this should suffice.
+        # FIXME: Check that name_from_ip is valid. This is necessary because
+        #        custom validation rules, including _validate_ipnet, are not
+        #        used when doing a search. Once we have a parameter type for
+        #        IP network objects, this will no longer be necessary, as the
+        #        parameter type will handle the validation itself (see
+        #        <https://fedorahosted.org/freeipa/ticket/2266>).
         if 'name_from_ip' in options:
             self.obj.params['name_from_ip'](unicode(options['name_from_ip']))
         return super(dnszone_find, self).args_options_2_params(*args, **options)
-- 
1.7.7.6

>From 457e84c0d292ab41eed9bc0eb270db88bdabcbbb Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 15 Mar 2012 04:32:37 -0400
Subject: [PATCH 1/2] Change parameters to use only default_from for dynamic
 default values.

Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for
proper parameter validation, as there is no way to tell which
parameters to validate prior to calling create_default, because
create_default does not provide information about which parameters are
used for generating the default value.
---
 ipalib/parameters.py                 |   79 +++-------------------------------
 ipalib/plugins/dns.py                |    4 +-
 ipalib/plugins/passwd.py             |    2 +-
 ipaserver/plugins/join.py            |    4 +-
 make-lint                            |    6 +-
 makeapi                              |    1 -
 tests/test_ipalib/test_parameters.py |   49 +++++++--------------
 7 files changed, 30 insertions(+), 115 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index ec6020e..5040234 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -198,6 +198,8 @@ class DefaultFrom(ReadOnly):
         self.callback = callback
         if len(keys) == 0:
             fc = callback.func_code
+            if fc.co_flags & 0x0c:
+                raise ValueError("callback: variable-length argument list not allowed")
             self.keys = fc.co_varnames[:fc.co_argcount]
         else:
             self.keys = keys
@@ -308,13 +310,9 @@ class Param(ReadOnly):
         encoder
       - default_from: a custom function for generating default values of
         parameter instance
-      - create_default: a custom function for generating default values of
-        parameter instance. Unlike default_from attribute, this function
-        is not wrapped. `Param.get_default()` documentation provides further
-        details
       - autofill: by default, only `required` parameters get a default value
-        from default_from or create_default functions. When autofill is
-        enabled, optional attributes get the default value filled too
+        from the default_from function. When autofill is enabled, optional
+        attributes get the default value filled too
       - query: this attribute is controlled by framework. When the `query`
         is enabled, framework assumes that the value is only queried and not
         inserted in the LDAP. Validation is then relaxed - custom
@@ -379,7 +377,6 @@ class Param(ReadOnly):
         ('normalizer', callable, None),
         ('encoder', callable, None),
         ('default_from', DefaultFrom, None),
-        ('create_default', callable, None),
         ('autofill', bool, False),
         ('query', bool, False),
         ('attribute', bool, False),
@@ -481,20 +478,6 @@ class Param(ReadOnly):
                 class_rules.append(getattr(self, rule_name))
         check_name(self.cli_name)
 
-        # Check that only default_from or create_default was provided:
-        assert not hasattr(self, '_get_default'), self.nice
-        if callable(self.default_from):
-            if callable(self.create_default):
-                raise ValueError(
-                    '%s: cannot have both %r and %r' % (
-                        self.nice, 'default_from', 'create_default')
-                )
-            self._get_default = self.default_from
-        elif callable(self.create_default):
-            self._get_default = self.create_default
-        else:
-            self._get_default = None
-
         # Check that only 'include' or 'exclude' was provided:
         if None not in (self.include, self.exclude):
             raise ValueError(
@@ -990,59 +973,9 @@ class Param(ReadOnly):
         >>> kw = dict(first=u'John', department=u'Engineering')
         >>> login.get_default(**kw)
         u'my-static-login-default'
-
-        The second, less common way to construct a dynamic default is to provide
-        a callback via the ``create_default`` keyword argument.  Unlike a
-        ``default_from`` callback, your ``create_default`` callback will not get
-        wrapped in any dispatcher.  Instead, it will be called directly, which
-        means your callback must accept arbitrary keyword arguments, although
-        whether your callback utilises these values is up to your
-        implementation.  For example:
-
-        >>> def make_csr(**kw):
-        ...     print '  make_csr(%r)' % (kw,)  # Note output below
-        ...     return 'Certificate Signing Request'
-        ...
-        >>> csr = Bytes('csr', create_default=make_csr)
-
-        Your ``create_default`` callback will be called with whatever keyword
-        arguments are passed to `Param.get_default()`.  For example:
-
-        >>> kw = dict(arbitrary='Keyword', arguments='Here')
-        >>> csr.get_default(**kw)
-          make_csr({'arguments': 'Here', 'arbitrary': 'Keyword'})
-        'Certificate Signing Request'
-
-        And your ``create_default`` callback is called even if
-        `Param.get_default()` is called with *zero* keyword arguments.
-        For example:
-
-        >>> csr.get_default()
-          make_csr({})
-        'Certificate Signing Request'
-
-        The ``create_default`` callback will most likely be used as a
-        pre-execute hook to perform some special client-side operation.  For
-        example, the ``csr`` parameter above might make a call to
-        ``/usr/bin/openssl``.  However, often a ``create_default`` callback
-        could also be implemented as a ``default_from`` callback.  When this is
-        the case, a ``default_from`` callback should be used as they are more
-        structured and therefore less error-prone.
-
-        The ``default_from`` and ``create_default`` keyword arguments are
-        mutually exclusive.  If you provide both, a ``ValueError`` will be
-        raised.  For example:
-
-        >>> homedir = Str('home',
-        ...     default_from=lambda login: '/home/%s' % login,
-        ...     create_default=lambda **kw: '/lets/use/this',
-        ... )
-        Traceback (most recent call last):
-          ...
-        ValueError: Str('home'): cannot have both 'default_from' and 'create_default'
         """
-        if self._get_default is not None:
-            default = self._get_default(**kw)
+        if self.default_from is not None:
+            default = self.default_from(**kw)
             if default is not None:
                 try:
                     return self.convert(self.normalize(default))
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index aea04b4..72e7822 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -234,7 +234,7 @@ def _rname_validator(ugettext, zonemgr):
         return unicode(e)
     return None
 
-def _create_zone_serial(**kwargs):
+def _create_zone_serial():
     """ Generate serial number for zones. The format follows RFC 1912 """
     return int('%s01' % time.strftime('%Y%m%d'))
 
@@ -1554,7 +1554,7 @@ class dnszone(LDAPObject):
             label=_('SOA serial'),
             doc=_('SOA record serial number'),
             minvalue=1,
-            create_default=_create_zone_serial,
+            default_from=_create_zone_serial,
             autofill=True,
         ),
         Int('idnssoarefresh',
diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py
index b26f7e9..9bee314 100644
--- a/ipalib/plugins/passwd.py
+++ b/ipalib/plugins/passwd.py
@@ -69,7 +69,7 @@ class passwd(Command):
             label=_('User name'),
             primary_key=True,
             autofill=True,
-            create_default=lambda **kw: util.get_current_principal(),
+            default_from=lambda: util.get_current_principal(),
             normalizer=lambda value: normalize_principal(value),
         ),
         Password('password',
diff --git a/ipaserver/plugins/join.py b/ipaserver/plugins/join.py
index 81c336b..0073363 100644
--- a/ipaserver/plugins/join.py
+++ b/ipaserver/plugins/join.py
@@ -52,7 +52,7 @@ class join(Command):
             validate_host,
             cli_name='hostname',
             doc=_("The hostname to register as"),
-            create_default=lambda **kw: unicode(util.get_fqdn()),
+            default_from=lambda: unicode(util.get_fqdn()),
             autofill=True,
             #normalizer=lamda value: value.lower(),
         ),
@@ -60,7 +60,7 @@ class join(Command):
     takes_options= (
         Str('realm',
             doc=_("The IPA realm"),
-            create_default=lambda **kw: get_realm(),
+            default_from=lambda: get_realm(),
             autofill=True,
         ),
         Str('nshardwareplatform?',
diff --git a/make-lint b/make-lint
index 6b54312..7ecd59d 100755
--- a/make-lint
+++ b/make-lint
@@ -56,9 +56,9 @@ class IPATypeChecker(TypeChecker):
         'ipalib.plugins.misc.env': ['env'],
         'ipalib.parameters.Param': ['cli_name', 'cli_short_name', 'label',
             'doc', 'required', 'multivalue', 'primary_key', 'normalizer',
-            'default', 'default_from', 'create_default', 'autofill', 'query',
-            'attribute', 'include', 'exclude', 'flags', 'hint', 'alwaysask',
-            'sortorder', 'csv', 'csv_separator', 'csv_skipspace'],
+            'default', 'default_from', 'autofill', 'query', 'attribute',
+            'include', 'exclude', 'flags', 'hint', 'alwaysask', 'sortorder',
+            'csv', 'csv_separator', 'csv_skipspace'],
         'ipalib.parameters.Bool': ['truths', 'falsehoods'],
         'ipalib.parameters.Int': ['minvalue', 'maxvalue'],
         'ipalib.parameters.Decimal': ['minvalue', 'maxvalue', 'precision'],
diff --git a/makeapi b/makeapi
index 007531a..b9c149e 100755
--- a/makeapi
+++ b/makeapi
@@ -45,7 +45,6 @@ PARAM_IGNORED_KW_ATTRIBUTES = ('label',
                                'normalizer',
                                'encoder',
                                'default_from',
-                               'create_default',
                                'hint',
                                'flags',
                                'sortorder',)
diff --git a/tests/test_ipalib/test_parameters.py b/tests/test_ipalib/test_parameters.py
index 83c33dd..fc9569e 100644
--- a/tests/test_ipalib/test_parameters.py
+++ b/tests/test_ipalib/test_parameters.py
@@ -64,6 +64,16 @@ class test_DefaultFrom(ClassChecker):
         e = raises(TypeError, self.cls, callback, 'givenname', 17)
         assert str(e) == TYPE_ERROR % ('keys', str, 17, int)
 
+        # Test that ValueError is raised when inferring keys from a callback
+        # which has *args:
+        e = raises(ValueError, self.cls, lambda foo, *args: None)
+        assert str(e) == "callback: variable-length argument list not allowed"
+
+        # Test that ValueError is raised when inferring keys from a callback
+        # which has **kwargs:
+        e = raises(ValueError, self.cls, lambda foo, **kwargs: None)
+        assert str(e) == "callback: variable-length argument list not allowed"
+
     def test_repr(self):
         """
         Test the `ipalib.parameters.DefaultFrom.__repr__` method.
@@ -185,8 +195,6 @@ class test_Param(ClassChecker):
         assert o.normalizer is None
         assert o.default is None
         assert o.default_from is None
-        assert o.create_default is None
-        assert o._get_default is None
         assert o.autofill is False
         assert o.query is False
         assert o.attribute is False
@@ -250,16 +258,6 @@ class test_Param(ClassChecker):
         assert str(e) == \
             "Param('my_param'): takes no such kwargs: 'ape', 'great'"
 
-        # Test that ValueError is raised if you provide both default_from and
-        # create_default:
-        e = raises(ValueError, self.cls, 'my_param',
-            default_from=lambda first, last: first[0] + last,
-            create_default=lambda **kw: 'The Default'
-        )
-        assert str(e) == '%s: cannot have both %r and %r' % (
-            "Param('my_param')", 'default_from', 'create_default',
-        )
-
         # Test that ValueError is raised if you provide both include and
         # exclude:
         e = raises(ValueError, self.cls, 'my_param',
@@ -276,15 +274,11 @@ class test_Param(ClassChecker):
         e = raises(ValueError, self.cls, 'my_param', csv=True)
         assert str(e) == '%s: cannot have csv without multivalue' % "Param('my_param')"
 
-        # Test that _get_default gets set:
-        call1 = lambda first, last: first[0] + last
-        call2 = lambda **kw: 'The Default'
-        o = self.cls('my_param', default_from=call1)
-        assert o.default_from.callback is call1
-        assert o._get_default is o.default_from
-        o = self.cls('my_param', create_default=call2)
-        assert o.create_default is call2
-        assert o._get_default is call2
+        # Test that default_from gets set:
+        call = lambda first, last: first[0] + last
+        o = self.cls('my_param', default_from=call)
+        assert type(o.default_from) is parameters.DefaultFrom
+        assert o.default_from.callback is call
 
     def test_repr(self):
         """
@@ -579,7 +573,7 @@ class test_Param(ClassChecker):
 
     def test_get_default(self):
         """
-        Test the `ipalib.parameters.Param._get_default` method.
+        Test the `ipalib.parameters.Param.get_default` method.
         """
         class PassThrough(object):
             value = None
@@ -624,17 +618,6 @@ class test_Param(ClassChecker):
         assert o._convert_scalar.value is default
         assert o.normalizer.value is default
 
-        # Test with create_default:
-        o = Str('my_str',
-            normalizer=PassThrough(),
-            default=u'Static Default',
-            create_default=lambda **kw: u'The created default',
-        )
-        default = o.get_default(first=u'john', last='doe')
-        assert_equal(default, u'The created default')
-        assert o._convert_scalar.value is default
-        assert o.normalizer.value is default
-
     def test_split_csv(self):
         """
         Test the `ipalib.parameters.Param.split_csv` method with csv.
-- 
1.7.7.6

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

Reply via email to