Reviewed in http://codereview.appspot.com/5036043/
diff --git a/apiclient/discovery.py b/apiclient/discovery.py
index 2a924a2..ee1ae54 100644
--- a/apiclient/discovery.py
+++ b/apiclient/discovery.py
@@ -71,11 +71,33 @@
else:
return name
+
def _write_headers(self):
# Utility no-op method for multipart media handling
pass
+def _add_query_parameter(url, name, value):
+ """Adds a query parameter to a url
+
+ Args:
+ url: string, url to add the query parameter to.
+ name: string, query parameter name.
+ value: string, query parameter value.
+
+ Returns:
+ Updated query parameter. Does not update the url if value is None.
+ """
+ if value is None:
+ return url
+ else:
+ parsed = list(urlparse.urlparse(url))
+ q = parse_qsl(parsed[4])
+ q.append((name, value))
+ parsed[4] = urllib.urlencode(q)
+ return urlparse.urlunparse(parsed)
+
+
def key2param(key):
"""Converts key names into parameter names.
@@ -131,21 +153,26 @@
if http is None:
http = httplib2.Http()
+
requested_url = uritemplate.expand(discoveryServiceUrl, params)
+ requested_url = _add_query_parameter(requested_url, 'key', developerKey)
logging.info('URL being requested: %s' % requested_url)
+
resp, content = http.request(requested_url)
- if resp.status > 400:
+
+ if resp.status >= 400:
raise HttpError(resp, content, requested_url)
+
try:
service = simplejson.loads(content)
except ValueError, e:
logging.error('Failed to parse as JSON: ' + content)
raise InvalidJsonError()
- fn = os.path.join(os.path.dirname(__file__), 'contrib',
+ filename = os.path.join(os.path.dirname(__file__), 'contrib',
serviceName, 'future.json')
try:
- f = file(fn, 'r')
+ f = file(filename, 'r')
future = f.read()
f.close()
except IOError:
@@ -249,6 +276,7 @@
"TB": 2 ** 40,
}
+
def _media_size_to_long(maxSize):
"""Convert a string media size, such as 10GB or 3TB into an integer."""
if len(maxSize) < 2:
@@ -478,21 +506,23 @@
setattr(method, '__doc__', ''.join(docs))
setattr(theclass, methodName, method)
- # This is a legacy method, as only Buzz and Moderator use the future.json
- # functionality for generating _next methods. It will be kept around as long
- # as those API versions are around, but no new APIs should depend upon it.
def createNextMethodFromFuture(theclass, methodName, methodDesc, futureDesc):
+ """ This is a legacy method, as only Buzz and Moderator use the future.json
+ functionality for generating _next methods. It will be kept around as long
+ as those API versions are around, but no new APIs should depend upon it.
+ """
methodName = _fix_method_name(methodName)
methodId = methodDesc['id'] + '.next'
def methodNext(self, previous):
- """
+ """Retrieve the next page of results.
+
Takes a single argument, 'body', which is the results
from the last call, and returns the next set of items
in the collection.
- Returns None if there are no more items in
- the collection.
+ Returns:
+ None if there are no more items in the collection.
"""
if futureDesc['type'] != 'uri':
raise UnknownLinkType(futureDesc['type'])
@@ -505,12 +535,7 @@
except (KeyError, TypeError):
return None
- if self._developerKey:
- parsed = list(urlparse.urlparse(url))
- q = parse_qsl(parsed[4])
- q.append(('key', self._developerKey))
- parsed[4] = urllib.urlencode(q)
- url = urlparse.urlunparse(parsed)
+ url = _add_query_parameter(url, 'key', self._developerKey)
headers = {}
headers, params, query, body = self._model.request(headers, {}, {}, None)
@@ -527,7 +552,6 @@
setattr(theclass, methodName, methodNext)
-
def createNextMethod(theclass, methodName, methodDesc, futureDesc):
methodName = _fix_method_name(methodName)
methodId = methodDesc['id'] + '.next'
diff --git a/tests/test_discovery.py b/tests/test_discovery.py
index 3070c60..5c7045e 100644
--- a/tests/test_discovery.py
+++ b/tests/test_discovery.py
@@ -27,6 +27,7 @@
import os
import unittest
import urlparse
+
try:
from urlparse import parse_qs
except ImportError:
@@ -44,6 +45,7 @@
DATA_DIR = os.path.join(os.path.dirname(__file__), 'data')
+
def datafile(filename):
return os.path.join(DATA_DIR, filename)
@@ -66,19 +68,49 @@
class DiscoveryFromDocument(unittest.TestCase):
+
def test_can_build_from_local_document(self):
discovery = file(datafile('buzz.json')).read()
buzz = build_from_document(discovery, base="https://www.googleapis.com/")
self.assertTrue(buzz is not None)
-
+
def test_building_with_base_remembers_base(self):
discovery = file(datafile('buzz.json')).read()
-
+
base = "https://www.example.com/"
buzz = build_from_document(discovery, base=base)
self.assertEquals(base + "buzz/v1/", buzz._baseUrl)
+class DiscoveryFromHttp(unittest.TestCase):
+
+ def test_api_key_is_added_to_discovery_uri(self):
+ # build() will raise an HttpError on a 400, use this to pick the request uri
+ # out of the raised exception.
+ try:
+ http = HttpMockSequence([
+ ({'status': '400'}, file(datafile('zoo.json'), 'r').read()),
+ ])
+ zoo = build('zoo', 'v1', http, developerKey='foo',
+ discoveryServiceUrl='http://example.com')
+ self.fail('Should have raised an exception.')
+ except HttpError, e:
+ self.assertEqual(e.uri, 'http://example.com?key=foo')
+
+ def test_api_key_of_none_is_not_added_to_discovery_uri(self):
+ # build() will raise an HttpError on a 400, use this to pick the request uri
+ # out of the raised exception.
+ try:
+ http = HttpMockSequence([
+ ({'status': '400'}, file(datafile('zoo.json'), 'r').read()),
+ ])
+ zoo = build('zoo', 'v1', http, developerKey=None,
+ discoveryServiceUrl='http://example.com')
+ self.fail('Should have raised an exception.')
+ except HttpError, e:
+ self.assertEqual(e.uri, 'http://example.com')
+
+
class Discovery(unittest.TestCase):
def test_method_error_checking(self):
@@ -128,12 +160,15 @@
http = HttpMock(datafile('zoo.json'), {'status': '200'})
zoo = build('zoo', 'v1', http)
- request = zoo.query(q="foo", i=1.0, n=1.0, b=0, a=[1,2,3], o={'a':1}, e='bar')
+ request = zoo.query(
+ q="foo", i=1.0, n=1.0, b=0, a=[1,2,3], o={'a':1}, e='bar')
self._check_query_types(request)
- request = zoo.query(q="foo", i=1, n=1, b=False, a=[1,2,3], o={'a':1}, e='bar')
+ request = zoo.query(
+ q="foo", i=1, n=1, b=False, a=[1,2,3], o={'a':1}, e='bar')
self._check_query_types(request)
- request = zoo.query(q="foo", i="1", n="1", b="", a=[1,2,3], o={'a':1}, e='bar')
+ request = zoo.query(
+ q="foo", i="1", n="1", b="", a=[1,2,3], o={'a':1}, e='bar')
self._check_query_types(request)
def test_optional_stack_query_parameters(self):
@@ -160,7 +195,8 @@
])
http = tunnel_patch(http)
zoo = build('zoo', 'v1', http)
- resp = zoo.animals().patch(name='lion', body='{"description": "foo"}').execute()
+ resp = zoo.animals().patch(
+ name='lion', body='{"description": "foo"}').execute()
self.assertTrue('x-http-method-override' in resp)
@@ -277,7 +313,8 @@
zoo = build('zoo', 'v1', self.http)
request = zoo.animals().insert(media_body=datafile('small.png'), body={})
- self.assertTrue(request.headers['content-type'].startswith('multipart/related'))
+ self.assertTrue(request.headers['content-type'].startswith(
+ 'multipart/related'))
self.assertEquals('--==', request.body[0:4])
def test_media_capable_method_without_media(self):
@@ -308,7 +345,8 @@
self.http = HttpMock(datafile('tasks.json'), {'status': '200'})
tasks = build('tasks', 'v1', self.http)
request = tasks.tasklists().list()
- next_request = tasks.tasklists().list_next(request, {'nextPageToken': '123abc'})
+ next_request = tasks.tasklists().list_next(
+ request, {'nextPageToken': '123abc'})
parsed = list(urlparse.urlparse(next_request.uri))
q = parse_qs(parsed[4])
self.assertEqual(q['pageToken'][0], '123abc')
@@ -318,7 +356,9 @@
service = build('latitude', 'v1', self.http)
request = service.currentLocation().get()
+
class DeveloperKey(unittest.TestCase):
+
def test_param(self):
self.http = HttpMock(datafile('buzz.json'), {'status': '200'})
buzz = build('buzz', 'v1', self.http, developerKey='foobie_bletch')