kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [kudu] 03/03: thrift: follow-up to 327be47820dcaba358095f118e516a5e92621a59
Date Fri, 14 Jun 2019 17:32:35 GMT
This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch branch-1.10.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 932b5371fa115d1aa87fd3869d38137597133da5
Author: Andrew Wong <awong@apache.org>
AuthorDate: Thu Jun 13 11:06:46 2019 -0700

    thrift: follow-up to 327be47820dcaba358095f118e516a5e92621a59
    
    Minor tweak and added some test coverage for proper ordering.
    
    Change-Id: I857a86c357c332a9cac731fa02aba8241d6cf826
    Reviewed-on: http://gerrit.cloudera.org:8080/13638
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
    (cherry picked from commit a7bdd06373a3862b3572db4690013c7c20a6166e)
    Reviewed-on: http://gerrit.cloudera.org:8080/13651
    Reviewed-by: Andrew Wong <awong@cloudera.com>
---
 src/kudu/sentry/thrift_operators-test.cc | 44 ++++++++++++++++++++++----------
 src/kudu/sentry/thrift_operators.cc      |  8 +++---
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/src/kudu/sentry/thrift_operators-test.cc b/src/kudu/sentry/thrift_operators-test.cc
index aa05b44..b3c2b2c 100644
--- a/src/kudu/sentry/thrift_operators-test.cc
+++ b/src/kudu/sentry/thrift_operators-test.cc
@@ -17,6 +17,7 @@
 
 #include <set>
 #include <string>
+#include <vector>
 
 #include <glog/stl_logging.h>
 #include <gtest/gtest.h>
@@ -26,6 +27,7 @@
 
 using std::set;
 using std::string;
+using std::vector;
 
 namespace sentry {
 
@@ -41,6 +43,16 @@ void AssertCompareRequirements(const T& a, const T& b) {
   }
 }
 
+// Asserts the contains orderings are the same.
+template<typename T>
+void AssertContainersOrdered(const vector<T>& ordered_vec_t, const set<T>&
set_t) {
+  ASSERT_EQ(ordered_vec_t.size(), set_t.size());
+  int i = 0;
+  for (const auto& t : set_t) {
+    ASSERT_EQ(ordered_vec_t[i++], t);
+  }
+}
+
 TEST(ThriftOperatorsTest, TestRoleOperatorLt) {
   // TSentryRole::operator<
   TSentryRole role_a;
@@ -49,13 +61,14 @@ TEST(ThriftOperatorsTest, TestRoleOperatorLt) {
   TSentryRole role_b;
   role_b.__set_roleName("b");
 
-  TSentryRole role_c;
-  role_c.__set_grantorPrincipal("c");
+  TSentryRole role_without_name;
+  role_without_name.__set_grantorPrincipal("grantor");
 
   NO_FATALS(AssertCompareRequirements(role_a, role_b));
-  NO_FATALS(AssertCompareRequirements(role_a, role_c));
-  set<TSentryRole> roles { role_a, role_b, role_c};
-  ASSERT_EQ(3, roles.size()) << roles;
+  NO_FATALS(AssertCompareRequirements(role_a, role_without_name));
+  vector<TSentryRole> ordered_roles { role_without_name, role_a, role_b };
+  set<TSentryRole> roles(ordered_roles.begin(), ordered_roles.end());
+  NO_FATALS(AssertContainersOrdered(ordered_roles, roles));
 }
 
 TEST(ThriftOperatorsTest, TestGroupOperatorLt) {
@@ -67,8 +80,9 @@ TEST(ThriftOperatorsTest, TestGroupOperatorLt) {
   group_b.__set_groupName("b");
 
   NO_FATALS(AssertCompareRequirements(group_a, group_b));
-  set<TSentryGroup> groups { group_a, group_b };
-  ASSERT_EQ(2, groups.size()) << groups;
+  vector<TSentryGroup> ordered_groups { group_a, group_b };
+  set<TSentryGroup> groups(ordered_groups.begin(), ordered_groups.end());
+  NO_FATALS(AssertContainersOrdered(ordered_groups, groups));
 }
 
 TEST(ThriftOperatorsTest, TestPrivilegeOperatorLt) {
@@ -99,8 +113,9 @@ TEST(ThriftOperatorsTest, TestPrivilegeOperatorLt) {
   NO_FATALS(AssertCompareRequirements(db_priv, tbl2_priv));
   NO_FATALS(AssertCompareRequirements(db_priv, tbl1_priv_no_db));
   NO_FATALS(AssertCompareRequirements(tbl1_priv, tbl2_priv));
-  set<TSentryPrivilege> privileges { db_priv, tbl1_priv, tbl2_priv, tbl1_priv_no_db
};
-  ASSERT_EQ(4, privileges.size()) << privileges;
+  vector<TSentryPrivilege> ordered_privileges { tbl1_priv_no_db, db_priv, tbl1_priv,
tbl2_priv };
+  set<TSentryPrivilege> privileges(ordered_privileges.begin(), ordered_privileges.end());
+  NO_FATALS(AssertContainersOrdered(ordered_privileges, privileges));
 }
 
 TEST(ThriftOperatorsTest, TestAuthorizableOperatorLt) {
@@ -133,14 +148,17 @@ TEST(ThriftOperatorsTest, TestAuthorizableOperatorLt) {
   NO_FATALS(AssertCompareRequirements(db_authorizable, tbl1_authorizable));
   NO_FATALS(AssertCompareRequirements(db_authorizable, tbl2_authorizable));
   NO_FATALS(AssertCompareRequirements(tbl1_authorizable, tbl2_authorizable));
-  set<TSentryAuthorizable> authorizables {
-      server_authorizable,
-      uri_authorizable,
+  vector<TSentryAuthorizable> ordered_authorizables {
       db_authorizable,
       tbl1_authorizable,
-      tbl2_authorizable
+      tbl2_authorizable,
+      uri_authorizable,
+      server_authorizable,
   };
+  set<TSentryAuthorizable> authorizables(
+      ordered_authorizables.begin(), ordered_authorizables.end());
   ASSERT_EQ(5, authorizables.size()) << authorizables;
+  NO_FATALS(AssertContainersOrdered(ordered_authorizables, authorizables));
 }
 
 } // namespace sentry
diff --git a/src/kudu/sentry/thrift_operators.cc b/src/kudu/sentry/thrift_operators.cc
index b6738ff..99f6750 100644
--- a/src/kudu/sentry/thrift_operators.cc
+++ b/src/kudu/sentry/thrift_operators.cc
@@ -31,22 +31,22 @@ namespace sentry {
 
 // Returns true if lhs < rhs, false if lhs > rhs, and passes through if the two
 // are equal.
-#define RETURN_IF_DIFFERENT_LT(lhs, rhs) { \
+#define RETURN_IF_DIFFERENT_LT(lhs, rhs) do { \
   if ((lhs) != (rhs)) { \
     return (lhs) < (rhs); \
   } \
-}
+} while (false)
 
 // Returns true if the optional field in 'this' is less than the optional field
 // in 'other', and false if greater than. Passes through if the two are equal.
 // Unset fields compare less than set fields.
-#define OPTIONAL_FIELD_RETURN_IF_DIFFERENT_LT(field) { \
+#define OPTIONAL_FIELD_RETURN_IF_DIFFERENT_LT(field) do { \
   if (this->__isset.field && other.__isset.field) { \
     RETURN_IF_DIFFERENT_LT(this->field, other.field); \
   } else if (this->__isset.field || other.__isset.field) { \
     return other.__isset.field; \
   } \
-}
+} while (false)
 
 bool TSentryRole::operator<(const TSentryRole& other) const {
   RETURN_IF_DIFFERENT_LT(this->roleName, other.roleName);


Mime
View raw message