Issue #1098 new

_cp_config + @cherrypy.tools == confusion

Joseph Tate
created an issue

If a method has both a _cp_config and a tools decorator, the decorators are ignored when the request is handled.

Here's a sample application:

import cherrypy
from cStringIO import StringIO

config = """
[global]
server.socket_port = 9939

"""

class Root(object):
    @cherrypy.expose
    @cherrypy.tools.json_out()
    def index(self):
        return {'a': 1, 'b': 2}

    @cherrypy.expose
    def foo(self):
        return {'a': 2, 'b': 3}
    foo._cp_config = {
        'tools.json_out.on': True,
        'tools.trailing_slash.on': True,
    }

    @cherrypy.expose
    @cherrypy.tools.json_out()
    def bar(self):
        return {'a': 2, 'b': 3}
    bar._cp_config = {
        'tools.trailing_slash.on': True,
    }

if __name__ == '__main__':
    cherrypy.config.update(StringIO(config))
    cherrypy.quickstart(Root())

I expected that foo and bar would return the same thing, but bar only returns the keys of the dictionary appended together, no json.

Here's a test case patch:

Index: cherrypy/test/test_tools.py
===================================================================
--- cherrypy/test/test_tools.py (revision 2832)
+++ cherrypy/test/test_tools.py (working copy)
@@ -200,6 +200,12 @@
                 for x in xrange(100000000):
                     yield str(x)
             stream._cp_config = {'response.stream': True}
+
+            # METHOD FOUR: Both a decorator and _cp_config (but without conf override)
+            def confusion(self):
+                return {'good': 'piece'}
+            confusion = cherrypy.tools.json_out()(confusion)
+            confusion._cp_config = {'tools.trailing_slash.on': True}


         conf = {
@@ -287,6 +293,10 @@
         # Test compile-time decorator with kwargs from config.
         self.getPage("/demo/userid")
         self.assertBody("Welcome!")
+
+        # Test that decorators AND _cp_config are applied
+        bb = self.getPage("/demo/confusion/")
+        assert bb.body == '{"horrorshow": "lomtick"}'

     def testEndRequestOnDrop(self):
         old_timeout = None

I can't move my _cp_config tool into a decorator because decorators have to be on the toolbox at load time for that to work, and mine isn't. However I can move the tools loaded via decorator into the _cp_config to work around the bug.

Comments (11)

  1. Anonymous

    What version of CherryPy is this patch against? I tried it with 102ee9f08271 and bb doesn't have a body attribute; bb is a tuple:

    ('200 OK',
     [('Date', 'Tue, 08 Nov 2011 19:21:37 GMT'),
      ('Content-Length', '10'),
      ('Content-Type', 'text/html;charset=utf-8'),
      ('Server', 'CherryPy/3.2.2')],
     'horrorshow')
    
    
  2. Anonymous

    Okay, made a little more progress. First, to get the test working we had to use this for an assertion (on the parallel of the other assertions in testHookErrors. BTW, seems to violate unit-testing zen to have more than one assertion per test, no? You're robbing yourself of dots. :) ):

    # Test that decorators AND _cp_config are applied
    self.getPage("/demo/confusion/")
    self.assertBody('{"horrorshow": "lomtick"}')  
    

    We determined the the iteration over cherrypy.response.body in the Nadsat tool defined in the test case is what results in the concatenated keys of the return dictionary showing up, since dict.iter gives you keys. We were able to get the expected body output by commenting out the confusion._cp_config assignment, and then we discovered a workaround: simply switch the order of the decoration and _cp_config assignment. This passes, in other words:

    confusion._cp_config = {'tools.trailing_slash.on': True}
    confusion = cherrypy.tools.json_out()(confusion)
    

    However, we didn't then test whether the trailing_slash behavior was as expected. Probably not?

    From there we descended into _cpdispatch, trying to untangle the interaction between _cp_config and method decoration. By moving the assertion to the top of the testHookErrors method we were able to more easily access node from pdb at _cpdispatch.py:336.

  3. Anonymous

    I should also say that the interactive session on a failing assertBody was a pleasant surprise.

    !m

  4. Anonymous

    There's a nose-related bug hidden in here. I've seen it in two environments and in two others I haven't. The bug manifests as a silent sys.exit() with code 70:

    (ve) $ nosetests -sx test_tools.py:ToolTests.testHookErrors
    (ve) $ echo $?
    70
    

    Nose does do some sort of exit value setting that I haven't tracked down yet. The CPWebCase.exit method is not involved.

    Nose exits before even reaching testHookErrors, though that test case does get collected (if the test were not collected for some reason one might expect similar behavior). It is something in ToolTests.setup_server that is causing nose to exit. We started whittling away at that mega-method to pinpoint the problem, but did not finish. If you see this behavior, that's where to pick up in debugging it, with an eye on Nose's use of sys.exit/SystemExit.

  5. Robert Brewer

    tool decorators work by updating func._cp_config, so if you then re-assign it after the function has been defined, you're out of luck. A more natural way to rewrite the above would be:

        @cherrypy.expose
        @cherrypy.tools.json_out()
        def bar(self):
            return {'a': 2, 'b': 3}
        bar._cp_config.update({
            'tools.trailing_slash.on': True,
        })
    
  6. Anonymous

    Why shouldn't we forgot about usage of _cp_config and use a tool like inline_config to get the same behavior of mixing inline_config + config.

    def inline_config(conf):
        def tool_decorator(f):
            if not hasattr(f, "_cp_config"):
                f._cp_config = {}
            for k, v in conf.items():
                f._cp_config[k] = v
            return f
        return tool_decorator
    cherrypy.tools.inline_config = inline_config
    

    Example:

    class Root(object):
        @cherrypy.expose
        @cherrypy.tools.inline_config({
                'tools.json_out.on': True,
                'tools.trailing_slash.on': True,
                })
        def foo(self):
            return {'a': 2, 'b': 3}
    
        @cherrypy.expose
        @cherrypy.tools.json_out()
        @cherrypy.tools.inline_config({
                    'tools.trailing_slash.on': True,
                    })
    
  7. Nicolae Dascalu

    Yes, and if the usage _cp_config is not deprecated, then at least should be document as a current limitation: _cp_config and decorators doesn't work simultaneous on same function/(or a callable class). Any way this combination of styles: decorator + _cp_config, is ugly, too, and should be discouraged:

        
        @cherrypy.expose
        @cherrypy.tools.json_out()
        def bar(self):
            return {'a': 2, 'b': 3}
        bar._cp_config = {
            'tools.trailing_slash.on': True,
        }
    
  8. Log in to comment