feat(storage): easier mocks with ObjectMetadata (#9899)

Recapitulates the changes to make `BucketMetadata` easier to mock. This
change adds modifiers for all the fields in `ObjectMetadata` and removes
the need for `internal::CommonMetadata<>`.

Since `internal::CommonMetadata<>` is not needed, I deprecated the
class, removed all references to the class or its header file, and
refactor any useful code out of its header file.

google/cloud/storage/bucket_metadata.h

@@ -16,10 +16,10 @@
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_BUCKET_METADATA_H
 
 #include "google/cloud/storage/bucket_access_control.h"
-#include "google/cloud/storage/internal/common_metadata.h"
 #include "google/cloud/storage/internal/patch_builder.h"
 #include "google/cloud/storage/lifecycle_rule.h"
 #include "google/cloud/storage/object_access_control.h"
+#include "google/cloud/storage/owner.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/optional.h"
 #include "absl/types/optional.h"

google/cloud/storage/google_cloud_cpp_storage.bzl

@@ -122,6 +122,7 @@ google_cloud_cpp_storage_hdrs = [
     "object_write_stream.h",
     "options.h",
     "override_default_project.h",
+    "owner.h",
     "parallel_upload.h",
     "policy_document.h",
     "retry_policy.h",

google/cloud/storage/google_cloud_cpp_storage.cmake

@@ -208,6 +208,7 @@ add_library(
     object_write_stream.h
     options.h
     override_default_project.h
+    owner.h
     parallel_upload.cc
     parallel_upload.h
     policy_document.cc

google/cloud/storage/internal/access_control_common.h

@@ -15,7 +15,6 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ACCESS_CONTROL_COMMON_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ACCESS_CONTROL_COMMON_H
 
-#include "google/cloud/storage/internal/common_metadata.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/status.h"
 #include "absl/types/optional.h"

google/cloud/storage/internal/bucket_metadata_parser.cc

@@ -14,8 +14,8 @@
 
 #include "google/cloud/storage/internal/bucket_metadata_parser.h"
 #include "google/cloud/storage/internal/bucket_access_control_parser.h"
-#include "google/cloud/storage/internal/common_metadata_parser.h"
 #include "google/cloud/storage/internal/lifecycle_rule_parser.h"
+#include "google/cloud/storage/internal/metadata_parser.h"
 #include "google/cloud/storage/internal/object_access_control_parser.h"
 #include "absl/strings/str_format.h"
 #include <nlohmann/json.hpp>

google/cloud/storage/internal/common_metadata.h

@@ -15,6 +15,7 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_COMMON_METADATA_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_COMMON_METADATA_H
 
+#include "google/cloud/storage/owner.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/status_or.h"
 #include "absl/types/optional.h"
@@ -27,38 +28,6 @@ namespace google {
 namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
-/// A simple wrapper for the `owner` field in `internal::CommonMetadata`.
-struct Owner {
-  std::string entity;
-  std::string entity_id;
-};
-
-inline bool operator==(Owner const& lhs, Owner const& rhs) {
-  return std::tie(lhs.entity, lhs.entity_id) ==
-         std::tie(rhs.entity, rhs.entity_id);
-}
-
-inline bool operator<(Owner const& lhs, Owner const& rhs) {
-  return std::tie(lhs.entity, lhs.entity_id) <
-         std::tie(rhs.entity, rhs.entity_id);
-}
-
-inline bool operator!=(Owner const& lhs, Owner const& rhs) {
-  return std::rel_ops::operator!=(lhs, rhs);
-}
-
-inline bool operator>(Owner const& lhs, Owner const& rhs) {
-  return std::rel_ops::operator>(lhs, rhs);
-}
-
-inline bool operator<=(Owner const& lhs, Owner const& rhs) {
-  return std::rel_ops::operator<=(lhs, rhs);
-}
-
-inline bool operator>=(Owner const& lhs, Owner const& rhs) {
-  return std::rel_ops::operator>=(lhs, rhs);
-}
-
 namespace internal {
 struct GrpcBucketMetadataParser;
 struct GrpcObjectMetadataParser;
@@ -73,8 +42,10 @@ struct CommonMetadataParser;
  *
  * @see https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern
  */
+// TODO(#9897) - remove this class and any references to it
 template <typename Derived>
-class CommonMetadata {
+class GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01") CommonMetadata {
  public:
   CommonMetadata() = default;
 
@@ -121,8 +92,10 @@ class CommonMetadata {
 };
 
 template <typename T>
-inline bool operator==(CommonMetadata<T> const& lhs,
-                       CommonMetadata<T> const& rhs) {
+GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01")
+inline bool
+operator==(CommonMetadata<T> const& lhs, CommonMetadata<T> const& rhs) {
   // etag changes each time the metadata changes, so that is the best field
   // to short-circuit this comparison.  The check the name, project number,
   // and metadata generation, which have the next best chance to
@@ -138,8 +111,10 @@ inline bool operator==(CommonMetadata<T> const& lhs,
 }
 
 template <typename T>
-inline bool operator!=(CommonMetadata<T> const& lhs,
-                       CommonMetadata<T> const& rhs) {
+GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01")
+inline bool
+operator!=(CommonMetadata<T> const& lhs, CommonMetadata<T> const& rhs) {
   return std::rel_ops::operator!=(lhs, rhs);
 }
 

google/cloud/storage/internal/common_metadata_parser.h

@@ -20,13 +20,17 @@
 #include "google/cloud/status.h"
 #include <nlohmann/json.hpp>
 #include <string>
+
 namespace google {
 namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
+// TODO(#9897) - remove this class and any references to it
 template <typename Derived>
-struct CommonMetadataParser {
+struct GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01")
+    CommonMetadataParser {
   static Status FromJson(CommonMetadata<Derived>& result,
                          nlohmann::json const& json) {
     if (!json.is_object()) {

google/cloud/storage/internal/grpc_object_metadata_parser.cc

@@ -86,13 +86,13 @@ ObjectMetadata GrpcObjectMetadataParser::FromProto(
   };
 
   ObjectMetadata metadata;
-  metadata.kind_ = "storage#object";
-  metadata.bucket_ = bucket_id(object);
-  metadata.name_ = std::move(*object.mutable_name());
-  metadata.generation_ = object.generation();
-  metadata.etag_ = object.etag();
-  metadata.id_ = metadata.bucket() + "/" + metadata.name() + "/" +
-                 std::to_string(metadata.generation());
+  metadata.set_kind("storage#object");
+  metadata.set_bucket(bucket_id(object));
+  metadata.set_name(std::move(*object.mutable_name()));
+  metadata.set_generation(object.generation());
+  metadata.set_etag(object.etag());
+  metadata.set_id(metadata.bucket() + "/" + metadata.name() + "/" +
+                  std::to_string(metadata.generation()));
   auto const metadata_endpoint = [&options]() -> std::string {
     if (options.get<RestEndpointOption>() != "https://storage.googleapis.com") {
       return options.get<RestEndpointOption>();
@@ -104,23 +104,23 @@ ObjectMetadata GrpcObjectMetadataParser::FromProto(
     return "/storage/" + options.get<TargetApiVersionOption>();
   }();
   auto const rel_path = "/b/" + metadata.bucket() + "/o/" + metadata.name();
-  metadata.self_link_ = metadata_endpoint + path + rel_path;
-  metadata.media_link_ =
+  metadata.set_self_link(metadata_endpoint + path + rel_path);
+  metadata.set_media_link(
       options.get<RestEndpointOption>() + "/download" + path + rel_path +
-      "?generation=" + std::to_string(metadata.generation()) + "&alt=media";
+      "?generation=" + std::to_string(metadata.generation()) + "&alt=media");
 
-  metadata.metageneration_ = object.metageneration();
+  metadata.set_metageneration(object.metageneration());
   if (object.has_owner()) {
-    metadata.owner_ = storage_internal::FromProto(*object.mutable_owner());
+    metadata.set_owner(storage_internal::FromProto(*object.mutable_owner()));
   }
-  metadata.storage_class_ = std::move(*object.mutable_storage_class());
+  metadata.set_storage_class(std::move(*object.mutable_storage_class()));
   if (object.has_create_time()) {
-    metadata.time_created_ =
-        google::cloud::internal::ToChronoTimePoint(object.create_time());
+    metadata.set_time_created(
+        google::cloud::internal::ToChronoTimePoint(object.create_time()));
   }
   if (object.has_update_time()) {
-    metadata.updated_ =
-        google::cloud::internal::ToChronoTimePoint(object.update_time());
+    metadata.set_updated(
+        google::cloud::internal::ToChronoTimePoint(object.update_time()));
   }
   std::vector<ObjectAccessControl> acl;
   acl.reserve(object.acl_size());
@@ -129,53 +129,53 @@ ObjectMetadata GrpcObjectMetadataParser::FromProto(
         std::move(item), metadata.bucket(), metadata.name(),
         metadata.generation()));
   }
-  metadata.acl_ = std::move(acl);
-  metadata.cache_control_ = std::move(*object.mutable_cache_control());
-  metadata.component_count_ = object.component_count();
-  metadata.content_disposition_ =
-      std::move(*object.mutable_content_disposition());
-  metadata.content_encoding_ = std::move(*object.mutable_content_encoding());
-  metadata.content_language_ = std::move(*object.mutable_content_language());
-  metadata.content_type_ = std::move(*object.mutable_content_type());
+  metadata.set_acl(std::move(acl));
+  metadata.set_cache_control(std::move(*object.mutable_cache_control()));
+  metadata.set_component_count(object.component_count());
+  metadata.set_content_disposition(
+      std::move(*object.mutable_content_disposition()));
+  metadata.set_content_encoding(std::move(*object.mutable_content_encoding()));
+  metadata.set_content_language(std::move(*object.mutable_content_language()));
+  metadata.set_content_type(std::move(*object.mutable_content_type()));
   if (object.has_checksums()) {
     if (object.checksums().has_crc32c()) {
-      metadata.crc32c_ = Crc32cFromProto(object.checksums().crc32c());
+      metadata.set_crc32c(Crc32cFromProto(object.checksums().crc32c()));
     }
     if (!object.checksums().md5_hash().empty()) {
-      metadata.md5_hash_ = MD5FromProto(object.checksums().md5_hash());
+      metadata.set_md5_hash(MD5FromProto(object.checksums().md5_hash()));
     }
   }
   if (object.has_customer_encryption()) {
-    metadata.customer_encryption_ =
-        FromProto(std::move(*object.mutable_customer_encryption()));
+    metadata.set_customer_encryption(
+        FromProto(std::move(*object.mutable_customer_encryption())));
   }
   if (object.has_event_based_hold()) {
-    metadata.event_based_hold_ = object.event_based_hold();
+    metadata.set_event_based_hold(object.event_based_hold());
   }
-  metadata.kms_key_name_ = std::move(*object.mutable_kms_key());
+  metadata.set_kms_key_name(std::move(*object.mutable_kms_key()));
 
   for (auto const& kv : object.metadata()) {
-    metadata.metadata_[kv.first] = kv.second;
+    metadata.upsert_metadata(kv.first, kv.second);
   }
   if (object.has_retention_expire_time()) {
-    metadata.retention_expiration_time_ =
+    metadata.set_retention_expiration_time(
         google::cloud::internal::ToChronoTimePoint(
-            object.retention_expire_time());
+            object.retention_expire_time()));
   }
-  metadata.size_ = static_cast<std::uint64_t>(object.size());
-  metadata.temporary_hold_ = object.temporary_hold();
+  metadata.set_size(static_cast<std::uint64_t>(object.size()));
+  metadata.set_temporary_hold(object.temporary_hold());
   if (object.has_delete_time()) {
-    metadata.time_deleted_ =
-        google::cloud::internal::ToChronoTimePoint(object.delete_time());
+    metadata.set_time_deleted(
+        google::cloud::internal::ToChronoTimePoint(object.delete_time()));
   }
   if (object.has_update_storage_class_time()) {
-    metadata.time_storage_class_updated_ =
+    metadata.set_time_storage_class_updated(
         google::cloud::internal::ToChronoTimePoint(
-            object.update_storage_class_time());
+            object.update_storage_class_time()));
   }
   if (object.has_custom_time()) {
-    metadata.custom_time_ =
-        google::cloud::internal::ToChronoTimePoint(object.custom_time());
+    metadata.set_custom_time(
+        google::cloud::internal::ToChronoTimePoint(object.custom_time()));
   }
 
   return metadata;

google/cloud/storage/internal/grpc_owner_parser.h

@@ -15,7 +15,7 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_GRPC_OWNER_PARSER_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_GRPC_OWNER_PARSER_H
 
-#include "google/cloud/storage/internal/common_metadata.h"
+#include "google/cloud/storage/owner.h"
 #include "google/cloud/storage/version.h"
 #include <google/storage/v2/storage.pb.h>
 

google/cloud/storage/internal/lifecycle_rule_parser.cc

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/storage/internal/lifecycle_rule_parser.h"
-#include "google/cloud/storage/internal/common_metadata_parser.h"
+#include "google/cloud/storage/internal/metadata_parser.h"
 
 namespace google {
 namespace cloud {

google/cloud/storage/internal/object_access_control_parser.cc

@@ -14,7 +14,7 @@
 
 #include "google/cloud/storage/internal/object_access_control_parser.h"
 #include "google/cloud/storage/internal/access_control_common_parser.h"
-#include "google/cloud/storage/internal/common_metadata_parser.h"
+#include "google/cloud/storage/internal/metadata_parser.h"
 #include <nlohmann/json.hpp>
 
 namespace google {