GH-1494: Fix NPE in DeadLetterPublishingRecoverer

Resolves #1494

Author: garyrussell

spring-kafka/src/main/java/org/springframework/kafka/listener/DeadLetterPublishingRecoverer.java

@@ -98,7 +98,9 @@ public DeadLetterPublishingRecoverer(KafkaTemplate<? extends Object, ? extends O
 	 * original topic. The templates map keys are classes and the value the corresponding
 	 * template to use for objects (producer record values) of that type. A
 	 * {@link java.util.LinkedHashMap} is recommended when there is more than one
-	 * template, to ensure the map is traversed in order.
+	 * template, to ensure the map is traversed in order. To send records with a null
+	 * value, add a template with the {@link Void} class as a key; otherwise the first
+	 * template from the map values iterator will be used.
 	 * @param templates the {@link KafkaTemplate}s to use for publishing.
 	 */
 	public DeadLetterPublishingRecoverer(Map<Class<?>, KafkaTemplate<? extends Object, ? extends Object>> templates) {
@@ -112,7 +114,9 @@ public DeadLetterPublishingRecoverer(Map<Class<?>, KafkaTemplate<? extends Objec
 	 * 0, no partition is set when publishing to the topic. The templates map keys are
 	 * classes and the value the corresponding template to use for objects (producer
 	 * record values) of that type. A {@link java.util.LinkedHashMap} is recommended when
-	 * there is more than one template, to ensure the map is traversed in order.
+	 * there is more than one template, to ensure the map is traversed in order. To send
+	 * records with a null value, add a template with the {@link Void} class as a key;
+	 * otherwise the first template from the map values iterator will be used.
 	 * @param templates the {@link KafkaTemplate}s to use for publishing.
 	 * @param destinationResolver the resolving function.
 	 */
@@ -161,10 +165,19 @@ public void accept(ConsumerRecord<?, ?> record, Exception exception) {
 	}
 
 	@SuppressWarnings("unchecked")
-	private KafkaTemplate<Object, Object> findTemplateForValue(Object value) {
+	private KafkaTemplate<Object, Object> findTemplateForValue(@Nullable Object value) {
 		if (this.template != null) {
 			return this.template;
 		}
+		if (value == null) {
+			KafkaOperations<?, ?> operations = this.templates.get(Void.class);
+			if (operations == null) {
+				return (KafkaTemplate<Object, Object>) this.templates.values().iterator().next();
+			}
+			else {
+				return (KafkaTemplate<Object, Object>) operations;
+			}
+		}
 		Optional<Class<?>> key = this.templates.keySet()
 			.stream()
 			.filter((k) -> k.isAssignableFrom(value.getClass()))

spring-kafka/src/test/java/org/springframework/kafka/listener/DeadLetterPublishingRecovererTests.java

@@ -23,12 +23,16 @@
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
+import java.util.LinkedHashMap;
+import java.util.Map;
+
 import org.apache.kafka.clients.consumer.ConsumerRecord;
 import org.apache.kafka.clients.producer.ProducerRecord;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.kafka.core.KafkaOperations.OperationsCallback;
 import org.springframework.kafka.core.KafkaTemplate;
+import org.springframework.util.concurrent.SettableListenableFuture;
 
 /**
  * @author Gary Russell
@@ -37,38 +41,41 @@
  */
 public class DeadLetterPublishingRecovererTests {
 
-	@SuppressWarnings("unchecked")
+	@SuppressWarnings({ "unchecked", "rawtypes" })
 	@Test
 	void testTxNoTx() {
 		KafkaTemplate<?, ?> template = mock(KafkaTemplate.class);
 		given(template.isTransactional()).willReturn(true);
 		given(template.inTransaction()).willReturn(false);
 		given(template.isAllowNonTransactional()).willReturn(true);
+		given(template.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
 		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(template);
 		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", "baz");
 		recoverer.accept(record, new RuntimeException());
 		verify(template, never()).executeInTransaction(any());
 		verify(template).send(any(ProducerRecord.class));
 	}
 
-	@SuppressWarnings("unchecked")
+	@SuppressWarnings({ "unchecked", "rawtypes" })
 	@Test
 	void testTxExisting() {
 		KafkaTemplate<?, ?> template = mock(KafkaTemplate.class);
 		given(template.isTransactional()).willReturn(true);
 		given(template.inTransaction()).willReturn(true);
+		given(template.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
 		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(template);
 		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", "baz");
 		recoverer.accept(record, new RuntimeException());
 		verify(template, never()).executeInTransaction(any());
 		verify(template).send(any(ProducerRecord.class));
 	}
 
-	@SuppressWarnings("unchecked")
+	@SuppressWarnings({ "unchecked", "rawtypes" })
 	@Test
 	void testNonTx() {
 		KafkaTemplate<?, ?> template = mock(KafkaTemplate.class);
 		given(template.isTransactional()).willReturn(false);
+		given(template.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
 		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(template);
 		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", "baz");
 		recoverer.accept(record, new RuntimeException());
@@ -88,11 +95,42 @@ void testTxNewTx() {
 			((OperationsCallback) inv.getArgument(0)).doInOperations(template);
 			return null;
 		}).given(template).executeInTransaction(any());
+		given(template.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
 		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(template);
 		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", "baz");
 		recoverer.accept(record, new RuntimeException());
 		verify(template).executeInTransaction(any());
 		verify(template).send(any(ProducerRecord.class));
 	}
 
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@Test
+	void tombstoneWithMultiTemplates() {
+		KafkaTemplate<?, ?> template1 = mock(KafkaTemplate.class);
+		given(template1.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
+		KafkaTemplate<?, ?> template2 = mock(KafkaTemplate.class);
+		Map<Class<?>, KafkaTemplate<?, ?>> templates = new LinkedHashMap<>();
+		templates.put(String.class, template1);
+		templates.put(Integer.class, template2);
+		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(templates);
+		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", null);
+		recoverer.accept(record, new RuntimeException());
+		verify(template1).send(any(ProducerRecord.class));
+	}
+
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@Test
+	void tombstoneWithMultiTemplatesExplicit() {
+		KafkaTemplate<?, ?> template1 = mock(KafkaTemplate.class);
+		KafkaTemplate<?, ?> template2 = mock(KafkaTemplate.class);
+		given(template2.send(any(ProducerRecord.class))).willReturn(new SettableListenableFuture());
+		Map<Class<?>, KafkaTemplate<?, ?>> templates = new LinkedHashMap<>();
+		templates.put(String.class, template1);
+		templates.put(Void.class, template2);
+		DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer(templates);
+		ConsumerRecord<String, String> record = new ConsumerRecord<>("foo", 0, 0L, "bar", null);
+		recoverer.accept(record, new RuntimeException());
+		verify(template2).send(any(ProducerRecord.class));
+	}
+
 }

src/reference/asciidoc/kafka.adoc

@@ -3944,6 +3944,8 @@ public DeadLetterPublishingRecoverer publisher(KafkaTemplate<?, ?> stringTemplat
 The publisher uses the map keys to locate a template that is suitable for the `value()` about to be published.
 A `LinkedHashMap` is recommended so that the keys are examined in order.
 
+When publishing `null` values, when there are multiple templates, the recoverer will look for a template for the `Void` class; if none is present, the first template from the `values().iterator()` will be used.
+
 IMPORTANT: If the recoverer fails (throws an exception), the record will be included in the seeks and recovery will be attempted again during the next delivery.
 
 Starting with version 2.3, the recoverer can also be used with Kafka Streams - see <<streams-deser-recovery>> for more information.