Merge pull request #3782 from ckan/3760-auto-test-request-context

[#3760] Create a test request context automatically if missing in url_for

ckan/config/middleware/__init__.py

@@ -39,6 +39,10 @@ def custom_charset__set(self, charset):
 
 # End of webob.requests.BaseRequest monkey patch
 
+# This is a test Flask request context to be used internally.
+# Do not use it!
+_internal_test_request_context = None
+
 
 def make_app(conf, full_stack=True, static_files=True, **app_conf):
     '''
@@ -55,6 +59,11 @@ def make_app(conf, full_stack=True, static_files=True, **app_conf):
     app = AskAppDispatcherMiddleware({'pylons_app': pylons_app,
                                       'flask_app': flask_app})
 
+    # Set this internal test request context with the configured environment so
+    # it can be used when calling url_for from tests
+    global _internal_test_request_context
+    _internal_test_request_context = flask_app._wsgi_app.test_request_context()
+
     return app
 
 

ckan/lib/alphabet_paginate.py

@@ -21,7 +21,7 @@
 from sqlalchemy import  __version__ as sqav
 from sqlalchemy.orm.query import Query
 from webhelpers.html.builder import HTML
-from ckan.lib.helpers import url_for
+from routes import url_for
 
 
 class AlphaPage(object):

ckan/lib/cli.py

@@ -28,11 +28,16 @@
 import ckan.include.rcssmin as rcssmin
 import ckan.plugins as p
 from ckan.common import config
+from ckan.tests.helpers import _get_test_app
 
+# This is a test Flask request context to be used internally.
+# Do not use it!
+_cli_test_request_context = None
 
-#NB No CKAN imports are allowed until after the config file is loaded.
-#   i.e. do the imports in methods, after _load_config is called.
-#   Otherwise loggers get disabled.
+
+# NB No CKAN imports are allowed until after the config file is loaded.
+#    i.e. do the imports in methods, after _load_config is called.
+#    Otherwise loggers get disabled.
 
 
 def deprecation_warning(message=None):
@@ -224,6 +229,12 @@ def load_config(config, load_site_user=True):
     from ckan.config.environment import load_environment
     load_environment(conf.global_conf, conf.local_conf)
 
+    # Set this internal test request context with the configured environment so
+    # it can be used when calling url_for from the CLI.
+    global _cli_test_request_context
+    flask_app = _get_test_app().flask_app
+    _cli_test_request_context = flask_app.test_request_context()
+
     registry = Registry()
     registry.prepare()
     import pylons

ckan/lib/helpers.py

@@ -28,6 +28,7 @@
 from pylons import url as _pylons_default_url
 from ckan.common import config, is_flask_request
 from flask import redirect as _flask_redirect
+from flask import _request_ctx_stack, current_app
 from routes import redirect_to as _routes_redirect_to
 from routes import url_for as _routes_default_url_for
 from flask import url_for as _flask_default_url_for
@@ -219,6 +220,30 @@ def get_site_protocol_and_host():
     return (None, None)
 
 
+def _get_auto_flask_context():
+    '''
+    Provides a Flask test request context if we are outside the context
+    of a web request (tests or CLI)
+    '''
+
+    from ckan.config.middleware import _internal_test_request_context
+    from ckan.lib.cli import _cli_test_request_context
+
+    # This is a normal web request, there is a request context present
+    if _request_ctx_stack.top:
+        return None
+
+    # We are outside a web request. A test web application was created
+    # (and with it a test request context with the relevant configuration)
+    if _internal_test_request_context:
+        return _internal_test_request_context
+
+    # We are outside a web request. This is a CLI command. A test request
+    # context was created when setting it up
+    if _cli_test_request_context:
+        return _cli_test_request_context
+
+
 @core_helper
 def url_for(*args, **kw):
     '''Return the URL for an endpoint given some parameters.
@@ -270,13 +295,23 @@ def url_for(*args, **kw):
         ver = kw.get('ver')
         if not ver:
             raise Exception('API URLs must specify the version (eg ver=3)')
+
+    _auto_flask_context = _get_auto_flask_context()
+
     try:
+        if _auto_flask_context:
+            _auto_flask_context.push()
+
         # First try to build the URL with the Flask router
         my_url = _url_for_flask(*args, **kw)
 
     except FlaskRouteBuildError:
+
         # If it doesn't succeed, fallback to the Pylons router
         my_url = _url_for_pylons(*args, **kw)
+    finally:
+        if _auto_flask_context:
+            _auto_flask_context.pop()
 
     # Add back internal params
     kw['__ckan_no_root'] = no_root

ckan/tests/config/test_middleware.py

@@ -3,6 +3,7 @@
 import mock
 import wsgiref
 from nose.tools import assert_equals, assert_not_equals, eq_, assert_raises
+from routes import url_for
 from flask import Blueprint
 import flask
 
@@ -29,9 +30,7 @@ def test_homepage_with_middleware_activated(self):
         We are just testing the home page renders without any troubles and that
         the middleware has not done anything strange to the response string'''
         app = self._get_test_app()
-        with app.flask_app.test_request_context():
-            response = app.get(
-                url=h.url_for(controller='home', action='index'))
+        response = app.get(url=url_for(controller='home', action='index'))
 
         assert_equals(200, response.status_int)
         # make sure we haven't overwritten the response too early.

ckan/tests/controllers/test_admin.py

@@ -3,7 +3,7 @@
 from nose.tools import assert_true, assert_equal
 
 from bs4 import BeautifulSoup
-from ckan.lib.helpers import url_for
+from routes import url_for
 from ckan.common import config
 
 import ckan.model as model
@@ -19,10 +19,8 @@
 def _get_admin_config_page(app):
     user = factories.Sysadmin()
     env = {'REMOTE_USER': user['name'].encode('ascii')}
-    with app.flask_app.test_request_context():
-        url = url_for(controller='admin', action='config')
     response = app.get(
-        url=url,
+        url=url_for(controller='admin', action='config'),
         extra_environ=env,
     )
     return env, response
@@ -32,10 +30,8 @@ def _reset_config(app):
     '''Reset config via action'''
     user = factories.Sysadmin()
     env = {'REMOTE_USER': user['name'].encode('ascii')}
-    with app.flask_app.test_request_context():
-        url = url_for(controller='admin', action='reset_config')
     app.post(
-        url=url,
+        url=url_for(controller='admin', action='reset_config'),
         extra_environ=env,
     )
 
@@ -266,19 +262,17 @@ class TestTrashView(helpers.FunctionalTestBase):
     def test_trash_view_anon_user(self):
         '''An anon user shouldn't be able to access trash view.'''
         app = self._get_test_app()
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
-        app.get(trash_url, status=403)
+
+        trash_url = url_for(controller='admin', action='trash')
+        trash_response = app.get(trash_url, status=403)
 
     def test_trash_view_normal_user(self):
         '''A normal logged in user shouldn't be able to access trash view.'''
         user = factories.User()
         app = self._get_test_app()
 
         env = {'REMOTE_USER': user['name'].encode('ascii')}
-
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
+        trash_url = url_for(controller='admin', action='trash')
         trash_response = app.get(trash_url, extra_environ=env, status=403)
         assert_true('Need to be system administrator to administer'
                     in trash_response)
@@ -289,9 +283,7 @@ def test_trash_view_sysadmin(self):
         app = self._get_test_app()
 
         env = {'REMOTE_USER': user['name'].encode('ascii')}
-
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
+        trash_url = url_for(controller='admin', action='trash')
         trash_response = app.get(trash_url, extra_environ=env, status=200)
         # On the purge page
         assert_true('form-purge-packages' in trash_response)
@@ -304,8 +296,7 @@ def test_trash_no_datasets(self):
         app = self._get_test_app()
 
         env = {'REMOTE_USER': user['name'].encode('ascii')}
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
+        trash_url = url_for(controller='admin', action='trash')
         trash_response = app.get(trash_url, extra_environ=env, status=200)
 
         trash_response_html = BeautifulSoup(trash_response.body)
@@ -324,8 +315,7 @@ def test_trash_with_deleted_datasets(self):
         app = self._get_test_app()
 
         env = {'REMOTE_USER': user['name'].encode('ascii')}
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
+        trash_url = url_for(controller='admin', action='trash')
         trash_response = app.get(trash_url, extra_environ=env, status=200)
 
         trash_response_html = BeautifulSoup(trash_response.body)
@@ -348,8 +338,7 @@ def test_trash_purge_deleted_datasets(self):
         assert_equal(pkgs_before_purge, 3)
 
         env = {'REMOTE_USER': user['name'].encode('ascii')}
-        with app.flask_app.test_request_context():
-            trash_url = url_for(controller='admin', action='trash')
+        trash_url = url_for(controller='admin', action='trash')
         trash_response = app.get(trash_url, extra_environ=env, status=200)
 
         # submit the purge form
@@ -375,8 +364,7 @@ def _update_config_option(self):
         sysadmin = factories.Sysadmin()
         env = {'REMOTE_USER': sysadmin['name'].encode('ascii')}
         app = self._get_test_app()
-        with app.flask_app.test_request_context():
-            url = url_for(controller='admin', action='config')
+        url = url_for(controller='admin', action='config')
 
         response = app.get(url=url, extra_environ=env)
         form = response.forms[1]