Feature/2954 allowing generic mappings #2959
Conversation
…rget with direct assignment.
…n using multiple parameters.
|
I am not sure that we should relax the requirement we have for this. However, I am going to look into the PR into detail a bit later. |
|
I need to play around with this to see what happens when the sources are not generic themselves. |
There was a problem hiding this comment.
I had a look at this @Zegveld and I have some questions for some things that I couldn't understand.
I am also not sure that support a type parameter for the @Context parameter makes sense here. The @Context parameter is meant to be passed to other places, e.g. @AfterMapping, @BeforeMapping, etc. How will this affect that functionality?
I think that we should limit the generic support only to valid source parameters and nothing more.
How does this work with @MappingTarget parameters?
| GenericSourceContainer<String> container = new GenericSourceContainer<>( containedObject, "otherValue" ); | ||
|
|
||
| GenericTargetContainer<String> resultString = GenericContainerMapper.INSTANCE.map( container ); | ||
| assertThat( resultString.getContained() ).isSameAs( containedObject ); |
There was a problem hiding this comment.
Doing isSameAs with a String isn't really recommended. If we want to test this properly we should use some other object.
| Integer replacement = 1234; | ||
| GenericTargetContainer<Integer> resultContext = | ||
| GenericContainerMapper.INSTANCE.mapWithContext( container, replacement ); | ||
| assertThat( resultContext.getContained() ).isSameAs( replacement ); |
There was a problem hiding this comment.
Same story here, we can't be using isSameAs for integer. We should try with actual objects here.
|
|
||
| GenericTargetContainer<Integer> resultSecondParameter = | ||
| GenericContainerMapper.INSTANCE.mapWithSecondParameter( container, replacement ); | ||
| assertThat( resultSecondParameter.getContained() ).isSameAs( replacement ); |
There was a problem hiding this comment.
Same story here for the isSameAs
| @@ -96,6 +96,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { | |||
| private final MethodReference finalizerMethod; | |||
|
|
|||
| private final MappingReferences mappingReferences; | |||
| private List<Type> typeParameters; | |||
There was a problem hiding this comment.
Why can't we make this final? I also see that this is not in the getImportTypes(). How will it with with things such as extends and super?
| @@ -552,7 +549,7 @@ private boolean checkParameterAndReturnType(ExecutableElement method, List<Param | |||
| } | |||
|
|
|||
| for ( Type typeParameter : resultType.getTypeParameters() ) { | |||
| if ( typeParameter.isTypeVar() ) { | |||
| if ( resultType.isIterableOrStreamType() && typeParameter.isTypeVar() ) { | |||
There was a problem hiding this comment.
Why do we need to check the resultType here?
| for ( Type typeParameter : parameterType.getTypeParameters() ) { | ||
| if ( typeParameter.hasSuperBound() ) { | ||
| messager.printMessage( method, Message.RETRIEVAL_WILDCARD_SUPER_BOUND_SOURCE ); | ||
| return false; | ||
| } | ||
|
|
||
| if ( typeParameter.isTypeVar() ) { | ||
| if ( parameterType.isIterableOrStreamType() && resultType.isIterableOrStreamType() |
There was a problem hiding this comment.
Why do we need to check for isIterableOrStreamType here?
First step in allowing generic mappings as long as the generic itself doesn't change.
closes #2954