Add origin value for Flask extension routes.

- fixes tests in test_middleware.py to include tests for flask
  extensions
- refactors how blueprints are registered in flask_app.py with a new
  `register_extension_blueprint` method. Extension authors shouldn't
need to call this directly, except when registering an extension
blueprint in tests.

ckan/config/middleware/flask_app.py

@@ -11,6 +11,7 @@
 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_babel import Babel
@@ -51,6 +52,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
 
@@ -159,8 +161,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
 
@@ -186,6 +187,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).'''
@@ -232,36 +242,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/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):

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