Merge branch 'poc-flask-views' into poc-flask-views.common-url_for-ta…

…ke-2

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

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

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))

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' }}
 

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):

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):
         '''

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):