zopefoundation / zope.configuration

Extensible system for supporting various kinds of configurations
https://zopeconfiguration.readthedocs.io
Other
1 stars 6 forks source link

Grouping directives do not accomodate certain attributes in directive schemas #9

Open tseaver opened 9 years ago

tseaver commented 9 years ago

In https://bugs.launchpad.net/zope.configuration/+bug/274655, Devin (devin@charityfinders.com) reported:

Adding an attribute to a grouping directive's schema that is directly accessed by zope.configuration.config.GroupingStackItem using its 'context' member variable exposes a bug in the configuration machinery. There are two attributes that I know of that are problematic ('action', 'factory'), but members of zope.configuration.config.ConfigurationMachine (and its superclasses) could be suspect as well.

Attached is a sample traceback from 'transcript.log' that occurred when I attempted to use the 'factory' attribute for one of my custom grouping directives.

Note that the last line in the traceback:

    ZopeXMLConfigurationError: File "/var/lib/zope3/instance/sandbox/lib/python/powersite/browser/configure.zcml", line 20.4

... refers to the first subdirective of the directive that contains the factory attribute. The subdirective is also registered as a grouping directive.

I believe that the intended 'factory' attribute is the 'factory' method of zope.configuration.configure.ConfigurationAdapterRegistry.

I've read that complex directives will eventually be deprecated in favor of grouping directives. If that's true, then this change will need to be made for complex directives that currently have a factory or action attribute.

and linked a traceback: https://bugs.launchpad.net/zope.configuration/+bug/274655/+attachment/360145/+files/traceback.txt

Devin later followed up:

Also, if the grouping directive you create doesn't subclass zope.configuration.config.GroupingContextDecorator and/or doesn't have similar 'getattr' functionality (and still satisfies the interface contract of zope.configuration.interfaces.IGroupingContext), then Zope will terminate with an AttributeError message complaining about the lack of a 'factory' attribute.

I> nstead of attempting to find the needed methods on its 'context' member, perhaps the various stack item class instances should find the objects with the methods they need by searching up the tree until an object that implements zope.configuration.interfaces.IConfigurationContext is found. Something like:

    def __getConfigurationContext(self):
        context = self.context
        while not IConfigurationContext.providedBy(context):
            context = context.context
        return context

... and replacing self.context with self.__getConfigurationContext() (where appropriate) should work (I haven't tested it), and would still allow grouping directives to contain attributes like 'action' and 'factory'.

@do3cc asked:

Can you provide a working example?

Devin replied:

I can provide a simple example that demonstrates the bug (attached). Here's the result of running the example from the command line:

devin@development:~/temporary$ python2.5 demo.py
Traceback (most recent call last):
  File "demo.py", line 66, in <module>
    main()
  File "demo.py", line 63, in main
    string(_CONFIG)
  File "/usr/lib/python2.5/site-packages/zope/configuration/xmlconfig.py", line 627, in string
    processxmlfile(f, context)
  File "/usr/lib/python2.5/site-packages/zope/configuration/xmlconfig.py", line 378, in processxmlfile
    parser.parse(src)
  File "/usr/lib/python2.5/xml/sax/expatreader.py", line 107, in parse
    xmlreader.IncrementalParser.parse(self, source)
  File "/usr/lib/python2.5/xml/sax/xmlreader.py", line 123, in parse
    self.feed(buffer)
  File "/usr/lib/python2.5/xml/sax/expatreader.py", line 207, in feed
    self._parser.Parse(data, isFinal)
  File "/usr/lib/python2.5/xml/sax/expatreader.py", line 338, in start_element_ns
    AttributesNSImpl(newattrs, qnames))
  File "/usr/lib/python2.5/site-packages/zope/configuration/xmlconfig.py", line 230, in startElementNS
    self.context.begin(name, data, info)
  File "/usr/lib/python2.5/site-packages/zope/configuration/config.py", line 539, in begin
    self.stack.append(self.stack[-1].contained(__name, __data, __info))
  File "/usr/lib/python2.5/site-packages/zope/configuration/config.py", line 840, in contained
    return RootStackItem.contained(self, name, data, info)
  File "/usr/lib/python2.5/site-packages/zope/configuration/config.py", line 708, in contained
    factory = self.context.factory(self.context, name)
  File "demo.py", line 37, in __call__
    raise Exception("You rang?")
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "<string>", line 20.6
    Exception: You rang?

The Zope package is the Zope 3.4 package from Ubuntu's stable 9.04 repository.

Devin attached a file: https://bugs.launchpad.net/zope.configuration/+bug/274655/+attachment/714017/+files/demo.py

jamadden commented 6 years ago

For reference, demo.py (updated for Python 3) is:

from zope.configuration.config import GroupingContextDecorator
from zope.configuration.interfaces import IGroupingContext
from zope.configuration.xmlconfig import string
from zope.interface import Interface, implementer
from zope.schema import TextLine

_CONFIG = """
<configure xmlns:meta="http://namespaces.zope.org/meta">

  <meta:directives namespace="http://namespaces.zope.org/simple">
    <meta:groupingDirective
        handler="__main__.SimpleParentDirective"
        name="parent"
        schema="__main__.ISimpleParentDirective"
        />
    <meta:directive
        handler="__main__.SimpleChildDirective"
        name="child"
        schema="__main__.ISimpleChildDirective"
        usedIn="__main__.ISimpleParentDirective"
        />
  </meta:directives>

  <configure xmlns:simple="http://namespaces.zope.org/simple">
    <simple:parent factory="">
      <simple:child />
    </simple:parent>
  </configure>

</configure>
"""

class DemoString(str):

    def __call__(self, *args, **kwArgs):
        raise Exception("You rang?")

class ISimpleChildDirective(Interface):
    pass

class ISimpleParentDirective(Interface):

    factory = TextLine(required=False)

@implementer(ISimpleChildDirective)
class SimpleChildDirective(object):
    pass

@implementer(ISimpleParentDirective, IGroupingContext)
class SimpleParentDirective(GroupingContextDecorator):

    def factory():
        def getFactory(self):
            return self.__factory
        def setFactory(self, s):
            self.__factory = DemoString(s)
        return property(getFactory, setFactory)
    factory = factory()

def main():
    string(_CONFIG)

if __name__ == "__main__":
    main()

And it still produces the exception:


Traceback (most recent call last):
  File "//src/zope/configuration/xmlconfig.py", line 204, in startElementNS
    self.context.begin(name, data, info)
  File "//src/zope/configuration/config.py", line 349, in begin
    self.stack.append(self.stack[-1].contained(__name, __data, __info))
  File "//src/zope/configuration/config.py", line 516, in contained
    return RootStackItem.contained(self, name, data, info)
  File "//src/zope/configuration/config.py", line 480, in contained
    factory = self.context.factory(self.context, name)
  File "/tmp/demo.py", line 37, in __call__
    raise Exception("You rang?")
Exception: You rang?

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/demo.py", line 65, in <module>
    main()
  File "/tmp/demo.py", line 62, in main
    string(_CONFIG)
  File "//src/zope/configuration/xmlconfig.py", line 513, in string
    processxmlfile(f, context)
  File "//src/zope/configuration/xmlconfig.py", line 295, in processxmlfile
    parser.parse(src)
  File "//3.7/lib/python3.7/xml/sax/expatreader.py", line 111, in parse
    xmlreader.IncrementalParser.parse(self, source)
  File "//3.7/lib/python3.7/xml/sax/xmlreader.py", line 125, in parse
    self.feed(buffer)
  File "//3.7/lib/python3.7/xml/sax/expatreader.py", line 217, in feed
    self._parser.Parse(data, isFinal)
  File "//Python-3.7.0/Modules/pyexpat.c", line 412, in StartElement
  File "//3.7/lib/python3.7/xml/sax/expatreader.py", line 370, in start_element_ns
    AttributesNSImpl(newattrs, qnames))
  File "//src/zope/configuration/xmlconfig.py", line 213, in startElementNS
    None, sys.exc_info()[2])
  File "//src/zope/configuration/_compat.py", line 38, in reraise
    raise value.with_traceback(tb)
  File "//src/zope/configuration/xmlconfig.py", line 204, in startElementNS
    self.context.begin(name, data, info)
  File "//src/zope/configuration/config.py", line 349, in begin
    self.stack.append(self.stack[-1].contained(__name, __data, __info))
  File "//src/zope/configuration/config.py", line 516, in contained
    return RootStackItem.contained(self, name, data, info)
  File "//src/zope/configuration/config.py", line 480, in contained
    factory = self.context.factory(self.context, name)
  File "/tmp/demo.py", line 37, in __call__
    raise Exception("You rang?")
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "<string>", line 20.6
    Exception: You rang?

The suggested fix (looking up the context stack for IConfigurationContext) still fails with the same error, because SimpleParentDirective extends GroupingContextDecorator which is a IConfigurationContext. But that interface doesn't define factory (in fact, no interface defines factory ). The mixin class ConfigurationAdapterRegistry is what defines factory; ConfigurationMachine extends that class.

So in RootStackItem we can define

def __getConfigurationContext(self):
    context = self.context
    while not isinstance(context, ConfigurationAdapterRegistry) and hasattr(context, 'context'):
        context = context.context
    return context

to fix this direct problem (all the tests pass with that change).

Now we run into problems with SimpleChildDirective: it's the handler for the SimpleStackItem, and it can't be called with arguments (it wants to get passed a GroupingContextDecorator object); if we adjust that, then it fails because it's not iterable (handler is supposed to return a list of actions). I'd have to refresh my knowledge about how grouping directives are supposed to work to understand if these are legitimate issues in the demo code.

As noted, ConfigurationContext.action is another attribute expected to be found on the context (by SimpleStackItem), it would need similar treatment if this is indeed a correct fix.

jamadden commented 5 years ago

It's not clear to me if this is a bug or a feature. On the one hand, the attributes the StackItems are looking for are undocumented, internal implementation details, meaning that a custom grouping context that accidentally overrides them (as in the example) is not "doing anything wrong" and still gets in trouble.

On the other hand, a custom grouping context could potentially deliberately override them to do something custom and interesting. In fact, just the other day, I saw a generic grouping directive that overrides __getattr__ to watch for factory and wraps that callable in another callable that invokes a debugger. Such code may be relying on implementation details, but they've been solid implementation details for more than a decade, and at least one of them ('action') is partially documented (though its interaction here isn't).

Maybe the best we can do here is document these details? Or prohibit grouping directives from declaring factory and action in their schema?