Replace stream with cycles

[kostya05983]

Work in progress, change streams to for loops. Please see #7154

[kostya05983]

Hello, @rwinch I think, it's ready for review. Now, you may review it.

[kostya05983]

I updated changes from master.

[jzheaux]

@kostya05983 Thanks for the earlier updates!

I've added some additional inline feedback.

[jzheaux]

Why is this check necessary?

[kostya05983]

@jzheaux
When we use toConcurrentMap from Collectors, it uses uniqKeysMapAccumulator

 /**
     * {@code BiConsumer<Map, T>} that accumulates (key, value) pairs
     * extracted from elements into the map, throwing {@code IllegalStateException}
     * if duplicate keys are encountered.
     *
     * @param keyMapper a function that maps an element into a key
     * @param valueMapper a function that maps an element into a value
     * @param <T> type of elements
     * @param <K> type of map keys
     * @param <V> type of map values
     * @return an accumulating consumer
     */
    private static <T, K, V>
    BiConsumer<Map<K, V>, T> uniqKeysMapAccumulator(Function<? super T, ? extends K> keyMapper,
                                                    Function<? super T, ? extends V> valueMapper) {
        return (map, element) -> {
            K k = keyMapper.apply(element);
            V v = Objects.requireNonNull(valueMapper.apply(element));
            V u = map.putIfAbsent(k, v);
            if (u != null) throw duplicateKeyException(k, u, v);
        };
    }

I also add check to save logic with throwing an exception, if we find duplicate key. Maybe, I understand something wrong and logic with client Registration can just rewrite value, how do you think? Can we break something?

[jzheaux]

Collisions aren't a concern in this case but thank you for keeping an eye on this.

[jzheaux]

So, let's go ahead and remove this extra check and exception. Collisions aren't a concern here, @kostya05983

[kostya05983]

Oks, also I need a remove test for it, because there is a test which checks duplicates.

[kostya05983]

Removed

[jzheaux]

@kostya05983 for some reason I'd missed that there is a test that relies on this functionality. I apologize, but because of that, we should add the duplicate checks as well as the test back in. My mistake.

[jzheaux]

Thanks for the updates, @kostya05983. I've left some additional feedback inline.

[jzheaux]

Collisions aren't a concern in this case but thank you for keeping an eye on this.

[jzheaux]

@kostya05983, thanks for how responsive you've been in this PR. You're nearly there!

I've left just a bit more feedback inline. Note that there are still a couple of open conversations from past reviews, too.

Once you believe everything is resolved, please squash your commits into one, in perparation for merging.

[kostya05983]

@jzheaux Thanks for guidance. I think it's ready, but I tried to rebase and squash my nearest commits into one,in such groups: [Replace stream, First version of replacing streams, fix wwAuthentication and code style] and other similar group with other dates. But I got a big diff on test branch. Can you tell me how I can squash into one my commits which between others commits and not to danger history. I tried with interactive git rebase.

[jzheaux]

To rebase, you can do something like (assuming that you've called Spring Security's upstream spring-projects)

git checkout master
git pull spring-projects master
git checkout gh-7154-replace-streams
git rebase master

You will likely have some conflicts to resolve, but at that point, the commits on your feature branch should be at the end of the chain. Once you've resolved the conflicts, then do (assuming your fork is called origin):

git push origin +gh-7154-replace-streams

[kostya05983]

@jzheaux , I squashed commits, it's ready for merge

[jzheaux]

@kostya05983 - thank you for your careful work on this. This is now merged into master via f6c650d.

I edited the commit message in order to align with the contributor guidelines.