From 5dcc4e8e3a071e979503a0172543876f11984a17 Mon Sep 17 00:00:00 2001 From: Carl Eberhard Date: Wed, 22 Apr 2015 17:20:37 -0400 Subject: [PATCH] API, batch: handle bad routes --- lib/galaxy/web/framework/middleware/batch.py | 42 +++++++++++--------- test/api/test_api_batch.py | 11 ++++- test/casperjs/api-batch-tests.js | 34 +++++++++++----- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/lib/galaxy/web/framework/middleware/batch.py b/lib/galaxy/web/framework/middleware/batch.py index b40b5ae2202..9926c38c443 100644 --- a/lib/galaxy/web/framework/middleware/batch.py +++ b/lib/galaxy/web/framework/middleware/batch.py @@ -1,14 +1,14 @@ """ """ -import pkg_resources -pkg_resources.require( "Routes" ) -import routes - import StringIO from urlparse import urlparse import simplejson as json +import pkg_resources +pkg_resources.require("Paste") +from paste import httpexceptions + import pprint import logging @@ -98,29 +98,35 @@ class BatchMiddleware(object): def _proccess_batch_request( self, request, environ, start_response ): # log.debug( ( '.' * 40 ) ) + # We may need to include middleware to record various reponses, but this way of doing that won't work: + # status, headers, body = self.application( environ, start_response, body_renderer=self.body_renderer ) + # We have to re-create the handle request method here in order to bypass reusing the 'api/batch' request # because reuse will cause the paste error: # File "/Users/carleberhard/galaxy/api-v2/eggs/Paste-1.7.5.1-py2.7.egg/paste/httpserver.py", line 166, in wsgi_start_response # assert 0, "Attempt to set headers a second time w/o an exc_info" - status, headers, body = self.galaxy.handle_request( environ, start_response, body_renderer=self.body_renderer ) - - # We may need to include middleware to record various reponses, but this way of doing that won't work: - # status, headers, body = self.application( environ, start_response, body_renderer=self.body_renderer ) - + try: + response = self.galaxy.handle_request( environ, start_response, body_renderer=self.body_renderer ) + # handle errors from galaxy.handle_request (only 404s) + except httpexceptions.HTTPNotFound: + response = dict( status=404, headers=self._default_headers(), body={} ) + return response # log.debug( ( '.' * 40 ) ) - return dict( - status=status, - headers=headers, - body=body, - ) def body_renderer( self, trans, body, environ, start_response ): # this is a dummy renderer that does not call start_response - return ( - trans.response.status, - trans.response.headers, - json.loads( self.galaxy.make_body_iterable( trans, body )[0] ) + return dict( + status=trans.response.status, + headers=trans.response.headers, + body=json.loads( self.galaxy.make_body_iterable( trans, body )[0] ) ) + def _default_headers( self ): + return { + 'x-frame-options': 'SAMEORIGIN', + 'content-type' : 'application/json', + 'cache-control' : 'max-age=0,no-cache,no-store' + } + def handle_exception( self, environ ): return False diff --git a/test/api/test_api_batch.py b/test/api/test_api_batch.py index f419326d9ad..5f277dc6ca6 100644 --- a/test/api/test_api_batch.py +++ b/test/api/test_api_batch.py @@ -21,7 +21,6 @@ class ApiBatchTestCase( api.ApiTestCase ): return post( "%s/batch" % ( self.galaxy_interactor.api_url ), data=data ) def test_simple_array( self ): - # post_body = dict( name='wert' ) batch = [ dict( url=self._with_key( '/api/histories' ) ), dict( url=self._with_key( '/api/histories' ), @@ -33,3 +32,13 @@ class ApiBatchTestCase( api.ApiTestCase ): # log.debug( 'RESPONSE %s\n%s', ( '-' * 40 ), pprint.pformat( response ) ) self.assertIsInstance( response, list ) self.assertEquals( len( response ), 3 ) + + def test_bad_route( self ): + batch = [ + dict( url=self._with_key( '/api/bler' ) ) + ] + response = self._post_batch( batch ) + response = response.json() + # log.debug( 'RESPONSE %s\n%s', ( '-' * 40 ), pprint.pformat( response ) ) + self.assertIsInstance( response, list ) + self.assertEquals( response[0][ 'status' ], 404 ) diff --git a/test/casperjs/api-batch-tests.js b/test/casperjs/api-batch-tests.js index e4f0c09fbf2..5bf77d39bbf 100644 --- a/test/casperjs/api-batch-tests.js +++ b/test/casperjs/api-batch-tests.js @@ -4,6 +4,14 @@ var require = patchRequire( require ), utils = require( 'utils' ), format = utils.format; +function apiBatch( batch ){ + return spaceghost.api._ajax( 'api/batch', { + type : 'POST', + contentType : 'application/json', + data : { batch : batch } + }); +} + spaceghost.test.begin( 'Test the API batch system', 0, function suite( test ){ spaceghost.start(); @@ -22,19 +30,12 @@ spaceghost.test.begin( 'Test the API batch system', 0, function suite( test ){ spaceghost.then( function(){ // -------------------------------------------------------------------- this.test.comment( 'API batching should allow multiple requests and responses, executed in order' ); - var batch = [ + var responses = apiBatch([ { url : '/api/histories' }, { url : '/api/histories', type: 'POST', body: JSON.stringify({ name: 'wert' }) }, { url : '/api/histories' }, - ], - responses = this.api._ajax( 'api/batch', { - type : 'POST', - contentType : 'application/json', - data : { batch : batch } - // data : JSON.stringify({ batch : batch }) - }); - this.debug( 'responses:' + this.jsonStr( responses ) ); - + ]); + // this.debug( 'responses:' + this.jsonStr( responses ) ); this.test.assert( utils.isArray( responses ), "returned an array: length " + responses.length ); this.test.assert( responses.length === 3, 'Has three responses' ); @@ -46,6 +47,19 @@ spaceghost.test.begin( 'Test the API batch system', 0, function suite( test ){ this.test.assert( utils.isObject( createdHistory.body ), 'history create returned an object' ); this.test.assert( historiesAfterCreate.body[0].id === createdHistory.body.id, "second histories call includes the newly created history:" + historiesAfterCreate.body[0].id ); + + + this.test.comment( 'API batching should handle bad routes well' ); + responses = apiBatch([ + { url : '/api/bler' } + ]); + // this.debug( 'responses:' + this.jsonStr( responses ) ); + this.test.assert( responses.length === 1, 'Has one response' ); + var badRouteResponse = responses[0]; + this.test.assert( badRouteResponse.status === 404 ); + this.test.assert( utils.isObject( badRouteResponse.body ) + && this.countKeys( badRouteResponse.body ) === 0 ); + /* */ });