Spring-boot: spring.config.name and spring.config.location are inconsistent; former replaces defaults but latter does not

Created on 11 Oct 2017  路  9Comments  路  Source: spring-projects/spring-boot

Spring Boot 2.0.0.SNAPSHOT

Hello,

I am now spending more than a day on a behaviour that I initially thought is a bug in Spring Boot, however now after I had a deeper look into ConfigFileApplicationListener I am not sure if it is more a misleading documentation than a bug.

When specifying an _alternative_ application.properties file by adding the commandline param
--spring.config.location=classpath:myApplication.properties, Spring Boot adds all the default locations anyhow since there is no else statement specified in this method in ConfigFileApplicationListener$Loader:

       private Set<String> getSearchLocations() {
        Set<String> locations = new LinkedHashSet<>();
        // User-configured settings take precedence, so we do them first
        if (this.environment.containsProperty(CONFIG_LOCATION_PROPERTY)) {
            for (String path : asResolvedSet(
                    this.environment.getProperty(CONFIG_LOCATION_PROPERTY), null)) {
                if (!path.contains("$")) {
                    path = StringUtils.cleanPath(path);
                    if (!ResourceUtils.isUrl(path)) {
                        path = ResourceUtils.FILE_URL_PREFIX + path;
                    }
                }
                locations.add(path);
            }
        }
                // Shouldn't here be an else condition?
        locations.addAll(
                asResolvedSet(ConfigFileApplicationListener.this.searchLocations,
                        DEFAULT_SEARCH_LOCATIONS));
        return locations;
    }

This leads to the effect that all application.properties props are loaded as well as the ones from the alternative specified source and I think this is not desired semantic, since the regular Dev understands the Spring Boot common properties as an Replacement rather than an extension of certain defaults (and indeed afaik all properties are implemented like that, e. g. logging.config

So in my opinion the whole property should act more as an replacement rather than a opportunity to specify additional config locations.

If this behaviour is -against my personal expectation- desired like that though, I would appreciate to have an explicit hint in Common application properties, or even better a config for that to indicate replacement over merging:

# SPRING CONFIG - using environment property only (ConfigFileApplicationListener)
spring.config.location= # Additional config file locations.
spring.config.name=application # Config file name.
spring.config.merge= # true if config specified above should merge with default configs, false if above specified configuration acts as replacement

Kind regards,

Stefano

bug

Most helpful comment

What Phil has described is what I was leaning towards. However, I don't like spring.config.include as it's not clear that it's a location that's being included. DevTools has a spring.devtools.restart.additional-exclude property so how about spring.config.additional-location?

All 9 comments

The behaviour is already described in the documentation where it says:

When custom config locations are configured, they are used in addition to the default locations. Custom locations are searched before the default locations.

I am not keen on the additional complexity of adding a configuration option for controlling how configuration is loaded, but let's see what the rest of the team think.

Hello @wilkinsona ,
for me everything is fine, I can live with that by just explicitly overwriting undesired properties with empty values, however I would appreciate a lot if there would be a more prominent documentation hint, since its a little bit lonely in the middle of the section there and also cannot be noticed at all when you take the Common application properties page as a browser bookmark (as a quick reference guide).

The "additional" word in the inline documentation would have prevented me to do lot of debugging in Spring code (however it would have been clever to first read deeper in documentation, thats for sure)

So instead

spring.config.location= # Config file locations.

this:

spring.config.location= # Additional config file locations. Configurations from these files overwrite the ones from the default locations.

It feels to use like the configuration being additive is a bug. It should be possible to completely override the locations (IIRC, the names can be overridden). That'll be a potentially breaking change for some so we'll tackle this in 2.0.

I'm wavering on this. Here are the proposed changes. Thoughts?

@wilkinsona LGTM. I've made a couple of comments on the commit. Thanks!

I've got mixed feelings about this. On one hand I like the consistency with spring.config.name, but on the other hand it's really hard for the user to include one additional location. Expecting people to remember that they need type --spring.config.location=classpath:myApplication.properties,classpath:/,classpath:/config/,file:./,file:./config/ to get the existing behavior isn't great.

Perhaps we should add a spring.config.include property (similar to spring.profiles.include) to allow the existing use-case?

So --spring.config.location=classpath:myApplication.properties will replace the default locations but --spring.config.include=classpath:myApplication.properties will add to them?

include-location ?

What Phil has described is what I was leaning towards. However, I don't like spring.config.include as it's not clear that it's a location that's being included. DevTools has a spring.devtools.restart.additional-exclude property so how about spring.config.additional-location?

Yeah, that's a lot better as a name.

Was this page helpful?
0 / 5 - 0 ratings