GH-1879: Factories Now Configure (De)Serializers #1907
Conversation
Resolves spring-projects#1879 It was not intuitive that configuration properties were not applied when construcing (De)Serializers programmatically. The producer and consumer factories now call the `configure()` method. However, the JSON implementations cannot be configured with a mixture of setters and configuration properties.
| } | ||
|
|
||
| @Override | ||
| public void configure(Map<String, ?> configs, boolean isKey) { | ||
| if (this.configured) { | ||
| return; |
There was a problem hiding this comment.
May be better to throw an exception (or at least WARN) to let end-users to know that they are wrong?
There was a problem hiding this comment.
We can't do that; because the supplier is called each time a new consumer is created. The default suppliers return the same instance each time.
I could add a DEBUG log; WDYT?
There was a problem hiding this comment.
Got it. Then WARN anyway: people have to know that they are doing something wrong.
With debug it may slip out and then we will get some issue that they config doesn't work this or that way.
There was a problem hiding this comment.
Hmmm - warn would be too much noise - maybe I should use 2 flags, one for setters (and throw exception) and one for configAlreadyDone?
There was a problem hiding this comment.
Or, maybe just close this as won't fix?
There was a problem hiding this comment.
It will be, but it will say you that there is something wrong with the app. Then you go to docs and JavaDocs and you see that you can't combine setters configuration with properties one.
Isn't the the reason why we use that Assert all around?
There was a problem hiding this comment.
My concern is a single shared version is used and if we call configure() multiple times, even though the complete operation is idempotent, the configuration will be in an indeterminate state while configure() is running, and another thread might be using it to deserialize.
I therefore only want to run configure() once.
Hence my suggestion to use 2 flags instead of 1.
There was a problem hiding this comment.
Can you make it working with two flags, please?
I would say the option to reject is not appropriate since such a feature have been asked already not one time...
Thanks
(or we just can go with your current solution without any warning or error!)
There was a problem hiding this comment.
Working on polishing it...
| } | ||
|
|
||
| @Override | ||
| public void configure(Map<String, ?> configs, boolean isKey) { | ||
| if (this.configured) { | ||
| return; |
Resolves #1879
It was not intuitive that configuration properties were not applied when
construcing (De)Serializers programmatically.
The producer and consumer factories now call the
configure()method.However, the JSON implementations cannot be configured with a mixture
of setters and configuration properties.