From 3a03686381d010cfd20e3457fe99eedbeb3953b4 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Wed, 20 Mar 2019 18:44:31 +0000 Subject: [PATCH 01/12] add default value for action param --- business_rules/actions.py | 18 +++++++++++++----- business_rules/engine.py | 5 +++++ business_rules/utils.py | 38 +++++++++++++++++++++++++++++++++---- tests/test_actions_class.py | 6 +++--- tests/test_engine_logic.py | 38 ++++++++++++++++++++++++++++++++++++- tests/test_utils.py | 6 ++++-- 6 files changed, 96 insertions(+), 15 deletions(-) diff --git a/business_rules/actions.py b/business_rules/actions.py index 4f0b6aa7..85aa7ecb 100644 --- a/business_rules/actions.py +++ b/business_rules/actions.py @@ -33,7 +33,8 @@ def _validate_action_parameters(func, params): for param in params: param_name, field_type = param['name'], param['fieldType'] - if param_name not in func.__code__.co_varnames: + default_value = param.get('defaultValue') + if param_name not in func.__code__.co_varnames and not default_value: raise AssertionError("Unknown parameter name {0} specified for action {1}".format( param_name, func.__name__)) @@ -58,10 +59,11 @@ def wrapper(func): if isinstance(params, dict): params_ = [ dict( - label=fn_name_to_pretty_label(name), - name=name, - fieldType=field_type, - ) for name, field_type in params.items() + label=fn_name_to_pretty_label(key), + name=key, + fieldType=getattr(value, "field_type", value), + defaultValue=getattr(value, "default_value", None) + ) for key, value in params.items() ] _validate_action_parameters(func, params_) @@ -73,3 +75,9 @@ def wrapper(func): return func return wrapper + + +class ActionParam: + def __init__(self, field_type, default_value=None): + self.field_type = field_type + self.default_value = default_value diff --git a/business_rules/engine.py b/business_rules/engine.py index 9b4c4810..e5145fcf 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -236,6 +236,11 @@ def _build_parameters(method, parameters, extra_parameters): else: method_params = {} + if method.params: + for param in method.params: + if isinstance(param, str) and getattr(method.params[param], 'default_value', None): + parameters[param] = method.params[param].default_value + method_params.update(parameters) return method_params diff --git a/business_rules/utils.py b/business_rules/utils.py index 0cbefbb7..c44c67a4 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -109,6 +109,10 @@ def check_params_valid_for_method(method, given_params, method_type_name): defined_params = [param.get('name') for param in method_params] missing_params = set(defined_params).difference(given_params) + # check for default value, if it is present, exclude param from missing params + if method_type_name == method_type.METHOD_TYPE_ACTION: + check_for_default_value_for_missing_params(missing_params, method.params) + if missing_params: raise AssertionError("Missing parameters {0} for {1} {2}".format( ', '.join(missing_params), method_type_name, method.__name__)) @@ -120,6 +124,29 @@ def check_params_valid_for_method(method, given_params, method_type_name): ', '.join(invalid_params), method_type_name, method.__name__)) +def check_for_default_value_for_missing_params(missing_params: set, method_params: list) -> set: + """ + :param missing_params: Params missing from Rule + :param method_params: Params defined on method, which could have default value for missing param + [{ + 'label': 'release voucher from template', + 'name': 'voucher_template_id', + 'fieldType': 'numeric', + 'defaultValue': 123 + }, + ... + ] + :return Updated missing parameters: + """ + if method_params: + for param in method_params: + if isinstance(param, str) and param in missing_params and getattr(method_params[param], 'default_value', + None): + missing_params.remove(param) + + return missing_params + + def validate_rule_data(variables, actions, rule): """ validate_rule_data is used to check a generated rule against a set of variables and actions @@ -196,10 +223,13 @@ def validate_actions(input_actions): """ if type(input_actions) is not list: raise AssertionError('"actions" key must be a list') - for action in input_actions: - method = getattr(actions, action.get('name'), None) - params = action.get('params', {}) - check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) + try: + for action in input_actions: + method = getattr(actions, action.get('name'), None) + params = action.get('params', {}) + check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) + except TypeError: + import pdb;pdb.set_trace() rule_schema = export_rule_data(variables, actions) validate_root_keys(rule) diff --git a/tests/test_actions_class.py b/tests/test_actions_class.py index d191e408..a7a52941 100644 --- a/tests/test_actions_class.py +++ b/tests/test_actions_class.py @@ -1,6 +1,6 @@ from __future__ import absolute_import -from business_rules.actions import BaseActions, rule_action -from business_rules.fields import FIELD_TEXT +from business_rules.actions import BaseActions, rule_action, ActionParam +from business_rules.fields import FIELD_TEXT, FIELD_NUMERIC from . import TestCase @@ -29,7 +29,7 @@ def non_action(self): self.assertEqual(actions[0]['name'], 'some_action') self.assertEqual(actions[0]['label'], 'Some Action') self.assertEqual(actions[0]['params'], [ - {'fieldType': FIELD_TEXT, 'name': 'foo', 'label': 'Foo'}, + {'fieldType': FIELD_TEXT, 'name': 'foo', 'label': 'Foo', 'defaultValue': None}, ]) # should work on an instance of the class too diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index 2ee8d6f7..0b0590eb 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -1,9 +1,11 @@ from __future__ import absolute_import + +import pytest from mock import patch, MagicMock from business_rules import engine from business_rules import fields -from business_rules.actions import BaseActions +from business_rules.actions import BaseActions, ActionParam from business_rules.models import ConditionResult from business_rules.operators import StringType from business_rules.variables import BaseVariables @@ -280,6 +282,40 @@ def test_do_with_invalid_action(self): with self.assertRaisesRegexp(AssertionError, err_string): engine.do_actions(actions, BaseActions(), checked_conditions_results, rule) + def test_do_with_parameter_with_default_value(self): + function_params_mock = MagicMock() + function_params_mock.varkw = None + with patch('business_rules.engine.getfullargspec', return_value=function_params_mock): + # param2 is not set in rule, but there is a default parameter for it in action which will be used instead + rule_actions = [ + { + 'name': 'action', + 'params': {'param1': 'foo'} + } + ] + + rule = { + 'conditions': { + + }, + 'actions': rule_actions + } + + defined_actions = BaseActions() + + action_param = ActionParam(field_type=fields.FIELD_NUMERIC, default_value=42) + defined_actions.action = MagicMock() + defined_actions.action.params = { + 'param1': fields.FIELD_TEXT, + 'param2': action_param + } + + payload = [(True, 'condition_name', 'operator_name', 'condition_value')] + + engine.do_actions(rule_actions, defined_actions, payload, rule) + + defined_actions.action.assert_called_once_with(param1='foo', param2=42) + class EngineCheckConditionsTests(TestCase): def test_case1(self): diff --git a/tests/test_utils.py b/tests/test_utils.py index 2228e7ea..b797cacf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -65,7 +65,8 @@ def test_export_rule_data(): { 'fieldType': 'numeric', 'label': 'Foo', - 'name': 'foo' + 'name': 'foo', + 'defaultValue': None } ] }, @@ -76,7 +77,8 @@ def test_export_rule_data(): { 'fieldType': 'text', 'label': 'Bar', - 'name': 'bar' + 'name': 'bar', + 'defaultValue': None } ] }, From 9972987867339d143afaa7b8de858a43f35c3b27 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Thu, 21 Mar 2019 14:55:39 +0000 Subject: [PATCH 02/12] Fix logic and fix tests --- business_rules/actions.py | 9 ++++-- business_rules/engine.py | 5 ++-- business_rules/utils.py | 19 +++++------- tests/test_engine_logic.py | 60 ++++++++++++++++++++++++++------------ 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/business_rules/actions.py b/business_rules/actions.py index 85aa7ecb..f2fa4bb1 100644 --- a/business_rules/actions.py +++ b/business_rules/actions.py @@ -25,6 +25,12 @@ def _validate_action_parameters(func, params): function `func`, and that the field types are FIELD_* types in fields. :param func: :param params: + { + 'label': 'release voucher from template', + 'name': 'voucher_template_id', + 'fieldType': 'numeric', + 'defaultValue': 123 + } :return: """ if params is not None: @@ -33,8 +39,7 @@ def _validate_action_parameters(func, params): for param in params: param_name, field_type = param['name'], param['fieldType'] - default_value = param.get('defaultValue') - if param_name not in func.__code__.co_varnames and not default_value: + if param_name not in func.__code__.co_varnames: raise AssertionError("Unknown parameter name {0} specified for action {1}".format( param_name, func.__name__)) diff --git a/business_rules/engine.py b/business_rules/engine.py index e5145fcf..4c232791 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -238,8 +238,9 @@ def _build_parameters(method, parameters, extra_parameters): if method.params: for param in method.params: - if isinstance(param, str) and getattr(method.params[param], 'default_value', None): - parameters[param] = method.params[param].default_value + default_param = param.get('defaultValue') + if default_param: + parameters[param['name']] = default_param method_params.update(parameters) diff --git a/business_rules/utils.py b/business_rules/utils.py index c44c67a4..bec593a3 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -111,7 +111,7 @@ def check_params_valid_for_method(method, given_params, method_type_name): # check for default value, if it is present, exclude param from missing params if method_type_name == method_type.METHOD_TYPE_ACTION: - check_for_default_value_for_missing_params(missing_params, method.params) + check_for_default_value_for_missing_params(missing_params, method_params) if missing_params: raise AssertionError("Missing parameters {0} for {1} {2}".format( @@ -140,9 +140,8 @@ def check_for_default_value_for_missing_params(missing_params: set, method_param """ if method_params: for param in method_params: - if isinstance(param, str) and param in missing_params and getattr(method_params[param], 'default_value', - None): - missing_params.remove(param) + if param['name'] in missing_params and param.get('defaultValue'): + missing_params.remove(param['name']) return missing_params @@ -223,13 +222,11 @@ def validate_actions(input_actions): """ if type(input_actions) is not list: raise AssertionError('"actions" key must be a list') - try: - for action in input_actions: - method = getattr(actions, action.get('name'), None) - params = action.get('params', {}) - check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) - except TypeError: - import pdb;pdb.set_trace() + for action in input_actions: + # params here are json params from rule + method = getattr(actions, action.get('name'), None) + params = action.get('params', {}) + check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) rule_schema = export_rule_data(variables, actions) validate_root_keys(rule) diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index 0b0590eb..939432c5 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -5,7 +5,8 @@ from business_rules import engine from business_rules import fields -from business_rules.actions import BaseActions, ActionParam +from business_rules.actions import BaseActions, ActionParam, rule_action +from business_rules.fields import FIELD_TEXT, FIELD_NUMERIC from business_rules.models import ConditionResult from business_rules.operators import StringType from business_rules.variables import BaseVariables @@ -216,21 +217,26 @@ def test_do_actions(self): 'actions': rule_actions } - defined_actions = BaseActions() + action1_mock = MagicMock() + action2_mock = MagicMock() - defined_actions.action1 = MagicMock() - defined_actions.action2 = MagicMock() - defined_actions.action2.params = { - 'param1': fields.FIELD_TEXT, - 'param2': fields.FIELD_NUMERIC - } + class SomeActions(BaseActions): + @rule_action() + def action1(self): + return action1_mock() + + @rule_action(params={'param1': FIELD_TEXT, "param2": FIELD_NUMERIC}) + def action2(self, param1, param2): + return action2_mock(param1=param1, param2=param2) + + defined_actions = SomeActions() payload = [(True, 'condition_name', 'operator_name', 'condition_value')] engine.do_actions(rule_actions, defined_actions, payload, rule) - defined_actions.action1.assert_called_once_with() - defined_actions.action2.assert_called_once_with(param1='foo', param2=10) + action1_mock.assert_called_once_with() + action2_mock.assert_called_once_with(param1='foo', param2=10) def test_do_actions_with_injected_parameters(self): function_params_mock = MagicMock() @@ -255,12 +261,21 @@ def test_do_actions_with_injected_parameters(self): defined_actions = BaseActions() defined_actions.action1 = MagicMock() + defined_actions.action1.params = [] defined_actions.action2 = MagicMock() - defined_actions.action2.params = { - 'param1': fields.FIELD_TEXT, - 'param2': fields.FIELD_NUMERIC - } - + defined_actions.action2.params = [{ + 'label': 'action2', + 'name': 'param1', + 'fieldType': fields.FIELD_TEXT, + 'defaultValue': None + }, + { + 'label': 'action2', + 'name': 'param2', + 'fieldType': fields.FIELD_NUMERIC, + 'defaultValue': None + } + ] payload = [(True, 'condition_name', 'operator_name', 'condition_value')] engine.do_actions(rule_actions, defined_actions, payload, rule) @@ -289,7 +304,7 @@ def test_do_with_parameter_with_default_value(self): # param2 is not set in rule, but there is a default parameter for it in action which will be used instead rule_actions = [ { - 'name': 'action', + 'name': 'some_action', 'params': {'param1': 'foo'} } ] @@ -301,7 +316,16 @@ def test_do_with_parameter_with_default_value(self): 'actions': rule_actions } - defined_actions = BaseActions() + action_param_with_default_value = ActionParam(field_type=fields.FIELD_NUMERIC, default_value=42) + + action_mock = MagicMock() + + class SomeActions(BaseActions): + @rule_action(params={'param1': FIELD_TEXT, 'param2': action_param_with_default_value}) + def some_action(self, param1, param2): + return action_mock(param1=param1, param2=param2) + + defined_actions = SomeActions() action_param = ActionParam(field_type=fields.FIELD_NUMERIC, default_value=42) defined_actions.action = MagicMock() @@ -314,7 +338,7 @@ def test_do_with_parameter_with_default_value(self): engine.do_actions(rule_actions, defined_actions, payload, rule) - defined_actions.action.assert_called_once_with(param1='foo', param2=42) + action_mock.assert_called_once_with(param1='foo', param2=42) class EngineCheckConditionsTests(TestCase): From a721ccdaee718c05dc31425b2760d2b8370cab41 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Thu, 21 Mar 2019 15:04:57 +0000 Subject: [PATCH 03/12] PY2 compatibility --- business_rules/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/business_rules/utils.py b/business_rules/utils.py index bec593a3..a81a0d4a 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -124,7 +124,7 @@ def check_params_valid_for_method(method, given_params, method_type_name): ', '.join(invalid_params), method_type_name, method.__name__)) -def check_for_default_value_for_missing_params(missing_params: set, method_params: list) -> set: +def check_for_default_value_for_missing_params(missing_params, method_params): """ :param missing_params: Params missing from Rule :param method_params: Params defined on method, which could have default value for missing param From 6c9035dc23d3f59c9c474ba14ff38da3a8dc3aeb Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Tue, 26 Mar 2019 15:07:17 +0000 Subject: [PATCH 04/12] Review --- business_rules/actions.py | 4 ++++ business_rules/utils.py | 14 +++++++++----- tests/test_engine_logic.py | 1 - 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/business_rules/actions.py b/business_rules/actions.py index f2fa4bb1..ddc88054 100644 --- a/business_rules/actions.py +++ b/business_rules/actions.py @@ -51,6 +51,10 @@ def _validate_action_parameters(func, params): def rule_action(label=None, params=None): """ Decorator to make a function into a rule action. + If value of params dictionary is field_type, defaultValue is None. + If action parameter requires a default value, ActionParam is used as value in parameters' dict. + It has field_type and default_value attributes which are used as fieldType and defaultValue values accordingly. + NOTE: add **kwargs argument to receive Rule and Matched Conditions as parameters in Action function diff --git a/business_rules/utils.py b/business_rules/utils.py index a81a0d4a..092ac4a1 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -110,8 +110,11 @@ def check_params_valid_for_method(method, given_params, method_type_name): missing_params = set(defined_params).difference(given_params) # check for default value, if it is present, exclude param from missing params + params_with_default_value = set() if method_type_name == method_type.METHOD_TYPE_ACTION: - check_for_default_value_for_missing_params(missing_params, method_params) + params_with_default_value = check_for_default_value_for_missing_params(missing_params, method_params) + + missing_params -= params_with_default_value if missing_params: raise AssertionError("Missing parameters {0} for {1} {2}".format( @@ -136,14 +139,15 @@ def check_for_default_value_for_missing_params(missing_params, method_params): }, ... ] - :return Updated missing parameters: + :return Params that are missing from rule but have default params: {'voucher_template_id'} """ + params_with_default_value = set() if method_params: for param in method_params: - if param['name'] in missing_params and param.get('defaultValue'): - missing_params.remove(param['name']) + if param['name'] in missing_params and param.get('defaultValue') is not None: + params_with_default_value.add(param['name']) - return missing_params + return params_with_default_value def validate_rule_data(variables, actions, rule): diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index 939432c5..f1ddf477 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -1,6 +1,5 @@ from __future__ import absolute_import -import pytest from mock import patch, MagicMock from business_rules import engine From ebd1c16a883aba156b0d697ce3f03aa5c7633cb6 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Fri, 29 Mar 2019 18:44:47 +0000 Subject: [PATCH 05/12] Review --- business_rules/actions.py | 17 +++++++++++------ business_rules/engine.py | 12 ++++++------ business_rules/utils.py | 2 +- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/business_rules/actions.py b/business_rules/actions.py index ddc88054..6f8fa5b8 100644 --- a/business_rules/actions.py +++ b/business_rules/actions.py @@ -51,12 +51,17 @@ def _validate_action_parameters(func, params): def rule_action(label=None, params=None): """ Decorator to make a function into a rule action. - If value of params dictionary is field_type, defaultValue is None. - If action parameter requires a default value, ActionParam is used as value in parameters' dict. - It has field_type and default_value attributes which are used as fieldType and defaultValue values accordingly. - - - NOTE: add **kwargs argument to receive Rule and Matched Conditions as parameters in Action function + `params` parameter could be one of the following: + 1. Dictionary with params names as keys and types as values + Example: + params={ + 'param_name': fields.FIELD_NUMERIC, + } + + 2. If a param has a default value, ActionParam can be used. Example: + params={ + 'voucher_template_id': ActionParam(field_type=fields.FIELD_NUMERIC, default_value=123) + } :param label: Label for Action :param params: Parameters expected by the Action function diff --git a/business_rules/engine.py b/business_rules/engine.py index 4c232791..42d49c43 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -212,6 +212,12 @@ def _build_action_parameters(method, parameters, rule, conditions): 'conditions': conditions } + if getattr(method, 'params'): + for param in method.params: + default_param = param.get('defaultValue') + if default_param: + parameters[param['name']] = default_param + return _build_parameters(method, parameters, extra_parameters) @@ -236,12 +242,6 @@ def _build_parameters(method, parameters, extra_parameters): else: method_params = {} - if method.params: - for param in method.params: - default_param = param.get('defaultValue') - if default_param: - parameters[param['name']] = default_param - method_params.update(parameters) return method_params diff --git a/business_rules/utils.py b/business_rules/utils.py index 092ac4a1..70908b72 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -109,7 +109,7 @@ def check_params_valid_for_method(method, given_params, method_type_name): defined_params = [param.get('name') for param in method_params] missing_params = set(defined_params).difference(given_params) - # check for default value, if it is present, exclude param from missing params + # check for default value in action parameters, if it is present, exclude param from missing params params_with_default_value = set() if method_type_name == method_type.METHOD_TYPE_ACTION: params_with_default_value = check_for_default_value_for_missing_params(missing_params, method_params) From c6b3c6b608ac31657b97def6d0f18ed6126278c3 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Mon, 1 Apr 2019 18:18:08 +0100 Subject: [PATCH 06/12] Fix logic, add test for default parameter --- business_rules/engine.py | 33 ++++++++++++++++++++++-------- business_rules/utils.py | 9 ++++++-- tests/test_engine_logic.py | 42 +++++++++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/business_rules/engine.py b/business_rules/engine.py index 42d49c43..b6a851bb 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -184,7 +184,7 @@ def do_actions(actions, defined_actions, checked_conditions_results, rule): for action in actions: method_name = action['name'] - params = action.get('params', {}) + action_params = action.get('params', {}) method = getattr(defined_actions, method_name, None) @@ -192,12 +192,33 @@ def do_actions(actions, defined_actions, checked_conditions_results, rule): raise AssertionError( "Action {0} is not defined in class {1}".format(method_name, defined_actions.__class__.__name__)) - utils.check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) + params_with_default_value = utils.check_params_valid_for_method(method, action_params, + method_type.METHOD_TYPE_ACTION) - method_params = _build_action_parameters(method, params, rule, successful_conditions) + if params_with_default_value: + action_params = _set_default_values_for_missing_action_params(method, params_with_default_value, action_params) + + method_params = _build_action_parameters(method, action_params, rule, successful_conditions) method(**method_params) +def _set_default_values_for_missing_action_params(method, parameters_with_default_value, action_params): + """ + Adds default parameter from method params to Action parameters. + :param method: Action object. + :param parameters_with_default_value: set of parameters which have a default value for Action parameters. + :param action_params: Action parameters dict, where default parameter will be added. + :return: Modified action_params. + """ + if getattr(method, 'params'): + for param in method.params: + if param['name'] in parameters_with_default_value: + default_param = param.get('defaultValue', None) + if default_param is not None: + action_params[param['name']] = default_param + return action_params + + def _build_action_parameters(method, parameters, rule, conditions): """ Adds extra parameters to the parameters defined for the method @@ -212,12 +233,6 @@ def _build_action_parameters(method, parameters, rule, conditions): 'conditions': conditions } - if getattr(method, 'params'): - for param in method.params: - default_param = param.get('defaultValue') - if default_param: - parameters[param['name']] = default_param - return _build_parameters(method, parameters, extra_parameters) diff --git a/business_rules/utils.py b/business_rules/utils.py index 70908b72..999a4952 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -103,7 +103,10 @@ def check_params_valid_for_method(method, given_params, method_type_name): :param method: :param given_params: Parameters defined within the Rule (Action or Condition) :param method_type_name: A method type defined in util.method_type module - :return: None. Raise exception if parameters don't match (defined in method and Rule) + :return: Set of default values for params which are missing but have a default value. Raise exception if parameters + don't + match (defined in method and + Rule) """ method_params = params_dict_to_list(method.params) defined_params = [param.get('name') for param in method_params] @@ -126,6 +129,8 @@ def check_params_valid_for_method(method, given_params, method_type_name): raise AssertionError("Invalid parameters {0} for {1} {2}".format( ', '.join(invalid_params), method_type_name, method.__name__)) + return params_with_default_value + def check_for_default_value_for_missing_params(missing_params, method_params): """ @@ -144,7 +149,7 @@ def check_for_default_value_for_missing_params(missing_params, method_params): params_with_default_value = set() if method_params: for param in method_params: - if param['name'] in missing_params and param.get('defaultValue') is not None: + if param['name'] in missing_params and param.get('defaultValue', None) is not None: params_with_default_value.add(param['name']) return params_with_default_value diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index f1ddf477..86d6f64b 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -1,7 +1,6 @@ from __future__ import absolute_import from mock import patch, MagicMock - from business_rules import engine from business_rules import fields from business_rules.actions import BaseActions, ActionParam, rule_action @@ -339,6 +338,47 @@ def some_action(self, param1, param2): action_mock.assert_called_once_with(param1='foo', param2=42) + def test_default_param_overrides_action_param(self): + + function_params_mock = MagicMock() + function_params_mock.varkw = None + with patch('business_rules.engine.getfullargspec', return_value=function_params_mock): + rule_actions = [ + { + 'name': 'some_action', + 'params': {'param1': 'foo'} + } + ] + + rule = { + 'conditions': { + + }, + 'actions': rule_actions + } + + action_param_with_default_value = ActionParam(field_type=fields.FIELD_TEXT, default_value='bar') + + action_mock = MagicMock() + + class SomeActions(BaseActions): + @rule_action(params={'param1': action_param_with_default_value}) + def some_action(self, param1): + return action_mock(param1=param1) + + defined_actions = SomeActions() + + defined_actions.action = MagicMock() + defined_actions.action.params = { + 'param1': action_param_with_default_value, + } + + payload = [(True, 'condition_name', 'operator_name', 'condition_value')] + + engine.do_actions(rule_actions, defined_actions, payload, rule) + + action_mock.assert_called_once_with(param1='foo') + class EngineCheckConditionsTests(TestCase): def test_case1(self): From 6085bb24ccd62df190abb880ed77234f43aae930 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Tue, 2 Apr 2019 16:46:25 +0100 Subject: [PATCH 07/12] Add default value for getattr, rename variables --- business_rules/engine.py | 20 +++++++++++--------- business_rules/utils.py | 1 - 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/business_rules/engine.py b/business_rules/engine.py index b6a851bb..b98e7604 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -175,7 +175,7 @@ def do_actions(actions, defined_actions, checked_conditions_results, rule): :param defined_actions: Class with function that implement the logic for each possible action defined in 'actions' parameter :param checked_conditions_results: - :param rule: Rule that is beign executed + :param rule: Rule that is being executed :return: None """ @@ -192,11 +192,13 @@ def do_actions(actions, defined_actions, checked_conditions_results, rule): raise AssertionError( "Action {0} is not defined in class {1}".format(method_name, defined_actions.__class__.__name__)) - params_with_default_value = utils.check_params_valid_for_method(method, action_params, - method_type.METHOD_TYPE_ACTION) + missing_params_with_default_value = utils.check_params_valid_for_method(method, action_params, + method_type.METHOD_TYPE_ACTION) - if params_with_default_value: - action_params = _set_default_values_for_missing_action_params(method, params_with_default_value, action_params) + if missing_params_with_default_value: + action_params = _set_default_values_for_missing_action_params(method, + missing_params_with_default_value, + action_params) method_params = _build_action_parameters(method, action_params, rule, successful_conditions) method(**method_params) @@ -210,12 +212,12 @@ def _set_default_values_for_missing_action_params(method, parameters_with_defaul :param action_params: Action parameters dict, where default parameter will be added. :return: Modified action_params. """ - if getattr(method, 'params'): + if getattr(method, 'params', None): for param in method.params: if param['name'] in parameters_with_default_value: - default_param = param.get('defaultValue', None) - if default_param is not None: - action_params[param['name']] = default_param + default_value = param.get('defaultValue', None) + if default_value is not None: + action_params[param['name']] = default_value return action_params diff --git a/business_rules/utils.py b/business_rules/utils.py index 999a4952..f4ee4072 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -232,7 +232,6 @@ def validate_actions(input_actions): if type(input_actions) is not list: raise AssertionError('"actions" key must be a list') for action in input_actions: - # params here are json params from rule method = getattr(actions, action.get('name'), None) params = action.get('params', {}) check_params_valid_for_method(method, params, method_type.METHOD_TYPE_ACTION) From b9ce51087fbe297c1486bc4cc5be9a2c5537803d Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Tue, 2 Apr 2019 17:40:48 +0100 Subject: [PATCH 08/12] Create new action_params dictionary instead of modifying the old one --- business_rules/engine.py | 12 ++++++++---- tests/test_engine_logic.py | 3 +-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/business_rules/engine.py b/business_rules/engine.py index b98e7604..80e6c146 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -209,16 +209,20 @@ def _set_default_values_for_missing_action_params(method, parameters_with_defaul Adds default parameter from method params to Action parameters. :param method: Action object. :param parameters_with_default_value: set of parameters which have a default value for Action parameters. - :param action_params: Action parameters dict, where default parameter will be added. + :param action_params: Action parameters dict. :return: Modified action_params. """ + modified_action_params = {} if getattr(method, 'params', None): for param in method.params: - if param['name'] in parameters_with_default_value: + param_name = param['name'] + if param_name in parameters_with_default_value: default_value = param.get('defaultValue', None) if default_value is not None: - action_params[param['name']] = default_value - return action_params + modified_action_params[param_name] = default_value + continue + modified_action_params[param_name] = action_params[param_name] + return modified_action_params def _build_action_parameters(method, parameters, rule, conditions): diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index 86d6f64b..72d82a25 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -325,11 +325,10 @@ def some_action(self, param1, param2): defined_actions = SomeActions() - action_param = ActionParam(field_type=fields.FIELD_NUMERIC, default_value=42) defined_actions.action = MagicMock() defined_actions.action.params = { 'param1': fields.FIELD_TEXT, - 'param2': action_param + 'param2': action_param_with_default_value } payload = [(True, 'condition_name', 'operator_name', 'condition_value')] From 4586829d20667aa3c6b8490c9b40be344512aa9d Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Wed, 3 Apr 2019 10:23:40 +0100 Subject: [PATCH 09/12] Bump version --- business_rules/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/business_rules/__init__.py b/business_rules/__init__.py index 54e87a48..e64d144d 100644 --- a/business_rules/__init__.py +++ b/business_rules/__init__.py @@ -1,4 +1,4 @@ -__version__ = '1.4.2' +__version__ = '1.5.0' from .engine import run_all, check_conditions_recursively from .utils import export_rule_data, validate_rule_data From bc3d49a91e1c4c18d36eed97256787f2a5d83b97 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Thu, 4 Apr 2019 17:37:16 +0100 Subject: [PATCH 10/12] Refactor --- business_rules/engine.py | 4 ++-- business_rules/utils.py | 11 +++++------ tests/test_engine_logic.py | 4 ++-- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/business_rules/engine.py b/business_rules/engine.py index 80e6c146..dc82c60a 100644 --- a/business_rules/engine.py +++ b/business_rules/engine.py @@ -204,7 +204,7 @@ def do_actions(actions, defined_actions, checked_conditions_results, rule): method(**method_params) -def _set_default_values_for_missing_action_params(method, parameters_with_default_value, action_params): +def _set_default_values_for_missing_action_params(method, missing_parameters_with_default_value, action_params): """ Adds default parameter from method params to Action parameters. :param method: Action object. @@ -216,7 +216,7 @@ def _set_default_values_for_missing_action_params(method, parameters_with_defaul if getattr(method, 'params', None): for param in method.params: param_name = param['name'] - if param_name in parameters_with_default_value: + if param_name in missing_parameters_with_default_value: default_value = param.get('defaultValue', None) if default_value is not None: modified_action_params[param_name] = default_value diff --git a/business_rules/utils.py b/business_rules/utils.py index f4ee4072..a8802d94 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -114,10 +114,9 @@ def check_params_valid_for_method(method, given_params, method_type_name): # check for default value in action parameters, if it is present, exclude param from missing params params_with_default_value = set() - if method_type_name == method_type.METHOD_TYPE_ACTION: + if method_type_name == method_type.METHOD_TYPE_ACTION and missing_params: params_with_default_value = check_for_default_value_for_missing_params(missing_params, method_params) - - missing_params -= params_with_default_value + missing_params -= params_with_default_value if missing_params: raise AssertionError("Missing parameters {0} for {1} {2}".format( @@ -146,13 +145,13 @@ def check_for_default_value_for_missing_params(missing_params, method_params): ] :return Params that are missing from rule but have default params: {'voucher_template_id'} """ - params_with_default_value = set() + missing_params_with_default_value = set() if method_params: for param in method_params: if param['name'] in missing_params and param.get('defaultValue', None) is not None: - params_with_default_value.add(param['name']) + missing_params_with_default_value.add(param['name']) - return params_with_default_value + return missing_params_with_default_value def validate_rule_data(variables, actions, rule): diff --git a/tests/test_engine_logic.py b/tests/test_engine_logic.py index 72d82a25..627dc423 100644 --- a/tests/test_engine_logic.py +++ b/tests/test_engine_logic.py @@ -345,7 +345,7 @@ def test_default_param_overrides_action_param(self): rule_actions = [ { 'name': 'some_action', - 'params': {'param1': 'foo'} + 'params': {'param1': False} } ] @@ -376,7 +376,7 @@ def some_action(self, param1): engine.do_actions(rule_actions, defined_actions, payload, rule) - action_mock.assert_called_once_with(param1='foo') + action_mock.assert_called_once_with(param1=False) class EngineCheckConditionsTests(TestCase): From 19da2cc5bc35aa707093fdcde91f6fa0cdf8b1d6 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Fri, 5 Apr 2019 18:14:06 +0100 Subject: [PATCH 11/12] Update readme, add boolean field type --- README.md | 20 ++++++++++++++++++++ business_rules/fields.py | 1 + tests/test_utils.py | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cf0fd64d..40369281 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,26 @@ class ProductActions(BaseActions): self.product.save() ``` +If you introduced a new action parameter but already have an older set of active Rules, +which will all require to be updated with this new parameter, you can use `ActionParam` class which +contains default value for parameter along with field type: +```python +from business_rules.actions import ActionParam, BaseActions, rule_action +from business_rules import fields + +class ProductActions(BaseActions): + + def __init__(self, product): + self.product = product + + @rule_action(params={"sale_percentage": fields.FIELD_NUMERIC, + "on_sale": ActionParam(field_type=fields.FIELD_BOOLEAN, default_value=False}) + def put_on_sale(self, sale_percentage, on_sale): + self.product.price = (1.0 - sale_percentage) * self.product.price + self.on_sale = on_sale + self.product.save() + +``` ### 3. Build the rules A rule is just a JSON object that gets interpreted by the business-rules engine. diff --git a/business_rules/fields.py b/business_rules/fields.py index cc3c8af8..2d41d8e5 100644 --- a/business_rules/fields.py +++ b/business_rules/fields.py @@ -5,3 +5,4 @@ FIELD_SELECT_MULTIPLE = 'select_multiple' FIELD_DATETIME = 'datetime' FIELD_TIME = 'time' +FIELD_BOOLEAN = 'boolean' diff --git a/tests/test_utils.py b/tests/test_utils.py index b797cacf..5cdddefa 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -37,7 +37,7 @@ def test_fn_name_to_pretty_label_with_different_cases(): def test_get_valid_fields(): valid_fields = utils.get_valid_fields() - assert len(valid_fields) == 7 + assert len(valid_fields) == 8 def test_params_dict_to_list_when_params_none(): From 4c96bdd7ad15d83e22270aa9bb0f053ef7500856 Mon Sep 17 00:00:00 2001 From: Denis Kolosov Date: Mon, 8 Apr 2019 12:53:10 +0100 Subject: [PATCH 12/12] Rename variables in docstrings to generic --- business_rules/actions.py | 6 +++--- business_rules/utils.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/business_rules/actions.py b/business_rules/actions.py index 6f8fa5b8..647269b0 100644 --- a/business_rules/actions.py +++ b/business_rules/actions.py @@ -26,8 +26,8 @@ def _validate_action_parameters(func, params): :param func: :param params: { - 'label': 'release voucher from template', - 'name': 'voucher_template_id', + 'label': 'action_label', + 'name': 'action_parameter', 'fieldType': 'numeric', 'defaultValue': 123 } @@ -60,7 +60,7 @@ def rule_action(label=None, params=None): 2. If a param has a default value, ActionParam can be used. Example: params={ - 'voucher_template_id': ActionParam(field_type=fields.FIELD_NUMERIC, default_value=123) + 'action_parameter': ActionParam(field_type=fields.FIELD_NUMERIC, default_value=123) } :param label: Label for Action diff --git a/business_rules/utils.py b/business_rules/utils.py index a8802d94..e18eb385 100644 --- a/business_rules/utils.py +++ b/business_rules/utils.py @@ -136,14 +136,14 @@ def check_for_default_value_for_missing_params(missing_params, method_params): :param missing_params: Params missing from Rule :param method_params: Params defined on method, which could have default value for missing param [{ - 'label': 'release voucher from template', - 'name': 'voucher_template_id', + 'label': 'action_label', + 'name': 'action_parameter', 'fieldType': 'numeric', 'defaultValue': 123 }, ... ] - :return Params that are missing from rule but have default params: {'voucher_template_id'} + :return Params that are missing from rule but have default params: {'action_parameter'} """ missing_params_with_default_value = set() if method_params: