plone / plone.dexterity

Base framework for building content types, both through-the-web and as filesystem code for Zope (CMF/Plone)
https://pypi.org/project/plone.dexterity
22 stars 25 forks source link

noLongerProvides seems broken -> ValueError #126

Open iham opened 4 years ago

iham commented 4 years ago

even this is in zope.interface.declarations.py

this error bumps up in collective.instancebehavior and also in simple behavior adapters.

plone.dexterity 2.9.1 does well; 2.9.2 breaks.

iham commented 4 years ago

@jensens can u help? there are lots of changes u made in that 2.9.2 release

jensens commented 4 years ago

collective.instancebehavior stores the aditional behaviors in annotations. Thus the instances _p_mtime https://github.com/plone/plone.dexterity/blob/24a59155fd956a2ed03bb12dcd079d74ede5a9c7/plone/dexterity/content.py#L162 was not modified and so the __providedBy__ cache is not invalidated.

It looks like _p_mtime as the factor to identify behavior assignable changes is not sufficient.

I am thinking about a solution, stay tuned.

iham commented 4 years ago
Traceback (innermost last):
  Module ZServer.ZPublisher.Publish, line 144, in publish
  Module ZPublisher.mapply, line 85, in mapply
  Module ZServer.ZPublisher.Publish, line 44, in call_object
  Module akbild.sync.browser.actions, line 44, in toggle_sync
  Module collective.instancebehavior, line 110, in disable_behaviors
  Module zope.interface.declarations, line 619, in noLongerProvides
ValueError: Can only remove directly provided interfaces.

plone.dexterity 2.9.5 (same with 2.9.7) zope.interface 4.6.0 collective.instancebehavior 0.4

mauritsvanrees commented 4 years ago

This is marked as blocker. Should this block a Plone 5.2.2 release?

iham commented 4 years ago

@mauritsvanrees if using plone.dexerity >= 2.9.41 (got the version wrong in my head) in plone 5.2.2 it kinda is, as you can‘t use instance bahaviors. And it seams not be the „fault“ of instance behaviors. @jensens tried some stuff there.

iham commented 4 years ago

maybe instancebehaviors are no more important?

i see a big issue on the rise when we are forced to pin plone.dexterity to 2.9.1 when we are officially already at 2.9.7

this starts to itch in the backside of my head...

@jensens i really need some reaction from your side, as you are - to me - the only one deep down into that topic.

mauritsvanrees commented 4 years ago

I would love to have this fixed, although I don't use instancebehaviors myself. But there has been no movement on this issue since January, and I don't want to wait indefinitely with the release.

Actually, I see Jens did a fix in https://github.com/collective/collective.instancebehavior/commit/30d904d44518f46828d3ad740a4ec6b8a4cf5540 in February that seems related. It notifies plone.dexterity.schema.SchemaInvalidatedEvent Can you try out this fix?

The fix is not in a release yet. I see only @rnix can make a PyPI release.

iham commented 4 years ago

@mauritsvanrees i totally understand your motivation and would have done the same.

the fix @30d9... did not do the trick sadly and it looks like a problem in plone.dexterity itself, not in instancebehavior.

jensens commented 4 years ago

I will have a look the next days ... but I do not think this is a blocker. Instancebehaviors are a special case.

iham commented 4 years ago

I will have a look the next days ... but I do not think this is a blocker. Instancebehaviors are a special case.

thank you jens - rly looking forward for your help on that. even they are a special case, they are a very mighty tool and therefore very appreciated ;)

i removed the blocker to not confuse/interfere with release plans and such.

kcleong commented 3 years ago

I'm using collective.instancebehavior 0.4 in plone.dexterity 2.10.0 (in Plone 5.2.3) and encountered this bug. It seems related to caching in zope.interface.

In zope.interface.declarations:

def noLongerProvides(object, interface): # pylint:disable=redefined-builtin
    """ Removes a directly provided interface from an object.
    """
    directlyProvides(object, directlyProvidedBy(object) - interface)
    if interface.providedBy(object):
        raise ValueError("Can only remove directly provided interfaces.")

The error is raised when interface.providedBy(object) is True. However if I disable this check in the last two lines and call noLongerProvides the interface is removed.

After completing the request/transaction the interface is no longer provided by the object; when I call interface.providedBy (in a second request) the return value is False.

If zope.interface caching is indeed the culprit the cache should be purged before the interface.providedBy check is called. My knowledge of the internals in zope interface is limited and I could not find an easy way to purge the interface cache.

jamadden commented 3 years ago

@kcleong I can't reproduce that using just zope.interface, so I suspect any caching issue is at a higher level.

>>> from zope.interface import Interface, directlyProvides, directlyProvidedBy
>>> class IDirect(Interface):
...     pass
...
>>> class Thing(object):
...     pass
...
>>> thing = Thing()
>>> directlyProvides(thing, (IDirect,))
None
>>> IDirect.providedBy(thing)
True
>>> # Now manually mimic what `noLongerProvides` does so we can check the conditions
>>> directlyProvides(thing, directlyProvidedBy(thing) - IDirect)
None
>>> IDirect.providedBy(thing)
False
jensens commented 3 years ago

I also do not think this is zope.interface related. Moreover it seems the SchemnaInvalidatedEvent https://github.com/collective/collective.instancebehavior/blob/30d904d44518f46828d3ad740a4ec6b8a4cf5540/src/collective/instancebehavior/__init__.py#L83 is not picked up correctly by plone.dexterity's schema cache.