Ensure PropertySourcesPlaceholderConfigurer uses the ConversionService from the Environment

[mbazos]

I noticed an issue with PropertySourcesPlaceholderConfigurer moving from spring framework 6.2.6 to 6.2.7.

Basically there is a new propertyResolver being created but it is not setting the environments configured conversion service. In the context of spring boot this could cause some issues if custom converters are registered as the propertyResolver if it doesn't have a conversion service set then uses the default and would not include any registered custom converters with the environment.

I don't think this PR is the exact correct fix and I didn't add any tests but this did resolve the issue with the propertyResolver having the environments conversion service. I figured this would be the best way to show the problem and how to fix it. If I have more time I can do a proper PR but might need some guidance if the fix is not in the right place.

If you look at PropertySourcesPlaceholderConfigurer in spring 6.2.6 you will see that the environments conversion service is set on the propertyResolver.

[sbrannen]

Suggested change

	if (this.environment instanceof ConfigurableEnvironment configurableEnvironment) {
	propertyResolver.setConversionService(configurableEnvironment.getConversionService());
	}

I looked into a possible fix before looking at your proposal, and I first came up with the analogous solution above.

We cannot use ApplicationServletEnvironment here, since that class is specific to Spring Boot and Servlet deployments.

Thus, downcasting to ConfigurableEnvironment using instanceof pattern matching would be the correct way to do that.

[mbazos]

Thanks something didn't feel right with the change I made

[sbrannen]

Hi @mbazos,

Congratulations on submitting your first PR for the Spring Framework! 👍

And thanks for reporting the issue, which appears to be a regression caused by the changes made in conjunction with #34861.

After having pondered it a bit more, I think your proposal (as well as the similar one I posted) may not be the best approach to restoring the original behavior in 6.2.x.

If we set the ConversionService on the supplied ConfigurablePropertyResolver in processProperties(...), that ConversionService would then apply to all PropertySources which would include local properties.

However, that was not the previous behavior.

Previously, the ConversionService used by the Environment was only applied to PropertySources in the Environment, and I plan to reinstate that behavior.

We could consider applying a custom ConversionService to PropertySourcesPlaceholderConfigurer in general, but that would be a new feature and a change in behavior that we would likely only want to apply in Spring Framework 7.0.

---

At this point, I will introduce tests to confirm that this is a regression, and if it's a regression I will reinstate the previous behavior (automatic conversion of values from PropertySources in the Environment using the ConversionService configured in the Environment).

In light of the above, I plan to decline this PR; however, I will leave it open until I have completed the aforementioned tasks.

Cheers,

Sam

[mbazos]

Thanks @sbrannen appreciate the thorough and prompt review. Let me know if you need anything else.