diff --git a/news/248.bugfix.md b/news/248.bugfix.md new file mode 100644 index 00000000..d091d2a3 --- /dev/null +++ b/news/248.bugfix.md @@ -0,0 +1 @@ +The `@required_parameters` decorator can work with arguments that do not have a default. @ale-rt diff --git a/src/plone/api/tests/test_validation.py b/src/plone/api/tests/test_validation.py index b35a9dc0..7ed4dd5c 100644 --- a/src/plone/api/tests/test_validation.py +++ b/src/plone/api/tests/test_validation.py @@ -1,5 +1,6 @@ """Tests for plone.api.validation.""" +from plone.api.exc import MissingParameterError from plone.api.tests.base import INTEGRATION_TESTING from plone.api.validation import _get_supplied_args as _gsa from plone.api.validation import at_least_one_of @@ -56,6 +57,38 @@ def test_non_existant_required_arg(self): ) _func(undecorated_func) + def test_required_parameters_with_no_default(self): + """Test that we have a nice validation message even if the + required parameter doesn't have a default value. + """ + + @required_parameters("arg1") + def _func1(arg1, arg2=None, arg3=None): + pass + + with self.assertRaises(MissingParameterError) as e: + _func1() + + self.assertEqual(str(e.exception), "Missing required parameter(s): arg1") + + @required_parameters("arg1", "arg2") + def _func2(arg1, arg2=None, arg3=None): + pass + + with self.assertRaises(MissingParameterError) as e: + _func2() + + self.assertEqual(str(e.exception), "Missing required parameter(s): arg1, arg2") + + @required_parameters("arg1", "arg2") + def _func3(arg1, arg2=1, arg3=None): + pass + + with self.assertRaises(MissingParameterError) as e: + _func3() + + self.assertEqual(str(e.exception), "Missing required parameter(s): arg1, arg2") + def test_get_supplied_args(self): """Test that positional and keyword args are recognised correctly.""" # the arguments specified in the function signature diff --git a/src/plone/api/validation.py b/src/plone/api/validation.py index d8e52b71..9976d75b 100644 --- a/src/plone/api/validation.py +++ b/src/plone/api/validation.py @@ -1,6 +1,7 @@ """Provide decorators for validating parameters.""" from decorator import decorator +from functools import wraps from plone.api.exc import InvalidParameterError from plone.api.exc import MissingParameterError @@ -12,7 +13,17 @@ def _get_arg_spec(func, validator_args): and check that the decorator doesn't refer to non-existent args. """ - signature_args = inspect.getfullargspec(func).args + signature = inspect.signature(func) + signature_args = [ + name + for name, param in signature.parameters.items() + if param.kind + in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.KEYWORD_ONLY, + ) + ] extra_args = set(validator_args) - set(signature_args) if extra_args: raise ValueError( @@ -32,9 +43,10 @@ def _get_supplied_args(signature_params, args, kwargs): either as positional or keyword arguments, and are not None. """ supplied_args = [] - for index in range(len(args)): - if args[index] is not None: - supplied_args.append(signature_params[index]) + for index, arg in enumerate(args): + if arg is not None: + arg_name = signature_params[index] + supplied_args.append(arg_name) for keyword in kwargs: if kwargs[keyword] is not None: @@ -58,7 +70,8 @@ def _required_parameters(func): """Provide actual decorator.""" signature_params = _get_arg_spec(func, required_params) - def wrapped(function, *args, **kwargs): + @wraps(func) + def wrapped(*args, **kwargs): """Provide wrapped function (whose docstring will get replaced).""" supplied_args = _get_supplied_args(signature_params, args, kwargs) @@ -70,9 +83,9 @@ def wrapped(function, *args, **kwargs): ), ) - return function(*args, **kwargs) + return func(*args, **kwargs) - return decorator(wrapped, func) + return wrapped return _required_parameters