From 8014114225432b2256dddab735b19dcf142e71b7 Mon Sep 17 00:00:00 2001 From: dperezcabrera Date: Fri, 16 Nov 2018 22:18:09 +0100 Subject: [PATCH] Make InMemory*ClientRegistrationRepository Consistent The previous builders with the list argument were inconsistent with their respective builders of var args. --- .../InMemoryClientRegistrationRepository.java | 22 +++++++++-------- ...yReactiveClientRegistrationRepository.java | 24 ++++--------------- ...moryClientRegistrationRepositoryTests.java | 6 +++++ ...tiveClientRegistrationRepositoryTests.java | 11 +++++++-- 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java index fa810e432b..a66c6c55a4 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java @@ -18,16 +18,13 @@ package org.springframework.security.oauth2.client.registration; import org.springframework.util.Assert; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentMap; import java.util.function.Function; -import java.util.stream.Collector; - -import static java.util.stream.Collectors.collectingAndThen; -import static java.util.stream.Collectors.toConcurrentMap; +import java.util.stream.Collectors; /** * A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory. @@ -39,6 +36,7 @@ import static java.util.stream.Collectors.toConcurrentMap; * @see ClientRegistration */ public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository, Iterable { + private final Map registrations; /** @@ -47,7 +45,8 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr * @param registrations the client registration(s) */ public InMemoryClientRegistrationRepository(ClientRegistration... registrations) { - this(Arrays.asList(registrations)); + Assert.notEmpty(registrations, "registrations cannot be empty"); + this.registrations = createClientRegistrationIdToClientRegistration(Arrays.asList(registrations)); } /** @@ -57,10 +56,13 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr */ public InMemoryClientRegistrationRepository(List registrations) { Assert.notEmpty(registrations, "registrations cannot be empty"); - Collector> collector = - toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity()); - this.registrations = registrations.stream() - .collect(collectingAndThen(collector, Collections::unmodifiableMap)); + this.registrations = createClientRegistrationIdToClientRegistration(registrations); + } + + private static Map createClientRegistrationIdToClientRegistration(Collection registrations) { + return Collections.unmodifiableMap(registrations.stream() + .peek(registration -> Assert.notNull(registration, "registrations cannot contain null values")) + .collect(Collectors.toMap(ClientRegistration::getRegistrationId, Function.identity()))); } @Override diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java index fa291c6a4f..d27628cefe 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java @@ -17,12 +17,6 @@ package org.springframework.security.oauth2.client.registration; import java.util.Iterator; import java.util.List; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - -import org.springframework.util.Assert; -import org.springframework.util.ConcurrentReferenceHashMap; import reactor.core.publisher.Mono; @@ -37,7 +31,7 @@ import reactor.core.publisher.Mono; public final class InMemoryReactiveClientRegistrationRepository implements ReactiveClientRegistrationRepository, Iterable { - private final Map clientIdToClientRegistration; + private final InMemoryClientRegistrationRepository delegate; /** * Constructs an {@code InMemoryReactiveClientRegistrationRepository} using the provided parameters. @@ -45,12 +39,7 @@ public final class InMemoryReactiveClientRegistrationRepository * @param registrations the client registration(s) */ public InMemoryReactiveClientRegistrationRepository(ClientRegistration... registrations) { - Assert.notEmpty(registrations, "registrations cannot be empty"); - this.clientIdToClientRegistration = new ConcurrentReferenceHashMap<>(); - for (ClientRegistration registration : registrations) { - Assert.notNull(registration, "registrations cannot contain null values"); - this.clientIdToClientRegistration.put(registration.getRegistrationId(), registration); - } + this.delegate = new InMemoryClientRegistrationRepository(registrations); } /** @@ -59,15 +48,12 @@ public final class InMemoryReactiveClientRegistrationRepository * @param registrations the client registration(s) */ public InMemoryReactiveClientRegistrationRepository(List registrations) { - Assert.notEmpty(registrations, "registrations cannot be null or empty"); - this.clientIdToClientRegistration = registrations.stream() - .collect(Collectors.toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity())); + this.delegate = new InMemoryClientRegistrationRepository(registrations); } - @Override public Mono findByRegistrationId(String registrationId) { - return Mono.justOrEmpty(this.clientIdToClientRegistration.get(registrationId)); + return Mono.justOrEmpty(this.delegate.findByRegistrationId(registrationId)); } /** @@ -77,6 +63,6 @@ public final class InMemoryReactiveClientRegistrationRepository */ @Override public Iterator iterator() { - return this.clientIdToClientRegistration.values().iterator(); + return delegate.iterator(); } } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java index efae7de7a9..2198b674f6 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java @@ -35,6 +35,12 @@ public class InMemoryClientRegistrationRepositoryTests { private InMemoryClientRegistrationRepository clients = new InMemoryClientRegistrationRepository(this.registration); + @Test(expected = IllegalArgumentException.class) + public void constructorVarArgsListClientRegistrationWhenNullThenIllegalArgumentException() { + ClientRegistration nullRegistration = null; + new InMemoryClientRegistrationRepository(nullRegistration); + } + @Test(expected = IllegalArgumentException.class) public void constructorListClientRegistrationWhenNullThenIllegalArgumentException() { List registrations = null; diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepositoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepositoryTests.java index ba2ef432a4..ec5a93d37d 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepositoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepositoryTests.java @@ -19,6 +19,7 @@ package org.springframework.security.oauth2.client.registration; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.Arrays; import java.util.List; import org.junit.Before; @@ -61,10 +62,16 @@ public class InMemoryReactiveClientRegistrationRepositoryTests { .isInstanceOf(IllegalArgumentException.class); } + @Test + public void constructorWhenClientRegistrationListHasNullThenIllegalArgumentException() { + List registrations = Arrays.asList(null, registration); + assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registrations)) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void constructorWhenClientRegistrationIsNullThenIllegalArgumentException() { - ClientRegistration registration = null; - assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration)) + assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration, null)) .isInstanceOf(IllegalArgumentException.class); }