-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Right now, all (non-annotated) arguments of the mapping method are considered beans with properties. This leads to annoying situations, which I'll explain - a clarification follows!
Problems
Problem 1: Source parameters are added
Consider:
public static class Metadata {
public String a;
public String b;
}
public static class Thing {
public String a;
public String b;
public Collection<String> foo;
public Path path;
}
@Mapping(target = "foo", source = "foo")
abstract Thing create(Metadata meta, Collection<String> foo, java.nio.Path path);However, mapstruct treats all of these arguments as beans, so empty (from Collection#isEmpty), absolute (Path#isAbsolute), parent (Path#getParent), etc. are added to the list of source parameters. This can both lead to unintentional mapping as well as very verbose ignoreUnmappedSourceProperties lists.
Indeed, if we add public boolean empty to Thing, it is set to foo.isEmpty. (BTW: This is inconsistent with the IntelliJ addon, that one complains about missing target parameter empty.)
Moreover (this might be a separate issue) without @Mapping(target = "foo", source = "foo"), mapstruct complains about the unmapped target property foo.
Problem 2: Cannot map a set of collections
public static class Container {
public Collection<String> things;
public Collection<String> otherThings;
}
@Mapping(target = "things", source = "things")
@Mapping(target = "otherThings", source = "otherThings")
abstract Container create(Collection<String> things, Collection<String> otherThings);doesn't work because mapstruct Can't generate mapping method from iterable type to non-iterable type.
Note that this works:
public static class Container {
public String thing;
public Collection<String> things;
public Collection<String> otherThings;
}
@Mapping(target = "things", source = "things")
@Mapping(target = "otherThings", source = "otherThings")
abstract Container create(String thing, Collection<String> things, Collection<String> otherThings);This example may seem a bit arbitrary, but if the collections contain to-be-mapped-types, it might be a bit clearer, for example:
public static class Container {
public Collection<Bar> things;
}
abstract Bar map(Foo foo);
@Mapping(target = "things", source = "things")
abstract Container create(Collection<Foo> things);(omitting irrelevant stuff).
IMO, this should generate something like container.things = things.stream().map(this::map).toList()), but instead fails with the mentioned error.
Clarification
All these are the "correct" / sensible behaviour by default! Mapstruct of course has no way to guess the users intention, and assuming that everything is a bean surely makes sense. Also, all these problems can be circumvented by, e.g., defining dummy container objects. For example, for the second case, we could do the following:
public static class Dummy {
public Collection<Foo> things;
}
public static class Container {
public Collection<Bar> things;
}
abstract Bar map(Foo foo);
abstract Container create(Dummy dummy);But this sort of defeats the purpose of mapstruct.
So, there should be a way of informing mapstruct about said intentions.
Proposed solution
Add a parameter annotation, e.g., @MappingParameter (analogous to @MappingTarget). Any source argument annotated with this annotation is not "unfolded" at all, instead it rather is considered like a property of a dummy bean. This means that the properties of the annotated object are not added to the list of source parameters and they also cannot be referenced as @Mapping source. For example:
@Mapping(target = "empty", source = "path.empty")
abstract Foo create(@MappingParameter Path path);should throw an error. However, these parameters should of course be "mapped", i.e. if I have a Thing with property Bar bar, and mappings Thing map(@MappingParameter Foo foo), Bar map(Foo foo), this should set thing.bar = map(foo).