From 4b54ca46d32c05a661123cf0b530ca816709b495 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 5 Aug 2023 09:33:07 +0300 Subject: [PATCH 1/3] Polish PropertySourceDescriptor --- .../core/io/support/PropertySourceDescriptor.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceDescriptor.java b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceDescriptor.java index 2f0b13b3cce..254a1d49d68 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceDescriptor.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceDescriptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-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. @@ -19,20 +19,22 @@ package org.springframework.core.io.support; import java.util.Arrays; import java.util.List; -import org.springframework.core.env.PropertySource; import org.springframework.lang.Nullable; /** - * Describe a {@link PropertySource}. + * Descriptor for a {@link org.springframework.core.env.PropertySource PropertySource}. * * @param locations the locations to consider - * @param ignoreResourceNotFound whether to fail if a location does not exist + * @param ignoreResourceNotFound whether a failure to find a property resource + * should be ignored * @param name the name of the property source, or {@code null} to infer one - * @param propertySourceFactory the {@link PropertySourceFactory} to use, or - * {@code null} to use the default + * @param propertySourceFactory the type of {@link PropertySourceFactory} to use, + * or {@code null} to use the default * @param encoding the encoding, or {@code null} to use the default encoding * @author Stephane Nicoll * @since 6.0 + * @see org.springframework.core.env.PropertySource + * @see org.springframework.context.annotation.PropertySource */ public record PropertySourceDescriptor(List locations, boolean ignoreResourceNotFound, @Nullable String name, @Nullable Class propertySourceFactory, From 1451f307814cf3144afd10791da13522cd54eca9 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 4 Aug 2023 16:07:22 +0300 Subject: [PATCH 2/3] Polish PropertySourceProcessor --- .../io/support/PropertySourceProcessor.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java index df19941a0b0..d24acd63275 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java @@ -49,22 +49,24 @@ import org.springframework.util.ReflectionUtils; */ public class PropertySourceProcessor { - private static final PropertySourceFactory DEFAULT_PROPERTY_SOURCE_FACTORY = new DefaultPropertySourceFactory(); + private static final PropertySourceFactory defaultPropertySourceFactory = new DefaultPropertySourceFactory(); private static final Log logger = LogFactory.getLog(PropertySourceProcessor.class); + private final ConfigurableEnvironment environment; private final ResourceLoader resourceLoader; - private final List propertySourceNames; + private final List propertySourceNames = new ArrayList<>(); + public PropertySourceProcessor(ConfigurableEnvironment environment, ResourceLoader resourceLoader) { this.environment = environment; this.resourceLoader = resourceLoader; - this.propertySourceNames = new ArrayList<>(); } + /** * Process the specified {@link PropertySourceDescriptor} against the * environment managed by this instance. @@ -78,7 +80,7 @@ public class PropertySourceProcessor { Assert.isTrue(locations.size() > 0, "At least one @PropertySource(value) location is required"); boolean ignoreResourceNotFound = descriptor.ignoreResourceNotFound(); PropertySourceFactory factory = (descriptor.propertySourceFactory() != null ? - instantiateClass(descriptor.propertySourceFactory()) : DEFAULT_PROPERTY_SOURCE_FACTORY); + instantiateClass(descriptor.propertySourceFactory()) : defaultPropertySourceFactory); for (String location : locations) { try { @@ -100,13 +102,13 @@ public class PropertySourceProcessor { } } - private void addPropertySource(org.springframework.core.env.PropertySource propertySource) { + private void addPropertySource(PropertySource propertySource) { String name = propertySource.getName(); MutablePropertySources propertySources = this.environment.getPropertySources(); if (this.propertySourceNames.contains(name)) { // We've already added a version, we need to extend it - org.springframework.core.env.PropertySource existing = propertySources.get(name); + PropertySource existing = propertySources.get(name); if (existing != null) { PropertySource newSource = (propertySource instanceof ResourcePropertySource rps ? rps.withResourceName() : propertySource); @@ -136,7 +138,8 @@ public class PropertySourceProcessor { this.propertySourceNames.add(name); } - private PropertySourceFactory instantiateClass(Class type) { + + private static PropertySourceFactory instantiateClass(Class type) { try { Constructor constructor = type.getDeclaredConstructor(); ReflectionUtils.makeAccessible(constructor); From 4a81814dbb54b20919154b1ed92fae657deb2223 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 5 Aug 2023 09:59:24 +0300 Subject: [PATCH 3/3] Check exception cause for @PropertySource(ignoreResourceNotFound) support Prior to this commit, the ignoreResourceNotFound flag in @PropertySource was ignored by PropertySourceProcessor if a PropertySourceFactory threw an exception which wrapped an exception that would otherwise be ignored -- for example, a FileNotFoundException. To address this issue, this commit updates PropertySourceFactory so that it catches RuntimeException and IOException and then checks if the exception or its cause is an "ignorable" exception in terms of ignoreResourceNotFound semantics. Closes gh-22276 --- .../io/support/PropertySourceProcessor.java | 19 +- .../support/PropertySourceProcessorTests.java | 182 ++++++++++++++++++ .../core/io/support/test.properties | 1 + 3 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java create mode 100644 spring-core/src/test/resources/org/springframework/core/io/support/test.properties diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java index d24acd63275..407c5019e71 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceProcessor.java @@ -34,6 +34,7 @@ import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; @@ -44,6 +45,7 @@ import org.springframework.util.ReflectionUtils; * single {@link PropertySource} rather than creating dedicated ones. * * @author Stephane Nicoll + * @author Sam Brannen * @since 6.0 * @see PropertySourceDescriptor */ @@ -88,9 +90,10 @@ public class PropertySourceProcessor { Resource resource = this.resourceLoader.getResource(resolvedLocation); addPropertySource(factory.createPropertySource(name, new EncodedResource(resource, encoding))); } - catch (IllegalArgumentException | FileNotFoundException | UnknownHostException | SocketException ex) { - // Placeholders not resolvable or resource not found when trying to open it - if (ignoreResourceNotFound) { + catch (RuntimeException | IOException ex) { + // Placeholders not resolvable (IllegalArgumentException) or resource not found when trying to open it + if (ignoreResourceNotFound && (ex instanceof IllegalArgumentException || isIgnorableException(ex) || + isIgnorableException(ex.getCause()))) { if (logger.isInfoEnabled()) { logger.info("Properties location [" + location + "] not resolvable: " + ex.getMessage()); } @@ -150,4 +153,14 @@ public class PropertySourceProcessor { } } + /** + * Determine if the supplied exception can be ignored according to + * {@code ignoreResourceNotFound} semantics. + */ + private static boolean isIgnorableException(@Nullable Throwable ex) { + return (ex instanceof FileNotFoundException || + ex instanceof UnknownHostException || + ex instanceof SocketException); + } + } diff --git a/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java b/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java new file mode 100644 index 00000000000..ee663ec9342 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/io/support/PropertySourceProcessorTests.java @@ -0,0 +1,182 @@ +/* + * Copyright 2002-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.core.io.support; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.SocketException; +import java.net.UnknownHostException; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import org.springframework.core.env.PropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.io.DefaultResourceLoader; +import org.springframework.core.io.ResourceLoader; +import org.springframework.util.ClassUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; + +/** + * Unit tests for {@link PropertySourceProcessor}. + * + * @author Sam Brannen + * @since 6.0.12 + */ +class PropertySourceProcessorTests { + + private static final String PROPS_FILE = ClassUtils.classPackageAsResourcePath(PropertySourceProcessorTests.class) + "/test.properties"; + + private final StandardEnvironment environment = new StandardEnvironment(); + private final ResourceLoader resourceLoader = new DefaultResourceLoader(); + private final PropertySourceProcessor processor = new PropertySourceProcessor(environment, resourceLoader); + + + @BeforeEach + void checkInitialPropertySources() { + assertThat(environment.getPropertySources()).hasSize(2); + } + + @Test + void processorRegistersPropertySource() throws Exception { + PropertySourceDescriptor descriptor = new PropertySourceDescriptor(List.of(PROPS_FILE), false, null, DefaultPropertySourceFactory.class, null); + processor.processPropertySource(descriptor); + assertThat(environment.getPropertySources()).hasSize(3); + assertThat(environment.getProperty("enigma")).isEqualTo("42"); + } + + @Nested + class FailOnErrorTests { + + @Test + void processorFailsOnIllegalArgumentException() { + assertProcessorFailsOnError(IllegalArgumentExceptionPropertySourceFactory.class, IllegalArgumentException.class); + } + + @Test + void processorFailsOnFileNotFoundException() { + assertProcessorFailsOnError(FileNotFoundExceptionPropertySourceFactory.class, FileNotFoundException.class); + } + + private void assertProcessorFailsOnError( + Class factoryClass, Class exceptionType) { + + PropertySourceDescriptor descriptor = + new PropertySourceDescriptor(List.of(PROPS_FILE), false, null, factoryClass, null); + assertThatExceptionOfType(exceptionType).isThrownBy(() -> processor.processPropertySource(descriptor)); + assertThat(environment.getPropertySources()).hasSize(2); + } + + } + + @Nested + class IgnoreResourceNotFoundTests { + + @Test + void processorIgnoresIllegalArgumentException() { + assertProcessorIgnoresFailure(IllegalArgumentExceptionPropertySourceFactory.class); + } + + @Test + void processorIgnoresFileNotFoundException() { + assertProcessorIgnoresFailure(FileNotFoundExceptionPropertySourceFactory.class); + } + + @Test + void processorIgnoresUnknownHostException() { + assertProcessorIgnoresFailure(UnknownHostExceptionPropertySourceFactory.class); + } + + @Test + void processorIgnoresSocketException() { + assertProcessorIgnoresFailure(SocketExceptionPropertySourceFactory.class); + } + + @Test + void processorIgnoresSupportedExceptionWrappedInIllegalStateException() { + assertProcessorIgnoresFailure(WrappedIOExceptionPropertySourceFactory.class); + } + + @Test + void processorIgnoresSupportedExceptionWrappedInUncheckedIOException() { + assertProcessorIgnoresFailure(UncheckedIOExceptionPropertySourceFactory.class); + } + + private void assertProcessorIgnoresFailure(Class factoryClass) { + PropertySourceDescriptor descriptor = new PropertySourceDescriptor(List.of(PROPS_FILE), true, null, factoryClass, null); + assertThatNoException().isThrownBy(() -> processor.processPropertySource(descriptor)); + assertThat(environment.getPropertySources()).hasSize(2); + } + + } + + + private static class IllegalArgumentExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) throws IOException { + throw new IllegalArgumentException("bogus"); + } + } + + private static class FileNotFoundExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) throws IOException { + throw new FileNotFoundException("bogus"); + } + } + + private static class UnknownHostExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) throws IOException { + throw new UnknownHostException("bogus"); + } + } + + private static class SocketExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) throws IOException { + throw new SocketException("bogus"); + } + } + + private static class WrappedIOExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) { + throw new IllegalStateException("Wrapped", new FileNotFoundException("bogus")); + } + } + + private static class UncheckedIOExceptionPropertySourceFactory implements PropertySourceFactory { + + @Override + public PropertySource createPropertySource(String name, EncodedResource resource) { + throw new UncheckedIOException("Wrapped", new FileNotFoundException("bogus")); + } + } + +} diff --git a/spring-core/src/test/resources/org/springframework/core/io/support/test.properties b/spring-core/src/test/resources/org/springframework/core/io/support/test.properties new file mode 100644 index 00000000000..75456eb3817 --- /dev/null +++ b/spring-core/src/test/resources/org/springframework/core/io/support/test.properties @@ -0,0 +1 @@ +enigma = 42