From 12244b2e51bfde11209211c3ee56eef905cd96a7 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 15 Feb 2022 15:38:55 -0800 Subject: [PATCH] Provide more control over factory failure handling Add an additional `FactoryInstantiationFailureHandler` strategy interface to `SpringFactoriesLoader` to allows instantiation failures to be handled on a per-factory bases. For example, to log trace messages for only factories that can't be created the following can be used: FactoryInstantiationFailureHandler.logging(logger); If no `FactoryInstantiationFailureHandler` instance is supplied then `FactoryInstantiationFailureHandler.throwing()` is used which provides back-compatible behavior by throwing an `IllegalArgumentException`. See gh-28057 Co-authored-by: Madhura Bhave Co-authored-by: Andy Wilkinson --- .../io/support/SpringFactoriesLoader.java | 129 +++++++++++++++++- .../support/SpringFactoriesLoaderTests.java | 74 ++++++++++ 2 files changed, 198 insertions(+), 5 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java b/spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java index 639d00068c..0b97b10ccc 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java @@ -29,6 +29,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Supplier; @@ -95,6 +97,8 @@ public final class SpringFactoriesLoader { private static final Log logger = LogFactory.getLog(SpringFactoriesLoader.class); + private static final FailureHandler THROWING_HANDLER = FailureHandler.throwing(); + static final Map>> cache = new ConcurrentReferenceHashMap<>(); @@ -115,7 +119,7 @@ public final class SpringFactoriesLoader { * be loaded or if an error occurs while instantiating any factory */ public static List loadFactories(Class factoryType, @Nullable ClassLoader classLoader) { - return loadFactories(factoryType, classLoader, null); + return loadFactories(factoryType, classLoader, null, null); } /** @@ -135,13 +139,59 @@ public final class SpringFactoriesLoader { public static List loadFactories(Class factoryType, @Nullable ClassLoader classLoader, @Nullable ArgumentResolver argumentResolver) { + return loadFactories(factoryType, classLoader, argumentResolver, null); + } + + /** + * Load and instantiate the factory implementations of the given type from + * {@value #FACTORIES_RESOURCE_LOCATION}, using the given class loader with custom failure + * handling provided by the given failure handler. + *

The returned factories are sorted through {@link AnnotationAwareOrderComparator}. + *

As of Spring Framework 5.3, if duplicate implementation class names are + * discovered for a given factory type, only one instance of the duplicated + * implementation type will be instantiated. + *

For any factory implementation class that cannot be loaded or error that occurs while + * instantiating it, the given failure handler is called. + * @param factoryType the interface or abstract class representing the factory + * @param classLoader the ClassLoader to use for loading (can be {@code null} to use the default) + * @param failureHandler the FactoryInstantiationFailureHandler to use for handling of factory instantiation failures + * @since 6.0 + */ + public static List loadFactories(Class factoryType, @Nullable ClassLoader classLoader, + @Nullable FailureHandler failureHandler) { + + return loadFactories(factoryType, classLoader, null, failureHandler); + } + + /** + * Load and instantiate the factory implementations of the given type from + * {@value #FACTORIES_RESOURCE_LOCATION}, using the given arguments and class loader with custom + * failure handling provided by the given failure handler. + *

The returned factories are sorted through {@link AnnotationAwareOrderComparator}. + *

As of Spring Framework 5.3, if duplicate implementation class names are + * discovered for a given factory type, only one instance of the duplicated + * implementation type will be instantiated. + *

For any factory implementation class that cannot be loaded or error that occurs while + * instantiating it, the given failure handler is called. + * @param factoryType the interface or abstract class representing the factory + * @param classLoader the ClassLoader to use for loading (can be {@code null} to use the default) + * @param argumentResolver strategy used to resolve constructor arguments by their type + * @param failureHandler the FactoryInstantiationFailureHandler to use for handling of factory + * instantiation failures + * @since 6.0 + */ + public static List loadFactories(Class factoryType, @Nullable ClassLoader classLoader, + @Nullable ArgumentResolver argumentResolver, @Nullable FailureHandler failureHandler) { + Assert.notNull(factoryType, "'factoryType' must not be null"); ClassLoader classLoaderToUse = (classLoader != null) ? classLoader : SpringFactoriesLoader.class.getClassLoader(); List factoryImplementationNames = loadFactoryNames(factoryType, classLoaderToUse); logger.trace(LogMessage.format("Loaded [%s] names: %s", factoryType.getName(), factoryImplementationNames)); List result = new ArrayList<>(factoryImplementationNames.size()); + FailureHandler failureHandlerToUse = (failureHandler != null) ? failureHandler : THROWING_HANDLER; for (String factoryImplementationName : factoryImplementationNames) { - T factory = instantiateFactory(factoryImplementationName, factoryType, argumentResolver, classLoaderToUse); + T factory = instantiateFactory(factoryImplementationName, factoryType, + argumentResolver, classLoaderToUse, failureHandlerToUse); if (factory != null) { result.add(factory); } @@ -213,7 +263,7 @@ public final class SpringFactoriesLoader { @Nullable private static T instantiateFactory(String factoryImplementationName, Class factoryType, @Nullable ArgumentResolver argumentResolver, - ClassLoader classLoader) { + ClassLoader classLoader, FailureHandler failureHandler) { try { Class factoryImplementationClass = ClassUtils.forName(factoryImplementationName, classLoader); Assert.isTrue(factoryType.isAssignableFrom(factoryImplementationClass), @@ -222,8 +272,8 @@ public final class SpringFactoriesLoader { return factoryInstantiator.instantiate(argumentResolver); } catch (Throwable ex) { - throw new IllegalArgumentException("Unable to instantiate factory class [" + factoryImplementationName + - "] for factory type [" + factoryType.getName() + "]", ex); + failureHandler.handleFailure(factoryType, factoryImplementationName, ex); + return null; } } @@ -353,6 +403,75 @@ public final class SpringFactoriesLoader { } + /** + * Strategy for handling a failure that occurs when instantiating a factory. + * + * @since 6.0 + * @see FailureHandler#throwing() + * @see FailureHandler#logging(Log) + */ + @FunctionalInterface + public interface FailureHandler { + + /** + * Handle the {@code failure} that occurred when instantiating the {@code factoryImplementationName} + * that was expected to be of the given {@code factoryType}. + * @param factoryType the type of the factory + * @param factoryImplementationName the name of the factory implementation + * @param failure the failure that occurred + * @see #throwing() + * @see #logging + */ + void handleFailure(Class factoryType, String factoryImplementationName, Throwable failure); + + /** + * Return a new {@link FailureHandler} that handles + * errors by throwing an {@link IllegalArgumentException}. + * @return a new {@link FailureHandler} instance + */ + static FailureHandler throwing() { + return throwing(IllegalArgumentException::new); + } + + /** + * Return a new {@link FailureHandler} that handles + * errors by throwing an exception. + * @param exceptionFactory factory used to create the exception + * @return a new {@link FailureHandler} instance + */ + static FailureHandler throwing(BiFunction exceptionFactory) { + return handleMessage((message, failure) -> { + throw exceptionFactory.apply(message.get(), failure); + }); + } + + /** + * Return a new {@link FailureHandler} that handles + * errors by logging trace messages. + * @param logger the logger used to log message + * @return a new {@link FailureHandler} instance + */ + static FailureHandler logging(Log logger) { + return handleMessage((message, failure) -> logger.trace(LogMessage.of(message), failure)); + } + + /** + * Return a new {@link FailureHandler} that handles + * errors with using a standard formatted message. + * @param messageHandler the message handler used to handle the problem + * @return a new {@link FailureHandler} instance + */ + static FailureHandler handleMessage(BiConsumer, Throwable> messageHandler) { + return (factoryType, factoryImplementationName, failure) -> { + Supplier message = () -> "Unable to instantiate factory class [" + factoryImplementationName + + "] for factory type [" + factoryType.getName() + "]"; + messageHandler.accept(message, failure); + }; + } + + } + + /** * Strategy for resolving constructor arguments based on their type. * diff --git a/spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java b/spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java index 86c8256acf..80d931a43b 100644 --- a/spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java @@ -21,8 +21,10 @@ import java.lang.reflect.Modifier; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; +import java.util.ArrayList; import java.util.List; +import org.apache.commons.logging.Log; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; @@ -30,10 +32,16 @@ import org.junit.jupiter.api.Test; import org.springframework.core.io.support.SpringFactoriesLoader.ArgumentResolver; import org.springframework.core.io.support.SpringFactoriesLoader.FactoryInstantiator; +import org.springframework.core.io.support.SpringFactoriesLoader.FailureHandler; +import org.springframework.core.log.LogMessage; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Tests for {@link SpringFactoriesLoader}. @@ -95,6 +103,14 @@ class SpringFactoriesLoaderTests { + "[org.springframework.core.io.support.MyDummyFactory1] for factory type [java.lang.String]"); } + @Test + void attemptToLoadFactoryOfIncompatibleTypeWithLoggingFailureHandler() { + Log logger = mock(Log.class); + FailureHandler failureHandler = FailureHandler.logging(logger); + List factories = SpringFactoriesLoader.loadFactories(String.class, null, failureHandler); + assertThat(factories.isEmpty()); + } + @Test void loadFactoryWithNonDefaultConstructor() { ArgumentResolver resolver = ArgumentResolver.of(String.class, "injected"); @@ -116,6 +132,64 @@ class SpringFactoriesLoaderTests { .havingRootCause().withMessageContaining("Class [org.springframework.core.io.support.MultipleConstructorArgsDummyFactory] has no suitable constructor"); } + @Test + void loadFactoryWithMissingArgumentUsingLoggingFailureHandler() { + Log logger = mock(Log.class); + FailureHandler failureHandler = FailureHandler.logging(logger); + List factories = SpringFactoriesLoader.loadFactories(DummyFactory.class, LimitedClassLoader.multipleArgumentFactories, failureHandler); + assertThat(factories).hasSize(2); + assertThat(factories.get(0)).isInstanceOf(MyDummyFactory1.class); + assertThat(factories.get(1)).isInstanceOf(MyDummyFactory2.class); + } + + + @Nested + class FailureHandlerTests { + + @Test + void throwingReturnsHandlerThatThrowsIllegalArgumentException() { + FailureHandler handler = FailureHandler.throwing(); + RuntimeException cause = new RuntimeException(); + assertThatIllegalArgumentException().isThrownBy(() -> handler.handleFailure( + DummyFactory.class, MyDummyFactory1.class.getName(), + cause)).withMessageStartingWith("Unable to instantiate factory class").withCause(cause); + } + + @Test + void throwingWithFactoryReturnsHandlerThatThrows() { + FailureHandler handler = FailureHandler.throwing(IllegalStateException::new); + RuntimeException cause = new RuntimeException(); + assertThatIllegalStateException().isThrownBy(() -> handler.handleFailure( + DummyFactory.class, MyDummyFactory1.class.getName(), + cause)).withMessageStartingWith("Unable to instantiate factory class").withCause(cause); + } + + @Test + void loggingReturnsHandlerThatLogs() { + Log logger = mock(Log.class); + FailureHandler handler = FailureHandler.logging(logger); + RuntimeException cause = new RuntimeException(); + handler.handleFailure(DummyFactory.class, MyDummyFactory1.class.getName(), cause); + verify(logger).trace(isA(LogMessage.class), eq(cause)); + } + + @Test + void handleMessageReturnsHandlerThatAcceptsMessage() { + List failures = new ArrayList<>(); + List messages = new ArrayList<>(); + FailureHandler handler = FailureHandler.handleMessage((message, failure) -> { + failures.add(failure); + messages.add(message.get()); + }); + RuntimeException cause = new RuntimeException(); + handler.handleFailure(DummyFactory.class, MyDummyFactory1.class.getName(), cause); + assertThat(failures).containsExactly(cause); + assertThat(messages).hasSize(1); + assertThat(messages.get(0)).startsWith("Unable to instantiate factory class"); + } + + } + @Nested class ArgumentResolverTests {