Add convenience method for constructing RoleHierarchy from Map.

[thomasdarimont]

Introduced RoleHierarchyUtils which enables convenient
construction of RoleHierarchy from map based representation.
Where the map key is the role name and the map value is a list
of implied role names.

Here is a small example for that in action:
https://gist.github.com/thomasdarimont/ee9fffdef1adb9243b12ad247478aad4

Fixes #3990.

Signed-off-by: Thomas Darimont thomas.darimont@gmail.com

[pivotal-issuemaster]

@thomasdarimont Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@thomasdarimont Thank you for signing the Contributor License Agreement!

[jgrandja]

Thanks for the PR @thomasdarimont. This would be a valuable feature to add.

I reviewed the PR and as you have noted, the tests in RoleHierarchyUtilsTests have been copied from RoleHierarchyImplTests and modified slightly. There is a lot of duplication of code here that I would prefer to avoid.

Given the implementation of RoleHierarchyUtils.roleHierarchyFromMap, it simply converts the Map representation to a String and then passes it to the existing RoleHierarchyImpl.setHierarchy implementation leveraging all the existing code of building the RoleHierarchyImpl.

I like the fact that it's leveraging the existing RoleHierarchyImpl.setHierarchy.

The added changes here is the conversion from a Map to a String. So what I would like to propose is the following method signature for RoleHierarchyUtils:

public static String toString(Map<String, List<String>> roleHierarchyMap)

This will allow for easier testing as well. For example:

String expectedRoleHierarchy = "ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_B > ROLE_D\nROLE_C > ROLE_D\n";

Map<String, List<String>> roleHierarchyMap = new TreeMap<String, List<String>>() {
    {
        put("ROLE_A", asList("ROLE_B", "ROLE_C"));
        put("ROLE_B", asList("ROLE_D"));
        put("ROLE_C", asList("ROLE_D"));
    }
};

assertThat(RoleHierarchyUtils.toString(roleHierarchyMap)).isEqualTo(expectedRoleHierarchy);

All we would really need to test is the conversion of the Map to a String. This would eliminate the duplication of test code in RoleHierarchyUtilsTests.

What do you think?

[thomasdarimont]

Hello @jgrandja,

thanks for the feedback - sounds legit to introduce:

public static String toString(Map<String, List<String>> roleHierarchyMap)

to RoleHierarchyUtils.

Will you adjust the PR or shall I?

[jgrandja]

Can you please adjust the PR @thomasdarimont?

I just want to clarify that my proposal was to replace:

public static RoleHierarchy roleHierarchyFromMap(Map<String, List<String>> roleHierarchyMapping)

with

public static String toString(Map<String, List<String>> roleHierarchyMap)

I want to make sure this is what you understood from my feedback?

[thomasdarimont]

Got it - should have been more precise in my reply... :)

Cheers,
Thomas

[jgrandja]

No worries Thomas.
@rwinch just let me know you used to be on the team a while back.
Appreciate your continued contributions!

[jgrandja]

Thanks again for the PR @thomasdarimont.
I've applied the adjustments as discussed and merged to master.