- Fix a bug whereby a ``renderer`` argument to the ``@bfg_view``

  decorator that provided a package-relative template filename might
  not have been resolved properly.  Symptom: inappropriate ``Missing
  template resource`` errors.

13 files changed, 117 insertions, 18 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index a2039c2c4..a86fa16a0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -10,6 +10,11 @@ Bug Fixes
   class was being called instead of the method named via
   ``view_attr``.
 
+- Fix a bug whereby a ``renderer`` argument to the ``@bfg_view``
+  decorator that provided a package-relative template filename might
+  not have been resolved properly.  Symptom: inappropriate ``Missing
+  template resource`` errors.
+
 1.2b4 (2010-02-03)
 ==================
 
diff --git a/repoze/bfg/path.py b/repoze/bfg/path.py
index 30c72b436..0afdf777d 100644
--- a/repoze/bfg/path.py
+++ b/repoze/bfg/path.py
@@ -56,4 +56,3 @@ def package_path(package):
             pass
     return prefix
 
-    
diff --git a/repoze/bfg/resource.py b/repoze/bfg/resource.py
index 25e6af4f7..ff10273e4 100644
--- a/repoze/bfg/resource.py
+++ b/repoze/bfg/resource.py
@@ -5,6 +5,7 @@ from zope.interface import implements
 
 from repoze.bfg.interfaces import IPackageOverrides
 
+from repoze.bfg.path import package_path
 from repoze.bfg.threadlocal import get_current_registry
 
 class OverrideProvider(pkg_resources.DefaultProvider):
@@ -180,3 +181,18 @@ def resolve_resource_spec(spec, package_name='__main__'):
     elif package_name is None:
         package_name, filename = None, spec
     return package_name, filename
+
+def resource_spec_from_abspath(abspath, package):
+    """ Try to convert an absolute path to a resource in a package to
+    a resource specification if possible; otherwise return the
+    absolute path.  """
+    if getattr(package, '__name__', None) == '__main__':
+        return abspath
+    pp = package_path(package) + os.path.sep
+    if abspath.startswith(pp):
+        relpath = abspath[len(pp):]
+        return '%s:%s' % (package.__name__,
+                          relpath.replace(os.path.sep, '/'))
+    return abspath
+            
+    
diff --git a/repoze/bfg/tests/restbugapp/views.py b/repoze/bfg/tests/restbugapp/views.py
index eb6a6591d..b94851099 100644
--- a/repoze/bfg/tests/restbugapp/views.py
+++ b/repoze/bfg/tests/restbugapp/views.py
@@ -13,6 +13,3 @@ class PetRESTView(BaseRESTView):
     def GET(self):
         return Response('gotten')
     
-    def POST(self):
-        return Response('posted')
-        
diff --git a/repoze/bfg/tests/test_integration.py b/repoze/bfg/tests/test_integration.py
index 35d113c5f..41144f7c3 100644
--- a/repoze/bfg/tests/test_integration.py
+++ b/repoze/bfg/tests/test_integration.py
@@ -151,6 +151,23 @@ class TestRestBugApp(TwillBase):
         self.assertEqual(browser.get_code(), 200)
         self.assertEqual(browser.get_html(), 'gotten')
 
+class TestViewDecoratorApp(TwillBase):
+    config = 'repoze.bfg.tests.viewdecoratorapp:configure.zcml'
+    def test_it(self):
+        import twill.commands
+        browser = twill.commands.get_browser()
+        browser.go('http://localhost:6543/first')
+        self.assertEqual(browser.get_code(), 200)
+        self.failUnless('OK' in browser.get_html())
+
+        browser.go('http://localhost:6543/second')
+        self.assertEqual(browser.get_code(), 200)
+        self.failUnless('OK2' in browser.get_html())
+
+        browser.go('http://localhost:6543/third')
+        self.assertEqual(browser.get_code(), 200)
+        self.failUnless('OK3' in browser.get_html())
+
 class DummyContext(object):
     pass
 
diff --git a/repoze/bfg/tests/test_view.py b/repoze/bfg/tests/test_view.py
index 994eb8c66..68184b5ac 100644
--- a/repoze/bfg/tests/test_view.py
+++ b/repoze/bfg/tests/test_view.py
@@ -367,6 +367,36 @@ class TestBFGViewDecorator(unittest.TestCase):
         settings = decorated.__bfg_view_settings__
         self.assertEqual(settings[0]['custom_predicates'], (1,))
 
+    def test_call_with_renderer_nodot(self):
+        decorator = self._makeOne(renderer='json')
+        def foo():
+            """ docstring """
+        wrapped = decorator(foo)
+        self.failUnless(wrapped is foo)
+        settings = wrapped.__bfg_view_settings__[0]
+        self.assertEqual(settings['renderer'], 'json')
+
+    def test_call_with_renderer_relpath(self):
+        decorator = self._makeOne(renderer='fixtures/minimal.pt')
+        def foo():
+            """ docstring """
+        wrapped = decorator(foo)
+        self.failUnless(wrapped is foo)
+        settings = wrapped.__bfg_view_settings__[0]
+        self.assertEqual(settings['renderer'],
+                         'repoze.bfg.tests.test_view:fixtures/minimal.pt')
+
+    def test_call_with_renderer_pkgpath(self):
+        decorator = self._makeOne(
+            renderer='repoze.bfg.tests.test_view:fixtures/minimal.pt')
+        def foo():
+            """ docstring """
+        wrapped = decorator(foo)
+        self.failUnless(wrapped is foo)
+        settings = wrapped.__bfg_view_settings__[0]
+        self.assertEqual(settings['renderer'],
+                         'repoze.bfg.tests.test_view:fixtures/minimal.pt')
+
 class TestDefaultForbiddenView(BaseTest, unittest.TestCase):
     def _callFUT(self, context, request):
         from repoze.bfg.view import default_forbidden_view
diff --git a/repoze/bfg/tests/viewdecoratorapp/__init__.py b/repoze/bfg/tests/viewdecoratorapp/__init__.py
new file mode 100644
index 000000000..5bb534f79
--- /dev/null
+++ b/repoze/bfg/tests/viewdecoratorapp/__init__.py
@@ -0,0 +1 @@
+# package
diff --git a/repoze/bfg/tests/viewdecoratorapp/configure.zcml b/repoze/bfg/tests/viewdecoratorapp/configure.zcml
new file mode 100644
index 000000000..6867046df
--- /dev/null
+++ b/repoze/bfg/tests/viewdecoratorapp/configure.zcml
@@ -0,0 +1,6 @@
+<configure xmlns="http://namespaces.repoze.org/bfg">
+
+  <include package="repoze.bfg.includes" />
+  <scan package="."/>
+
+</configure>
diff --git a/repoze/bfg/tests/viewdecoratorapp/views/__init__.py b/repoze/bfg/tests/viewdecoratorapp/views/__init__.py
new file mode 100644
index 000000000..5bb534f79
--- /dev/null
+++ b/repoze/bfg/tests/viewdecoratorapp/views/__init__.py
@@ -0,0 +1 @@
+# package
diff --git a/repoze/bfg/tests/viewdecoratorapp/views/templates/foo.pt b/repoze/bfg/tests/viewdecoratorapp/views/templates/foo.pt
new file mode 100644
index 000000000..6a2f701b6
--- /dev/null
+++ b/repoze/bfg/tests/viewdecoratorapp/views/templates/foo.pt
@@ -0,0 +1,3 @@
+<html>
+${result}
+</html>
diff --git a/repoze/bfg/tests/viewdecoratorapp/views/views.py b/repoze/bfg/tests/viewdecoratorapp/views/views.py
new file mode 100644
index 000000000..29f8c7fd4
--- /dev/null
+++ b/repoze/bfg/tests/viewdecoratorapp/views/views.py
@@ -0,0 +1,17 @@
+import os
+from repoze.bfg.view import bfg_view
+
+@bfg_view(renderer='templates/foo.pt', name='first')
+def first(request):
+    return {'result':'OK1'}
+
+@bfg_view(renderer='repoze.bfg.tests.viewdecoratorapp.views:templates/foo.pt',
+          name='second')
+def second(request):
+    return {'result':'OK2'}
+
+here = os.path.normpath(os.path.dirname(os.path.abspath(__file__)))
+foo = os.path.join(here, 'templates', 'foo.pt')
+@bfg_view(renderer=foo, name='third')
+def third(request):
+    return {'result':'OK3'}
diff --git a/repoze/bfg/view.py b/repoze/bfg/view.py
index 02a53824f..ac0977378 100644
--- a/repoze/bfg/view.py
+++ b/repoze/bfg/view.py
@@ -1,5 +1,6 @@
 import cgi
 import mimetypes
+import os
 import sys
 
 # See http://bugs.python.org/issue5853 which is a recursion bug
@@ -26,7 +27,9 @@ from repoze.bfg.interfaces import IRoutesMapper
 from repoze.bfg.interfaces import IView
 
 from repoze.bfg.path import caller_package
+from repoze.bfg.path import package_path
 from repoze.bfg.resource import resolve_resource_spec
+from repoze.bfg.resource import resource_spec_from_abspath
 from repoze.bfg.static import PackageURLParser
 from repoze.bfg.threadlocal import get_current_registry
 
@@ -489,6 +492,20 @@ class bfg_view(object):
         else:
             settings = getattr(wrapped, '__bfg_view_settings__', [])
             wrapped.__bfg_view_settings__ = settings
+
+        # try to convert the renderer provided into a fully qualified
+        # resource specification
+        abspath = setting.get('renderer')
+        if abspath is not None and '.' in abspath:
+            isabs = os.path.isabs(abspath)
+            if not (':' in abspath and not isabs):
+                # not already a resource spec
+                if not isabs:
+                    pp = package_path(module)
+                    abspath = os.path.join(pp, abspath)
+                resource = resource_spec_from_abspath(abspath, module)
+                setting['renderer'] = resource
+
         settings.append(setting)
         return wrapped
 
diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py
index fabc1a03c..4ae04387f 100644
--- a/repoze/bfg/zcml.py
+++ b/repoze/bfg/zcml.py
@@ -30,8 +30,8 @@ from repoze.bfg.authentication import RepozeWho1AuthenticationPolicy
 from repoze.bfg.authorization import ACLAuthorizationPolicy
 from repoze.bfg.configuration import Configurator
 from repoze.bfg.exceptions import ConfigurationError
-from repoze.bfg.path import package_path
 from repoze.bfg.request import route_request_iface
+from repoze.bfg.resource import resource_spec_from_abspath
 from repoze.bfg.static import StaticRootFactory
 from repoze.bfg.threadlocal import get_current_registry
 
@@ -775,24 +775,14 @@ def utility(_context, provides=None, component=None, factory=None, name=''):
         )
 
 def path_spec(context, path):
-    # Convert an absolute path to a resource in a package to a
-    # resource specification if possible; otherwise return the
-    # absolute path; we prefer registering resource specifications
-    # over absolute paths because these can be overridden by the
-    # resource directive.
+    # we prefer registering resource specifications over absolute
+    # paths because these can be overridden by the resource directive.
     if ':' in path and not os.path.isabs(path):
         # it's already a resource specification
         return path
     abspath = context.path(path)
     if hasattr(context, 'package') and context.package:
-        package = context.package
-        if getattr(package, '__name__', None) == '__main__':
-            return abspath
-        pp = package_path(package) + os.path.sep
-        if abspath.startswith(pp):
-            relpath = abspath[len(pp):]
-            return '%s:%s' % (package.__name__,
-                              relpath.replace(os.path.sep, '/'))
+        return resource_spec_from_abspath(abspath, context.package)
     return abspath
 
 def zcml_configure(name, package):