Support List<T> and Map<String, T> components in Java records #1002
Conversation
|
Particularly looking for feedback on a new If it looks good I'll proceed with adding more tests. |
jyemin
changed the title
This enhances the support for any record component that is a generic List or Map where the generic type of the List or Map is a record or POJO. It used to incorrectly decode to org.bson.Document. Now it respects the generic type of the List or Map when decoding. JAVA-4667
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| abstract class AbstractIterableCodec<T> implements Codec<Iterable<T>> { |
There was a problem hiding this comment.
Extracted superclass so that the main logic can be shared between IterableCodec and the new ParameterizedIterableCodec.
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public abstract class AbstractMapCodec<T> implements Codec<Map<String, T>> { |
| @@ -62,6 +66,23 @@ static Object readValue(final BsonReader reader, final DecoderContext decoderCon | |||
| } | |||
| } | |||
|
|
|||
| static Codec<?> getCodec(final CodecRegistry codecRegistry, final Type type) { | |||
There was a problem hiding this comment.
This is the guts of the implementation: a recursive algorithm for creating a parameterized codec of any depth.
There was a problem hiding this comment.
Seems familiar to the Pojo codec parameterizedType handling but cleaner.
|
|
||
| /** | ||
| * Encodes and decodes {@code Iterable} objects. | ||
| * | ||
| * @since 3.3 | ||
| */ | ||
| @SuppressWarnings("rawtypes") | ||
| public class IterableCodec implements Codec<Iterable>, OverridableUuidRepresentationCodec<Iterable> { | ||
| public class IterableCodec extends AbstractIterableCodec<Object> |
There was a problem hiding this comment.
IterableCodec now implements Codec<Iterable<Object>> instead of Codec<Iterable>. This is binary but not source compatibile, which I think is ok given how this class is likely (not) used directly in applications.
| @@ -79,48 +79,31 @@ public Object transform(final Object objectToTransform) { | |||
|
|
|||
|
|
|||
| @Override | |||
| public Codec<Iterable> withUuidRepresentation(final UuidRepresentation uuidRepresentation) { | |||
There was a problem hiding this comment.
The diffs are a bit ugly due to the mix of functional changes with refactoring. I didn't realize I would need to extract a superclass until after the functional work was done. If it's too hard to read, I can go back and rebase everything, doing the refactoring first in a separate PR.
| return codecRegistry.get((Class<?>) type); | ||
| } else if (type instanceof ParameterizedType) { | ||
| ParameterizedType parameterizedType = (ParameterizedType) type; | ||
| Codec<?> rawCodec = codecRegistry.get((Class<?>) parameterizedType.getRawType()); |
There was a problem hiding this comment.
ParameterizedType#getRawType returns Type, not Class, but can the raw type of a parameterized type be anything else except a class? If so, this is an unsafe cast.
There was a problem hiding this comment.
I see that sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl's rawType field is of type Class<?>, which is a clue that this is safe.
There was a problem hiding this comment.
Can you add a ParameterizedRecordCodecas well then top level records can contain nested generic records eg:
public record TestRecordWithListGenericConcrete(@BsonId ObjectId id, TestRecordWithListGeneric<TestRecordEmbedded> nestedRecords) {
}
public record TestRecordWithListGeneric<T>(List<T> nestedRecords) {
}
| @@ -83,6 +85,10 @@ Object getValue(final Record record) throws InvocationTargetException, IllegalAc | |||
| @SuppressWarnings("deprecation") | |||
| private static Codec<?> computeCodec(final RecordComponent component, final CodecRegistry codecRegistry) { | |||
| var codec = codecRegistry.get(toWrapper(component.getType())); | |||
| if (codec instanceof Parameterizable parameterizableCodec | |||
There was a problem hiding this comment.
What is this magic? assigning variables in an if statement 🤯
There was a problem hiding this comment.
Not just one but two!
| @@ -62,6 +66,23 @@ static Object readValue(final BsonReader reader, final DecoderContext decoderCon | |||
| } | |||
| } | |||
|
|
|||
| static Codec<?> getCodec(final CodecRegistry codecRegistry, final Type type) { | |||
There was a problem hiding this comment.
Seems familiar to the Pojo codec parameterizedType handling but cleaner.
Yes, I don't see why not. The implementation will be a bit more complicated but I think it fits in with the overall design. I'll open a follow-up ticket for that if you don't mind. |
|
Opened JAVA-4740 to track request for support of nested parameterized records. |
This enhances the support for any record component that is a generic List or Map where the generic type of the List or Map value is a record or POJO. It used to incorrectly decode to org.bson.Document. Now it respects the generic type of the List or Map value when decoding.
JAVA-4667