Gary Poster wrote:
I object. :-)
Thanks for the feedback!
First, your change effectively adds _database_name to an unspoken interface for persistent IKeyReferences. That's not a good idea.
Yup, I did not respect that point.
Second, the error you are getting is an error condition and should be raised as one. Maybe a more helpful error would be an improvement. But your __cmp__ is somehow comparing against a persistent keyreference with an object that appears to have not yet been added to a connection (_p_jar)--something that should not have been able to happen with the default persistent adapter to keyreference.
Yes, that was my opinion too. But the error happen anyhow...
Are you working with a custom keyreference adaptor anywhere?
In any case, shuffling over this problem as your change does would likely cause incorrect sorting/indexing in the intids btree, which would be very bad. This is an error, and should be dealt with as one.
I tried this approach before. But how can I remove an incorrect sorting/indexing respecting the dict api without invoking the compare function?

class IntIds(Persistent, Contained):
   def register(self, ob):
           if key in self.ids: # <- causing the comparision error
                return self.ids[key]
      except SpecialCmpError:
                # clean up incorrect sorting/indexing
                uid = self.ids[key] # <- causing the comparision error too
                del self.refs[uid]
                del self.ids[key]

The error occurs after a savepoint.rollback. I can reproduce the error within the basic error story (-> see testcase). It happens if we compare a commited persistent object with a rollbacked persistent object.

The current implementation hurts the demeter's law. A fresh instance does not support the db attribute, even though the situation showed within the basic error story

   >>> from persistent import Persistent
   >>> p = Persistent()
   >>> p._p_jar.db().database_name
   Traceback (most recent call last):
   AttributeError: 'NoneType' object has no attribute 'db'

IMO the error should still be handled by KeyReferenceToPersistent __cmp__ method. What do you think about the following implementation respecting your first argument (unspoken interface extension):

class KeyReferenceToPersistent(object):
   def __cmp__(self, other):
       if self.key_type_id == other.key_type_id:
               return cmp(
(self.object._p_jar.db().database_name, self.object._p_oid), (other.object._p_jar.db().database_name, other.object._p_oid),
           except AttributeError:
               # a transaction might be rollbacked
               self_db = getattr(self.object._p_jar, 'db', '')
               if self_db:
                   self_db = self_db().database_name
               other_db = getattr(other.object._p_jar, 'db', '')
               if other_db:
                   other_db = other_db().database_name

               return cmp(
                   (self_db,  self.object._p_oid),
                   (other_db, other.object._p_oid),

Gary, thank you very much for your support!


KeyReferenceToPersistent __cmp__ error

   >>> from ZODB.tests.util import DB
   >>> from zope import component
   >>> from import setSite
   >>> from import Folder, rootFolder
   >>> from import setup
   >>> import sys
   >>> import traceback
   >>> import transaction

Basic Error Story

We declare an persistent example class:

   >>> from persistent import Persistent
   >>> from import Contained
   >>> class P(Contained, Persistent):
   ...    pass

   >>> p1 = P()

The instance does not have an associatet db yet that would be called by the
__cmp__ function:

   >>> p1._p_jar.db().database_name
   Traceback (most recent call last):
   AttributeError: 'NoneType' object has no attribute 'db'

This is not a reail problem because the KeyReferenceToPersistent asserts that the
db can be evaluated. Otherwise it raises the NotYet error:

>>> from import KeyReferenceToPersistent

   >>> keyref1 = KeyReferenceToPersistent(p1)
   Traceback (most recent call last):
   NotYet: <example.P object at ...>

For the further exploration we have to do a placeful setup:

   >>> setup.placefulSetUp()
   >>> db = DB()
   >>> conn =
   >>> root = conn.root()

   >>> root['top'] = top = rootFolder()
   >>> top.__name__ = 'top'
   >>> sm = setup.createSiteManager(top, True)
   >>> transaction.commit()
   >>> setSite(top)

An register the relevant adapters to IConnection and IKeyReference:

   >>> from ZODB.interfaces import IConnection
   >>> from persistent.interfaces import IPersistent
   >>> from import IKeyReference
   >>> from import connectionOfPersistent

>>> component.provideAdapter(KeyReferenceToPersistent, adapts=(IPersistent,), provides=IKeyReference) >>> component.provideAdapter(connectionOfPersistent, adapts=(IPersistent,), provides=IConnection)

Now we can drive on. After the __parent__ got set a persistent object can be
adapted to IKeyReference using the underlying connection:

   >>> p1.__parent__ = top

   >>> keyref1 = KeyReferenceToPersistent(p1)

We commit the transaction and create a savepoint for the further experiment:

   >>> transaction.commit()
   >>> savepoint = transaction.savepoint()

If we create an other P instance, we are able to compare p1 and p2:

   >>> p2 = P()
   >>> p2.__parent__ = top
   >>> keyref2 = KeyReferenceToPersistent(p2)
   >>> cmp(keyref1, keyref2)

But if we rollback to the savepoint the comparison will crash:

   >>> savepoint.rollback()
   >>> cmp(keyref1, keyref2)
   Traceback (most recent call last):
   AttributeError: 'NoneType' object has no attribute 'db'

Error Story using IntIds utility (That does not reproduce the error)

Add intid utility and its subscriber:

   >>> from import IObjectAddedEvent
   >>> from import IObjectRemovedEvent
   >>> from import IntIds
   >>> from import addIntIdSubscriber
   >>> from import removeIntIdSubscriber
   >>> from import IIntIdAddedEvent
   >>> from import IIntIdRemovedEvent
   >>> from import IIntIds
   >>> from zope.location.interfaces import ILocation

>>> component.provideHandler(removeIntIdSubscriber, (ILocation, IObjectRemovedEvent)) >>> component.provideHandler(addIntIdSubscriber, (ILocation, IObjectAddedEvent))

   >>> top['intids'] = intids = IntIds()
   >>> sm.registerUtility(intids, IIntIds, '')
   >>> transaction.commit()

Provide a notification mechanism using a simple subscription handler:

   >>> def notifyRegistration(e):
   ...    print "Registered '%s'." % e.object.__name__

   >>> def notifyUnRegistration(e):
   ...    print "Unregistered '%s'." % e.object.__name__

   >>> component.provideHandler(notifyRegistration, (IIntIdAddedEvent,))
>>> component.provideHandler(notifyUnRegistration, (IIntIdRemovedEvent,))

We add our p1 to the top folder:

   >>> top['p1'] = p1
   Registered 'p1'.

We commit the transaction and create a savepoint for the further experiment:

   >>> transaction.commit()
   >>> savepoint = transaction.savepoint()

We create p2 instance of P:

   >>> p2 = P()
   >>> top['p2'] = p2
   Registered 'p2'.

In our real application an error occurs within a initialisation-handler which is
listen to ObjectAddedEvent. Therefore we have to rollback the transaction to
the savepoint:

   >>> savepoint.rollback()

Now we try to add a further p. Within this test it works, ...:

   >>> p3 = P()
   >>> top['p3'] = p3
   Registered 'p3'.

... but in our application we get the same error as in the Basic Error Story

After the addition of p2 was failed the following failes too, because of the
__cmp__ error. The incorrect sorting/indexing is not persistent if the
application is restarted no further error occurs. The __cmp__ error appears
only once after an unsuccessfull import. Therefore the __cmp__ error causes
something like a chain reaction. That means if we can ommit the __cmp__
error once and commit an import correctly and *no* incorrect sorting/indexing
are stored within the intids utitility.

   >>> setup.placefulTearDown()
Zope3-dev mailing list

Reply via email to