Bug Fixed:

  Conflict resolution failed when state included persistent references
  (local or cross-database) with classes that couldn't be imported.

src/BTrees/tests/testConflict.py

@@ -600,31 +600,23 @@ class NastyConfict(Base, TestCase):
     # to decref a NULL pointer if conflict resolution was fed 3 empty
     # buckets.  http://collector.zope.org/Zope/553
     def testThreeEmptyBucketsNoSegfault(self):
-        self.openDB()
-
-        tm1 = transaction.TransactionManager()
-        r1 = self.db.open(transaction_manager=tm1).root()
-        self.assertEqual(len(self.t), 0)
-        r1["t"] = b = self.t  # an empty tree
-        tm1.commit()
-
-        tm2 = transaction.TransactionManager()
-        r2 = self.db.open(transaction_manager=tm2).root()
-        copy = r2["t"]
-        # Make sure all of copy is loaded.
-        list(copy.values())
-
-        # In one transaction, add and delete a key.
-        b[2] = 2
-        del b[2]
-        tm1.commit()
-
-        # In the other transaction, also add and delete a key.
-        b = copy
-        b[1] = 1
-        del b[1]
-        # If the commit() segfaults, the C code is still wrong for this case.
-        self.assertRaises(ConflictError, tm2.commit)
+        self.t[1] = 1
+        bucket = self.t._firstbucket
+        del self.t[1]
+        state1 = bucket.__getstate__()
+        state2 = bucket.__getstate__()
+        state3 = bucket.__getstate__()
+        self.assert_(state2 is not state1 and
+                     state2 is not state3 and
+                     state3 is not state1)
+        self.assert_(state2 == state1 and
+                     state3 == state1)
+        self.assertRaises(ConflictError, bucket._p_resolveConflict,
+                          state1, state2, state3)
+        # When an empty BTree resolves conflicts, it computes the
+        # bucket state as None, so...
+        self.assertRaises(ConflictError, bucket._p_resolveConflict,
+                          None, None, None)
 
     def testCantResolveBTreeConflict(self):
         # Test that a conflict involving two different changes to

src/CHANGES.txt

@@ -13,15 +13,30 @@ New Features
   ZODB.event.notify to provide your own event handling, although
   zope.event is recommended.
 
-Bugs Fixed
-----------
-
 - BTrees allowed object keys with insane comparison. (Comparison
   inherited from object, which compares based on in-process address.)
   Now BTrees raise TypeError if an attempt is made to save a key with
   comparison inherited from object. (This doesn't apply to old-style
   class instances.)
 
+3.10.5 (2011-11-19)
+===================
+
+Bugs Fixed
+----------
+
+- Conflict resolution failed when state included cross-database
+  persistent references with classes that couldn't be imported.
+
+3.10.4 (2011-11-17)
+===================
+
+Bugs Fixed
+----------
+
+- Conflict resolution failed when state included persistent references
+  with classes that couldn't be imported.
+
 3.10.3 (2011-04-12)
 ===================
 

src/ZODB/ConflictResolution.py

@@ -29,6 +29,14 @@ ResolvedSerial = 'rs'
 class BadClassName(Exception):
     pass
 
+class BadClass(object):
+
+    def __init__(self, *args):
+        self.args = args
+
+    def __reduce__(self):
+        raise BadClassName(*self.args)
+
 _class_cache = {}
 _class_cache_get = _class_cache.get
 def find_global(*args):
@@ -48,6 +56,12 @@ def find_global(*args):
 
     if cls == 1:
         # Not importable
+        if (isinstance(args, tuple) and len(args) == 2 and
+            isinstance(args[0], basestring) and
+            isinstance(args[1], basestring)
+            ):
+            return BadClass(*args)
+        else:
             raise BadClassName(*args)
     return cls
 
@@ -109,7 +123,13 @@ class PersistentReference(object):
         self.data = data
         # see serialize.py, ObjectReader._persistent_load
         if isinstance(data, tuple):
-            self.oid, self.klass = data
+            self.oid, klass = data
+            if isinstance(klass, BadClass):
+                # We can't use the BadClass directly because, if
+                # resolution succeeds, there's no good way to pickle
+                # it.  Fortunately, a class reference in a persistent
+                # reference is allowed to be a module+name tuple.
+                self.data = self.oid, klass.args
         elif isinstance(data, str):
             self.oid = data
         else: # a list
@@ -120,7 +140,10 @@ class PersistentReference(object):
             #    or persistent weakref: (oid, database_name)
             # else it is a weakref: reference_type
             if reference_type == 'm':
-                self.database_name, self.oid, self.klass = data[1]
+                self.database_name, self.oid, klass = data[1]
+                if isinstance(klass, BadClass):
+                    # see above wrt BadClass
+                    data[1] = self.database_name, self.oid, klass.args
             elif reference_type == 'n':
                 self.database_name, self.oid = data[1]
             elif reference_type == 'w':
@@ -153,6 +176,16 @@ class PersistentReference(object):
     def __getstate__(self):
         raise PicklingError("Can't pickle PersistentReference")
 
+
+    @property
+    def klass(self):
+        # for tests
+        data = self.data
+        if isinstance(data, tuple):
+            return data[1]
+        elif isinstance(data, list) and data[0] == 'm':
+            return data[1][2]
+
 class PersistentReferenceFactory:
 
     data = None
@@ -198,7 +231,6 @@ def tryToResolveConflict(self, oid, committedSerial, oldSerial, newpickle,
         if klass in _unresolvable:
             raise ConflictError
 
-        newstate = unpickler.load()
         inst = klass.__new__(klass, *newargs)
 
         try:
@@ -207,7 +239,20 @@ def tryToResolveConflict(self, oid, committedSerial, oldSerial, newpickle,
             _unresolvable[klass] = 1
             raise ConflictError
 
-        old = state(self, oid, oldSerial, prfactory)
+
+        oldData = self.loadSerial(oid, oldSerial)
+        if not committedData:
+            committedData  = self.loadSerial(oid, committedSerial)
+
+        if newpickle == oldData:
+            # old -> new diff is empty, so merge is trivial
+            return committedData
+        if committedData == oldData:
+            # old -> committed diff is empty, so merge is trivial
+            return newpickle
+
+        newstate = unpickler.load()
+        old       = state(self, oid, oldSerial, prfactory, oldData)
         committed = state(self, oid, committedSerial, prfactory, committedData)
 
         resolved = resolve(old, committed, newstate)

src/ZODB/tests/testconflictresolution.py

@@ -13,27 +13,296 @@
 ##############################################################################
 import manuel.doctest
 import manuel.footnote
+import doctest
 import manuel.capture
 import manuel.testing
+import persistent
+import transaction
+import unittest
 import ZODB.ConflictResolution
 import ZODB.tests.util
+import ZODB.POSException
 import zope.testing.module
 
 def setUp(test):
     ZODB.tests.util.setUp(test)
     zope.testing.module.setUp(test, 'ConflictResolution_txt')
+    ZODB.ConflictResolution._class_cache.clear()
+    ZODB.ConflictResolution._unresolvable.clear()
 
 def tearDown(test):
     zope.testing.module.tearDown(test)
     ZODB.tests.util.tearDown(test)
     ZODB.ConflictResolution._class_cache.clear()
+    ZODB.ConflictResolution._unresolvable.clear()
+
+
+class ResolveableWhenStateDoesNotChange(persistent.Persistent):
+
+    def _p_resolveConflict(old, committed, new):
+        raise ZODB.POSException.ConflictError
+
+class Unresolvable(persistent.Persistent):
+    pass
+
+def succeed_with_resolution_when_state_is_unchanged():
+    """
+    If a conflicting change doesn't change the state, then don't even
+    bother calling _p_resolveConflict
+
+    >>> db = ZODB.DB('t.fs') # FileStorage!
+    >>> storage = db.storage
+    >>> conn = db.open()
+    >>> conn.root.x = ResolveableWhenStateDoesNotChange()
+    >>> conn.root.x.v = 1
+    >>> transaction.commit()
+    >>> serial1 = conn.root.x._p_serial
+    >>> conn.root.x.v = 2
+    >>> transaction.commit()
+    >>> serial2 = conn.root.x._p_serial
+    >>> oid = conn.root.x._p_oid
+
+So, let's try resolving when the old and committed states are the same
+bit the new state (pickle) is different:
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial1, serial1, storage.loadSerial(oid, serial2))
+
+    >>> p == storage.loadSerial(oid, serial2)
+    True
+
+
+And when the old and new states are the same bit the committed state
+is different:
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial2, serial1, storage.loadSerial(oid, serial1))
+
+    >>> p == storage.loadSerial(oid, serial2)
+    True
+
+But we still conflict if both the committed and new are different than
+the original:
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial2, serial1, storage.loadSerial(oid, serial2))
+    ... # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ...
+    ConflictError: database conflict error (oid 0x01, ...
+
+
+Of course, none of this applies if content doesn't support conflict resolution.
+
+    >>> conn.root.y = Unresolvable()
+    >>> conn.root.y.v = 1
+    >>> transaction.commit()
+    >>> oid = conn.root.y._p_oid
+    >>> serial = conn.root.y._p_serial
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial, serial, storage.loadSerial(oid, serial))
+    ... # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ...
+    ConflictError: database conflict error (oid 0x02, ...
+
+    >>> db.close()
+    """
+
+class Resolveable(persistent.Persistent):
+
+    def _p_resolveConflict(self, old, committed, new):
+
+        resolved = {}
+        for k in old:
+            if k not in committed:
+                if k in new and new[k] == old[k]:
+                    continue
+                raise ZODB.POSException.ConflictError
+            if k not in new:
+                if k in committed and committed[k] == old[k]:
+                    continue
+                raise ZODB.POSException.ConflictError
+            if committed[k] != old[k]:
+                if new[k] == old[k]:
+                    resolved[k] = committed[k]
+                    continue
+                raise ZODB.POSException.ConflictError
+            if new[k] != old[k]:
+                if committed[k] == old[k]:
+                    resolved[k] = new[k]
+                    continue
+                raise ZODB.POSException.ConflictError
+            resolved[k] = old[k]
+
+        for k in new:
+            if k in old:
+                continue
+            if k in committed:
+                raise ZODB.POSException.ConflictError
+            resolved[k] = new[k]
+
+        for k in committed:
+            if k in old:
+                continue
+            if k in new:
+                raise ZODB.POSException.ConflictError
+            resolved[k] = committed[k]
+
+        return resolved
+
+def resolve_even_when_referenced_classes_are_absent():
+    """
+
+We often want to be able to resolve even when there are pesistent
+references to classes that can't be imported.
+
+    >>> class P(persistent.Persistent):
+    ...     pass
+
+    >>> db = ZODB.DB('t.fs') # FileStorage!
+    >>> storage = db.storage
+    >>> conn = db.open()
+    >>> conn.root.x = Resolveable()
+    >>> transaction.commit()
+    >>> oid = conn.root.x._p_oid
+    >>> serial = conn.root.x._p_serial
+
+    >>> conn.root.x.a = P()
+    >>> transaction.commit()
+    >>> aid = conn.root.x.a._p_oid
+    >>> serial1 = conn.root.x._p_serial
+
+    >>> del conn.root.x.a
+    >>> conn.root.x.b = P()
+    >>> transaction.commit()
+    >>> serial2 = conn.root.x._p_serial
+
+Bwahaha:
+
+    >>> P_aside = P
+    >>> del P
+
+Now, even though we can't import P, we can still resolve the conflict:
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial1, serial, storage.loadSerial(oid, serial2))
+
+And load the pickle:
+
+    >>> conn2 = db.open()
+    >>> P = P_aside
+    >>> p = conn2._reader.getState(p)
+    >>> sorted(p), p['a'] is conn2.get(aid), p['b'] is conn2.root.x.b
+    (['a', 'b'], True, True)
+
+    >>> isinstance(p['a'], P) and isinstance(p['b'], P)
+    True
+
+
+Oooooof course, this won't work if the subobjects aren't persistent:
+
+    >>> class NP:
+    ...     pass
+
+
+    >>> conn.root.x = Resolveable()
+    >>> transaction.commit()
+    >>> oid = conn.root.x._p_oid
+    >>> serial = conn.root.x._p_serial
+
+    >>> conn.root.x.a = a = NP()
+    >>> transaction.commit()
+    >>> serial1 = conn.root.x._p_serial
+
+    >>> del conn.root.x.a
+    >>> conn.root.x.b = b = NP()
+    >>> transaction.commit()
+    >>> serial2 = conn.root.x._p_serial
+
+Bwahaha:
+
+    >>> del NP
+
+
+    >>> storage.tryToResolveConflict(
+    ...         oid, serial1, serial, storage.loadSerial(oid, serial2))
+    ... # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ...
+    ConflictError: database conflict error (oid ...
+
+    >>> db.close()
+    """
+
+
+def resolve_even_when_xdb_referenced_classes_are_absent():
+    """Cross-database persistent refs!
+
+    >>> class P(persistent.Persistent):
+    ...     pass
+
+    >>> databases = {}
+    >>> db = ZODB.DB('t.fs', databases=databases, database_name='')
+    >>> db2 = ZODB.DB('o.fs', databases=databases, database_name='o')
+    >>> storage = db.storage
+    >>> conn = db.open()
+    >>> conn.root.x = Resolveable()
+    >>> transaction.commit()
+    >>> oid = conn.root.x._p_oid
+    >>> serial = conn.root.x._p_serial
+
+    >>> p = P(); conn.get_connection('o').add(p)
+    >>> conn.root.x.a = p
+    >>> transaction.commit()
+    >>> aid = conn.root.x.a._p_oid
+    >>> serial1 = conn.root.x._p_serial
+
+    >>> del conn.root.x.a
+    >>> p = P(); conn.get_connection('o').add(p)
+    >>> conn.root.x.b = p
+    >>> transaction.commit()
+    >>> serial2 = conn.root.x._p_serial
+
+    >>> del p
+
+Bwahaha:
+
+    >>> P_aside = P
+    >>> del P
+
+Now, even though we can't import P, we can still resolve the conflict:
+
+    >>> p = storage.tryToResolveConflict(
+    ...         oid, serial1, serial, storage.loadSerial(oid, serial2))
+
+And load the pickle:
+
+    >>> conn2 = db.open()
+    >>> conn2o = conn2.get_connection('o')
+    >>> P = P_aside
+    >>> p = conn2._reader.getState(p)
+    >>> sorted(p), p['a'] is conn2o.get(aid), p['b'] is conn2.root.x.b
+    (['a', 'b'], True, True)
+
+    >>> isinstance(p['a'], P) and isinstance(p['b'], P)
+    True
+
+    >>> db.close()
+    >>> db2.close()
+    """
 
 def test_suite():
-    return manuel.testing.TestSuite(
+    return unittest.TestSuite([
+        manuel.testing.TestSuite(
             manuel.doctest.Manuel()
             + manuel.footnote.Manuel()
             + manuel.capture.Manuel(),
             '../ConflictResolution.txt',
             setUp=setUp, tearDown=tearDown,
-        )
+            ),
+        doctest.DocTestSuite(
+            setUp=setUp, tearDown=tearDown),
+        ])