Behavior Changes

----------------

- The ``repoze.bfg.view.render_view_to_response`` API will no longer
  raise a ValueError if an object returned by a view function it calls
  does not possess certain attributes (``headerlist``, ``app_iter``,
  ``status``).  This API used to attempt to perform a check using the
  ``is_response`` function in ``repoze.bfg.view``, and raised a
  ``ValueError`` if the ``is_response`` check failed.  The
  responsibility is now the caller's to ensure that the return value
  from a view function is a "real" response.

- WSGI environ dicts passed to ``repoze.bfg`` 's Router must now
  contain a REQUEST_METHOD key/value; if they do not, a KeyError will
  be raised (speed).  

Implementation Changes
----------------------

- Various speed micro-tweaks.

9 files changed, 179 insertions, 166 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 05f116a01..ab8b6febe 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,26 @@
 Next Release
 
+Behavior Changes
+----------------
+
+- The ``repoze.bfg.view.render_view_to_response`` API will no longer
+  raise a ValueError if an object returned by a view function it calls
+  does not possess certain attributes (``headerlist``, ``app_iter``,
+  ``status``).  This API used to attempt to perform a check using the
+  ``is_response`` function in ``repoze.bfg.view``, and raised a
+  ``ValueError`` if the ``is_response`` check failed.  The
+  responsibility is now the caller's to ensure that the return value
+  from a view function is a "real" response.
+
+- WSGI environ dicts passed to ``repoze.bfg`` 's Router must now
+  contain a REQUEST_METHOD key/value; if they do not, a KeyError will
+  be raised (speed).  
+
+Implementation Changes
+----------------------
+
+- Various speed micro-tweaks.
+
 Bug Fixes
 ---------
 
diff --git a/repoze/bfg/registry.py b/repoze/bfg/registry.py
index 26feb73e0..41012a7c9 100644
--- a/repoze/bfg/registry.py
+++ b/repoze/bfg/registry.py
@@ -28,20 +28,20 @@ class Registry(Components):
 
     # for optimization purposes, if no listeners are listening, don't try
     # to notify them
-    _has_listeners = False
+    has_listeners = False
 
     def registerSubscriptionAdapter(self, *arg, **kw):
         result = Components.registerSubscriptionAdapter(self, *arg, **kw)
-        self._has_listeners = True
+        self.has_listeners = True
         return result
         
     def registerHandler(self, *arg, **kw):
         result = Components.registerHandler(self, *arg, **kw)
-        self._has_listeners = True
+        self.has_listeners = True
         return result
 
     def notify(self, *events):
-        if self._has_listeners:
+        if self.has_listeners:
             # iterating over subscribers assures they get executed
             for ignored in self.subscribers(events, None):
                 """ """
diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py
index 05236cfed..c9366477c 100644
--- a/repoze/bfg/router.py
+++ b/repoze/bfg/router.py
@@ -18,7 +18,10 @@ from repoze.bfg.interfaces import IRootFactory
 from repoze.bfg.interfaces import IRouter
 from repoze.bfg.interfaces import IRoutesMapper
 from repoze.bfg.interfaces import ITraverserFactory
+from repoze.bfg.interfaces import ISecurityPolicy
 from repoze.bfg.interfaces import ISettings
+from repoze.bfg.interfaces import IView
+from repoze.bfg.interfaces import IViewPermission
 
 from repoze.bfg.log import make_stream_logger
 
@@ -33,10 +36,11 @@ from repoze.bfg.settings import Settings
 
 from repoze.bfg.urldispatch import RoutesRootFactory
 
-from repoze.bfg.view import render_view_to_response
-from repoze.bfg.view import view_execution_permitted
+from repoze.bfg.view import _view_execution_permitted
 
-_marker = ()
+_marker = object()
+
+# 95090 function calls (95087 primitive calls) in 0.277 CPU seconds
 
 class Router(object):
     """ The main repoze.bfg WSGI application. """
@@ -44,6 +48,10 @@ class Router(object):
     
     def __init__(self, registry):
         self.registry = registry
+        self.settings = registry.queryUtility(ISettings)
+        self.request_factory = registry.queryUtility(IRequestFactory)
+        self.security_policy = registry.queryUtility(ISecurityPolicy)
+        self.logger = registry.queryUtility(ILogger, 'repoze.bfg.debug')
 
     @property
     def root_policy(self):
@@ -53,23 +61,25 @@ class Router(object):
     def __call__(self, environ, start_response):
         """
         Accept ``environ`` and ``start_response``; route requests to
-        'view' code based on registrations within the application
-        registry; call ``start_response`` and return an iterable.
+        ``repoze.bfg`` views based on registrations within the
+        application registry; call ``start_response`` and return an
+        iterable.
         """
-        reg = self.registry
-        registry_manager.push(reg)
+        registry = self.registry
+        registry_manager.push(registry)
 
         try:
-            request_factory = reg.queryUtility(IRequestFactory)
+            request_factory = self.request_factory
             if request_factory is None:
-                method = environ.get('REQUEST_METHOD', 'GET')
+                method = environ['REQUEST_METHOD']
                 request_factory = HTTP_METHOD_FACTORIES.get(method, Request)
-            request = request_factory(environ)
 
-            reg.notify(NewRequest(request))
-            root_factory = reg.getUtility(IRootFactory)
+            request = request_factory(environ)
+            
+            registry.has_listeners and registry.notify(NewRequest(request))
+            root_factory = registry.getUtility(IRootFactory)
             root = root_factory(environ)
-            traverser = reg.getAdapter(root, ITraverserFactory)
+            traverser = registry.getAdapter(root, ITraverserFactory)
             context, view_name, subpath = traverser(environ)
 
             # XXX webob.Request's __setattr__ is slow here: investigate.
@@ -78,36 +88,44 @@ class Router(object):
             request.view_name = view_name
             request.subpath = subpath
 
-            permitted = view_execution_permitted(context, request, view_name)
+            settings = self.settings
 
-            settings = reg.queryUtility(ISettings)
             debug_authorization = settings and settings.debug_authorization
+            security_policy = self.security_policy
+
+            permission = None
+
+            if security_policy is not None:
+                permission = registry.queryMultiAdapter((context, request),
+                                                        IViewPermission,
+                                                        name=view_name)
+
+            permitted = _view_execution_permitted(context, request, view_name,
+                                                  security_policy, permission,
+                                                  debug_authorization)
 
-            logger = None
+            logger = self.logger
 
             if debug_authorization:
-                logger = reg.queryUtility(ILogger, 'repoze.bfg.debug')
                 logger and logger.debug(
                     'debug_authorization of url %s (view name %r against '
                     'context %r): %s' % (
-                    request.url, view_name, context, permitted.msg)
+                    request.url, view_name, context, permitted)
                     )
             if not permitted:
                 if debug_authorization:
-                    msg = permitted.msg
+                    msg = str(permitted)
                 else:
                     msg = 'Unauthorized: failed security policy check'
                 app = HTTPUnauthorized(escape(msg))
                 return app(environ, start_response)
 
-            response = render_view_to_response(context, request, view_name,
-                                               secure=False)
+            response = registry.queryMultiAdapter(
+                (context, request), IView, name=view_name)
 
             if response is None:
                 debug_notfound = settings and settings.debug_notfound
                 if debug_notfound:
-                    if logger is None:
-                        logger = reg.queryUtility(ILogger, 'repoze.bfg.debug')
                     msg = (
                         'debug_notfound of url %s; path_info: %r, context: %r, '
                         'view_name: %r, subpath: %r' % (
@@ -120,10 +138,14 @@ class Router(object):
                 app = HTTPNotFound(escape(msg))
                 return app(environ, start_response)
 
-            reg.notify(NewResponse(response))
+            registry.has_listeners and registry.notify(NewResponse(response))
 
-            start_response(response.status, response.headerlist)
-            return response.app_iter
+            try:
+                start_response(response.status, response.headerlist)
+                return response.app_iter
+            except AttributeError:
+                raise ValueError(
+                    'Non-response object returned from view: %r' % response)
 
         finally:
             registry_manager.pop()
diff --git a/repoze/bfg/security.py b/repoze/bfg/security.py
index ccc223919..4e61b0ed9 100644
--- a/repoze/bfg/security.py
+++ b/repoze/bfg/security.py
@@ -217,7 +217,17 @@ def RepozeWhoIdentityACLSecurityPolicy():
     """
     return ACLSecurityPolicy(get_who_principals)
 
-class PermitsResult:
+class PermitsResult(int):
+    def __new__(cls, s, *args):
+        inst = int.__new__(cls, cls.boolval)
+        inst.s = s
+        inst.args = args
+        return inst
+        
+    @property
+    def msg(self):
+        return self.s % self.args
+
     def __str__(self):
         return self.msg
 
@@ -231,19 +241,7 @@ class Denied(PermitsResult):
     or other ``repoze.bfg`` code denies an action unlrelated to an ACL
     check.  It evaluates equal to all boolean false types.  It has an
     attribute named ``msg`` describing the circumstances for the deny."""
-    def __init__(self, s, *args):
-        self.s = s
-        self.args = args
-
-    @property
-    def msg(self):
-        return self.s % self.args
-
-    def __nonzero__(self):
-        return False
-
-    def __eq__(self, other):
-        return bool(other) is False
+    boolval = 0
 
 class Allowed(PermitsResult):
     """ An instance of ``Allowed`` is returned when a security policy
@@ -251,27 +249,17 @@ class Allowed(PermitsResult):
     check.  It evaluates equal to all boolean true types.  It has an
     attribute named ``msg`` describing the circumstances for the
     allow."""
-    def __init__(self, s, *args):
-        self.s = s
-        self.args = args
-
-    @property
-    def msg(self):
-        return self.s % self.args
-
-    def __nonzero__(self):
-        return True
-
-    def __eq__(self, other):
-        return bool(other) is True
-
-class ACLPermitsResult:
-    def __init__(self, ace, acl, permission, principals, context):
-        self.permission = permission
-        self.ace = ace
-        self.acl = acl
-        self.principals = principals
-        self.context = context
+    boolval = 1
+
+class ACLPermitsResult(int):
+    def __new__(cls, ace, acl, permission, principals, context):
+        inst = int.__new__(cls, cls.boolval)
+        inst.permission = permission
+        inst.ace = ace
+        inst.acl = acl
+        inst.principals = principals
+        inst.context = context
+        return inst
 
     @property
     def msg(self):
@@ -284,7 +272,15 @@ class ACLPermitsResult:
                     self.context,
                     self.principals)
 
-class ACLDenied(ACLPermitsResult, Denied):
+    def __str__(self):
+        return self.msg
+
+    def __repr__(self):
+        return '<%s instance at %s with msg %r>' % (self.__class__.__name__,
+                                                    id(self),
+                                                    self.msg)
+
+class ACLDenied(ACLPermitsResult):
     """ An instance of ``ACLDenied`` represents that a security check
     made explicitly against ACL was denied.  It evaluates equal to all
     boolean false types.  It also has attributes which indicate which
@@ -292,8 +288,9 @@ class ACLDenied(ACLPermitsResult, Denied):
     request.  Its __str__ method prints a summary of these attributes
     for debugging purposes.  The same summary is available as he
     ``msg`` attribute."""
+    boolval = 0
 
-class ACLAllowed(ACLPermitsResult, Allowed):
+class ACLAllowed(ACLPermitsResult):
     """ An instance of ``ACLDenied`` represents that a security check
     made explicitly against ACL was allowed.  It evaluates equal to
     all boolean true types.  It also has attributes which indicate
@@ -301,6 +298,7 @@ class ACLAllowed(ACLPermitsResult, Allowed):
     in the request.  Its __str__ method prints a summary of these
     attributes for debugging purposes.  The same summary is available
     as he ``msg`` attribute."""
+    boolval = 1
 
 def flatten(x):
     """flatten(sequence) -> list
diff --git a/repoze/bfg/tests/test_registry.py b/repoze/bfg/tests/test_registry.py
index f54ac934f..2f7123cff 100644
--- a/repoze/bfg/tests/test_registry.py
+++ b/repoze/bfg/tests/test_registry.py
@@ -12,7 +12,7 @@ class TestRegistry(unittest.TestCase):
 
     def test_registerHandler_and_notify(self):
         registry = self._makeOne()
-        self.assertEqual(registry._has_listeners, False)
+        self.assertEqual(registry.has_listeners, False)
         from zope.interface import Interface
         from zope.interface import implements
         class IFoo(Interface):
@@ -23,21 +23,21 @@ class TestRegistry(unittest.TestCase):
         def f(event):
             L.append(event)
         registry.registerHandler(f, [IFoo])
-        self.assertEqual(registry._has_listeners, True)
+        self.assertEqual(registry.has_listeners, True)
         event = FooEvent()
         registry.notify(event)
         self.assertEqual(L, [event])
 
     def test_registerSubscriptionAdapter_and_notify(self):
         registry = self._makeOne()
-        self.assertEqual(registry._has_listeners, False)
+        self.assertEqual(registry.has_listeners, False)
         from zope.interface import Interface
         class EventHandler:
             pass
         class IFoo(Interface):
             pass
         registry.registerSubscriptionAdapter(EventHandler, [IFoo], Interface)
-        self.assertEqual(registry._has_listeners, True)
+        self.assertEqual(registry.has_listeners, True)
 
 class TestPopulateRegistry(unittest.TestCase):
     def setUp(self):
diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py
index 8c2a5cf04..c17bc623a 100644
--- a/repoze/bfg/tests/test_router.py
+++ b/repoze/bfg/tests/test_router.py
@@ -180,6 +180,19 @@ class RouterTests(unittest.TestCase):
         self.failUnless("view_name: ''" in message)
         self.failUnless("subpath: []" in message)
 
+    def test_call_view_returns_nonresponse(self):
+        rootfactory = make_rootfactory(None)
+        context = DummyContext()
+        traversalfactory = make_traversal_factory(context, '', [])
+        environ = self._makeEnviron()
+        self._registerTraverserFactory(traversalfactory, '', None)
+        view = make_view('abc')
+        self._registerView(view, '', None, None)
+        self._registerRootFactory(rootfactory)
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        self.assertRaises(ValueError, router, environ, start_response)
+
     def test_call_view_registered_nonspecific_default_path(self):
         rootfactory = make_rootfactory(None)
         context = DummyContext()
diff --git a/repoze/bfg/tests/test_view.py b/repoze/bfg/tests/test_view.py
index 9bdc7d80a..e32a04deb 100644
--- a/repoze/bfg/tests/test_view.py
+++ b/repoze/bfg/tests/test_view.py
@@ -123,26 +123,6 @@ class RenderViewToResponseTests(BaseTest, unittest.TestCase):
                                  secure=False)
         self.assertEqual(response.status, '200 OK')
 
-
-    def test_call_view_response_doesnt_implement_IResponse(self):
-        context = DummyContext()
-        from zope.interface import Interface
-        from zope.interface import directlyProvides
-        from repoze.bfg.interfaces import IRequest
-        class IContext(Interface):
-            pass
-        directlyProvides(context, IContext)
-        response = 'abc'
-        view = make_view(response)
-        self._registerView(view, 'registered', IContext, IRequest)
-        environ = self._makeEnviron()
-        from webob import Request
-        request = Request(environ)
-        directlyProvides(request, IRequest)
-        self.assertRaises(ValueError, self._callFUT, context, request,
-                          name='registered', secure=False)
-
-
 class RenderViewToIterableTests(BaseTest, unittest.TestCase):
     def _callFUT(self, *arg, **kw):
         from repoze.bfg.view import render_view_to_iterable
@@ -228,24 +208,6 @@ class RenderViewToIterableTests(BaseTest, unittest.TestCase):
                                  secure=False)
         self.assertEqual(iterable, ())
 
-    def test_call_view_response_doesnt_implement_IResponse(self):
-        context = DummyContext()
-        from zope.interface import Interface
-        from zope.interface import directlyProvides
-        from repoze.bfg.interfaces import IRequest
-        class IContext(Interface):
-            pass
-        directlyProvides(context, IContext)
-        response = 'abc'
-        view = make_view(response)
-        self._registerView(view, 'registered', IContext, IRequest)
-        environ = self._makeEnviron()
-        from webob import Request
-        request = Request(environ)
-        directlyProvides(request, IRequest)
-        self.assertRaises(ValueError, self._callFUT, context, request,
-                          name='registered', secure=False)
-
 class RenderViewTests(unittest.TestCase, BaseTest):
     def _callFUT(self, *arg, **kw):
         from repoze.bfg.view import render_view
@@ -329,24 +291,6 @@ class RenderViewTests(unittest.TestCase, BaseTest):
         s = self._callFUT(context, request, name='registered', secure=False)
         self.assertEqual(s, '')
 
-    def test_call_view_response_doesnt_implement_IResponse(self):
-        context = DummyContext()
-        from zope.interface import Interface
-        from zope.interface import directlyProvides
-        from repoze.bfg.interfaces import IRequest
-        class IContext(Interface):
-            pass
-        directlyProvides(context, IContext)
-        response = 'abc'
-        view = make_view(response)
-        self._registerView(view, 'registered', IContext, IRequest)
-        environ = self._makeEnviron()
-        from webob import Request
-        request = Request(environ)
-        directlyProvides(request, IRequest)
-        self.assertRaises(ValueError, self._callFUT, context, request,
-                          name='registered', secure=False)
-
 class TestIsResponse(unittest.TestCase):
     def _callFUT(self, *arg, **kw):
         from repoze.bfg.view import is_response
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index 4c1cb0eb3..95a0e88e7 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -131,8 +131,12 @@ class ModelGraphTraverser(object):
         self.root = root
         self.locatable = ILocation.providedBy(root)
 
+ #61089 function calls (61086 primitive calls) in 0.191 CPU seconds
     def __call__(self, environ, _marker=_marker):
-        path = environ.get('PATH_INFO', '/')
+        try:
+            path = environ['PATH_INFO']
+        except KeyError:
+            path = '/'
         path = list(split_path(path))
         locatable = self.locatable
         step = self._step
diff --git a/repoze/bfg/view.py b/repoze/bfg/view.py
index 8ed23748c..0e4e47f5d 100644
--- a/repoze/bfg/view.py
+++ b/repoze/bfg/view.py
@@ -21,31 +21,49 @@ def view_execution_permitted(context, request, name=''):
     by a permission, return the result of checking the permission
     associated with the view using the effective security policy and
     the ``request``.  If no security policy is in effect, or if the
-    view is not protected by a permission, return a True value. """
+    view is not protected by a permission, return True. """
     security_policy = queryUtility(ISecurityPolicy)
-    if security_policy is not None:
-        permission = queryMultiAdapter((context, request), IViewPermission,
-                                       name=name)
-        if permission is None:
+    permission = queryMultiAdapter((context, request), IViewPermission,
+                                   name=name)
+    return _view_execution_permitted(context, request, name, security_policy,
+                                     permission, True)
+
+def _view_execution_permitted(context, request, view_name, security_policy,
+                              permission, debug_authorization):
+    """ Rawer (faster) form of view_execution_permitted which does not
+    need to do a CA lookup for the security policy or other values and
+    which returns plain booleans if debug_authorization is off instead
+    of constructing ``Allowed`` objects.  This function is used by
+    ``view_execution_permitted`` and the Router; it is not a public
+    API."""
+    if security_policy is None:
+        if debug_authorization:
+            return Allowed(
+                'Allowed: view name %r in context %r (no security policy in '
+                'use)', view_name, context)
+        else:
+            return True
+
+    elif permission is None:
+        if debug_authorization:
             return Allowed(
                 'Allowed: view name %r in context %r (no permission '
-                'registered for name %r).', name, context, name)
+                'registered for name %r).', view_name, context, view_name)
+        else:
+            return True
+
+    else:
         return permission(security_policy)
-    return Allowed('Allowed: view name %r in context %r (no security policy '
-                   'in use).', name, context)
 
 def render_view_to_response(context, request, name='', secure=True):
     """ Render the view named ``name`` against the specified
     ``context`` and ``request`` to an object implementing
     ``repoze.bfg.interfaces.IResponse`` or ``None`` if no such view
     exists.  This function will return ``None`` if a corresponding
-    view cannot be found.  Additionally, this function will raise a
-    ``ValueError`` if a view function is found and called but the view
-    returns an object which does not implement
-    ``repoze.bfg.interfaces.IResponse``.  If ``secure`` is ``True``,
-    and the view is protected by a permission, the permission will be
-    checked before calling the view function.  If the permission check
-    disallows view execution (based on the current security policy), a
+    view cannot be found.  If ``secure`` is ``True``, and the view is
+    protected by a permission, the permission will be checked before
+    calling the view function.  If the permission check disallows view
+    execution (based on the current security policy), a
     ``repoze.bfg.security.Unauthorized`` exception will be raised; its
     ``args`` attribute explains why the view access was disallowed.
     If ``secure`` is ``False``, no permission checking is done."""
@@ -53,17 +71,15 @@ def render_view_to_response(context, request, name='', secure=True):
         permitted = view_execution_permitted(context, request, name)
         if not permitted:
             raise Unauthorized(permitted)
+        
+    # It's no use trying to distinguish below whether response is None
+    # because a) we were returned a default or b) because the view
+    # function returned None: the zope.component/zope.interface
+    # machinery doesn't distinguish a None returned from the view from
+    # a sentinel None returned during queryMultiAdapter (even if we
+    # pass a non-None default).
 
-    response = queryMultiAdapter((context, request), IView, name=name,
-                                 default=_marker)
-
-    if response is _marker:
-        return None
-
-    if not is_response(response):
-        raise ValueError('response did not implement IResponse: %r' % response)
-
-    return response
+    return queryMultiAdapter((context, request), IView, name=name)
 
 def render_view_to_iterable(context, request, name='', secure=True):
     """ Render the view named ``name`` against the specified
@@ -116,19 +132,14 @@ def is_response(ob):
     duck-typing check, as response objects are not obligated to
     actually implement a Zope interface."""
     # response objects aren't obligated to implement a Zope interface,
-    # so we do it the hard way; this is written awkwardly for
-    # performance reasons
-    try:
-        ob.app_iter, ob.headerlist, ob.status
-    except AttributeError:
-        return False
-    try:
-        ob.app_iter.__iter__, ob.headerlist.__iter__
-    except AttributeError:
-        return False
-    if not isinstance(ob.status, basestring):
-        return False
-    return True
+    # so we do it the hard way
+    if ( hasattr(ob, 'app_iter') and hasattr(ob, 'headerlist') and
+         hasattr(ob, 'status') ):
+        if ( hasattr(ob.app_iter, '__iter__') and
+             hasattr(ob.headerlist, '__iter__') and
+             isinstance(ob.status, basestring) ) :
+            return True
+    return False
 
 class static(object):
     """ An instance of this class is a callable which can act as a BFG