Short circuit checking of source already covered by ConfigurationPropertySources

[brucelwl]

(edited with updated description)

Currently ConfigurationPropertySources is added to the environment so that relaxed property resolution can work directly. The ConfigurationPropertySources acts as a facade over the existing property sources.

When making a call such as environment.getProperty("test"), if the property is found then ConfigurationPropertySources will return the result. If, however, the property is not found by ConfigurationPropertySources the PropertySourcesPropertyResolver continues to check all subsequent sources. This is a little inefficient for any source that has already been check via ConfigurationPropertySources.

---

Original report:

getProperty(String key, Class<T> targetValueType, boolean resolveNestedPlaceholders) in org.springframework.core.env.PropertySourcesPropertyResolver traverses all PropertySources, so why use ConfigurationPropertySourcesPropertySource to wrap these PropertySources collections, and ConfigurationPropertySourcesPropertySource is traversed as well? So it seems that after ConfigurationPropertySourcesPropertySource has been traversed, it seems meaningless that the remaining elements are traversed, because ConfigurationPropertySourcesPropertySource has been checked

[philwebb]

I'm sorry but I don't understand what you're reporting. If you think you've found a bug can you please provide a sample application that shows the problem.

[brucelwl]

@philwebb I want to know why to call method ConfigurationPropertySources.attach(environment); in method SpringApplication.prepareEnvironment(...) , It just contains the PropertySource collection and then puts it in environment.

So when invoking environment.getProperty("user.password"), query ConfigurationPropertySourcesPropertySource("configurationProperties") first, but if the attribute value cannot be found in configurationProperties, It will not be queried in the rest of the collection, because configurationProperties already contains the following set .

commandLineArgs
servletConfigInitParams
servletContextInitParams
systemProperties
systemEnvironment
random
applicationConfig: [classpath:/application.properties]

I just want to express that because ConfigurationPropertySources.attach(environment); is called, causes traversal lookups in PropertySourcesPropertyResolver.getProperty(...) to be redundant. So I want to know why to call ConfigurationPropertySources.attach(environment);

[philwebb]

I've edited your comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

[philwebb]

@brucelwl The issue tracker isn't the place for these kinds of discussions as It adds a lot of noise for everyone subscribed. We prefer to keep it for bugs or feature requests. Questions like this can be asked at stackoverflow.com or gitter.im.

[brucelwl]

@philwebb I'm not just asking why, it's also a logical error, because it leads to a redundant loop traversal

[philwebb]

@brucelwl I don't believe there's a logical error, but if you can provide a sample that shows what you mean I'm happy to take a look. The ConfigurationPropertySourcesPropertySource allows you to access Spring Boot properties using the standard environment. So for example, you can call environment.getProperty("customer.someExample") and it will find customer.some-example from the application.properties and CUSTOMER_SOMEEXAMPLE from the system environment. The PropertySourcesPropertyResolver class returns on the first non-null result.

[brucelwl]

Please take a look at my demo, https://github.com/brucelwl/demo
the code is very simple is to call environment.getProperty("user.password"), but you may need to add a breakpoint in PropertySourcesPropertyResolver.getProperty(...), at line 81. we can see that the collection this.propertySources contains the following elements:
configurationProperties
commandLineArgs
servletConfigInitParams
servletContextInitParams
systemProperties
systemEnvironment
random
applicationConfig: [classpath:/application.properties]

But we can see that the first PropertySource configurationProperties also contains the following collection elements:
commandLineArgs
servletConfigInitParams
servletContextInitParams
systemProperties
systemEnvironment
random
applicationConfig: [classpath:/application.properties]

When using a loop to look up a key-value pair from each propertySource, if it can't be found from the first PropertySource configurationProperties, then the rest of the PropertySource will definitely not be found, so there is no need to continue looking

[philwebb]

@brucelwl Thanks for the sample, I see what you're getting at now. The checking of subsequent property sources is only short circuited if the value is found. If the value is not found then we search the source again.

Unfortunately it's not that easy to fix this one. We might be able to improve things if we can change PropertySourcesPropertyResolver in Spring Framework to somehow be aware that a property source also covers other sources.

[dngzs]

Excuse me, have you fixed it?

[wilkinsona]

No, not yet. This issue is still open and currently assigned to the general backlog.

[dngzs]

Did you finally fix it with cache？

[philwebb]

@dngzs The issue is still open