Skip to content

It is recommended that when the parsing value is null, it can be customized by BindHandler #17050

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

Closed
brucelwl opened this issue Jun 3, 2019 · 10 comments
Labels
status: duplicate A duplicate of another issue

Comments

@brucelwl
Copy link

brucelwl commented Jun 3, 2019

org.springframework.boot.context.properties.bind.Binder
image

It is recommended that when the parsing value is null, it can be customized by BindHandler,because when the return value is null, I need to give a default value through BindHandler,the recommended code is modified as follows:
image

The second solution is that the finish method of bindhander can add method return value,the code is as follows:
image

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 3, 2019
@wilkinsona
Copy link
Member

Thanks for the suggestions. Unfortunately, I don't think we can proceed with them as they are both breaking changes:

  1. The result that is passed to onSuccess is documented as never being null.
  2. Changing the runtime type of onFinish from void to Object would break any existing implementations of that method.

Rather than describing possible solutions, can you please take a step back and describe the problem that you are facing?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jun 3, 2019
@brucelwl
Copy link
Author

brucelwl commented Jun 3, 2019

I need to dynamically bind attributes to beans, so I use binder API, but some attributes are not standard field mapping, so I want to use a custom handler to process @Value and assign special attributes.

Look at my demo please. I hope that the slaveJdbcUrl attribute in MyBatis Properties can get the configuration value of mybatis.config.slave.slaveJdbcUrl, and preferably support Spring's EL expression

https://github.com/brucelwl/demo.git

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 3, 2019
@brucelwl
Copy link
Author

brucelwl commented Jun 3, 2019

Do you understand my question? look forward to your reply @wilkinsona @spring-issuemaster

@philwebb
Copy link
Member

philwebb commented Jun 3, 2019

I would not recommend trying to reuse the @Value annotation in the way that you're doing. It's very likely to cause you problems (for example, it doesn't support relaxed binding).

It seems like you're trying to support a form of fallback properties. The user can set mybatis.config.slave-jdbc-url, or leave it blank to have it set to mybatis.config.slave.slave-jdbc-url. Rather than trying to do that in the binder, I'd create some explicit logic for that:

public static String getSlaveJdbcUrl(MyBatisProperties properties) {
    String result = properties.getSlaveJdbcUrl();
    if (result == null) {
        result = properties.getSlave().getSlaveJdbcUrl();
    }
    return result;
}

@philwebb
Copy link
Member

philwebb commented Jun 3, 2019

We're not keen to add additional complexity to the Binder to support fallback functionality. See #7986 for background.

@philwebb philwebb closed this as completed Jun 3, 2019
@philwebb philwebb added status: duplicate A duplicate of another issue and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jun 3, 2019
@brucelwl
Copy link
Author

brucelwl commented Jun 4, 2019

I would not recommend trying to reuse the @Value annotation in the way that you're doing. It's very likely to cause you problems (for example, it doesn't support relaxed binding).

It seems like you're trying to support a form of fallback properties. The user can set mybatis.config.slave-jdbc-url, or leave it blank to have it set to mybatis.config.slave.slave-jdbc-url. Rather than trying to do that in the binder, I'd create some explicit logic for that:

public static String getSlaveJdbcUrl(MyBatisProperties properties) {
    String result = properties.getSlaveJdbcUrl();
    if (result == null) {
        result = properties.getSlave().getSlaveJdbcUrl();
    }
    return result;
}

@philwebb Sorry, I don't think this is a good solution to my problem, because I may have to write a lot of such logic code, I want to call Binder API binding dynamically, so @ConfigurationProperties can't solve my problem either. Do you think it's feasible to change the name attribute through the handler`s start method?

@philwebb
Copy link
Member

philwebb commented Jun 4, 2019

Do you think it's feasible to change the name attribute through the handler`s start method?

You mean the onStart method? I don't see how we could do that.

Perhaps you can subclass the Binder and override the following protected method to do what you need:

protected final <T> T bind(ConfigurationPropertyName name, Bindable<T> target,
		BindHandler handler, Context context, boolean allowRecursiveBinding) {

@brucelwl
Copy link
Author

brucelwl commented Jun 4, 2019

@philwebb Thank you for your reply, but the final method cannot be rewritten. I've implemented my requirements through two solutions.
The first one is to rewrite attribute values using reflection through Binder Handler's onStart method, but it can't support nested attributes.
The second one is more violent, replicates a lot of spring source code, but supports embedded attributes. I hope you can consider my second one and modify the interface of binderHandler. Method, see my custom 'MyBinderHandler', add a return value to the onFinish method, and add the onNoDescendSkip method.
I recommend expanding the access rights of the classes under package'org. spring framework. boot. context. properties. bind'so that there is no need to duplicate too much redundant code
Please take a look at my demo: https://github.com/brucelwl/demo.git

@philwebb
Copy link
Member

philwebb commented Jun 4, 2019

I recommend expanding the access rights of the classes under package'org. spring framework. boot. context. properties. bind'so that there is no need to duplicate too much redundant code

I'm sorry, but we don't want to expand the surface area of the public API. Your specific use-case is quite unusual and as such I think that maintaining your own copy of the code is the best approach.

@brucelwl
Copy link
Author

brucelwl commented Jun 5, 2019

@philwebb Thank you for your reply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

4 participants