From 9f498f9c4be7d1ba6cfba38a2cc8993f732c013b Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 9 Mar 2017 17:55:50 -0500 Subject: [PATCH 1/5] Avoid reusing 'map' builtin --- lib/galaxy/web/framework/base.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/web/framework/base.py b/lib/galaxy/web/framework/base.py index e227a8080eb..60c93dae4a8 100644 --- a/lib/galaxy/web/framework/base.py +++ b/lib/galaxy/web/framework/base.py @@ -142,32 +142,32 @@ class WebApplication( object ): request_id = environ.get( 'request_id', 'unknown' ) # Map url using routes path_info = environ.get( 'PATH_INFO', '' ) - map = self.mapper.match( path_info, environ ) + map_match = self.mapper.match( path_info, environ ) if path_info.startswith('/api'): environ[ 'is_api_request' ] = True controllers = self.api_controllers else: environ[ 'is_api_request' ] = False controllers = self.controllers - if map is None: + if map_match is None: raise httpexceptions.HTTPNotFound( "No route for " + path_info ) - self.trace( path_info=path_info, map=map ) + self.trace( path_info=path_info, map_match=map_match ) # Setup routes rc = routes.request_config() rc.mapper = self.mapper - rc.mapper_dict = map + rc.mapper_dict = map_match rc.environ = environ # Setup the transaction trans = self.transaction_factory( environ ) trans.request_id = request_id rc.redirect = trans.response.send_redirect # Get the controller class - controller_name = map.pop( 'controller', None ) + controller_name = map_match.pop( 'controller', None ) controller = controllers.get( controller_name, None ) if controller_name is None: raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) # Resolve action method on controller - action = map.pop( 'action', 'index' ) + action = map_match.pop( 'action', 'index' ) # This is the easiest way to make the controller/action accessible for # url_for invocations. Specifically, grids. trans.controller = controller_name @@ -186,7 +186,7 @@ class WebApplication( object ): environ['controller_action_key'] = "%s.%s.%s" % ('api' if environ['is_api_request'] else 'web', controller_name, action or 'default') # Combine mapper args and query string / form args and call kwargs = trans.request.params.mixed() - kwargs.update( map ) + kwargs.update( map_match ) # Special key for AJAX debugging, remove to avoid confusing methods kwargs.pop( '_', None ) try: From 3ef22d2e6728bf890f14ef6465e88f757bd88b94 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 9 Mar 2017 18:33:34 -0500 Subject: [PATCH 2/5] Slightly more accurate check to verify we have a controller match --- lib/galaxy/web/framework/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/web/framework/base.py b/lib/galaxy/web/framework/base.py index 60c93dae4a8..52a3f9aa5cf 100644 --- a/lib/galaxy/web/framework/base.py +++ b/lib/galaxy/web/framework/base.py @@ -164,7 +164,7 @@ class WebApplication( object ): # Get the controller class controller_name = map_match.pop( 'controller', None ) controller = controllers.get( controller_name, None ) - if controller_name is None: + if controller is None: raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) # Resolve action method on controller action = map_match.pop( 'action', 'index' ) From 675c375cad69affacda74521d7e88f5227160cb6 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 9 Mar 2017 20:57:32 -0500 Subject: [PATCH 3/5] A try at two layer routing with failthrough to clientside routes. --- lib/galaxy/web/framework/base.py | 70 ++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/lib/galaxy/web/framework/base.py b/lib/galaxy/web/framework/base.py index 52a3f9aa5cf..a637f3a6fed 100644 --- a/lib/galaxy/web/framework/base.py +++ b/lib/galaxy/web/framework/base.py @@ -62,6 +62,7 @@ class WebApplication( object ): self.controllers = dict() self.api_controllers = dict() self.mapper = routes.Mapper() + self.clientside_routes = routes.Mapper() # FIXME: The following two options are deprecated and should be # removed. Consult the Routes documentation. self.mapper.minimization = True @@ -98,7 +99,7 @@ class WebApplication( object ): self.mapper.connect( route, **kwargs ) def add_client_route( self, route ): - self.add_route(route, controller='root', action='client') + self.clientside_routes.connect( route, controller='root', action='client' ) def set_transaction_factory( self, transaction_factory ): """ @@ -152,6 +153,7 @@ class WebApplication( object ): if map_match is None: raise httpexceptions.HTTPNotFound( "No route for " + path_info ) self.trace( path_info=path_info, map_match=map_match ) + # Setup routes rc = routes.request_config() rc.mapper = self.mapper @@ -161,28 +163,54 @@ class WebApplication( object ): trans = self.transaction_factory( environ ) trans.request_id = request_id rc.redirect = trans.response.send_redirect - # Get the controller class - controller_name = map_match.pop( 'controller', None ) - controller = controllers.get( controller_name, None ) - if controller is None: - raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) - # Resolve action method on controller - action = map_match.pop( 'action', 'index' ) - # This is the easiest way to make the controller/action accessible for - # url_for invocations. Specifically, grids. + + try: + # Get the controller class + controller_name = map_match.pop( 'controller', None ) + controller = controllers.get( controller_name, None ) + if controller is None: + raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) + # Resolve action method on controller + # This is the easiest way to make the controller/action accessible for + # url_for invocations. Specifically, grids. + action = map_match.pop( 'action', 'index' ) + method = getattr( controller, action, None ) + if method is None: + method = getattr( controller, 'default', None ) + if method is None: + raise httpexceptions.HTTPNotFound( "No action for " + path_info ) + # Is the method exposed + if not getattr( method, 'exposed', False ): + raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) + # Is the method callable + if not callable( method ): + raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) + except: + # Check client routes + map_match = self.clientside_routes.match( path_info, environ ) + # rc.mapper_dict = map_match + controller_name = map_match.pop( 'controller', None ) + controller = controllers.get( controller_name, None ) + if controller is None: + raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) + # Resolve action method on controller + # This is the easiest way to make the controller/action accessible for + # url_for invocations. Specifically, grids. + action = map_match.pop( 'action', 'index' ) + method = getattr( controller, action, None ) + if method is None: + method = getattr( controller, 'default', None ) + if method is None: + raise httpexceptions.HTTPNotFound( "No action for " + path_info ) + # Is the method exposed + if not getattr( method, 'exposed', False ): + raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) + # Is the method callable + if not callable( method ): + raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) + trans.controller = controller_name trans.action = action - method = getattr( controller, action, None ) - if method is None: - method = getattr( controller, 'default', None ) - if method is None: - raise httpexceptions.HTTPNotFound( "No action for " + path_info ) - # Is the method exposed - if not getattr( method, 'exposed', False ): - raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) - # Is the method callable - if not callable( method ): - raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) environ['controller_action_key'] = "%s.%s.%s" % ('api' if environ['is_api_request'] else 'web', controller_name, action or 'default') # Combine mapper args and query string / form args and call kwargs = trans.request.params.mixed() From 036d35ac643ace26f1bad289d1989d3db8971ff2 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 9 Mar 2017 21:05:29 -0500 Subject: [PATCH 4/5] Slight refactoring to reduce duplication in map matching --- lib/galaxy/web/framework/base.py | 71 ++++++++++++-------------------- 1 file changed, 27 insertions(+), 44 deletions(-) diff --git a/lib/galaxy/web/framework/base.py b/lib/galaxy/web/framework/base.py index a637f3a6fed..ecc87bfb346 100644 --- a/lib/galaxy/web/framework/base.py +++ b/lib/galaxy/web/framework/base.py @@ -138,6 +138,29 @@ class WebApplication( object ): if self.trace_logger: self.trace_logger.context_remove( "request_id" ) + def _resolve_map_match( self, map_match, path_info, controllers ): + # Get the controller class + controller_name = map_match.pop( 'controller', None ) + controller = controllers.get( controller_name, None ) + if controller is None: + raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) + # Resolve action method on controller + # This is the easiest way to make the controller/action accessible for + # url_for invocations. Specifically, grids. + action = map_match.pop( 'action', 'index' ) + method = getattr( controller, action, None ) + if method is None: + method = getattr( controller, 'default', None ) + if method is None: + raise httpexceptions.HTTPNotFound( "No action for " + path_info ) + # Is the method exposed + if not getattr( method, 'exposed', False ): + raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) + # Is the method callable + if not callable( method ): + raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) + return ( controller_name, controller, action, method ) + def handle_request( self, environ, start_response, body_renderer=None ): # Grab the request_id (should have been set by middleware) request_id = environ.get( 'request_id', 'unknown' ) @@ -153,7 +176,6 @@ class WebApplication( object ): if map_match is None: raise httpexceptions.HTTPNotFound( "No route for " + path_info ) self.trace( path_info=path_info, map_match=map_match ) - # Setup routes rc = routes.request_config() rc.mapper = self.mapper @@ -163,52 +185,13 @@ class WebApplication( object ): trans = self.transaction_factory( environ ) trans.request_id = request_id rc.redirect = trans.response.send_redirect - + # Resolve mapping to controller/method try: - # Get the controller class - controller_name = map_match.pop( 'controller', None ) - controller = controllers.get( controller_name, None ) - if controller is None: - raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) - # Resolve action method on controller - # This is the easiest way to make the controller/action accessible for - # url_for invocations. Specifically, grids. - action = map_match.pop( 'action', 'index' ) - method = getattr( controller, action, None ) - if method is None: - method = getattr( controller, 'default', None ) - if method is None: - raise httpexceptions.HTTPNotFound( "No action for " + path_info ) - # Is the method exposed - if not getattr( method, 'exposed', False ): - raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) - # Is the method callable - if not callable( method ): - raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) + controller_name, controller, action, method = self._resolve_map_match( map_match, path_info, controllers ) except: - # Check client routes + # Failed, let's check client routes map_match = self.clientside_routes.match( path_info, environ ) - # rc.mapper_dict = map_match - controller_name = map_match.pop( 'controller', None ) - controller = controllers.get( controller_name, None ) - if controller is None: - raise httpexceptions.HTTPNotFound( "No controller for " + path_info ) - # Resolve action method on controller - # This is the easiest way to make the controller/action accessible for - # url_for invocations. Specifically, grids. - action = map_match.pop( 'action', 'index' ) - method = getattr( controller, action, None ) - if method is None: - method = getattr( controller, 'default', None ) - if method is None: - raise httpexceptions.HTTPNotFound( "No action for " + path_info ) - # Is the method exposed - if not getattr( method, 'exposed', False ): - raise httpexceptions.HTTPNotFound( "Action not exposed for " + path_info ) - # Is the method callable - if not callable( method ): - raise httpexceptions.HTTPNotFound( "Action not callable for " + path_info ) - + controller_name, controller, action, method = self._resolve_map_match( map_match, path_info, controllers ) trans.controller = controller_name trans.action = action environ['controller_action_key'] = "%s.%s.%s" % ('api' if environ['is_api_request'] else 'web', controller_name, action or 'default') From 66e6be890bedbf59f98b1d10a54146175524df8a Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 9 Mar 2017 21:12:23 -0500 Subject: [PATCH 5/5] No need to check API routes against clientside mapper, catch specific exception --- lib/galaxy/web/framework/base.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/web/framework/base.py b/lib/galaxy/web/framework/base.py index ecc87bfb346..b9179f0a11e 100644 --- a/lib/galaxy/web/framework/base.py +++ b/lib/galaxy/web/framework/base.py @@ -188,10 +188,13 @@ class WebApplication( object ): # Resolve mapping to controller/method try: controller_name, controller, action, method = self._resolve_map_match( map_match, path_info, controllers ) - except: + except httpexceptions.HTTPNotFound: # Failed, let's check client routes - map_match = self.clientside_routes.match( path_info, environ ) - controller_name, controller, action, method = self._resolve_map_match( map_match, path_info, controllers ) + if not environ[ 'is_api_request' ]: + map_match = self.clientside_routes.match( path_info, environ ) + controller_name, controller, action, method = self._resolve_map_match( map_match, path_info, controllers ) + else: + raise trans.controller = controller_name trans.action = action environ['controller_action_key'] = "%s.%s.%s" % ('api' if environ['is_api_request'] else 'web', controller_name, action or 'default')