Polish RoleHierarchyUtils and add tests

core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2012-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,84 +15,55 @@
  */
 package org.springframework.security.access.hierarchicalroles;
 
+import org.springframework.util.Assert;
+
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.List;
 import java.util.Map;
 
 /**
- * Utility method for working with {@link RoleHierarchy}.
+ * Utility methods for {@link RoleHierarchy}.
  *
  * @author Thomas Darimont
- * @since
+ * @since 4.2.0
  */
-public class RoleHierarchyUtils {
+public final class RoleHierarchyUtils {
+
+	private RoleHierarchyUtils() {
+	}
 
 	/**
-	 * Builds a {@link RoleHierarchy} representation from the given {@link Map} of role name to implied roles.
+	 * Converts the supplied {@link Map} of role name to implied role name(s) to a string
-	 * The map key is the role name and the map value is a {@link List} of implied role names.
+	 * representation understood by {@link RoleHierarchyImpl#setHierarchy(String)}.
+	 * The map key is the role name and the map value is a {@link List} of implied role name(s).
 	 *
-	 * <p>
+	 * @param roleHierarchyMap the mapping(s) of role name to implied role name(s)
-	 *     Here is an example configuration of a role hierarchy configured via yaml.
+	 * @return a string representation of a role hierarchy
-	 *     wich follows the pattern:
+	 * @throws IllegalArgumentException if roleHierarchyMap is null or empty or if a role name is null or
-	 *     {@code ROLE_NAME: List of implied role names}
+	 * empty or if an implied role name(s) is null or empty
-	 * </p>
-	 * <pre>
-	 * <code>
 	 *
-	 * security:
-	 *   roles:
-	 *     hierarchy:
-	 *       ROLE_ALL: ROLE_A, ROLE_C
-	 *       ROLE_A: ROLE_B
-	 *       ROLE_B: ROLE_AUTHENTICATED
-	 *       ROLE_C: ROLE_AUTHENTICATED
-	 *       ROLE_AUTHENTICATED: ROLE_UNAUTHENTICATED
-	 * </code>
-	 * </pre>
-	 * <p>This yaml configuration could then be mapped by the following {@literal ConfigurationProperties}</p>
-	 * <pre>
-	 * <code>
-	 *   {@literal @}ConfigurationProperties("security.roles")
-	 *   class SecurityPropertiesExtension {
-	 *     Map<String, List<String>> hierarchy = new LinkedHashMap<>();
-	 *
-	 *     //getter | setter
-	 *   }
-	 * </code>
-	 * </pre>
-	 * <p>To define the role hierarchy just declare a {@link org.springframework.context.annotation.Bean} of
-	 * type {@link RoleHierarchy} as follows:</p>
-	 * <pre>
-	 * <code>
-	 *   {@literal @}Bean
-	 *   RoleHierarchy roleHierarchy(SecurityPropertiesExtension spe) {
-	 *     return RoleHierarchyUtils.roleHierarchyFromMap(spe.getHierarchy());
-	 *   }
-	 * </code>
-	 * </pre>
-	 *
-	 * @param roleHierarchyMapping the role name to implied role names mapping
-	 * @return
 	 */
-	public static RoleHierarchy roleHierarchyFromMap(Map<String, List<String>> roleHierarchyMapping) {
+	public static String roleHierarchyFromMap(Map<String, List<String>> roleHierarchyMap) {
+		Assert.notEmpty(roleHierarchyMap, "roleHierarchyMap cannot be empty");
 
-		StringWriter roleHierachyDescriptionBuffer = new StringWriter();
+		StringWriter roleHierarchyBuffer = new StringWriter();
-		PrintWriter roleHierarchyDescriptionWriter = new PrintWriter(roleHierachyDescriptionBuffer);
+		PrintWriter roleHierarchyWriter = new PrintWriter(roleHierarchyBuffer);
 
-		for (Map.Entry<String, List<String>> entry : roleHierarchyMapping.entrySet()) {
+		for (Map.Entry<String, List<String>> roleHierarchyEntry : roleHierarchyMap.entrySet()) {
+			String role = roleHierarchyEntry.getKey();
+			List<String> impliedRoles = roleHierarchyEntry.getValue();
 
-			String currentRole = entry.getKey();
+			Assert.hasLength(role, "role name must be supplied");
-			List<String> impliedRoles = entry.getValue();
+			Assert.notEmpty(impliedRoles, "implied role name(s) cannot be empty");
 
 			for (String impliedRole : impliedRoles) {
-				String roleMapping = currentRole + " > " + impliedRole;
+				String roleMapping = role + " > " + impliedRole;
-				roleHierarchyDescriptionWriter.println(roleMapping);
+				roleHierarchyWriter.println(roleMapping);
 			}
 		}
 
-		RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl();
+		return roleHierarchyBuffer.toString();
-		roleHierarchy.setHierarchy(roleHierachyDescriptionBuffer.toString());
-		return roleHierarchy;
 	}
+
 }

core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2012-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,251 +16,72 @@
 package org.springframework.security.access.hierarchicalroles;
 
 import org.junit.Test;
-import org.springframework.security.core.GrantedAuthority;
-import org.springframework.security.core.authority.AuthorityUtils;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.HashMap;
-import java.util.List;
 
 import static java.util.Arrays.asList;
-import static java.util.Collections.singletonList;
-import static java.util.Collections.singletonMap;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.fail;
-import static org.springframework.security.access.hierarchicalroles.RoleHierarchyUtils.roleHierarchyFromMap;
 
 /**
  * Tests for {@link RoleHierarchyUtils}.
  *
- * Copied from {@link RoleHierarchyImplTests} with adaptations for {@link RoleHierarchyUtils}.
+ * @author Joe Grandja
- *
- * @author Thomas Darimont
  */
 public class RoleHierarchyUtilsTests {
 
 	@Test
-	public void testRoleHierarchyWithNullOrEmptyAuthorities() {
+	public void roleHierarchyFromMapWhenMapValidThenConvertsCorrectly() throws Exception {
+		String expectedRoleHierarchy = "ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_B > ROLE_D\nROLE_C > ROLE_D\n";
 
-		List<GrantedAuthority> authorities0 = null;
+		Map<String, List<String>> roleHierarchyMap = new TreeMap<String, List<String>>();
-		List<GrantedAuthority> authorities1 = new ArrayList<GrantedAuthority>();
+		roleHierarchyMap.put("ROLE_A", asList("ROLE_B", "ROLE_C"));
+		roleHierarchyMap.put("ROLE_B", asList("ROLE_D"));
+		roleHierarchyMap.put("ROLE_C", asList("ROLE_D"));
 
-		RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B")));
+		String roleHierarchy = RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap);
 
-		assertThat(roleHierarchy.getReachableGrantedAuthorities(
+		assertThat(roleHierarchy).isEqualTo(expectedRoleHierarchy);
-				authorities0)).isNotNull();
-		assertThat(
-				roleHierarchy.getReachableGrantedAuthorities(authorities0)).isEmpty();
-		;
-		assertThat(roleHierarchy.getReachableGrantedAuthorities(
-				authorities1)).isNotNull();
-		assertThat(
-				roleHierarchy.getReachableGrantedAuthorities(authorities1)).isEmpty();
-		;
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testSimpleRoleHierarchy() {
+	public void roleHierarchyFromMapWhenMapNullThenThrowsIllegalArgumentException() throws Exception {
-
+		RoleHierarchyUtils.roleHierarchyFromMap(null);
-		List<GrantedAuthority> authorities0 = AuthorityUtils.createAuthorityList(
-				"ROLE_0");
-		List<GrantedAuthority> authorities1 = AuthorityUtils.createAuthorityList(
-				"ROLE_A");
-		List<GrantedAuthority> authorities2 = AuthorityUtils.createAuthorityList("ROLE_A",
-				"ROLE_B");
-
-		RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B")));
-
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy.getReachableGrantedAuthorities(authorities0),
-				authorities0)).isTrue();
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy.getReachableGrantedAuthorities(authorities1),
-				authorities2)).isTrue();
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy.getReachableGrantedAuthorities(authorities2),
-				authorities2)).isTrue();
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testTransitiveRoleHierarchies() {
+	public void roleHierarchyFromMapWhenMapEmptyThenThrowsIllegalArgumentException() throws Exception {
-		List<GrantedAuthority> authorities1 = AuthorityUtils.createAuthorityList(
+		RoleHierarchyUtils.roleHierarchyFromMap(Collections.<String, List<String>>emptyMap());
-				"ROLE_A");
-		List<GrantedAuthority> authorities2 = AuthorityUtils.createAuthorityList("ROLE_A",
-				"ROLE_B", "ROLE_C");
-		List<GrantedAuthority> authorities3 = AuthorityUtils.createAuthorityList("ROLE_A",
-				"ROLE_B", "ROLE_C", "ROLE_D");
-
-		RoleHierarchy roleHierarchy2Levels = roleHierarchyFromMap(new HashMap<String, List<String>>(){
-			{
-				put("ROLE_A", asList("ROLE_B"));
-				put("ROLE_B", asList("ROLE_C"));
-			}
-		});
-
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy2Levels.getReachableGrantedAuthorities(authorities1),
-				authorities2)).isTrue();
-
-		RoleHierarchy roleHierarchy3Levels = roleHierarchyFromMap(new HashMap<String, List<String>>(){
-			{
-				put("ROLE_A", asList("ROLE_B"));
-				put("ROLE_B", asList("ROLE_C"));
-				put("ROLE_C", asList("ROLE_D"));
-			}
-		});
-
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy3Levels.getReachableGrantedAuthorities(authorities1),
-				authorities3)).isTrue();
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testComplexRoleHierarchy() {
+	public void roleHierarchyFromMapWhenRoleNullThenThrowsIllegalArgumentException() throws Exception {
-		List<GrantedAuthority> authoritiesInput1 = AuthorityUtils.createAuthorityList(
+		Map<String, List<String>> roleHierarchyMap = new HashMap<String, List<String>>();
-				"ROLE_A");
+		roleHierarchyMap.put(null, asList("ROLE_B", "ROLE_C"));
-		List<GrantedAuthority> authoritiesOutput1 = AuthorityUtils.createAuthorityList(
-				"ROLE_A", "ROLE_B", "ROLE_C", "ROLE_D");
-		List<GrantedAuthority> authoritiesInput2 = AuthorityUtils.createAuthorityList(
-				"ROLE_B");
-		List<GrantedAuthority> authoritiesOutput2 = AuthorityUtils.createAuthorityList(
-				"ROLE_B", "ROLE_D");
-		List<GrantedAuthority> authoritiesInput3 = AuthorityUtils.createAuthorityList(
-				"ROLE_C");
-		List<GrantedAuthority> authoritiesOutput3 = AuthorityUtils.createAuthorityList(
-				"ROLE_C", "ROLE_D");
-		List<GrantedAuthority> authoritiesInput4 = AuthorityUtils.createAuthorityList(
-				"ROLE_D");
-		List<GrantedAuthority> authoritiesOutput4 = AuthorityUtils.createAuthorityList(
-				"ROLE_D");
 
-
+		RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap);
-		RoleHierarchy roleHierarchy3LevelsMultipleRoles = roleHierarchyFromMap(new HashMap<String, List<String>>(){
-			{
-				put("ROLE_A", asList("ROLE_B","ROLE_C"));
-				put("ROLE_B", asList("ROLE_D"));
-				put("ROLE_C", asList("ROLE_D"));
-			}
-		});
-
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput1),
-				authoritiesOutput1)).isTrue();
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput2),
-				authoritiesOutput2)).isTrue();
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput3),
-				authoritiesOutput3)).isTrue();
-		assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(
-			roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput4),
-				authoritiesOutput4)).isTrue();
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testCyclesInRoleHierarchy() {
+	public void roleHierarchyFromMapWhenRoleEmptyThenThrowsIllegalArgumentException() throws Exception {
+		Map<String, List<String>> roleHierarchyMap = new HashMap<String, List<String>>();
+		roleHierarchyMap.put("", asList("ROLE_B", "ROLE_C"));
 
-		try {
+		RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap);
-			roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_A")));
-			fail("Cycle in role hierarchy was not detected!");
-		}
-		catch (CycleInRoleHierarchyException e) {
-		}
-
-		try {
-
-			roleHierarchyFromMap(new HashMap<String, List<String>>(){
-				{
-					put("ROLE_A", asList("ROLE_B"));
-					put("ROLE_B", asList("ROLE_A"));
-				}
-			});
-
-			fail("Cycle in role hierarchy was not detected!");
-		}
-		catch (CycleInRoleHierarchyException e) {
-		}
-
-		try {
-
-			roleHierarchyFromMap(new HashMap<String, List<String>>(){
-				{
-					put("ROLE_A", asList("ROLE_B"));
-					put("ROLE_B", asList("ROLE_C"));
-					put("ROLE_C", asList("ROLE_A"));
-				}
-			});
-
-			fail("Cycle in role hierarchy was not detected!");
-		}
-		catch (CycleInRoleHierarchyException e) {
-		}
-
-		try {
-
-			roleHierarchyFromMap(new HashMap<String, List<String>>(){
-				{
-					put("ROLE_A", asList("ROLE_B"));
-					put("ROLE_B", asList("ROLE_C"));
-					put("ROLE_C", asList("ROLE_E"));
-					put("ROLE_E", asList("ROLE_D"));
-					put("ROLE_D", asList("ROLE_B"));
-				}
-			});
-
-
-			fail("Cycle in role hierarchy was not detected!");
-		}
-		catch (CycleInRoleHierarchyException e) {
-		}
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testNoCyclesInRoleHierarchy() {
+	public void roleHierarchyFromMapWhenImpliedRolesNullThenThrowsIllegalArgumentException() throws Exception {
-		RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl();
+		Map<String, List<String>> roleHierarchyMap = new HashMap<String, List<String>>();
+		roleHierarchyMap.put("ROLE_A", null);
 
-		try {
+		RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap);
-			roleHierarchyImpl.setHierarchy(
-					"ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_C > ROLE_D\nROLE_B > ROLE_D");
-
-			roleHierarchyFromMap(new HashMap<String, List<String>>(){
-				{
-					put("ROLE_A", asList("ROLE_B"));
-					put("ROLE_A", asList("ROLE_C"));
-					put("ROLE_C", asList("ROLE_D"));
-					put("ROLE_B", asList("ROLE_D"));
-				}
-			});
-
-		}
-		catch (CycleInRoleHierarchyException e) {
-			fail("A cycle in role hierarchy was incorrectly detected!");
-		}
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
-	public void testSimpleRoleHierarchyWithCustomGrantedAuthorityImplementation() {
+	public void roleHierarchyFromMapWhenImpliedRolesEmptyThenThrowsIllegalArgumentException() throws Exception {
+		Map<String, List<String>> roleHierarchyMap = new HashMap<String, List<String>>();
+		roleHierarchyMap.put("ROLE_A", Collections.<String>emptyList());
 
-		List<GrantedAuthority> authorities0 = HierarchicalRolesTestHelper.createAuthorityList(
+		RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap);
-				"ROLE_0");
-		List<GrantedAuthority> authorities1 = HierarchicalRolesTestHelper.createAuthorityList(
-				"ROLE_A");
-		List<GrantedAuthority> authorities2 = HierarchicalRolesTestHelper.createAuthorityList(
-				"ROLE_A", "ROLE_B");
-
-		RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B")));
-
-		assertThat(
-				HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString(
-					roleHierarchy.getReachableGrantedAuthorities(authorities0),
-						authorities0)).isTrue();
-		assertThat(
-				HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString(
-					roleHierarchy.getReachableGrantedAuthorities(authorities1),
-						authorities2)).isTrue();
-		assertThat(
-				HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString(
-					roleHierarchy.getReachableGrantedAuthorities(authorities2),
-						authorities2)).isTrue();
 	}
 }