cleanup(storage): use Disable{MD5*,Crc32c*} consistently (#7034)

coryan · web-flow · commit 69cc6a82d313 · 2021-07-20T17:10:10.000-04:00
All GCS options are implemented as an `absl::optional&lt;T&gt;`, in the case
of `DisableMD5Hash` and `DisableCrc32cChecksum` this is
`absl::optional&lt;bool&gt;`. The MD5 hashes are disabled by default, this is
achieved by having the default constructor for `DisableMD5Hash`
initialize the option to `true`. CRC32C checksums are enabled by
default, the default constructor initializes the optional to
`absl::nullopt` (this is consistent with all the other options).

After this change we always use these options by doing:

```cc
auto value = request.GetOption&lt;Disable*&gt;().value_or(false);
```

This makes the usage consistent, and only the constructor encodes
what the default is.
diff --git a/google/cloud/storage/internal/curl_client.cc b/google/cloud/storage/internal/curl_client.cc
@@ -535,7 +535,7 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMedia(
   // If the application has set an explicit hash value we need to use multipart
   // uploads. `DisableMD5Hash` and `DisableCrc32cChecksum` should not be
   // dependent on each other.
-  if (!request.GetOption<DisableMD5Hash>().value() ||
+  if (!request.GetOption<DisableMD5Hash>().value_or(false) ||
       !request.GetOption<DisableCrc32cChecksum>().value_or(false) ||
       request.HasOption<MD5HashValue>() ||
       request.HasOption<Crc32cChecksumValue>()) {
@@ -1236,7 +1236,7 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMediaXml(
   if (request.HasOption<MD5HashValue>()) {
     builder.AddHeader("x-goog-hash: md5=" +
                       request.GetOption<MD5HashValue>().value());
-  } else if (!request.GetOption<DisableMD5Hash>().value()) {
+  } else if (!request.GetOption<DisableMD5Hash>().value_or(false)) {
     builder.AddHeader("x-goog-hash: md5=" + ComputeMD5Hash(request.contents()));
   }
   if (request.HasOption<Crc32cChecksumValue>()) {
@@ -1350,7 +1350,7 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMediaMultipart(
   }
   if (request.HasOption<MD5HashValue>()) {
     metadata["md5Hash"] = request.GetOption<MD5HashValue>().value();
-  } else if (!request.GetOption<DisableMD5Hash>().value()) {
+  } else if (!request.GetOption<DisableMD5Hash>().value_or(false)) {
     metadata["md5Hash"] = ComputeMD5Hash(request.contents());
   }
 
diff --git a/google/cloud/storage/internal/hash_function.cc b/google/cloud/storage/internal/hash_function.cc
@@ -45,7 +45,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
   if (request.RequiresRangeHeader()) return CreateNullHashFunction();
   return CreateHashFunction(
       request.GetOption<DisableCrc32cChecksum>().value_or(false),
-      request.GetOption<DisableMD5Hash>().value_or(true));
+      request.GetOption<DisableMD5Hash>().value_or(false));
 }
 
 std::unique_ptr<HashFunction> CreateHashFunction(
@@ -55,7 +55,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
   return CreateHashFunction(
       request.GetOption<DisableCrc32cChecksum>().value_or(false) ||
           !request.GetOption<Crc32cChecksumValue>().value_or("").empty(),
-      request.GetOption<DisableMD5Hash>().value_or(true) ||
+      request.GetOption<DisableMD5Hash>().value_or(false) ||
           !request.GetOption<MD5HashValue>().value_or("").empty());
 }
 
diff --git a/google/cloud/storage/internal/hash_validator.cc b/google/cloud/storage/internal/hash_validator.cc
@@ -52,18 +52,18 @@ std::unique_ptr<HashValidator> CreateHashValidator(
   if (request.RequiresRangeHeader()) return CreateNullHashValidator();
 
   // `DisableMD5Hash`'s default value is `true`.
-  auto disable_md5 = request.GetOption<DisableMD5Hash>().value();
-  auto disable_crc32c = request.HasOption<DisableCrc32cChecksum>() &&
-                        request.GetOption<DisableCrc32cChecksum>().value();
+  auto disable_md5 = request.GetOption<DisableMD5Hash>().value_or(false);
+  auto disable_crc32c =
+      request.GetOption<DisableCrc32cChecksum>().value_or(false);
   return CreateHashValidator(disable_md5, disable_crc32c);
 }
 
 std::unique_ptr<HashValidator> CreateHashValidator(
     ResumableUploadRequest const& request) {
   // `DisableMD5Hash`'s default value is `true`.
-  auto disable_md5 = request.GetOption<DisableMD5Hash>().value();
-  auto disable_crc32c = request.HasOption<DisableCrc32cChecksum>() &&
-                        request.GetOption<DisableCrc32cChecksum>().value();
+  auto disable_md5 = request.GetOption<DisableMD5Hash>().value_or(false);
+  auto disable_crc32c =
+      request.GetOption<DisableCrc32cChecksum>().value_or(false);
   return CreateHashValidator(disable_md5, disable_crc32c);
 }
 
