[Zodb-checkins] SVN: ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/ Made handling of deactivation and invalidation more distict and
Jim Fulton
jim at zope.com
Wed Aug 26 04:05:32 EDT 2009
Log message for revision 103215:
Made handling of deactivation and invalidation more distict and
rational. In the distant past they were the same. They were still
much more mixed together than they should have been. For example,
deleting _p_changed didn't call _p_invalidate, but it should have.
Now deleting _p_changed calls _p_invalidate and setting it to None
calls _p_deactivate. The code for handling the attribute is much
simpler. Also, the code for _p_invalidate used to work by assigning
_p_changed, which was insane.
Undfortunately, the default implementation for _p_invalidate still
calls _p_deactivate. There are implementations that rely on this.
Changed:
U ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/cPersistence.c
U ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/interfaces.py
U ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/tests/persistent.txt
-=-
Modified: ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/cPersistence.c
===================================================================
--- ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/cPersistence.c 2009-08-26 08:04:28 UTC (rev 103214)
+++ ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/cPersistence.c 2009-08-26 08:05:31 UTC (rev 103215)
@@ -28,7 +28,7 @@
/* Strings initialized by init_strings() below. */
static PyObject *py_keys, *py_setstate, *py___dict__, *py_timeTime;
-static PyObject *py__p_changed, *py__p_deactivate;
+static PyObject *py__p_changed, *py__p_deactivate, *py__p_invalidate;
static PyObject *py___getattr__, *py___setattr__, *py___delattr__;
static PyObject *py___slotnames__, *copy_reg_slotnames, *__newobj__;
static PyObject *py___getnewargs__, *py___getstate__;
@@ -46,6 +46,7 @@
INIT_STRING(__dict__);
INIT_STRING(_p_changed);
INIT_STRING(_p_deactivate);
+ INIT_STRING(_p_invalidate);
INIT_STRING(__getattr__);
INIT_STRING(__setattr__);
INIT_STRING(__delattr__);
@@ -254,18 +255,20 @@
return Py_None;
}
-static int Per_set_changed(cPersistentObject *self, PyObject *v);
-
static PyObject *
Per__p_invalidate(cPersistentObject *self)
{
- signed char old_state = self->state;
-
- if (old_state != cPersistent_GHOST_STATE)
+ if (self->state != cPersistent_GHOST_STATE && self->jar)
{
- if (Per_set_changed(self, NULL) < 0)
- return NULL;
- ghostify(self);
+ PyObject *r;
+
+ self->state = cPersistent_UPTODATE_STATE;
+ r = PyObject_CallMethodObjArgs((PyObject *)self, py__p_deactivate,
+ NULL);
+ if (r != NULL)
+ /* Make sure we're a ghost, even if _p_deactivate is a pass */
+ ghostify(self);
+ return r;
}
Py_INCREF(Py_None);
return Py_None;
@@ -884,47 +887,32 @@
static int
Per_set_changed(cPersistentObject *self, PyObject *v)
{
- int deactivate = 0;
int true;
if (!v)
{
/* delattr is used to invalidate an object even if it has changed. */
- if (self->state != cPersistent_GHOST_STATE)
- self->state = cPersistent_UPTODATE_STATE;
- deactivate = 1;
+ if (self->state == cPersistent_GHOST_STATE || self->jar == NULL)
+ return 0; /* noop if already ghost or no jar */
+ v = PyObject_CallMethodObjArgs((PyObject *)self, py__p_invalidate, NULL);
+ if (! v)
+ return -1;
+ Py_DECREF(v);
+ return 0;
}
- else if (v == Py_None)
- deactivate = 1;
- if (deactivate)
+ if (v == Py_None)
{
- PyObject *res, *meth;
- meth = PyObject_GetAttr((PyObject *)self, py__p_deactivate);
- if (meth == NULL)
+ /* Setting to None deactivates */
+ if (self->state != cPersistent_UPTODATE_STATE || self->jar == NULL)
+ return 0; /* noop if not in saved state */
+ v = PyObject_CallMethodObjArgs((PyObject *)self, py__p_deactivate, NULL);
+ if (! v)
return -1;
- res = PyObject_CallObject(meth, NULL);
- if (res)
- Py_DECREF(res);
- else
- {
- /* an error occured in _p_deactivate().
-
- It's not clear what we should do here. The code is
- obviously ignoring the exception, but it shouldn't return
- 0 for a getattr and set an exception. The simplest change
- is to clear the exception, but that simply masks the
- error.
-
- This prints an error to stderr just like exceptions in
- __del__(). It would probably be better to log it but that
- would be painful from C.
- */
- PyErr_WriteUnraisable(meth);
- }
- Py_DECREF(meth);
+ Py_DECREF(v);
return 0;
}
+
/* !deactivate. If passed a true argument, mark self as changed (starting
* with ZODB 3.6, that includes activating the object if it's a ghost).
* If passed a false argument, and the object isn't a ghost, set the
Modified: ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/interfaces.py
===================================================================
--- ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/interfaces.py 2009-08-26 08:04:28 UTC (rev 103214)
+++ ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/interfaces.py 2009-08-26 08:05:31 UTC (rev 103215)
@@ -235,15 +235,25 @@
ghost state. It may not be possible to make some persistent
objects ghosts, and, for optimization reasons, the implementation
may choose to keep an object in the saved state.
+
+ This method must be a no-op if the object is not in the saved
+ state, which implies that _p_jar is not None.
"""
def _p_invalidate():
"""Invalidate the object.
Invalidate the object. This causes any data to be thrown
- away, even if the object is in the changed state. The object
- is moved to the ghost state; further accesses will cause
- object data to be reloaded.
+ away, even if the object is in the changed state.
+
+ An implementation of this method must either discard state
+ data and go to the ghost state, or load current data (by
+ calling _p_jat.setstate(self) to reload data and enter the
+ saved state.
+
+ This method must be a no-op if the object is already in the
+ ghost state or if _p_jar isn't set. It must not be ignored
+ otherwise.
"""
class IPersistentNoReadConflicts(IPersistent):
Modified: ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/tests/persistent.txt
===================================================================
--- ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/tests/persistent.txt 2009-08-26 08:04:28 UTC (rev 103214)
+++ ZODB/branches/jim-saner-invalidate-and-deactivate/src/persistent/tests/persistent.txt 2009-08-26 08:05:31 UTC (rev 103215)
@@ -480,3 +480,137 @@
... assert IPersistent.implementedBy(P)
... p = P()
... assert IPersistent.providedBy(p)
+
+Overridden _p_deactivate and _p_invalidate
+------------------------------------------
+
+These methods are distinct. The default implementations must not be
+implemented in terms of each other.
+
+ >>> class OverrideDeactivate(Persistent):
+ ... def __init__(self):
+ ... self.x = 1
+ ... def _p_deactivate(self):
+ ... if (self._p_jar is None or
+ ... self._p_changed is None or self._p_changed):
+ ... print 'ignore deactivate'
+ ... return
+ ... print 'deactivate'
+ ... Persistent._p_deactivate(self)
+
+ >>> o = OverrideDeactivate()
+ >>> o._p_deactivate()
+ ignore deactivate
+
+ >>> o._p_invalidate()
+ >>> o._p_changed
+ 0
+ >>> o.__dict__
+ {'x': 1}
+
+ >>> o._p_changed = None
+ >>> del o._p_changed
+ >>> o._p_changed
+ 0
+ >>> o.__dict__
+ {'x': 1}
+
+ >>> o._p_jar = dm
+ >>> o._p_deactivate()
+ deactivate
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideDeactivate()
+ >>> o._p_jar = dm
+ >>> o._p_changed = None
+ deactivate
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideDeactivate()
+ >>> o._p_jar = dm
+
+The following is suprizing:
+
+ >>> o._p_invalidate()
+ deactivate
+
+This is due to the fact that, historically, the default implementation
+of _p_invalidate called _p_deactivate. This isn't really quite right,
+but it's possible there are persistent implementations that rely on
+this, so we're not going to change it for now.
+
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideDeactivate()
+ >>> o._p_jar = dm
+ >>> del o._p_changed
+ deactivate
+
+ditto :)
+
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> class OverrideInvalidate(Persistent):
+ ... def __init__(self):
+ ... self.x = 1
+ ... def _p_invalidate(self):
+ ... if (self._p_jar is None or self._p_changed is None):
+ ... print 'ignore invalidate'
+ ... return
+ ... print 'invalidate'
+ ... Persistent._p_invalidate(self)
+
+ >>> o = OverrideInvalidate()
+ >>> o._p_deactivate()
+ >>> o._p_changed
+ 0
+ >>> o.__dict__
+ {'x': 1}
+
+ >>> o._p_invalidate()
+ ignore invalidate
+
+ >>> o._p_changed = None
+
+ >>> del o._p_changed
+ >>> o._p_changed
+ 0
+ >>> o.__dict__
+ {'x': 1}
+
+ >>> o._p_jar = dm
+ >>> o._p_deactivate()
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideInvalidate()
+ >>> o._p_jar = dm
+ >>> o._p_changed = None
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideInvalidate()
+ >>> o._p_jar = dm
+ >>> o._p_invalidate()
+ invalidate
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
+
+ >>> o = OverrideInvalidate()
+ >>> o._p_jar = dm
+ >>> del o._p_changed
+ invalidate
+ >>> o._p_changed
+ >>> o.__dict__
+ {}
More information about the Zodb-checkins
mailing list