Skip to content

Commit

Permalink
Small tweaks to Connection in storage; towards a cleaner API.
Browse files Browse the repository at this point in the history 
- Making Connection.make_request non-public
- Making Connection.build_api_url a class method
  • Loading branch information
@dhermes
dhermes committed Feb 5, 2015
1 parent 9518db7 commit 9dc8773
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 48 deletions.
5 changes: 3 additions & 2 deletions gcloud/storage/blob.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,15 @@ def upload_from_file(self, file_obj, rewind=False, size=None,

# Temporary URL, until we know simple vs. resumable.
upload_url = conn.build_api_url(
path=self.bucket.path + '/o', upload=True)
project=conn.project, path=self.bucket.path + '/o', upload=True)

# Use apitools 'Upload' facility.
request = http_wrapper.Request(upload_url, 'POST', headers)

upload.ConfigureRequest(upload_config, request, url_builder)
query_params = url_builder.query_params
request.url = conn.build_api_url(path=self.bucket.path + '/o',
request.url = conn.build_api_url(project=conn.project,
path=self.bucket.path + '/o',
query_params=query_params,
upload=True)
upload.InitializeUpload(request, conn.http)
Expand Down
32 changes: 18 additions & 14 deletions gcloud/storage/connection.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class Connection(_Base):
>>> print 'my-bucket-name' in connection
True
:type project: string
:param project: The project name to connect to.
"""

API_VERSION = 'v1'
Expand All @@ -75,10 +78,6 @@ class Connection(_Base):
"""A template for the URL of a particular API call."""

def __init__(self, project, *args, **kwargs):
""":type project: string
:param project: The project name to connect to.
"""
super(Connection, self).__init__(*args, **kwargs)
self.project = project

Expand All @@ -88,12 +87,16 @@ def __iter__(self):
def __contains__(self, bucket_name):
return self.lookup(bucket_name) is not None

def build_api_url(self, path, query_params=None, api_base_url=None,
@classmethod
def build_api_url(cls, project, path, query_params=None, api_base_url=None,
api_version=None, upload=False):
"""Construct an API url given a few components, some optional.
Typically, you shouldn't need to use this method.
:type project: string
:param project: The project name to connect to.
:type path: string
:param path: The path to the resource (ie, ``'/b/bucket-name'``).
Expand All @@ -116,23 +119,23 @@ def build_api_url(self, path, query_params=None, api_base_url=None,
:rtype: string
:returns: The URL assembled from the pieces provided.
"""
api_base_url = api_base_url or self.API_BASE_URL
api_base_url = api_base_url or cls.API_BASE_URL
if upload:
api_base_url += '/upload'

url = self.API_URL_TEMPLATE.format(
api_base_url=(api_base_url or self.API_BASE_URL),
api_version=(api_version or self.API_VERSION),
url = cls.API_URL_TEMPLATE.format(
api_base_url=(api_base_url or cls.API_BASE_URL),
api_version=(api_version or cls.API_VERSION),
path=path)

query_params = query_params or {}
query_params.update({'project': self.project})
query_params.update({'project': project})
url += '?' + urlencode(query_params)

return url

def make_request(self, method, url, data=None, content_type=None,
headers=None):
def _make_request(self, method, url, data=None, content_type=None,
headers=None):
"""A low level method to send a request to the API.
Typically, you shouldn't need to use this method.
Expand Down Expand Up @@ -224,7 +227,8 @@ def api_request(self, method, path, query_params=None,
:raises: Exception if the response code is not 200 OK.
"""
url = self.build_api_url(path=path, query_params=query_params,
url = self.build_api_url(project=self.project, path=path,
query_params=query_params,
api_base_url=api_base_url,
api_version=api_version)

Expand All @@ -234,7 +238,7 @@ def api_request(self, method, path, query_params=None,
data = json.dumps(data)
content_type = 'application/json'

response, content = self.make_request(
response, content = self._make_request(
method=method, url=url, data=data, content_type=content_type)

if not 200 <= response.status < 300:
Expand Down
55 changes: 41 additions & 14 deletions gcloud/storage/test_blob.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,12 @@ def test_upload_from_file_simple(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -376,8 +380,12 @@ def test_upload_from_file_resumable(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'resumable', 'name': BLOB_NAME})
query_params = {
'uploadType': 'resumable',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['X-Upload-Content-Length'], '6')
Expand Down Expand Up @@ -431,8 +439,12 @@ def test_upload_from_file_w_slash_in_name(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'parent/child'})
query_params = {
'uploadType': 'media',
'name': 'parent/child',
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -471,8 +483,12 @@ def test_upload_from_filename(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -507,8 +523,12 @@ def test_upload_from_string_w_bytes(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'project': 'PROJECT',
'uploadType': 'media',
'name': BLOB_NAME,
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -545,8 +565,12 @@ def test_upload_from_string_w_text(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], str(len(ENCODED)))
Expand Down Expand Up @@ -906,6 +930,7 @@ class _Connection(_Responder):
API_BASE_URL = 'http://example.com'
USER_AGENT = 'testing 1.2.3'
credentials = object()
project = 'PROJECT'

def __init__(self, *responses):
super(_Connection, self).__init__(*responses)
Expand All @@ -915,15 +940,17 @@ def __init__(self, *responses):
def api_request(self, **kw):
return self._respond(**kw)

def build_api_url(self, path, query_params=None,
def build_api_url(self, project, path, query_params=None,
api_base_url=API_BASE_URL, upload=False):
from six.moves.urllib.parse import urlencode
from six.moves.urllib.parse import urlsplit
from six.moves.urllib.parse import urlunsplit
# mimic the build_api_url interface, but avoid unused param and
# missed coverage errors
upload = not upload # pragma NO COVER
qs = urlencode(query_params or {})
query_params = query_params or {}
query_params['project'] = project
qs = urlencode(query_params)
scheme, netloc, _, _, _ = urlsplit(api_base_url)
return urlunsplit((scheme, netloc, path, qs, ''))

Expand Down
37 changes: 19 additions & 18 deletions gcloud/storage/test_connection.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,50 +141,51 @@ def test___contains___hit(self):

def test_build_api_url_no_extra_query_params(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
klass = self._getTargetClass()
URI = '/'.join([
conn.API_BASE_URL,
klass.API_BASE_URL,
'storage',
conn.API_VERSION,
klass.API_VERSION,
'foo?project=%s' % PROJECT,
])
self.assertEqual(conn.build_api_url('/foo'), URI)
self.assertEqual(klass.build_api_url(PROJECT, '/foo'), URI)

def test_build_api_url_w_extra_query_params(self):
from six.moves.urllib.parse import parse_qsl
from six.moves.urllib.parse import urlsplit
PROJECT = 'project'
conn = self._makeOne(PROJECT)
uri = conn.build_api_url('/foo', {'bar': 'baz'})
klass = self._getTargetClass()
uri = klass.build_api_url(PROJECT, '/foo', query_params={'bar': 'baz'})
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL)
self.assertEqual('%s://%s' % (scheme, netloc), klass.API_BASE_URL)
self.assertEqual(path,
'/'.join(['', 'storage', conn.API_VERSION, 'foo']))
'/'.join(['', 'storage', klass.API_VERSION, 'foo']))
parms = dict(parse_qsl(qs))
self.assertEqual(parms['project'], PROJECT)
self.assertEqual(parms['bar'], 'baz')

def test_build_api_url_w_upload(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
klass = self._getTargetClass()
URI = '/'.join([
conn.API_BASE_URL,
klass.API_BASE_URL,
'upload',
'storage',
conn.API_VERSION,
klass.API_VERSION,
'foo?project=%s' % PROJECT,
])
self.assertEqual(conn.build_api_url('/foo', upload=True), URI)
self.assertEqual(klass.build_api_url(PROJECT, '/foo', upload=True),
URI)

def test_make_request_no_data_no_content_type_no_headers(self):
def test__make_request_no_data_no_content_type_no_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
headers, content = conn.make_request('GET', URI)
headers, content = conn._make_request('GET', URI)
self.assertEqual(headers['status'], '200')
self.assertEqual(headers['content-type'], 'text/plain')
self.assertEqual(content, '')
Expand All @@ -198,15 +199,15 @@ def test_make_request_no_data_no_content_type_no_headers(self):
}
self.assertEqual(http._called_with['headers'], expected_headers)

def test_make_request_w_data_no_extra_headers(self):
def test__make_request_w_data_no_extra_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
conn.make_request('GET', URI, {}, 'application/json')
conn._make_request('GET', URI, {}, 'application/json')
self.assertEqual(http._called_with['method'], 'GET')
self.assertEqual(http._called_with['uri'], URI)
self.assertEqual(http._called_with['body'], {})
Expand All @@ -218,15 +219,15 @@ def test_make_request_w_data_no_extra_headers(self):
}
self.assertEqual(http._called_with['headers'], expected_headers)

def test_make_request_w_extra_headers(self):
def test__make_request_w_extra_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
conn.make_request('GET', URI, headers={'X-Foo': 'foo'})
conn._make_request('GET', URI, headers={'X-Foo': 'foo'})
self.assertEqual(http._called_with['method'], 'GET')
self.assertEqual(http._called_with['uri'], URI)
self.assertEqual(http._called_with['body'], None)
Expand Down

0 comments on commit 9dc8773

Please sign in to comment.