New Param: changed kwargs class attribute to a tuple so the subclass interface is simpler

ipalib/parameter.py

@@ -21,6 +21,7 @@
 Parameter system for command plugins.
 """
 
+from types import NoneType
 from plugable import ReadOnly, lock, check_name
 from constants import NULLS, TYPE_ERROR, CALLABLE_ERROR
 
@@ -178,44 +179,46 @@ class DefaultFrom(ReadOnly):
 
 class Param(ReadOnly):
     """
-    Base class for all IPA types.
+    Base class for all parameters.
     """
 
-    __kwargs = dict(
+    kwargs = (
-        cli_name=(str, None),
+        ('cli_name', str, None),
-        doc=(str, ''),
+        ('doc', str, ''),
-        required=(bool, True),
+        ('required', bool, True),
-        multivalue=(bool, False),
+        ('multivalue', bool, False),
-        primary_key=(bool, False),
+        ('primary_key', bool, False),
-        normalizer=(callable, None),
+        ('normalizer', callable, None),
-        default=(None, None),
+        ('default_from', callable, None),
-        default_from=(callable, None),
+        ('flags', frozenset, frozenset()),
-        flags=(frozenset, frozenset()),
+
+        # The 'default' kwarg gets appended in Param.__init__():
+        # ('default', self.type, None),
     )
 
-    def __init__(self, name, kwargs, **override):
+    # This is a dummy type so that most of the functionality of Param can be
+    # unit tested directly without always creating a subclass; however, real
+    # (direct) subclasses should *always* override this class attribute:
+    type = NoneType  # This isn't very useful in the real world!
+
+    def __init__(self, name, **kw):
+        assert type(self.type) is type
         self.param_spec = name
-        self.__override = dict(override)
+        self.__kw = dict(kw)
-        if not ('required' in override or 'multivalue' in override):
+        if not ('required' in kw or 'multivalue' in kw):
             (name, kw_from_spec) = parse_param_spec(name)
-            override.update(kw_from_spec)
+            kw.update(kw_from_spec)
         self.name = check_name(name)
-        if 'cli_name' not in override:
+        if kw.get('cli_name', None) is None:
-            override['cli_name'] = self.name
+            kw['cli_name'] = self.name
-        df = override.get('default_from', None)
+        df = kw.get('default_from', None)
         if callable(df) and not isinstance(df, DefaultFrom):
-            override['default_from'] = DefaultFrom(df)
+            kw['default_from'] = DefaultFrom(df)
-        kwargs = dict(kwargs)
+        self.__clonekw = kw
-        assert set(self.__kwargs).intersection(kwargs) == set()
+        self.kwargs += (('default', self.type, None),)
-        kwargs.update(self.__kwargs)
+        for (key, kind, default) in self.kwargs:
-        for (key, (kind, default)) in kwargs.iteritems():
+            value = kw.get(key, default)
-            value = override.get(key, default)
+            if value is not None:
-            if value is None:
-                if kind is bool:
-                    raise TypeError(
-                        TYPE_ERROR % (key, bool, value, type(value))
-                    )
-            else:
                 if (
                     type(kind) is type and type(value) is not kind
                     or
@@ -275,7 +278,7 @@ class Param(ReadOnly):
         """
         Normalize a scalar value.
 
-        This method is called once for each value in multivalue.
+        This method is called once for each value in a multivalue.
         """
         if type(value) is not unicode:
             return value
@@ -308,8 +311,6 @@ class Param(ReadOnly):
         )
 
 
-
-
 class Bool(Param):
     """
 
@@ -333,15 +334,27 @@ class Bytes(Param):
 
     """
 
+    type = str
+
+    def __init__(self, name, **kw):
+        kwargs = dict(
+            minlength=(int, None),
+            maxlength=(int, None),
+            length=(int, None),
+            pattern=(str, None),
+        )
+
+
 
 class Str(Param):
     """
 
     """
 
-    def __init__(self, name, **overrides):
+    type = unicode
-        self.type = unicode
+
-        super(Str, self).__init__(name, {}, **overrides)
+    def __init__(self, name, **kw):
+        super(Str, self).__init__(name, **kw)
 
     def _convert_scalar(self, value, index=None):
         if type(value) in (self.type, int, float, bool):

tests/test_ipalib/test_parameter.py

@@ -92,6 +92,8 @@ def test_parse_param_spec():
     assert f('name?') == ('name', dict(required=False, multivalue=False))
     assert f('name*') == ('name', dict(required=False, multivalue=True))
     assert f('name+') == ('name', dict(required=True, multivalue=True))
+    # Make sure other "funny" endings are treated special:
+    assert f('name^') == ('name^', dict(required=True, multivalue=False))
 
 
 class test_Param(ClassChecker):
@@ -105,72 +107,67 @@ class test_Param(ClassChecker):
         Test the `ipalib.parameter.Param.__init__` method.
         """
         name = 'my_param'
-        o = self.cls(name, {})
+        o = self.cls(name)
+        assert o.name is name
         assert o.__islocked__() is True
 
-        # Test default values:
+        # Test default kwarg values:
-        assert o.name is name
         assert o.cli_name is name
         assert o.doc == ''
         assert o.required is True
         assert o.multivalue is False
         assert o.primary_key is False
         assert o.normalizer is None
-        assert o.default is None
+        #assert o.default is None
         assert o.default_from is None
         assert o.flags == frozenset()
 
         # Test that ValueError is raised when a kwarg from a subclass
         # conflicts with an attribute:
-        kwarg = dict(convert=(callable, None))
-        e = raises(ValueError, self.cls, name, kwarg)
-        assert str(e) == "kwarg 'convert' conflicts with attribute on Param"
         class Subclass(self.cls):
-            pass
+            kwargs = self.cls.kwargs + (
-        e = raises(ValueError, Subclass, name, kwarg)
+                ('convert', callable, None),
+            )
+        e = raises(ValueError, Subclass, name)
         assert str(e) == "kwarg 'convert' conflicts with attribute on Subclass"
 
         # Test type validation of keyword arguments:
-        kwargs = dict(
+        class Subclass(self.cls):
-            extra1=(bool, True),
+            kwargs = self.cls.kwargs + (
-            extra2=(str, 'Hello'),
+                ('extra1', bool, True),
-            extra3=((int, float), 42),
+                ('extra2', str, 'Hello'),
-            extra4=(callable, lambda whatever: whatever + 7),
+                ('extra3', (int, float), 42),
+                ('extra4', callable, lambda whatever: whatever + 7),
             )
-        # Note: we don't accept None if kind is bool:
+        o = Subclass('my_param')  # Test with no **kw:
-        e = raises(TypeError, self.cls, 'my_param', kwargs, extra1=None)
+        for (key, kind, default) in o.kwargs:
-        assert str(e) == TYPE_ERROR % ('extra1', bool, None, type(None))
-        for (key, (kind, default)) in kwargs.items():
-            o = self.cls('my_param', kwargs)
             # Test with a type invalid for all:
             value = object()
-            overrides = {key: value}
+            kw = {key: value}
-            e = raises(TypeError, self.cls, 'my_param', kwargs, **overrides)
+            e = raises(TypeError, Subclass, 'my_param', **kw)
             if kind is callable:
                 assert str(e) == CALLABLE_ERROR % (key, value, type(value))
             else:
                 assert str(e) == TYPE_ERROR % (key, kind, value, type(value))
-            if kind is bool:  # See note above
+
-                continue
             # Test with None:
-            overrides = {key: None}
+            kw = {key: None}
-            o = self.cls('my_param', kwargs, **overrides)
+            Subclass('my_param', **kw)
 
     def test_convert_scalar(self):
         """
         Test the `ipalib.parameter.Param._convert_scalar` method.
         """
-        o = self.cls('my_param', {})
+        o = self.cls('my_param')
         e = raises(NotImplementedError, o._convert_scalar, 'some value')
         assert str(e) == 'Param._convert_scalar()'
         class Subclass(self.cls):
             pass
-        o = Subclass('my_param', {})
+        o = Subclass('my_param')
         e = raises(NotImplementedError, o._convert_scalar, 'some value')
         assert str(e) == 'Subclass._convert_scalar()'
 
 
-
 class test_Str(ClassChecker):
     """
     Test the `ipalib.parameter.Str` class.