Fix issue with batchHandler not being called without also using handler

danielbcorreia commented 1 year ago

Motivation:

The Kafka Client API provides two ways of processing records, using the handler and batchHandler methods. It turns out that batchHandler cannot be used independently in the current implementation, since a call to handler (even if the function itself does nothing) is required in order for the batchHandler to be called.

The same issue in #170.

This fix changes the following:

null check on schedule now takes into account both handler types
to preserve the same guarantees provided for handler, the batchHandler reference is now captured in schedule and passed down to the run method.
the test for batchHandler no longer calls handler (verified that this makes the test time out without the actual fix)

The name multiHandler on the schedule method is not great but I didn't want to shadow the field name, I'm open to suggestions.

vietj commented 1 year ago

can you add a test for this ?

danielbcorreia commented 1 year ago

can you add a test for this ?

hey @vietj , there was testConsumerBatchHandler already, but it was calling handler (thus bypassing the issue) - I fixed that as well.

vietj commented 1 year ago

do you mind having a look @ppatierno ?

tmbull commented 7 months ago

@ppatierno @vietj can this be merged in? I recently ran into this issue as well.

vietj commented 7 months ago

@danielbcorreia can you provide a backport PR for the 4.x branch ?

vert-x3 / vertx-kafka-client

Fix issue with batchHandler not being called without also using handler #250