[Zope3-dev] Weird error within __cmp__ method of KeyReferenceToPersistent

[Gary Poster]

On Jul 27, 2006, at 12:42 PM, Dominik Huber wrote:

> Hi,
>
> Today I got a weird error caused by the __cmp__ method of  
> KeyReferenceToPersistent. Unfortunately I couldn't setup a  
> dedicated environment to reproduce the error within a test :(. The  
> only evidence is this traceback:
>
> Traceback (most recent call last):
>  File "E:\workspace\bopp.ms\src\perse\generic\csv\base.py", line  
> 208, in __call__
>    importhandler(configuration)
> [...]
>  File "E:\workspace\bopp.ms\Zope3\src\zope\interface\adapter.py",  
> line 535, in subscribers
>    subscription(*objects)
>  File "E:\workspace\bopp.ms\Zope3\src\zope\app\intid\__init__.py",  
> line 165, in addIntIdSubscriber
>    utility.register(key)
>  File "E:\workspace\bopp.ms\Zope3\src\zope\app\intid\__init__.py",  
> line 112, in register
>    if key in self.ids:
>  File "E:\workspace\bopp.ms\Zope3\src\zope\app\keyreference 
> \persistent.py", line 57, in __cmp__
>    return cmp(
> AttributeError: 'NoneType' object has no attribute 'db'
>
> I would propose the following fix, but I'm not aware about all  
> possible impacts. Therefore I'm asking for supervision.
>
> class KeyReferenceToPersistent(object):
>    [...]
>    def _database_name(self):
>        try:
>            return self.object._p_jar.db().database_name
>        except AttributeError:
>            return ''
>
>    def __cmp__(self, other):
>        if self.key_type_id == other.key_type_id:
>            return cmp(
>                (self._database_name(),  self.object._p_oid),
>                (other._database_name(), other.object._p_oid),
>                )
>
>        return cmp(self.key_type_id, other.key_type_id)
>
> If there are no objections, I'm going to commit the fix tomorrow.

I object. :-)

First, your change effectively adds _database_name to an unspoken  
interface for persistent IKeyReferences.  That's not a good idea.

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.  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.

Gary