Fix Batch issues. Fixes issue #41.

apiclient/discovery.py

@@ -343,7 +343,7 @@ def createResource(http, baseUrl, model, requestBuilder,
           'location': 'query'
           }
 
-    if httpMethod in ['PUT', 'POST', 'PATCH']:
+    if httpMethod in ['PUT', 'POST', 'PATCH'] and 'request' in methodDesc:
       methodDesc['parameters']['body'] = {
           'description': 'The request body.',
           'type': 'object',
@@ -353,12 +353,13 @@ def createResource(http, baseUrl, model, requestBuilder,
         methodDesc['parameters']['body'].update(methodDesc['request'])
       else:
         methodDesc['parameters']['body']['type'] = 'object'
-      if 'mediaUpload' in methodDesc:
-        methodDesc['parameters']['media_body'] = {
-            'description': 'The filename of the media request body.',
-            'type': 'string',
-            'required': False,
-            }
+    if 'mediaUpload' in methodDesc:
+      methodDesc['parameters']['media_body'] = {
+          'description': 'The filename of the media request body.',
+          'type': 'string',
+          'required': False,
+          }
+      if 'body' in methodDesc['parameters']:
         methodDesc['parameters']['body']['required'] = False
 
     argmap = {} # Map from method parameter name to query parameter name

apiclient/errors.py

@@ -91,9 +91,18 @@ class ResumableUploadError(Error):
   pass
 
 
-class BatchError(Error):
+class BatchError(HttpError):
   """Error occured during batch operations."""
-  pass
+
+  def __init__(self, reason, resp=None, content=None):
+    self.resp = resp
+    self.content = content
+    self.reason = reason
+
+  def __repr__(self):
+      return '<BatchError %s "%s">' % (self.resp.status, self.reason)
+
+  __str__ = __repr__
 
 
 class UnexpectedMethodError(Error):

apiclient/http.py

@@ -491,7 +491,7 @@ class BatchHttpRequest(object):
         (None, None, parsed.path, parsed.params, parsed.query, None)
         )
     status_line = request.method + ' ' + request_line + ' HTTP/1.1\n'
-    major, minor = request.headers.get('content-type', 'text/plain').split('/')
+    major, minor = request.headers.get('content-type', 'application/json').split('/')
     msg = MIMENonMultipart(major, minor)
     headers = request.headers.copy()
 
@@ -506,6 +506,7 @@ class BatchHttpRequest(object):
 
     if request.body is not None:
       msg.set_payload(request.body)
+      msg['content-length'] = str(len(request.body))
 
     body = msg.as_string(False)
     # Strip off the \n\n that the MIME lib tacks onto the end of the payload.
@@ -525,7 +526,7 @@ class BatchHttpRequest(object):
     """
     # Strip off the status line
     status_line, payload = payload.split('\n', 1)
-    protocol, status, reason = status_line.split(' ')
+    protocol, status, reason = status_line.split(' ', 2)
 
     # Parse the rest of the response
     parser = FeedParser()
@@ -604,6 +605,7 @@ class BatchHttpRequest(object):
     Raises:
       apiclient.errors.HttpError if the response was not a 2xx.
       httplib2.Error if a transport error has occured.
+      apiclient.errors.BatchError if the response is the wrong format.
     """
     if http is None:
       for request_id in self._order:
@@ -649,14 +651,15 @@ class BatchHttpRequest(object):
 
     # Prepend with a content-type header so FeedParser can handle it.
     header = 'Content-Type: %s\r\n\r\n' % resp['content-type']
-    content = header + content
+    for_parser = header + content
 
     parser = FeedParser()
-    parser.feed(content)
+    parser.feed(for_parser)
     respRoot = parser.close()
 
     if not respRoot.is_multipart():
-      raise BatchError("Response not in multipart/mixed format.")
+      raise BatchError("Response not in multipart/mixed format.", resp,
+          content)
 
     parts = respRoot.get_payload()
     for part in parts:

tests/test_http.py

@@ -114,7 +114,15 @@ class TestUserAgent(unittest.TestCase):
 EXPECTED = """POST /someapi/v1/collection/?foo=bar HTTP/1.1
 Content-Type: application/json
 MIME-Version: 1.0
-Host: www.googleapis.com\r\n\r\n{}"""
+Host: www.googleapis.com
+content-length: 2\r\n\r\n{}"""
+
+
+NO_BODY_EXPECTED = """POST /someapi/v1/collection/?foo=bar HTTP/1.1
+Content-Type: application/json
+MIME-Version: 1.0
+Host: www.googleapis.com
+content-length: 0\r\n\r\n"""
 
 
 RESPONSE = """HTTP/1.1 200 OK
@@ -144,6 +152,7 @@ Content-Length: 14
 ETag: "etag/sheep"\r\n\r\n{"baz": "qux"}
 --batch_foobarbaz--"""
 
+
 class TestBatch(unittest.TestCase):
 
   def setUp(self):
@@ -160,8 +169,8 @@ class TestBatch(unittest.TestCase):
         None,
         model.response,
         'https://www.googleapis.com/someapi/v1/collection/?foo=bar',
-        method='POST',
-        body='{}',
+        method='GET',
+        body='',
         headers={'content-type': 'application/json'})
 
 
@@ -189,6 +198,20 @@ class TestBatch(unittest.TestCase):
     s = batch._serialize_request(request).splitlines()
     self.assertEquals(s, EXPECTED.splitlines())
 
+  def test_serialize_request_no_body(self):
+    batch = BatchHttpRequest()
+    request = HttpRequest(
+        None,
+        None,
+        'https://www.googleapis.com/someapi/v1/collection/?foo=bar',
+        method='POST',
+        body='',
+        headers={'content-type': 'application/json'},
+        methodId=None,
+        resumable=None)
+    s = batch._serialize_request(request).splitlines()
+    self.assertEquals(s, NO_BODY_EXPECTED.splitlines())
+
   def test_deserialize_response(self):
     batch = BatchHttpRequest()
     resp, content = batch._deserialize_response(RESPONSE)
@@ -247,6 +270,29 @@ class TestBatch(unittest.TestCase):
     self.assertEqual(callbacks.responses['1'], {'foo': 42})
     self.assertEqual(callbacks.responses['2'], {'baz': 'qux'})
 
+  def test_execute_request_body(self):
+    batch = BatchHttpRequest()
+
+    batch.add(self.request1)
+    batch.add(self.request2)
+    http = HttpMockSequence([
+      ({'status': '200',
+        'content-type': 'multipart/mixed; boundary="batch_foobarbaz"'},
+        'echo_request_body'),
+      ])
+    try:
+      batch.execute(http)
+      self.fail('Should raise exception')
+    except BatchError, e:
+      boundary, _ = e.content.split(None, 1)
+      self.assertEqual('--', boundary[:2])
+      parts = e.content.split(boundary)
+      self.assertEqual(4, len(parts))
+      self.assertEqual('', parts[0])
+      self.assertEqual('--', parts[3])
+      header = parts[1].splitlines()[1]
+      self.assertEqual('Content-Type: application/http', header)
+
   def test_execute_global_callback(self):
     class Callbacks(object):
       def __init__(self):