Skip to content

Commit

Permalink
[#2485] Enforce order of precedence on request handler middleware
Browse files Browse the repository at this point in the history
Enforce Flask Extension > Pylons Extension > Flask Core > Pylons Core

Pylons requests are marked core=True or core=False depending on whether
they were set up in core or by an extension. This information is passed
to the handler so it can prioritize which app to use.

TODO: It's still not clear how we will mark Flask routes as core or not,
it will depend on how we end up setting up routing
  • Loading branch information
amercader committed Feb 26, 2016
1 parent ac33380 commit f0b2c87
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
30 changes: 23 additions & 7 deletions ckan/config/middleware.py
Expand Up @@ -252,10 +252,12 @@ def can_handle_request(self, environ):
Returns (True, 'flask_app') if this is the case.
'''

# TODO: identify matching urls as core or extension. This will depend
# on how we setup routing in Flask

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)
Expand Down Expand Up @@ -316,15 +318,29 @@ def __call__(self, environ, start_response):

app_name = 'pylons_app' # currently defaulting to pylons app
answers = self.ask_around('can_handle_request', environ)
for answer in answers:
can_handle, asked_app = answer
if can_handle:
app_name = asked_app
break

log.debug('Route support answers for {0} {1}: {2}'.format(
environ.get('REQUEST_METHOD'), environ.get('PATH_INFO'),
answers))
available_handlers = []
for answer in answers:
if len(answer) == 2:
can_handle, asked_app = answer
origin = 'core'
else:
can_handle, asked_app, origin = answer
if can_handle:
available_handlers.append('{0}_{1}'.format(asked_app, origin))

# Enforce order of precedence:
# Flask Extension > Pylons Extension > Flask Core > Pylons Core
if available_handlers:
if 'flask_app_extension' in available_handlers:
app_name = 'flask_app'
elif 'pylons_app_extension' in available_handlers:
app_name = 'pylons_app'
elif 'flask_app_core' in available_handlers:
app_name = 'flask_app'

log.debug('Serving request via {0} app'.format(app_name))
return self.apps[app_name](environ, start_response)

Expand Down
25 changes: 21 additions & 4 deletions ckan/config/routing.py
Expand Up @@ -88,16 +88,22 @@ def make_map():

# The ErrorController route (handles 404/500 error pages); it should
# likely stay at the top, ensuring it can always be resolved.
map.connect('/error/{action}', controller='error')
map.connect('/error/{action}/{id}', controller='error')
map.connect('/error/{action}', controller='error', core=True)
map.connect('/error/{action}/{id}', controller='error', core=True)

map.connect('*url', controller='home', action='cors_options',
conditions=OPTIONS)
conditions=OPTIONS, core=True)

# CUSTOM ROUTES HERE
for plugin in p.PluginImplementations(p.IRoutes):
map = plugin.before_map(map)

# Mark all routes added from extensions on the `before_map` extension point
# as non-core
for route in map.matchlist:
if 'core' not in route.defaults:
route.defaults['core'] = False

map.connect('invite', '/__invite__/', controller='partyline', action='join_party')

map.connect('home', '/', controller='home', action='index')
Expand Down Expand Up @@ -415,15 +421,26 @@ def make_map():
m.connect('/testing/primer', action='primer')
m.connect('/testing/markup', action='markup')

# Mark all unmarked routes added up until now as core routes
for route in map.matchlist:
if 'core' not in route.defaults:
route.defaults['core'] = True

for plugin in p.PluginImplementations(p.IRoutes):
map = plugin.after_map(map)

# Mark all routes added from extensions on the `after_map` extension point
# as non-core
for route in map.matchlist:
if 'core' not in route.defaults:
route.defaults['core'] = False

# sometimes we get requests for favicon.ico we should redirect to
# the real favicon location.
map.redirect('/favicon.ico', config.get('ckan.favicon'))

map.redirect('/*(url)/', '/{url}',
_redirect_code='301 Moved Permanently')
map.connect('/*url', controller='template', action='view')
map.connect('/*url', controller='template', action='view', core=True)

return map
8 changes: 6 additions & 2 deletions ckan/controllers/partyline.py
Expand Up @@ -33,7 +33,10 @@ def _can_handle_request(self, environ):
Decides whether it can handle a request with the Pylons app by
matching the request environ against the route mapper
Returns (True, 'pylons_app') if this is the case.
Returns (True, 'pylons_app', origin) if this is the case.
origin can be either 'core' or 'extension' depending on where
the route was defined.
NOTE: There is currently a catch all route for GET requests to
point arbitrary urls to templates with the same name:
Expand All @@ -49,7 +52,8 @@ def _can_handle_request(self, environ):
pylons_mapper = config['routes.map']
match = pylons_mapper.match(environ=environ)
if match:
origin = 'core' if match.get('core') == 'True' else 'extension'
log.debug('Pylons route match: {0}'.format(match))
return (True, self.app_name)
return (True, self.app_name, origin)
else:
raise HighAndDry()

0 comments on commit f0b2c87

Please sign in to comment.