-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix type parameters not being passed to fallback property codec #1765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@rozza , I am not sure about the validity of the code changes proposed. For one thing, it breaks 6 unit testing cases; secondly it seems |
@GliczDev looks like this PR needs some tests and as @NathanQingyangXu mentioned this does appear to break some existing test cases. To run the bson tests and checks run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple of requested changes - to support java 8+ and to ensure that types are only provided if the list is not empty.
The LookupCountingCodecRegistry
implementation in tests will need to implement <T> Codec<T> get(Class<T> clazz, List<Type> typeArguments)
inorder not to throw an exception.
eg:
@Override
public <T> Codec<T> get(final Class<T> clazz, final List<Type> typeArguments) {
incrementCount(clazz);
for (CodecProvider provider : codecProviders) {
Codec<T> codec = provider.get(clazz, typeArguments, this);
if (codec != null) {
return codec;
}
}
return null;
}
It would also be good to add a regression test where this code path is used.
@@ -19,6 +19,8 @@ | |||
import org.bson.codecs.Codec; | |||
import org.bson.codecs.configuration.CodecRegistry; | |||
|
|||
import java.lang.reflect.Type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import java.lang.reflect.Type; | |
import java.lang.reflect.Type; | |
import java.util.List; | |
import java.util.stream.Collectors; |
@@ -35,6 +37,6 @@ public <S> Codec<S> get(final TypeWithTypeParameters<S> type, final PropertyCode | |||
if (clazz == pojoCodec.getEncoderClass()) { | |||
return (Codec<S>) pojoCodec; | |||
} | |||
return codecRegistry.get(type.getType()); | |||
return codecRegistry.get(type.getType(), type.getTypeParameters().stream().<Type>map(TypeWithTypeParameters::getType).toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return codecRegistry.get(type.getType(), type.getTypeParameters().stream().<Type>map(TypeWithTypeParameters::getType).toList()); | |
List<Type> types = type.getTypeParameters().stream().<Type>map(TypeWithTypeParameters::getType).collect(Collectors.toList()); | |
if (!types.isEmpty()) { | |
return codecRegistry.get(type.getType(), types); | |
} | |
return codecRegistry.get(type.getType()); |
For some reason, type parameters weren't passed to fallback property codec, which caused an exception when trying to access them. Now, this issue is fixed.