diff --git a/ckan/common.py b/ckan/common.py index cf42b08cf91..05b4864efc6 100644 --- a/ckan/common.py +++ b/ckan/common.py @@ -11,7 +11,7 @@ import flask import pylons -from flask.ext.babel import gettext as flask_gettext +from flask_babel import gettext as flask_gettext from pylons.i18n import _ as pylons_gettext, ungettext from pylons import g, response diff --git a/ckan/config/middleware/flask_app.py b/ckan/config/middleware/flask_app.py index 14e210f92a9..aaaef5dbd45 100644 --- a/ckan/config/middleware/flask_app.py +++ b/ckan/config/middleware/flask_app.py @@ -1,6 +1,7 @@ # encoding: utf-8 import os +import time import itertools import urlparse @@ -11,9 +12,10 @@ from flask.ctx import _AppCtxGlobals from flask.sessions import SessionInterface from werkzeug.exceptions import HTTPException +from werkzeug.routing import Rule from wsgi_party import WSGIParty, HighAndDry -from flask.ext.babel import Babel +from flask_babel import Babel from flask_debugtoolbar import DebugToolbarExtension from pylons import config @@ -27,7 +29,9 @@ from ckan.common import c from ckan.plugins import PluginImplementations from ckan.plugins.interfaces import IBlueprint -from ckan.views import identify_user, set_cors_headers_for_response +from ckan.views import (identify_user, + set_cors_headers_for_response, + check_session_cookie) import logging @@ -49,6 +53,7 @@ def make_flask_stack(conf, **app_conf): app.debug = debug app.template_folder = os.path.join(root, 'templates') app.app_ctx_globals_class = CKAN_AppCtxGlobals + app.url_rule_class = CKAN_Rule # Do all the Flask-specific stuff before adding other middlewares @@ -114,11 +119,20 @@ def save_session(self, app, session, response): @app.before_request def ckan_before_request(): + c._request_timer = time.time() + app_globals.app_globals._check_uptodate() identify_user() @app.after_request def ckan_after_request(response): - set_cors_headers_for_response(response) + response = check_session_cookie(response) + response = set_cors_headers_for_response(response) + + # log time between before and after view + r_time = time.time() - c._request_timer + url = request.environ['CKAN_CURRENT_URL'].split('?')[0] + log.info('{url} render time {r_time:.3f} seconds'.format( + url=url, r_time=r_time)) return response # Template context processors @@ -165,8 +179,7 @@ def hello_world_post(): # Set up each iRoute extension as a Flask Blueprint for plugin in PluginImplementations(IBlueprint): if hasattr(plugin, 'get_blueprint'): - app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + app.register_extension_blueprint(plugin.get_blueprint()) # Start other middleware @@ -193,6 +206,15 @@ def hello_world_post(): return app +class CKAN_Rule(Rule): + + '''Custom Flask url_rule_class.''' + + def __init__(self, *args, **kwargs): + self.ckan_core = True + super(CKAN_Rule, self).__init__(*args, **kwargs) + + class CKAN_AppCtxGlobals(_AppCtxGlobals): '''Custom Flask AppCtxGlobal class (flask.g).''' @@ -239,36 +261,45 @@ def can_handle_request(self, environ): Decides whether it can handle a request with the Flask app by matching the request environ against the route mapper - Returns (True, 'flask_app') if this is the case. - ''' + Returns (True, 'flask_app', origin) if this is the case. - # TODO: identify matching urls as core or extension. This will depend - # on how we setup routing in Flask + `origin` can be either 'core' or 'extension' depending on where + the route was defined. + ''' urls = self.url_map.bind_to_environ(environ) try: - endpoint, args = urls.match() - log.debug('Flask route match, endpoint: {0}, args: {1}'.format( - endpoint, args)) - return (True, self.app_name) + rule, args = urls.match(return_rule=True) + origin = 'core' + if hasattr(rule, 'ckan_core') and not rule.ckan_core: + origin = 'extension' + log.debug('Flask route match, endpoint: {0}, args: {1}, ' + 'origin: {2}'.format(rule.endpoint, args, origin)) + return (True, self.app_name, origin) except HTTPException: raise HighAndDry() - def register_blueprint(self, blueprint, prioritise_rules=False, **options): + def register_extension_blueprint(self, blueprint, **kwargs): ''' + This method should be used to register blueprints that come from + extensions, so there's an opportunity to add extension-specific + options. + If prioritise_rules is True, add complexity to each url rule in the blueprint, to ensure they will override similar existing rules. - ''' - # Register the blueprint with the app. - super(CKANFlask, self).register_blueprint(blueprint, **options) - if prioritise_rules: - # Get the new blueprint rules - bp_rules = [v for k, v in self.url_map._rules_by_endpoint.items() - if k.startswith(blueprint.name)] - bp_rules = list(itertools.chain.from_iterable(bp_rules)) - - # This compare key will ensure the rule will be near the top. - top_compare_key = False, -100, [(-2, 0)] - for r in bp_rules: - r.match_compare_key = lambda: top_compare_key + Sets the rule property `ckan_core` to False, to indicate that the rule + applies to an extension route. + ''' + self.register_blueprint(blueprint, **kwargs) + + # Get the new blueprint rules + bp_rules = [v for k, v in self.url_map._rules_by_endpoint.items() + if k.startswith(blueprint.name)] + bp_rules = list(itertools.chain.from_iterable(bp_rules)) + + # This compare key will ensure the rule will be near the top. + top_compare_key = False, -100, [(-2, 0)] + for r in bp_rules: + r.ckan_core = False + r.match_compare_key = lambda: top_compare_key diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 17dc6dc49f9..1bf7e292376 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -26,7 +26,9 @@ import ckan.plugins as p import ckan.model as model import ckan.lib.maintain as maintain -from ckan.views import identify_user, set_cors_headers_for_response +from ckan.views import (identify_user, + set_cors_headers_for_response, + check_session_cookie) # These imports are for legacy usages and will be removed soon these should # be imported directly from ckan.common for internal ckan code and via the @@ -246,38 +248,13 @@ def __call__(self, environ, start_response): finally: model.Session.remove() - for cookie in request.cookies: - # Remove the ckan session cookie if not used e.g. logged out - if cookie == 'ckan' and not c.user: - # Check session for valid data (including flash messages) - # (DGU also uses session for a shopping basket-type behaviour) - is_valid_cookie_data = False - for key, value in session.items(): - if not key.startswith('_') and value: - is_valid_cookie_data = True - break - if not is_valid_cookie_data: - if session.id: - if not session.get('lang'): - self.log.debug('No session data any more - ' - 'deleting session') - self.log.debug('Session: %r', session.items()) - session.delete() - else: - response.delete_cookie(cookie) - self.log.debug('No session data any more - ' - 'deleting session cookie') - # Remove auth_tkt repoze.who cookie if user not logged in. - elif cookie == 'auth_tkt' and not session.id: - response.delete_cookie(cookie) + check_session_cookie(response) return res def __after__(self, action, **params): - # Do we have CORS settings in config? - if config.get('ckan.cors.origin_allow_all') \ - and request.headers.get('Origin'): - set_cors_headers_for_response(response) + set_cors_headers_for_response(response) + r_time = time.time() - c.__timer url = request.environ['CKAN_CURRENT_URL'].split('?')[0] log.info(' %s render time %.3f seconds' % (url, r_time)) diff --git a/ckan/templates/activity_streams/activity_stream_email_notifications.text b/ckan/templates/activity_streams/activity_stream_email_notifications.text index 951f3abde48..e850d486452 100644 --- a/ckan/templates/activity_streams/activity_stream_email_notifications.text +++ b/ckan/templates/activity_streams/activity_stream_email_notifications.text @@ -1,4 +1,5 @@ -{% set num = activities|length %}{{ ungettext("You have {num} new activity on your {site_title} dashboard", "You have {num} new activities on your {site_title} dashboard", num).format(site_title=g.site_title, num=num) }} {{ _('To view your dashboard, click on this link:') }} +{# TODO: check usage on ungettext vs ngettext in Flask/Pylons #} +{% set num = activities|length %}{{ ngettext("You have {num} new activity on your {site_title} dashboard", "You have {num} new activities on your {site_title} dashboard", num).format(site_title=g.site_title, num=num) }} {{ _('To view your dashboard, click on this link:') }} {{ g.site_url + '/dashboard' }} diff --git a/ckan/tests/config/test_middleware.py b/ckan/tests/config/test_middleware.py index 8731cf8d03c..d1dd42b134d 100644 --- a/ckan/tests/config/test_middleware.py +++ b/ckan/tests/config/test_middleware.py @@ -104,7 +104,7 @@ def test_pylons_can_handle_request_is_called_with_environ(self): assert mock_can_handle_request.called_with(environ) -class TestAppDispatcher(helpers.FunctionalTestBase): +class TestAppDispatcherAskAround(helpers.FunctionalTestBase): def test_ask_around_is_called(self): @@ -151,8 +151,9 @@ def test_ask_around_flask_core_route_get(self): # the same name, so we get two positive answers eq_(len(answers), 2) eq_([a[0] for a in answers], [True, True]) - eq_(sorted([a[1] for a in answers]), ['flask_app', 'pylons_app']) - # TODO: check origin (core/extension) when that is in place + # Check the app and origin + eq_(sorted([(a[1], a[2]) for a in answers]), [('flask_app', 'core'), + ('pylons_app', 'core')]) def test_ask_around_flask_core_route_post(self): @@ -174,8 +175,9 @@ def test_ask_around_flask_core_route_post(self): # the same name, so we get two positive answers eq_(len(answers), 2) eq_([a[0] for a in answers], [True, True]) - eq_(sorted([a[1] for a in answers]), ['flask_app', 'pylons_app']) - # TODO: check origin (core/extension) when that is in place + # Check the app and origin + eq_(sorted([(a[1], a[2]) for a in answers]), [('flask_app', 'core'), + ('pylons_app', 'core')]) def test_ask_around_pylons_core_route_get(self): @@ -327,8 +329,7 @@ def test_ask_around_flask_extension_and_pylons_extension_route(self): if not p.plugin_loaded('test_routing_plugin'): p.load('test_routing_plugin') plugin = p.get_plugin('test_routing_plugin') - flask_app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + flask_app.register_extension_blueprint(plugin.get_blueprint()) # We want our CKAN app, not the WebTest one app = app.app @@ -345,23 +346,39 @@ def test_ask_around_flask_extension_and_pylons_extension_route(self): eq_(len(answers), 2) eq_([a[0] for a in answers], [True, True]) eq_([a[1] for a in answers], ['flask_app', 'pylons_app']) + eq_([a[2] for a in answers], ['extension', 'extension']) - # TODO: we still can't distinguish between Flask core and extension - # eq_(answers[0][2], 'extension') + p.unload('test_routing_plugin') - eq_(answers[1][2], 'extension') - p.unload('test_routing_plugin') +class TestAppDispatcher(helpers.FunctionalTestBase): def test_flask_core_route_is_served_by_flask(self): app = self._get_test_app() - res = app.get('/hello') + # api served from core flask + res = app.get('/api/action/status_show') + + eq_(res.environ['ckan.app'], 'flask_app') + + def test_flask_extension_route_is_served_by_flask(self): + + app = self._get_test_app() + flask_app = helpers.find_flask_app(app) + + # Install plugin and register its blueprint + if not p.plugin_loaded('test_simple_flask_plugin'): + p.load('test_simple_flask_plugin') + plugin = p.get_plugin('test_simple_flask_plugin') + flask_app.register_extension_blueprint(plugin.get_blueprint()) + + # api served from extension flask + res = app.get('/simple_flask') eq_(res.environ['ckan.app'], 'flask_app') - # TODO: test flask extension route + p.unload('test_simple_flask_plugin') def test_pylons_core_route_is_served_by_pylons(self): @@ -392,7 +409,7 @@ def test_flask_core_and_pylons_extension_route_is_served_by_pylons(self): app = self._get_test_app() - res = app.get('/pylons_and_flask') + res = app.get('/hello') eq_(res.environ['ckan.app'], 'pylons_app') eq_(res.body, 'Hello World, this is served from a Pylons extension') @@ -426,8 +443,7 @@ def setup_class(cls): if not p.plugin_loaded('test_simple_flask_plugin'): p.load('test_simple_flask_plugin') plugin = p.get_plugin('test_simple_flask_plugin') - cls.flask_app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + cls.flask_app.register_extension_blueprint(plugin.get_blueprint()) @classmethod def teardown_class(cls): @@ -575,10 +591,13 @@ def before_map(self, _map): _map.connect('/from_pylons_extension_before_map_post_only', controller=self.controller, action='view', conditions={'method': 'POST'}) - # This one conflicts with a core Flask route + # This one conflicts with an extension Flask route _map.connect('/pylons_and_flask', controller=self.controller, action='view') + # This one conflicts with a core Flask route + _map.connect('/hello', + controller=self.controller, action='view') return _map def after_map(self, _map): diff --git a/ckan/tests/config/test_sessions.py b/ckan/tests/config/test_sessions.py index e3f35328980..beb9d237966 100644 --- a/ckan/tests/config/test_sessions.py +++ b/ckan/tests/config/test_sessions.py @@ -28,8 +28,7 @@ def setup(self): if not p.plugin_loaded('test_flash_plugin'): p.load('test_flash_plugin') plugin = p.get_plugin('test_flash_plugin') - self.flask_app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + self.flask_app.register_extension_blueprint(plugin.get_blueprint()) def test_flash_populated_by_flask_redirect_to_flask(self): ''' diff --git a/ckan/tests/lib/test_base.py b/ckan/tests/lib/test_base.py index dbb96176c52..3347ca4a0ec 100644 --- a/ckan/tests/lib/test_base.py +++ b/ckan/tests/lib/test_base.py @@ -174,8 +174,7 @@ def setup_class(cls): if not p.plugin_loaded('test_simple_flask_plugin'): p.load('test_simple_flask_plugin') plugin = p.get_plugin('test_simple_flask_plugin') - flask_app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + flask_app.register_extension_blueprint(plugin.get_blueprint()) @classmethod def teardown_class(cls): diff --git a/ckan/views/__init__.py b/ckan/views/__init__.py index c0538e8977b..b8be17f6fde 100644 --- a/ckan/views/__init__.py +++ b/ckan/views/__init__.py @@ -5,7 +5,7 @@ from paste.deploy.converters import asbool import ckan.model as model -from ckan.common import c, request +from ckan.common import c, request, session import ckan.plugins as p import logging @@ -15,6 +15,36 @@ APIKEY_HEADER_NAME_DEFAULT = 'X-CKAN-API-Key' +def check_session_cookie(response): + ''' + The cookies for auth (auth_tkt) and session (ckan) are separate. This + checks whether a user is logged in, and determines the validity of the + session cookie, removing it if necessary. + ''' + for cookie in request.cookies: + # Remove the ckan session cookie if logged out. + if cookie == 'ckan' and not c.user: + # Check session for valid data (including flash messages) + is_valid_cookie_data = False + for key, value in session.items(): + if not key.startswith('_') and value: + is_valid_cookie_data = True + break + if not is_valid_cookie_data: + if session.id: + log.debug('No valid session data - deleting session') + log.debug('Session: %r', session.items()) + session.delete() + else: + log.debug('No session id - deleting session cookie') + response.delete_cookie(cookie) + # Remove auth_tkt repoze.who cookie if user not logged in. + elif cookie == 'auth_tkt' and not session.id: + response.delete_cookie(cookie) + + return response + + def set_cors_headers_for_response(response): ''' Set up Access Control Allow headers if either origin_allow_all is True, or @@ -40,6 +70,8 @@ def set_cors_headers_for_response(response): response.headers['Access-Control-Allow-Headers'] = \ "X-CKAN-API-KEY, Authorization, Content-Type" + return response + def identify_user(): '''Try to identify the user diff --git a/ckanext/example_flask_iblueprint/tests/test_routes.py b/ckanext/example_flask_iblueprint/tests/test_routes.py index bf2bcc5a958..b34fe7bff13 100644 --- a/ckanext/example_flask_iblueprint/tests/test_routes.py +++ b/ckanext/example_flask_iblueprint/tests/test_routes.py @@ -16,8 +16,7 @@ def setup(self): if not plugins.plugin_loaded('example_flask_iblueprint'): plugins.load('example_flask_iblueprint') plugin = plugins.get_plugin('example_flask_iblueprint') - flask_app.register_blueprint(plugin.get_blueprint(), - prioritise_rules=True) + flask_app.register_extension_blueprint(plugin.get_blueprint()) def test_plugin_route(self): '''Test extension sets up a unique route.''' diff --git a/requirements.in b/requirements.in index 4d8d32d9a74..ddee7fa5da1 100644 --- a/requirements.in +++ b/requirements.in @@ -32,5 +32,5 @@ unicodecsv>=0.9 pytz==2016.4 tzlocal==1.2.2 wsgi-party==0.1b1 -Flask==0.10.1 +Flask==0.11.1 Flask-Babel==0.10.0 diff --git a/requirements.txt b/requirements.txt index c0b763edbd9..5b99977bcb1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ bleach==1.4.2 decorator==4.0.6 # via pylons, sqlalchemy-migrate fanstatic==0.12 Flask-Babel==0.10.0 -Flask==0.10.1 # via flask-babel +Flask==0.11.1 FormEncode==1.3.0 # via pylons html5lib==0.9999999 # via bleach itsdangerous==0.24 # via flask