From e1cda7d263489b169d493e326b7764e6d2c2c174 Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Mon, 11 Dec 2023 15:02:16 +0100 Subject: [PATCH] fix missing import in change message Transported values in a change must be converted first. As this is only relevant for the exotic "scaled" and "blob" datatypes, this was not detected yet. - add tests - suppress warning PytestUnhandledThreadExceptionWarning in tests + change import_value methods to raise no other exceptions than WrongTypeError and RangeError + simplify Command.do: as import_value already raises the appropriate error, no more try/except is needed Change-Id: I299e511468dc0fcecff4c20cf8a917da38b70786 Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/32743 Tested-by: Jenkins Automated Tests Reviewed-by: Enrico Faulhaber Reviewed-by: Alexander Zaft Reviewed-by: Markus Zolliker --- frappy/datatypes.py | 32 +++++++++----------------------- frappy/params.py | 12 ++++-------- frappy/protocol/dispatcher.py | 4 +++- test/test_datatypes.py | 29 ++++++++++++++++++++++++++++- test/test_modules.py | 2 ++ test/test_poller.py | 1 + 6 files changed, 47 insertions(+), 33 deletions(-) diff --git a/frappy/datatypes.py b/frappy/datatypes.py index 47a5654..8b36df7 100644 --- a/frappy/datatypes.py +++ b/frappy/datatypes.py @@ -106,7 +106,7 @@ class DataType(HasProperties): note: for importing from gui/configfile/commandline use :meth:`from_string` instead. """ - return value + return self(value) def format_value(self, value, unit=None): """format a value of this type into a str string @@ -260,10 +260,6 @@ class FloatRange(HasUnit, DataType): """returns a python object fit for serialisation""" return float(value) - def import_value(self, value): - """returns a python object from serialisation""" - return float(value) - def from_string(self, text): value = float(text) return self(value) @@ -342,10 +338,6 @@ class IntRange(DataType): """returns a python object fit for serialisation""" return int(value) - def import_value(self, value): - """returns a python object from serialisation""" - return int(value) - def from_string(self, text): value = int(text) return self(value) @@ -462,7 +454,10 @@ class ScaledInteger(HasUnit, DataType): def import_value(self, value): """returns a python object from serialisation""" - return self.scale * int(value) + try: + return self.scale * int(value) + except Exception: + raise WrongTypeError(f'can not import {shortrepr(value)} to scaled') from None def from_string(self, text): value = float(text) @@ -514,10 +509,6 @@ class EnumType(DataType): """returns a python object fit for serialisation""" return int(self(value)) - def import_value(self, value): - """returns a python object from serialisation""" - return self(value) - def __call__(self, value): """accepts integers and strings, converts to EnumMember (may be used like an int)""" try: @@ -589,7 +580,10 @@ class BLOBType(DataType): def import_value(self, value): """returns a python object from serialisation""" - return b64decode(value) + try: + return b64decode(value) + except Exception: + raise WrongTypeError(f'can not b64decode {shortrepr(value)}') from None def from_string(self, text): value = text @@ -660,10 +654,6 @@ class StringType(DataType): """returns a python object fit for serialisation""" return f'{value}' - def import_value(self, value): - """returns a python object from serialisation""" - return str(value) - def from_string(self, text): value = str(text) return self(value) @@ -724,10 +714,6 @@ class BoolType(DataType): """returns a python object fit for serialisation""" return self(value) - def import_value(self, value): - """returns a python object from serialisation""" - return self(value) - def from_string(self, text): value = text return self(value) diff --git a/frappy/params.py b/frappy/params.py index 6eab9a7..ee24c7a 100644 --- a/frappy/params.py +++ b/frappy/params.py @@ -500,7 +500,7 @@ class Command(Accessible): """perform function call :param module_obj: the module on which the command is to be executed - :param argument: the argument from the do command + :param argument: the argument from the do command (transported value!) :returns: the return value converted to the result type - when the argument type is TupleOf, the function is called with multiple arguments @@ -515,13 +515,9 @@ class Command(Accessible): f'{module_obj.__class__.__name__}.{self.name} needs an' f' argument of type {self.argument}!' ) - try: - argument = self.argument.import_value(argument) - except TypeError: - pass # validate will raise appropriate message - except ValueError: - pass # validate will raise appropriate message - + # convert transported value to internal value + argument = self.argument.import_value(argument) + # verify range self.argument.validate(argument) if isinstance(self.argument, TupleOf): res = func(*argument) diff --git a/frappy/protocol/dispatcher.py b/frappy/protocol/dispatcher.py index 31a07fa..36792d3 100644 --- a/frappy/protocol/dispatcher.py +++ b/frappy/protocol/dispatcher.py @@ -163,7 +163,9 @@ class Dispatcher: if pobj.readonly: raise ReadOnlyError(f"Parameter {modulename}:{pname} can not be changed remotely") - # validate! + # convert transported value to internal value + value = pobj.datatype.import_value(value) + # verify range value = pobj.datatype.validate(value, previous=pobj.value) # note: exceptions are handled in handle_request, not here! getattr(moduleobj, 'write_' + pname)(value) diff --git a/test/test_datatypes.py b/test/test_datatypes.py index 7bf0a04..8fc5ca1 100644 --- a/test/test_datatypes.py +++ b/test/test_datatypes.py @@ -46,7 +46,6 @@ def test_DataType(): with pytest.raises(NotImplementedError): dt('') dt.export_value('') - dt.import_value('') def test_FloatRange(): @@ -62,8 +61,12 @@ def test_FloatRange(): dt(-9) # convert, but do not check limits with pytest.raises(WrongTypeError): dt('XX') + with pytest.raises(WrongTypeError): + dt.import_value('XX') with pytest.raises(WrongTypeError): dt([19, 'X']) + with pytest.raises(WrongTypeError): + dt.import_value([19, 'X']) dt(1) dt(0) dt(13.14 - 10) # raises an error, if resolution is not handled correctly @@ -115,12 +118,20 @@ def test_IntRange(): dt(-9) # convert, but do not check limits with pytest.raises(WrongTypeError): dt('XX') + with pytest.raises(WrongTypeError): + dt.import_value('XX') with pytest.raises(WrongTypeError): dt([19, 'X']) + with pytest.raises(WrongTypeError): + dt.import_value([19, 'X']) with pytest.raises(WrongTypeError): dt(1.3) + with pytest.raises(WrongTypeError): + dt.import_value(1.3) with pytest.raises(WrongTypeError): dt('1.3') + with pytest.raises(WrongTypeError): + dt.import_value('1.3') dt(1) dt(0) with pytest.raises(ProgrammingError): @@ -153,8 +164,12 @@ def test_ScaledInteger(): dt(-9) # convert, but do not check limits with pytest.raises(WrongTypeError): dt('XX') + with pytest.raises(WrongTypeError): + dt.import_value('XX') with pytest.raises(WrongTypeError): dt([19, 'X']) + with pytest.raises(WrongTypeError): + dt.import_value([19, 'X']) dt(1) dt(0) with pytest.raises(ProgrammingError): @@ -218,12 +233,20 @@ def test_EnumType(): with pytest.raises(RangeError): dt(9) + with pytest.raises(RangeError): + dt.import_value(9) with pytest.raises(RangeError): dt(-9) + with pytest.raises(RangeError): + dt.import_value(-9) with pytest.raises(RangeError): dt('XX') + with pytest.raises(RangeError): + dt.import_value('XX') with pytest.raises(WrongTypeError): dt([19, 'X']) + with pytest.raises(WrongTypeError): + dt.import_value([19, 'X']) assert dt('a') == 3 assert dt('stuff') == 1 @@ -266,6 +289,10 @@ def test_BLOBType(): dt(b'abcdefghijklmno') with pytest.raises(WrongTypeError): dt('abcd') + with pytest.raises(WrongTypeError): + dt.import_value('ab') + with pytest.raises(WrongTypeError): + dt.import_value(b'xxx') assert dt(b'abcd') == b'abcd' dt.setProperty('minbytes', 1) diff --git a/test/test_modules.py b/test/test_modules.py index e1d8313..45490bf 100644 --- a/test/test_modules.py +++ b/test/test_modules.py @@ -77,6 +77,7 @@ class DummyMultiEvent(threading.Event): return trigger +@pytest.mark.filterwarnings('ignore') # ignore PytestUnhandledThreadExceptionWarning def test_Communicator(): o = Communicator('communicator', LoggerStub(), {'description': ''}, ServerStub({})) o.earlyInit() @@ -87,6 +88,7 @@ def test_Communicator(): assert event.wait(timeout=0.1) +@pytest.mark.filterwarnings('ignore') # ignore PytestUnhandledThreadExceptionWarning def test_ModuleMagic(): class Newclass1(Drivable): param1 = Parameter('param1', datatype=BoolType(), default=False) diff --git a/test/test_poller.py b/test/test_poller.py index 13ef74d..57daea0 100644 --- a/test/test_poller.py +++ b/test/test_poller.py @@ -122,6 +122,7 @@ class Mod1(Base, Readable): return 0 +@pytest.mark.filterwarnings('ignore') # ignore PytestUnhandledThreadExceptionWarning @pytest.mark.parametrize( 'ncycles, pollinterval, slowinterval, mspan, pspan', [ # normal case: