Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow PropertySourcesPlaceholderConfigurer subclass to customize PropertyResolver #26761

Closed
wants to merge 1 commit into from

Conversation

1zg12
Copy link
Contributor

@1zg12 1zg12 commented Apr 5, 2021

Background

Prior to Spring 5.2, when we need to customize the PropertiesLoader, we can subclass PropertyPlaceholderConfigurer, and override the resolvePlaceholder method.

Now with PropertySourcesPlaceholderConfigurer deprecated PropertyPlaceholderConfigurer, the resolvePlaceholder method has been moved out of the PlaceHolderConfigurer and into a separate PropertyResolver.

Somehow the PropertyResolver is hardcoded [*]:

processProperties(beanFactory, new PropertySourcesPropertyResolver(this.propertySources));

Due to the hardcode, the only way left for developers to override the resolvePlaceholder now becomes to duplicate the whole postProcessBeanFactory method and then provide custom PropertyResolver.

Changes

Instead of hardcoding a PropertyResolver as existing and make it very difficult to customize, this PR will point to a factory method to provide the PropertyResolver as needed.

I believe there is no side effect on this. It only adds the flexibility for customization which has been lost.

Note [*]

Even though PropertyPlaceholderConfigurer was also using a hardcoded class,

StringValueResolver valueResolver = new PlaceholderResolvingStringValueResolver(props);

It's a private inner class, which even doesn't allow override. However, after several redirects, it eventually points to the resolvePlaceholder of the current class implementation

return PropertyPlaceholderConfigurer.this.resolvePlaceholder(placeholderName,

@pivotal-issuemaster
Copy link

@1wpro2 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@1wpro2 Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 5, 2021
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 11, 2021
@pivotal-cla
Copy link

@1zg12 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@1zg12 Thank you for signing the Contributor License Agreement!

@1zg12
Copy link
Contributor Author

1zg12 commented May 25, 2022

hi @rstoyanchev , can you review this ?

@1zg12
Copy link
Contributor Author

1zg12 commented Sep 1, 2022

Bump up this, as we are still having issue to overwrite this setting in the newer version of spring. Thanks.

@snicoll snicoll changed the title Make the property resolver configurable PropertySourcesPropertyResolver no longer allows customization of PropertyResolver Aug 26, 2023
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 26, 2023
@snicoll snicoll self-assigned this Aug 26, 2023
@snicoll snicoll added this to the 6.0.12 milestone Aug 26, 2023
snicoll pushed a commit that referenced this pull request Aug 26, 2023
This commit reintroduces the ability to customize the PropertyResolver
to use in PropertySourcesPropertyResolver

See gh-26761
snicoll added a commit that referenced this pull request Aug 26, 2023
@snicoll snicoll closed this in 2b3539a Aug 26, 2023
@snicoll
Copy link
Member

snicoll commented Aug 26, 2023

@1zg12 thanks very much for making your first contribution to the Spring Framework.

@sbrannen sbrannen changed the title PropertySourcesPropertyResolver no longer allows customization of PropertyResolver Allow PropertySourcesPlaceholderConfigurer to customize PropertyResolver Aug 27, 2023
@sbrannen sbrannen changed the title Allow PropertySourcesPlaceholderConfigurer to customize PropertyResolver Allow PropertySourcesPlaceholderConfigurer subclass to customize PropertyResolver Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants