API, batch: handle bad routes

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 @@ def _build_request_environ( self, original_environ, request ):
     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

test/api/test_api_batch.py

@@ -21,7 +21,6 @@ def _post_batch( self, batch ):
         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 @@ def test_simple_array( self ):
         # 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 )

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 );
+
     /*
     */
     });