[Zope3-checkins] CVS: ZODB4/src/zodb/btrees - check.py:1.3 TreeSetTemplate.c:1.5 SetTemplate.c:1.5 BucketTemplate.c:1.18 BTreeTemplate.c:1.16 BTreeModuleTemplate.c:1.8

Jeremy Hylton jeremy@zope.com
Tue, 20 May 2003 15:07:54 -0400


Update of /cvs-repository/ZODB4/src/zodb/btrees
In directory cvs.zope.org:/tmp/cvs-serv19336/zodb/btrees

Modified Files:
	check.py TreeSetTemplate.c SetTemplate.c BucketTemplate.c 
	BTreeTemplate.c BTreeModuleTemplate.c 
Log Message:
Refactor persistence api to use _p_changed only to mark an object as changed.

Use _p_deactivate() to turn an object into a ghost, and use the
keyword argument force=1 if you want to turn a modified object into a
ghost.  Several occurrences of the old interface have been updated.

This refactoring uncovered a number of subtle bugs in the persistence
C API.  The two chief problems were that the load function in the C
API struct did not set the state and that the functions return 0 for
error and 1 for success.  Regardless of whether these APIs are doing
the right thing, fix the code to use them correctly.

One downside of the new API is the C objects (BTrees) that override
_p_deactivate() have to deal with all the cruft for keyword
arguments.  Since BTrees only add a single line of extra code to
_p_deactivate(), it seems useful to provide a hook in the persistence
framework for this purpose.

Also:

If an object is in the changed state, don't call register() on its
data manager a second time.

Ignore state changes that result from setstate() being called.

Don't load an object's state to call __setstate__().

In BTrees check module, if an object has an oid, print that along with
its id().



=== ZODB4/src/zodb/btrees/check.py 1.2 => 1.3 ===
--- ZODB4/src/zodb/btrees/check.py:1.2	Tue Jan 14 17:32:27 2003
+++ ZODB4/src/zodb/btrees/check.py	Tue May 20 15:07:23 2003
@@ -201,8 +201,14 @@
         i += 1
     return keys, values
 
+from zodb.interfaces import _fmt_oid
+
 def type_and_adr(obj):
-    return "%s (0x%x)" % (type(obj).__name__, id(obj))
+    if obj._p_oid:
+        return "%s (0x%x oid=%s)" % (type(obj).__name__, id(obj),
+                                     _fmt_oid(obj._p_oid))
+    else:
+        return "%s (0x%x)" % (type(obj).__name__, id(obj))
 
 # Walker implements a depth-first search of a BTree (or TreeSet or Set or
 # Bucket).  Subclasses must implement the visit_btree() and visit_bucket()


=== ZODB4/src/zodb/btrees/TreeSetTemplate.c 1.4 => 1.5 ===
--- ZODB4/src/zodb/btrees/TreeSetTemplate.c:1.4	Fri Apr 25 15:02:34 2003
+++ ZODB4/src/zodb/btrees/TreeSetTemplate.c	Tue May 20 15:07:23 2003
@@ -17,12 +17,15 @@
 static PyObject *
 TreeSet_insert(BTree *self, PyObject *args)
 {
-  PyObject *key;
-  int i;
+    PyObject *key;
+    int i;
 
-  UNLESS (PyArg_ParseTuple(args, "O", &key)) return NULL;
-  if ((i=_BTree_set(self, key, Py_None, 1, 1)) < 0) return NULL;
-  return PyInt_FromLong(i);
+    if (!PyArg_ParseTuple(args, "O:insert", &key)) 
+	return NULL;
+    i = _BTree_set(self, key, Py_None, 1, 1);
+    if (i < 0) 
+	return NULL;
+    return PyInt_FromLong(i);
 }
 
 /* _Set_update and _TreeSet_update are identical except for the
@@ -161,7 +164,7 @@
   {"_p_resolveConflict", (PyCFunction) BTree__p_resolveConflict, METH_VARARGS,
    "_p_resolveConflict() -- Reinitialize from a newly created copy"},
 
-  {"_p_deactivate", (PyCFunction) BTree__p_deactivate,	METH_NOARGS,
+  {"_p_deactivate", (PyCFunction) BTree__p_deactivate,	METH_KEYWORDS,
    "_p_deactivate()\n\nReinitialize from a newly created copy."},
 #endif
   {NULL,		NULL}		/* sentinel */


=== ZODB4/src/zodb/btrees/SetTemplate.c 1.4 => 1.5 ===
--- ZODB4/src/zodb/btrees/SetTemplate.c:1.4	Fri Apr 25 15:02:34 2003
+++ ZODB4/src/zodb/btrees/SetTemplate.c	Tue May 20 15:07:23 2003
@@ -193,7 +193,7 @@
   {"_p_resolveConflict", (PyCFunction) bucket__p_resolveConflict, METH_VARARGS,
    "_p_resolveConflict() -- Reinitialize from a newly created copy"},
 
-  {"_p_deactivate", (PyCFunction) bucket__p_deactivate, METH_VARARGS,
+  {"_p_deactivate", (PyCFunction) bucket__p_deactivate, METH_KEYWORDS,
    "_p_deactivate() -- Reinitialize from a newly created copy"},
 #endif
 
@@ -231,8 +231,8 @@
   static PyObject *format;
   PyObject *r, *t;
 
-  UNLESS (format) UNLESS (format=PyString_FromString(MOD_NAME_PREFIX "Set(%s)"))
-    return NULL;
+  if (!format)
+      format = PyString_FromString(MOD_NAME_PREFIX "Set(%s)");
   UNLESS (t = PyTuple_New(1)) return NULL;
   UNLESS (r = bucket_keys(self, NULL, NULL)) goto err;
   PyTuple_SET_ITEM(t, 0, r);


=== ZODB4/src/zodb/btrees/BucketTemplate.c 1.17 => 1.18 ===
--- ZODB4/src/zodb/btrees/BucketTemplate.c:1.17	Fri Apr 25 15:02:34 2003
+++ ZODB4/src/zodb/btrees/BucketTemplate.c	Tue May 20 15:07:23 2003
@@ -353,7 +353,7 @@
             DECREF_VALUE(self->values[i]);
             COPY_VALUE(self->values[i], value);
             INCREF_VALUE(self->values[i]);
-            if (PER_CHANGED(self) >= 0)
+            if (PyPersist_CHANGED(self))
                 result = 0;
             goto Done;
         }
@@ -384,7 +384,7 @@
 
         if (changed)
             *changed = 1;
-        if (PER_CHANGED(self) >= 0)
+        if (PyPersist_CHANGED(self))
             result = 1;
         goto Done;
     }
@@ -420,7 +420,7 @@
     self->len++;
     if (changed)
         *changed = 1;
-    if (PER_CHANGED(self) >= 0)
+    if (PyPersist_CHANGED(self))
         result = 1;
 
 Done:
@@ -564,7 +564,7 @@
     Py_INCREF(next);
     self->next = next;
 
-    if (PER_CHANGED(self) < 0)
+    if (!PyPersist_CHANGED(self))
         return -1;
 
     return 0;
@@ -597,7 +597,7 @@
         Py_XINCREF(next);       /* it may be NULL, of course */
         self->next = next;
         Py_DECREF(successor);
-	if (PER_CHANGED(self) < 0)
+	if (!PyPersist_CHANGED(self))
 	    goto Done;
     }
     result = 0;
@@ -1039,14 +1039,47 @@
 
 #ifdef PERSISTENT
 static PyObject *
-bucket__p_deactivate(Bucket *self)
+bucket__p_deactivate(Bucket *self, PyObject *args, PyObject *keywords)
 {
-    if (self->po_state == UPTODATE && self->po_dm && self->po_oid) {
-	if (_bucket_clear(self) < 0)
+    int ghostify = 1;
+    PyObject *force = NULL;
+
+    if (args && PyTuple_GET_SIZE(args) > 0) {
+	PyErr_SetString(PyExc_TypeError, 
+			"_p_deactivate takes not positional arguments");
+	return NULL;
+    }
+    if (keywords) {
+	int size = PyDict_Size(keywords);
+	force = PyDict_GetItemString(keywords, "force");
+	if (force)
+	    size--;
+	if (size) {
+	    PyErr_SetString(PyExc_TypeError, 
+			    "_p_deactivate only accepts keyword arg force");
 	    return NULL;
-	self->po_state = GHOST;
+	}
     }
 
+    if (self->po_dm && self->po_oid) {
+	ghostify = self->po_state == UPTODATE;
+	if (!ghostify && force) {
+	    if (PyObject_IsTrue(force))
+		ghostify = 1;
+	    if (PyErr_Occurred())
+		return NULL;
+	}
+	if (ghostify) {
+	    PyObject **pdict = _PyObject_GetDictPtr((PyObject *)self);
+	    if (pdict && *pdict) {
+		Py_DECREF(*pdict);
+		*pdict = NULL;
+	    }
+	    if (_bucket_clear(self) < 0)
+		return NULL;
+	    self->po_state = GHOST;
+	}
+    }
     Py_INCREF(Py_None);
     return Py_None;
 }
@@ -1060,7 +1093,7 @@
   if (self->len) {
       if (_bucket_clear(self) < 0)
 	  return NULL;
-      if (PER_CHANGED(self) < 0)
+      if (!PyPersist_CHANGED(self))
 	  goto err;
   }
   PyPersist_DECREF(self);
@@ -1481,7 +1514,7 @@
      METH_VARARGS,
      "_p_resolveConflict() -- Reinitialize from a newly created copy"},
 
-    {"_p_deactivate", (PyCFunction) bucket__p_deactivate, METH_NOARGS,
+    {"_p_deactivate", (PyCFunction) bucket__p_deactivate, METH_KEYWORDS,
      "_p_deactivate() -- Reinitialize from a newly created copy"},
 #endif
     {NULL, NULL}


=== ZODB4/src/zodb/btrees/BTreeTemplate.c 1.15 => 1.16 ===
--- ZODB4/src/zodb/btrees/BTreeTemplate.c:1.15	Fri Apr 25 15:02:34 2003
+++ ZODB4/src/zodb/btrees/BTreeTemplate.c	Tue May 20 15:07:23 2003
@@ -293,7 +293,7 @@
 
     next->len = next_size;
     self->len = index;
-    return PER_CHANGED(self) < 0 ? -1 : 0;
+    return PyPersist_CHANGED(self) ? 0 : -1;
 }
 
 
@@ -407,6 +407,7 @@
 
       if (i < 0) {
           Py_DECREF(e);
+	  assert(PyErr_Occurred());
           return -1;
       }
 
@@ -791,13 +792,14 @@
 Done:
 #ifdef PERSISTENT
     if (changed) {
-        if (PyPersist_CHANGED(self) < 0) goto Error;
+        if (!PyPersist_CHANGED(self)) goto Error;
     }
 #endif
     PER_UNUSE(self);
     return status;
 
 Error:
+    assert(PyErr_Occurred());
     if (self_was_empty) {
         /* BTree_grow may have left the BTree in an invalid state.  Make
          * sure the tree is a legitimate empty tree.
@@ -830,12 +832,46 @@
 
 #ifdef PERSISTENT
 static PyObject *
-BTree__p_deactivate(BTree *self)
+BTree__p_deactivate(BTree *self, PyObject *args, PyObject *keywords)
 {
-    if (self->po_state == UPTODATE && self->po_dm && self->po_oid) {
-	if (_BTree_clear(self) < 0)
+    int ghostify = 1;
+    PyObject *force = NULL;
+
+    if (args && PyTuple_GET_SIZE(args) > 0) {
+	PyErr_SetString(PyExc_TypeError, 
+			"_p_deactivate takes not positional arguments");
+	return NULL;
+    }
+    if (keywords) {
+	int size = PyDict_Size(keywords);
+	force = PyDict_GetItemString(keywords, "force");
+	if (force)
+	    size--;
+	if (size) {
+	    PyErr_SetString(PyExc_TypeError, 
+			    "_p_deactivate only accepts keyword arg force");
 	    return NULL;
-	self->po_state = GHOST;
+	}
+    }
+
+    if (self->po_dm && self->po_oid) {
+	ghostify = self->po_state == UPTODATE;
+	if (!ghostify && force) {
+	    if (PyObject_IsTrue(force))
+		ghostify = 1;
+	    if (PyErr_Occurred())
+		return NULL;
+	}
+	if (ghostify) {
+	    PyObject **pdict = _PyObject_GetDictPtr((PyObject *)self);
+	    if (pdict && *pdict) {
+		Py_DECREF(*pdict);
+		*pdict = NULL;
+	    }
+	    if (_BTree_clear(self) < 0)
+		return NULL;
+	    self->po_state = GHOST;
+	}
     }
 
     Py_INCREF(Py_None);
@@ -854,7 +890,7 @@
     {
       if (_BTree_clear(self) < 0)
 	  goto err;
-      if (PyPersist_CHANGED(self) < 0)
+      if (!PyPersist_CHANGED(self))
 	  goto err;
     }
 
@@ -1878,7 +1914,7 @@
      METH_VARARGS,
      "_p_resolveConflict() -- Reinitialize from a newly created copy"},
 
-    {"_p_deactivate", (PyCFunction) BTree__p_deactivate,	METH_NOARGS,
+    {"_p_deactivate", (PyCFunction) BTree__p_deactivate,	METH_KEYWORDS,
      "_p_deactivate()\n\nReinitialize from a newly created copy."},
 #endif
     {NULL, NULL}


=== ZODB4/src/zodb/btrees/BTreeModuleTemplate.c 1.7 => 1.8 ===
--- ZODB4/src/zodb/btrees/BTreeModuleTemplate.c:1.7	Thu May  8 16:39:45 2003
+++ ZODB4/src/zodb/btrees/BTreeModuleTemplate.c	Tue May 20 15:07:23 2003
@@ -26,7 +26,6 @@
 #define PER_DEL(self)
 #define PER_USE(O) 1
 #define PER_ACCESSED(O) 1
-#define PER_CHANGED(O) 0
 #endif
 
 /* So sue me.  This pair gets used all over the place, so much so that it