core: better command handling

* check argument of do
* automatically set optional struct members from function signature

Change-Id: I95684f1826c1318ea92fad2bd4c9681d85ea72f5
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/32501
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Alexander Zaft <a.zaft@fz-juelich.de>

frappy/params.py

@@ -25,12 +25,12 @@
 
 import inspect
 
-from frappy.datatypes import BoolType, CommandType, DataType, \
+from frappy.datatypes import ArrayOf, BoolType, CommandType, DataType, \
-    DataTypeType, EnumType, NoneOr, OrType, FloatRange, \
+    DataTypeType, EnumType, FloatRange, NoneOr, OrType, StringType, StructOf, \
-    StringType, StructOf, TextType, TupleOf, ValueType, ArrayOf
+    TextType, TupleOf, ValueType
-from frappy.errors import BadValueError, WrongTypeError, ProgrammingError
+from frappy.errors import BadValueError, ProgrammingError, WrongTypeError
-from frappy.properties import HasProperties, Property
 from frappy.lib import generalConfig
+from frappy.properties import HasProperties, Property
 
 generalConfig.set_default('tolerate_poll_property', False)
 generalConfig.set_default('omit_unchanged_within', 0.1)
@@ -428,6 +428,18 @@ class Command(Accessible):
 
     def __call__(self, func):
         """called when used as decorator"""
+        if isinstance(self.argument, StructOf):
+            # automatically set optional struct members
+            sig = inspect.signature(func)
+            params = set(sig.parameters.keys())
+            params.discard('self')
+            members = set(self.argument.members)
+            if params != members:
+                raise ProgrammingError(f'Command {func.__name__}: Function'
+                                       f' argument names do not match struct'
+                                       f' members!: {params} != {members}')
+            self.argument.optional = [p for p,v in sig.parameters.items()
+                   if v.default is not inspect.Parameter.empty]
         if 'description' not in self.propertyValues and func.__doc__:
             self.description = inspect.cleandoc(func.__doc__)
             self.ownProperties['description'] = self.description
@@ -498,6 +510,19 @@ class Command(Accessible):
         # pylint: disable=unnecessary-dunder-call
         func = self.__get__(module_obj)
         if self.argument:
+            if argument is None:
+                raise WrongTypeError(
+                    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
+
+            self.argument.validate(argument)
             if isinstance(self.argument, TupleOf):
                 res = func(*argument)
             elif isinstance(self.argument, StructOf):

frappy/protocol/dispatcher.py

@@ -132,6 +132,8 @@ class Dispatcher:
         self.reset_connection(conn)
 
     def _execute_command(self, modulename, exportedname, argument=None):
+        """ Execute a command. Importing the value is done in 'do' for nicer
+        error messages."""
         moduleobj = self.secnode.get_module(modulename)
         if moduleobj is None:
             raise NoSuchModuleError(f'Module {modulename!r} does not exist')
@@ -140,9 +142,6 @@ class Dispatcher:
         cobj = moduleobj.commands.get(cname)
         if cobj is None:
             raise NoSuchCommandError(f'Module {modulename!r} has no command {cname or exportedname!r}')
-
-        if cobj.argument:
-            argument = cobj.argument.import_value(argument)
         # now call func
         # note: exceptions are handled in handle_request, not here!
         result = cobj.do(moduleobj, argument)

test/test_params.py

@@ -25,7 +25,7 @@
 # no fixtures needed
 import pytest
 
-from frappy.datatypes import BoolType, FloatRange, IntRange
+from frappy.datatypes import BoolType, FloatRange, IntRange, StructOf
 from frappy.errors import ProgrammingError
 from frappy.modules import HasAccessibles
 from frappy.params import Command, Parameter
@@ -57,6 +57,29 @@ def test_Command():
                                            'description': 'do some other thing'}
 
 
+def test_cmd_struct_opt():
+    with pytest.raises(ProgrammingError):
+        class WrongName(HasAccessibles):  # pylint: disable=unused-variable
+            @Command(StructOf(a=IntRange(), b=IntRange()))
+            def cmd(self, a, c):
+                pass
+    class Mod(HasAccessibles):
+        @Command(StructOf(a=IntRange(), b=IntRange()))
+        def cmd(self, a=5, b=5):
+            pass
+    assert Mod.cmd.datatype.argument.optional == ['a', 'b']
+    class Mod2(HasAccessibles):
+        @Command(StructOf(a=IntRange(), b=IntRange()))
+        def cmd(self, a, b=5):
+            pass
+    assert Mod2.cmd.datatype.argument.optional == ['b']
+    class Mod3(HasAccessibles):
+        @Command(StructOf(a=IntRange(), b=IntRange()))
+        def cmd(self, a, b):
+            pass
+    assert Mod3.cmd.datatype.argument.optional == []
+
+
 def test_Parameter():
     class Mod(HasAccessibles):
         p1 = Parameter('desc1', datatype=FloatRange(), default=0)