From bd2bf09d1e3cf5cd6b1977e24d151100da6c5072 Mon Sep 17 00:00:00 2001 From: Soby Chacko Date: Thu, 16 May 2024 11:56:55 -0400 Subject: [PATCH] GH-2951: Batch consumer and DLQ issues Batch consumer with `ListenerContainerWithDlqAndRetryCustomizer` should disable binder-based DLQ. Resolves https://github.com/spring-cloud/spring-cloud-stream/issues/2951 * Addressing PR review comments * Addressing review --- .../kafka/KafkaMessageChannelBinder.java | 9 +- ...thDlqCustomizerDisablesBinderDlqTests.java | 87 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 binders/kafka-binder/spring-cloud-stream-binder-kafka/src/test/java/org/springframework/cloud/stream/binder/kafka/BatchWithDlqCustomizerDisablesBinderDlqTests.java diff --git a/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/main/java/org/springframework/cloud/stream/binder/kafka/KafkaMessageChannelBinder.java b/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/main/java/org/springframework/cloud/stream/binder/kafka/KafkaMessageChannelBinder.java index 663a941c9..2e1249ae8 100644 --- a/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/main/java/org/springframework/cloud/stream/binder/kafka/KafkaMessageChannelBinder.java +++ b/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/main/java/org/springframework/cloud/stream/binder/kafka/KafkaMessageChannelBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2023 the original author or authors. + * Copyright 2014-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1077,6 +1077,11 @@ protected ErrorMessageStrategy getErrorMessageStrategy() { return new RawRecordHeaderErrorMessageStrategy(); } + private Boolean isBatchAndListenerContainerWithDlqAndRetryCustomizer(ExtendedConsumerProperties properties) { + ListenerContainerCustomizer customizer = getContainerCustomizer(); + return properties.isBatchMode() && customizer instanceof ListenerContainerWithDlqAndRetryCustomizer; + } + @SuppressWarnings("unchecked") @Override protected MessageHandler getErrorMessageHandler(final ConsumerDestination destination, @@ -1084,7 +1089,7 @@ protected MessageHandler getErrorMessageHandler(final ConsumerDestination destin final ExtendedConsumerProperties properties) { KafkaConsumerProperties kafkaConsumerProperties = properties.getExtension(); - if (kafkaConsumerProperties.isEnableDlq()) { + if (kafkaConsumerProperties.isEnableDlq() && !isBatchAndListenerContainerWithDlqAndRetryCustomizer(properties)) { KafkaProducerProperties dlqProducerProperties = kafkaConsumerProperties .getDlqProducerProperties(); KafkaAwareTransactionManager transMan = transactionManager( diff --git a/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/test/java/org/springframework/cloud/stream/binder/kafka/BatchWithDlqCustomizerDisablesBinderDlqTests.java b/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/test/java/org/springframework/cloud/stream/binder/kafka/BatchWithDlqCustomizerDisablesBinderDlqTests.java new file mode 100644 index 000000000..65fd56f2b --- /dev/null +++ b/binders/kafka-binder/spring-cloud-stream-binder-kafka/src/test/java/org/springframework/cloud/stream/binder/kafka/BatchWithDlqCustomizerDisablesBinderDlqTests.java @@ -0,0 +1,87 @@ +/* + * Copyright 2024-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.stream.binder.kafka; + +import java.util.List; +import java.util.function.Consumer; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.stream.binder.DefaultBinderFactory; +import org.springframework.cloud.stream.binder.ExtendedConsumerProperties; +import org.springframework.cloud.stream.binder.kafka.properties.KafkaConsumerProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.kafka.test.context.EmbeddedKafka; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; +import org.springframework.messaging.MessageHandler; +import org.springframework.test.annotation.DirtiesContext; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Soby Chacko + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, properties = { + "spring.cloud.function.definition=consumer1", + "spring.cloud.stream.bindings.consumer-in-0.group=batchWithDlqCustomizerDisablesBinderDlq", + "spring.cloud.stream.kafka.bindings.consumer1-in-0.consumer.enable-dlq=true"}) +@EmbeddedKafka +@DirtiesContext +public class BatchWithDlqCustomizerDisablesBinderDlqTests { + + @Autowired + private DefaultBinderFactory binderFactory; + + @Test + void batchWithDlqCustomizerDisablesBinderDlq() { + KafkaMessageChannelBinder kafka = (KafkaMessageChannelBinder) this.binderFactory.getBinder("kafka", MessageChannel.class); + + KafkaConsumerProperties kafkaConsumerProperties = + kafka.getExtendedConsumerProperties("consumer1-in-0"); + ExtendedConsumerProperties extendedConsumerProperties = + new ExtendedConsumerProperties<>(kafkaConsumerProperties); + extendedConsumerProperties.setBatchMode(true); + + MessageHandler errorMessageHandler = + kafka.getErrorMessageHandler(null, null, extendedConsumerProperties); + // verifies that binder does not create a message handler for errors, which otherwise creates a handler for DLQ. + assertThat(errorMessageHandler).isNull(); + } + + @EnableAutoConfiguration + @Configuration + public static class BatchWithDlqDisablesBinderDlqTestsConfig { + + @Bean + Consumer>> consumer1() { + return message -> { + }; + } + + @Bean + ListenerContainerWithDlqAndRetryCustomizer customizer() { + return (container, destinationName, group, dlqDestinationResolver, backOff) -> { + }; + } + } + +}