From 1c1b36985be5a4c49b9cfc808edcdfd288c6d0cc Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 18 Jul 2012 16:00:49 -0400 Subject: Update iteritems test case to actually test iteritems. This patch updates the jsonutils.to_primitive() test case for when an object has an iteritems() method. The previous implementation was mostly a copy of another test and didn't actually test calling iteritems() at all. Now it does. This is used by NovaBase in nova.db.sqlalchemy.models. Related to nova blueprint no-db-messaging. Change-Id: Ie1d71b859219392ab35b82dd3c7932b30e759c89 --- tests/unit/test_jsonutils.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/unit/test_jsonutils.py b/tests/unit/test_jsonutils.py index 4a18b77..fe25697 100644 --- a/tests/unit/test_jsonutils.py +++ b/tests/unit/test_jsonutils.py @@ -88,19 +88,12 @@ class ToPrimitiveTestCase(unittest.TestCase): self.data = dict(a=1, b=2, c=3).items() self.index = 0 - def __iter__(self): - return self - - def next(self): - if self.index == len(self.data): - raise StopIteration - self.index = self.index + 1 - return self.data[self.index - 1] + def iteritems(self): + return self.data x = IterItemsClass() - ordered = jsonutils.to_primitive(x) - ordered.sort() - self.assertEquals(ordered, [['a', 1], ['b', 2], ['c', 3]]) + p = jsonutils.to_primitive(x) + self.assertEquals(p, {'a': 1, 'b': 2, 'c': 3}) def test_instance(self): class MysteryClass(object): -- cgit From 2d6f84742a3e8ea51ebbfb82cbeacefe97e199d5 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 18 Jul 2012 16:15:52 -0400 Subject: Track to_primitive() depth after iteritems(). Change jsonutils.to_primitive() to increase the recursion depth counter when calling to_primitive() on the result of iteritems() from the current element. Previously, the only time the counter was increased was when converting the __dict__ from an object. The iteritems() case risks cycles, as well. I hit a problem with this when trying to call to_primitive on an instance of nova.db.sqlalchemy.models.Instance. An Instance includes a reference to InstanceInfoCache, which has a reference back to the Instance. Without this change, to_primitive() would raise an exception for an Instance due to excessive recursion. Related to nova blueprint no-db-messaging. Change-Id: Ifb878368d97e92ab6c361a4dd5f5ab2e68fc16e2 --- openstack/common/jsonutils.py | 2 +- tests/unit/test_jsonutils.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/openstack/common/jsonutils.py b/openstack/common/jsonutils.py index 6130a7f..e8e14c4 100644 --- a/openstack/common/jsonutils.py +++ b/openstack/common/jsonutils.py @@ -107,7 +107,7 @@ def to_primitive(value, convert_instances=False, level=0): elif hasattr(value, 'iteritems'): return to_primitive(dict(value.iteritems()), convert_instances=convert_instances, - level=level) + level=level + 1) elif hasattr(value, '__iter__'): return to_primitive(list(value), level) elif convert_instances and hasattr(value, '__dict__'): diff --git a/tests/unit/test_jsonutils.py b/tests/unit/test_jsonutils.py index fe25697..46b5b36 100644 --- a/tests/unit/test_jsonutils.py +++ b/tests/unit/test_jsonutils.py @@ -95,6 +95,24 @@ class ToPrimitiveTestCase(unittest.TestCase): p = jsonutils.to_primitive(x) self.assertEquals(p, {'a': 1, 'b': 2, 'c': 3}) + def test_iteritems_with_cycle(self): + class IterItemsClass(object): + def __init__(self): + self.data = dict(a=1, b=2, c=3) + self.index = 0 + + def iteritems(self): + return self.data.items() + + x = IterItemsClass() + x2 = IterItemsClass() + x.data['other'] = x2 + x2.data['other'] = x + + # If the cycle isn't caught, to_primitive() will eventually result in + # an exception due to excessive recursion depth. + p = jsonutils.to_primitive(x) + def test_instance(self): class MysteryClass(object): a = 10 -- cgit From 9e1bd9d9313a9f324c5b7b02232e8bd2fd12ea8a Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 18 Jul 2012 16:47:34 -0400 Subject: Add missing convert_instances arg. When calling jsonutils.to_primitive() recursively, the convert_instances argument should be passed along. This change fixes one place where it was not. Change-Id: I536e1ca05bb4e613fba71298797879587e8b4b00 --- openstack/common/jsonutils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openstack/common/jsonutils.py b/openstack/common/jsonutils.py index e8e14c4..f96e727 100644 --- a/openstack/common/jsonutils.py +++ b/openstack/common/jsonutils.py @@ -109,7 +109,9 @@ def to_primitive(value, convert_instances=False, level=0): convert_instances=convert_instances, level=level + 1) elif hasattr(value, '__iter__'): - return to_primitive(list(value), level) + return to_primitive(list(value), + convert_instances=convert_instances, + level=level) elif convert_instances and hasattr(value, '__dict__'): # Likely an instance of something. Watch for cycles. # Ignore class member vars. -- cgit