From 403481ff961fea69ffda2fa8eba3e50f884d7ddb Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 29 Apr 2023 13:23:48 -0700 Subject: [PATCH] Add 'required' parameter to ConnectionDetailsFactories Update `ConnectionDetailsFactories` so that callers can now declare if a result is required or not and improve exception hierarchy. See gh-35168 --- .../ConnectionDetailsFactories.java | 17 +++++-- ...ectionDetailsFactoryNotFoundException.java | 2 +- .../ConnectionDetailsNotFoundException.java | 34 ++++++++++++++ .../ConnectionDetailsFactoriesTests.java | 44 ++++++++++++++++--- ...ServiceConnectionsApplicationListener.java | 2 +- .../ContainerConnectionSourcesRegistrar.java | 11 +---- ...rviceConnectionContextCustomizerTests.java | 15 +------ 7 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsNotFoundException.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactories.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactories.java index 66e9ccefc8b..d93146eb82c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactories.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactories.java @@ -65,10 +65,16 @@ public class ConnectionDetailsFactories { * given source. * @param the source type * @param source the source + * @param required if a connection details result is required * @return a map of {@link ConnectionDetails} instances + * @throws ConnectionDetailsFactoryNotFoundException if a result is required but no + * connection details factory is registered for the source + * @throws ConnectionDetailsNotFoundException if a result is required but no + * connection details instance was created from a registered factory */ - public Map, ConnectionDetails> getConnectionDetails(S source) { - List> registrations = getRegistrations(source); + public Map, ConnectionDetails> getConnectionDetails(S source, boolean required) + throws ConnectionDetailsFactoryNotFoundException, ConnectionDetailsNotFoundException { + List> registrations = getRegistrations(source, required); Map, ConnectionDetails> result = new LinkedHashMap<>(); for (Registration registration : registrations) { ConnectionDetails connectionDetails = registration.factory().getConnectionDetails(source); @@ -79,11 +85,14 @@ public class ConnectionDetailsFactories { .formatted(connectionDetailsType.getName())); } } + if (required && result.isEmpty()) { + throw new ConnectionDetailsNotFoundException(source); + } return Map.copyOf(result); } @SuppressWarnings("unchecked") - List> getRegistrations(S source) { + List> getRegistrations(S source, boolean required) { Class sourceType = (Class) source.getClass(); List> result = new ArrayList<>(); for (Registration candidate : this.registrations) { @@ -91,7 +100,7 @@ public class ConnectionDetailsFactories { result.add((Registration) candidate); } } - if (result.isEmpty()) { + if (required && result.isEmpty()) { throw new ConnectionDetailsFactoryNotFoundException(source); } result.sort(Comparator.comparing(Registration::factory, AnnotationAwareOrderComparator.INSTANCE)); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoryNotFoundException.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoryNotFoundException.java index f618da5f01a..84b50fb8147 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoryNotFoundException.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoryNotFoundException.java @@ -28,7 +28,7 @@ package org.springframework.boot.autoconfigure.service.connection; public class ConnectionDetailsFactoryNotFoundException extends RuntimeException { public ConnectionDetailsFactoryNotFoundException(S source) { - super("No ConnectionDetailsFactory found for source '" + source + "'"); + super("No ConnectionDetailsFactory found for source '%s'".formatted(source)); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsNotFoundException.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsNotFoundException.java new file mode 100644 index 00000000000..0f63cdecfe9 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsNotFoundException.java @@ -0,0 +1,34 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.service.connection; + +/** + * {@link RuntimeException} thrown when required {@link ConnectionDetails} could not be + * found. + * + * @author Moritz Halbritter + * @author Andy Wilkinson + * @author Phillip Webb + * @since 3.1.0 + */ +public class ConnectionDetailsNotFoundException extends RuntimeException { + + public ConnectionDetailsNotFoundException(S source) { + super("No ConnectionDetails found for source '%s'".formatted(source)); + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoriesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoriesTests.java index 68c8bf6b091..989d7002228 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoriesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/service/connection/ConnectionDetailsFactoriesTests.java @@ -42,27 +42,49 @@ class ConnectionDetailsFactoriesTests { private final MockSpringFactoriesLoader loader = new MockSpringFactoriesLoader(); @Test - void getConnectionDetailsWhenNoFactoryForSourceThrowsException() { + void getRequiredConnectionDetailsWhenNoFactoryForSourceThrowsException() { ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); assertThatExceptionOfType(ConnectionDetailsFactoryNotFoundException.class) - .isThrownBy(() -> factories.getConnectionDetails("source")); + .isThrownBy(() -> factories.getConnectionDetails("source", true)); + } + + @Test + void getOptionalConnectionDetailsWhenNoFactoryForSourceThrowsException() { + ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); + assertThat(factories.getConnectionDetails("source", false)).isEmpty(); } @Test void getConnectionDetailsWhenSourceHasOneMatchReturnsSingleResult() { this.loader.addInstance(ConnectionDetailsFactory.class, new TestConnectionDetailsFactory()); ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); - Map, ConnectionDetails> connectionDetails = factories.getConnectionDetails("source"); + Map, ConnectionDetails> connectionDetails = factories.getConnectionDetails("source", false); assertThat(connectionDetails).hasSize(1); assertThat(connectionDetails.get(TestConnectionDetails.class)).isInstanceOf(TestConnectionDetailsImpl.class); } + @Test + void getRequiredConnectionDetailsWhenSourceHasNoMatchTheowsException() { + this.loader.addInstance(ConnectionDetailsFactory.class, new NullResultTestConnectionDetailsFactory()); + ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); + assertThatExceptionOfType(ConnectionDetailsNotFoundException.class) + .isThrownBy(() -> factories.getConnectionDetails("source", true)); + } + + @Test + void getOptionalConnectionDetailsWhenSourceHasNoMatchReturnsEmptyMap() { + this.loader.addInstance(ConnectionDetailsFactory.class, new NullResultTestConnectionDetailsFactory()); + ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); + Map, ConnectionDetails> connectionDetails = factories.getConnectionDetails("source", false); + assertThat(connectionDetails).isEmpty(); + } + @Test void getConnectionDetailsWhenSourceHasMultipleMatchesReturnsMultipleResults() { this.loader.addInstance(ConnectionDetailsFactory.class, new TestConnectionDetailsFactory(), new OtherConnectionDetailsFactory()); ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); - Map, ConnectionDetails> connectionDetails = factories.getConnectionDetails("source"); + Map, ConnectionDetails> connectionDetails = factories.getConnectionDetails("source", false); assertThat(connectionDetails).hasSize(2); } @@ -71,7 +93,7 @@ class ConnectionDetailsFactoriesTests { this.loader.addInstance(ConnectionDetailsFactory.class, new TestConnectionDetailsFactory(), new TestConnectionDetailsFactory()); ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); - assertThatIllegalStateException().isThrownBy(() -> factories.getConnectionDetails("source")) + assertThatIllegalStateException().isThrownBy(() -> factories.getConnectionDetails("source", false)) .withMessage("Duplicate connection details supplied for " + TestConnectionDetails.class.getName()); } @@ -82,7 +104,7 @@ class ConnectionDetailsFactoriesTests { TestConnectionDetailsFactory orderThree = new TestConnectionDetailsFactory(3); this.loader.addInstance(ConnectionDetailsFactory.class, orderOne, orderThree, orderTwo); ConnectionDetailsFactories factories = new ConnectionDetailsFactories(this.loader); - List> registrations = factories.getRegistrations("source"); + List> registrations = factories.getRegistrations("source", false); assertThat(registrations.get(0).factory()).isEqualTo(orderOne); assertThat(registrations.get(1).factory()).isEqualTo(orderTwo); assertThat(registrations.get(2).factory()).isEqualTo(orderThree); @@ -119,6 +141,16 @@ class ConnectionDetailsFactoriesTests { } + private static final class NullResultTestConnectionDetailsFactory + implements ConnectionDetailsFactory { + + @Override + public TestConnectionDetails getConnectionDetails(String source) { + return null; + } + + } + private static final class OtherConnectionDetailsFactory implements ConnectionDetailsFactory { diff --git a/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/service/connection/DockerComposeServiceConnectionsApplicationListener.java b/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/service/connection/DockerComposeServiceConnectionsApplicationListener.java index 3e9e3a2fa53..93338c8eed8 100644 --- a/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/service/connection/DockerComposeServiceConnectionsApplicationListener.java +++ b/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/service/connection/DockerComposeServiceConnectionsApplicationListener.java @@ -65,7 +65,7 @@ class DockerComposeServiceConnectionsApplicationListener private void registerConnectionDetails(BeanDefinitionRegistry registry, List runningServices) { for (RunningService runningService : runningServices) { DockerComposeConnectionSource source = new DockerComposeConnectionSource(runningService); - this.factories.getConnectionDetails(source) + this.factories.getConnectionDetails(source, false) .forEach((connectionDetailsType, connectionDetails) -> register(registry, runningService, connectionDetailsType, connectionDetails)); } diff --git a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSourcesRegistrar.java b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSourcesRegistrar.java index 563b7194a60..c8d155162ab 100644 --- a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSourcesRegistrar.java +++ b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSourcesRegistrar.java @@ -19,7 +19,6 @@ package org.springframework.boot.testcontainers.service.connection; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -32,7 +31,6 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.autoconfigure.service.connection.ConnectionDetails; import org.springframework.boot.autoconfigure.service.connection.ConnectionDetailsFactories; import org.springframework.core.log.LogMessage; -import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -67,18 +65,11 @@ class ContainerConnectionSourcesRegistrar { } private void registerBeanDefinition(BeanDefinitionRegistry registry, ContainerConnectionSource source) { - getConnectionDetails(source) + this.connectionDetailsFactories.getConnectionDetails(source, true) .forEach((connectionDetailsType, connectionDetails) -> registerBeanDefinition(registry, source, connectionDetailsType, connectionDetails)); } - private Map, ConnectionDetails> getConnectionDetails(S source) { - Map, ConnectionDetails> connectionDetails = this.connectionDetailsFactories - .getConnectionDetails(source); - Assert.state(!connectionDetails.isEmpty(), () -> "No connection details created for %s".formatted(source)); - return connectionDetails; - } - @SuppressWarnings("unchecked") private void registerBeanDefinition(BeanDefinitionRegistry registry, ContainerConnectionSource source, Class connectionDetailsType, ConnectionDetails connectionDetails) { diff --git a/spring-boot-project/spring-boot-testcontainers/src/test/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizerTests.java b/spring-boot-project/spring-boot-testcontainers/src/test/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizerTests.java index e2899187a57..7f3d5e16ff7 100644 --- a/spring-boot-project/spring-boot-testcontainers/src/test/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizerTests.java +++ b/spring-boot-project/spring-boot-testcontainers/src/test/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizerTests.java @@ -36,7 +36,6 @@ import org.springframework.core.annotation.MergedAnnotation; import org.springframework.test.context.MergedContextConfiguration; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; @@ -85,7 +84,7 @@ class ServiceConnectionContextCustomizerTests { given(context.getBeanFactory()).willReturn(beanFactory); MergedContextConfiguration mergedConfig = mock(MergedContextConfiguration.class); JdbcConnectionDetails connectionDetails = new TestJdbcConnectionDetails(); - given(this.factories.getConnectionDetails(this.source)) + given(this.factories.getConnectionDetails(this.source, true)) .willReturn(Map.of(JdbcConnectionDetails.class, connectionDetails)); customizer.customizeContext(context, mergedConfig); ArgumentCaptor beanDefinitionCaptor = ArgumentCaptor.forClass(BeanDefinition.class); @@ -96,18 +95,6 @@ class ServiceConnectionContextCustomizerTests { assertThat(beanDefinition.getBeanClass()).isEqualTo(TestJdbcConnectionDetails.class); } - @Test - void customizeContextWhenFactoriesHasNoConnectionDetailsThrowsException() { - ServiceConnectionContextCustomizer customizer = new ServiceConnectionContextCustomizer(List.of(this.source), - this.factories); - ConfigurableApplicationContext context = mock(ConfigurableApplicationContext.class); - DefaultListableBeanFactory beanFactory = spy(new DefaultListableBeanFactory()); - given(context.getBeanFactory()).willReturn(beanFactory); - MergedContextConfiguration mergedConfig = mock(MergedContextConfiguration.class); - assertThatIllegalStateException().isThrownBy(() -> customizer.customizeContext(context, mergedConfig)) - .withMessageStartingWith("No connection details created for @ServiceConnection source"); - } - /** * Test {@link JdbcConnectionDetails}. */