Conversation
There was a problem hiding this comment.
This is my first glance and the solution is complex. I'm having troubles to see the full impact. Which makes me think: do we need to add more testcases for this one? What else did we miss when we introduced the 'target-this' feature.?
For instance: the target mapping with "." now acts as a template and we have a balanced property mapping underneath it (source and target make the same step in nesting). What if its unbalanced and the 'template overriding mapping' maps a deeper nested target to a shallower nested source or vice versa? What if you've got combinations?
I'm still not understanding the essence of the solution. We already have "." mappings and methods handling the "target this". So, what is different in this scenario that we have to generate a new TargetReference reflecting the "." mapping in the middle of a handling routine? Did we miss something prior? Again. It could be a comment thing.
I guess it becomes more understandable if we add comments explaining a particular scenario we try to solve.
Also, the issue mentioned the source = ".".. I think we need to open (perhaps a separate PR) to
- actively replace this with the source parameter if there's one source param only or, issue an error when there's more than one source param
- Simply not allow it and give an error straight away.
But not leave it in the open.
| String property = first( targetReference.getPropertyEntries() ); | ||
| MappingReference newMapping = mapping.popTargetReference(); | ||
|
|
||
| if ( newMapping == null && method.getSourceParameters().size() == 1 && sourceReference != null && sourceReference.getParameter() != null && |
There was a problem hiding this comment.
I think we need to add some examples here in the code. . I'm pretty lost here (so an actual example).
Also we have a lot of checks on whether there are more source-parameters in our bean mapping. WDYT? is it interesting to factor this case out (in the future) in its own BeanMappingMethod e.g. MultipleSourceBeanMappingMethod?
| List<String> newPathProperties = new ArrayList<>( targetReference.getPathProperties() ); | ||
| newPathProperties.add( targetReference.getPropertyEntries().get( 0 ) ); | ||
| TargetReference newTargetReference = new TargetReference( null, Collections.singletonList( "." ), newPathProperties); | ||
| newMapping = new MappingReference( mapping.getMapping(), newTargetReference, sourceReference ); |
There was a problem hiding this comment.
So, what you are doing here is creating a new "." mapping when you only have a source parameter (no properties) in the mapping.. But then I get lost...
| sourceReference.getPropertyEntries().isEmpty() && | ||
| !targetReference.getPropertyEntries().isEmpty() ) { | ||
| List<String> newPathProperties = new ArrayList<>( targetReference.getPathProperties() ); | ||
| newPathProperties.add( targetReference.getPropertyEntries().get( 0 ) ); |
There was a problem hiding this comment.
TargetReference#getShallowestPropertyName
| Set<MappingReference> singleTargetReferences) { | ||
| Set<MappingReference> mappings = entryByParam.getValue(); | ||
| Set<MappingReference> nonNested = new LinkedHashSet<>(); | ||
| NestedReferences nonNested = new NestedReferences(); |
There was a problem hiding this comment.
this feels contradictory: assigning 'nonNested' to a type 'NestedReferences'
2 participants
No description provided.