Fix immutability StreamsBuilderFactory properties#4382
Conversation
Signed-off-by: jad837 <jadhavsaurabh037@gmail.com>
615f48d to
d7b8527
Compare
Signed-off-by: jad837 <jadhavsaurabh037@gmail.com>
d7b8527 to
a20e6ed
Compare
artembilan
left a comment
There was a problem hiding this comment.
Any comments why this is in the draft?
Thanks
| @Nullable | ||
| public Properties getStreamsConfiguration() { | ||
| return this.properties; // NOSONAR - inconsistent synchronization | ||
| return (Properties) this.properties.clone(); // NOSONAR - inconsistent synchronization |
There was a problem hiding this comment.
Let's remove that NOSONAR comment!
Not only saying that we don't use it anymore, but also it won't make sense after cloning anyway.
Signed-off-by: jad837 <jadhavsaurabh037@gmail.com>
|
@jad837 , Thank you! |
* Replace ConsumerConfig with StreamsConfig * Update StreamsBuilderFactoryBean logic * Fix broken group protocol test cases Signed-off-by: jad837 <jadhavsaurabh037@gmail.com>
Fixes: #4329 * GH-4329 Added change doc for configuration and whats new * GH-4329 Fixed group protocol not being added to properties. Changed group protocol signature to GroupProtocol enum to ensure that correct group protocol is set for beans. * GH-4329 Updated documentation for Group Protocol usage * GH-4329 added change doc for configuration and whats new * GH-4329 Fixed flaky test due to stream config * GH-4329 Refactor & add warn for group protocol override * Refactor following GH-4382 backport * Replace ConsumerConfig with StreamsConfig * Update StreamsBuilderFactoryBean logic * Fix broken group protocol test cases * Refactor to address review comments * Fix version in docs Signed-off-by: jad837 <jadhavsaurabh037@gmail.com>
|
The changes cause NPE when the properties are not initialized. |
|
Hey @pdevalck is this in case when StreamsBuilderFactoryBean.getProperties() is called? Can you share a bit more on how Can I create this example? I believe this could be when StreamsBuilderFactoryBean is created without any constructor argument... Let me know if I am correct, I can check on this.. |
|
Yes, you are right. In our integration tests, the constructor without arguments is being called to instantiate te factory. When this happens and we happen to get the streams properties, the NPE happens |
Fix immuable object sharing across
StreamsBuilderFactoryBeanAs discussed #4373 (comment), This PR ensures that
StreamsBuilderFactoryBean&KafkaStreamsConfigurationalways sends a new copy of properties, ensuring immutablility.