diff --git a/ckan/config/middleware/flask_app.py b/ckan/config/middleware/flask_app.py index c6b0ae84d78..ebb763b139b 100644 --- a/ckan/config/middleware/flask_app.py +++ b/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 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/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.'''