Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign cc923c8db -> 5d09e7e25
SENTRY-1788: Sentry Store may use JDO object after the associated data is removed in DB (Kalyan Kalvagadda, reviewed by Vamsee Yarlagadda and Alex Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/5d09e7e2 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/5d09e7e2 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/5d09e7e2 Branch: refs/heads/sentry-ha-redesign Commit: 5d09e7e25aa93e5b01f3e610e1cebbc3dc1dc233 Parents: cc923c8 Author: Alexander Kolbasov <[email protected]> Authored: Fri Jun 2 16:31:39 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Fri Jun 2 16:31:39 2017 -0700 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 53 +++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/5d09e7e2/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 cb05a84..97b3636 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 @@ -934,7 +934,7 @@ public class SentryStore { * The responsibility to commit/rollback the transaction should be handled by the caller. */ private void populateChildren(PersistenceManager pm, Set<String> roleNames, MSentryPrivilege priv, - Set<MSentryPrivilege> children) throws SentryInvalidInputException { + Collection<MSentryPrivilege> children) throws SentryInvalidInputException { Preconditions.checkNotNull(pm); if (!isNULL(priv.getServerName()) || !isNULL(priv.getDbName()) || !isNULL(priv.getTableName())) { @@ -2199,44 +2199,49 @@ public class SentryStore { TSentryPrivilege tPrivilege, TSentryPrivilege newTPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { - HashSet<MSentryRole> roleSet = Sets.newHashSet(); - + Collection<MSentryRole> roleSet = new HashSet<>(); List<MSentryPrivilege> mPrivileges = getMSentryPrivileges(tPrivilege, pm); - if (mPrivileges != null && !mPrivileges.isEmpty()) { - for (MSentryPrivilege mPrivilege : mPrivileges) { - roleSet.addAll(ImmutableSet.copyOf(mPrivilege.getRoles())); + for (MSentryPrivilege mPrivilege : mPrivileges) { + roleSet.addAll(ImmutableSet.copyOf(mPrivilege.getRoles())); + } + // Dropping the privilege + if (newTPrivilege == null) { + for (MSentryRole role : roleSet) { + alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege); } + return; } - + // Renaming privilege MSentryPrivilege parent = getMSentryPrivilege(tPrivilege, pm); + if (parent != null) { + // When all the roles associated with that privilege are revoked, privilege + // will be removed from the database. + // parent is an JDO object which is associated with privilege data in the database. + // When the associated row is deleted in database, JDO should be not be + // dereferenced. If object has to be used even after that it should have been detached. + parent = pm.detachCopy(parent); + } for (MSentryRole role : roleSet) { // 1. get privilege and child privileges - Set<MSentryPrivilege> privilegeGraph = Sets.newHashSet(); + Collection<MSentryPrivilege> privilegeGraph = new HashSet<>(); if (parent != null) { - - // Parent privilege object is used after revoking. - // If the privilege was associated to only role which is revoked, - // privilege object should not be used. It is safe to insert - // a copy of the parent object in the privilegeGraph - privilegeGraph.add(convertToMSentryPrivilege(convertToTSentryPrivilege(parent))); + privilegeGraph.add(parent); populateChildren(pm, Sets.newHashSet(role.getRoleName()), parent, privilegeGraph); } else { populateChildren(pm, Sets.newHashSet(role.getRoleName()), convertToMSentryPrivilege(tPrivilege), - privilegeGraph); + privilegeGraph); } // 2. revoke privilege and child privileges alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege); // 3. add new privilege and child privileges with new tableName - if (newTPrivilege != null) { - for (MSentryPrivilege m : privilegeGraph) { - TSentryPrivilege t = convertToTSentryPrivilege(m); - if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.DATABASE.name())) { - t.setDbName(newTPrivilege.getDbName()); - } else if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.TABLE.name())) { - t.setTableName(newTPrivilege.getTableName()); - } - alterSentryRoleGrantPrivilegeCore(pm, role.getRoleName(), t); + for (MSentryPrivilege mPriv : privilegeGraph) { + TSentryPrivilege tPriv = convertToTSentryPrivilege(mPriv); + if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.DATABASE.name())) { + tPriv.setDbName(newTPrivilege.getDbName()); + } else if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.TABLE.name())) { + tPriv.setTableName(newTPrivilege.getTableName()); } + alterSentryRoleGrantPrivilegeCore(pm, role.getRoleName(), tPriv); } } }
