Fix tests to work correctly with conflict resolution.

Add new tests for conflict resolution.

The old blanket try-except for conflict resolution was hiding some
bugs caused by the pickles used in the test suite.  The conflict
resolution code imposes some restrictions on the format of pickles.
Basically, the conflict resolution requires that the storage API only
accept a pickles that constructed according to the ZODB rules.

XXX This new restriction sounds unfortunate, but it would require a
substantial change to conflict resolution to remove it.

The key changes to the test suite are to store only persistent objects
and to format the pickles using the standard ZODB format.  All tests
now use ZODB.tests.MinPO.MinPO instances for data.  The pickling is
down with zodb_pickle() and zodb_unpickle() defined in
StorageTestBase.

Add conflict resolution tests to testFileStorage.  (They pass.)

src/ZODB/tests/BasicStorage.py

@@ -7,8 +7,10 @@
 from ZODB.Transaction import Transaction
 from ZODB import POSException
 
+from ZODB.tests.MinPO import MinPO
+from ZODB.tests.StorageTestBase import zodb_unpickle
+
 ZERO = '\0'*8
-import pickle
 
 
 
@@ -70,16 +72,16 @@ class BasicStorage:
 
     def checkNonVersionStoreAndLoad(self):
         oid = self._storage.new_oid()
-        self._dostore(oid=oid, data=7)
+        self._dostore(oid=oid, data=MinPO(7))
         data, revid = self._storage.load(oid, '')
-        value = pickle.loads(data)
-        assert value == 7
+        value = zodb_unpickle(data)
+        assert value == MinPO(7)
         # Now do a bunch of updates to an object
         for i in range(13, 22):
-            revid = self._dostore(oid, revid=revid, data=i)
+            revid = self._dostore(oid, revid=revid, data=MinPO(i))
         # Now get the latest revision of the object
         data, revid = self._storage.load(oid, '')
-        assert pickle.loads(data) == 21
+        assert zodb_unpickle(data) == MinPO(21)
 
     def checkNonVersionModifiedInVersion(self):
         oid = self._storage.new_oid()
@@ -91,18 +93,18 @@ class BasicStorage:
         revid = ZERO
         revisions = {}
         for i in range(31, 38):
-            revid = self._dostore(oid, revid=revid, data=i)
-            revisions[revid] = i
+            revid = self._dostore(oid, revid=revid, data=MinPO(i))
+            revisions[revid] = MinPO(i)
         # Now make sure all the revisions have the correct value
         for revid, value in revisions.items():
             data = self._storage.loadSerial(oid, revid)
-            assert pickle.loads(data) == value
+            assert zodb_unpickle(data) == value
     
 
     def checkConflicts(self):
         oid = self._storage.new_oid()
-        revid1 = self._dostore(oid, data=11)
-        revid2 = self._dostore(oid, revid=revid1, data=12)
+        revid1 = self._dostore(oid, data=MinPO(11))
+        revid2 = self._dostore(oid, revid=revid1, data=MinPO(12))
         self.assertRaises(POSException.ConflictError,
                           self._dostore,
-                          oid, revid=revid1, data=13)
+                          oid, revid=revid1, data=MinPO(13))

src/ZODB/tests/ConflictResolution.py

+"""Tests for application-level conflict resolution."""
+
+from ZODB.Transaction import Transaction
+from ZODB.POSException import ConflictError
+from Persistence import Persistent
+
+from ZODB.tests.StorageTestBase import zodb_unpickle, zodb_pickle
+
+import sys
+import types
+from cPickle import Pickler, Unpickler
+from cStringIO import StringIO
+
+class PCounter(Persistent):
+
+    _value = 0
+
+    def __repr__(self):
+        return "<PCounter %d>" % self._value
+
+    def inc(self):
+        self._value = self._value + 1
+
+    def _p_resolveConflict(self, oldState, savedState, newState):
+        savedDiff = savedState['_value'] - oldState['_value']
+        newDiff = newState['_value'] - oldState['_value']
+
+        oldState['_value'] = oldState['_value'] + savedDiff + newDiff
+
+        return oldState
+
+    # XXX What if _p_resolveConflict _thinks_ it resolved the
+    # conflict, but did something wrong?
+
+class PCounter2(PCounter):
+
+    def _p_resolveConflict(self, oldState, savedState, newState):
+        raise ConflictError
+
+class PCounter3(PCounter):
+    def _p_resolveConflict(self, oldState, savedState, newState):
+        raise AttributeError, "no attribute"
+
+class ConflictResolvingStorage:
+
+    def checkResolve(self):
+        obj = PCounter()
+        obj.inc()
+
+        oid = self._storage.new_oid()
+
+        revid1 = self._dostoreNP(oid, data=zodb_pickle(obj))
+
+        obj.inc()
+        obj.inc()
+        # The effect of committing two transactions with the same
+        # pickle is to commit two different transactions relative to
+        # revid1 that add two to _value.  
+        revid2 = self._dostoreNP(oid, revid=revid1, data=zodb_pickle(obj))
+        revid3 = self._dostoreNP(oid, revid=revid1, data=zodb_pickle(obj))
+
+        data, serialno = self._storage.load(oid, '')
+        inst = zodb_unpickle(data)
+        self.assert_(inst._value == 5)
+
+    def checkUnresolvable(self):
+        obj = PCounter2()
+        obj.inc()
+
+        oid = self._storage.new_oid()
+
+        revid1 = self._dostoreNP(oid, data=zodb_pickle(obj))
+
+        obj.inc()
+        obj.inc()
+        # The effect of committing two transactions with the same
+        # pickle is to commit two different transactions relative to
+        # revid1 that add two to _value.  
+        revid2 = self._dostoreNP(oid, revid=revid1, data=zodb_pickle(obj))
+        self.assertRaises(ConflictError,
+                          self._dostoreNP,
+                          oid, revid=revid1, data=zodb_pickle(obj))
+
+    
+    def checkBuggyResolve(self):
+        obj = PCounter3()
+        obj.inc()
+
+        oid = self._storage.new_oid()
+
+        revid1 = self._dostoreNP(oid, data=zodb_pickle(obj))
+
+        obj.inc()
+        obj.inc()
+        # The effect of committing two transactions with the same
+        # pickle is to commit two different transactions relative to
+        # revid1 that add two to _value.  
+        revid2 = self._dostoreNP(oid, revid=revid1, data=zodb_pickle(obj))
+        self.assertRaises(AttributeError,
+                          self._dostoreNP,
+                          oid, revid=revid1, data=zodb_pickle(obj))
+
+    # XXX test conflict error raised during undo

src/ZODB/tests/StorageTestBase.py

@@ -4,11 +4,69 @@
 # store transaction for a single object revision.
 
 import pickle
+import string
+import sys
+import types
 import unittest
+from cPickle import Pickler, Unpickler
+from cStringIO import StringIO
+
 from ZODB.Transaction import Transaction
 
+from ZODB.tests.MinPO import MinPO
+
 ZERO = '\0'*8
 
+def zodb_pickle(obj):
+    f = StringIO()
+    p = Pickler(f, 1)
+    klass = obj.__class__
+    assert not hasattr(obj, '__getinitargs__'), "not ready for constructors"
+    args = None
+
+    mod = getattr(klass, '__module__', None)
+    if mod is not None:
+        klass = mod, klass.__name__
+
+    state = obj.__getstate__()
+
+    p.dump((klass, args))
+    p.dump(state)
+    return f.getvalue(1)
+
+def zodb_unpickle(data):
+    f = StringIO(data)
+    u = Unpickler(f)
+    klass_info = u.load()
+    if isinstance(klass_info, types.TupleType):
+        if isinstance(klass_info[0], types.TupleType):
+            modname, klassname = klass_info[0]
+            args = klass_info[1]
+        else:
+            modname, klassname = klass_info
+            args = None
+        if modname == "__main__":
+            ns = globals()
+        else:
+            mod = import_helper(modname)
+            ns = mod.__dict__
+        try:
+            klass = ns[klassname]
+        except KeyError:
+            print >> sys.stderr, "can't find %s in %s" % (klassname,
+                                                          repr(ns))
+        inst = klass()
+    else:
+        raise ValueError, "expected class info: %s" % repr(klass_info)
+    state = u.load()
+    inst.__setstate__(state)
+    return inst
+
+def import_helper(name):
+    mod = __import__(name)
+    for part in string.split(name, ".")[1:]:
+        mod = getattr(mod, part)
+    return mod
 
 
 class StorageTestBase(unittest.TestCase):
@@ -39,9 +97,12 @@ class StorageTestBase(unittest.TestCase):
         if revid is None:
             revid = ZERO
         if data is None:
-            data = 7
+            data = MinPO(7)
+        if type(data) == types.IntType:
+            data = MinPO(data)
         if not already_pickled:
-            data = pickle.dumps(data)
+##            data = pickle.dumps(data)
+            data = zodb_pickle(data)
         if version is None:
             version = ''
         # Begin the transaction

src/ZODB/tests/TransactionalUndoVersionStorage.py

 # Check interactions between transactionalUndo() and versions.  Any storage
 # that supports both transactionalUndo() and versions must pass these tests.
 
-import pickle
 from ZODB import POSException
-
+from ZODB.tests.MinPO import MinPO
+from ZODB.tests.StorageTestBase import zodb_unpickle
 
 
 class TransactionalUndoVersionStorage:
     def checkUndoInVersion(self):
         oid = self._storage.new_oid()
         version = 'one'
-        revid_a = self._dostore(oid, data=91)
-        revid_b = self._dostore(oid, revid=revid_a, data=92, version=version)
-        revid_c = self._dostore(oid, revid=revid_b, data=93, version=version)
+        revid_a = self._dostore(oid, data=MinPO(91))
+        revid_b = self._dostore(oid, revid=revid_a, data=MinPO(92), version=version)
+        revid_c = self._dostore(oid, revid=revid_b, data=MinPO(93), version=version)
         info=self._storage.undoInfo()
         tid=info[0]['id']
         self._storage.tpc_begin(self._transaction)
@@ -23,10 +23,10 @@ class TransactionalUndoVersionStorage:
         assert oids[0] == oid
         data, revid = self._storage.load(oid, '')
         assert revid == revid_a
-        assert pickle.loads(data) == 91
+        assert zodb_unpickle(data) == MinPO(91)
         data, revid = self._storage.load(oid, version)
         assert revid > revid_b and revid > revid_c
-        assert pickle.loads(data) == 92
+        assert zodb_unpickle(data) == MinPO(92)
         # Now commit the version...
         self._storage.tpc_begin(self._transaction)
         oids = self._storage.commitVersion(version, '', self._transaction)
@@ -40,9 +40,9 @@ class TransactionalUndoVersionStorage:
         #JF#                   self._storage.load,
         #JF#                   oid, version)
         data, revid = self._storage.load(oid, version)
-        assert pickle.loads(data) == 92
+        assert zodb_unpickle(data) == MinPO(92)
         data, revid = self._storage.load(oid, '')
-        assert pickle.loads(data) == 92
+        assert zodb_unpickle(data) == MinPO(92)
         # ...and undo the commit
         info=self._storage.undoInfo()
         tid=info[0]['id']
@@ -53,9 +53,9 @@ class TransactionalUndoVersionStorage:
         assert len(oids) == 1
         assert oids[0] == oid
         data, revid = self._storage.load(oid, version)
-        assert pickle.loads(data) == 92
+        assert zodb_unpickle(data) == MinPO(92)
         data, revid = self._storage.load(oid, '')
-        assert pickle.loads(data) == 91
+        assert zodb_unpickle(data) == MinPO(91)
         # Now abort the version
         self._storage.tpc_begin(self._transaction)
         oids = self._storage.abortVersion(version, self._transaction)
@@ -70,9 +70,9 @@ class TransactionalUndoVersionStorage:
         #JF#                   self._storage.load,
         #JF#                   oid, version)
         data, revid = self._storage.load(oid, version)
-        assert pickle.loads(data) == 91
+        assert zodb_unpickle(data) == MinPO(91)
         data, revid = self._storage.load(oid, '')
-        assert pickle.loads(data) == 91
+        assert zodb_unpickle(data) == MinPO(91)
         # Now undo the abort
         info=self._storage.undoInfo()
         tid=info[0]['id']
@@ -84,6 +84,6 @@ class TransactionalUndoVersionStorage:
         assert oids[0] == oid
         # And the object should be back in versions 'one' and ''
         data, revid = self._storage.load(oid, version)
-        assert pickle.loads(data) == 92
+        assert zodb_unpickle(data) == MinPO(92)
         data, revid = self._storage.load(oid, '')
-        assert pickle.loads(data) == 91
+        assert zodb_unpickle(data) == MinPO(91)

src/ZODB/tests/testFileStorage.py

@@ -7,6 +7,7 @@ import StorageTestBase, BasicStorage, TransactionalUndoStorage
 import VersionStorage, TransactionalUndoVersionStorage
 import PackableStorage
 import Synchronization
+import ConflictResolution
 
 class FileStorageTests(
     StorageTestBase.StorageTestBase,
@@ -16,6 +17,7 @@ class FileStorageTests(
     TransactionalUndoVersionStorage.TransactionalUndoVersionStorage,
     PackableStorage.PackableStorage,
     Synchronization.SynchronizedStorage,
+    ConflictResolution.ConflictResolvingStorage,
     ):
 
     def setUp(self):