Skip to content

Commit 797b96e

Browse files
committed
GH-3953: Addressing PR review comments
Fixes: #3953 **Auto-cherry-pick to `3.3.x` & `3.2.x`**" Signed-off-by: Soby Chacko <[email protected]>
1 parent df5860d commit 797b96e

File tree

3 files changed

+39
-71
lines changed

3 files changed

+39
-71
lines changed

spring-kafka/src/main/java/org/springframework/kafka/support/serializer/DelegatingByTypeSerializer.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package org.springframework.kafka.support.serializer;
1818

19-
import java.util.*;
19+
import java.util.Comparator;
20+
import java.util.Map;
2021
import java.util.Map.Entry;
22+
import java.util.TreeMap;
2123

2224
import org.apache.kafka.common.errors.SerializationException;
2325
import org.apache.kafka.common.header.Headers;
@@ -39,16 +41,18 @@
3941
public class DelegatingByTypeSerializer implements Serializer<Object> {
4042

4143
private static final Comparator<Class<?>> DELEGATES_ASSIGNABILITY_COMPARATOR =
42-
(type1, type2) -> {
44+
(class1, class2) -> {
4345

44-
if (type1.isAssignableFrom(type2)) {
45-
return 1;
46+
if (class1 == class2) {
47+
return 0; // Classes are the same
4648
}
47-
if (type2.isAssignableFrom(type1)) {
48-
return -1;
49+
if (class1.isAssignableFrom(class2)) {
50+
return 1; // class2 is a superclass or superinterface of class1, so class2 should come first
4951
}
50-
51-
return 0;
52+
if (class2.isAssignableFrom(class1)) {
53+
return -1; // class1 is a superclass or superinterface of class2, so class1 should come first
54+
}
55+
return class1.getName().compareTo(class2.getName()); // If no inheritance relation, compare by name
5256
};
5357

5458
private final Map<Class<?>, Serializer<?>> delegates = new TreeMap<>(DELEGATES_ASSIGNABILITY_COMPARATOR);
@@ -70,11 +74,9 @@ public DelegatingByTypeSerializer(Map<Class<?>, Serializer<?>> delegates) {
7074
* is assignable from the target object's class. When multiple matches are possible,
7175
* the most specific matching class is selected — that is, the closest match in the
7276
* class hierarchy.
73-
*
7477
* @param delegates the delegates
7578
* @param assignable true if {@link #findDelegate(Object, Map)} should consider assignability to
7679
* the key rather than an exact match.
77-
*
7880
* @since 2.8.3
7981
*/
8082
public DelegatingByTypeSerializer(Map<Class<?>, Serializer<?>> delegates, boolean assignable) {

spring-kafka/src/test/java/org/springframework/kafka/support/serializer/DelegatingByTypeSerializerTests.java

Lines changed: 0 additions & 61 deletions
This file was deleted.

spring-kafka/src/test/java/org/springframework/kafka/support/serializer/DelegatingSerializationTests.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.nio.ByteBuffer;
2020
import java.util.Collections;
2121
import java.util.HashMap;
22+
import java.util.LinkedHashMap;
2223
import java.util.Map;
2324

2425
import org.apache.kafka.common.errors.SerializationException;
@@ -41,6 +42,7 @@
4142

4243
import static org.assertj.core.api.Assertions.assertThat;
4344
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
45+
import static org.mockito.Mockito.mock;
4446
import static org.mockito.Mockito.spy;
4547
import static org.mockito.Mockito.times;
4648
import static org.mockito.Mockito.verify;
@@ -122,6 +124,31 @@ void testWithPropertyConfigKeys() {
122124
doTestKeys(serializer, deserializer);
123125
}
124126

127+
@Test
128+
void shouldOrderDelegatesSoChildComesBeforeParent() {
129+
class Parent { }
130+
131+
class Child extends Parent { }
132+
133+
Serializer<?> mockParentSerializer = mock(Serializer.class);
134+
Serializer<?> mockChildSerializer = mock(Serializer.class);
135+
136+
// Using LinkedHashMap to ensure the order is always wrong
137+
Map<Class<?>, Serializer<?>> delegates = new LinkedHashMap<>();
138+
delegates.put(Parent.class, mockParentSerializer);
139+
delegates.put(Child.class, mockChildSerializer);
140+
141+
DelegatingByTypeSerializer serializer = new DelegatingByTypeSerializer(delegates, true);
142+
143+
144+
Serializer<?> childSerializer = serializer.findDelegate(mock(Child.class));
145+
Serializer<?> parentSerializer = serializer.findDelegate(mock(Parent.class));
146+
147+
148+
assertThat(childSerializer).isEqualTo(mockChildSerializer);
149+
assertThat(parentSerializer).isEqualTo(mockParentSerializer);
150+
}
151+
125152
private void doTest(DelegatingSerializer serializer, DelegatingDeserializer deserializer) {
126153
Headers headers = new RecordHeaders();
127154
headers.add(new RecordHeader(DelegatingSerializer.VALUE_SERIALIZATION_SELECTOR, "bytes".getBytes()));

0 commit comments

Comments
 (0)