diff --git a/brewtils/decorators.py b/brewtils/decorators.py index 21508f7b..643d47bf 100644 --- a/brewtils/decorators.py +++ b/brewtils/decorators.py @@ -18,7 +18,7 @@ from lark.common import ParseError from brewtils.choices import parse -from brewtils.errors import PluginParamError +from brewtils.errors import PluginParamError, _deprecate from brewtils.models import Command, Parameter, Choices if sys.version_info.major == 2: @@ -314,7 +314,7 @@ def echo(self, message): # Model is another special case - it requires its own handling if model is not None: param.type = "Dictionary" - param.parameters = _generate_nested_params(model) + param.parameters = _generate_nested_params(model.parameters) # If the model is not nullable and does not have a default we will try # to generate a one using the defaults defined on the model parameters @@ -452,49 +452,57 @@ def _generate_params_from_function(func): return parameters_to_return -def _generate_nested_params(model_class): - """Generates Nested Parameters from a Model Class""" +def _generate_nested_params(parameter_list): + """Generate nested parameters from a list of Parameters + + This function will take a list of Parameters and will return a new list of "real" + Parameters. + + The main thing this does is ensure the choices specification is correct for all + Parameters in the tree. + """ parameters_to_return = [] - for parameter_definition in model_class.parameters: - key = parameter_definition.key - parameter_type = parameter_definition.type - multi = parameter_definition.multi - display_name = parameter_definition.display_name - optional = parameter_definition.optional - default = parameter_definition.default - description = parameter_definition.description - nullable = parameter_definition.nullable - maximum = parameter_definition.maximum - minimum = parameter_definition.minimum - regex = parameter_definition.regex - type_info = parameter_definition.type_info - - choices = _format_choices(parameter_definition.choices) - - nested_parameters = [] - if parameter_definition.parameters: - parameter_type = "Dictionary" - for nested_class in parameter_definition.parameters: - nested_parameters = _generate_nested_params(nested_class) - parameters_to_return.append( - Parameter( - key=key, - type=parameter_type, - multi=multi, - display_name=display_name, - optional=optional, - default=default, - description=description, - choices=choices, - parameters=nested_parameters, - nullable=nullable, - maximum=maximum, - minimum=minimum, - regex=regex, - type_info=type_info, + for param in parameter_list: + + # This is already a Parameter. Only really need to interpret the choices + # definition and recurse down into nested Parameters + if isinstance(param, Parameter): + new_param = Parameter( + key=param.key, + type=param.type, + multi=param.multi, + display_name=param.display_name, + optional=param.optional, + default=param.default, + description=param.description, + choices=_format_choices(param.choices), + nullable=param.nullable, + maximum=param.maximum, + minimum=param.minimum, + regex=param.regex, + type_info=param.type_info, ) - ) + + if param.parameters: + new_param.type = "Dictionary" + new_param.parameters = _generate_nested_params(param.parameters) + + parameters_to_return.append(new_param) + + # This is a model class object. Needed for backwards compatibility + # See https://github.com/beer-garden/beer-garden/issues/354 + elif hasattr(param, "parameters"): + _deprecate( + "Constructing a nested Parameters list using model class objects " + "is deprecated. Please pass the model's parameter list directly." + ) + parameters_to_return += _generate_nested_params(param.parameters) + + # No clue! + else: + raise PluginParamError("Unable to generate parameter from '%s'" % param) + return parameters_to_return diff --git a/test/decorators_test.py b/test/decorators_test.py index 53d8786b..abc5fbdf 100644 --- a/test/decorators_test.py +++ b/test/decorators_test.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import warnings import pytest from mock import Mock, patch @@ -202,62 +203,189 @@ def cmd(_, foo): assert_parameter_equal(model_param.parameters[0], param_1) assert_parameter_equal(model_param.parameters[1], param_2) - def test_deep_nesting(self): - class MyNestedModel: - parameters = [ - Parameter( - key="key2", - type="String", - multi=False, - display_name="y", - optional=False, - default="100", - description="key2", - ) - ] - - class MyModel: - parameters = [ - Parameter( - key="key1", - multi=False, - display_name="x", - optional=True, - description="key1", - parameters=[MyNestedModel], - default="xval", - ) - ] - - @parameter(key="nested_complex", model=MyModel) - def foo(_, nested_complex): - return nested_complex - - assert hasattr(foo, "_command") - assert len(foo._command.parameters) == 1 - - assert foo._command.parameters[0].key == "nested_complex" - assert foo._command.parameters[0].type == "Dictionary" - assert len(foo._command.parameters[0].parameters) == 1 - - nested_param = foo._command.parameters[0].parameters[0] - assert nested_param.key == "key1" - assert nested_param.type == "Dictionary" - assert nested_param.multi is False - assert nested_param.display_name == "x" - assert nested_param.optional is True - assert nested_param.description == "key1" - assert len(nested_param.parameters) == 1 - - double_nested = nested_param.parameters[0] - assert double_nested.key == "key2" - assert double_nested.type == "String" - assert double_nested.multi is False - assert double_nested.display_name == "y" - assert double_nested.optional is False - assert double_nested.default == "100" - assert double_nested.description == "key2" - assert len(double_nested.parameters) == 0 + class TestNesting(object): + """Tests nested model Parameter construction + + This tests both the new, "correct" way to nest parameters (where the given + parameters are, in fact, actual Parameters): + + foo = Parameter(..., parameters=[list of actual Parameter objects], ...) + + And the old, deprecated way (where the given parameters are *not* actual + Parameters, instead they're other Model class objects): + + foo = Parameter(..., parameters=[NestedModel], ...) + + See https://github.com/beer-garden/beer-garden/issues/354 for full details. + """ + + @pytest.fixture + def nested_1(self): + class NestedModel1(object): + parameters = [ + Parameter( + key="key2", + type="String", + multi=False, + display_name="y", + optional=False, + default="100", + description="key2", + ) + ] + + return NestedModel1 + + @pytest.fixture + def nested_2(self): + class NestedModel2(object): + parameters = [ + Parameter( + key="key3", + type="String", + multi=False, + display_name="z", + optional=False, + default="101", + description="key3", + ) + ] + + return NestedModel2 + + def test_nested_parameter_list(self, nested_1, nested_2): + class MyModel(object): + parameters = [ + Parameter( + key="key1", + multi=False, + display_name="x", + optional=True, + description="key1", + parameters=nested_1.parameters + nested_2.parameters, + default="xval", + ) + ] + + @parameter(key="nested_complex", model=MyModel) + def foo(_, nested_complex): + return nested_complex + + self._assert_correct(foo) + + def test_nested_model_list(self, nested_1, nested_2): + class MyModel(object): + parameters = [ + Parameter( + key="key1", + multi=False, + display_name="x", + optional=True, + description="key1", + parameters=[nested_1, nested_2], + default="xval", + ) + ] + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + + @parameter(key="nested_complex", model=MyModel) + def foo(_, nested_complex): + return nested_complex + + # There are 2 nested model class objects so there should be 2 warnings + assert len(w) == 2 + assert w[0].category == DeprecationWarning + assert w[1].category == DeprecationWarning + + self._assert_correct(foo) + + def test_mixed_list(self, nested_1, nested_2): + class MyModel(object): + parameters = [ + Parameter( + key="key1", + multi=False, + display_name="x", + optional=True, + description="key1", + parameters=nested_1.parameters + [nested_2], + default="xval", + ) + ] + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + + @parameter(key="nested_complex", model=MyModel) + def foo(_, nested_complex): + return nested_complex + + # Only 1 nested model class object this time + assert len(w) == 1 + assert w[0].category == DeprecationWarning + + self._assert_correct(foo) + + def test_non_parameter(self): + class MyModel(object): + parameters = [ + Parameter( + key="key1", + multi=False, + display_name="x", + optional=True, + description="key1", + parameters=["Not valid!"], + default="xval", + ) + ] + + with pytest.raises(PluginParamError): + + @parameter(key="nested_complex", model=MyModel) + def foo(_, nested_complex): + return nested_complex + + @staticmethod + def _assert_correct(foo): + assert hasattr(foo, "_command") + assert len(foo._command.parameters) == 1 + + foo_param = foo._command.parameters[0] + assert foo_param.key == "nested_complex" + assert foo_param.type == "Dictionary" + assert len(foo_param.parameters) == 1 + + key1_param = foo_param.parameters[0] + assert key1_param.key == "key1" + assert key1_param.type == "Dictionary" + assert key1_param.multi is False + assert key1_param.display_name == "x" + assert key1_param.optional is True + assert key1_param.description == "key1" + assert len(key1_param.parameters) == 2 + + key2_param = key1_param.parameters[0] + assert key2_param.key == "key2" + assert key2_param.type == "String" + assert key2_param.multi is False + assert key2_param.display_name == "y" + assert key2_param.optional is False + assert key2_param.default == "100" + assert key2_param.description == "key2" + assert len(key2_param.parameters) == 0 + + key3_param = key1_param.parameters[1] + assert key3_param.key == "key3" + assert key3_param.type == "String" + assert key3_param.multi is False + assert key3_param.display_name == "z" + assert key3_param.optional is False + assert key3_param.default == "101" + assert key3_param.description == "key3" + assert len(key3_param.parameters) == 0 @pytest.mark.parametrize( "choices,expected",