Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign 7ed5c4864 -> 7c3456020
SENTRY-1557: getRolesForGroups(),getRolesForUsers() does too many trips to the the DB (Vamsee Yarlagadda, reviewed by Alexander Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/7c345602 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/7c345602 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/7c345602 Branch: refs/heads/sentry-ha-redesign Commit: 7c345602033dcad6f6470d2b990aeeaf49fd630e Parents: 7ed5c48 Author: Vamsee Yarlagadda <[email protected]> Authored: Wed Dec 7 23:42:36 2016 -0800 Committer: Vamsee Yarlagadda <[email protected]> Committed: Thu Dec 8 17:49:12 2016 -0800 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 20 ++-- .../db/service/persistent/TestSentryStore.java | 96 ++++++++++++++++++++ 2 files changed, 104 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/7c345602/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 5218715..6ba040d 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -1255,12 +1255,10 @@ public class SentryStore { Set<MSentryRole> result = Sets.newHashSet(); if (groups != null) { Query query = pm.newQuery(MSentryGroup.class); - query.setFilter("this.groupName == t"); - query.declareParameters("java.lang.String t"); - query.setUnique(true); - for (String group : groups) { - MSentryGroup sentryGroup = (MSentryGroup) query.execute(group.trim()); - if (sentryGroup != null) { + query.setFilter(":p1.contains(this.groupName)"); + List<MSentryGroup> sentryGroups = (List) query.execute(groups.toArray()); + if (sentryGroups != null) { + for (MSentryGroup sentryGroup : sentryGroups) { result.addAll(sentryGroup.getRoles()); } } @@ -1272,12 +1270,10 @@ public class SentryStore { Set<MSentryRole> result = Sets.newHashSet(); if (users != null) { Query query = pm.newQuery(MSentryUser.class); - query.setFilter("this.userName == t"); - query.declareParameters("java.lang.String t"); - query.setUnique(true); - for (String user : users) { - MSentryUser sentryUser = (MSentryUser) query.execute(user.trim()); - if (sentryUser != null) { + query.setFilter(":p1.contains(this.userName)"); + List<MSentryUser> sentryUsers = (List) query.execute(users.toArray()); + if (sentryUsers != null) { + for (MSentryUser sentryUser : sentryUsers) { result.addAll(sentryUser.getRoles()); } } http://git-wip-us.apache.org/repos/asf/sentry/blob/7c345602/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index ae9f1ed..14b5e8e 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -381,6 +381,102 @@ public class TestSentryStore extends org.junit.Assert { } @Test + public void testGetTSentryRolesForUsers() throws Exception { + // Test the method getTSentryRolesByUserNames according to the following test data: + // user1->r1 + // user2->r3 + // user3->r2 + // user4->r3, r2 + String roleName1 = "r1"; + String roleName2 = "r2"; + String roleName3 = "r3"; + String user1 = "u1"; + String user2 = "u2"; + String user3 = "u3"; + String user4 = "u4"; + + createRole(roleName1); + createRole(roleName2); + createRole(roleName3); + sentryStore.alterSentryRoleAddUsers(roleName1, Sets.newHashSet(user1)); + sentryStore.alterSentryRoleAddUsers(roleName2, Sets.newHashSet(user3)); + sentryStore.alterSentryRoleAddUsers(roleName2, Sets.newHashSet(user4)); + sentryStore.alterSentryRoleAddUsers(roleName3, Sets.newHashSet(user2, user4)); + + Set<String> userSet1 = Sets.newHashSet(user1, user2, user3); + Set<String> roleSet1 = Sets.newHashSet(roleName1, roleName2, roleName3); + + Set<String> userSet2 = Sets.newHashSet(user4); + Set<String> roleSet2 = Sets.newHashSet(roleName2, roleName3); + + Set<String> userSet3 = Sets.newHashSet("foo"); + Set<String> roleSet3 = Sets.newHashSet(); + + // Query for multiple users + Set<String> roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet1)); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet1).size()); + + // Query for single users + roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet2)); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet2).size()); + + // Query for non-existing user + roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet3)); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet3).size()); + } + + private Set<String> convertToRoleNameSet(Set<TSentryRole> tSentryRoles) { + Set<String> roleNameSet = Sets.newHashSet(); + for (TSentryRole role : tSentryRoles) { + roleNameSet.add(role.getRoleName()); + } + return roleNameSet; + } + + @Test + public void testGetTSentryRolesForGroups() throws Exception { + // Test the method getRoleNamesForGroups according to the following test data: + // group1->r1 + // group2->r2 + // group3->r2 + String grantor = "g1"; + String roleName1 = "r1"; + String roleName2 = "r2"; + String roleName3 = "r3"; + String group1 = "group1"; + String group2 = "group2"; + String group3 = "group3"; + + createRole(roleName1); + createRole(roleName2); + createRole(roleName3); + sentryStore.alterSentryRoleAddGroups(grantor, roleName1, Sets.newHashSet(new TSentryGroup(group1))); + sentryStore.alterSentryRoleAddGroups(grantor, roleName2, Sets.newHashSet(new TSentryGroup(group2), + new TSentryGroup(group3))); + + Set<String> groupSet1 = Sets.newHashSet(group1, group2, group3); + Set<String> roleSet1 = Sets.newHashSet(roleName1, roleName2); + + Set<String> groupSet2 = Sets.newHashSet(group1); + Set<String> roleSet2 = Sets.newHashSet(roleName1); + + Set<String> groupSet3 = Sets.newHashSet("foo"); + Set<String> roleSet3 = Sets.newHashSet(); + + // Query for multiple groups + Set<String> roles = sentryStore.getRoleNamesForGroups(groupSet1); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet1).size()); + + // Query for single group + roles = sentryStore.getRoleNamesForGroups(groupSet2); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet2).size()); + + // Query for non-existing group + roles = sentryStore.getRoleNamesForGroups(groupSet3); + assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles, roleSet3).size()); + } + + @Test public void testGrantRevokePrivilege() throws Exception { String roleName = "test-privilege"; String grantor = "g1";
