rgw: reorder rgw_user members for default operator<=>

the default operator<=> does a memberwise comparison the same that
rgw_user::compare() did, except that it compared `ns` before `id`

reorder the rgw_user members so that the default operator<=> can
replace compare() and the related comparison operators

replaces uses of rgw_user::compare() with operator== and !=

Signed-off-by: Casey Bodley <cbodley@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc
index 506ac0acf112206e599499411069ab9e832c05e5..b5569e481c53e04cd584e5ffae4e56550e6b40af 100644 (file)
--- a/src/rgw/driver/rados/rgw_user.cc
+++ b/src/rgw/driver/rados/rgw_user.cc
@@ -237,7 +237,7 @@ int RGWAccessKeyPool::init(RGWUserAdminOpState& op_state)
   }
 
   const rgw_user& uid = op_state.get_user_id();
-  if (uid.compare(RGW_USER_ANON_ID) == 0) {
+  if (uid == rgw_user(RGW_USER_ANON_ID)) {
     keys_allowed = false;
     return -EINVAL;
   }
@@ -890,7 +890,7 @@ int RGWSubUserPool::init(RGWUserAdminOpState& op_state)
   }
 
   const rgw_user& uid = op_state.get_user_id();
-  if (uid.compare(RGW_USER_ANON_ID) == 0) {
+  if (uid == rgw_user(RGW_USER_ANON_ID)) {
     subusers_allowed = false;
     return -EACCES;
   }
@@ -1198,7 +1198,7 @@ int RGWUserCapPool::init(RGWUserAdminOpState& op_state)
   }
 
   const rgw_user& uid = op_state.get_user_id();
-  if (uid.compare(RGW_USER_ANON_ID) == 0) {
+  if (uid == rgw_user(RGW_USER_ANON_ID)) {
     caps_allowed = false;
     return -EACCES;
   }
@@ -1373,7 +1373,7 @@ int RGWUser::init(const DoutPrefixProvider *dpp, RGWUserAdminOpState& op_state,
     }
   }
 
-  if (!user_id.empty() && (user_id.compare(RGW_USER_ANON_ID) != 0)) {
+  if (!user_id.empty() && user_id != rgw_user(RGW_USER_ANON_ID)) {
     user = driver->get_user(user_id);
     found = (user->load_user(dpp, y) >= 0);
     op_state.found_by_uid = found;
@@ -1477,12 +1477,12 @@ int RGWUser::check_op(RGWUserAdminOpState& op_state, std::string *err_msg)
   int ret = 0;
   const rgw_user& uid = op_state.get_user_id();
 
-  if (uid.compare(RGW_USER_ANON_ID) == 0) {
+  if (uid == rgw_user(RGW_USER_ANON_ID)) {
     set_err_msg(err_msg, "unable to perform operations on the anonymous user");
     return -EINVAL;
   }
 
-  if (is_populated() && user_id.compare(uid) != 0) {
+  if (is_populated() && user_id != uid) {
     set_err_msg(err_msg, "user id mismatch, operation id: " + uid.to_str()
             + " does not match: " + user_id.to_str());
 
@@ -1858,7 +1858,7 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState&
   }
 
   // ensure that we can modify the user's attributes
-  if (user_id.compare(RGW_USER_ANON_ID) == 0) {
+  if (user_id == rgw_user(RGW_USER_ANON_ID)) {
     set_err_msg(err_msg, "unable to modify anonymous user's info");
     return -EACCES;
   }
@@ -1870,7 +1870,7 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState&
     // make sure we are not adding a duplicate email
     if (old_email != op_email) {
       ret = driver->get_user_by_email(dpp, op_email, y, &duplicate_check);
-      if (ret >= 0 && duplicate_check->get_id().compare(user_id) != 0) {
+      if (ret >= 0 && duplicate_check->get_id() != user_id) {
         set_err_msg(err_msg, "cannot add duplicate email");
         return -ERR_EMAIL_EXIST;
       }

diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc
index 9f71e3281500651e3dede92e6fcf89e732adc31c..3fc072a33ca6be801c7cbd780bb52312c7bae445 100644 (file)
--- a/src/rgw/rgw_acl_s3.cc
+++ b/src/rgw/rgw_acl_s3.cc
@@ -392,11 +392,11 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own
     add_grant(&group_grant);
   } else if (canned_acl.compare("bucket-owner-read") == 0) {
     bucket_owner_grant.set_canon(bid, bname, RGW_PERM_READ);
-    if (bid.compare(owner.get_id()) != 0)
+    if (bid != owner.get_id())
       add_grant(&bucket_owner_grant);
   } else if (canned_acl.compare("bucket-owner-full-control") == 0) {
     bucket_owner_grant.set_canon(bid, bname, RGW_PERM_FULL_CONTROL);
-    if (bid.compare(owner.get_id()) != 0)
+    if (bid != owner.get_id())
       add_grant(&bucket_owner_grant);
   } else {
     return -EINVAL;
@@ -489,7 +489,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp,
   ACLOwner *requested_owner = static_cast<ACLOwner_S3 *>(find_first("Owner"));
   if (requested_owner) {
     rgw_user& requested_id = requested_owner->get_id();
-    if (!requested_id.empty() && requested_id.compare(owner->get_id()) != 0)
+    if (!requested_id.empty() && requested_id != owner->get_id())
       return -EPERM;
   }
 

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index a290388efe0c74a43ffdfd7993d0804d075ae364..2193ff104c966e374b89a895f2c1966b514156e6 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -457,7 +457,7 @@ static int read_obj_policy(const DoutPrefixProvider *dpp,
       return ret;
     }
     const rgw_user& bucket_owner = bucket_policy.get_owner().get_id();
-    if (bucket_owner.compare(s->user->get_id()) != 0 &&
+    if (bucket_owner != s->user->get_id() &&
         ! s->auth.identity->is_admin_of(bucket_owner)) {
       auto r = eval_identity_or_session_policies(dpp, s->iam_user_policies, s->env,
                                   rgw::IAM::s3ListBucket, ARN(bucket->get_key()));

diff --git a/src/rgw/rgw_user_types.h b/src/rgw/rgw_user_types.h
index c9a1a46ade1e9b0317667aa1d9c32cdeb6c72285..1aaf4cfa5d34826645e29cd452960078b9d168f5 100644 (file)
--- a/src/rgw/rgw_user_types.h
+++ b/src/rgw/rgw_user_types.h
@@ -26,9 +26,10 @@
 #include "common/Formatter.h"
 
 struct rgw_user {
+  // note: order of member variables matches the sort order of operator<=>
   std::string tenant;
-  std::string id;
   std::string ns;
+  std::string id;
 
   rgw_user() {}
   explicit rgw_user(const std::string& s) {
@@ -36,13 +37,13 @@ struct rgw_user {
   }
   rgw_user(const std::string& tenant, const std::string& id, const std::string& ns="")
     : tenant(tenant),
-      id(id),
-      ns(ns) {
+      ns(ns),
+      id(id) {
   }
   rgw_user(std::string&& tenant, std::string&& id, std::string&& ns="")
     : tenant(std::move(tenant)),
-      id(std::move(id)),
-      ns(std::move(ns)) {
+      ns(std::move(ns)),
+      id(std::move(id)) {
   }
 
   void encode(ceph::buffer::list& bl) const {
@@ -118,40 +119,8 @@ struct rgw_user {
     return *this;
   }
 
-  int compare(const rgw_user& u) const {
-    int r = tenant.compare(u.tenant);
-    if (r != 0)
-      return r;
-    r = ns.compare(u.ns);
-    if (r != 0) {
-      return r;
-    }
-    return id.compare(u.id);
-  }
-  int compare(const std::string& str) const {
-    rgw_user u(str);
-    return compare(u);
-  }
+  friend auto operator<=>(const rgw_user&, const rgw_user&) = default;
 
-  bool operator!=(const rgw_user& rhs) const {
-    return (compare(rhs) != 0);
-  }
-  bool operator==(const rgw_user& rhs) const {
-    return (compare(rhs) == 0);
-  }
-  bool operator<(const rgw_user& rhs) const {
-    if (tenant < rhs.tenant) {
-      return true;
-    } else if (tenant > rhs.tenant) {
-      return false;
-    }
-    if (ns < rhs.ns) {
-      return true;
-    } else if (ns > rhs.ns) {
-      return false;
-    }
-    return (id < rhs.id);
-  }
   void dump(ceph::Formatter *f) const;
   static void generate_test_instances(std::list<rgw_user*>& o);
 };

diff --git a/src/test/test_rgw_admin_meta.cc b/src/test/test_rgw_admin_meta.cc
index b1d5fad06009d6b0a59b71c4bd847157bf2f3b3f..00c43d10b5495717948830b13d3041d483418b53 100644 (file)
--- a/src/test/test_rgw_admin_meta.cc
+++ b/src/test/test_rgw_admin_meta.cc
@@ -460,7 +460,7 @@ int compare_access_keys(RGWAccessKey& k1, RGWAccessKey& k2) {
 int compare_user_info(RGWUserInfo& i1, RGWUserInfo& i2) {
   int rv;
 
-  if ((rv = i1.user_id.compare(i2.user_id)) != 0)
+  if ((rv = i1.user_id.id.compare(i2.user_id.id)) != 0)
     return rv;
   if ((rv = i1.display_name.compare(i2.display_name)) != 0)
     return rv;