Merge pull request #629 from dhermes/fix-536

DISCUSSION PR: Uses Blob's current content type (in an upload) when present.

Author: dhermes

gcloud/storage/blob.py

@@ -274,6 +274,11 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
                          content_type=None, num_retries=6):
         """Upload the contents of this blob from a file-like object.
 
+        The content type of the upload will either be
+        - The value passed in to the function (if any)
+        - The value stored on the current blob
+        - The default value of 'application/octet-stream'
+
         .. note::
            The effect of uploading to an existing blob depends on the
            "versioning" and "lifecycle" policies defined on the blob's
@@ -296,7 +301,16 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
         :param size: The number of bytes to read from the file handle.
                      If not provided, we'll try to guess the size using
                      :func:`os.fstat`
+
+        :type content_type: string or ``NoneType``
+        :param content_type: Optional type of content being uploaded.
+
+        :type num_retries: integer
+        :param num_retries: Number of upload retries. Defaults to 6.
         """
+        content_type = (content_type or self._properties.get('contentType') or
+                        'application/octet-stream')
+
         # Rewind the file if desired.
         if rewind:
             file_obj.seek(0, os.SEEK_SET)
@@ -310,9 +324,8 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
             'User-Agent': conn.USER_AGENT,
         }
 
-        upload = transfer.Upload(file_obj,
-                                 content_type or 'application/unknown',
-                                 total_bytes, auto_transfer=False,
+        upload = transfer.Upload(file_obj, content_type, total_bytes,
+                                 auto_transfer=False,
                                  chunksize=self.CHUNK_SIZE)
 
         url_builder = _UrlBuilder(bucket_name=self.bucket.name,
@@ -342,9 +355,14 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
         else:
             http_wrapper.MakeRequest(conn.http, request, retries=num_retries)
 
-    def upload_from_filename(self, filename):
+    def upload_from_filename(self, filename, content_type=None):
         """Upload this blob's contents from the content of a named file.
 
+        The content type of the upload will either be
+        - The value passed in to the function (if any)
+        - The value stored on the current blob
+        - The value given by mimetypes.guess_type
+
         .. note::
            The effect of uploading to an existing blob depends on the
            "versioning" and "lifecycle" policies defined on the blob's
@@ -358,8 +376,13 @@ def upload_from_filename(self, filename):
 
         :type filename: string
         :param filename: The path to the file.
+
+        :type content_type: string or ``NoneType``
+        :param content_type: Optional type of content being uploaded.
         """
-        content_type, _ = mimetypes.guess_type(filename)
+        content_type = content_type or self._properties.get('contentType')
+        if content_type is None:
+            content_type, _ = mimetypes.guess_type(filename)
 
         with open(filename, 'rb') as file_obj:
             self.upload_from_file(file_obj, content_type=content_type)

gcloud/storage/test_blob.py

@@ -327,7 +327,9 @@ def test_download_as_string(self):
         fetched = blob.download_as_string()
         self.assertEqual(fetched, b'abcdef')
 
-    def test_upload_from_file_simple(self):
+    def _upload_from_file_simple_test_helper(self, properties=None,
+                                             content_type_arg=None,
+                                             expected_content_type=None):
         from six.moves.http_client import OK
         from six.moves.urllib.parse import parse_qsl
         from six.moves.urllib.parse import urlsplit
@@ -339,12 +341,13 @@ def test_upload_from_file_simple(self):
             (response, b''),
         )
         bucket = _Bucket(connection)
-        blob = self._makeOne(BLOB_NAME, bucket=bucket)
+        blob = self._makeOne(BLOB_NAME, bucket=bucket, properties=properties)
         blob.CHUNK_SIZE = 5
         with NamedTemporaryFile() as fh:
             fh.write(DATA)
             fh.flush()
-            blob.upload_from_file(fh, rewind=True)
+            blob.upload_from_file(fh, rewind=True,
+                                  content_type=content_type_arg)
         rq = connection.http._requested
         self.assertEqual(len(rq), 1)
         self.assertEqual(rq[0]['method'], 'POST')
@@ -358,7 +361,31 @@ def test_upload_from_file_simple(self):
         headers = dict(
             [(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
         self.assertEqual(headers['Content-Length'], '6')
-        self.assertEqual(headers['Content-Type'], 'application/unknown')
+        self.assertEqual(headers['Content-Type'], expected_content_type)
+
+    def test_upload_from_file_simple(self):
+        self._upload_from_file_simple_test_helper(
+            expected_content_type='application/octet-stream')
+
+    def test_upload_from_file_simple_with_content_type(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        self._upload_from_file_simple_test_helper(
+            properties={'contentType': EXPECTED_CONTENT_TYPE},
+            expected_content_type=EXPECTED_CONTENT_TYPE)
+
+    def test_upload_from_file_simple_with_content_type_passed(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        self._upload_from_file_simple_test_helper(
+            content_type_arg=EXPECTED_CONTENT_TYPE,
+            expected_content_type=EXPECTED_CONTENT_TYPE)
+
+    def test_upload_from_file_simple_both_content_type_sources(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        ALT_CONTENT_TYPE = 'foo/baz'
+        self._upload_from_file_simple_test_helper(
+            properties={'contentType': ALT_CONTENT_TYPE},
+            content_type_arg=EXPECTED_CONTENT_TYPE,
+            expected_content_type=EXPECTED_CONTENT_TYPE)
 
     def test_upload_from_file_resumable(self):
         from six.moves.http_client import OK
@@ -403,7 +430,7 @@ def test_upload_from_file_resumable(self):
             [(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
         self.assertEqual(headers['X-Upload-Content-Length'], '6')
         self.assertEqual(headers['X-Upload-Content-Type'],
-                         'application/unknown')
+                         'application/octet-stream')
         self.assertEqual(rq[1]['method'], 'PUT')
         self.assertEqual(rq[1]['uri'], UPLOAD_URL)
         headers = dict(
@@ -457,9 +484,11 @@ def test_upload_from_file_w_slash_in_name(self):
         headers = dict(
             [(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
         self.assertEqual(headers['Content-Length'], '6')
-        self.assertEqual(headers['Content-Type'], 'application/unknown')
+        self.assertEqual(headers['Content-Type'], 'application/octet-stream')
 
-    def test_upload_from_filename(self):
+    def _upload_from_filename_test_helper(self, properties=None,
+                                          content_type_arg=None,
+                                          expected_content_type=None):
         from six.moves.http_client import OK
         from six.moves.urllib.parse import parse_qsl
         from six.moves.urllib.parse import urlsplit
@@ -478,12 +507,13 @@ def test_upload_from_filename(self):
             (chunk2_response, ''),
         )
         bucket = _Bucket(connection)
-        blob = self._makeOne(BLOB_NAME, bucket=bucket)
+        blob = self._makeOne(BLOB_NAME, bucket=bucket,
+                             properties=properties)
         blob.CHUNK_SIZE = 5
         with NamedTemporaryFile(suffix='.jpeg') as fh:
             fh.write(DATA)
             fh.flush()
-            blob.upload_from_filename(fh.name)
+            blob.upload_from_filename(fh.name, content_type=content_type_arg)
         rq = connection.http._requested
         self.assertEqual(len(rq), 1)
         self.assertEqual(rq[0]['method'], 'POST')
@@ -497,7 +527,31 @@ def test_upload_from_filename(self):
         headers = dict(
             [(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
         self.assertEqual(headers['Content-Length'], '6')
-        self.assertEqual(headers['Content-Type'], 'image/jpeg')
+        self.assertEqual(headers['Content-Type'], expected_content_type)
+
+    def test_upload_from_filename(self):
+        self._upload_from_filename_test_helper(
+            expected_content_type='image/jpeg')
+
+    def test_upload_from_filename_with_content_type(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        self._upload_from_filename_test_helper(
+            properties={'contentType': EXPECTED_CONTENT_TYPE},
+            expected_content_type=EXPECTED_CONTENT_TYPE)
+
+    def test_upload_from_filename_with_content_type_passed(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        self._upload_from_filename_test_helper(
+            content_type_arg=EXPECTED_CONTENT_TYPE,
+            expected_content_type=EXPECTED_CONTENT_TYPE)
+
+    def test_upload_from_filename_both_content_type_sources(self):
+        EXPECTED_CONTENT_TYPE = 'foo/bar'
+        ALT_CONTENT_TYPE = 'foo/baz'
+        self._upload_from_filename_test_helper(
+            properties={'contentType': ALT_CONTENT_TYPE},
+            content_type_arg=EXPECTED_CONTENT_TYPE,
+            expected_content_type=EXPECTED_CONTENT_TYPE)
 
     def test_upload_from_string_w_bytes(self):
         from six.moves.http_client import OK