Mailing List Archive: Zope: CMF

five.intid and DirectoryView

Index | Next | Previous | View Threaded

me at rpatterson

Jun 25, 2008, 8:26 PM

Post #1 of 9 (766 views)
Permalink

five.intid and DirectoryView

I've run into an interaction between five.intid and
Products.CMFCore.DirectoryView a couple of times now.

DirectoryView stores instances of Persistent classes (like
FSPageTemplate) in a global registry and thus counts on those instances
never being added to a ZODB connection. When added to a ZODB
connection, the retrieval from global state could result in an instance
from a closed connection being de-ghosted raising a
ConnectionStateError. This seems wrong in that the object subclasses
Persistent but is being used in a way that doesn't allow it to be stored
in a ZODB. Why do the FSObject classes subclass Persistent?

For Persistent objects that have no connection yet, five.intid adds them
to the connection of the nearest parent object that already has a
connection. This is the same approach taken by zope.app.keyreference.

I previously committed a workaround to five.intid for a related
FSPythonScripts pickling error and I've extended that workaround to all
FSObjects for now. But I'd like to put the question of what the "right"
fix is before y'all. Whaddya think?

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

me at rpatterson

Jun 26, 2008, 10:15 AM

Post #2 of 9 (733 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

"Martijn Pieters" <mj[at]zopatista.com> writes:

> On Thu, Jun 26, 2008 at 5:26 AM, Ross Patterson <me[at]rpatterson.net> wrote:
>> This seems wrong in that the object subclasses
>> Persistent but is being used in a way that doesn't allow it to be stored
>> in a ZODB. Why do the FSObject classes subclass Persistent?
>
> I think because they are also used to create TTW skin objects; hit
> 'customize' on one such object in the skins directory and a copy is
> created that *is* persisted.

So wouldn't that mean that DirectoryView should not store instances of
FSObjects in a registry stored in global state and shared between ZODB
connections?

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

me at rpatterson

Jun 26, 2008, 1:08 PM

Post #3 of 9 (730 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

"Martijn Pieters" <mj[at]zopatista.com> writes:

> On Thu, Jun 26, 2008 at 7:15 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>>> I think because they are also used to create TTW skin objects; hit
>>> 'customize' on one such object in the skins directory and a copy is
>>> created that *is* persisted.
>>
>> So wouldn't that mean that DirectoryView should not store instances of
>> FSObjects in a registry stored in global state and shared between ZODB
>> connections?
>
> I don't see why not. Those stored in the global state are not the
> persisted objects, only those loaded from filesystem files.

But since the FSObjects provide IPersistent, they can be added to a
connection. If they're added to a connection, the object can be ghosted
when that connection is closed. If another thread subsequently consults
the DirectoryView registry for the same object, the persistnce machinery
will raise ConnectionStateError when it tries to de-ghost the object
from the closed connection.

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

me at rpatterson

Jun 27, 2008, 9:53 AM

Post #4 of 9 (723 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

"Martijn Pieters" <mj[at]zopatista.com> writes:

> On Thu, Jun 26, 2008 at 10:08 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>> But since the FSObjects provide IPersistent, they can be added to a
>> connection. If they're added to a connection, the object can be
>> ghosted when that connection is closed. If another thread
>> subsequently consults the DirectoryView registry for the same object,
>> the persistnce machinery will raise ConnectionStateError when it
>> tries to de-ghost the object from the closed connection.
>
> But the code never does that. When cloning a file-based FSObject, a
> new instance is created and that is added to the ZODB. Noone else
> should do this either.

zope.app.keyreference does. The persistence machinery doesn't add an
object to a connection until commit. As such, an IPersistent and
IObjectAdded event handler, such as the one in zope.app.intid, that
needs the object to have a connection needs to add the object to a
connection.

Shouldn't anything that implements IPersistent be able to be added to a
connection? Wouldn't that be considered part of providing the
interface? Where else is an object that provides IPersistent stored in
global state?

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

tseaver at palladion

Jun 28, 2008, 8:25 AM

Post #5 of 9 (715 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martijn Pieters wrote:
> On Fri, Jun 27, 2008 at 6:53 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>> "Martijn Pieters" <mj[at]zopatista.com> writes:
>>> But the code never does that. When cloning a file-based FSObject, a
>>> new instance is created and that is added to the ZODB. Noone else
>>> should do this either.
>> zope.app.keyreference does. The persistence machinery doesn't add an
>> object to a connection until commit. As such, an IPersistent and
>> IObjectAdded event handler, such as the one in zope.app.intid, that
>> needs the object to have a connection needs to add the object to a
>> connection.

Sounds like a bug in zope.app.intid to me: it shouldn't be forcing
objects to have connections.

> Why is the IObjectAdded event fired at all? Perhaps that's the bug here.
>
>> Shouldn't anything that implements IPersistent be able to be added to a
>> connection? Wouldn't that be considered part of providing the
>> interface? Where else is an object that provides IPersistent stored in
>> global state?
>
> I assume it was easier at the time to use just one class. Perhaps this
> should be reconsidered now. However, just providing the IPersistence
> interface does not mean the object expects to be added to a connection
> arbitrarily.

Exactly. Nobody is supposed to add objects to a connection except their
already-persisted containers.

- --
===================================================================
Tres Seaver +1 540-429-0999 tseaver[at]palladion.com
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIZlfe+gerLs4ltQ4RAoGOAJ954hDazoOKVj7TZKJQX84wB0LRUwCgox23
rjY5Q3Xjhu+eDcVnmmzXdjs=
=pF+8
-----END PGP SIGNATURE-----

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

me at rpatterson

Jun 28, 2008, 8:43 AM

Post #6 of 9 (715 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

Tres Seaver <tseaver[at]palladion.com> writes:

> Martijn Pieters wrote:
>> On Fri, Jun 27, 2008 at 6:53 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>>> "Martijn Pieters" <mj[at]zopatista.com> writes:
>>>> But the code never does that. When cloning a file-based FSObject, a
>>>> new instance is created and that is added to the ZODB. Noone else
>>>> should do this either.
>>> zope.app.keyreference does. The persistence machinery doesn't add an
>>> object to a connection until commit. As such, an IPersistent and
>>> IObjectAdded event handler, such as the one in zope.app.intid, that
>>> needs the object to have a connection needs to add the object to a
>>> connection.
>
> Sounds like a bug in zope.app.intid to me: it shouldn't be forcing
> objects to have connections.
>
>> Why is the IObjectAdded event fired at all? Perhaps that's the bug here.
>>
>>> Shouldn't anything that implements IPersistent be able to be added to a
>>> connection? Wouldn't that be considered part of providing the
>>> interface? Where else is an object that provides IPersistent stored in
>>> global state?
>>
>> I assume it was easier at the time to use just one class. Perhaps this
>> should be reconsidered now. However, just providing the IPersistence
>> interface does not mean the object expects to be added to a connection
>> arbitrarily.
>
> Exactly. Nobody is supposed to add objects to a connection except their
> already-persisted containers.

That sounds right to me especially given that an object's parent isn't
necessarily "where" the object is persisted. Shouldn't it be possible,
for example, to have a container that looks up it's contained items from
a utility that actually is stored in another ZODB. Such a container's
items would not share their parent's connection.

FWIW, this happens in zope.app.keyreference. The reason it needs the
object to have a connection is so that it can get the object's _p_oid.
If this is a bug, how can zope.app.keyreference get _p_oid for an object
added in the current, as yet uncommitted transaction?

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

l at lrowe

Jun 30, 2008, 3:20 AM

Post #7 of 9 (696 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

Ross Patterson wrote:
> Tres Seaver <tseaver[at]palladion.com> writes:
>
>> Martijn Pieters wrote:
>>> On Fri, Jun 27, 2008 at 6:53 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>>>> "Martijn Pieters" <mj[at]zopatista.com> writes:
>>>>> But the code never does that. When cloning a file-based FSObject, a
>>>>> new instance is created and that is added to the ZODB. Noone else
>>>>> should do this either.
>>>> zope.app.keyreference does. The persistence machinery doesn't add an
>>>> object to a connection until commit. As such, an IPersistent and
>>>> IObjectAdded event handler, such as the one in zope.app.intid, that
>>>> needs the object to have a connection needs to add the object to a
>>>> connection.
>> Sounds like a bug in zope.app.intid to me: it shouldn't be forcing
>> objects to have connections.
>>
>>> Why is the IObjectAdded event fired at all? Perhaps that's the bug here.
>>>
>>>> Shouldn't anything that implements IPersistent be able to be added to a
>>>> connection? Wouldn't that be considered part of providing the
>>>> interface? Where else is an object that provides IPersistent stored in
>>>> global state?
>>> I assume it was easier at the time to use just one class. Perhaps this
>>> should be reconsidered now. However, just providing the IPersistence
>>> interface does not mean the object expects to be added to a connection
>>> arbitrarily.
>> Exactly. Nobody is supposed to add objects to a connection except their
>> already-persisted containers.
>
> That sounds right to me especially given that an object's parent isn't
> necessarily "where" the object is persisted. Shouldn't it be possible,
> for example, to have a container that looks up it's contained items from
> a utility that actually is stored in another ZODB. Such a container's
> items would not share their parent's connection.
>
> FWIW, this happens in zope.app.keyreference. The reason it needs the
> object to have a connection is so that it can get the object's _p_oid.
> If this is a bug, how can zope.app.keyreference get _p_oid for an object
> added in the current, as yet uncommitted transaction?

Creating a savepoint seems to do the trick:

>>> from persistent import Persistent
>>> import transaction
>>> app.pob = Persistent()
>>> app.pob._p_oid is None
True
>>> s = transaction.savepoint()
>>> app.pob._p_oid
'\x00\x00\x00\x00\x00\x00E\xfa'

Laurence

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

me at rpatterson

Jun 30, 2008, 10:34 AM

Post #8 of 9 (696 views)
Permalink

Re: five.intid and DirectoryView [In reply to]

Laurence Rowe <l[at]lrowe.co.uk> writes:

> Ross Patterson wrote:
>> Tres Seaver <tseaver[at]palladion.com> writes:
>>
>>> Martijn Pieters wrote:
>>>> On Fri, Jun 27, 2008 at 6:53 PM, Ross Patterson <me[at]rpatterson.net> wrote:
>>>>> "Martijn Pieters" <mj[at]zopatista.com> writes:
>>>>>> But the code never does that. When cloning a file-based FSObject, a
>>>>>> new instance is created and that is added to the ZODB. Noone else
>>>>>> should do this either.
>>>>> zope.app.keyreference does. The persistence machinery doesn't add an
>>>>> object to a connection until commit. As such, an IPersistent and
>>>>> IObjectAdded event handler, such as the one in zope.app.intid, that
>>>>> needs the object to have a connection needs to add the object to a
>>>>> connection.
>>> Sounds like a bug in zope.app.intid to me: it shouldn't be forcing
>>> objects to have connections.
>>>
>>>> Why is the IObjectAdded event fired at all? Perhaps that's the bug here.
>>>>
>>>>> Shouldn't anything that implements IPersistent be able to be added to a
>>>>> connection? Wouldn't that be considered part of providing the
>>>>> interface? Where else is an object that provides IPersistent stored in
>>>>> global state?
>>>> I assume it was easier at the time to use just one class. Perhaps this
>>>> should be reconsidered now. However, just providing the IPersistence
>>>> interface does not mean the object expects to be added to a connection
>>>> arbitrarily.
>>> Exactly. Nobody is supposed to add objects to a connection except their
>>> already-persisted containers.
>>
>> That sounds right to me especially given that an object's parent isn't
>> necessarily "where" the object is persisted. Shouldn't it be possible,
>> for example, to have a container that looks up it's contained items from
>> a utility that actually is stored in another ZODB. Such a container's
>> items would not share their parent's connection.
>>
>> FWIW, this happens in zope.app.keyreference. The reason it needs the
>> object to have a connection is so that it can get the object's _p_oid.
>> If this is a bug, how can zope.app.keyreference get _p_oid for an object
>> added in the current, as yet uncommitted transaction?
>
> Creating a savepoint seems to do the trick:
>
>>>> from persistent import Persistent
>>>> import transaction
>>>> app.pob = Persistent()
>>>> app.pob._p_oid is None
> True
>>>> s = transaction.savepoint()
>>>> app.pob._p_oid
> '\x00\x00\x00\x00\x00\x00E\xfa'

Would that be a problem causing ZODB bloat? Say if the object were
deleted later in the same transaction? Would it slow down transactions
adding large numbers of IPersistent objects?

Can anyone else see any other potential problems with using savepoints
instead of manually adding objects to a transaction? If my questions
aboved are all resolved, should the implementation of
zope.app.keyreference (and five.intid) be changed?

Ross

_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

dieter at handshake

Jul 3, 2008, 10:07 PM

Post #9 of 9 (669 views)
Permalink

Re: Re: five.intid and DirectoryView [In reply to]

Ross Patterson wrote at 2008-6-30 10:34 -0700:
> ...
>> Creating a savepoint seems to do the trick:
>>
>>>>> from persistent import Persistent
>>>>> import transaction
>>>>> app.pob = Persistent()
>>>>> app.pob._p_oid is None
>> True
>>>>> s = transaction.savepoint()
>>>>> app.pob._p_oid
>> '\x00\x00\x00\x00\x00\x00E\xfa'
>
>Would that be a problem causing ZODB bloat?

"savepoint" does not write to the main storage.
The main storage is only modified after the final "commit".

--
Dieter
_______________________________________________
Zope-CMF maillist - Zope-CMF[at]lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com