feat(storage): easier mocks for *AccessControl (#9910)

Add modifiers for all the attributes in `ObjectAccessControl` and
`BucketAccessControl`. That makes it easier to mock the class.

I deprecated `internal::AccessControlCommon` as this class is no longer
used.

google/cloud/storage/bucket_access_control.cc

@@ -21,7 +21,17 @@ namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 bool operator==(BucketAccessControl const& lhs,
                 BucketAccessControl const& rhs) {
-  return *static_cast<internal::AccessControlCommon const*>(&lhs) == rhs;
+  return lhs.bucket_ == rhs.bucket_                 //
+         && lhs.domain_ == rhs.domain_              //
+         && lhs.email_ == rhs.email_                //
+         && lhs.entity_ == rhs.entity_              //
+         && lhs.entity_id_ == rhs.entity_id_        //
+         && lhs.etag_ == rhs.etag_                  //
+         && lhs.id_ == rhs.id_                      //
+         && lhs.kind_ == rhs.kind_                  //
+         && lhs.project_team_ == rhs.project_team_  //
+         && lhs.role_ == rhs.role_                  //
+         && lhs.self_link_ == rhs.self_link_;
 }
 
 std::ostream& operator<<(std::ostream& os, BucketAccessControl const& rhs) {

google/cloud/storage/bucket_access_control.h

@@ -15,8 +15,8 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_BUCKET_ACCESS_CONTROL_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_BUCKET_ACCESS_CONTROL_H
 
-#include "google/cloud/storage/internal/access_control_common.h"
 #include "google/cloud/storage/internal/patch_builder.h"
+#include "google/cloud/storage/project_team.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/status_or.h"
 #include <string>
@@ -40,40 +40,117 @@ struct GrpcBucketAccessControlParser;
  * https://cloud.google.com/storage/docs/json_api/v1/bucketAccessControls for
  *     an authoritative source of field definitions.
  */
-class BucketAccessControl : private internal::AccessControlCommon {
+class BucketAccessControl {
  public:
   BucketAccessControl() = default;
 
-  using AccessControlCommon::ROLE_OWNER;
-  using AccessControlCommon::ROLE_READER;
-  using AccessControlCommon::TEAM_EDITORS;
-  using AccessControlCommon::TEAM_OWNERS;
-  using AccessControlCommon::TEAM_VIEWERS;
-
-  using AccessControlCommon::bucket;
-  using AccessControlCommon::domain;
-  using AccessControlCommon::email;
-
-  using AccessControlCommon::entity;
+  ///@{
+  /**
+   * @name Well-known values for the role() field..
+   *
+   * The following functions are handy to avoid common typos in the role names.
+   * We use functions instead of enums because enums are not backwards
+   * compatible and are brittle to changes in the server-side.
+   */
+  static std::string ROLE_OWNER() { return "OWNER"; }
+  static std::string ROLE_READER() { return "READER"; }
+  ///@}
+
+  ///@{
+  /**
+   * @name Well-known values for the project_team().team field..
+   *
+   * The following functions are handy to avoid common typos in the team names.
+   * We use functions instead of enums because enums are not backwards
+   * compatible and are brittle to changes in the server-side.
+   */
+  static std::string TEAM_EDITORS() { return storage::TEAM_EDITORS(); }
+  static std::string TEAM_OWNERS() { return storage::TEAM_OWNERS(); }
+  static std::string TEAM_VIEWERS() { return storage::TEAM_VIEWERS(); }
+  ///@}
+
+  ///@{
+  /**
+   * @name Accessors.
+   */
+  std::string const& bucket() const { return bucket_; }
+  std::string const& domain() const { return domain_; }
+  std::string const& email() const { return email_; }
+  std::string const& entity() const { return entity_; }
+  std::string const& entity_id() const { return entity_id_; }
+  std::string const& etag() const { return etag_; }
+  std::string const& id() const { return id_; }
+  std::string const& kind() const { return kind_; }
+  bool has_project_team() const { return project_team_.has_value(); }
+  ProjectTeam const& project_team() const { return *project_team_; }
+  absl::optional<ProjectTeam> const& project_team_as_optional() const {
+    return project_team_;
+  }
+  std::string const& role() const { return role_; }
+  std::string const& self_link() const { return self_link_; }
+  ///@}
+
+  ///@{
+  /**
+   * @name Modifiers for mutable attributes.
+   *
+   * The follow attributes can be changed in update and patch operations.
+   */
   BucketAccessControl& set_entity(std::string v) {
-    AccessControlCommon::set_entity(std::move(v));
+    entity_ = std::move(v);
     return *this;
   }
-
-  using AccessControlCommon::entity_id;
-  using AccessControlCommon::etag;
-  using AccessControlCommon::has_project_team;
-  using AccessControlCommon::id;
-  using AccessControlCommon::kind;
-  using AccessControlCommon::project_team;
-
-  using AccessControlCommon::role;
   BucketAccessControl& set_role(std::string v) {
-    AccessControlCommon::set_role(std::move(v));
+    role_ = std::move(v);
     return *this;
   }
-
-  using AccessControlCommon::self_link;
+  ///@}
+
+  ///@{
+  /**
+   * @name Testing modifiers.
+   *
+   * The following attributes cannot be changed when updating, creating, or
+   * patching an BucketAccessControl resource. However, it is useful to change
+   * them in tests, e.g., when mocking the results from the C++ client library.
+   */
+  BucketAccessControl& set_bucket(std::string v) {
+    bucket_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_domain(std::string v) {
+    domain_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_email(std::string v) {
+    email_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_entity_id(std::string v) {
+    entity_id_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_etag(std::string v) {
+    etag_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_id(std::string v) {
+    id_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_kind(std::string v) {
+    kind_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_project_team(ProjectTeam v) {
+    project_team_ = std::move(v);
+    return *this;
+  }
+  BucketAccessControl& set_self_link(std::string v) {
+    self_link_ = std::move(v);
+    return *this;
+  }
+  ///@}
 
   friend bool operator==(BucketAccessControl const& lhs,
                          BucketAccessControl const& rhs);
@@ -82,8 +159,18 @@ class BucketAccessControl : private internal::AccessControlCommon {
     return !(lhs == rhs);
   }
 
-  friend struct internal::BucketAccessControlParser;
-  friend struct internal::GrpcBucketAccessControlParser;
+ private:
+  std::string bucket_;
+  std::string domain_;
+  std::string email_;
+  std::string entity_;
+  std::string entity_id_;
+  std::string etag_;
+  std::string id_;
+  std::string kind_;
+  absl::optional<ProjectTeam> project_team_;
+  std::string role_;
+  std::string self_link_;
 };
 
 std::ostream& operator<<(std::ostream& os, BucketAccessControl const& rhs);

google/cloud/storage/internal/access_control_common.h

@@ -38,7 +38,9 @@ struct GrpcObjectAccessControlParser;
  * https://cloud.google.com/storage/docs/json_api/v1/bucketAccessControls
  * https://cloud.google.com/storage/docs/json_api/v1/objectAccessControls
  */
-class AccessControlCommon {
+// TODO(#9897) - remove this class and any references to it
+class GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01") AccessControlCommon {
  public:
   AccessControlCommon() = default;
 
@@ -62,9 +64,9 @@ class AccessControlCommon {
    * We use functions instead of enums because enums are not backwards
    * compatible and are brittle to changes in the server-side.
    */
-  static std::string TEAM_EDITORS() { return "editors"; }
-  static std::string TEAM_OWNERS() { return "owners"; }
-  static std::string TEAM_VIEWERS() { return "viewers"; }
+  static std::string TEAM_EDITORS() { return storage::TEAM_EDITORS(); }
+  static std::string TEAM_OWNERS() { return storage::TEAM_OWNERS(); }
+  static std::string TEAM_VIEWERS() { return storage::TEAM_VIEWERS(); }
   //@}
 
   std::string const& bucket() const { return bucket_; }
@@ -108,6 +110,8 @@ class AccessControlCommon {
   std::string self_link_;
 };
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
+
 inline bool operator==(AccessControlCommon const& lhs,
                        AccessControlCommon const& rhs) {
   // Start with id, bucket, etag because they should fail early, then
@@ -127,6 +131,8 @@ inline bool operator!=(AccessControlCommon const& lhs,
   return std::rel_ops::operator!=(lhs, rhs);
 }
 
+#include "google/cloud/internal/diagnostics_pop.inc"
+
 }  // namespace internal
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage

google/cloud/storage/internal/access_control_common_parser.cc

@@ -13,6 +13,9 @@
 // limitations under the License.
 
 #include "google/cloud/storage/internal/access_control_common_parser.h"
+// This file contains the implementation for deprecated functions, we need to
+// disable the warnings.
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 
 namespace google {
 namespace cloud {

google/cloud/storage/internal/access_control_common_parser.h

@@ -24,9 +24,17 @@ namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
-struct AccessControlCommonParser {
+
+// TODO(#9897) - remove this class and any references to it
+struct GOOGLE_CLOUD_CPP_DEPRECATED(
+    "This class will be removed shortly after 2023-06-01")
+    AccessControlCommonParser {
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
+
   static Status FromJson(AccessControlCommon& result,
                          nlohmann::json const& json);
+
+#include "google/cloud/internal/diagnostics_pop.inc"
 };
 
 }  // namespace internal

google/cloud/storage/internal/access_control_common_parser_test.cc

@@ -16,6 +16,9 @@
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <nlohmann/json.hpp>
+// This file contains tests for deprecated functions, we need to disable the
+// warnings
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 
 namespace google {
 namespace cloud {

google/cloud/storage/internal/access_control_common_test.cc

@@ -14,6 +14,9 @@
 
 #include "google/cloud/storage/internal/access_control_common.h"
 #include <gmock/gmock.h>
+// This file contains tests for deprecated functions, we need to disable the
+// warnings.
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 
 namespace google {
 namespace cloud {
@@ -22,7 +25,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
 namespace {
 /// @test Verify the well-known values defined in AccessControlCommon.
-TEST(ccessControlCommonTest, WellKnownValues) {
+TEST(AccessControlCommonTest, WellKnownValues) {
   EXPECT_EQ("OWNER", AccessControlCommon::ROLE_OWNER());
   EXPECT_EQ("READER", AccessControlCommon::ROLE_READER());
 

google/cloud/storage/internal/bucket_access_control_parser.cc

@@ -23,13 +23,27 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
 StatusOr<BucketAccessControl> BucketAccessControlParser::FromJson(
     nlohmann::json const& json) {
-  if (!json.is_object()) {
-    return Status(StatusCode::kInvalidArgument, __func__);
-  }
-  BucketAccessControl result{};
-  auto status = internal::AccessControlCommonParser::FromJson(result, json);
-  if (!status.ok()) {
-    return status;
+  if (!json.is_object()) return Status(StatusCode::kInvalidArgument, __func__);
+
+  BucketAccessControl result;
+  result.set_bucket(json.value("bucket", ""));
+  result.set_domain(json.value("domain", ""));
+  result.set_email(json.value("email", ""));
+  result.set_entity(json.value("entity", ""));
+  result.set_entity_id(json.value("entityId", ""));
+  result.set_etag(json.value("etag", ""));
+  result.set_id(json.value("id", ""));
+  result.set_kind(json.value("kind", ""));
+  result.set_role(json.value("role", ""));
+  result.set_self_link(json.value("selfLink", ""));
+  auto team = json.find("projectTeam");
+  if (team != json.end()) {
+    auto const& tmp = *team;
+    if (tmp.is_null()) return result;
+    ProjectTeam p;
+    p.project_number = tmp.value("projectNumber", "");
+    p.team = tmp.value("team", "");
+    result.set_project_team(std::move(p));
   }
   return result;
 }

google/cloud/storage/internal/common_metadata.h

@@ -29,10 +29,6 @@ namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
-struct GrpcBucketMetadataParser;
-struct GrpcObjectMetadataParser;
-template <typename Derived>
-struct CommonMetadataParser;
 
 /**
  * Defines common attributes to both `BucketMetadata` and `ObjectMetadata`.
@@ -93,7 +89,7 @@ class GOOGLE_CLOUD_CPP_DEPRECATED(
 
 template <typename T>
 GOOGLE_CLOUD_CPP_DEPRECATED(
-    "This class will be removed shortly after 2023-06-01")
+    "This function 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
@@ -112,7 +108,7 @@ operator==(CommonMetadata<T> const& lhs, CommonMetadata<T> const& rhs) {
 
 template <typename T>
 GOOGLE_CLOUD_CPP_DEPRECATED(
-    "This class will be removed shortly after 2023-06-01")
+    "This function 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/grpc_bucket_access_control_parser.cc

@@ -45,22 +45,21 @@ BucketAccessControl GrpcBucketAccessControlParser::FromProto(
     google::storage::v2::BucketAccessControl acl,
     std::string const& bucket_name) {
   BucketAccessControl result;
-  result.kind_ = "storage#bucketAccessControl";
-  result.bucket_ = bucket_name;
-  result.domain_ = std::move(*acl.mutable_domain());
-  result.email_ = std::move(*acl.mutable_email());
-  result.entity_ = std::move(*acl.mutable_entity());
-  result.entity_id_ = std::move(*acl.mutable_entity_id());
-  result.id_ = std::move(*acl.mutable_id());
+  result.set_kind("storage#bucketAccessControl");
+  result.set_bucket(bucket_name);
+  result.set_domain(std::move(*acl.mutable_domain()));
+  result.set_email(std::move(*acl.mutable_email()));
+  result.set_entity(std::move(*acl.mutable_entity()));
+  result.set_entity_id(std::move(*acl.mutable_entity_id()));
+  result.set_id(std::move(*acl.mutable_id()));
   if (acl.has_project_team()) {
-    result.project_team_ = ProjectTeam{
+    result.set_project_team(ProjectTeam{
         std::move(*acl.mutable_project_team()->mutable_project_number()),
         std::move(*acl.mutable_project_team()->mutable_team()),
-    };
+    });
   }
-  result.role_ = std::move(*acl.mutable_role());
-  result.self_link_.clear();
-  result.etag_ = std::move(*acl.mutable_etag());
+  result.set_role(std::move(*acl.mutable_role()));
+  result.set_etag(std::move(*acl.mutable_etag()));
 
   return result;
 }