Polish RoleHierarchyImpl#Builder

- Added documentation
- Removed withNoRolePrefix for now; let's see how folks
use the minimal API first
- Adjusted class hierarchy to match AuthorizeHttpRequests more
closely
- Adjusted to match Spring Security style guide
- Added needed @since attributes

Issue gh-13300

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

@@ -24,7 +24,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Stream;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -35,8 +34,6 @@ import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.util.Assert;
 
-import static org.springframework.security.access.hierarchicalroles.RoleHierarchyUtils.roleHierarchyFromMap;
-
 /**
  * <p>
  * This class defines a role hierarchy for use with various access checking components.
@@ -109,6 +106,7 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 	 * Factory method that creates a {@link Builder} instance with the default role prefix
 	 * "ROLE_"
 	 * @return a {@link Builder} instance with the default role prefix "ROLE_"
+	 * @since 6.3
 	 */
 	public static Builder withDefaultRolePrefix() {
 		return withRolePrefix("ROLE_");
@@ -120,20 +118,13 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 	 * @param rolePrefix the prefix to be used for the roles in the hierarchy.
 	 * @return a new {@link Builder} instance with the specified role prefix
 	 * @throws IllegalArgumentException if the provided role prefix is null
+	 * @since 6.3
 	 */
 	public static Builder withRolePrefix(String rolePrefix) {
 		Assert.notNull(rolePrefix, "rolePrefix must not be null");
 		return new Builder(rolePrefix);
 	}
 
-	/**
-	 * Factory method that creates a {@link Builder} instance with no role prefix.
-	 * @return a new {@link Builder} instance with no role prefix.
-	 */
-	public static Builder withNoRolePrefix() {
-		return withRolePrefix("");
-	}
-
 	/**
 	 * Set the role hierarchy and pre-calculate for every role the set of all reachable
 	 * roles, i.e. all roles lower in the hierarchy of every given role. Pre-calculation
@@ -259,22 +250,22 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 
 		private final String rolePrefix;
 
-		private final Map<String, List<String>> roleBranches;
+		private final Map<String, List<String>> hierarchy;
 
 		private Builder(String rolePrefix) {
 			this.rolePrefix = rolePrefix;
-			this.roleBranches = new LinkedHashMap<>();
+			this.hierarchy = new LinkedHashMap<>();
 		}
 
 		/**
 		 * Creates a new hierarchy branch to define a role and its child roles.
 		 * @param role the highest role in this branch
-		 * @return a {@link RoleBranchBuilder} to define the child roles for the
+		 * @return a {@link ImpliedRoles} to define the child roles for the
 		 * <code>role</code>
 		 */
-		public RoleBranchBuilder role(String role) {
+		public ImpliedRoles role(String role) {
 			Assert.hasText(role, "role must not be empty");
-			return new RoleBranchBuilder(this, rolePrefix.concat(role));
+			return new ImpliedRoles(role);
 		}
 
 		/**
@@ -283,23 +274,29 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 		 * @return a {@link RoleHierarchyImpl}
 		 */
 		public RoleHierarchyImpl build() {
-			String roleHierarchyRepresentation = roleHierarchyFromMap(roleBranches);
+			String roleHierarchyRepresentation = RoleHierarchyUtils.roleHierarchyFromMap(this.hierarchy);
 			RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl();
 			roleHierarchy.setHierarchy(roleHierarchyRepresentation);
 			return roleHierarchy;
 		}
 
+		private Builder addHierarchy(String role, String... impliedRoles) {
+			List<String> withPrefix = new ArrayList<>();
+			for (String impliedRole : impliedRoles) {
+				withPrefix.add(this.rolePrefix.concat(impliedRole));
+			}
+			this.hierarchy.put(this.rolePrefix.concat(role), withPrefix);
+			return this;
+		}
+
 		/**
 		 * Builder class for constructing child roles within a role hierarchy branch.
 		 */
-		public static final class RoleBranchBuilder {
+		public final class ImpliedRoles {
-
-			private final Builder parentBuilder;
 
 			private final String role;
 
-			private RoleBranchBuilder(Builder parentBuilder, String role) {
+			private ImpliedRoles(String role) {
-				this.parentBuilder = parentBuilder;
 				this.role = role;
 			}
 
@@ -313,9 +310,7 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 			public Builder implies(String... impliedRoles) {
 				Assert.notEmpty(impliedRoles, "at least one implied role must be provided");
 				Assert.noNullElements(impliedRoles, "implied role name(s) cannot be empty");
-				parentBuilder.roleBranches.put(role,
+				return Builder.this.addHierarchy(this.role, impliedRoles);
-						Stream.of(impliedRoles).map(parentBuilder.rolePrefix::concat).toList());
-				return parentBuilder;
 			}
 
 		}

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

@@ -208,9 +208,15 @@ public class RoleHierarchyImplTests {
 
 	@Test
 	public void testBuilderWithDefaultRolePrefix() {
-		RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withDefaultRolePrefix().role("A").implies("B").build();
+		RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withDefaultRolePrefix()
+			.role("A")
+			.implies("B")
+			.role("B")
+			.implies("C", "D")
+			.build();
 		List<GrantedAuthority> flatAuthorities = AuthorityUtils.createAuthorityList("ROLE_A");
-		List<GrantedAuthority> allAuthorities = AuthorityUtils.createAuthorityList("ROLE_A", "ROLE_B");
+		List<GrantedAuthority> allAuthorities = AuthorityUtils.createAuthorityList("ROLE_A", "ROLE_B", "ROLE_C",
+				"ROLE_D");
 
 		assertThat(roleHierarchyImpl).isNotNull();
 		assertThat(roleHierarchyImpl.getReachableGrantedAuthorities(flatAuthorities))
@@ -232,17 +238,6 @@ public class RoleHierarchyImplTests {
 			.containsExactlyInAnyOrderElementsOf(allAuthorities);
 	}
 
-	@Test
-	public void testBuilderWithNoRolePrefix() {
-		RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withNoRolePrefix().role("A").implies("B").build();
-		List<GrantedAuthority> flatAuthorities = AuthorityUtils.createAuthorityList("A");
-		List<GrantedAuthority> allAuthorities = AuthorityUtils.createAuthorityList("A", "B");
-
-		assertThat(roleHierarchyImpl).isNotNull();
-		assertThat(roleHierarchyImpl.getReachableGrantedAuthorities(flatAuthorities))
-			.containsExactlyInAnyOrderElementsOf(allAuthorities);
-	}
-
 	@Test
 	public void testBuilderThrowIllegalArgumentExceptionWhenPrefixRoleNull() {
 		assertThatIllegalArgumentException().isThrownBy(() -> RoleHierarchyImpl.withRolePrefix(null));

docs/modules/ROOT/pages/servlet/authorization/architecture.adoc

@@ -253,11 +253,10 @@ Java::
 ----
 @Bean
 static RoleHierarchy roleHierarchy() {
-    RoleHierarchyImpl hierarchy = new RoleHierarchyImpl();
+    return RoleHierarchyImpl.withDefaultRolePrefix()
-    hierarchy.setHierarchy("ROLE_ADMIN > ROLE_STAFF\n" +
+        .role("ADMIN").implies("STAFF")
-            "ROLE_STAFF > ROLE_USER\n" +
+        .role("STAFF").implies("USER")
-            "ROLE_USER > ROLE_GUEST");
+        .role("USER").implies("GUEST");
-    return hierarchy;
 }
 
 // and, if using method security also add