Skip to content
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

Support for optional subtype-only source properties #2438

Closed
mjustin opened this issue May 7, 2021 · 8 comments
Closed

Support for optional subtype-only source properties #2438

mjustin opened this issue May 7, 2021 · 8 comments
Labels
Milestone

Comments

@mjustin
Copy link

mjustin commented May 7, 2021

My source type is a superclass with a certain set of properties, with a subclass that adds additional properties. My target type contains properties to be mapped from the superclass, as well as some optional properties to be mapped if the source is an instance of the subclass. It would be convenient to be able to be able to express this mapping relationship in a single annotation-based mapping method, rather than using multiple mapping methods with a custom mapping method to tie them together.

This is certainly related to #131, and possibly involves the same solution. The difference is that that issue concerns mapping to different target types based on the source type, whereas this involves mapping different target types to a single source type, with optional fields for subclasses.

Example

Types

public class Source {
    private String property1;
    private String property2;
    private String property3;

    // Getters, setters, & constructors omitted for brevity
}

public class SourceSubclass extends Source {
    private String subclassProperty;
    private String target5;

    // Getters, setters, & constructors omitted for brevity
}

public class Target {
    private String target1;
    private String target2;
    private String target3;
    private String target4;
    private String target5;

    // Getters, setters, & constructors omitted for brevity
}

Mapper

This is an solution to the problem using the features currently available in MapStruct.

@Mapper
public interface SourceMapper {
    default Target map(Source source) {
        if (source instanceof SourceSubclass) {
            return mapSubclass((SourceSubclass) source);
        } else {
            return mapSuperclass(source);
        }
    }

    @Mapping(target = "target1", source = "property1")
    @Mapping(target = "target2", source = "property2")
    @Mapping(target = "target3", source = "property3")
    @Mapping(target = "target4", expression = "java(null)")
    @Mapping(target = "target5", expression = "java(null)")
    Target mapSuperclass(Source source);

    @InheritConfiguration(name = "mapSuperclass")
    @Mapping(target = "target4", source = "subclassProperty")
    @Mapping(target = "target5") // Have to declare in order to override with default behavior
    Target mapSubclass(SourceSubclass source);
}
@filiphr
Copy link
Member

filiphr commented May 15, 2021

Thanks for the detailed explanation @mjustin.

I guess you are looking for MapStruct to generate your default Target map(Source source) source method for you? If that is the case then the solution is identical. The fact that #131 talks about different target types is only a small detail. If you agree, I would close this one in favour of #131. You can perhaps add your example to it as well.

Just so I understand you correctly. The mapSuperclass and mapSubclass will still be defined by you. You don't expect MapStruct to generate something like that for you?

Zegveld added a commit to rigd-loxia/mapstruct that referenced this issue Jul 9, 2021
Zegveld added a commit to rigd-loxia/mapstruct that referenced this issue Jul 9, 2021
…le. fixed missing newline in mapper used for testing issue 2438.
Zegveld added a commit to rigd-loxia/mapstruct that referenced this issue Jul 11, 2021
mapstruct#131, mapstruct#2438, mapstruct#366: Added support for SubClassMapping annotations. This will allow for hierarchy mappings.
@mjustin
Copy link
Author

mjustin commented Aug 2, 2021

@filiphr Sorry I'm just now getting back to your question.

Generating the Target map(Source source) would be a nice step. I think in an ideal world for my solution, I'd want something where certain properties remain unset unless the input is of a particular subclass. Here's a general idea, using a bogus ifInstance annotation element (not suggesting this specifically, just showing what it might look like).

@Mapping(target = "target1", source = "property1")
@Mapping(target = "target2", source = "property2")
@Mapping(target = "target3", source = "property3")
@Mapping(target = "target4", source = "subclassProperty", ifInstance = SourceSubclass.java)
@Mapping(target = "target5", expression = "java(null)")
Target map(Source source);

@mjustin
Copy link
Author

mjustin commented Aug 2, 2021

I was playing around a bit more, and stumbled upon another approach using existing functionality to achieving this mapping:

@Mapper
public interface SourceMapper {
    @Mapping(target = "target1", source = "property1")
    @Mapping(target = "target2", source = "property2")
    @Mapping(target = "target3", source = "property3")
    @Mapping(target = "target4", source = ".", qualifiedByName = "target4")
    @Mapping(target = "target5", expression = "java(null)")
    Target map(Source source);
    
    @Named("target4")
    String getTarget4(Source source) {
        return source instanceof SourceSubclass ? (((SourceSubclass) source).getSubclassProperty()) : null;
    }
}

In some ways I think it's cleaner than what I had as the current workaround, though it does require some manual field mapping that in a more convenient world the framework might be able to take care of.

@Zegveld
Copy link
Contributor

Zegveld commented Aug 10, 2021

Just letting you know that when the PR for 131, 2438 and 366 (see mentions above) is finished you will be able to generate the instanceof code from your first example by adding the @SubClassMapping as listed below.

@Mapper
public interface SourceMapper {
    @SubClassMapping( sourceClass = SourceSubclass.class, targetClass = Target.class )
    @Mapping( target = "target1", source = "property1" )
    @Mapping( target = "target2", source = "property2" )
    @Mapping( target = "target3", source = "property3" )
    @Mapping( target = "target4", expression = "java(null)" )
    @Mapping( target = "target5", expression = "java(null)" )
    Target mapSuperclass(Source source);

    @InheritConfiguration( name = "mapSuperclass" )
    @Mapping( target = "target4", source = "subclassProperty" )
    @Mapping( target = "target5" ) // Have to declare in order to override with default behavior
    Target mapSubclass(SourceSubclass source);
}

The @SubClassMapping annotation will trigger the presence of an instanceof check at the start of the mapSuperclass method, if it matches the mentioned SourceSubclass it will attempt to map it using a mapping method that has the signature <targetClass> <methodName>(<SourceSubclass> <argName>). In this example the mapSubclass method has that signature, so that one will be used. If the mapping method is missing it will generate it.

Any suggestions that might improve the @SubClassMapping annotation are welcome.

@filiphr
Copy link
Member

filiphr commented Aug 14, 2021

I like the approach by @Zegveld for this one. I would even go so far and say that the mapping should look like:

@Mapper
public interface SourceMapper {
    @SubClassMapping( sourceClass = SourceSubclass.class, targetClass = Target.class )
    @Mapping( target = "target1", source = "property1" )
    @Mapping( target = "target2", source = "property2" )
    @Mapping( target = "target3", source = "property3" )
    @Mapping( target = "target4", ignore = true )
    @Mapping( target = "target5", ignore = true )
    Target mapSuperclass(Source source);

    @InheritConfiguration( name = "mapSuperclass" )
    @Mapping( target = "target4", source = "subclassProperty" )
    @Mapping( target = "target5" ) // Have to declare in order to override with default behavior
    Target mapSubclass(SourceSubclass source);
}

Nice work @Zegveld, I only need to spend some good time in going through the entire PR 😄

Zegveld added a commit to rigd-loxia/mapstruct that referenced this issue Aug 29, 2021
@filiphr filiphr added this to the 1.5.0 milestone Oct 19, 2021
filiphr pushed a commit that referenced this issue Oct 19, 2021
…2512)


Add new `@SubclassMapping` for creating Downcast mapping.
When a parent mapping method is annotated with `@SubclassMapping` 
it will now generate an instanceof check inside the parent mapping 
and generate the subclass mappings if they are not manually defined. 

There is also `SubclassExhaustiveStrategy` for controlling what MapStruct should do in case the target type is abstract and there is no suitable way to create it.
@filiphr
Copy link
Member

filiphr commented Oct 19, 2021

Thanks to @Zegveld this issue can be closed since its solution is to use the new @SubclassMapping. You can give it a go @mjustin with 1.5.0-SNAPSHOT and we are going to release 1.5.Beta2 soon

@filiphr filiphr closed this as completed Oct 19, 2021
@mjustin
Copy link
Author

mjustin commented Oct 20, 2021

@filiphr I've confirmed that the following mapper works for my example in 1.5.0-SNAPSHOT:

@Mapper
public interface SubclassMapper {
    @SubclassMapping(source = SourceSubclass.class, target = Target.class)
    @Mapping(target = "target1", source = "property1")
    @Mapping(target = "target2", source = "property2")
    @Mapping(target = "target3", source = "property3")
    @Mapping(target = "target4", expression = "java(null)")
    @Mapping(target = "target5", expression = "java(null)")
    Target map(Source source);

    @InheritConfiguration(name = "map")
    @Mapping(target = "target4", source = "subclassProperty")
    @Mapping(target = "target5") // Have to declare in order to override with default behavior
    Target mapSubclass(SourceSubclass source);
}

Thanks, @Zegveld & @filiphr!

@filiphr
Copy link
Member

filiphr commented Oct 20, 2021

Thanks for testing this out @mjustin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants