Skip to content

Commit

Permalink
Fix non-optional classpath location checking
Browse files Browse the repository at this point in the history
Allow directory locations that exist but do not contribute properties
to be specified without an `optional:` prefix. This commit fixes logic
introduced in commit 3dc03ac which didn't account for the fact that
a directory might contain only profile specific property files and that
profiles might not always be active.

Closes gh-24499
  • Loading branch information
philwebb committed Dec 15, 2020
1 parent 923ddd3 commit d1f2aab
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 19 deletions.
Expand Up @@ -45,6 +45,11 @@ public final class ConfigData {

private final Set<Option> options;

/**
* A {@link ConfigData} instance that contains no data.
*/
public static final ConfigData EMPTY = new ConfigData(Collections.emptySet());

/**
* Create a new {@link ConfigData} instance.
* @param propertySources the config data property sources in ascending priority
Expand Down
Expand Up @@ -300,6 +300,15 @@ static ConfigDataEnvironmentContributor ofUnboundImport(ConfigDataLocation locat
configurationPropertySource, null, ignoreImports, null);
}

/**
* Factory method to create an {@link Kind#EMPTY_LOCATION empty location} contributor.
* @param location the location of this contributor
* @return a new {@link ConfigDataEnvironmentContributor} instance
*/
static ConfigDataEnvironmentContributor ofEmptyLocation(ConfigDataLocation location) {
return new ConfigDataEnvironmentContributor(Kind.EMPTY_LOCATION, location, null, null, null, null, true, null);
}

/**
* The various kinds of contributor.
*/
Expand Down Expand Up @@ -330,7 +339,12 @@ enum Kind {
* A contributor with {@link ConfigData} imported from another contributor that
* has been.
*/
BOUND_IMPORT;
BOUND_IMPORT,

/**
* A valid location that contained noething to load.
*/
EMPTY_LOCATION;

}

Expand Down
Expand Up @@ -161,10 +161,15 @@ private List<ConfigDataEnvironmentContributor> asContributors(
Map<ConfigDataResolutionResult, ConfigData> imported) {
List<ConfigDataEnvironmentContributor> contributors = new ArrayList<>(imported.size() * 5);
imported.forEach((resolutionResult, data) -> {
for (int i = data.getPropertySources().size() - 1; i >= 0; i--) {
ConfigDataLocation location = resolutionResult.getLocation();
ConfigDataResource resource = resolutionResult.getResource();
contributors.add(ConfigDataEnvironmentContributor.ofUnboundImport(location, resource, data, i));
ConfigDataLocation location = resolutionResult.getLocation();
ConfigDataResource resource = resolutionResult.getResource();
if (data.getPropertySources().isEmpty()) {
contributors.add(ConfigDataEnvironmentContributor.ofEmptyLocation(location));
}
else {
for (int i = data.getPropertySources().size() - 1; i >= 0; i--) {
contributors.add(ConfigDataEnvironmentContributor.ofUnboundImport(location, resource, data, i));
}
}
});
return Collections.unmodifiableList(contributors);
Expand Down
Expand Up @@ -36,6 +36,9 @@ public class StandardConfigDataLoader implements ConfigDataLoader<StandardConfig
@Override
public ConfigData load(ConfigDataLoaderContext context, StandardConfigDataResource resource)
throws IOException, ConfigDataNotFoundException {
if (resource.isEmptyDirectory()) {
return ConfigData.EMPTY;
}
ConfigDataResourceNotFoundException.throwIfDoesNotExist(resource, resource.getResource());
StandardConfigDataReference reference = resource.getReference();
Resource originTrackedResource = OriginTrackedResource.of(resource.getResource(),
Expand Down
Expand Up @@ -17,6 +17,7 @@
package org.springframework.boot.context.config;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -220,25 +221,26 @@ private List<StandardConfigDataResource> resolve(Set<StandardConfigDataReference
resolved.addAll(resolve(reference));
}
if (resolved.isEmpty()) {
assertNonOptionalDirectories(references);
resolved.addAll(resolveEmptyDirectories(references));
}
return resolved;
}

private void assertNonOptionalDirectories(Set<StandardConfigDataReference> references) {
private Collection<StandardConfigDataResource> resolveEmptyDirectories(
Set<StandardConfigDataReference> references) {
Set<StandardConfigDataResource> empty = new LinkedHashSet<>();
for (StandardConfigDataReference reference : references) {
if (reference.isNonOptionalDirectory()) {
assertDirectoryExists(reference);
if (reference.isMandatoryDirectory()) {
Resource resource = this.resourceLoader.getResource(reference.getDirectory());
if (resource instanceof ClassPathResource) {
continue;
}
StandardConfigDataResource configDataResource = new StandardConfigDataResource(reference, resource);
ConfigDataResourceNotFoundException.throwIfDoesNotExist(configDataResource, resource);
empty.add(new StandardConfigDataResource(reference, resource, true));
}
}
}

private void assertDirectoryExists(StandardConfigDataReference reference) {
Resource resource = this.resourceLoader.getResource(reference.getDirectory());
if (!(resource instanceof ClassPathResource)) {
StandardConfigDataResource configDataResource = new StandardConfigDataResource(reference, resource);
ConfigDataResourceNotFoundException.throwIfDoesNotExist(configDataResource, resource);
}
return empty;
}

private List<StandardConfigDataResource> resolve(StandardConfigDataReference reference) {
Expand Down
Expand Up @@ -66,7 +66,7 @@ String getResourceLocation() {
return this.resourceLocation;
}

boolean isNonOptionalDirectory() {
boolean isMandatoryDirectory() {
return !this.configDataLocation.isOptional() && this.directory != null;
}

Expand Down
Expand Up @@ -36,16 +36,29 @@ public class StandardConfigDataResource extends ConfigDataResource {

private final Resource resource;

private boolean emptyDirectory;

/**
* Create a new {@link StandardConfigDataResource} instance.
* @param reference the resource reference
* @param resource the underlying resource
*/
StandardConfigDataResource(StandardConfigDataReference reference, Resource resource) {
this(reference, resource, false);
}

/**
* Create a new {@link StandardConfigDataResource} instance.
* @param reference the resource reference
* @param resource the underlying resource
* @param emptyDirectory if the resource is an empty directory that we know exists
*/
StandardConfigDataResource(StandardConfigDataReference reference, Resource resource, boolean emptyDirectory) {
Assert.notNull(reference, "Reference must not be null");
Assert.notNull(resource, "Resource must not be null");
this.reference = reference;
this.resource = resource;
this.emptyDirectory = emptyDirectory;
}

StandardConfigDataReference getReference() {
Expand All @@ -56,6 +69,10 @@ Resource getResource() {
return this.resource;
}

boolean isEmptyDirectory() {
return this.emptyDirectory;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
Expand All @@ -65,7 +82,7 @@ public boolean equals(Object obj) {
return false;
}
StandardConfigDataResource other = (StandardConfigDataResource) obj;
return this.resource.equals(other.resource);
return this.resource.equals(other.resource) && this.emptyDirectory == other.emptyDirectory;
}

@Override
Expand Down
Expand Up @@ -61,6 +61,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatNoException;

/**
* Integration tests for {@link ConfigDataEnvironmentPostProcessor}.
Expand Down Expand Up @@ -535,6 +536,14 @@ void runWhenConfigLocationHasNonOptionalMissingClasspathDirectoryThrowsLocationN
.isThrownBy(() -> this.application.run("--spring.config.location=" + location));
}

@Test
void runWhenConfigLocationHasNonOptionalEmptyFileDirectoryDoesNotThrowException() {
File location = new File(this.temp, "application.empty");
location.mkdirs();
assertThatNoException().isThrownBy(() -> this.application
.run("--spring.config.location=" + StringUtils.cleanPath(location.getAbsolutePath()) + "/"));
}

@Test
@Disabled("Disabled until spring.profiles suppport is dropped")
void runWhenUsingInvalidPropertyThrowsException() {
Expand Down

0 comments on commit d1f2aab

Please sign in to comment.