Ensure correct ordering of delegates regardless of user input #3956
Conversation
When there are multiple serializers available for that a type can be assigned to, the most specific one should be used Signed-off-by: Mahesh Aravind V <[email protected]>
Signed-off-by: Mahesh Aravind V <[email protected]>
Signed-off-by: Mahesh Aravind V <[email protected]>
Signed-off-by: Mahesh Aravind V <[email protected]>
MaheshAravindV
changed the title
...a/src/main/java/org/springframework/kafka/support/serializer/DelegatingByTypeSerializer.java
Outdated
Show resolved
Hide resolved
| * is assignable from the target object's class. When multiple matches are possible, | ||
| * the most specific matching class is selected — that is, the closest match in the | ||
| * class hierarchy. | ||
| * |
There was a problem hiding this comment.
No blank lines in method JavaDocs, please.
You can use spring-framework.xml IntelliJ IDEA editor config from the project to realign with our code style.
| * @author Mahesh Aravind V | ||
| * | ||
| */ | ||
| class DelegatingByTypeSerializerTests { |
There was a problem hiding this comment.
Why do you create a new test class if we have one on the matter already: DelegatingSerializationTests?
Please, consider to add your test over there.
There was a problem hiding this comment.
There is a class DelegatingSerializer and the DelegatingSerializationTests is for tests of that class I assume.
There was a problem hiding this comment.
It is true, but I see tests for DelegatingByTypeSerializer over there, so I would like to have your new one over there as well.
And what is the reason of @Nested?
Cannot we really have it as a top-level test method and have the code flow as simple as it could be?
There was a problem hiding this comment.
@MaheshAravindV It's better to have all the tests in DelegatingSerializationTests. Could you move it there?
| * is assignable from the target object's class. When multiple matches are possible, | ||
| * the most specific matching class is selected — that is, the closest match in the | ||
| * class hierarchy. | ||
| * |
| * @author Mahesh Aravind V | ||
| * | ||
| */ | ||
| class DelegatingByTypeSerializerTests { |
There was a problem hiding this comment.
It is true, but I see tests for DelegatingByTypeSerializer over there, so I would like to have your new one over there as well.
And what is the reason of @Nested?
Cannot we really have it as a top-level test method and have the code flow as simple as it could be?
| return -1; | ||
| } | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
I wonder if more robust algorithm would be like this:
public class ClassHierarchyComparator implements Comparator<Class<?>> {
@Override
public int compare(Class<?> class1, Class<?> class2) {
if (class1 == class2) {
return 0; // Classes are the same
}
if (class1.isAssignableFrom(class2)) {
return -1; // class1 is a superclass or superinterface of class2, so class1 should come first
}
if (class2.isAssignableFrom(class1)) {
return 1; // class2 is a superclass or superinterface of class1, so class2 should come first
}
return class1.getName().compareTo(class2.getName()); // If no inheritance relation, compare by name
}
}
Ensure correct ordering of delegates regardless of user input.
Fixes: #3953