Make InMemory*ClientRegistrationRepository Consistent

The previous builders with the list argument were inconsistent with their
respective builders of var args.
This commit is contained in:
dperezcabrera 2018-11-16 22:18:09 +01:00 committed by Rob Winch
parent e1d68e4f6b
commit f6414e9a52
4 changed files with 35 additions and 43 deletions

View File

@ -18,16 +18,13 @@ package org.springframework.security.oauth2.client.registration;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function; import java.util.function.Function;
import java.util.stream.Collector; import java.util.stream.Collectors;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toConcurrentMap;
/** /**
* A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory. * A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory.
@ -40,6 +37,7 @@ import static java.util.stream.Collectors.toConcurrentMap;
* @see ClientRegistration * @see ClientRegistration
*/ */
public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository, Iterable<ClientRegistration> { public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository, Iterable<ClientRegistration> {
private final Map<String, ClientRegistration> registrations; private final Map<String, ClientRegistration> registrations;
/** /**
@ -48,7 +46,8 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
* @param registrations the client registration(s) * @param registrations the client registration(s)
*/ */
public InMemoryClientRegistrationRepository(ClientRegistration... registrations) { public InMemoryClientRegistrationRepository(ClientRegistration... registrations) {
this(Arrays.asList(registrations)); Assert.notEmpty(registrations, "registrations cannot be empty");
this.registrations = createClientRegistrationIdToClientRegistration(Arrays.asList(registrations));
} }
/** /**
@ -57,14 +56,7 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
* @param registrations the client registration(s) * @param registrations the client registration(s)
*/ */
public InMemoryClientRegistrationRepository(List<ClientRegistration> registrations) { public InMemoryClientRegistrationRepository(List<ClientRegistration> registrations) {
this(createRegistrationsMap(registrations)); this(createClientRegistrationIdToClientRegistration(registrations));
}
private static Map<String, ClientRegistration> createRegistrationsMap(List<ClientRegistration> registrations) {
Assert.notEmpty(registrations, "registrations cannot be empty");
Collector<ClientRegistration, ?, ConcurrentMap<String, ClientRegistration>> collector =
toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity());
return registrations.stream().collect(collectingAndThen(collector, Collections::unmodifiableMap));
} }
/** /**
@ -79,6 +71,13 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
this.registrations = registrations; this.registrations = registrations;
} }
private static Map<String, ClientRegistration> createClientRegistrationIdToClientRegistration(Collection<ClientRegistration> registrations) {
Assert.notNull(registrations, "registrations cannot be null");
return Collections.unmodifiableMap(registrations.stream()
.peek(registration -> Assert.notNull(registration, "registrations cannot contain null values"))
.collect(Collectors.toMap(ClientRegistration::getRegistrationId, Function.identity())));
}
@Override @Override
public ClientRegistration findByRegistrationId(String registrationId) { public ClientRegistration findByRegistrationId(String registrationId) {
Assert.hasText(registrationId, "registrationId cannot be empty"); Assert.hasText(registrationId, "registrationId cannot be empty");

View File

@ -18,11 +18,6 @@ package org.springframework.security.oauth2.client.registration;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; 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; import reactor.core.publisher.Mono;
@ -38,7 +33,7 @@ import reactor.core.publisher.Mono;
public final class InMemoryReactiveClientRegistrationRepository public final class InMemoryReactiveClientRegistrationRepository
implements ReactiveClientRegistrationRepository, Iterable<ClientRegistration> { implements ReactiveClientRegistrationRepository, Iterable<ClientRegistration> {
private final Map<String, ClientRegistration> clientIdToClientRegistration; private final InMemoryClientRegistrationRepository delegate;
/** /**
* Constructs an {@code InMemoryReactiveClientRegistrationRepository} using the provided parameters. * Constructs an {@code InMemoryReactiveClientRegistrationRepository} using the provided parameters.
@ -46,12 +41,7 @@ public final class InMemoryReactiveClientRegistrationRepository
* @param registrations the client registration(s) * @param registrations the client registration(s)
*/ */
public InMemoryReactiveClientRegistrationRepository(ClientRegistration... registrations) { public InMemoryReactiveClientRegistrationRepository(ClientRegistration... registrations) {
Assert.notEmpty(registrations, "registrations cannot be empty"); this.delegate = new InMemoryClientRegistrationRepository(registrations);
this.clientIdToClientRegistration = new ConcurrentReferenceHashMap<>();
for (ClientRegistration registration : registrations) {
Assert.notNull(registration, "registrations cannot contain null values");
this.clientIdToClientRegistration.put(registration.getRegistrationId(), registration);
}
} }
/** /**
@ -60,9 +50,7 @@ public final class InMemoryReactiveClientRegistrationRepository
* @param registrations the client registration(s) * @param registrations the client registration(s)
*/ */
public InMemoryReactiveClientRegistrationRepository(List<ClientRegistration> registrations) { public InMemoryReactiveClientRegistrationRepository(List<ClientRegistration> registrations) {
Assert.notEmpty(registrations, "registrations cannot be null or empty"); this.delegate = new InMemoryClientRegistrationRepository(registrations);
this.clientIdToClientRegistration = registrations.stream()
.collect(Collectors.toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity()));
} }
/** /**
@ -73,14 +61,13 @@ public final class InMemoryReactiveClientRegistrationRepository
* @since 5.2 * @since 5.2
* @param registrations the {@code Map} of client registration(s) * @param registrations the {@code Map} of client registration(s)
*/ */
public InMemoryReactiveClientRegistrationRepository(Map<String, ClientRegistration> registrations) { public InMemoryReactiveClientRegistrationRepository(
Assert.notNull(registrations, "registrations cannot be null"); Map<String, ClientRegistration> registrations) {
this.clientIdToClientRegistration = registrations; this.delegate = new InMemoryClientRegistrationRepository(registrations);
} }
@Override
public Mono<ClientRegistration> findByRegistrationId(String registrationId) { public Mono<ClientRegistration> findByRegistrationId(String registrationId) {
return Mono.justOrEmpty(this.clientIdToClientRegistration.get(registrationId)); return Mono.justOrEmpty(this.delegate.findByRegistrationId(registrationId));
} }
/** /**
@ -90,6 +77,6 @@ public final class InMemoryReactiveClientRegistrationRepository
*/ */
@Override @Override
public Iterator<ClientRegistration> iterator() { public Iterator<ClientRegistration> iterator() {
return this.clientIdToClientRegistration.values().iterator(); return delegate.iterator();
} }
} }

View File

@ -19,7 +19,6 @@ package org.springframework.security.oauth2.client.registration;
import org.junit.Test; import org.junit.Test;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@ -39,14 +38,14 @@ public class InMemoryClientRegistrationRepositoryTests {
private InMemoryClientRegistrationRepository clients = new InMemoryClientRegistrationRepository(this.registration); private InMemoryClientRegistrationRepository clients = new InMemoryClientRegistrationRepository(this.registration);
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void constructorListClientRegistrationWhenNullThenIllegalArgumentException() { public void constructorVarArgsListClientRegistrationWhenNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = null; ClientRegistration nullRegistration = null;
new InMemoryClientRegistrationRepository(registrations); new InMemoryClientRegistrationRepository(nullRegistration);
} }
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void constructorListClientRegistrationWhenEmptyThenIllegalArgumentException() { public void constructorListClientRegistrationWhenNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = Collections.emptyList(); List<ClientRegistration> registrations = null;
new InMemoryClientRegistrationRepository(registrations); new InMemoryClientRegistrationRepository(registrations);
} }

View File

@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.HashMap; import java.util.HashMap;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@ -64,10 +65,16 @@ public class InMemoryReactiveClientRegistrationRepositoryTests {
.isInstanceOf(IllegalArgumentException.class); .isInstanceOf(IllegalArgumentException.class);
} }
@Test
public void constructorWhenClientRegistrationListHasNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = Arrays.asList(null, registration);
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registrations))
.isInstanceOf(IllegalArgumentException.class);
}
@Test @Test
public void constructorWhenClientRegistrationIsNullThenIllegalArgumentException() { public void constructorWhenClientRegistrationIsNullThenIllegalArgumentException() {
ClientRegistration registration = null; assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration, null))
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration))
.isInstanceOf(IllegalArgumentException.class); .isInstanceOf(IllegalArgumentException.class);
} }