From 42e556dd73853e9642886f00a7b54326f2e092a8 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 7 Jul 2021 15:41:58 -0700 Subject: [PATCH] Handle file with name matching an optional wildcard location Previously, the presence of a file with the same name as an optional wildcard location would cause a failure. With this change the pattern is resolved only if the resource is a directory. Additionally, if an optional wildcard search location that was a file would also fail with an exception. This commit fixes that so that those locations are not resolved. Fixes gh-27120 Fixes gh-27209 --- .../config/LocationResourceLoader.java | 19 ++++++++++--------- .../StandardConfigDataLocationResolver.java | 4 +++- ...ironmentPostProcessorIntegrationTests.java | 12 ++++++++++++ ...andardConfigDataLocationResolverTests.java | 7 +++++++ .../spring-boot/src/test/resources/a-file | 0 5 files changed, 32 insertions(+), 10 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/resources/a-file diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/LocationResourceLoader.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/LocationResourceLoader.java index b3d232cce17..551daf9742f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/LocationResourceLoader.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/LocationResourceLoader.java @@ -95,12 +95,15 @@ class LocationResourceLoader { validatePattern(location, type); String directoryPath = location.substring(0, location.indexOf("*/")); String fileName = location.substring(location.lastIndexOf("/") + 1); - Resource directoryResource = getResource(directoryPath); - if (!directoryResource.exists()) { - return new Resource[] { directoryResource }; + Resource resource = getResource(directoryPath); + if (!resource.exists()) { + return EMPTY_RESOURCES; } - File directory = getDirectory(location, directoryResource); - File[] subDirectories = directory.listFiles(this::isVisibleDirectory); + File file = getFile(location, resource); + if (!file.isDirectory()) { + return EMPTY_RESOURCES; + } + File[] subDirectories = file.listFiles(this::isVisibleDirectory); if (subDirectories == null) { return EMPTY_RESOURCES; } @@ -131,11 +134,9 @@ class LocationResourceLoader { Assert.state(directoryPath.endsWith("*/"), () -> String.format("Location '%s' must end with '*/'", location)); } - private File getDirectory(String patternLocation, Resource resource) { + private File getFile(String patternLocation, Resource resource) { try { - File directory = resource.getFile(); - Assert.state(directory.isDirectory(), () -> "'" + directory + "' is not a directory"); - return directory; + return resource.getFile(); } catch (Exception ex) { throw new IllegalStateException( diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java index 5cba39ba03f..1002ef62331 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/StandardConfigDataLocationResolver.java @@ -257,7 +257,9 @@ public class StandardConfigDataLocationResolver Set references) { Set empty = new LinkedHashSet<>(); for (StandardConfigDataReference reference : references) { - empty.addAll(resolveEmptyDirectories(reference)); + if (reference.getDirectory() != null) { + empty.addAll(resolveEmptyDirectories(reference)); + } } return empty; } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java index 6d2c1c7d7dc..2c28404b031 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java @@ -709,6 +709,18 @@ class ConfigDataEnvironmentPostProcessorIntegrationTests { assertThat(environment.getProperty("second.property")).isEqualTo("ball"); } + @Test + void runWhenOptionalWildcardLocationDoesNotExistDoesNotThrowException() { + assertThatNoException().isThrownBy(() -> this.application.run( + "--spring.config.location=optional:file:src/test/resources/nonexistent/*/testproperties.properties")); + } + + @Test + void runWhenMandatoryWildcardLocationDoesNotExistThrowsException() { + assertThatExceptionOfType(ConfigDataLocationNotFoundException.class).isThrownBy(() -> this.application + .run("--spring.config.location=file:src/test/resources/nonexistent/*/testproperties.properties")); + } + @Test void runWhenMandatoryWildcardLocationHasEmptyFileDirectory() { assertThatNoException() diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java index d41f3552167..cfca9fbe0ed 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/StandardConfigDataLocationResolverTests.java @@ -34,6 +34,7 @@ import org.springframework.mock.env.MockEnvironment; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -151,6 +152,12 @@ public class StandardConfigDataLocationResolverTests { filePath("src", "test", "resources", "config", "2-second", "testproperties.properties")); } + @Test + void resolveWhenLocationIsWildcardAndMatchingFilePresentShouldNotFail() { + ConfigDataLocation location = ConfigDataLocation.of("optional:file:src/test/resources/a-file/*/"); + assertThatNoException().isThrownBy(() -> this.resolver.resolve(this.context, location)); + } + @Test void resolveWhenLocationIsWildcardFilesLoadsAllFilesThatMatch() { ConfigDataLocation location = ConfigDataLocation diff --git a/spring-boot-project/spring-boot/src/test/resources/a-file b/spring-boot-project/spring-boot/src/test/resources/a-file new file mode 100644 index 00000000000..e69de29bb2d