On 7/3/12 13:26 , Cykooz wrote:

2012/7/3 Jan-Wijbrand Kolman <janwijbrand@gmail.com mailto:janwijbrand@gmail.com>

Are you basically using a forked zope.keyreference for the time being?

No, I do not use a forked zope.keyreference. I used my fork of the zope.intid.

Ah, yes of course, that's what I meant: zope.intid.

I would not consider myself in a position currently to vouch for you to have comitter rights. I do however wonder if anyone else would like to comment on your proposed change.

If the comments a favorable I could try to help apply the patch to zope.intid. This would probably help me and you as you then would not have to use a forked package anymore.

At the end of this post, I pasted the diff from the current zope.intid trunk against your "fork" on bitbucket. Maybe this would make it easier for others to comment on it?

regards, jw

Proposed patch: ===============

diff -u zope.intid/trunk/src/zope/intid//__init__.py zope.intid-cykooz/src/zope/intid//__init__.py --- zope.intid/trunk/src/zope/intid//__init__.py 2012-07-03 11:56:11.576511518 +0200 +++ zope.intid-cykooz/src/zope/intid//__init__.py 2012-07-03 11:55:17.261865415 +0200 @@ -19,12 +19,14 @@ This functionality can be used in cataloging. """ import random +import threading +from weakref import WeakKeyDictionary, WeakSet

import BTrees from persistent import Persistent from zope.component import adapter, getAllUtilitiesRegisteredFor, subscribers from zope.event import notify -from zope.interface import implementer +from zope.interface import implements from zope.keyreference.interfaces import IKeyReference, NotYet from zope.lifecycleevent.interfaces import IObjectAddedEvent from zope.lifecycleevent.interfaces import IObjectRemovedEvent @@ -32,16 +34,16 @@ from zope.location.interfaces import IContained from zope.security.proxy import removeSecurityProxy

-from zope.intid.interfaces import IIntIds, IIntIdEvent +from zope.intid.interfaces import IIntIds, IIntIdEvent, IIntIdsDisabled from zope.intid.interfaces import IntIdAddedEvent, IntIdRemovedEvent

-@implementer(IIntIds, IContained) class IntIds(Persistent): """This utility provides a two way mapping between objects and integer ids.

IKeyReferences to objects are stored in the indexes. """ + implements(IIntIds, IContained)

__parent__ = __name__ = None

@@ -136,6 +138,10 @@ del self.ids[key]

+thread_data = threading.local() +thread_data.deferred_objects = WeakKeyDictionary() + + @adapter(ILocation, IObjectRemovedEvent) def removeIntIdSubscriber(ob, event): """A subscriber to ObjectRemovedEvent @@ -143,9 +149,22 @@ Removes the unique ids registered for the object in all the unique id utilities. """ + utilities = tuple(getAllUtilitiesRegisteredFor(IIntIds)) if utilities: - key = IKeyReference(ob, None) + try: + key = IKeyReference(ob, None) + except NotYet: + deferred_objects = thread_data.deferred_objects + if ob in deferred_objects: + del deferred_objects[ob] + parent = getattr(ob, '__parent__', None) + if parent in deferred_objects and ob in deferred_objects[parent]: + deferred_objects[parent].remove(ob) + if len(deferred_objects[parent]) == 0: + del deferred_objects[parent] + return + # Register only objects that adapt to key reference if key is not None: # Notify the catalogs that this object is about to be removed. @@ -156,6 +175,7 @@ except KeyError: pass

+ @adapter(ILocation, IObjectAddedEvent) def addIntIdSubscriber(ob, event): """A subscriber to ObjectAddedEvent @@ -163,16 +183,46 @@ Registers the object added in all unique id utilities and fires an event for the catalogs. """ + utilities = tuple(getAllUtilitiesRegisteredFor(IIntIds)) if utilities: # assert that there are any utilites + register_object(ob, utilities, event) + + +def register_object(ob, utilities, event): + deferred_objects = thread_data.deferred_objects + intids_enabled = not IIntIdsDisabled.providedBy(ob) + try: key = IKeyReference(ob, None) - # Register only objects that adapt to key reference - if key is not None: - idmap = {} - for utility in utilities: - idmap[utility] = utility.register(key) - # Notify the catalogs that this object was added. - notify(IntIdAddedEvent(ob, event, idmap)) + except NotYet: + if intids_enabled: + parent = getattr(ob, '__parent__', None) + if parent is None: + raise + if parent not in deferred_objects: + deferred_objects[parent] = WeakSet() + deferred_objects[parent].add(ob) + return + + # Register only objects that adapt to key reference + if key is None: + return + + # Register the current object if it is enabled + if intids_enabled: + idmap = {} + for utility in utilities: + idmap[utility] = utility.register(key) + # Notify the catalogs that this object was added. + notify(IntIdAddedEvent(ob, event, idmap)) + + # Register the deferred children of the current object + if ob in deferred_objects: + children = deferred_objects.pop(ob) + for child in children: + if child.__parent__ is ob: + register_object(child, utilities, event) +

@adapter(IIntIdEvent) def intIdEventNotify(event): diff -u zope.intid/trunk/src/zope/intid//interfaces.py zope.intid-cykooz/src/zope/intid//interfaces.py --- zope.intid/trunk/src/zope/intid//interfaces.py 2012-07-03 11:56:11.576511518 +0200 +++ zope.intid-cykooz/src/zope/intid//interfaces.py 2012-07-03 11:55:17.261865415 +0200 @@ -1,6 +1,6 @@ """Interfaces for the unique id utility. """ -from zope.interface import Interface, Attribute, implementer +from zope.interface import Interface, Attribute, implements

class IIntIdsQuery(Interface): @@ -61,6 +61,10 @@ """

+class IIntIdsDisabled(Interface): + """ Marker for objects that should not be indexed. """ + + class IIntIdEvent(Interface): """Generic base interface for IntId-related events"""

@@ -77,12 +81,13 @@ """

-@implementer(IIntIdRemovedEvent) class IntIdRemovedEvent(object): """The event which is published before the unique id is removed from the utility so that the catalogs can unindex the object. """

+ implements(IIntIdRemovedEvent) + def __init__(self, object, event): self.object = object self.original_event = event @@ -98,12 +103,13 @@ idmap = Attribute("The dictionary that holds an (utility -> id) mapping of created ids")

-@implementer(IIntIdAddedEvent) class IntIdAddedEvent(object): """The event which gets sent when an object is registered in a unique id utility. """

+ implements(IIntIdAddedEvent) + def __init__(self, object, event, idmap=None): self.object = object self.original_event = event Only in zope.intid/trunk/src/zope/intid/: .svn diff -u zope.intid/trunk/src/zope/intid//tests.py zope.intid-cykooz/src/zope/intid//tests.py --- zope.intid/trunk/src/zope/intid//tests.py 2012-07-03 11:56:11.576511518 +0200 +++ zope.intid-cykooz/src/zope/intid//tests.py 2012-07-03 11:55:17.261865415 +0200 @@ -13,6 +13,7 @@ ############################################################################## """Tests for the unique id utility. """ +import gc import random import unittest

@@ -25,23 +26,25 @@ from zope.component import provideHandler from zope.component import testing, eventtesting from zope.component.interfaces import ISite, IComponentLookup -from zope.interface import implementer, Interface +from zope.container.interfaces import ISimpleReadContainer +from zope.container.traversal import ContainerTraversable +from zope.interface import implements, Interface +from zope.interface.declarations import directlyProvides from zope.interface.verify import verifyObject from zope.keyreference.persistent import KeyReferenceToPersistent from zope.keyreference.persistent import connectionOfPersistent from zope.keyreference.interfaces import IKeyReference +from zope.lifecycleevent import ObjectAddedEvent from zope.location.interfaces import ILocation from zope.site.hooks import setSite, setHooks, resetHooks -from zope.site.folder import rootFolder +from zope.site.folder import rootFolder, Folder from zope.site.site import SiteManagerAdapter, LocalSiteManager from zope.traversing import api from zope.traversing.testing import setUp as traversingSetUp from zope.traversing.interfaces import ITraversable -from zope.container.traversal import ContainerTraversable -from zope.container.interfaces import ISimpleReadContainer

-from zope.intid import IntIds, intIdEventNotify -from zope.intid.interfaces import IIntIds +from zope.intid import IntIds, intIdEventNotify, addIntIdSubscriber +from zope.intid.interfaces import IIntIds, IIntIdsDisabled

# Local Utility Addition @@ -67,9 +70,8 @@ return api.traverse(folder, "++etc++site")

-@implementer(ILocation) class P(Persistent): - pass + implements(ILocation)

class ConnectionStub(object): @@ -101,9 +103,9 @@ self.root = rootFolder() createSiteManager(self.root, setsite=True)

- provideAdapter(connectionOfPersistent, (IPersistent, ), IConnection) + provideAdapter(connectionOfPersistent, (IPersistent,), IConnection) provideAdapter( - KeyReferenceToPersistent, (IPersistent, ), IKeyReference) + KeyReferenceToPersistent, (IPersistent,), IKeyReference)

def tearDown(self): resetHooks() @@ -159,14 +161,14 @@ # behaviour of the _generateId method u = self.createIntIds() maxint = u.family.maxint - u._randrange = lambda x,y: maxint-1 + u._randrange = lambda x, y: maxint - 1

conn = ConnectionStub()

obj = P() conn.add(obj) uid = u.register(obj) - self.assertEquals(maxint-1, uid) + self.assertEquals(maxint - 1, uid) self.assertEquals(maxint, u._v_nextid)

# The next chosen int is exactly the largest number possible that is @@ -243,7 +245,6 @@ class TestSubscribers(ReferenceSetupMixin, unittest.TestCase):

def setUp(self): - from zope.site.folder import Folder

ReferenceSetupMixin.setUp(self)

@@ -298,8 +299,6 @@ self.assertEquals(objevents[0][1].original_event.object, parent_folder)

def test_addIntIdSubscriber(self): - from zope.lifecycleevent import ObjectAddedEvent - from zope.intid import addIntIdSubscriber from zope.intid.interfaces import IIntIdAddedEvent from zope.site.interfaces import IFolder parent_folder = self.root['folder1']['folder1_1'] @@ -344,11 +343,63 @@ return IntIds(family=BTrees.family64)

+class TestNotYetFix(ReferenceSetupMixin, unittest.TestCase): + + def setUp(self): + ReferenceSetupMixin.setUp(self) + sm = getSiteManager(self.root) + self.utility = addUtility(sm, '1', IIntIds, IntIds()) + provideHandler(intIdEventNotify) + self.root._p_jar = ConnectionStub() + setSite(self.root) + + def test_deferred_indexes(self): + folder1 = Folder() + folder2 = Folder() + + folder1['folder2'] = folder2 + addIntIdSubscriber(folder2, ObjectAddedEvent(folder1)) + self.assertRaises(KeyError, self.utility.getId, folder2) + + self.root['folder1'] = folder1 + addIntIdSubscriber(folder1, ObjectAddedEvent(self.root)) + self.assertIsNotNone(self.utility.getId(folder1)) + self.assertIsNotNone(self.utility.getId(folder2)) + + def test_clear_deferred_objects(self): + from zope.intid import thread_data + + folder1 = Folder() + folder2 = Folder() + folder1['folder2'] = folder2 + + addIntIdSubscriber(folder2, ObjectAddedEvent(folder1)) + eventtesting.clearEvents() + + self.assertRaises(KeyError, self.utility.getId, folder2) + self.assertTrue(len(thread_data.deferred_objects) > 0) + + del folder2 + del folder1 + + gc.collect() + + self.assertTrue(len(thread_data.deferred_objects) == 0) + + def test_IIntIdsDisabled(self): + folder = Folder() + directlyProvides(folder, IIntIdsDisabled) + self.root['folder'] = folder + addIntIdSubscriber(folder, ObjectAddedEvent(self.root)) + self.assertRaises(KeyError, self.utility.getId, folder) + + def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(TestIntIds)) suite.addTest(unittest.makeSuite(TestIntIds64)) suite.addTest(unittest.makeSuite(TestSubscribers)) + suite.addTest(unittest.makeSuite(TestNotYetFix)) return suite

if __name__ == '__main__':