do not convert string to float

a read method should not reply on the automatic conversion
of the return value from string to a number.

- transitional solution with generalConfig.lazy_numer_validation
+ changing slighly generalInit mechanism: for above feature
  generalConfig.init is not required to be called (i.e. when
  used on the client side)

Change-Id: Ibecce1a45669273c105932acdc0908de55bfd1b9
Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/27516
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

secop/datatypes.py

@@ -30,7 +30,7 @@ from base64 import b64decode, b64encode
 
 from secop.errors import BadValueError, \
     ConfigError, ProgrammingError, ProtocolError
-from secop.lib import clamp
+from secop.lib import clamp, generalConfig
 from secop.lib.enum import Enum
 from secop.parse import Parser
 from secop.properties import HasProperties, Property
@@ -49,6 +49,7 @@ __all__ = [
 DEFAULT_MIN_INT = -16777216
 DEFAULT_MAX_INT = 16777216
 UNLIMITED = 1 << 64  # internal limit for integers, is probably high enough for any datatype size
+generalConfig.defaults['lazy_number_validation'] = True  # should be changed to False later
 
 Parser = Parser()
 
@@ -192,7 +193,10 @@ class FloatRange(DataType):
 
     def __call__(self, value):
         try:
-            value = float(value)
+            if generalConfig.lazy_number_validation:
+                value = float(value)  # for legacy code
+            else:
+                value += 0.0  # do not accept strings here
         except Exception:
             raise BadValueError('Can not convert %r to float' % value) from None
         # map +/-infty to +/-max possible number
@@ -265,7 +269,10 @@ class IntRange(DataType):
 
     def __call__(self, value):
         try:
-            fvalue = float(value)
+            if generalConfig.lazy_number_validation:
+                fvalue = float(value)  # for legacy code
+            else:
+                fvalue = value + 0.0  # do not accept strings here
             value = int(value)
         except Exception:
             raise BadValueError('Can not convert %r to int' % value) from None
@@ -369,7 +376,10 @@ class ScaledInteger(DataType):
 
     def __call__(self, value):
         try:
-            value = float(value)
+            if generalConfig.lazy_number_validation:
+                value = float(value)  # for legacy code
+            else:
+                value += 0.0  # do not accept strings here
         except Exception:
             raise BadValueError('Can not convert %r to float' % value) from None
         prec = max(self.scale, abs(value * self.relative_resolution),

secop/lib/__init__.py

@@ -32,8 +32,10 @@ from os import environ, path
 
 
 class GeneralConfig:
+
     def __init__(self):
         self._config = None
+        self.defaults = {}  #: default values. may be set before or after :meth:`init`
 
     def init(self, configfile=None):
         cfg = {}
@@ -82,7 +84,13 @@ class GeneralConfig:
     def __getitem__(self, key):
         try:
             return self._config[key]
+        except KeyError:
+            return self.defaults[key]
         except TypeError:
+            if key in self.defaults:
+                # accept retrieving defaults before init
+                # e.g. 'lazy_number_validation' in secop.datatypes
+                return self.defaults[key]
             raise TypeError('generalConfig.init() has to be called first') from None
 
     def get(self, key, default=None):
@@ -101,18 +109,14 @@ class GeneralConfig:
         """goodie: use generalConfig.<key> instead of generalConfig.get('<key>')"""
         return self.get(key)
 
-    def __setattr__(self, key, value):
-        if key == '_config':
-            super().__setattr__(key, value)
-            return
-        if hasattr(type(self), key):
-            raise AttributeError('can not set generalConfig.%s' % key)
-        self._config[key] = value  # for test purposes
-
     @property
     def initialized(self):
         return bool(self._config)
 
+    def testinit(self, **kwds):
+        """for test purposes"""
+        self._config = kwds
+
 
 generalConfig = GeneralConfig()
 

test/test_datatypes.py

@@ -29,6 +29,7 @@ from secop.datatypes import ArrayOf, BLOBType, BoolType, \
     CommandType, ConfigError, DataType, Enum, EnumType, FloatRange, \
     IntRange, ProgrammingError, ScaledInteger, StatusType, \
     StringType, StructOf, TextType, TupleOf, get_datatype
+from secop.lib import generalConfig
 
 
 def copytest(dt):
@@ -36,6 +37,7 @@ def copytest(dt):
     assert dt.export_datatype() == dt.copy().export_datatype()
     assert dt != dt.copy()
 
+
 def test_DataType():
     dt = DataType()
     with pytest.raises(NotImplementedError):
@@ -116,7 +118,6 @@ def test_IntRange():
         dt('1.3')
     dt(1)
     dt(0)
-    dt('1')
     with pytest.raises(ProgrammingError):
         IntRange('xc', 'Yx')
 
@@ -132,6 +133,7 @@ def test_IntRange():
     with pytest.raises(ConfigError):
         dt.checkProperties()
 
+
 def test_ScaledInteger():
     dt = ScaledInteger(0.01, -3, 3)
     copytest(dt)
@@ -407,6 +409,7 @@ def test_ArrayOf():
     dt = ArrayOf(EnumType('myenum', single=0), 5)
     copytest(dt)
 
+
 def test_TupleOf():
     # test constructor catching illegal arguments
     with pytest.raises(ValueError):
@@ -641,6 +644,7 @@ def test_oneway_compatible(dt, contained_in):
     with pytest.raises(ValueError):
         contained_in.compatible(dt)
 
+
 @pytest.mark.parametrize('dt1, dt2', [
     (FloatRange(-5.5, 5.5), ScaledInteger(10, -5.5, 5.5)),
     (IntRange(0,1), BoolType()),
@@ -650,6 +654,7 @@ def test_twoway_compatible(dt1, dt2):
     dt1.compatible(dt1)
     dt2.compatible(dt2)
 
+
 @pytest.mark.parametrize('dt1, dt2', [
     (StringType(), FloatRange()),
     (IntRange(-10, 10), StringType()),
@@ -665,3 +670,12 @@ def test_incompatible(dt1, dt2):
         dt1.compatible(dt2)
     with pytest.raises(ValueError):
         dt2.compatible(dt1)
+
+
+@pytest.mark.parametrize('dt', [FloatRange(), IntRange(), ScaledInteger(1)])
+def test_lazy_validation(dt):
+    generalConfig.defaults['lazy_number_validation'] = True
+    dt('0')
+    generalConfig.defaults['lazy_number_validation'] = False
+    with pytest.raises(ValueError):
+        dt('0')

test/test_logging.py

@@ -112,8 +112,7 @@ def init_(monkeypatch):
                 def communicate(self, request):
                     self.comLog('> %s', request)
 
-            generalConfig.init()
+            generalConfig.testinit(logger_root='frappy', comlog=comlog)
-            generalConfig.comlog = comlog
             logger.init(console_level)
             self.srv = ServerStub()
 
@@ -139,7 +138,7 @@ def init_(monkeypatch):
 
     yield Playground
     # revert settings
-    generalConfig.__init__()
+    generalConfig.testinit()
     logger.__init__()
 
 

test/test_properties.py

@@ -38,10 +38,10 @@ V_test_Property = [
     [Prop(StringType(), 'default', extname='extname', mandatory=False),
      dict(default='default', extname='extname', export=True, mandatory=False)
      ],
-    [Prop(IntRange(), '42', export=True, name='custom', mandatory=True),
+    [Prop(IntRange(), 42, export=True, name='custom', mandatory=True),
      dict(default=42, extname='_custom', export=True, mandatory=True),
      ],
-    [Prop(IntRange(), '42', export=True, name='name'),
+    [Prop(IntRange(), 42, export=True, name='name'),
      dict(default=42, extname='_name', export=True, mandatory=False)
      ],
     [Prop(IntRange(), 42, '_extname', mandatory=True),
@@ -85,12 +85,12 @@ def test_Property_basic():
         Property('')
     with pytest.raises(ValueError):
         Property('', 1)
-    Property('', IntRange(), '42', 'extname', False, False)
+    Property('', IntRange(), 42, 'extname', False, False)
 
 
 def test_Properties():
     class Cls(HasProperties):
-        aa = Property('', IntRange(0, 99), '42', export=True)
+        aa = Property('', IntRange(0, 99), 42, export=True)
         bb = Property('', IntRange(), 0, export=False)
 
     assert Cls.aa.default == 42