Repository: googleapis/python-storage

Issue:
Storage: mtime of downloaded file is incorrect by UTC offset
Google Cloud Storage v1.25.0
Python 3.7.3
OS: OSX & Win7

Issue: If I upload a file to Google Cloud Storage and then immediately download it, the mtime is incorrect - for me, I'm in EST, so I'm 5 hours behind UTC.  That's the exact timedelta that occurs between the file's original mtime and the recorded mtime after the file is downloaded.

Here's an example screenshot:
![Screen Shot 2020-01-27 at 9 35 52 PM](https://user-images.githubusercontent.com/26491362/73231091-5a360b80-414d-11ea-9001-695bc0757061.png)
The original file mtime in Google Cloud Storage is 1/23/20 9:04 PM (which is correct from the file I uploaded), but when I download the file, the mtime becomes 1/24/20 2:04 AM, which is 5 hours ahead of what is supposed to be (the UTC offset from my timezone).

The issue is here in `blob.download_to_filename`:
```
updated = self.updated
if updated is not None:
            mtime = time.mktime(updated.timetuple())
            os.utime(file_obj.name, (mtime, mtime))
```
In my example, `updated` is the timezone-aware datetime corresponding to `2020-01-24 02:04:11.184000+00:00` (it has `tzinfo==UTC`).  The `updated.timetuple()` is
```
time.struct_time(tm_year=2020, tm_mon=1, tm_mday=24, tm_hour=2, tm_min=4, tm_sec=9, tm_wday=4, tm_yday=24, tm_isdst=0)
```

The problem, I believe, is that the timetuple doesn't know this is a UTC date, nor did it convert to my timezone.  The docs of `mktime` note, "Its argument is the struct_time or full 9-tuple (since the dst flag is needed; use -1 as the dst flag if it is unknown) which expresses the time in local time, not UTC."  Perhaps, we should do this instead:
```
if updated is not None:
   mtime = updated.timestamp() # For Python3, not sure of the Python2 version
   os.utime(file_obj.name, (mtime, mtime))
```

The `timestamp()` function accounts for the timezone information in the datetime object.
I've just been doing this manually in my code after downloading a file because my application is sensitive to mtimes, and it seems to fix the issue.

Discussion:
HemangChothani: PTAL.
frankyn: Thanks for your patience @HemangChothani, this LGTM. IIUC, you're making last update consistent with server side after objects are downloaded to a local file. 


Patch:
diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py
index 1a1aca8..b649384 100644
--- a/google/cloud/storage/_helpers.py
+++ b/google/cloud/storage/_helpers.py
@@ -19,6 +19,7 @@ These are *not* part of the API.
 
 import base64
 from hashlib import md5
+from datetime import datetime
 import os
 
 from google.cloud.storage.constants import _DEFAULT_TIMEOUT
@@ -297,3 +298,17 @@ def _base64_md5hash(buffer_object):
     _write_buffer_to_hash(buffer_object, hash_obj)
     digest_bytes = hash_obj.digest()
     return base64.b64encode(digest_bytes)
+
+
+def _convert_to_timestamp(value):
+    """Convert non-none datetime to timestamp.
+
+    :type value: :class:`datetime.datetime`
+    :param value: The datetime to convert.
+
+    :rtype: int
+    :returns: The timestamp.
+    """
+    utc_naive = value.replace(tzinfo=None) - value.utcoffset()
+    mtime = (utc_naive - datetime(1970, 1, 1)).total_seconds()
+    return mtime
diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py
index e8299d4..7358d57 100644
--- a/google/cloud/storage/blob.py
+++ b/google/cloud/storage/blob.py
@@ -31,8 +31,8 @@ import hashlib
 from io import BytesIO
 import mimetypes
 import os
-import time
 import warnings
+import six
 
 from six.moves.urllib.parse import parse_qsl
 from six.moves.urllib.parse import quote
@@ -57,6 +57,7 @@ from google.cloud.exceptions import NotFound
 from google.cloud.storage._helpers import _get_storage_host
 from google.cloud.storage._helpers import _PropertyMixin
 from google.cloud.storage._helpers import _scalar_property
+from google.cloud.storage._helpers import _convert_to_timestamp
 from google.cloud.storage._signing import generate_signed_url_v2
 from google.cloud.storage._signing import generate_signed_url_v4
 from google.cloud.storage.acl import ACL
@@ -846,7 +847,10 @@ class Blob(_PropertyMixin):
 
         updated = self.updated
         if updated is not None:
-            mtime = time.mktime(updated.timetuple())
+            if six.PY2:
+                mtime = _convert_to_timestamp(updated)
+            else:
+                mtime = updated.timestamp()
             os.utime(file_obj.name, (mtime, mtime))
 
     def download_as_string(self, client=None, start=None, end=None, raw_download=False):


Test patch:
diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py
index f656e64..91af389 100644
--- a/tests/unit/test_blob.py
+++ b/tests/unit/test_blob.py
@@ -1060,7 +1060,7 @@ class Test_Blob(unittest.TestCase):
 
     def _download_to_filename_helper(self, updated, raw_download):
         import os
-        import time
+        from google.cloud.storage._helpers import _convert_to_timestamp
         from google.cloud._testing import _NamedTemporaryFile
 
         blob_name = "blob-name"
@@ -1080,7 +1080,10 @@ class Test_Blob(unittest.TestCase):
                 self.assertIsNone(blob.updated)
             else:
                 mtime = os.path.getmtime(temp.name)
-                updated_time = time.mktime(blob.updated.timetuple())
+                if six.PY2:
+                    updated_time = _convert_to_timestamp(blob.updated)
+                else:
+                    updated_time = blob.updated.timestamp()
                 self.assertEqual(mtime, updated_time)
 
         headers = {"accept-encoding": "gzip"}