fix api endpoints for storage upload/download

Author: silvolu

gcloud/storage/connection.py

@@ -91,7 +91,7 @@ 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,
-                      api_version=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.
@@ -112,9 +112,16 @@ def build_api_url(self, path, query_params=None, api_base_url=None,
                             Typically you shouldn't provide this and instead
                             use the default for the library.
 
+        :type upload: boolean
+        :param upload: True if the URL is for uploading purposes.
+
         :rtype: string
         :returns: The URL assembled from the pieces provided.
         """
+        api_base_url = api_base_url or self.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),

gcloud/storage/key.py

@@ -209,13 +209,14 @@ def download_to_file(self, file_obj):
 
         :raises: :class:`gcloud.storage.exceptions.NotFound`
         """
+
+        download_url = self.media_link
+
         # Use apitools 'Download' facility.
         download = transfer.Download.FromStream(file_obj, auto_transfer=False)
         download.chunksize = self.CHUNK_SIZE
-        download_url = self.connection.build_api_url(
-            path=self.path, query_params={'alt': 'media'})
         headers = {'Range': 'bytes=0-%d' % (self.CHUNK_SIZE - 1)}
-        request = http_wrapper.Request(download_url, 'POST', headers)
+        request = http_wrapper.Request(download_url, 'GET', headers)
 
         download.InitializeDownload(request, self.connection.http)
 
@@ -308,15 +309,16 @@ 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')
+            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)
-        path = url_builder.relative_path.format(bucket=self.bucket.name)
         query_params = url_builder.query_params
-        request.url = conn.build_api_url(path=path, query_params=query_params)
+        request.url = conn.build_api_url(path=self.bucket.path + '/o',
+                                         query_params=query_params,
+                                         upload=True)
         upload.InitializeUpload(request, conn.http)
 
         # Should we be passing callbacks through from caller?  We can't
@@ -628,16 +630,3 @@ def __init__(self, bucket_name, object_name):
         self.query_params = {'name': object_name}
         self._bucket_name = bucket_name
         self._relative_path = ''
-
-    @property
-    def relative_path(self):
-        """Inject bucket name into path."""
-        return self._relative_path.format(bucket=self._bucket_name)
-
-    @relative_path.setter
-    def relative_path(self, value):
-        """Allow update of path template.
-
-        ``value`` should be a string template taking ``{bucket}``.
-        """
-        self._relative_path = value

gcloud/storage/test_connection.py

@@ -150,6 +150,18 @@ def test_build_api_url_w_extra_query_params(self):
         self.assertEqual(parms['project'], PROJECT)
         self.assertEqual(parms['bar'], 'baz')
 
+    def test_build_api_url_w_upload(self):
+        PROJECT = 'project'
+        conn = self._makeOne(PROJECT)
+        URI = '/'.join([
+            conn.API_BASE_URL,
+            'upload',
+            'storage',
+            conn.API_VERSION,
+            'foo?project=%s' % PROJECT,
+        ])
+        self.assertEqual(conn.build_api_url('/foo', upload=True), URI)
+
     def test_make_request_no_data_no_content_type_no_headers(self):
         PROJECT = 'project'
         conn = self._makeOne(PROJECT)

gcloud/storage/test_key.py

@@ -191,7 +191,9 @@ def test_download_to_file(self):
             (chunk2_response, 'def'),
         )
         bucket = _Bucket(connection)
-        key = self._makeOne(bucket, KEY)
+        MEDIA_LINK = 'http://example.com/media/'
+        properties = {'mediaLink': MEDIA_LINK}
+        key = self._makeOne(bucket, KEY, properties)
         key.CHUNK_SIZE = 3
         fh = StringIO()
         key.download_to_file(fh)
@@ -210,7 +212,9 @@ def test_download_to_filename(self):
             (chunk2_response, 'def'),
         )
         bucket = _Bucket(connection)
-        key = self._makeOne(bucket, KEY)
+        MEDIA_LINK = 'http://example.com/media/'
+        properties = {'mediaLink': MEDIA_LINK}
+        key = self._makeOne(bucket, KEY, properties)
         key.CHUNK_SIZE = 3
         with NamedTemporaryFile() as f:
             key.download_to_filename(f.name)
@@ -231,7 +235,9 @@ def test_download_as_string(self):
             (chunk2_response, 'def'),
         )
         bucket = _Bucket(connection)
-        key = self._makeOne(bucket, KEY)
+        MEDIA_LINK = 'http://example.com/media/'
+        properties = {'mediaLink': MEDIA_LINK}
+        key = self._makeOne(bucket, KEY, properties)
         key.CHUNK_SIZE = 3
         fetched = key.download_as_string()
         self.assertEqual(fetched, 'abcdef')
@@ -261,7 +267,7 @@ def test_upload_from_file_simple(self):
         scheme, netloc, path, qs, _ = urlsplit(uri)
         self.assertEqual(scheme, 'http')
         self.assertEqual(netloc, 'example.com')
-        self.assertEqual(path, '/upload/storage/v1/b/name/o')
+        self.assertEqual(path, '/b/name/o')
         self.assertEqual(dict(parse_qsl(qs)),
                          {'uploadType': 'media', 'name': 'key'})
         headers = dict(
@@ -305,7 +311,7 @@ def test_upload_from_file_resumable(self):
         scheme, netloc, path, qs, _ = urlsplit(uri)
         self.assertEqual(scheme, 'http')
         self.assertEqual(netloc, 'example.com')
-        self.assertEqual(path, '/resumable/upload/storage/v1/b/name/o')
+        self.assertEqual(path, '/b/name/o')
         self.assertEqual(dict(parse_qsl(qs)),
                          {'uploadType': 'resumable', 'name': 'key'})
         headers = dict(
@@ -360,7 +366,7 @@ def test_upload_from_file_w_slash_in_name(self):
         scheme, netloc, path, qs, _ = urlsplit(uri)
         self.assertEqual(scheme, 'http')
         self.assertEqual(netloc, 'example.com')
-        self.assertEqual(path, '/upload/storage/v1/b/name/o')
+        self.assertEqual(path, '/b/name/o')
         self.assertEqual(dict(parse_qsl(qs)),
                          {'uploadType': 'media', 'name': 'parent/child'})
         headers = dict(
@@ -400,7 +406,7 @@ def test_upload_from_filename(self):
         scheme, netloc, path, qs, _ = urlsplit(uri)
         self.assertEqual(scheme, 'http')
         self.assertEqual(netloc, 'example.com')
-        self.assertEqual(path, '/upload/storage/v1/b/name/o')
+        self.assertEqual(path, '/b/name/o')
         self.assertEqual(dict(parse_qsl(qs)),
                          {'uploadType': 'media', 'name': 'key'})
         headers = dict(
@@ -436,7 +442,7 @@ def test_upload_from_string(self):
         scheme, netloc, path, qs, _ = urlsplit(uri)
         self.assertEqual(scheme, 'http')
         self.assertEqual(netloc, 'example.com')
-        self.assertEqual(path, '/upload/storage/v1/b/name/o')
+        self.assertEqual(path, '/b/name/o')
         self.assertEqual(dict(parse_qsl(qs)),
                          {'uploadType': 'media', 'name': 'key'})
         headers = dict(
@@ -806,10 +812,13 @@ def api_request(self, **kw):
         return self._respond(**kw)
 
     def build_api_url(self, path, query_params=None,
-                      api_base_url=API_BASE_URL):
+                      api_base_url=API_BASE_URL, upload=False):
         from urllib import urlencode
         from urlparse import urlsplit
         from urlparse 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 {})
         scheme, netloc, _, _, _ = urlsplit(api_base_url)
         return urlunsplit((scheme, netloc, path, qs, ''))