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

GH-3696: DeserializationEx support for KafkaMS #8689

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

artembilan
Copy link
Member

Fixes #3696

  • Add an internal logic into KafkaMessageSource to react properly for the ErrorHandlingDeserializer configuration and re-throw DeserializationException

Fixes spring-projects#3696

* Add an internal logic into `KafkaMessageSource` to react properly for the
`ErrorHandlingDeserializer` configuration and re-throw `DeserializationException`
@artembilan artembilan requested a review from garyrussell July 26, 2023 18:20
@artembilan
Copy link
Member Author

In general just copy/pasted the logic from the KafkaMessageListenerContainer.
Not sure what could be done better though...

Thanks

}

@Nullable
private Object findDeserializerClass(Map<String, Object> props, Properties consumerOverrides, boolean isValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will move most of the duplicated code to SK.ErrorHandlingUtils; then you can use it from there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Test
void deserializationErrorIsThrownFromSource() throws Exception {
Map<String, Object> consumerProps = KafkaTestUtils.consumerProps(brokers, "testErrorChannelSource", "false");
consumerProps.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this is now the default in KTU.

String testData = "test data";
template.send(TOPIC2, testData);

source.receive(); // Trigger Kafka Consumer creation and poll()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure, but the record might be returned by this initial poll.

It would be safer to remove this and move the following assertion into the untilAsserted() -> {...} block.

no reason to wait for assignment or check for returned record.
We just need to be sure that `DeserializationException` is thrown eventually
@artembilan artembilan requested a review from garyrussell July 26, 2023 20:14
@garyrussell garyrussell merged commit b6d39e4 into spring-projects:main Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ErrorHandlingDeserializer in KafkaMessageSource
2 participants