From 80e0c6ee58ceeacac01a638ba175670c830f6875 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Sat, 11 Apr 2015 07:55:14 -0500 Subject: [PATCH] The python _p_activate does state management like the C version does: set to changed during jar.setstate to prevent objects that set attributes in their __setstate__ from being registered with the jar, and unconditionally mark the object as saved when finished. This fixes problems with BTrees and DB churn, among other things. Test this. --- persistent/persistence.py | 16 ++++++++++- persistent/tests/test_persistence.py | 41 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/persistent/persistence.py b/persistent/persistence.py index f7e671c..c8f5b79 100644 --- a/persistent/persistence.py +++ b/persistent/persistence.py @@ -345,7 +345,8 @@ def _p_activate(self): oga = _OGA before = oga(self, '_Persistent__flags') if before is None: # Only do this if we're a ghost - _OSA(self, '_Persistent__flags', 0) # up-to-date + # Begin by marking up-to-date in case we bail early + _OSA(self, '_Persistent__flags', 0) jar = oga(self, '_Persistent__jar') if jar is None: return @@ -353,11 +354,24 @@ def _p_activate(self): if oid is None: return + # If we're actually going to execute a set-state, + # mark as changed to prevent any recursive call + # (actually, our earlier check that we're a ghost should + # prevent this, but the C implementation sets it to changed + # while calling jar.setstate, and this is observable to clients). + # The main point of this is to prevent changes made during + # setstate from registering the object with the jar. + _OSA(self, '_Persistent__flags', CHANGED) try: jar.setstate(self) except: _OSA(self, '_Persistent__flags', before) raise + else: + # If we succeed, no matter what the implementation + # of setstate did, mark ourself as up-to-date. The + # C implementation unconditionally does this. + _OSA(self, '_Persistent__flags', 0) # up-to-date # In the C implementation, _p_invalidate winds up calling # _p_deactivate. There are ZODB tests that depend on this; diff --git a/persistent/tests/test_persistence.py b/persistent/tests/test_persistence.py index 2bfc2e1..cf88997 100644 --- a/persistent/tests/test_persistence.py +++ b/persistent/tests/test_persistence.py @@ -19,9 +19,18 @@ py_impl = getattr(platform, 'python_implementation', lambda: None) _is_pypy3 = py_impl() == 'PyPy' and sys.version_info[0] > 2 +#pylint: disable=R0904,W0212,E1101 class _Persistent_Base(object): + def _getTargetClass(self): + """Return the type of the persistent object to test""" + raise NotImplementedError() + + def _makeCache(self, jar): + """Return a new pickle cache""" + raise NotImplementedError() + def _makeOne(self, *args, **kw): return self._getTargetClass()(*args, **kw) @@ -31,11 +40,17 @@ def _makeJar(self): @implementer(IPersistentDataManager) class _Jar(object): + _cache = None + # Set this to a value to have our `setstate` + # pass it through to the object's __setstate__ + setstate_calls_object = None def __init__(self): self._loaded = [] self._registered = [] def setstate(self, obj): self._loaded.append(obj._p_oid) + if self.setstate_calls_object is not None: + obj.__setstate__(self.setstate_calls_object) def register(self, obj): self._registered.append(obj._p_oid) @@ -1073,6 +1088,32 @@ def test__p_activate_only_sets_state_once(self): inst._p_activate() self.assertEqual(list(jar._loaded), [OID]) + def test__p_activate_leaves_object_in_saved_even_if_object_mutated_self(self): + # If the object's __setstate__ set's attributes + # when called by p_activate, the state is still + # 'saved' when done. Furthemore, the object is not + # registered with the jar + + class WithSetstate(self._getTargetClass()): + state = None + def __setstate__(self, state): + self.state = state + + inst, jar, OID = self._makeOneWithJar(klass=WithSetstate) + inst._p_invalidate() # make it a ghost + self.assertEqual(inst._p_status, 'ghost') + + jar.setstate_calls_object = 42 + inst._p_activate() + # It get loaded + self.assertEqual(list(jar._loaded), [OID]) + # and __setstate__ got called to mutate the object + self.assertEqual(inst.state, 42) + # but it's still in the saved state + self.assertEqual(inst._p_status, 'saved') + # and it is not registered as changed by the jar + self.assertEqual(list(jar._registered), []) + def test__p_deactivate_from_unsaved(self): inst = self._makeOne() inst._p_deactivate()