Move view derivation code into a function.

2 files changed, 265 insertions, 161 deletions

diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py
index b93788663..d537cf581 100644
--- a/repoze/bfg/tests/test_zcml.py
+++ b/repoze/bfg/tests/test_zcml.py
@@ -520,6 +520,101 @@ class TestViewDirective(unittest.TestCase):
         self.assertRaises(ConfigurationError, self._callFUT, context,
                           'repoze.view', None, view, '', None, 'foo')
 
+class TestDeriveView(unittest.TestCase):
+    def _callFUT(self, view):
+        from repoze.bfg.zcml import derive_view
+        return derive_view(view)
+    
+    def test_view_as_function_context_and_request(self):
+        def view(context, request):
+            return 'OK'
+        result = self._callFUT(view)
+        self.failUnless(result is view)
+        self.assertEqual(view(None, None), 'OK')
+        
+    def test_view_as_function_requestonly(self):
+        def view(request):
+            return 'OK'
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.assertEqual(view.__name__, result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+
+    def test_view_as_newstyle_class_context_and_request(self):
+        class view(object):
+            def __init__(self, context, request):
+                pass
+            def __call__(self):
+                return 'OK'
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.assertEqual(view.__name__, result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+        
+    def test_view_as_newstyle_class_requestonly(self):
+        class view(object):
+            def __init__(self, context, request):
+                pass
+            def __call__(self):
+                return 'OK'
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.assertEqual(view.__name__, result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+
+    def test_view_as_oldstyle_class_context_and_request(self):
+        class view:
+            def __init__(self, context, request):
+                pass
+            def __call__(self):
+                return 'OK'
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.assertEqual(view.__name__, result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+        
+    def test_view_as_oldstyle_class_requestonly(self):
+        class view:
+            def __init__(self, context, request):
+                pass
+            def __call__(self):
+                return 'OK'
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.assertEqual(view.__name__, result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+
+    def test_view_as_instance_context_and_request(self):
+        class View:
+            def __call__(self, context, request):
+                return 'OK'
+        view = View()
+        result = self._callFUT(view)
+        self.failUnless(result is view)
+        self.assertEqual(result(None, None), 'OK')
+        
+    def test_view_as_instance_requestonly(self):
+        class View:
+            def __call__(self, request):
+                return 'OK'
+        view = View()
+        result = self._callFUT(view)
+        self.failIf(result is view)
+        self.assertEqual(view.__module__, result.__module__)
+        self.assertEqual(view.__doc__, result.__doc__)
+        self.failUnless('instance' in result.__name__)
+        self.assertEqual(result(None, None), 'OK')
+
 class TestRouteRequirementFunction(unittest.TestCase):
     def _callFUT(self, context, attr, expr):
         from repoze.bfg.zcml import route_requirement
diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py
index 80f8f1f38..e53a359da 100644
--- a/repoze/bfg/zcml.py
+++ b/repoze/bfg/zcml.py
@@ -64,8 +64,28 @@ def view(
     else:
         request_type = _context.resolve(request_type)
 
-    derived_view = view
+    derived_view = derive_view(view)
 
+    if permission:
+        pfactory = ViewPermissionFactory(permission)
+        _context.action(
+            discriminator = ('permission', for_, name, request_type,
+                             IViewPermission),
+            callable = handler,
+            args = ('registerAdapter',
+                    pfactory, (for_, request_type), IViewPermission, name,
+                    _context.info),
+            )
+
+    _context.action(
+        discriminator = ('view', for_, name, request_type, IView),
+        callable = handler,
+        args = ('registerAdapter',
+                derived_view, (for_, request_type), IView, name, _context.info),
+        )
+
+def derive_view(view):
+    derived_view = view
     if inspect.isclass(view):
         # If the object we've located is a class, turn it into a
         # function that operates like a Zope view (when it's invoked,
@@ -74,17 +94,22 @@ def view(
         # method of the instance with no arguments; __call__ should
         # return an IResponse).
         if requestonly(view):
+            # its __init__ accepts only a single request argument,
+            # instead of both context and request
             def _bfg_class_requestonly_view(context, request):
                 inst = view(request)
                 return inst()
             derived_view = _bfg_class_requestonly_view
         else:
+            # its __init__ accepts both context and request
             def _bfg_class_view(context, request):
                 inst = view(context, request)
                 return inst()
             derived_view = _bfg_class_view
 
     elif requestonly(view):
+        # its __call__ accepts only a single request argument,
+        # instead of both context and request
         def _bfg_requestonly_view(context, request):
             return view(request)
         derived_view = _bfg_requestonly_view
@@ -97,78 +122,8 @@ def view(
         except AttributeError:
             derived_view.__name__ = repr(view)
 
-    if permission:
-        pfactory = ViewPermissionFactory(permission)
-        _context.action(
-            discriminator = ('permission', for_, name, request_type,
-                             IViewPermission),
-            callable = handler,
-            args = ('registerAdapter',
-                    pfactory, (for_, request_type), IViewPermission, name,
-                    _context.info),
-            )
-
-    _context.action(
-        discriminator = ('view', for_, name, request_type, IView),
-        callable = handler,
-        args = ('registerAdapter',
-                derived_view, (for_, request_type), IView, name, _context.info),
-        )
-
-class IViewDirective(Interface):
-    for_ = GlobalObject(
-        title=u"The interface or class this view is for.",
-        required=False
-        )
-
-    permission = TextLine(
-        title=u"Permission",
-        description=u"The permission needed to use the view.",
-        required=False
-        )
-
-    view = GlobalObject(
-        title=u"",
-        description=u"The view function",
-        required=False,
-        )
-
-    name = TextLine(
-        title=u"The name of the view",
-        description=u"""
-        The name shows up in URLs/paths. For example 'foo' or
-        'foo.html'.""",
-        required=False,
-        )
-
-    request_type = TextLine(
-        title=u"The request type string or dotted name interface for the view",
-        description=(u"The view will be called if the interface represented by "
-                     u"'request_type' is implemented by the request.  The "
-                     u"default request type is repoze.bfg.interfaces.IRequest"),
-        required=False
-        )
-
-    route_name = TextLine(
-        title = u'The route that must match for this view to be used',
-        required = False)
-
-
-def zcml_configure(name, package):
-    context = zope.configuration.config.ConfigurationMachine()
-    xmlconfig.registerCommonDirectives(context)
-    context.package = package
-    xmlconfig.include(context, name, package)
-    context.execute_actions(clear=False)
-    return context.actions
-
-file_configure = zcml_configure # backwards compat (>0.8.1)
-
-def exclude(name):
-    if name.startswith('.'):
-        return True
-    return False
-
+    return derived_view
+    
 def scan(_context, package, martian=martian):
     # martian overrideable only for unit tests
     module_grokker = martian.ModuleGrokker()
@@ -176,40 +131,6 @@ def scan(_context, package, martian=martian):
     martian.grok_dotted_name(package.__name__, grokker=module_grokker,
                              context=_context, exclude_filter=exclude)
 
-class IScanDirective(Interface):
-    package = GlobalObject(
-        title=u"The package we'd like to scan.",
-        required=True,
-        )
-
-class BFGViewFunctionGrokker(martian.InstanceGrokker):
-    martian.component(types.FunctionType)
-
-    def grok(self, name, obj, **kw):
-        if hasattr(obj, '__is_bfg_view__'):
-            permission = obj.__permission__
-            for_ = obj.__for__
-            name = obj.__view_name__
-            request_type = obj.__request_type__
-            route_name = obj.__route_name__
-            context = kw['context']
-            view(context, permission=permission, for_=for_,
-                 view=obj, name=name, request_type=request_type,
-                 route_name=route_name)
-            return True
-        return False
-
-class IRouteRequirementDirective(Interface):
-    """ The interface for the ``requirement`` route subdirective """
-    attr = TextLine(title=u'attr', required=True)
-    expr = TextLine(title=u'expression', required=True)
-
-def route_requirement(context, attr, expr):
-    route = context.context
-    if attr in route.requirements:
-        raise ValueError('Duplicate requirement', attr)
-    route.requirements[attr] = expr
-
 class IRouteDirective(Interface):
     """ The interface for the ``route`` ZCML directive
     """
@@ -237,57 +158,6 @@ class IRouteDirective(Interface):
     subdomains = Tokens(title=u'subdomains', required=False,
                         value_type=TextLine())
 
-def connect_route(directive):
-    mapper = queryUtility(IRoutesMapper)
-    if mapper is None:
-        return
-    args = [directive.name, directive.path]
-    kw = dict(requirements=directive.requirements)
-    if directive.minimize:
-        kw['_minimize'] = True
-    if directive.explicit:
-        kw['_explicit'] = True
-    if directive.encoding:
-        kw['_encoding'] = directive.encoding
-    if directive.static:
-        kw['_static'] = True
-    if directive.filter:
-        kw['_filter'] = directive.filter
-    if directive.absolute:
-        kw['_absolute'] = True
-    if directive.member_name:
-        kw['_member_name'] = directive.member_name
-    if directive.collection_name:
-        kw['_collection_name'] = directive.collection_name
-    if directive.parent_member_name and directive.parent_collection_name:
-            kw['_parent_resource'] = {
-                'member_name':directive.parent_member_name,
-                'collection_name':directive.parent_collection_name,
-                }
-    conditions = {}
-
-    # request_type and condition_method are aliases; condition_method
-    # "wins"
-    if directive.request_type:
-        conditions['method'] = directive.request_type
-    if directive.condition_method:
-        conditions['method'] = directive.condition_method
-    if directive.condition_subdomain:
-        conditions['sub_domain'] = directive.condition_subdomain
-    if directive.condition_function:
-        conditions['function'] = directive.condition_function
-    if directive.subdomains:
-        conditions['sub_domain'] = directive.subdomains
-    if conditions:
-        kw['conditions'] = conditions
-
-    result = mapper.connect(*args, **kw)
-    route = mapper.matchlist[-1]
-    route._factory = directive.factory
-    context = directive.context
-    route.request_factories = context.request_factories[directive.name]
-    return result
-
 class Route(zope.configuration.config.GroupingContextDecorator):
     """ Handle ``route`` ZCML directives
     """
@@ -351,9 +221,143 @@ class Route(zope.configuration.config.GroupingContextDecorator):
             args = (self,),
             )
 
-class Uncacheable(object):
-    """ Include in discriminators of actions which are not cacheable;
-    this class only exists for backwards compatibility (<0.8.1)"""
+def route_requirement(context, attr, expr):
+    route = context.context
+    if attr in route.requirements:
+        raise ValueError('Duplicate requirement', attr)
+    route.requirements[attr] = expr
+
+def connect_route(directive):
+    mapper = queryUtility(IRoutesMapper)
+    if mapper is None:
+        return
+    args = [directive.name, directive.path]
+    kw = dict(requirements=directive.requirements)
+    if directive.minimize:
+        kw['_minimize'] = True
+    if directive.explicit:
+        kw['_explicit'] = True
+    if directive.encoding:
+        kw['_encoding'] = directive.encoding
+    if directive.static:
+        kw['_static'] = True
+    if directive.filter:
+        kw['_filter'] = directive.filter
+    if directive.absolute:
+        kw['_absolute'] = True
+    if directive.member_name:
+        kw['_member_name'] = directive.member_name
+    if directive.collection_name:
+        kw['_collection_name'] = directive.collection_name
+    if directive.parent_member_name and directive.parent_collection_name:
+            kw['_parent_resource'] = {
+                'member_name':directive.parent_member_name,
+                'collection_name':directive.parent_collection_name,
+                }
+    conditions = {}
+
+    # request_type and condition_method are aliases; condition_method
+    # "wins"
+    if directive.request_type:
+        conditions['method'] = directive.request_type
+    if directive.condition_method:
+        conditions['method'] = directive.condition_method
+    if directive.condition_subdomain:
+        conditions['sub_domain'] = directive.condition_subdomain
+    if directive.condition_function:
+        conditions['function'] = directive.condition_function
+    if directive.subdomains:
+        conditions['sub_domain'] = directive.subdomains
+    if conditions:
+        kw['conditions'] = conditions
+
+    result = mapper.connect(*args, **kw)
+    route = mapper.matchlist[-1]
+    route._factory = directive.factory
+    context = directive.context
+    route.request_factories = context.request_factories[directive.name]
+    return result
+
+class IViewDirective(Interface):
+    for_ = GlobalObject(
+        title=u"The interface or class this view is for.",
+        required=False
+        )
+
+    permission = TextLine(
+        title=u"Permission",
+        description=u"The permission needed to use the view.",
+        required=False
+        )
+
+    view = GlobalObject(
+        title=u"",
+        description=u"The view function",
+        required=False,
+        )
+
+    name = TextLine(
+        title=u"The name of the view",
+        description=u"""
+        The name shows up in URLs/paths. For example 'foo' or
+        'foo.html'.""",
+        required=False,
+        )
+
+    request_type = TextLine(
+        title=u"The request type string or dotted name interface for the view",
+        description=(u"The view will be called if the interface represented by "
+                     u"'request_type' is implemented by the request.  The "
+                     u"default request type is repoze.bfg.interfaces.IRequest"),
+        required=False
+        )
+
+    route_name = TextLine(
+        title = u'The route that must match for this view to be used',
+        required = False)
+
+class IRouteRequirementDirective(Interface):
+    """ The interface for the ``requirement`` route subdirective """
+    attr = TextLine(title=u'attr', required=True)
+    expr = TextLine(title=u'expression', required=True)
+
+class IScanDirective(Interface):
+    package = GlobalObject(
+        title=u"The package we'd like to scan.",
+        required=True,
+        )
+
+def zcml_configure(name, package):
+    context = zope.configuration.config.ConfigurationMachine()
+    xmlconfig.registerCommonDirectives(context)
+    context.package = package
+    xmlconfig.include(context, name, package)
+    context.execute_actions(clear=False)
+    return context.actions
+
+file_configure = zcml_configure # backwards compat (>0.8.1)
+
+class BFGViewFunctionGrokker(martian.InstanceGrokker):
+    martian.component(types.FunctionType)
+
+    def grok(self, name, obj, **kw):
+        if hasattr(obj, '__is_bfg_view__'):
+            permission = obj.__permission__
+            for_ = obj.__for__
+            name = obj.__view_name__
+            request_type = obj.__request_type__
+            route_name = obj.__route_name__
+            context = kw['context']
+            view(context, permission=permission, for_=for_,
+                 view=obj, name=name, request_type=request_type,
+                 route_name=route_name)
+            return True
+        return False
+
+def exclude(name):
+    if name.startswith('.'):
+        return True
+    return False
 
 def requestonly(class_or_callable):
     """ Return true of the class or callable accepts only a request argument,
@@ -395,3 +399,8 @@ def requestonly(class_or_callable):
             return True
 
     return False
+
+class Uncacheable(object):
+    """ Include in discriminators of actions which are not cacheable;
+    this class only exists for backwards compatibility (<0.8.1)"""
+