Merge pull request #3042 from ckan/3041-helpererror

Raise HelperError exception when calling non-existing helper

ckan/lib/helpers.py

@@ -54,8 +54,8 @@ def __init__(self, *args, **kwargs):
 
     def __getitem__(self, key):
         try:
-            value = super(HelperAttributeDict, self).__getitem__(self, key)
-        except AttributeError:
+            value = super(HelperAttributeDict, self).__getitem__(key)
+        except KeyError:
             raise ckan.exceptions.HelperError(
                 'Helper \'{key}\' has not been defined.'.format(
                     key=key

ckan/templates/tests/broken_helper_as_attribute.html

@@ -0,0 +1,5 @@
+{% extends 'page.html' %}
+
+{% block page %}
+    {{ h.not_here() }}
+{% endblock %}

ckan/templates/tests/broken_helper_as_item.html

@@ -0,0 +1,5 @@
+{% extends 'page.html' %}
+
+{% block page %}
+    {{ h['not_here']() }}
+{% endblock %}

ckan/templates/tests/helper_as_attribute.html

@@ -0,0 +1,5 @@
+{% extends 'page.html' %}
+
+{% block page %}
+    My lang is: {{ h.lang() }}
+{% endblock %}

ckan/templates/tests/helper_as_item.html

@@ -0,0 +1,5 @@
+{% extends 'page.html' %}
+
+{% block page %}
+    My lang is: {{ h['lang']() }}
+{% endblock %}

ckan/tests/lib/test_helpers.py

@@ -4,10 +4,14 @@
 from babel import Locale
 
 import ckan.lib.helpers as h
+import ckan.plugins as p
 import ckan.exceptions
 from ckan.tests import helpers
+import ckan.lib.base as base
 
+ok_ = nose.tools.ok_
 eq_ = nose.tools.eq_
+raises = nose.tools.raises
 CkanUrlException = ckan.exceptions.CkanUrlException
 
 
@@ -266,3 +270,102 @@ def test_server_timezone(self):
     @helpers.change_config('ckan.display_timezone', 'America/New_York')
     def test_named_timezone(self):
         eq_(h.get_display_timezone(), pytz.timezone('America/New_York'))
+
+
+class TestHelperException(helpers.FunctionalTestBase):
+
+    @raises(ckan.exceptions.HelperError)
+    def test_helper_exception_non_existing_helper_as_attribute(self):
+        '''Calling a non-existing helper on `h` raises a HelperException.'''
+        if not p.plugin_loaded('test_helpers_plugin'):
+            p.load('test_helpers_plugin')
+
+        app = self._get_test_app()
+
+        app.get('/broken_helper_as_attribute')
+
+        p.unload('test_helpers_plugin')
+
+    @raises(ckan.exceptions.HelperError)
+    def test_helper_exception_non_existing_helper_as_item(self):
+        '''Calling a non-existing helper on `h` raises a HelperException.'''
+        if not p.plugin_loaded('test_helpers_plugin'):
+            p.load('test_helpers_plugin')
+
+        app = self._get_test_app()
+
+        app.get('/broken_helper_as_item')
+
+        p.unload('test_helpers_plugin')
+
+    def test_helper_existing_helper_as_attribute(self):
+        '''Calling an existing helper on `h` doesn't raises a
+        HelperException.'''
+
+        if not p.plugin_loaded('test_helpers_plugin'):
+            p.load('test_helpers_plugin')
+
+        app = self._get_test_app()
+
+        res = app.get('/helper_as_attribute')
+
+        ok_('My lang is: en' in res.body)
+
+        p.unload('test_helpers_plugin')
+
+    def test_helper_existing_helper_as_item(self):
+        '''Calling an existing helper on `h` doesn't raises a
+        HelperException.'''
+
+        if not p.plugin_loaded('test_helpers_plugin'):
+            p.load('test_helpers_plugin')
+
+        app = self._get_test_app()
+
+        res = app.get('/helper_as_item')
+
+        ok_('My lang is: en' in res.body)
+
+        p.unload('test_helpers_plugin')
+
+
+class TestHelpersPlugin(p.SingletonPlugin):
+
+    p.implements(p.IRoutes, inherit=True)
+
+    controller = 'ckan.tests.lib.test_helpers:TestHelperController'
+
+    def after_map(self, _map):
+
+        _map.connect('/broken_helper_as_attribute',
+                     controller=self.controller,
+                     action='broken_helper_as_attribute')
+
+        _map.connect('/broken_helper_as_item',
+                     controller=self.controller,
+                     action='broken_helper_as_item')
+
+        _map.connect('/helper_as_attribute',
+                     controller=self.controller,
+                     action='helper_as_attribute')
+
+        _map.connect('/helper_as_item',
+                     controller=self.controller,
+                     action='helper_as_item')
+
+        return _map
+
+
+class TestHelperController(p.toolkit.BaseController):
+
+    def broken_helper_as_attribute(self):
+        return base.render('tests/broken_helper_as_attribute.html')
+
+    def broken_helper_as_item(self):
+        return base.render('tests/broken_helper_as_item.html')
+
+    def helper_as_attribute(self):
+        return base.render('tests/helper_as_attribute.html')
+
+    def helper_as_item(self):
+        return base.render('tests/helper_as_item.html')

ckan/tests/plugins/test_toolkit.py

@@ -1,4 +1,4 @@
-from nose.tools import assert_equal, assert_raises, assert_true
+from nose.tools import assert_equal, assert_raises, assert_true, raises
 
 from ckan.plugins import toolkit as tk
 
@@ -260,11 +260,21 @@ def test_raise(self):
                       tk.requires_ckan_version, min_version='3')
 
 
-class TestTemplateHelper(object):
+class TestToolkitHelper(object):
     def test_call_helper(self):
         # the null_function would return ''
         assert_true(tk.h.icon_url('x'))
 
-    def test_attribute_error_on_missing_helper(self):
+    def test_tk_helper_attribute_error_on_missing_helper(self):
         assert_raises(AttributeError, getattr,
                       tk.h, 'not_a_real_helper_function')
+
+    @raises(AttributeError)
+    def test_tk_helper_as_attribute_missing_helper(self):
+        '''Directly attempt access to module function'''
+        tk.h.nothere()
+
+    @raises(tk.HelperError)
+    def test_tk_helper_as_item_missing_helper(self):
+        '''Directly attempt access as item'''
+        tk.h['nothere']()

setup.py

@@ -159,6 +159,7 @@
         'test_datastore_view = ckan.tests.lib.test_datapreview:MockDatastoreBasedResourceView',
         'test_datapusher_plugin = ckanext.datapusher.tests.test_interfaces:FakeDataPusherPlugin',
         'test_routing_plugin = ckan.tests.config.test_middleware:MockRoutingPlugin',
+        'test_helpers_plugin = ckan.tests.lib.test_helpers:TestHelpersPlugin',
     ],
     'babel.extractors': [
         'ckan = ckan.lib.extract:extract_ckan',