- Make Routes mapper responsible for doing magic to fix up PATH_INFO

  and SCRIPT_NAME when a ``path_info`` key exists in the matchdict.
  This used to be done in the traverser, which made no sense.

5 files changed, 72 insertions, 39 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index b49d0f204..fe210bc34 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -193,6 +193,13 @@ Documentation
 - Updated Routes bfgwiki2 tutorial to reflect the fact that context
   factories are now no longer used.
 
+Internal
+--------
+
+- Make Routes mapper responsible for doing magic to fix up PATH_INFO
+  and SCRIPT_NAME when a ``path_info`` key exists in the matchdict.
+  This used to be done in the traverser, which made no sense.
+
 0.9.1 (2009-06-02)
 ==================
 
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index a36cbb1dc..36429222e 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -223,31 +223,6 @@ class ModelGraphTraverserTests(unittest.TestCase):
         self.assertEqual(result['virtual_root_path'], [])
         self.assertEqual(result['matchdict'], {'subpath':'/a/b/c'})
 
-    def test_withroute_with_path_info(self):
-        model = DummyContext()
-        traverser = self._makeOne(model)
-        environ = {'bfg.routes.matchdict': {'path_info':'foo/bar'},
-                   'PATH_INFO':'/a/b/foo/bar', 'SCRIPT_NAME':''}
-        result = traverser(environ)
-        self.assertEqual(result['context'], model)
-        self.assertEqual(result['view_name'], '')
-        self.assertEqual(result['subpath'], [])
-        self.assertEqual(result['traversed'], [])
-        self.assertEqual(result['virtual_root'], model)
-        self.assertEqual(result['virtual_root_path'], [])
-        self.assertEqual(environ['PATH_INFO'], '/foo/bar')
-        self.assertEqual(environ['SCRIPT_NAME'], '/a/b')
-        self.assertEqual(result['matchdict'], {'path_info':'foo/bar'})
-
-    def test_withroute_with_path_info_PATH_INFO_w_extra_slash(self):
-        model = DummyContext()
-        traverser = self._makeOne(model)
-        environ = {'bfg.routes.matchdict':{'path_info':'foo/bar'},
-                   'PATH_INFO':'/a/b//foo/bar', 'SCRIPT_NAME':''}
-        traverser(environ)
-        self.assertEqual(environ['PATH_INFO'], '/foo/bar')
-        self.assertEqual(environ['SCRIPT_NAME'], '/a/b')
-
     def test_withroute_and_traverse(self):
         model = DummyContext()
         traverser = self._makeOne(model)
diff --git a/repoze/bfg/tests/test_urldispatch.py b/repoze/bfg/tests/test_urldispatch.py
index 68fda032d..eb3be4323 100644
--- a/repoze/bfg/tests/test_urldispatch.py
+++ b/repoze/bfg/tests/test_urldispatch.py
@@ -70,6 +70,54 @@ class RoutesRootFactoryTests(unittest.TestCase):
         self.assertEqual(environ['bfg.routes.matchdict'], {})
         self.assertEqual(environ['wsgiorg.routing_args'], ((), {}))
 
+    def test_matches_with_path_info_no_scriptname(self):
+        root_factory = make_get_root(123)
+        mapper = self._makeOne(root_factory)
+        mapper.connect('root', '/a/b/*path_info')
+        environ = self._getEnviron(PATH_INFO='/a/b/c/d')
+        result = mapper(environ)
+        self.assertEqual(result, 123)
+        self.assertEqual(environ['bfg.routes.route'].name, 'root')
+        self.assertEqual(environ['bfg.routes.matchdict'], {'path_info':'c/d'})
+        self.assertEqual(environ['PATH_INFO'], '/c/d')
+        self.assertEqual(environ['SCRIPT_NAME'], '/a/b')
+
+    def test_matches_with_path_info_with_scriptname(self):
+        root_factory = make_get_root(123)
+        mapper = self._makeOne(root_factory)
+        mapper.connect('root', '/a/b/*path_info')
+        environ = self._getEnviron(PATH_INFO='/a/b/c/d', SCRIPT_NAME='z')
+        result = mapper(environ)
+        self.assertEqual(result, 123)
+        self.assertEqual(environ['bfg.routes.route'].name, 'root')
+        self.assertEqual(environ['bfg.routes.matchdict'], {'path_info':'c/d'})
+        self.assertEqual(environ['PATH_INFO'], '/c/d')
+        self.assertEqual(environ['SCRIPT_NAME'], 'z/a/b')
+
+    def test_matches_PATH_INFO_w_extra_slash(self):
+        root_factory = make_get_root(123)
+        mapper = self._makeOne(root_factory)
+        mapper.connect('root', '/a/b/*path_info')
+        environ = self._getEnviron(PATH_INFO='/a/b//c/d', SCRIPT_NAME='')
+        result = mapper(environ)
+        self.assertEqual(result, 123)
+        self.assertEqual(environ['bfg.routes.route'].name, 'root')
+        self.assertEqual(environ['bfg.routes.matchdict'], {'path_info':'/c/d'})
+        self.assertEqual(environ['PATH_INFO'], '/c/d')
+        self.assertEqual(environ['SCRIPT_NAME'], '/a/b')
+
+    def test_matches_SCRIPT_NAME_endswith_slash(self):
+        root_factory = make_get_root(123)
+        mapper = self._makeOne(root_factory)
+        mapper.connect('root', '/a/b//*path_info')
+        environ = self._getEnviron(PATH_INFO='/a/b//c/d', SCRIPT_NAME='')
+        result = mapper(environ)
+        self.assertEqual(result, 123)
+        self.assertEqual(environ['bfg.routes.route'].name, 'root')
+        self.assertEqual(environ['bfg.routes.matchdict'], {'path_info':'c/d'})
+        self.assertEqual(environ['PATH_INFO'], '/c/d')
+        self.assertEqual(environ['SCRIPT_NAME'], '/a/b')
+
     def test_unicode_in_route_default(self):
         root_factory = make_get_root(123)
         mapper = self._makeOne(root_factory)
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index 63fd71e74..350c1098f 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -495,21 +495,7 @@ class ModelGraphTraverser(object):
     def __call__(self, environ, _marker=_marker):
         matchdict = None
         if 'bfg.routes.matchdict' in environ:
-            # this request matched a Routes route
             matchdict = environ['bfg.routes.matchdict']
-            if 'path_info' in matchdict:
-                # this is stolen from routes.middleware; if the route map
-                # has a *path_info capture, use it to influence the path
-                # info and script_name of the generated environment
-                oldpath = environ['PATH_INFO']
-                newpath = matchdict['path_info'] or ''
-                environ['PATH_INFO'] = newpath
-                if not environ['PATH_INFO'].startswith('/'):
-                    environ['PATH_INFO'] = '/' + environ['PATH_INFO']
-                pattern = r'^(.*?)/' + re.escape(newpath) + '$'
-                environ['SCRIPT_NAME'] += re.sub(pattern, r'\1', oldpath)
-                if environ['SCRIPT_NAME'].endswith('/'):
-                    environ['SCRIPT_NAME'] = environ['SCRIPT_NAME'][:-1]
             path = matchdict.get('traverse', '/')
             subpath = filter(None, matchdict.get('subpath', '').split('/'))
         else:
diff --git a/repoze/bfg/urldispatch.py b/repoze/bfg/urldispatch.py
index bfeae3333..dcd4af208 100644
--- a/repoze/bfg/urldispatch.py
+++ b/repoze/bfg/urldispatch.py
@@ -1,3 +1,5 @@
+import re
+
 from routes import Mapper
 from routes import request_config
 
@@ -45,6 +47,21 @@ class RoutesRootFactory(Mapper):
             environ['wsgiorg.routing_args'] = ((), args)
             environ['bfg.routes.route'] = route
             environ['bfg.routes.matchdict'] = args
+            # this is stolen from routes.middleware; if the route map
+            # has a *path_info capture, use it to influence the path
+            # info and script_name of the generated environment
+            if 'path_info' in args:
+                if not 'SCRIPT_NAME' in environ:
+                    environ['SCRIPT_NAME'] = ''
+                oldpath = environ['PATH_INFO']
+                newpath = args['path_info'] or ''
+                environ['PATH_INFO'] = newpath
+                if not environ['PATH_INFO'].startswith('/'):
+                    environ['PATH_INFO'] = '/' + environ['PATH_INFO']
+                pattern = r'^(.*?)/' + re.escape(newpath) + '$'
+                environ['SCRIPT_NAME'] += re.sub(pattern, r'\1', oldpath)
+                if environ['SCRIPT_NAME'].endswith('/'):
+                    environ['SCRIPT_NAME'] = environ['SCRIPT_NAME'][:-1]
             factory = route._factory or self.default_root_factory
             return factory(environ)