sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vam...@apache.org
Subject [39/52] [abbrv] sentry git commit: 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) CDH-54250
Date Wed, 14 Jun 2017 00:57:17 GMT
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)
CDH-54250

Change-Id: I6220fb4b918adf4a3d13091abbb44cbca0b81573
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23108
Tested-by: Jenkins User
Reviewed-by: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
(cherry picked from commit 7c789e05ffeb71769a8da7950720419f5290ac75)
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23321


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/acc13302
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/acc13302
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/acc13302

Branch: refs/for/cdh5-1.5.1_ha
Commit: acc13302c0bcb98417eb8ff8520ebd41fbb377b5
Parents: 926c6d0
Author: Alexander Kolbasov <akolb@cloudera.com>
Authored: Wed May 31 16:19:52 2017 -0700
Committer: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
Committed: Thu Jun 1 10:34:06 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/acc13302/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 2a96811..891c1b1 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
@@ -910,7 +910,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()))) {
@@ -2050,44 +2050,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);
       }
     }
   }


Mime
View raw message