[ https://issues.apache.org/jira/browse/HADOOP-15836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705357#comment-16705357 ]
BELUGA BEHR edited comment on HADOOP-15836 at 11/30/18 10:25 PM: ----------------------------------------------------------------- OK, so I am providing a new patch with lessons learned from the failed unit tests and from HADOOP-12640. The two unit tests for MAPREDUCE-7155 and YARN-8928 pass locally. I have made one functional change from how things are now: {code:java} // The space in the group list is acceptable. // However, there is no test (and therefore ambiguity) // about what to do if the user list has a space in it. // There is only a test for a space in the group list Iterator<String> iter; acl = new AccessControlList("drwho,joe tardis, users"); users = acl.getUsers(); assertEquals(users.size(), 2); iter = users.iterator(); assertEquals(iter.next(), "drwho"); assertEquals(iter.next(), "joe"); groups = acl.getGroups(); assertEquals(groups.size(), 2); iter = groups.iterator(); assertEquals(iter.next(), "tardis"); assertEquals(iter.next(), "users"); {code} I have made both situations fail as an invalid format. To avoid any ambiguity, the format should be strict: {{u1,u2<white-space>g1,g2}} {code:java|title=Patch} @Test(expected = IllegalArgumentException.class) public void testSpaceInGroupString() { // Proper format is: u1,u2 g1,g2 new AccessControlList("drwho,joe tardis, group2"); } @Test(expected = IllegalArgumentException.class) public void testSpaceInUSerString() { // Proper format is: u1,u2 g1,g2 new AccessControlList("drwho, joe tardis,group2"); } {code} was (Author: belugabehr): OK, so I am providing a new patch with lessons learned from the failed unit tests and from HADOOP-12640. The two unit tests for MAPREDUCE-7155 and YARN-8928 pass locally. I have made one functional change from how things are now: {code:java} // The space in the group list is acceptable. // However, there is no test (and therefore ambiguity) // about what to do if the user list has a space in it. // There is only a test for a space in the group list Iterator<String> iter; acl = new AccessControlList("drwho,joe tardis, users"); users = acl.getUsers(); assertEquals(users.size(), 2); iter = users.iterator(); assertEquals(iter.next(), "drwho"); assertEquals(iter.next(), "joe"); groups = acl.getGroups(); assertEquals(groups.size(), 2); iter = groups.iterator(); assertEquals(iter.next(), "tardis"); assertEquals(iter.next(), "users"); {code} I have made both situations fail as an invalid format. To avoid any ambiguity, the format should be strict: {{u1,u2 g1,g2}} {code:java|title=Patch} @Test(expected = IllegalArgumentException.class) public void testSpaceInGroupString() { // Proper format is: u1,u2 g1,g2 new AccessControlList("drwho,joe tardis, group2"); } @Test(expected = IllegalArgumentException.class) public void testSpaceInUSerString() { // Proper format is: u1,u2 g1,g2 new AccessControlList("drwho, joe tardis,group2"); } {code} > Review of AccessControlList > --------------------------- > > Key: HADOOP-15836 > URL: https://issues.apache.org/jira/browse/HADOOP-15836 > Project: Hadoop Common > Issue Type: Improvement > Components: common, security > Affects Versions: 3.2.0 > Reporter: BELUGA BEHR > Assignee: BELUGA BEHR > Priority: Minor > Fix For: 3.3.0 > > Attachments: HADOOP-15836.1.patch, HADOOP-15836.2.patch, > assertEqualACLStrings.patch > > > * Improve unit tests (expected / actual were backwards) > * Unit test expected elements to be in order but the class's return > Collections were unordered > * Formatting cleanup > * Removed superfluous white space > * Remove use of LinkedList > * Removed superfluous code > * Use {{unmodifiable}} Collections where JavaDoc states that caller must not > manipulate the data structure -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org