diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index cb49bc5343f..d7d0f569401 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -445,6 +445,9 @@ Release 2.5.0 - UNRELEASED HADOOP-10279. Create multiplexer, a requirement for the fair queue. (Chris Li via Arpit Agarwal) + HADOOP-10659. Refactor AccessControlList to reuse utility functions + and to improve performance. (Benoy Antony via Arpit Agarwal) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java index e23612ec0f6..9d328c00ed6 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java @@ -20,22 +20,21 @@ package org.apache.hadoop.security.authorize; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; -import java.util.Set; -import java.util.TreeSet; -import java.util.Arrays; -import java.util.List; +import java.util.Collection; +import java.util.HashSet; import java.util.LinkedList; -import java.util.ListIterator; +import java.util.List; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.io.Text; import org.apache.hadoop.io.Writable; import org.apache.hadoop.io.WritableFactories; import org.apache.hadoop.io.WritableFactory; -import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.Groups; -import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.util.StringUtils; /** * Class representing a configured access control list. @@ -58,9 +57,9 @@ public class AccessControlList implements Writable { private static final int INITIAL_CAPACITY = 256; // Set of users who are granted access. - private Set users; + private Collection users; // Set of groups which are granted access - private Set groups; + private Collection groups; // Whether all users are granted access. private boolean allAllowed; @@ -92,27 +91,21 @@ public class AccessControlList implements Writable { * @param aclString build ACL from this string */ private void buildACL(String aclString) { - users = new TreeSet(); - groups = new TreeSet(); + users = new HashSet(); + groups = new HashSet(); if (isWildCardACLValue(aclString)) { allAllowed = true; } else { allAllowed = false; String[] userGroupStrings = aclString.split(" ", 2); - + if (userGroupStrings.length >= 1) { - List usersList = new LinkedList( - Arrays.asList(userGroupStrings[0].split(","))); - cleanupList(usersList); - addToSet(users, usersList); - } + users = StringUtils.getTrimmedStringCollection(userGroupStrings[0]); + } if (userGroupStrings.length == 2) { - List groupsList = new LinkedList( - Arrays.asList(userGroupStrings[1].split(","))); - cleanupList(groupsList); - addToSet(groups, groupsList); - groupsMapping.cacheGroupsAdd(groupsList); + groups = StringUtils.getTrimmedStringCollection(userGroupStrings[1]); + groupsMapping.cacheGroupsAdd(new LinkedList(groups)); } } } @@ -203,7 +196,7 @@ public class AccessControlList implements Writable { * Get the names of users allowed for this service. * @return the set of user names. the set must not be modified. */ - Set getUsers() { + Collection getUsers() { return users; } @@ -211,7 +204,7 @@ public class AccessControlList implements Writable { * Get the names of user groups allowed for this service. * @return the set of group names. the set must not be modified. */ - Set getGroups() { + Collection getGroups() { return groups; } @@ -228,36 +221,6 @@ public class AccessControlList implements Writable { return false; } - /** - * Cleanup list, remove empty strings, trim leading/trailing spaces - * - * @param list clean this list - */ - private static final void cleanupList(List list) { - ListIterator i = list.listIterator(); - while(i.hasNext()) { - String s = i.next(); - if(s.length() == 0) { - i.remove(); - } else { - s = s.trim(); - i.set(s); - } - } - } - - /** - * Add list to a set - * - * @param set add list to this set - * @param list add items of this list to the set - */ - private static final void addToSet(Set set, List list) { - for(String s : list) { - set.add(s); - } - } - /** * Returns descriptive way of users and groups that are part of this ACL. * Use {@link #getAclString()} to get the exact String that can be given to @@ -358,7 +321,7 @@ public class AccessControlList implements Writable { * * @param strings set of strings to concatenate */ - private String getString(Set strings) { + private String getString(Collection strings) { StringBuilder sb = new StringBuilder(INITIAL_CAPACITY); boolean first = true; for(String str: strings) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java index 32f1fa1501f..ca7bec4164a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java @@ -17,27 +17,25 @@ */ package org.apache.hadoop.security.authorize; -import java.util.Iterator; -import java.util.Set; -import java.util.List; - -import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import org.apache.hadoop.classification.InterfaceAudience; -import org.apache.hadoop.classification.InterfaceStability; + +import java.util.Collection; +import java.util.Iterator; +import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - -import org.apache.hadoop.fs.CommonConfigurationKeysPublic; -import org.apache.hadoop.util.NativeCodeLoader; +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.security.Groups; import org.apache.hadoop.security.UserGroupInformation; -import org.apache.hadoop.security.authorize.AccessControlList; +import org.apache.hadoop.util.NativeCodeLoader; +import org.junit.Test; @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Evolving @@ -221,8 +219,8 @@ public class TestAccessControlList { @Test public void testAccessControlList() throws Exception { AccessControlList acl; - Set users; - Set groups; + Collection users; + Collection groups; acl = new AccessControlList("drwho tardis"); users = acl.getUsers(); @@ -273,8 +271,8 @@ public class TestAccessControlList { @Test public void testAddRemoveAPI() { AccessControlList acl; - Set users; - Set groups; + Collection users; + Collection groups; acl = new AccessControlList(" "); assertEquals(0, acl.getUsers().size()); assertEquals(0, acl.getGroups().size());