Repository: cwacek/python-jsonschema-objects

Issue:
repeated validation of arrays and nested objects
I m dealing with big objects, some made of nested objects and arrays of objects.
Initialization of such objects can get veeery long, and it seems that many values are validated multiple times.
Each literal value is validated when set, each array re-validates its elements, each object re-validates its properties and nested objects. When initializing an object with a big nested dictionary, it can take a very long time, mostly spent in validation already done.

How could we change the validation mechanism to avoid this?

Constraints I see:
+ for the moment, some construction steps are done during validation (typed_elems are set during validation)

Ideas:
+ add a "validated/dirty" flag in literals to avoid to validate a value already validated
+ only validate dirty properties

Any other ideas? any side effects to consider?


Discussion:

Patch:
diff --git a/development.txt b/development.txt
index d0653b0..f4ede09 100644
--- a/development.txt
+++ b/development.txt
@@ -6,3 +6,4 @@ sphinx
 sphinx-autobuild
 recommonmark
 pytest
+pytest-mock
diff --git a/python_jsonschema_objects/classbuilder.py b/python_jsonschema_objects/classbuilder.py
index 75ad2eb..65dbfad 100644
--- a/python_jsonschema_objects/classbuilder.py
+++ b/python_jsonschema_objects/classbuilder.py
@@ -197,7 +197,7 @@ class ProtocolBase(collections.MutableMapping):
             # run its setter. We get it from the class definition and call
             # it directly. XXX Heinous.
             prop = getattr(self.__class__, self.__prop_names__[name])
-            prop.fset(self, val)
+            prop.__set__(self, val)
         else:
             # This is an additional property of some kind
             try:
@@ -629,12 +629,15 @@ class ClassBuilder(object):
                             self.resolver.resolution_scope,
                             detail['items']['$ref'])
                         typ = self.construct(uri, detail['items'])
+                        constraints = copy.copy(detail)
+                        constraints['strict'] = kw.get('strict')
                         propdata = {
                             'type': 'array',
                             'validator': python_jsonschema_objects.wrapper_types.ArrayWrapper.create(
                                 uri,
                                 item_constraint=typ,
-                                **detail)}
+                                **constraints)}
+
                     else:
                         uri = "{0}/{1}_{2}".format(nm,
                                                    prop, "<anonymous_field>")
@@ -652,15 +655,24 @@ class ClassBuilder(object):
                                     )
                             else:
                                 typ = self.construct(uri, detail['items'])
+
+                            constraints = copy.copy(detail)
+                            constraints['strict'] = kw.get('strict')
                             propdata = {'type': 'array',
-                                        'validator': python_jsonschema_objects.wrapper_types.ArrayWrapper.create(uri, item_constraint=typ,
-                                                                                                                 **detail)}
+                                        'validator': python_jsonschema_objects.wrapper_types.ArrayWrapper.create(
+                                            uri,
+                                            item_constraint=typ,
+                                            **constraints)}
+
                         except NotImplementedError:
                             typ = detail['items']
+                            constraints = copy.copy(detail)
+                            constraints['strict'] = kw.get('strict')
                             propdata = {'type': 'array',
-                                        'validator': python_jsonschema_objects.wrapper_types.ArrayWrapper.create(uri,
-                                                                                                                 item_constraint=typ,
-                                                                                                                 **detail)}
+                                        'validator': python_jsonschema_objects.wrapper_types.ArrayWrapper.create(
+                                            uri,
+                                            item_constraint=typ,
+                                            **constraints)}
 
                     props[prop] = make_property(prop,
                                                 propdata,
@@ -725,125 +737,7 @@ class ClassBuilder(object):
 
 
 def make_property(prop, info, desc=""):
+    from . import descriptors
 
-    def getprop(self):
-        try:
-            return self._properties[prop]
-        except KeyError:
-            raise AttributeError("No such attribute")
-
-    def setprop(self, val):
-        if isinstance(info['type'], (list, tuple)):
-            ok = False
-            errors = []
-            type_checks = []
-
-            for typ in info['type']:
-              if not isinstance(typ, dict):
-                type_checks.append(typ)
-                continue
-              typ = next(t
-                         for n, t in validators.SCHEMA_TYPE_MAPPING
-                         if typ['type'] == n)
-              if typ is None:
-                  typ = type(None)
-              if isinstance(typ, (list, tuple)):
-                  type_checks.extend(typ)
-              else:
-                  type_checks.append(typ)
-
-            for typ in type_checks:
-                if isinstance(val, typ):
-                    ok = True
-                    break
-                elif hasattr(typ, 'isLiteralClass'):
-                    try:
-                        validator = typ(val)
-                    except Exception as e:
-                        errors.append(
-                            "Failed to coerce to '{0}': {1}".format(typ, e))
-                        pass
-                    else:
-                        validator.validate()
-                        ok = True
-                        break
-                elif util.safe_issubclass(typ, ProtocolBase):
-                    # force conversion- thus the val rather than validator assignment
-                    try:
-                        val = typ(**util.coerce_for_expansion(val))
-                    except Exception as e:
-                        errors.append(
-                            "Failed to coerce to '{0}': {1}".format(typ, e))
-                        pass
-                    else:
-                        val.validate()
-                        ok = True
-                        break
-                elif util.safe_issubclass(typ, python_jsonschema_objects.wrapper_types.ArrayWrapper):
-                    try:
-                        val = typ(val)
-                    except Exception as e:
-                        errors.append(
-                            "Failed to coerce to '{0}': {1}".format(typ, e))
-                        pass
-                    else:
-                        val.validate()
-                        ok = True
-                        break
-
-            if not ok:
-                errstr = "\n".join(errors)
-                raise validators.ValidationError(
-                    "Object must be one of {0}: \n{1}".format(info['type'], errstr))
-
-        elif info['type'] == 'array':
-            val = info['validator'](val)
-            val.validate()
-
-        elif util.safe_issubclass(info['type'],
-                                  python_jsonschema_objects.wrapper_types.ArrayWrapper):
-            # An array type may have already been converted into an ArrayValidator
-            val = info['type'](val)
-            val.validate()
-
-        elif getattr(info['type'], 'isLiteralClass', False) is True:
-            if not isinstance(val, info['type']):
-                validator = info['type'](val)
-                validator.validate()
-                if validator._value is not None:
-                    # This allows setting of default Literal values
-                    val = validator
-
-        elif util.safe_issubclass(info['type'], ProtocolBase):
-            if not isinstance(val, info['type']):
-                val = info['type'](**util.coerce_for_expansion(val))
-
-            val.validate()
-
-        elif isinstance(info['type'], TypeProxy):
-            val = info['type'](val)
-
-        elif isinstance(info['type'], TypeRef):
-            if not isinstance(val, info['type'].ref_class):
-                val = info['type'](**val)
-
-            val.validate()
-
-        elif info['type'] is None:
-            # This is the null value
-            if val is not None:
-                raise validators.ValidationError(
-                    "None is only valid value for null")
-
-        else:
-            raise TypeError("Unknown object type: '{0}'".format(info['type']))
-
-        self._properties[prop] = val
-
-    def delprop(self):
-        if prop in self.__required__:
-            raise AttributeError("'%s' is required" % prop)
-        else:
-            del self._properties[prop]
-
-    return property(getprop, setprop, delprop, desc)
+    prop = descriptors.AttributeDescriptor(prop, info, desc)
+    return prop
diff --git a/python_jsonschema_objects/descriptors.py b/python_jsonschema_objects/descriptors.py
new file mode 100644
index 0000000..a53bcbc
--- /dev/null
+++ b/python_jsonschema_objects/descriptors.py
@@ -0,0 +1,135 @@
+from . import validators, util, wrapper_types
+from .classbuilder import ProtocolBase, TypeProxy, TypeRef
+
+
+class AttributeDescriptor(object):
+    """ Provides property access for constructed class properties """
+
+    def __init__(self, prop, info, desc=""):
+        self.prop = prop
+        self.info = info
+        self.desc = desc
+
+    def __doc__(self):
+        return self.desc
+
+    def __get__(self, obj, owner=None):
+        if obj is None and owner is not None:
+            return self
+
+        try:
+            return obj._properties[self.prop]
+        except KeyError:
+            raise AttributeError("No such attribute")
+
+    def __set__(self, obj, val):
+        info = self.info
+        if isinstance(info["type"], (list, tuple)):
+            ok = False
+            errors = []
+            type_checks = []
+
+            for typ in info["type"]:
+                if not isinstance(typ, dict):
+                    type_checks.append(typ)
+                    continue
+                typ = next(
+                    t for n, t in validators.SCHEMA_TYPE_MAPPING if typ["type"] == n
+                )
+                if typ is None:
+                    typ = type(None)
+                if isinstance(typ, (list, tuple)):
+                    type_checks.extend(typ)
+                else:
+                    type_checks.append(typ)
+
+            for typ in type_checks:
+                if isinstance(val, typ):
+                    ok = True
+                    break
+                elif hasattr(typ, "isLiteralClass"):
+                    try:
+                        validator = typ(val)
+                    except Exception as e:
+                        errors.append("Failed to coerce to '{0}': {1}".format(typ, e))
+                        pass
+                    else:
+                        validator.validate()
+                        ok = True
+                        break
+                elif util.safe_issubclass(typ, ProtocolBase):
+                    # force conversion- thus the val rather than validator assignment
+                    try:
+                        val = typ(**util.coerce_for_expansion(val))
+                    except Exception as e:
+                        errors.append("Failed to coerce to '{0}': {1}".format(typ, e))
+                        pass
+                    else:
+                        val.validate()
+                        ok = True
+                        break
+                elif util.safe_issubclass(typ, wrapper_types.ArrayWrapper):
+                    try:
+                        val = typ(val)
+                    except Exception as e:
+                        errors.append("Failed to coerce to '{0}': {1}".format(typ, e))
+                        pass
+                    else:
+                        val.validate()
+                        ok = True
+                        break
+
+            if not ok:
+                errstr = "\n".join(errors)
+                raise validators.ValidationError(
+                    "Object must be one of {0}: \n{1}".format(info["type"], errstr)
+                )
+
+        elif info["type"] == "array":
+            val = info["validator"](val)
+            val.validate()
+
+        elif util.safe_issubclass(info["type"], wrapper_types.ArrayWrapper):
+            # An array type may have already been converted into an ArrayValidator
+            val = info["type"](val)
+            val.validate()
+
+        elif getattr(info["type"], "isLiteralClass", False) is True:
+            if not isinstance(val, info["type"]):
+                validator = info["type"](val)
+                validator.validate()
+                if validator._value is not None:
+                    # This allows setting of default Literal values
+                    val = validator
+
+        elif util.safe_issubclass(info["type"], ProtocolBase):
+            if not isinstance(val, info["type"]):
+                val = info["type"](**util.coerce_for_expansion(val))
+
+            val.validate()
+
+        elif isinstance(info["type"], TypeProxy):
+            val = info["type"](val)
+
+        elif isinstance(info["type"], TypeRef):
+            if not isinstance(val, info["type"].ref_class):
+                val = info["type"](**val)
+
+            val.validate()
+
+        elif info["type"] is None:
+            # This is the null value
+            if val is not None:
+                raise validators.ValidationError("None is only valid value for null")
+
+        else:
+            raise TypeError("Unknown object type: '{0}'".format(info["type"]))
+
+        obj._properties[self.prop] = val
+
+    def __delete__(self, obj):
+        prop = self.prop
+        if prop in obj.__required__:
+            raise AttributeError("'%s' is required" % prop)
+        else:
+            del obj._properties[prop]
diff --git a/python_jsonschema_objects/wrapper_types.py b/python_jsonschema_objects/wrapper_types.py
index 4f5aafa..c733700 100644
--- a/python_jsonschema_objects/wrapper_types.py
+++ b/python_jsonschema_objects/wrapper_types.py
@@ -5,6 +5,7 @@ import six
 
 from python_jsonschema_objects import util
 from python_jsonschema_objects.validators import registry, ValidationError
+from python_jsonschema_objects.util import lazy_format as fmt
 
 logger = logging.getLogger(__name__)
 
@@ -16,20 +17,30 @@ class ArrayWrapper(collections.MutableSequence):
     with a dirty-tracking mechanism to avoid constant validation costs.
     """
 
+    @property
+    def strict(self):
+        return getattr(self, '_strict_', False)
+
     def __len__(self):
         return len(self.data)
 
+    def mark_or_revalidate(self):
+        if self.strict:
+            self.validate()
+        else:
+            self._dirty = True
+
     def __delitem__(self, index):
-        self.data.remove(index)
-        self._dirty = True
+        self.data.pop(index)
+        self.mark_or_revalidate()
 
     def insert(self, index, value):
         self.data.insert(index, value)
-        self._dirty = True
+        self.mark_or_revalidate()
 
     def __setitem__(self, index, value):
         self.data[index] = value
-        self._dirty = True
+        self.mark_or_revalidate()
 
     def __getitem__(self, idx):
         return self.typed_elems[idx]
@@ -41,23 +52,33 @@ class ArrayWrapper(collections.MutableSequence):
             return self.for_json() == other
 
     def __init__(self, ary):
+        """ Initialize a wrapper for the array
+
+        Args:
+            ary: (list-like, or ArrayWrapper)
+        """
+
+        """ Marks whether or not the underlying array has been modified """
+        self._dirty = True
+
+        """ Holds a typed copy of the array """
+        self._typed = None
+
         if isinstance(ary, (list, tuple, collections.Sequence)):
             self.data = ary
-            self._dirty = True
-            self._typed = None
         elif isinstance(ary, ArrayWrapper):
             self.data = ary.data
-            self._dirty = True
-            self._typed = None
         else:
             raise TypeError("Invalid value given to array validator: {0}"
                             .format(ary))
 
+        logger.debug(fmt("Initializing ArrayWrapper {} with {}", self, ary))
+
     @property
     def typed_elems(self):
+        logger.debug(fmt("Accessing typed_elems of ArrayWrapper {} ", self))
         if self._typed is None or self._dirty is True:
-            self._typed = self.validate_items()
-            self._dirty = False
+            self.validate()
 
         return self._typed
 
@@ -77,9 +98,8 @@ class ArrayWrapper(collections.MutableSequence):
         return obj
 
     def serialize(self):
-        d = self.validate_items()
         enc = util.ProtocolJSONEncoder()
-        return enc.encode(d)
+        return enc.encode(self.typed_elems)
 
     def for_json(self):
         from python_jsonschema_objects import classbuilder
@@ -97,13 +117,13 @@ class ArrayWrapper(collections.MutableSequence):
         return out
 
     def validate(self):
-        self.validate_items()
-        self.validate_length()
-        self.validate_uniqueness()
+        if self.strict or self._dirty:
+            self.validate_items()
+            self.validate_length()
+            self.validate_uniqueness()
         return True
 
     def validate_uniqueness(self):
-        from python_jsonschema_objects import classbuilder
 
         if getattr(self, 'uniqueItems', None) is not None:
             testset = set(self.data)
@@ -113,7 +133,6 @@ class ArrayWrapper(collections.MutableSequence):
                     .format(self.data))
 
     def validate_length(self):
-        from python_jsonschema_objects import classbuilder
 
         if getattr(self, 'minItems', None) is not None:
             if len(self.data) < self.minItems:
@@ -128,6 +147,15 @@ class ArrayWrapper(collections.MutableSequence):
                     .format(self.maxItems, self.data))
 
     def validate_items(self):
+        """ Validates the items in the backing array, including
+        performing type validation.
+
+        Sets the _typed property and clears the dirty flag as a side effect
+
+        Returns:
+            The typed array
+        """
+        logger.debug(fmt("Validating {}", self))
         from python_jsonschema_objects import classbuilder
 
         if self.__itemtype__ is None:
@@ -188,6 +216,8 @@ class ArrayWrapper(collections.MutableSequence):
                     val.validate()
                     typed_elems.append(val)
 
+        self._dirty = False
+        self._typed = typed_elems
         return typed_elems
 
     @staticmethod
@@ -202,6 +232,7 @@ class ArrayWrapper(collections.MutableSequence):
         addl_constraints is expected to be key-value pairs of any of the other
         constraints permitted by JSON Schema v4.
         """
+        logger.debug(fmt("Constructing ArrayValidator with {} and {}", item_constraint, addl_constraints))
         from python_jsonschema_objects import classbuilder
         klassbuilder = addl_constraints.pop("classbuilder", None)
         props = {}
@@ -283,6 +314,8 @@ class ArrayWrapper(collections.MutableSequence):
 
         props['__itemtype__'] = item_constraint
 
+        strict = addl_constraints.pop('strict', False)
+        props['_strict_'] = strict
         props.update(addl_constraints)
 
         validator = type(str(name), (ArrayWrapper,), props)
diff --git a/tox.ini b/tox.ini
index 4a88451..dd08d32 100644
--- a/tox.ini
+++ b/tox.ini
@@ -10,4 +10,5 @@ deps =
   .
   coverage
   pytest
+  pytest-mock
 


Test patch:
diff --git a/test/test_regression_143.py b/test/test_regression_143.py
new file mode 100644
index 0000000..6822a9a
--- /dev/null
+++ b/test/test_regression_143.py
@@ -0,0 +1,82 @@
+import python_jsonschema_objects as pjs
+import python_jsonschema_objects.wrapper_types
+
+
+def test_limited_validation(mocker):
+    schema = {
+        "title": "Example Schema",
+        "type": "object",
+        "properties": {
+            "a": {"type": "array", "items": {"type": "string"}, "default": []}
+        },
+    }
+
+    ob = pjs.ObjectBuilder(schema)
+    ns1 = ob.build_classes()
+    validator = ns1.ExampleSchema.a.info["validator"]([])
+    validate_items = mocker.patch.object(
+        validator, "validate_items", side_effect=validator.validate_items
+    )
+    validate = mocker.patch.object(
+        validator, "validate", side_effect=validator.validate
+    )
+    mocker.patch.dict(
+        ns1.ExampleSchema.a.info,
+        {"validator": mocker.MagicMock(return_value=validator)},
+    )
+
+    foo = ns1.ExampleSchema()
+    # We expect validation to be called on creation
+    assert validate_items.call_count == 1
+
+    # We expect manipulation to not revalidate immediately without strict
+    foo.a.append("foo")
+    foo.a.append("bar")
+    assert validate_items.call_count == 1
+
+    # We expect accessing data elements to cause a revalidate, but only the first time
+    print(foo.a[0])
+    assert validate_items.call_count == 2
+
+    print(foo.a)
+    assert foo.a == ["foo", "bar"]
+    assert validate_items.call_count == 2
+
+
+def test_strict_validation(mocker):
+    """ Validate that when specified as strict, validation still occurs on every change"""
+    schema = {
+        "title": "Example Schema",
+        "type": "object",
+        "properties": {
+            "a": {"type": "array", "items": {"type": "string"}, "default": []}
+        },
+    }
+
+    ob = pjs.ObjectBuilder(schema)
+    ns1 = ob.build_classes(strict=True)
+    validator = ns1.ExampleSchema.a.info["validator"]([])
+    validate_items = mocker.patch.object(
+        validator, "validate_items", side_effect=validator.validate_items
+    )
+    validate = mocker.patch.object(
+        validator, "validate", side_effect=validator.validate
+    )
+    mocker.patch.dict(
+        ns1.ExampleSchema.a.info,
+        {"validator": mocker.MagicMock(return_value=validator)},
+    )
+
+    foo = ns1.ExampleSchema()
+    # We expect validation to be called on creation
+    assert validate_items.call_count == 1
+
+    # We expect manipulation to revalidate immediately with strict
+    foo.a.append("foo")
+    foo.a.append("bar")
+    assert validate_items.call_count == 3
+
+    # We expect accessing data elements to not revalidate because strict would have revalidated on load
+    print(foo.a[0])
+    print(foo.a)
+    assert foo.a == ["foo", "bar"]