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 <[email protected]> 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 <[email protected]> (cherry picked from commit a7bdd06373a3862b3572db4690013c7c20a6166e) Reviewed-on: http://gerrit.cloudera.org:8080/13651 Reviewed-by: Andrew Wong <[email protected]> --- 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);
