sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kal...@apache.org
Subject [2/2] sentry git commit: SENTRY-2252: Normalize the Sentry store API's to handle both user/role privileges (Kalyan Kumar kalvagadda, reviewed-by Na Li)
Date Fri, 08 Jun 2018 20:20:17 GMT
SENTRY-2252: Normalize the Sentry store API's to handle both user/role privileges (Kalyan Kumar kalvagadda, reviewed-by Na Li)


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

Branch: refs/heads/master
Commit: 112cdcd4f671bd9554f68752ecafdb943cd1e163
Parents: 097948e
Author: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
Authored: Fri Jun 8 15:20:02 2018 -0500
Committer: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
Committed: Fri Jun 8 15:20:02 2018 -0500

----------------------------------------------------------------------
 .../db/service/model/MSentryPrivilege.java      |  32 +-
 .../provider/db/service/model/MSentryRole.java  |  23 +-
 .../provider/db/service/model/MSentryUser.java  |  19 +-
 .../db/service/persistent/PrivilegeEntity.java  |  48 ++
 .../db/service/persistent/SentryStore.java      | 579 +++++--------------
 .../service/persistent/TestSentryRole.java      |  16 +-
 .../TestHMSFollowerSentryStoreIntegration.java  |  17 +-
 .../db/service/persistent/TestSentryStore.java  | 294 +++++-----
 8 files changed, 404 insertions(+), 624 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
index 85477b6..e5eb4c4 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
@@ -26,6 +26,8 @@ import javax.jdo.annotations.PersistenceCapable;
 import org.apache.sentry.core.common.utils.PathUtils;
 import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
+import org.apache.sentry.provider.db.service.persistent.PrivilegeEntity;
+import org.apache.sentry.service.common.ServiceConstants.SentryEntityType;
 
 /**
  * Database backed Sentry Privilege. Any changes to this object
@@ -165,12 +167,16 @@ public class MSentryPrivilege {
      this.grantOption = grantOption;
    }
 
-  public void appendRole(MSentryRole role) {
-    roles.add(role);
-  }
-
-  public void appendUser(MSentryUser user) {
-    users.add(user);
+  /**
+   * Appends Role/User in the privilege.
+   * @param entity Role/User to be appended.
+   */
+  public void appendEntity(PrivilegeEntity entity) {
+    if(entity.getType() == SentryEntityType.ROLE) {
+      roles.add((MSentryRole)entity);
+    } else  if(entity.getType() == SentryEntityType.USER) {
+      users.add((MSentryUser)entity);
+    }
   }
 
   public Set<MSentryRole> getRoles() {
@@ -179,9 +185,17 @@ public class MSentryPrivilege {
 
   public Set<MSentryUser> getUsers() { return users; }
 
-  public void removeRole(MSentryRole role) {
-    roles.remove(role);
-    role.removePrivilege(this);
+  /**
+   * Removes Role/User in the privilege.
+   * @param entity Role/User to be removed.
+   */
+  public void removeEntity(PrivilegeEntity entity) {
+    if(entity.getType() == SentryEntityType.ROLE && (roles != null) && (roles.size() > 0)) {
+      roles.remove((MSentryRole)entity);
+    } else  if(entity.getType() == SentryEntityType.USER && (users != null) && (users.size() > 0)) {
+      users.remove((MSentryUser)entity);
+    }
+    entity.removePrivilege(this);
   }
 
   public void removeUser(MSentryUser user) {

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
index fb8f5d2..74213af 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
@@ -26,12 +26,15 @@ import javax.jdo.annotations.PersistenceCapable;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 
+import org.apache.sentry.service.common.ServiceConstants;
+import org.apache.sentry.provider.db.service.persistent.PrivilegeEntity;
+
 /**
  * Database backed Sentry Role. Any changes to this object
  * require re-running the maven build so DN an re-enhance.
  */
 @PersistenceCapable
-public class MSentryRole {
+public class MSentryRole implements PrivilegeEntity {
 
   private String roleName;
   // set of privileges granted to this role
@@ -66,6 +69,18 @@ public class MSentryRole {
     this.createTime = createTime;
   }
 
+  /**
+   * Get the Name of the Role.
+   * @return roleName
+   */
+  public String getEntityName() {
+    return roleName;
+  }
+
+  public ServiceConstants.SentryEntityType getType() {
+    return ServiceConstants.SentryEntityType.ROLE;
+  }
+
   public String getRoleName() {
     return roleName;
   }
@@ -108,7 +123,7 @@ public class MSentryRole {
 
   public void removePrivilege(MSentryPrivilege privilege) {
     if (privileges.remove(privilege)) {
-      privilege.removeRole(this);
+      privilege.removeEntity(this);
     }
   }
 
@@ -118,7 +133,7 @@ public class MSentryRole {
 
   public void appendPrivilege(MSentryPrivilege privilege) {
     if (privileges.add(privilege)) {
-      privilege.appendRole(this);
+      privilege.appendEntity(this);
     }
   }
 
@@ -180,7 +195,7 @@ public class MSentryRole {
     // the actual privilege set in MSentryRole instance.
 
     for (MSentryPrivilege privilege : ImmutableSet.copyOf(privileges)) {
-      privilege.removeRole(this);
+      privilege.removeEntity(this);
     }
     Preconditions.checkState(privileges.isEmpty(), "Privileges should be empty: " + privileges);
   }

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java
index 9188738..6e44c79 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java
@@ -25,12 +25,15 @@ import java.util.Set;
 
 import javax.jdo.annotations.PersistenceCapable;
 
+import org.apache.sentry.service.common.ServiceConstants;
+import org.apache.sentry.provider.db.service.persistent.PrivilegeEntity;
+
 /**
  * Database backed Sentry User. Any changes to this object
  * require re-running the maven build so DN an re-enhance.
  */
 @PersistenceCapable
-public class MSentryUser {
+public class MSentryUser implements PrivilegeEntity {
 
   /**
    * User name is unique
@@ -57,6 +60,18 @@ public class MSentryUser {
     this.createTime = createTime;
   }
 
+  /**
+   * Gets the User name
+   * @return username
+   */
+  public String getEntityName() {
+    return userName;
+  }
+
+  public ServiceConstants.SentryEntityType getType() {
+    return ServiceConstants.SentryEntityType.USER;
+  }
+
   public Set<MSentryRole> getRoles() {
     return roles;
   }
@@ -97,7 +112,7 @@ public class MSentryUser {
 
   public void appendPrivilege(MSentryPrivilege privilege) {
     if (privileges.add(privilege)) {
-      privilege.appendUser(this);
+      privilege.appendEntity(this);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PrivilegeEntity.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PrivilegeEntity.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PrivilegeEntity.java
new file mode 100644
index 0000000..3f7ba97
--- /dev/null
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PrivilegeEntity.java
@@ -0,0 +1,48 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.persistent;
+
+import org.apache.sentry.service.common.ServiceConstants.SentryEntityType;
+import org.apache.sentry.provider.db.service.model.MSentryPrivilege;
+
+
+import java.util.Set;
+
+/**
+ * All the entities to which privileges are granted should implement this interface.
+ */
+public interface PrivilegeEntity {
+
+  String getEntityName();
+
+  SentryEntityType getType();
+
+  void setPrivileges(Set<MSentryPrivilege> privileges);
+
+  Set<MSentryPrivilege> getPrivileges();
+
+  void removePrivilege(MSentryPrivilege privilege);
+
+  void appendPrivileges(Set<MSentryPrivilege> privileges);
+
+  void appendPrivilege(MSentryPrivilege privilege);
+
+  void removePrivileges();
+
+};
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index e6b71b5..f0e373a 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -683,38 +683,10 @@ public class SentryStore {
       LOGGER.error("MSentryHmsNotification cleaning process encountered an error", e);
     }
   }
-  /**
-   * Alter a given sentry role to grant a privilege.
-   *
-   * @param grantorPrincipal User name
-   * @param roleName the given role name
-   * @param privilege the given privilege
-   * @throws Exception
-   */
-  void alterSentryRoleGrantPrivilege(final String grantorPrincipal,
-      final String roleName, final TSentryPrivilege privilege) throws Exception {
-    tm.executeTransactionWithRetry(
-            pm -> {
-              pm.setDetachAllOnCommit(false); // No need to detach objects
-              String trimmedRoleName = trimAndLower(roleName);
-              // first do grant check
-              grantOptionCheck(pm, grantorPrincipal, privilege);
-
-              // Alter sentry Role and grant Privilege.
-              MSentryPrivilege mPrivilege = alterSentryRoleGrantPrivilegeCore(
-                pm, trimmedRoleName, privilege);
-
-              if (mPrivilege != null) {
-                // update the privilege to be the one actually updated.
-                convertToTSentryPrivilege(mPrivilege, privilege);
-              }
-              return null;
-            });
-  }
 
   /**
    * Alter a given sentry role to grant a set of privileges.
-   * Internally calls alterSentryRoleGrantPrivilege.
+   * Internally calls alterSentryGrantPrivilege.
    *
    * @param grantorPrincipal User name
    * @param roleName Role name
@@ -724,34 +696,35 @@ public class SentryStore {
   public void alterSentryRoleGrantPrivileges(final String grantorPrincipal,
       final String roleName, final Set<TSentryPrivilege> privileges) throws Exception {
     for (TSentryPrivilege privilege : privileges) {
-      alterSentryRoleGrantPrivilege(grantorPrincipal, roleName, privilege);
+      alterSentryGrantPrivilege(grantorPrincipal, SentryEntityType.ROLE, roleName, privilege, null);
     }
   }
 
   /**
-   * Alter a given sentry role to grant a privilege, as well as persist the corresponding
+   * Alter a given sentry role/user to grant a privilege, as well as persist the corresponding
    * permission change to MSentryPermChange table in a single transaction.
    *
    * @param grantorPrincipal User name
-   * @param roleName the given role name
+   * @param type Type of entity to which privilege is granted.
+   * @param name the name of the entity to which privilege is granted.
    * @param privilege the given privilege
-   * @param update the corresponding permission delta update.
+   * @param update the corresponding permission delta update if any.
    * @throws Exception
    *
    */
-  synchronized void alterSentryRoleGrantPrivilege(final String grantorPrincipal,
-      final String roleName, final TSentryPrivilege privilege,
-      final Update update) throws Exception {
+  synchronized void alterSentryGrantPrivilege(final String grantorPrincipal, SentryEntityType type,
+     final String name, final TSentryPrivilege privilege,
+     final Update update) throws Exception {
 
     execute(update, pm -> {
       pm.setDetachAllOnCommit(false); // No need to detach objects
-      String trimmedRoleName = trimAndLower(roleName);
+      String trimmedEntityName = trimAndLower(name);
       // first do grant check
       grantOptionCheck(pm, grantorPrincipal, privilege);
 
       // Alter sentry Role and grant Privilege.
-      MSentryPrivilege mPrivilege = alterSentryRoleGrantPrivilegeCore(pm,
-        trimmedRoleName, privilege);
+      MSentryPrivilege mPrivilege = alterSentryGrantPrivilegeCore(pm, type,
+              trimmedEntityName, privilege);
 
       if (mPrivilege != null) {
         // update the privilege to be the one actually updated.
@@ -764,7 +737,7 @@ public class SentryStore {
   /**
    * Alter a given sentry role to grant a set of privileges, as well as persist the
    * corresponding permission change to MSentryPermChange table in a single transaction.
-   * Internally calls alterSentryRoleGrantPrivilege.
+   * Internally calls alterSentryGrantPrivilege.
    *
    * @param grantorPrincipal User name
    * @param roleName the given role name
@@ -780,12 +753,8 @@ public class SentryStore {
     Preconditions.checkNotNull(privilegesUpdateMap);
     for (TSentryPrivilege privilege : privileges) {
       Update update = privilegesUpdateMap.get(privilege);
-      if (update != null) {
-        alterSentryRoleGrantPrivilege(grantorPrincipal, roleName, privilege,
+      alterSentryGrantPrivilege(grantorPrincipal, SentryEntityType.ROLE, roleName, privilege,
           update);
-      } else {
-        alterSentryRoleGrantPrivilege(grantorPrincipal, roleName, privilege);
-      }
     }
   }
 
@@ -814,13 +783,18 @@ public class SentryStore {
      return null;
    }
 
-   private MSentryPrivilege alterSentryRoleGrantPrivilegeCore(PersistenceManager pm,
-      String roleName, TSentryPrivilege privilege)
+  private MSentryPrivilege alterSentryGrantPrivilegeCore(PersistenceManager pm,
+     SentryEntityType type,
+     String entityName, TSentryPrivilege privilege)
       throws SentryNoSuchObjectException, SentryInvalidInputException {
     MSentryPrivilege mPrivilege = null;
-    MSentryRole mRole = getRole(pm, roleName);
-    if (mRole == null) {
-      throw noSuchRole(roleName);
+    PrivilegeEntity mEntity = getEntity(pm, entityName, type);
+    if (mEntity == null) {
+      if(type == SentryEntityType.ROLE) {
+        throw noSuchRole(entityName);
+      } else if(type == SentryEntityType.USER) {
+        throw noSuchUser(entityName);
+      }
     }
 
     if(privilege.getPrivilegeScope().equalsIgnoreCase(PrivilegeScope.URI.name())
@@ -837,16 +811,16 @@ public class SentryStore {
         TSentryPrivilege tNotAll = new TSentryPrivilege(privilege);
         tNotAll.setAction(AccessConstants.SELECT);
         MSentryPrivilege mSelect =
-            findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tNotAll));
+            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tNotAll));
         tNotAll.setAction(AccessConstants.INSERT);
         MSentryPrivilege mInsert =
-            findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tNotAll));
+            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tNotAll));
         if (mSelect != null) {
-          mSelect.removeRole(mRole);
+          mSelect.removeEntity(mEntity);
           persistPrivilege(pm, mSelect);
         }
         if (mInsert != null) {
-          mInsert.removeRole(mRole);
+          mInsert.removeEntity(mEntity);
           persistPrivilege(pm, mInsert);
         }
       } else {
@@ -855,10 +829,10 @@ public class SentryStore {
         TSentryPrivilege tAll = new TSentryPrivilege(privilege);
         tAll.setAction(AccessConstants.ALL);
         MSentryPrivilege mAll1 =
-            findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tAll));
+            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tAll));
         tAll.setAction(AccessConstants.ACTION_ALL);
         MSentryPrivilege mAll2 =
-            findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tAll));
+            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tAll));
         if (mAll1 != null) {
           return null;
         }
@@ -872,14 +846,14 @@ public class SentryStore {
     if (mPrivilege == null) {
       mPrivilege = convertToMSentryPrivilege(privilege);
     }
-    mPrivilege.appendRole(mRole);
+    mPrivilege.appendEntity(mEntity);
     pm.makePersistent(mPrivilege);
     return mPrivilege;
   }
 
   /**
    * Alter a given sentry user to grant a set of privileges.
-   * Internally calls alterSentryUserGrantPrivilege.
+   * Internally calls alterSentryGrantPrivilege.
    *
    * @param grantorPrincipal User name
    * @param userName User name
@@ -899,80 +873,14 @@ public class SentryStore {
     }
 
     for (TSentryPrivilege privilege : privileges) {
-      alterSentryUserGrantPrivilege(grantorPrincipal, userName, privilege);
+      alterSentryGrantPrivilege(grantorPrincipal, SentryEntityType.USER, userName, privilege, null);
     }
   }
 
   /**
-   * Alter a given sentry user to grant a privilege.
-   *
-   * @param grantorPrincipal User name
-   * @param userName the given user name
-   * @param privilege the given privilege
-   * @throws Exception
-   */
-  void alterSentryUserGrantPrivilege(final String grantorPrincipal,
-      final String userName, final TSentryPrivilege privilege) throws Exception {
-    tm.executeTransactionWithRetry(
-        new TransactionBlock<Object>() {
-          public Object execute(PersistenceManager pm) throws Exception {
-            pm.setDetachAllOnCommit(false); // No need to detach objects
-            String trimmedUserName = trimAndLower(userName);
-            // first do grant check
-            grantOptionCheck(pm, grantorPrincipal, privilege);
-
-            // Alter sentry User and grant Privilege.
-            MSentryPrivilege mPrivilege = alterSentryUserGrantPrivilegeCore(
-                pm, trimmedUserName, privilege);
-
-            if (mPrivilege != null) {
-              // update the privilege to be the one actually updated.
-              convertToTSentryPrivilege(mPrivilege, privilege);
-            }
-            return null;
-          }
-        });
-  }
-
-  /**
-   * Alter a given sentry user to grant a privilege, as well as persist the corresponding
-   * permission change to MSentryPermChange table in a single transaction.
-   *
-   * @param grantorPrincipal User name
-   * @param userName the given user name
-   * @param privilege the given privilege
-   * @param update the corresponding permission delta update.
-   * @throws Exception
-   *
-   */
-  synchronized void alterSentryUserGrantPrivilege(final String grantorPrincipal,
-      final String userName, final TSentryPrivilege privilege,
-      final Update update) throws Exception {
-
-    execute(update, new TransactionBlock<Object>() {
-      public Object execute(PersistenceManager pm) throws Exception {
-        pm.setDetachAllOnCommit(false); // No need to detach objects
-        String trimmedUserName = trimAndLower(userName);
-        // first do grant check
-        grantOptionCheck(pm, grantorPrincipal, privilege);
-
-        // Alter sentry User and grant Privilege.
-        MSentryPrivilege mPrivilege = alterSentryUserGrantPrivilegeCore(pm,
-            trimmedUserName, privilege);
-
-        if (mPrivilege != null) {
-          // update the privilege to be the one actually updated.
-          convertToTSentryPrivilege(mPrivilege, privilege);
-        }
-        return null;
-      }
-    });
-  }
-
-  /**
    * Alter a given sentry user to grant a set of privileges, as well as persist the
    * corresponding permission change to MSentryPermChange table in a single transaction.
-   * Internally calls alterSentryUserGrantPrivilege.
+   * Internally calls alterSentryGrantPrivilege.
    *
    * @param grantorPrincipal User name
    * @param userName the given user name
@@ -997,12 +905,8 @@ public class SentryStore {
     Preconditions.checkNotNull(privilegesUpdateMap);
     for (TSentryPrivilege privilege : privileges) {
       Update update = privilegesUpdateMap.get(privilege);
-      if (update != null) {
-        alterSentryUserGrantPrivilege(grantorPrincipal, userName, privilege,
-            update);
-      } else {
-        alterSentryUserGrantPrivilege(grantorPrincipal, userName, privilege);
-      }
+      alterSentryGrantPrivilege(grantorPrincipal, SentryEntityType.USER, userName, privilege,
+              update);
     }
   }
 
@@ -1043,98 +947,9 @@ public class SentryStore {
         });
   }
 
-  private MSentryPrivilege alterSentryUserGrantPrivilegeCore(PersistenceManager pm,
-      String userName, TSentryPrivilege privilege)
-      throws SentryNoSuchObjectException, SentryInvalidInputException {
-    MSentryPrivilege mPrivilege = null;
-    MSentryUser mUser = getUser(pm, userName);
-    if (mUser == null) {
-      throw noSuchUser(userName);
-    }
-
-    if(privilege.getPrivilegeScope().equalsIgnoreCase(PrivilegeScope.URI.name())
-        && StringUtils.isBlank(privilege.getURI())) {
-      throw new SentryInvalidInputException("cannot grant URI privileges to Null or EMPTY location");
-    }
-
-    if (!isNULL(privilege.getColumnName()) || !isNULL(privilege.getTableName())
-        || !isNULL(privilege.getDbName())) {
-      // If Grant is for ALL and Either INSERT/SELECT already exists..
-      // need to remove it and GRANT ALL..
-      if (AccessConstants.ALL.equalsIgnoreCase(privilege.getAction())
-          || AccessConstants.ACTION_ALL.equalsIgnoreCase(privilege.getAction())) {
-        TSentryPrivilege tNotAll = new TSentryPrivilege(privilege);
-        tNotAll.setAction(AccessConstants.SELECT);
-        MSentryPrivilege mSelect =
-            findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tNotAll));
-        tNotAll.setAction(AccessConstants.INSERT);
-        MSentryPrivilege mInsert =
-            findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tNotAll));
-        if (mSelect != null) {
-          mSelect.removeUser(mUser);
-          persistPrivilege(pm, mSelect);
-        }
-        if (mInsert != null) {
-          mInsert.removeUser(mUser);
-          persistPrivilege(pm, mInsert);
-        }
-      } else {
-        // If Grant is for Either INSERT/SELECT and ALL already exists..
-        // do nothing..
-        TSentryPrivilege tAll = new TSentryPrivilege(privilege);
-        tAll.setAction(AccessConstants.ALL);
-        MSentryPrivilege mAll1 =
-            findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tAll));
-        tAll.setAction(AccessConstants.ACTION_ALL);
-        MSentryPrivilege mAll2 =
-            findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tAll));
-        if (mAll1 != null) {
-          return null;
-        }
-        if (mAll2 != null) {
-          return null;
-        }
-      }
-    }
-
-    mPrivilege = getMSentryPrivilege(privilege, pm);
-    if (mPrivilege == null) {
-      mPrivilege = convertToMSentryPrivilege(privilege);
-    }
-    mPrivilege.appendUser(mUser);
-    pm.makePersistent(mPrivilege);
-    return mPrivilege;
-  }
-
-  /**
-   * Alter a given sentry user to revoke a privilege.
-   *
-   * @param grantorPrincipal User name
-   * @param userName the given user name
-   * @param tPrivilege the given privilege
-   * @throws Exception
-   *
-   */
-  void alterSentryUserRevokePrivilege(final String grantorPrincipal,
-      final String userName, final TSentryPrivilege tPrivilege) throws Exception {
-
-    tm.executeTransactionWithRetry(
-        new TransactionBlock<Object>() {
-          public Object execute(PersistenceManager pm) throws Exception {
-            pm.setDetachAllOnCommit(false); // No need to detach objects
-            String trimmedUserName = safeTrimLower(userName);
-            // first do revoke check
-            grantOptionCheck(pm, grantorPrincipal, tPrivilege);
-
-            alterSentryUserRevokePrivilegeCore(pm, trimmedUserName, tPrivilege);
-            return null;
-          }
-        });
-  }
-
   /**
    * Alter a given sentry user to revoke a set of privileges.
-   * Internally calls alterSentryUserRevokePrivilege.
+   * Internally calls alterSentryRevokePrivilege.
    *
    * @param grantorPrincipal User name
    * @param userName the given user name
@@ -1145,14 +960,14 @@ public class SentryStore {
   public void alterSentryUserRevokePrivileges(final String grantorPrincipal,
       final String userName, final Set<TSentryPrivilege> tPrivileges) throws Exception {
     for (TSentryPrivilege tPrivilege : tPrivileges) {
-      alterSentryUserRevokePrivilege(grantorPrincipal, userName, tPrivilege);
+      alterSentryRevokePrivilege(grantorPrincipal, SentryEntityType.USER, userName, tPrivilege, null);
     }
   }
 
   /**
    * Alter a given sentry user to revoke a set of privileges, as well as persist the
    * corresponding permission change to MSentryPermChange table in a single transaction.
-   * Internally calls alterSentryUserRevokePrivilege.
+   * Internally calls alterSentryRevokePrivilege.
    *
    * @param grantorPrincipal User name
    * @param userName the given user name
@@ -1169,123 +984,14 @@ public class SentryStore {
     Preconditions.checkNotNull(privilegesUpdateMap);
     for (TSentryPrivilege tPrivilege : tPrivileges) {
       Update update = privilegesUpdateMap.get(tPrivilege);
-      if (update != null) {
-        alterSentryUserRevokePrivilege(grantorPrincipal, userName,
-            tPrivilege, update);
-      } else {
-        alterSentryUserRevokePrivilege(grantorPrincipal, userName,
-            tPrivilege);
-      }
-    }
-  }
-
-  /**
-   * Alter a given sentry user to revoke a privilege, as well as persist the corresponding
-   * permission change to MSentryPermChange table in a single transaction.
-   *
-   * @param grantorPrincipal User name
-   * @param userName the given user name
-   * @param tPrivilege the given privilege
-   * @param update the corresponding permission delta update transaction block
-   * @throws Exception
-   *
-   */
-  private synchronized void alterSentryUserRevokePrivilege(final String grantorPrincipal,
-      final String userName, final TSentryPrivilege tPrivilege,
-      final Update update) throws Exception {
-    execute(update, new TransactionBlock<Object>() {
-      public Object execute(PersistenceManager pm) throws Exception {
-        pm.setDetachAllOnCommit(false); // No need to detach objects
-        String trimmedUserName = safeTrimLower(userName);
-        // first do revoke check
-        grantOptionCheck(pm, grantorPrincipal, tPrivilege);
-
-        alterSentryUserRevokePrivilegeCore(pm, trimmedUserName, tPrivilege);
-        return null;
-      }
-    });
-  }
-
-  private void alterSentryUserRevokePrivilegeCore(PersistenceManager pm,
-      String userName, TSentryPrivilege tPrivilege)
-      throws SentryNoSuchObjectException, SentryInvalidInputException {
-    MSentryUser mUser = getUser(pm, userName);
-    if (mUser == null) {
-      throw noSuchUser(userName);
-    }
-    if(tPrivilege.getPrivilegeScope().equalsIgnoreCase(PrivilegeScope.URI.name())
-        && StringUtils.isBlank(tPrivilege.getURI())) {
-      throw new SentryInvalidInputException("cannot revoke URI privileges from Null or EMPTY location");
-    }
-
-    MSentryPrivilege mPrivilege = getMSentryPrivilege(tPrivilege, pm);
-    if (mPrivilege == null) {
-      mPrivilege = convertToMSentryPrivilege(tPrivilege);
-    } else {
-      mPrivilege = pm.detachCopy(mPrivilege);
-    }
-
-    Set<MSentryPrivilege> privilegeGraph = new HashSet<>();
-    if (mPrivilege.getGrantOption() != null) {
-      privilegeGraph.add(mPrivilege);
-    } else {
-      MSentryPrivilege mTure = new MSentryPrivilege(mPrivilege);
-      mTure.setGrantOption(true);
-      privilegeGraph.add(mTure);
-      MSentryPrivilege mFalse = new MSentryPrivilege(mPrivilege);
-      mFalse.setGrantOption(false);
-      privilegeGraph.add(mFalse);
+      alterSentryRevokePrivilege(grantorPrincipal, SentryEntityType.USER, userName,
+              tPrivilege, update);
     }
-    // Get the privilege graph
-    populateChildren(pm, SentryEntityType.USER, Sets.newHashSet(userName), mPrivilege, privilegeGraph);
-    for (MSentryPrivilege childPriv : privilegeGraph) {
-      revokePrivilegeFromUser(pm, tPrivilege, mUser, childPriv);
-    }
-
-    persistUser(pm, mUser);
-  }
-
-  /**
-   * If user is stale, delete it from database. Otherwise, persist the user
-   * @param pm persistence manager
-   * @param user user to persist
-   */
-  private void persistUser(PersistenceManager pm, MSentryUser user) {
-    if (isUserStale(user)) {
-      pm.deletePersistent(user);
-      return;
-    }
-
-    pm.makePersistent(user);
-  }
-
-  /**
-  * Alter a given sentry role to revoke a privilege.
-  *
-  * @param grantorPrincipal User name
-  * @param roleName the given role name
-  * @param tPrivilege the given privilege
-  * @throws Exception
-  *
-  */
-  void alterSentryRoleRevokePrivilege(final String grantorPrincipal,
-      final String roleName, final TSentryPrivilege tPrivilege) throws Exception {
-
-    tm.executeTransactionWithRetry(
-            pm -> {
-              pm.setDetachAllOnCommit(false); // No need to detach objects
-              String trimmedRoleName = safeTrimLower(roleName);
-              // first do revoke check
-              grantOptionCheck(pm, grantorPrincipal, tPrivilege);
-
-              alterSentryRoleRevokePrivilegeCore(pm, trimmedRoleName, tPrivilege);
-              return null;
-            });
   }
 
   /**
    * Alter a given sentry role to revoke a set of privileges.
-   * Internally calls alterSentryRoleRevokePrivilege.
+   * Internally calls alterSentryRevokePrivilege.
    *
    * @param grantorPrincipal User name
    * @param roleName the given role name
@@ -1296,7 +1002,7 @@ public class SentryStore {
   public void alterSentryRoleRevokePrivileges(final String grantorPrincipal,
       final String roleName, final Set<TSentryPrivilege> tPrivileges) throws Exception {
     for (TSentryPrivilege tPrivilege : tPrivileges) {
-      alterSentryRoleRevokePrivilege(grantorPrincipal, roleName, tPrivilege);
+      alterSentryRevokePrivilege(grantorPrincipal, SentryEntityType.ROLE, roleName, tPrivilege, null);
     }
   }
 
@@ -1305,22 +1011,23 @@ public class SentryStore {
    * permission change to MSentryPermChange table in a single transaction.
    *
    * @param grantorPrincipal User name
-   * @param roleName the given role name
+   * @param type Type of entity to which privilege is granted.
+   * @param entityName the name of the entity from which privilege is revoked.
    * @param tPrivilege the given privilege
    * @param update the corresponding permission delta update transaction block
    * @throws Exception
    *
    */
-  private synchronized void alterSentryRoleRevokePrivilege(final String grantorPrincipal,
-                                              final String roleName, final TSentryPrivilege tPrivilege,
+  synchronized void alterSentryRevokePrivilege(final String grantorPrincipal, SentryEntityType type,
+                                              final String entityName, final TSentryPrivilege tPrivilege,
                                               final Update update) throws Exception {
     execute(update, pm -> {
       pm.setDetachAllOnCommit(false); // No need to detach objects
-      String trimmedRoleName = safeTrimLower(roleName);
+      String trimmedEntityName = safeTrimLower(entityName);
       // first do revoke check
       grantOptionCheck(pm, grantorPrincipal, tPrivilege);
 
-      alterSentryRoleRevokePrivilegeCore(pm, trimmedRoleName, tPrivilege);
+      alterSentryRevokePrivilegeCore(pm, type, trimmedEntityName, tPrivilege);
       return null;
     });
   }
@@ -1328,7 +1035,7 @@ public class SentryStore {
   /**
    * Alter a given sentry role to revoke a set of privileges, as well as persist the
    * corresponding permission change to MSentryPermChange table in a single transaction.
-   * Internally calls alterSentryRoleRevokePrivilege.
+   * Internally calls alterSentryRevokePrivilege.
    *
    * @param grantorPrincipal User name
    * @param roleName the given role name
@@ -1345,22 +1052,21 @@ public class SentryStore {
     Preconditions.checkNotNull(privilegesUpdateMap);
     for (TSentryPrivilege tPrivilege : tPrivileges) {
       Update update = privilegesUpdateMap.get(tPrivilege);
-      if (update != null) {
-        alterSentryRoleRevokePrivilege(grantorPrincipal, roleName,
-          tPrivilege, update);
-      } else {
-        alterSentryRoleRevokePrivilege(grantorPrincipal, roleName,
-          tPrivilege);
-      }
+      alterSentryRevokePrivilege(grantorPrincipal, SentryEntityType.ROLE, roleName,
+              tPrivilege, update);
     }
   }
 
-  private void alterSentryRoleRevokePrivilegeCore(PersistenceManager pm,
-      String roleName, TSentryPrivilege tPrivilege)
+  private void alterSentryRevokePrivilegeCore(PersistenceManager pm, SentryEntityType type,
+      String entityName, TSentryPrivilege tPrivilege)
       throws SentryNoSuchObjectException, SentryInvalidInputException {
-    MSentryRole mRole = getRole(pm, roleName);
-    if (mRole == null) {
-      throw noSuchRole(roleName);
+    PrivilegeEntity mEntity = getEntity(pm, entityName, type);
+    if (mEntity == null) {
+      if(type == SentryEntityType.ROLE) {
+        throw noSuchRole(entityName);
+      } else if(type == SentryEntityType.USER) {
+        throw noSuchUser (entityName);
+      }
     }
     if(tPrivilege.getPrivilegeScope().equalsIgnoreCase(PrivilegeScope.URI.name())
         && StringUtils.isBlank(tPrivilege.getURI())) {
@@ -1386,21 +1092,21 @@ public class SentryStore {
       privilegeGraph.add(mFalse);
     }
     // Get the privilege graph
-    populateChildren(pm, SentryEntityType.ROLE, Sets.newHashSet(roleName), mPrivilege, privilegeGraph);
+    populateChildren(pm, SentryEntityType.ROLE, Sets.newHashSet(entityName), mPrivilege, privilegeGraph);
     for (MSentryPrivilege childPriv : privilegeGraph) {
-      revokePrivilegeFromRole(pm, tPrivilege, mRole, childPriv);
+      revokePrivilege(pm, tPrivilege, mEntity, childPriv);
     }
-    pm.makePersistent(mRole);
+    persistEntity(pm , type, mEntity);
   }
 
   /**
-   * Roles can be granted ALL, SELECT, and INSERT on tables. When
-   * a role has ALL and SELECT or INSERT are revoked, we need to remove the ALL
+   * Roles/Users can be granted ALL, SELECT, and INSERT on tables. When
+   * a role/user has ALL and SELECT or INSERT are revoked, we need to remove the ALL
    * privilege and add SELECT (INSERT was revoked) or INSERT (SELECT was revoked).
    */
   private void revokePartial(PersistenceManager pm,
                              TSentryPrivilege requestedPrivToRevoke,
-                             MSentryRole mRole, MSentryUser mUser,
+                             PrivilegeEntity mEntity,
                              MSentryPrivilege currentPrivilege) throws SentryInvalidInputException {
     MSentryPrivilege persistedPriv =
       getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
@@ -1414,13 +1120,9 @@ public class SentryStore {
     if (requestedPrivToRevoke.getAction().equalsIgnoreCase(AccessConstants.ALL) ||
       requestedPrivToRevoke.getAction().equalsIgnoreCase(AccessConstants.ACTION_ALL)) {
       if (!persistedPriv.getRoles().isEmpty()) {
-        if (mRole != null) {
-          persistedPriv.removeRole(mRole);
-        }
-        if (mUser != null) {
-          persistedPriv.removeUser(mUser);
+        if (mEntity != null) {
+          persistedPriv.removeEntity(mEntity);
         }
-
         persistPrivilege(pm, persistedPriv);
       }
     } else {
@@ -1435,14 +1137,25 @@ public class SentryStore {
         }
       }
 
-      if (mRole != null) {
-        revokeRolePartial(pm, mRole, currentPrivilege, persistedPriv, addActions);
+      if (mEntity != null) {
+        revokePrivilegeAndGrantPartial(pm, mEntity, currentPrivilege, persistedPriv, addActions);
       }
+    }
+  }
 
-      if (mUser != null) {
-        revokeUserPartial(pm, mUser, currentPrivilege, persistedPriv, addActions);
-      }
+  /**
+   * Persists the changes in entity
+   * @param pm persistence manager
+   * @param type Type of privilege entity
+   * @param entity privilege entity to persist
+   *
+   */
+  private void persistEntity(PersistenceManager pm, SentryEntityType type, PrivilegeEntity entity) {
+    if (type == SentryEntityType.USER && isUserStale((MSentryUser) entity)) {
+      pm.deletePersistent(entity);
+      return;
     }
+    pm.makePersistent(entity);
   }
 
   private boolean isUserStale(MSentryUser user) {
@@ -1462,6 +1175,7 @@ public class SentryStore {
     pm.makePersistent(privilege);
   }
 
+
   private boolean isPrivilegeStale(MSentryPrivilege privilege) {
     if (privilege.getUsers().isEmpty() && privilege.getRoles().isEmpty()) {
       return true;
@@ -1478,23 +1192,22 @@ public class SentryStore {
     return false;
   }
 
-  private void revokeRolePartial(PersistenceManager pm, MSentryRole mRole,
-                                 MSentryPrivilege currentPrivilege,
-                                 MSentryPrivilege persistedPriv,
-                                 Set<String> addActions) throws SentryInvalidInputException {
+  private void revokePrivilegeAndGrantPartial(PersistenceManager pm, PrivilegeEntity mEntity,
+                                              MSentryPrivilege currentPrivilege,
+                                              MSentryPrivilege persistedPriv,
+                                              Set<String> addActions) throws SentryInvalidInputException {
     // If table / URI, remove ALL
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
-    if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
-      persistedPriv.removeRole(mRole);
+    if (persistedPriv != null) {
+      persistedPriv.removeEntity(mEntity);
       persistPrivilege(pm, persistedPriv);
     }
     currentPrivilege.setAction(AccessConstants.ALL);
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
-    if (persistedPriv != null && mRole.getPrivileges().contains(persistedPriv)) {
-      persistedPriv.removeRole(mRole);
+    if (persistedPriv != null && mEntity.getPrivileges().contains(persistedPriv)) {
+      persistedPriv.removeEntity(mEntity);
       persistPrivilege(pm, persistedPriv);
-
-      // add decomposted actions
+      // add decomposed actions
       for (String addAction : addActions) {
         currentPrivilege.setAction(addAction);
         TSentryPrivilege tSentryPrivilege = convertToTSentryPrivilege(currentPrivilege);
@@ -1502,40 +1215,9 @@ public class SentryStore {
         if (persistedPriv == null) {
           persistedPriv = convertToMSentryPrivilege(tSentryPrivilege);
         }
-        mRole.appendPrivilege(persistedPriv);
+        mEntity.appendPrivilege(persistedPriv);
       }
-      persistedPriv.appendRole(mRole);
-      pm.makePersistent(persistedPriv);
-    }
-  }
-
-  private void revokeUserPartial(PersistenceManager pm, MSentryUser mUser,
-      MSentryPrivilege currentPrivilege,
-      MSentryPrivilege persistedPriv,
-      Set<String> addActions) throws SentryInvalidInputException {
-    // If table / URI, remove ALL
-    persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
-    if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
-      persistedPriv.removeUser(mUser);
-      persistPrivilege(pm, persistedPriv);
-    }
-    currentPrivilege.setAction(AccessConstants.ALL);
-    persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
-    if (persistedPriv != null && mUser.getPrivileges().contains(persistedPriv)) {
-      persistedPriv.removeUser(mUser);
-      persistPrivilege(pm, persistedPriv);
-
-      // add decomposted actions
-      for (String addAction : addActions) {
-        currentPrivilege.setAction(addAction);
-        TSentryPrivilege tSentryPrivilege = convertToTSentryPrivilege(currentPrivilege);
-        persistedPriv = getMSentryPrivilege(tSentryPrivilege, pm);
-        if (persistedPriv == null) {
-          persistedPriv = convertToMSentryPrivilege(tSentryPrivilege);
-        }
-        mUser.appendPrivilege(persistedPriv);
-      }
-      persistedPriv.appendUser(mUser);
+      persistedPriv.appendEntity(mEntity);
       pm.makePersistent(persistedPriv);
     }
   }
@@ -1543,40 +1225,19 @@ public class SentryStore {
   /**
    * Revoke privilege from role
    */
-  private void revokePrivilegeFromRole(PersistenceManager pm, TSentryPrivilege tPrivilege,
-                                       MSentryRole mRole, MSentryPrivilege mPrivilege)
+  private void revokePrivilege(PersistenceManager pm, TSentryPrivilege tPrivilege,
+                               PrivilegeEntity mEntity, MSentryPrivilege mPrivilege)
     throws SentryInvalidInputException {
     if (PARTIAL_REVOKE_ACTIONS.contains(mPrivilege.getAction())) {
-      // if this privilege is in parital revoke actions
+      // if this privilege is in partial revoke actions
       // we will do partial revoke
-      revokePartial(pm, tPrivilege, mRole, null, mPrivilege);
+      revokePartial(pm, tPrivilege, mEntity, mPrivilege);
     } else {
       // otherwise,
       // we will revoke it from role directly
       MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege), pm);
       if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
-        persistedPriv.removeRole(mRole);
-        persistPrivilege(pm, persistedPriv);
-      }
-    }
-  }
-
-  /**
-   * Revoke privilege from user
-   */
-  private void revokePrivilegeFromUser(PersistenceManager pm, TSentryPrivilege tPrivilege,
-      MSentryUser mUser, MSentryPrivilege mPrivilege)
-      throws SentryInvalidInputException {
-    if (PARTIAL_REVOKE_ACTIONS.contains(mPrivilege.getAction())) {
-      // if this privilege is in parital revoke actions
-      // we will do partial revoke
-      revokePartial(pm, tPrivilege, null, mUser, mPrivilege);
-    } else {
-      // otherwise,
-      // we will revoke it from user directly
-      MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege), pm);
-      if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
-        persistedPriv.removeUser(mUser);
+        persistedPriv.removeEntity(mEntity);
         persistPrivilege(pm, persistedPriv);
       }
     }
@@ -3038,7 +2699,7 @@ public class SentryStore {
     // Dropping the privilege
     if (newTPrivilege == null) {
       for (MSentryRole role : roleSet) {
-        alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege);
+        alterSentryRevokePrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPrivilege);
       }
       return;
     }
@@ -3063,7 +2724,7 @@ public class SentryStore {
           privilegeGraph);
       }
       // 2. revoke privilege and child privileges
-      alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege);
+      alterSentryRevokePrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPrivilege);
       // 3. add new privilege and child privileges with new tableName
       for (MSentryPrivilege mPriv : privilegeGraph) {
         TSentryPrivilege tPriv = convertToTSentryPrivilege(mPriv);
@@ -3074,7 +2735,7 @@ public class SentryStore {
           tPriv.setDbName(newTPrivilege.getDbName());
           tPriv.setTableName(newTPrivilege.getTableName());
         }
-        alterSentryRoleGrantPrivilegeCore(pm, role.getRoleName(), tPriv);
+        alterSentryGrantPrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPriv);
       }
     }
   }
@@ -4360,7 +4021,7 @@ public class SentryStore {
         // get the privileges for the role
         Set<TSentryPrivilege> tSentryPrivileges = entry.getValue();
         for (TSentryPrivilege tSentryPrivilege : tSentryPrivileges) {
-          alterSentryRoleGrantPrivilegeCore(pm, entry.getKey(), tSentryPrivilege);
+          alterSentryGrantPrivilegeCore(pm, SentryEntityType.ROLE, entry.getKey(), tSentryPrivilege);
         }
       }
     }
@@ -4716,7 +4377,7 @@ public class SentryStore {
   public List<MSentryPathChange> getMSentryPathChanges(final long changeID)
           throws Exception {
     return tm.executeTransaction(pm -> {
-      // 1. We first retrieve the entire list of latest delta changes since the changeID
+      // 1. We first rextrieve the entire list of latest delta changes since the changeID
       List<MSentryPathChange> pathChanges =
               getMSentryChangesCore(pm, MSentryPathChange.class, changeID);
       // 2. We then check for consistency issues with delta changes
@@ -4811,7 +4472,7 @@ public class SentryStore {
         TransactionBlock<Object> transactionBlock) throws Exception {
     List<TransactionBlock<Object>> tbs = new ArrayList<>(2);
 
-    if (persistUpdateDeltas) {
+    if (persistUpdateDeltas && update != null) {
       tbs.add(new DeltaTransactionBlock(update));
     }
 
@@ -4838,4 +4499,28 @@ public class SentryStore {
       return changes != null;
     });
   }
+
+  /**
+   * Get a single entity with the given name and type inside a transaction
+   * @param pm Persistence Manager instance
+   * @param name Role/user name (should not be null)
+   * @param type Type of entity
+   * @return single PrivilegeEntity with the given name and type
+   */
+  public PrivilegeEntity getEntity(PersistenceManager pm, String name, SentryEntityType type) {
+    Query query;
+    if(type == SentryEntityType.ROLE) {
+      query = pm.newQuery(MSentryRole.class);
+      query.addExtension(LOAD_RESULTS_AT_COMMIT, "false");
+      query.setFilter("this.roleName == :roleName");
+      query.setUnique(true);
+    } else {
+      query = pm.newQuery(MSentryUser.class);
+      query.addExtension(LOAD_RESULTS_AT_COMMIT, "false");
+      query.setFilter("this.userName == :userName");
+      query.setUnique(true);
+    }
+    return (PrivilegeEntity) query.execute(name);
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java
index 65d26c0..a9e8230 100644
--- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java
+++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java
@@ -102,7 +102,7 @@ public class TestSentryRole {
     //add hivePrivilege to role
     pm = openTransaction();
     MSentryRole role = getMSentryRole(pm, roleName);
-    hivePrivilege.appendRole(role);
+    hivePrivilege.appendEntity(role);
     pm.makePersistent(hivePrivilege);
     commitTransaction(pm);
     //check hivePrivlege and solrPrivilege
@@ -163,7 +163,7 @@ public class TestSentryRole {
     pm = openTransaction();
     MSentryRole role = getMSentryRole(pm, roleName);
     solrPrivilege.appendRole(role);
-    hivePrivilege.appendRole(role);
+    hivePrivilege.appendEntity(role);
     pm.makePersistent(solrPrivilege);
     pm.makePersistent(hivePrivilege);
     commitTransaction(pm);
@@ -221,7 +221,7 @@ public class TestSentryRole {
     //grant hivePrivilege and solrPrivilege to role
     pm = openTransaction();
     MSentryRole role = getMSentryRole(pm, roleName);
-    hivePrivilege.appendRole(role);
+    hivePrivilege.appendEntity(role);
     solrPrivilege.appendRole(role);
     pm.makePersistent(hivePrivilege);
     pm.makePersistent(solrPrivilege);
@@ -256,7 +256,7 @@ public class TestSentryRole {
     role = getMSentryRole(pm, roleName);
     pm.retrieve(role);
     hivePrivilege = (MSentryPrivilege)role.getPrivileges().toArray()[0];
-    hivePrivilege.removeRole(role);
+    hivePrivilege.removeEntity(role);
     pm.makePersistent(hivePrivilege);
     commitTransaction(pm);
 
@@ -300,7 +300,7 @@ public class TestSentryRole {
     //grant hivePrivilege and solrPrivilege to role
     pm = openTransaction();
     MSentryRole role = getMSentryRole(pm, roleName);
-    hivePrivilege.appendRole(role);
+    hivePrivilege.appendEntity(role);
     solrPrivilege.appendRole(role);
     pm.makePersistent(hivePrivilege);
     pm.makePersistent(solrPrivilege);
@@ -379,7 +379,7 @@ public class TestSentryRole {
     //grant hivePrivilege and solrPrivilege to role
     pm = openTransaction();
     MSentryRole role = getMSentryRole(pm, roleName);
-    hivePrivilege.appendRole(role);
+    hivePrivilege.appendEntity(role);
     solrPrivilege.appendRole(role);
     pm.makePersistent(hivePrivilege);
     pm.makePersistent(solrPrivilege);
@@ -465,9 +465,9 @@ public class TestSentryRole {
     pm = openTransaction();
     MSentryRole role1 = getMSentryRole(pm, roleName1);
     MSentryRole role2 = getMSentryRole(pm, roleName2);
-    hivePrivilege.appendRole(role1);
+    hivePrivilege.appendEntity(role1);
     solrPrivilege.appendRole(role1);
-    hivePrivilege.appendRole(role2);
+    hivePrivilege.appendEntity(role2);
     solrPrivilege.appendRole(role2);
     pm.makePersistent(hivePrivilege);
     pm.makePersistent(solrPrivilege);

http://git-wip-us.apache.org/repos/asf/sentry/blob/112cdcd4/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
index 4e8a2e6..6a84b05 100644
--- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
+++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
@@ -34,6 +34,7 @@ import org.apache.hadoop.hive.metastore.messaging.EventMessage.EventType;
 import org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageFactory;
 
 import org.apache.sentry.api.service.thrift.TSentryPrivilege;
+import org.apache.sentry.service.common.ServiceConstants;
 import org.apache.sentry.service.thrift.HiveSimpleConnectionFactory;
 import org.apache.sentry.provider.file.PolicyFile;
 import org.apache.sentry.service.common.ServiceConstants.ServerConfig;
@@ -176,11 +177,11 @@ public class TestHMSFollowerSentryStoreIntegration {
     privilege_server.setServerName(serverName1);
     privilege_server.setCreateTime(System.currentTimeMillis());
 
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1, null);
 
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1_2);
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_server);
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1_3);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1_2, null);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege_server, null);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1_3, null);
 
     // Create notification events to drop the table
     StorageDescriptor sd = new StorageDescriptor();
@@ -236,11 +237,11 @@ public class TestHMSFollowerSentryStoreIntegration {
     privilege_server.setServerName(serverName1);
     privilege_server.setCreateTime(System.currentTimeMillis());
 
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1, null);
 
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1_2);
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_server);
-    sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege1_3);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1_2, null);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege_server, null);
+    sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryEntityType.ROLE, roleName1, privilege1_3, null);
 
     // Create notification events to drop the database
     NotificationEvent notificationEvent = new NotificationEvent(1, 0, EventType.DROP_DATABASE.toString(),


Mime
View raw message