ContainedProxy not working for mutable non-persistent objects

[Jacob Holm]

I'll upload a patch to test_contained.py that demonstrates the problem.

Basically, the issue is that ContainedProxy uses the argument to __new__ as the only way to set the state of the proxied object. However, __new__ is only called for objects that are not in the cache. If the ContainedProxy object is a ghost, the proxied object will not be updated.

I think a fix needs to handle non-persistent objects differently by adding a setProxiedObject function and changing __getnewargs__, __getstate__, and __setstate__ to use a different format for non-persistent objects. Here is what I propose:

__getnewargs__ is changed to return None if proxy_object is not an instance of Persistent.
__getstate__ is extended to (__name__, __parent__, proxy_object) if proxy_object is not an instance of Persistent.
__setstate__ checks length of the "state" tuple and calls setProxiedObject if there is a 3rd arg.
__reduce__ and __reduce_ex__ are changed to match the change in __getstate__ .

In addition, we might want to change the __setattr__ implementation to check if we are wrapping a non-persistent object and automatically set the _p_changed attribute if we are.

[Jacob Holm (jacobholm)]

This patch extends the test_basic_persistent_w_non_persistent_proxied test by mutating the proxied object and verifying the change. It fails with the current implementation.

[Jacob Holm (jacobholm)]

After thinking a bit more, I realize that the changes suggested above won't work.

If we want to allow replacing the proxied object (and I think we already do allow that by calling ContainedProxyBase.__init__) we need the following changes instead:

__getnewargs__ should always return (None,) or (). (If (), we also need to change __new__ to make the argument optional)
__getstate__ always returns (__name__, __parent__, proxy_object)
__setstate__ accepts either (__name__, __parent__) for backward bug compatibility or (__name__, __parent__, proxy_object)
__reduce__ and __reduce_ex__ are changed to match the change in __getnewargs__ and __getstate__

In addition, the macro Proxy_GET_OBJECT needs to be changed to ensure that our object is not a ghost. This looks to me like it will be the biggest part of the change. It will also most likely mean that we have to drop the implementation hack of using a copy of _zope_proxy_proxy.c. (The comment in the top of that file says that the file is supposed to be identical in zope.app.container and zope.proxy. This is currently not the case on the trunks)

[Tres Seaver (tseaver)]

I have opened lp:566016 to track the problem of the disparity between the two copies of _zope_proxy_proxy.c.

[Tres Seaver (tseaver)]

The test in the patch looks reasonable. It needs to be applied with some care to zope.container, as it was originally made against zope.app.container. E.g.,

  $ cd /path/to/zope.container/trunk/src/zope/container
  $ patch -p4 /tmp/185054-demonstration.patch

[Colin Watson (cjwatson)]

The zope.container project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope.container.