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
This commit is contained in:
Jan Cholasta 2012-01-16 09:21:50 -05:00 committed by Martin Kosek
parent 9bb1e6c03e
commit 5a55e11a25
3 changed files with 75 additions and 34 deletions

View File

@ -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(

View File

@ -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()

View File

@ -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)