[ELY-2413] Re-authentication after reboot, even though HttpSession are persisted

[Skyllarr]

https://issues.redhat.com/browse/ELY-2413

[pferraro]

Given that this set will be generated per user and will typically be small in size, I might suggest using a more lightweight data structure (e.g. TreeSet) and leverage empty and singleton implementations if possible.
e.g.

    private Set<String> rolesToSet(Roles roles) {
        Iterator<String> iterator = roles.iterator();
        if (!iterator.hasNext()) return Set.of();
        String role = iterator.next();
        if (!iterator.hasNext()) return Set.of(role);
        Set<String> result = new TreeSet<>();
        result.add(role);
        while (iterator.hasNext()) {
            result.add(iterator.next());
        }
        return result;
    }

Perhaps this logic could reside within a Roles.toSet() method?

[Skyllarr]

@pferraro Good points! I have moved the method to the Roles class and I have used your TreeSet approach. I just modified it slightly to not use Set.of because we have to use Java 8 APIs. Thank you!

[pferraro]

return Collections.emptySet();

[Skyllarr]

fixed

[pferraro]

return Collections.singleton(role);

[Skyllarr]

fixed

[pferraro]

Roles.isEmpty() internally creates an Iterator, so you can avoid creating the iterator twice by testing for empty via the Iterator (created in line 146).

[Skyllarr]

@pferraro thanks, updated

[pferraro]

Why not use a normal public method instead of a static method?
e.g.

Set<String> toSet();

[Skyllarr]

@pferraro this is an interface. Also the above static Roles fromSet(Set<String> set) method is static so it is congruent now

[pferraro]

Interfaces can contain default method implementations.
e.g.

default Set<String> toSet() {
    // ...
}

[pferraro]

Anyway - this is just (my) stylistic preference, and does not need to block approval.

[pferraro]

I would suggest that this method return an immutable view of the roles.

[Skyllarr]

@pferraro Thank you, fixed

[pferraro]

return Collections.unmodifiableSet(result);

[Skyllarr]

fixed