Feat: Add "auto" checksum option and make default

Author: andrewsg

google/cloud/storage/_helpers.py

@@ -651,9 +651,6 @@ def _api_core_retry_to_resumable_media_retry(retry):
 def _get_invocation_id():
     return "gccl-invocation-id/" + str(uuid4())
 
-def _detect_supported_checksums():
-    """Returns 'crc32c' if a fast crc32c impl. is available, else 'md5'"""
-    
 
 def _get_default_headers(
     user_agent,

google/cloud/storage/_media/_download.py

@@ -136,14 +136,22 @@ class Download(DownloadBase):
     """
 
     def __init__(
-        self, media_url, stream=None, start=None, end=None, headers=None, checksum="auto"
+        self,
+        media_url,
+        stream=None,
+        start=None,
+        end=None,
+        headers=None,
+        checksum="auto",
     ):
         super(Download, self).__init__(
             media_url, stream=stream, start=start, end=end, headers=headers
         )
         self.checksum = checksum
         if self.checksum == "auto":
-            self.checksum = "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            self.checksum = (
+                "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            )
         self._bytes_downloaded = 0
         self._expected_checksum = None
         self._checksum_object = None

google/cloud/storage/_media/_helpers.py

@@ -20,7 +20,6 @@
 import hashlib
 import logging
 import random
-import warnings
 
 from urllib.parse import parse_qs
 from urllib.parse import urlencode
@@ -294,6 +293,7 @@ def _get_checksum_object(checksum_type):
         # In order to support platforms that don't have google_crc32c
         # support, only perform the import on demand.
         import google_crc32c
+
         return google_crc32c.Checksum()
     elif checksum_type is None:
         return None
@@ -304,6 +304,7 @@ def _get_checksum_object(checksum_type):
 def _is_crc32c_available_and_fast():
     try:
         import google_crc32c
+
         if google_crc32c.implementation == "c":
             return True
     except Exception:

google/cloud/storage/_media/_upload.py

@@ -265,7 +265,9 @@ def __init__(self, upload_url, headers=None, checksum="auto"):
         super(MultipartUpload, self).__init__(upload_url, headers=headers)
         self._checksum_type = checksum
         if self._checksum_type == "auto":
-            self.checksum_type = "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            self._checksum_type = (
+                "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            )
 
     def _prepare_request(self, data, metadata, content_type):
         """Prepare the contents of an HTTP request.
@@ -390,7 +392,9 @@ def __init__(self, upload_url, chunk_size, checksum="auto", headers=None):
         self._bytes_checksummed = 0
         self._checksum_type = checksum
         if self._checksum_type == "auto":
-            self.checksum_type = "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            self._checksum_type = (
+                "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            )
         self._checksum_object = None
         self._total_bytes = None
         self._resumable_url = None
@@ -1228,7 +1232,9 @@ def __init__(
         self._etag = None
         self._checksum_type = checksum
         if self._checksum_type == "auto":
-            self.checksum_type = "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            self._checksum_type = (
+                "crc32c" if _helpers._is_crc32c_available_and_fast() else "md5"
+            )
         self._checksum_object = None
 
     @property

tests/resumable_media/unit/requests/test_upload.py

@@ -389,7 +389,9 @@ def test_mpu_container_cancel():
 
 
 def test_mpu_part(filename):
-    part = upload_mod.XMLMPUPart(EXAMPLE_XML_UPLOAD_URL, UPLOAD_ID, filename, 0, 128, 1)
+    part = upload_mod.XMLMPUPart(
+        EXAMPLE_XML_UPLOAD_URL, UPLOAD_ID, filename, 0, 128, 1, checksum=None
+    )
 
     transport = mock.Mock(spec=["request"])
     transport.request.return_value = _make_response(headers={"etag": PARTS[1]})

tests/resumable_media/unit/test__helpers.py

@@ -209,80 +209,26 @@ def test__get_checksum_object_invalid():
         _helpers._get_checksum_object("invalid")
 
 
-# @mock.patch("builtins.__import__")
-# def test__get_crc32_object_wo_google_crc32c_wo_crcmod(mock_import):
-#     mock_import.side_effect = ImportError("testing")
+def test__is_crc32c_available_and_fast():
+    import sys
 
-#     with pytest.raises(ImportError):
-#         _helpers._get_crc32c_object()
+    import google_crc32c
 
-#     expected_calls = [
-#         mock.call("google_crc32c", mock.ANY, None, None, 0),
-#         mock.call("crcmod", mock.ANY, None, None, 0),
-#     ]
-#     mock_import.assert_has_calls(expected_calls)
+    assert google_crc32c.implementation == "c"
+    assert _helpers._is_crc32c_available_and_fast() is True
 
+    del sys.modules["google_crc32c"]
+    with mock.patch("builtins.__import__", side_effect=ImportError):
+        assert _helpers._is_crc32c_available_and_fast() is False
 
-# @mock.patch("builtins.__import__")
-# def test__get_crc32_object_w_google_crc32c(mock_import):
-#     google_crc32c = mock.Mock(spec=["Checksum"])
-#     mock_import.return_value = google_crc32c
+    import google_crc32c
 
-#     found = _helpers._get_crc32c_object()
+    assert google_crc32c.implementation == "c"
+    with mock.patch("google_crc32c.implementation", new="python"):
+        assert _helpers._is_crc32c_available_and_fast() is False
 
-#     assert found is google_crc32c.Checksum.return_value
-#     google_crc32c.Checksum.assert_called_once_with()
-
-#     mock_import.assert_called_once_with("google_crc32c", mock.ANY, None, None, 0)
-
-
-# @mock.patch("builtins.__import__")
-# def test__get_crc32_object_wo_google_crc32c_w_crcmod(mock_import):
-#     crcmod = mock.Mock(spec=["predefined", "crcmod"])
-#     crcmod.predefined = mock.Mock(spec=["Crc"])
-#     crcmod.crcmod = mock.Mock(spec=["_usingExtension"])
-#     mock_import.side_effect = [ImportError("testing"), crcmod, crcmod.crcmod]
-
-#     found = _helpers._get_crc32c_object()
-
-#     assert found is crcmod.predefined.Crc.return_value
-#     crcmod.predefined.Crc.assert_called_once_with("crc-32c")
-
-#     expected_calls = [
-#         mock.call("google_crc32c", mock.ANY, None, None, 0),
-#         mock.call("crcmod", mock.ANY, None, None, 0),
-#         mock.call("crcmod.crcmod", mock.ANY, {}, ["_usingExtension"], 0),
-#     ]
-#     mock_import.assert_has_calls(expected_calls)
-
-
-@pytest.mark.filterwarnings("ignore::RuntimeWarning")
-@mock.patch("builtins.__import__")
-def test__is_fast_crcmod_wo_extension_warning(mock_import):
-    crcmod = mock.Mock(spec=["crcmod"])
-    crcmod.crcmod = mock.Mock(spec=["_usingExtension"])
-    crcmod.crcmod._usingExtension = False
-    mock_import.return_value = crcmod.crcmod
-
-    assert not _helpers._is_fast_crcmod()
-
-    mock_import.assert_called_once_with(
-        "crcmod.crcmod",
-        mock.ANY,
-        {},
-        ["_usingExtension"],
-        0,
-    )
-
-
-@mock.patch("builtins.__import__")
-def test__is_fast_crcmod_w_extension(mock_import):
-    crcmod = mock.Mock(spec=["crcmod"])
-    crcmod.crcmod = mock.Mock(spec=["_usingExtension"])
-    crcmod.crcmod._usingExtension = True
-    mock_import.return_value = crcmod.crcmod
-
-    assert _helpers._is_fast_crcmod()
+    # Run this again to confirm we're back to the initial state.
+    assert _helpers._is_crc32c_available_and_fast() is True
 
 
 def test__DoNothingHash():
@@ -312,7 +258,7 @@ def _get_headers(response):
 
         checksum_types = {
             "md5": type(hashlib.md5()),
-            "crc32c": type(_helpers._get_crc32c_object()),
+            "crc32c": type(google_crc32c.Checksum()),
         }
         assert isinstance(checksum_obj, checksum_types[checksum])
 

tests/resumable_media/unit/test__upload.py

@@ -191,7 +191,7 @@ def test_constructor_defaults(self):
         upload = _upload.MultipartUpload(MULTIPART_URL)
         assert upload.upload_url == MULTIPART_URL
         assert upload._headers == {}
-        assert upload._checksum_type is None
+        assert upload._checksum_type == "crc32c"  # converted from "auto"
         assert not upload._finished
         _check_retry_strategy(upload)
 
@@ -204,6 +204,17 @@ def test_constructor_explicit(self):
         assert not upload._finished
         _check_retry_strategy(upload)
 
+    def test_constructor_explicit_auto(self):
+        headers = {"spin": "doctors"}
+        upload = _upload.MultipartUpload(
+            MULTIPART_URL, headers=headers, checksum="auto"
+        )
+        assert upload.upload_url == MULTIPART_URL
+        assert upload._headers is headers
+        assert upload._checksum_type == "crc32c"
+        assert not upload._finished
+        _check_retry_strategy(upload)
+
     def test__prepare_request_already_finished(self):
         upload = _upload.MultipartUpload(MULTIPART_URL)
         upload._finished = True
@@ -345,7 +356,7 @@ def test_constructor(self):
         assert upload._checksum_object is None
         assert upload._total_bytes is None
         assert upload._resumable_url is None
-        assert upload._checksum_type is None
+        assert upload._checksum_type == "crc32c"  # converted from "auto"
 
     def test_constructor_bad_chunk_size(self):
         with pytest.raises(ValueError):
@@ -771,7 +782,7 @@ def test__process_resumable_response_bad_status(self):
         assert upload.invalid
 
     def test__process_resumable_response_success(self):
-        upload = _upload.ResumableUpload(RESUMABLE_URL, ONE_MB)
+        upload = _upload.ResumableUpload(RESUMABLE_URL, ONE_MB, checksum=None)
         _fix_up_virtual(upload)
 
         # Check / set status before.
@@ -1401,6 +1412,28 @@ def test_xml_mpu_part(filename):
         END,
         PART_NUMBER,
         headers=EXAMPLE_HEADERS,
+        checksum="auto",
+    )
+    assert part.upload_url == EXAMPLE_XML_UPLOAD_URL
+    assert part.upload_id == UPLOAD_ID
+    assert part.filename == filename
+    assert part.etag is None
+    assert part.start == START
+    assert part.end == END
+    assert part.part_number == PART_NUMBER
+    assert part._headers == EXAMPLE_HEADERS
+    assert part._checksum_type == "crc32c"  # transformed from "auto"
+    assert part._checksum_object is None
+
+    part = _upload.XMLMPUPart(
+        EXAMPLE_XML_UPLOAD_URL,
+        UPLOAD_ID,
+        filename,
+        START,
+        END,
+        PART_NUMBER,
+        headers=EXAMPLE_HEADERS,
+        checksum=None,
     )
     verb, url, payload, headers = part._prepare_upload_request()
     assert verb == _upload._PUT