Repository: cwacek/python-jsonschema-objects

Issue:
Infinite recursion when deleting internal attribute
The recent fix of #165 introduced a new bug, unfortunately.
The function `ProtocolBase.__delattr__` calls `delattr` again, if the two conditions don't match. This causes an infinite recursion when trying to delete an internal attribute, or one that does not exist:
```
>>> test = namespace.Test()
>>> delattr(test, 'foo')
…
RecursionError: maximum recursion depth exceeded
```
Instead of calling `delattr`, the function should call `object.__delattr__` to avoid this problem (similar to what `__setattr__` does).

Discussion:

Patch:
diff --git a/python_jsonschema_objects/classbuilder.py b/python_jsonschema_objects/classbuilder.py
index b230977..94a8396 100644
--- a/python_jsonschema_objects/classbuilder.py
+++ b/python_jsonschema_objects/classbuilder.py
@@ -276,7 +276,7 @@ class ProtocolBase(collections.MutableMapping):
             prop.__delete__(self)
             return
 
-        return delattr(self, name)
+        return object.__delattr__(self, name)
 
     @classmethod
     def propinfo(cls, propname):


Test patch:
diff --git a/test/test_regression_165.py b/test/test_regression_165.py
index 0b5fd97..d785714 100644
--- a/test/test_regression_165.py
+++ b/test/test_regression_165.py
@@ -10,8 +10,11 @@ def testclass():
 
 
 def test_extra_properties_can_be_deleted_with_item_syntax(testclass):
-    testclass.foo = 42
+    # Deletion before setting should throw attribute errors
+    with pytest.raises(AttributeError):
+        del testclass["foo"]
 
+    testclass.foo = 42
     assert testclass.foo == 42
     del testclass["foo"]
 
@@ -21,8 +24,11 @@ def test_extra_properties_can_be_deleted_with_item_syntax(testclass):
 
 
 def test_extra_properties_can_be_deleted_with_attribute_syntax(testclass):
-    testclass.foo = 42
+    # Deletion before setting should throw attribute errors
+    with pytest.raises(AttributeError):
+        del testclass.foo
 
+    testclass.foo = 42
     assert testclass.foo == 42
     del testclass.foo
 
@@ -32,8 +38,11 @@ def test_extra_properties_can_be_deleted_with_attribute_syntax(testclass):
 
 
 def test_extra_properties_can_be_deleted_directly(testclass):
-    testclass.foo = 42
+    # Deletion before setting should throw attribute errors
+    with pytest.raises(AttributeError):
+        delattr(testclass, "foo")
 
+    testclass.foo = 42
     assert testclass.foo == 42
     delattr(testclass, "foo")
 
@@ -42,9 +51,37 @@ def test_extra_properties_can_be_deleted_directly(testclass):
         testclass.foo
 
 
-def test_real_properties_arent_really_deleted(Person):
+def test_unrequired_real_properties_arent_really_deleted(Person):
     p = Person(age=20)
 
     assert p.age == 20
     del p.age
     assert p.age == None
+
+    p.age = 20
+    assert p.age == 20
+    delattr(p, "age")
+    assert p.age == None
+
+    p.age = 20
+    assert p.age == 20
+    del p["age"]
+    assert p.age == None
+
+
+def test_required_real_properties_throw_attributeerror_on_delete(Person):
+    p = Person(firstName="Fred")
+
+    assert p.firstName == "Fred"
+    with pytest.raises(AttributeError):
+        del p.firstName
+
+    p.firstName = "Fred"
+    assert p.firstName == "Fred"
+    with pytest.raises(AttributeError):
+        delattr(p, "firstName")
+
+    p.firstName = "Fred"
+    assert p.firstName == "Fred"
+    with pytest.raises(AttributeError):
+        del p["firstName"]