sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject sentry git commit: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned (Alexander Kolbasov, Reviewed by: Misha Dmitriev)
Date Tue, 07 Feb 2017 20:10:49 GMT
Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign 9f09f305a -> 3ada8a36e


SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned (Alexander
Kolbasov, Reviewed by: Misha Dmitriev)


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

Branch: refs/heads/sentry-ha-redesign
Commit: 3ada8a36e08b14fd6c86a879867cc2748e97e693
Parents: 9f09f30
Author: Alexander Kolbasov <akolb@cloudera.com>
Authored: Tue Feb 7 08:16:18 2017 -0800
Committer: Alexander Kolbasov <akolb@cloudera.com>
Committed: Tue Feb 7 12:05:17 2017 -0800

----------------------------------------------------------------------
 .../service/persistent/DelegateSentryStore.java |  86 +++----
 .../db/service/persistent/SentryStore.java      | 248 ++++++++++---------
 2 files changed, 170 insertions(+), 164 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/3ada8a36/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
index 5f396b8..23f0891 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
@@ -18,6 +18,7 @@
 package org.apache.sentry.provider.db.generic.service.persistent;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -234,12 +235,15 @@ public class DelegateSentryStore implements SentryStoreLayer {
   }
 
   @Override
-  public Set<String> getRolesByGroups(String component, Set<String> groups) throws
Exception {
-    Set<String> roles = Sets.newHashSet();
-    if (groups == null) {
-      return roles;
+  public Set<String> getRolesByGroups(String component, Set<String> groups)
+      throws Exception {
+    if (groups == null || groups.isEmpty()) {
+      return Collections.emptySet();
     }
-    for (TSentryRole tSentryRole : delegate.getTSentryRolesByGroupName(groups, true)) {
+
+    Set<String> roles = Sets.newHashSet();
+    for (TSentryRole tSentryRole : delegate.getTSentryRolesByGroupName(groups,
+            true)) {
       roles.add(tSentryRole.getRoleName());
     }
     return roles;
@@ -248,20 +252,20 @@ public class DelegateSentryStore implements SentryStoreLayer {
   @Override
   public Set<String> getGroupsByRoles(final String component, final Set<String>
roles)
       throws Exception {
-    final Set<String> trimmedRoles = toTrimmedLower(roles);
-    final Set<String> groupNames = Sets.newHashSet();
-    if (trimmedRoles.size() == 0) {
-      return groupNames;
+    if (roles.isEmpty()) {
+      return Collections.emptySet();
     }
 
     return delegate.getTransactionManager().executeTransaction(
       new TransactionBlock<Set<String>>() {
         public Set<String> execute(PersistenceManager pm) throws Exception {
           //get groups by roles
+          final Set<String> trimmedRoles = SentryStore.toTrimedLower(roles);
+
           Query query = pm.newQuery(MSentryGroup.class);
           StringBuilder filters = new StringBuilder();
           query.declareVariables("MSentryRole role");
-          List<String> rolesFiler = new LinkedList<String>();
+          List<String> rolesFiler = new LinkedList<>();
           for (String role : trimmedRoles) {
             rolesFiler.add("role.roleName == \"" + role + "\" ");
           }
@@ -269,9 +273,10 @@ public class DelegateSentryStore implements SentryStoreLayer {
           query.setFilter(filters.toString());
           @SuppressWarnings("unchecked")
           List<MSentryGroup> groups = (List<MSentryGroup>)query.execute();
-          if (groups == null) {
-            return groupNames;
+          if (groups.isEmpty()) {
+            return Collections.emptySet();
           }
+          Set<String> groupNames = new HashSet<>(groups.size());
           for (MSentryGroup group : groups) {
             groupNames.add(group.getGroupName());
           }
@@ -284,15 +289,14 @@ public class DelegateSentryStore implements SentryStoreLayer {
   public Set<PrivilegeObject> getPrivilegesByRole(final String component,
       final Set<String> roles) throws Exception {
     Preconditions.checkNotNull(roles);
-    final Set<PrivilegeObject> privileges = Sets.newHashSet();
     if (roles.isEmpty()) {
-      return privileges;
+      return Collections.emptySet();
     }
-
     return delegate.getTransactionManager().executeTransaction(
       new TransactionBlock<Set<PrivilegeObject>>() {
         public Set<PrivilegeObject> execute(PersistenceManager pm) throws Exception
{
-          Set<MSentryRole> mRoles = Sets.newHashSet();
+          Set<PrivilegeObject> privileges = new HashSet<>();
+          Set<MSentryRole> mRoles = new HashSet<>();
           for (String role : roles) {
             MSentryRole mRole = getRole(toTrimmedLower(role), pm);
             if (mRole != null) {
@@ -318,19 +322,18 @@ public class DelegateSentryStore implements SentryStoreLayer {
           String trimmedComponent = toTrimmedLower(component);
           String trimmedService = toTrimmedLower(service);
 
-          Set<PrivilegeObject> privileges = Sets.newHashSet();
           //CaseInsensitive roleNames
-          Set<String> trimmedRoles = toTrimmedLower(roles);
+          Set<String> trimmedRoles = SentryStore.toTrimedLower(roles);
 
           if (groups != null) {
             trimmedRoles.addAll(delegate.getRoleNamesForGroups(groups));
           }
 
-          if (trimmedRoles.size() == 0) {
-            return privileges;
+          if (trimmedRoles.isEmpty()) {
+            return Collections.emptySet();
           }
 
-          Set<MSentryRole> mRoles = Sets.newHashSet();
+          Set<MSentryRole> mRoles = new HashSet<>(trimmedRoles.size());
           for (String role : trimmedRoles) {
             MSentryRole mRole = getRole(role, pm);
             if (mRole != null) {
@@ -338,6 +341,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
             }
           }
           //get the privileges
+          Set<PrivilegeObject> privileges = new HashSet<>();
           privileges.addAll(privilegeOperator.
                   getPrivilegesByProvider(trimmedComponent,
                           trimmedService, mRoles, authorizables, pm));
@@ -350,21 +354,19 @@ public class DelegateSentryStore implements SentryStoreLayer {
   public Set<MSentryGMPrivilege> getPrivilegesByAuthorizable(final String component,
       final String service, final Set<String> validActiveRoles,
       final List<? extends Authorizable> authorizables) throws Exception {
-    Preconditions.checkNotNull(component);
-    Preconditions.checkNotNull(service);
-
-    final Set<MSentryGMPrivilege> privileges = Sets.newHashSet();
-
     if (validActiveRoles == null || validActiveRoles.isEmpty()) {
-      return privileges;
+      return Collections.emptySet();
     }
 
+    Preconditions.checkNotNull(component);
+    Preconditions.checkNotNull(service);
+
     return delegate.getTransactionManager().executeTransaction(
       new TransactionBlock<Set<MSentryGMPrivilege>>() {
         public Set<MSentryGMPrivilege> execute(PersistenceManager pm) throws Exception
{
           String lComponent = toTrimmedLower(component);
           String lService = toTrimmedLower(service);
-          Set<MSentryRole> mRoles = Sets.newHashSet();
+          Set<MSentryRole> mRoles = new HashSet<>(validActiveRoles.size());
           for (String role : validActiveRoles) {
             MSentryRole mRole = getRole(role, pm);
             if (mRole != null) {
@@ -377,6 +379,8 @@ public class DelegateSentryStore implements SentryStoreLayer {
               privilegeOperator.getPrivilegesByAuthorizable(lComponent, lService,
                       mRoles, authorizables, pm);
 
+          final Set<MSentryGMPrivilege> privileges =
+                  new HashSet<>(mSentryGMPrivileges.size());
           for (MSentryGMPrivilege mSentryGMPrivilege : mSentryGMPrivileges) {
             /*
              * force to load all roles related this privilege
@@ -396,36 +400,28 @@ public class DelegateSentryStore implements SentryStoreLayer {
   }
 
   private Set<TSentryGroup> toTSentryGroups(Set<String> groups) {
-    Set<TSentryGroup> tSentryGroups = Sets.newHashSet();
+    if (groups.isEmpty()) {
+      return Collections.emptySet();
+    }
+    Set<TSentryGroup> tSentryGroups = new HashSet<>(groups.size());
     for (String group : groups) {
       tSentryGroups.add(new TSentryGroup(group));
     }
     return tSentryGroups;
   }
 
-  private Set<String> toTrimmedLower(Set<String> s) {
-    if (s == null) {
-      return new HashSet<String>();
-    }
-    Set<String> result = Sets.newHashSet();
-    for (String v : s) {
-      result.add(v.trim().toLowerCase());
-    }
-    return result;
-  }
-
-  private Set<String> toTrimmed(Set<String> s) {
-    if (s == null) {
-      return new HashSet<String>();
+  private static Set<String> toTrimmed(Set<String> s) {
+    if (s.isEmpty()) {
+      return Collections.emptySet();
     }
-    Set<String> result = Sets.newHashSet();
+    Set<String> result = new HashSet<>(s.size());
     for (String v : s) {
       result.add(v.trim());
     }
     return result;
   }
 
-  private String toTrimmedLower(String s) {
+  private static String toTrimmedLower(String s) {
     if (s == null) {
       return "";
     }

http://git-wip-us.apache.org/repos/asf/sentry/blob/3ada8a36/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 9b54db1..bfc6da5 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
@@ -24,6 +24,7 @@ import static org.apache.sentry.core.common.utils.SentryConstants.KV_JOINER;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
@@ -711,7 +712,7 @@ public class SentryStore {
       mPrivilege = pm.detachCopy(mPrivilege);
     }
 
-    Set<MSentryPrivilege> privilegeGraph = Sets.newHashSet();
+    Set<MSentryPrivilege> privilegeGraph = new HashSet<>();
     if (mPrivilege.getGrantOption() != null) {
       privilegeGraph.add(mPrivilege);
     } else {
@@ -814,9 +815,6 @@ public class SentryStore {
         || !isNULL(priv.getTableName())) {
       // Get all TableLevel Privs
       Set<MSentryPrivilege> childPrivs = getChildPrivileges(pm, roleNames, priv);
-      if (childPrivs == null) {
-        return;
-      }
       for (MSentryPrivilege childPriv : childPrivs) {
         // Only recurse for table level privs..
         if (!isNULL(childPriv.getDbName()) && !isNULL(childPriv.getTableName())
@@ -850,7 +848,7 @@ public class SentryStore {
       MSentryPrivilege parent) throws SentryInvalidInputException {
     // Column and URI do not have children
     if (!isNULL(parent.getColumnName()) || !isNULL(parent.getURI())) {
-      return null;
+      return Collections.emptySet();
     }
 
     Query query = pm.newQuery(MSentryPrivilege.class);
@@ -872,14 +870,13 @@ public class SentryStore {
               .addNotNull(URI);
     }
 
-    LOGGER.debug("getChildPrivileges() Query: " + paramBuilder.toString());
-
     query.setFilter(paramBuilder.toString());
     query.setResult("privilegeScope, serverName, dbName, tableName, columnName," +
         " URI, action, grantOption");
-    Set<MSentryPrivilege> privileges = new HashSet<>();
-    for (Object[] privObj :
-            (List<Object[]>)query.executeWithMap(paramBuilder.getArguments())) {
+    List<Object[]> privObjects =
+            (List<Object[]>) query.executeWithMap(paramBuilder.getArguments());
+    Set<MSentryPrivilege> privileges = new HashSet<>(privObjects.size());
+    for (Object[] privObj : privObjects) {
       String scope        = (String)privObj[0];
       String serverName   = (String)privObj[1];
       String dbName       = (String)privObj[2];
@@ -916,7 +913,6 @@ public class SentryStore {
       // if db is null, uri is not null
       paramBuilder.add(URI, tPriv.getURI(), true);
     }
-    // LOGGER.debug("getMSentryPrivileges() Query: " + paramBuilder.toString());
 
     query.setFilter(paramBuilder.toString());
     return (List<MSentryPrivilege>) query.executeWithMap(paramBuilder.getArguments());
@@ -939,8 +935,6 @@ public class SentryStore {
             .addObject(GRANT_OPTION, grantOption)
             .add(ACTION, tPriv.getAction());
 
-    LOGGER.debug("getMSentryPrivilege() Query: " +  paramBuilder.toString());
-
     Query query = pm.newQuery(MSentryPrivilege.class);
     query.setUnique(true);
     query.setFilter(paramBuilder.toString());
@@ -1240,7 +1234,7 @@ public class SentryStore {
                                                               authHierarchy)
           throws Exception {
     if (roleNames == null || roleNames.isEmpty()) {
-      return new ArrayList<>();
+      return Collections.emptyList();
     }
 
     return tm.executeTransaction(
@@ -1285,7 +1279,7 @@ public class SentryStore {
                                   "\".startsWith(:URI)", URI, authHierarchy.getUri());
             }
           }
-          LOGGER.debug("getMSentryPrivileges1() Query: " + paramBuilder.toString());
+
           query.setFilter(paramBuilder.toString());
           @SuppressWarnings("unchecked")
           List<MSentryPrivilege> result =
@@ -1327,8 +1321,8 @@ public class SentryStore {
                       .addNull(URI);
               }
             } else {
-              // if no server, then return empty resultset
-              return new ArrayList<MSentryPrivilege>();
+              // if no server, then return empty result
+              return Collections.emptyList();
             }
             FetchGroup grp = pm.getFetchGroup(MSentryPrivilege.class, "fetchRole");
             grp.addMember("roles");
@@ -1590,7 +1584,7 @@ public class SentryStore {
 
   @VisibleForTesting
   static String toAuthorizable(MSentryPrivilege privilege) {
-    List<String> authorizable = new ArrayList<String>(4);
+    List<String> authorizable = new ArrayList<>(4);
     authorizable.add(KV_JOINER.join(AuthorizableType.Server.name().toLowerCase(),
         privilege.getServerName()));
     if (isNULL(privilege.getURI())) {
@@ -1620,9 +1614,9 @@ public class SentryStore {
   }
 
   @VisibleForTesting
-  static Set<String> toTrimedLower(Set<String> s) {
-    if (null == s) {
-      return new HashSet<String>();
+  public static Set<String> toTrimedLower(Set<String> s) {
+    if (s == null || s.isEmpty()) {
+      return Collections.emptySet();
     }
     Set<String> result = Sets.newHashSet();
     for (String v : s) {
@@ -1640,7 +1634,10 @@ public class SentryStore {
    */
 
   private Set<TSentryPrivilege> convertToTSentryPrivileges(Collection<MSentryPrivilege>
mSentryPrivileges) {
-    Set<TSentryPrivilege> privileges = new HashSet<TSentryPrivilege>();
+    if (mSentryPrivileges.isEmpty()) {
+      return Collections.emptySet();
+    }
+    Set<TSentryPrivilege> privileges = new HashSet<>(mSentryPrivileges.size());
     for(MSentryPrivilege mSentryPrivilege:mSentryPrivileges) {
       privileges.add(convertToTSentryPrivilege(mSentryPrivilege));
     }
@@ -1648,7 +1645,10 @@ public class SentryStore {
   }
 
   private Set<TSentryRole> convertToTSentryRoles(Set<MSentryRole> mSentryRoles)
{
-    Set<TSentryRole> roles = new HashSet<TSentryRole>();
+    if (mSentryRoles.isEmpty()) {
+      return Collections.emptySet();
+    }
+    Set<TSentryRole> roles = new HashSet<>(mSentryRoles.size());
     for(MSentryRole mSentryRole:mSentryRoles) {
       roles.add(convertToTSentryRole(mSentryRole));
     }
@@ -1656,7 +1656,10 @@ public class SentryStore {
   }
 
   private Set<String> convertToRoleNameSet(Set<MSentryRole> mSentryRoles) {
-    Set<String> roleNameSet = Sets.newHashSet();
+    if (mSentryRoles.isEmpty()) {
+      return Collections.emptySet();
+    }
+    Set<String> roleNameSet = new HashSet<>(mSentryRoles.size());
     for (MSentryRole role : mSentryRoles) {
       roleNameSet.add(role.getRoleName());
     }
@@ -1667,8 +1670,9 @@ public class SentryStore {
     TSentryRole role = new TSentryRole();
     role.setRoleName(mSentryRole.getRoleName());
     role.setGrantorPrincipal("--");
-    Set<TSentryGroup> sentryGroups = new HashSet<TSentryGroup>();
-    for(MSentryGroup mSentryGroup:mSentryRole.getGroups()) {
+    Set<MSentryGroup> groups = mSentryRole.getGroups();
+    Set<TSentryGroup> sentryGroups = new HashSet<>(groups.size());
+    for(MSentryGroup mSentryGroup: groups) {
       TSentryGroup group = convertToTSentryGroup(mSentryGroup);
       sentryGroups.add(group);
     }
@@ -2133,17 +2137,15 @@ public class SentryStore {
       // get all privileges for group and user
       Set<MSentryRole> roles = getRolesForGroups(pm, groups);
       roles.addAll(getRolesForUsers(pm, Sets.newHashSet(grantorPrincipal)));
-      if (roles != null && !roles.isEmpty()) {
-        for (MSentryRole role : roles) {
-          Set<MSentryPrivilege> privilegeSet = role.getPrivileges();
-          if (privilegeSet != null && !privilegeSet.isEmpty()) {
-            // if role has a privilege p with grant option
-            // and mPrivilege is a child privilege of p
-            for (MSentryPrivilege p : privilegeSet) {
-              if (p.getGrantOption() && p.implies(mPrivilege)) {
-                hasGrant = true;
-                break;
-              }
+      for (MSentryRole role : roles) {
+        Set<MSentryPrivilege> privilegeSet = role.getPrivileges();
+        if (privilegeSet != null && !privilegeSet.isEmpty()) {
+          // if role has a privilege p with grant option
+          // and mPrivilege is a child privilege of p
+          for (MSentryPrivilege p : privilegeSet) {
+            if (p.getGrantOption() && p.implies(mPrivilege)) {
+              hasGrant = true;
+              break;
             }
           }
         }
@@ -2163,7 +2165,6 @@ public class SentryStore {
   }
 
   public Map<String, HashMap<String, String>> retrieveFullPrivilegeImage() throws
Exception {
-    Map<String, HashMap<String, String>> result = new HashMap<>();
     return tm.executeTransaction(
       new TransactionBlock<Map<String, HashMap<String, String>>>() {
         public Map<String, HashMap<String, String>> execute(PersistenceManager
pm)
@@ -2188,7 +2189,7 @@ public class SentryStore {
             }
             HashMap<String, String> pUpdate = retVal.get(authzObj);
             if (pUpdate == null) {
-              pUpdate = new HashMap<String, String>();
+              pUpdate = new HashMap<>();
               retVal.put(authzObj, pUpdate);
             }
             for (MSentryRole mRole : mPriv.getRoles()) {
@@ -2210,14 +2211,17 @@ public class SentryStore {
    * @return Mapping of Role -> [Groups]
    */
   public Map<String, LinkedList<String>> retrieveFullRoleImage() throws Exception
{
-    Map<String, LinkedList<String>> result = new HashMap<>();
     return tm.executeTransaction(
         new TransactionBlock<Map<String, LinkedList<String>>>() {
           public Map<String, LinkedList<String>> execute(PersistenceManager pm)
throws Exception {
-            Map<String, LinkedList<String>> retVal = new HashMap<>();
             Query query = pm.newQuery(MSentryGroup.class);
             @SuppressWarnings("unchecked")
             List<MSentryGroup> groups = (List<MSentryGroup>) query.execute();
+            if (groups.isEmpty()) {
+              return Collections.emptyMap();
+            }
+
+            Map<String, LinkedList<String>> retVal = new HashMap<>();
             for (MSentryGroup mGroup : groups) {
               for (MSentryRole role : mGroup.getRoles()) {
                 LinkedList<String> rUpdate = retVal.get(role.getRoleName());
@@ -2429,7 +2433,7 @@ public class SentryStore {
               " )";
       boolean rollback = true;
       int orphansRemoved = 0;
-      ArrayList<Object> idList = new ArrayList<Object>();
+      ArrayList<Object> idList = new ArrayList<>();
       PersistenceManager pm = pmf.getPersistenceManager();
 
       // Transaction 1: Perform a SQL query to get things that look like orphans
@@ -2515,10 +2519,11 @@ public class SentryStore {
   }
 
   private Map<String, Set<String>> getGroupRolesMap(List<MSentryRole> mSentryRoles)
{
-    Map<String, Set<String>> groupRolesMap = Maps.newHashMap();
-    if (mSentryRoles == null) {
-      return groupRolesMap;
+    if (mSentryRoles.isEmpty()) {
+      return Collections.emptyMap();
     }
+
+    Map<String, Set<String>> groupRolesMap = new HashMap<>();
     // change the List<MSentryRole> -> Map<groupName, Set<roleName>>
     for (MSentryRole mSentryRole : mSentryRoles) {
       Set<MSentryGroup> groups = mSentryRole.getGroups();
@@ -2526,7 +2531,7 @@ public class SentryStore {
         String groupName = group.getGroupName();
         Set<String> rNames = groupRolesMap.get(groupName);
         if (rNames == null) {
-          rNames = new HashSet<String>();
+          rNames = new HashSet<>();
         }
         rNames.add(mSentryRole.getRoleName());
         groupRolesMap.put(groupName, rNames);
@@ -2536,10 +2541,11 @@ public class SentryStore {
   }
 
   private Map<String, Set<String>> getUserRolesMap(List<MSentryRole> mSentryRoles)
{
-    Map<String, Set<String>> userRolesMap = Maps.newHashMap();
-    if (mSentryRoles == null) {
-      return userRolesMap;
+    if (mSentryRoles.isEmpty()) {
+      return Collections.emptyMap();
     }
+
+    Map<String, Set<String>> userRolesMap = new HashMap<>();
     // change the List<MSentryRole> -> Map<userName, Set<roleName>>
     for (MSentryRole mSentryRole : mSentryRoles) {
       Set<MSentryUser> users = mSentryRole.getUsers();
@@ -2547,7 +2553,7 @@ public class SentryStore {
         String userName = user.getUserName();
         Set<String> rNames = userRolesMap.get(userName);
         if (rNames == null) {
-          rNames = new HashSet<String>();
+          rNames = new HashSet<>();
         }
         rNames.add(mSentryRole.getRoleName());
         userRolesMap.put(userName, rNames);
@@ -2591,18 +2597,19 @@ public class SentryStore {
 
   private Map<String, Set<TSentryPrivilege>> getRolePrivilegesMap(
           List<MSentryPrivilege> mSentryPrivileges) {
-    Map<String, Set<TSentryPrivilege>> rolePrivilegesMap = Maps.newHashMap();
-    if (mSentryPrivileges == null) {
-      return rolePrivilegesMap;
+    if (mSentryPrivileges.isEmpty()) {
+      return Collections.emptyMap();
     }
+
     // change the List<MSentryPrivilege> -> Map<roleName, Set<TSentryPrivilege>>
+    Map<String, Set<TSentryPrivilege>> rolePrivilegesMap = new HashMap<>();
     for (MSentryPrivilege mSentryPrivilege : mSentryPrivileges) {
       TSentryPrivilege privilege = convertToTSentryPrivilege(mSentryPrivilege);
       for (MSentryRole mSentryRole : mSentryPrivilege.getRoles()) {
         String roleName = mSentryRole.getRoleName();
         Set<TSentryPrivilege> privileges = rolePrivilegesMap.get(roleName);
         if (privileges == null) {
-          privileges = new HashSet<TSentryPrivilege>();
+          privileges = new HashSet<>();
         }
         privileges.add(privilege);
         rolePrivilegesMap.put(roleName, privileges);
@@ -2631,11 +2638,13 @@ public class SentryStore {
    */
   private Set<String> getAllRoleNamesCore(PersistenceManager pm) {
     List<MSentryRole> mSentryRoles = getAllRoles(pm);
-    Set<String> roleNames = Sets.newHashSet();
-    if (mSentryRoles != null) {
-      for (MSentryRole mSentryRole : mSentryRoles) {
-        roleNames.add(mSentryRole.getRoleName());
-      }
+    if (mSentryRoles.isEmpty()) {
+      return Collections.emptySet();
+    }
+
+    Set<String> roleNames = new HashSet<>(mSentryRoles.size());
+    for (MSentryRole mSentryRole : mSentryRoles) {
+      roleNames.add(mSentryRole.getRoleName());
     }
     return roleNames;
   }
@@ -2649,12 +2658,14 @@ public class SentryStore {
     Query query = pm.newQuery(MSentryGroup.class);
     @SuppressWarnings("unchecked")
     List<MSentryGroup> mSentryGroups = (List<MSentryGroup>) query.execute();
-    Map<String, MSentryGroup> existGroupsMap = Maps.newHashMap();
-    if (mSentryGroups != null) {
-      // change the List<MSentryGroup> -> Map<groupName, MSentryGroup>
-      for (MSentryGroup mSentryGroup : mSentryGroups) {
-        existGroupsMap.put(mSentryGroup.getGroupName(), mSentryGroup);
-      }
+    if (mSentryGroups.isEmpty()) {
+      return Collections.emptyMap();
+    }
+
+    Map<String, MSentryGroup> existGroupsMap = new HashMap<>(mSentryGroups.size());
+    // change the List<MSentryGroup> -> Map<groupName, MSentryGroup>
+    for (MSentryGroup mSentryGroup : mSentryGroups) {
+      existGroupsMap.put(mSentryGroup.getGroupName(), mSentryGroup);
     }
     return existGroupsMap;
   }
@@ -2668,29 +2679,15 @@ public class SentryStore {
     Query query = pm.newQuery(MSentryUser.class);
     @SuppressWarnings("unchecked")
     List<MSentryUser> users = (List<MSentryUser>) query.execute();
-    Map<String, MSentryUser> existUsersMap = Maps.newHashMap();
-    if (users != null) {
-      // change the List<MSentryUser> -> Map<userName, MSentryUser>
-      for (MSentryUser user : users) {
-        existUsersMap.put(user.getUserName(), user);
-      }
+    if (users.isEmpty()) {
+      return Collections.emptyMap();
     }
-    return existUsersMap;
-  }
-
-  /**
-   * Returl list of all privileges
-   * @param pm PersistenceManager instance
-   * @return List of all privileges
-   */
-  private List<MSentryPrivilege> getPrivilegesList(PersistenceManager pm) {
-    Query query = pm.newQuery(MSentryPrivilege.class);
-    @SuppressWarnings("unchecked")
-    List<MSentryPrivilege> resultList = (List<MSentryPrivilege>) query.execute();
-    if (resultList == null) {
-      resultList = Lists.newArrayList();
+    Map<String, MSentryUser> existUsersMap = new HashMap<>(users.size());
+    // change the List<MSentryUser> -> Map<userName, MSentryUser>
+    for (MSentryUser user : users) {
+      existUsersMap.put(user.getUserName(), user);
     }
-    return resultList;
+    return existUsersMap;
   }
 
   @VisibleForTesting
@@ -2699,12 +2696,14 @@ public class SentryStore {
         new TransactionBlock<Map<String, MSentryRole>>() {
           public Map<String, MSentryRole> execute(PersistenceManager pm) throws Exception
{
             List<MSentryRole> mSentryRoles = getAllRoles(pm);
-            Map<String, MSentryRole> existRolesMap = Maps.newHashMap();
-            if (mSentryRoles != null) {
-              // change the List<MSentryRole> -> Map<roleName, Set<MSentryRole>>
-              for (MSentryRole mSentryRole : mSentryRoles) {
-                existRolesMap.put(mSentryRole.getRoleName(), mSentryRole);
-              }
+            if (mSentryRoles.isEmpty()) {
+              return Collections.emptyMap();
+            }
+            Map<String, MSentryRole> existRolesMap =
+                    new HashMap<>(mSentryRoles.size());
+            // change the List<MSentryRole> -> Map<roleName, Set<MSentryRole>>
+            for (MSentryRole mSentryRole : mSentryRoles) {
+              existRolesMap.put(mSentryRole.getRoleName(), mSentryRole);
             }
 
             return existRolesMap;
@@ -2738,7 +2737,8 @@ public class SentryStore {
       new TransactionBlock<List<MSentryPrivilege>>() {
         public List<MSentryPrivilege> execute(PersistenceManager pm)
                 throws Exception {
-          return getPrivilegesList(pm);
+          Query query = pm.newQuery(MSentryPrivilege.class);
+          return (List<MSentryPrivilege>) query.execute();
         }
       });
   }
@@ -2799,6 +2799,12 @@ public class SentryStore {
               roleNames = getAllRoleNamesCore(pm);
             }
 
+            // Empty roleNames is most likely the COllections.emptySet().
+            // We are going to modify roleNames below, so create an actual set.
+            if (roleNames.isEmpty()) {
+              roleNames = new HashSet<>();
+            }
+
             // import the mapping data for [role,privilege], the roleNames will be updated
             importRolePrivilegeMapping(pm, roleNames, mappingData.getRolePrivilegesMap());
             // import the mapping data for [role,group], the roleNames will be updated
@@ -2813,19 +2819,21 @@ public class SentryStore {
   // covert the Map[group->roles] to Map[role->groups]
   private Map<String, Set<TSentryGroup>> covertToRoleNameTGroupsMap(
       Map<String, Set<String>> groupRolesMap) {
+    if (groupRolesMap == null || groupRolesMap.isEmpty()) {
+      return Collections.emptyMap();
+    }
+
     Map<String, Set<TSentryGroup>> roleGroupsMap = Maps.newHashMap();
-    if (groupRolesMap != null) {
-      for (Map.Entry<String, Set<String>> entry : groupRolesMap.entrySet()) {
-        Set<String> roleNames = entry.getValue();
-        if (roleNames != null) {
-          for (String roleName : roleNames) {
-            Set<TSentryGroup> tSentryGroups = roleGroupsMap.get(roleName);
-            if (tSentryGroups == null) {
-              tSentryGroups = Sets.newHashSet();
-            }
-            tSentryGroups.add(new TSentryGroup(entry.getKey()));
-            roleGroupsMap.put(roleName, tSentryGroups);
+    for (Map.Entry<String, Set<String>> entry : groupRolesMap.entrySet()) {
+      Set<String> roleNames = entry.getValue();
+      if (roleNames != null) {
+        for (String roleName : roleNames) {
+          Set<TSentryGroup> tSentryGroups = roleGroupsMap.get(roleName);
+          if (tSentryGroups == null) {
+            tSentryGroups = Sets.newHashSet();
           }
+          tSentryGroups.add(new TSentryGroup(entry.getKey()));
+          roleGroupsMap.put(roleName, tSentryGroups);
         }
       }
     }
@@ -2835,19 +2843,21 @@ public class SentryStore {
   // covert the Map[user->roles] to Map[role->users]
   private Map<String, Set<String>> covertToRoleUsersMap(
       Map<String, Set<String>> userRolesMap) {
-    Map<String, Set<String>> roleUsersMap = Maps.newHashMap();
-    if (userRolesMap != null) {
-      for (Map.Entry<String, Set<String>> entry : userRolesMap.entrySet()) {
-        Set<String> roleNames = entry.getValue();
-        if (roleNames != null) {
-          for (String roleName : roleNames) {
-            Set<String> users = roleUsersMap.get(roleName);
-            if (users == null) {
-              users = Sets.newHashSet();
-            }
-            users.add(entry.getKey());
-            roleUsersMap.put(roleName, users);
+    if (userRolesMap == null || userRolesMap.isEmpty()) {
+      return Collections.emptyMap();
+    }
+
+    Map<String, Set<String>> roleUsersMap = new HashMap<>();
+    for (Map.Entry<String, Set<String>> entry : userRolesMap.entrySet()) {
+      Set<String> roleNames = entry.getValue();
+      if (roleNames != null) {
+        for (String roleName : roleNames) {
+          Set<String> users = roleUsersMap.get(roleName);
+          if (users == null) {
+            users = new HashSet<>();
           }
+          users.add(entry.getKey());
+          roleUsersMap.put(roleName, users);
         }
       }
     }
@@ -2891,8 +2901,8 @@ public class SentryStore {
     Map<String, Set<TSentryPrivilege>> sentryRolePrivilegesMap = tSentryMappingData
         .getRolePrivilegesMap();
 
-    Map<String, Set<String>> newSentryGroupRolesMap = Maps.newHashMap();
-    Map<String, Set<TSentryPrivilege>> newSentryRolePrivilegesMap = Maps.newHashMap();
+    Map<String, Set<String>> newSentryGroupRolesMap = new HashMap<>();
+    Map<String, Set<TSentryPrivilege>> newSentryRolePrivilegesMap = new HashMap<>();
     // for mapping data [group,role]
     for (Map.Entry<String, Set<String>> entry : sentryGroupRolesMap.entrySet())
{
       Collection<String> lowcaseRoles = Collections2.transform(entry.getValue(),
@@ -2902,7 +2912,7 @@ public class SentryStore {
               return input.toLowerCase();
             }
           });
-      newSentryGroupRolesMap.put(entry.getKey(), Sets.newHashSet(lowcaseRoles));
+      newSentryGroupRolesMap.put(entry.getKey(), new HashSet<>(lowcaseRoles));
     }
 
     // for mapping data [role,privilege]


Mime
View raw message