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

mapstruct-mapper-repo example bug #79

Open
everettwolf opened this issue Apr 10, 2020 · 4 comments
Open

mapstruct-mapper-repo example bug #79

everettwolf opened this issue Apr 10, 2020 · 4 comments

Comments

@everettwolf
Copy link

in testMapObjectToObject add:
carDto.setSetCount(7);
and
Assert.assertEquals(5, carDto.getSeatCount());
test will fail

Also, despite the annotation
@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE)
on CarMapper:
in testMapObjectToObject
change
Car car = new Car("Morris", 5, CarType.SPORTS);
to
Car car = new Car("Morris", 5, null);
and add
carDto.setType("anything");
and
Assert.assertEquals("anything", carDto.getType());
test will fail

For the latter, I tried to make sense of your recent change of "don't overwrite target if source is null" pull request, but there was so much discussion on what the annotation should be, I couldn't easily figure out the end result. ;o)

I also can't find a test case in this git repo that covers it. Can you point me to it, if there is one?
Thanks!

@filiphr
Copy link
Member

filiphr commented Apr 10, 2020

@everettwolf I don't think I understand what you are trying to do?

Can you provide a test case with what is wrong? The NullValuePropertyMappingStrategy is only applied for updated mappings. Basically for

which the CarMapper from the test inherits from. The test you are referring to tests only the map method

@everettwolf
Copy link
Author

Hey @filiphr thanks for the reply!
For the former, I think it's just a bug in your example code. I believe the expectation of the mapper is that if the source -- in this case Car -- has a different type and a different value that CarDto, the values should get updated on Target as the target datatype. That IS the case for brand and type, but not for seat count. Maybe it's having a difficult time both translating the datatype and updating the value. I believe it'd be self-explanatory if you add what I put in my original comment into the existing test and just run it.

For the latter, my use case is this (and this was what I was trying to solve when I stumbled over the above):
Source{
String stringone;
String stringtwo;
}
Target {
String iamstringone;
String iamstringtwo;
}
So, same datatypes, different fieldnames.
I want to map the values from a Source to an existing Target.
So source could look like this:
stringone="foo"
stringtwo=null

and Target could look like this:
iamstringone="hello"
iamstringtwo="goodbye"

After mapping the source to the target, I would want Target to look like this:
iamstringone="foo"
iamstringtwo="goodbye"

IOW, any field where the source property is null, the equivalent target property is NOT updated.

I saw a few people asking about this, and a response in the form of this PR:
mapstruct/mapstruct#1618

However, I can't get it to work with the latest version, and the examples provided in this repo don't appear to cover it.

It's no big deal; I was hoping to drop mapstruct into a project I'm working on to solve the use case I mentioned, but I ended up just creating my own mapping annotations, and it works fine.

Thanks!!

@everettwolf
Copy link
Author

Just took a quick look (at home w/ the kids due to Covid, so it's a bit tough ;o))

For mapstruct-mapper-repo, this is what the generated Implementation of the CarMapper looks like:
public class CarMapperImpl implements CarMapper {

@Override
public CarDto update(Car arg0, CarDto arg1) {
    if ( arg0 == null ) {
        return null;
    }

    if ( arg0.getMake() != null ) {
        arg1.setMake( arg0.getMake() );
    }
    if ( arg0.getType() != null ) {
        arg1.setType( arg0.getType().name() );
    }

    return arg1;
}

@Override
public CarDto map(Car CarDto) {
    if ( CarDto == null ) {
        return null;
    }

    CarDto carDto = new CarDto();

    carDto.setSeatCount( CarDto.getNumberOfSeats() );
    carDto.setMake( CarDto.getMake() );
    if ( CarDto.getType() != null ) {
        carDto.setType( CarDto.getType().name() );
    }

    return carDto;
}

}

Note that the mapping method includes the seatCount conversion, but the update method does not. If this is by design, can you explain why?

Thanks!

@everettwolf
Copy link
Author

Hmm, thought I was on to something: numberOfSeats and seatCount (in Car & CarDto) were a primitive (int) so the null checking annotation wouldn't be able to do anything with that, so thinking the generator realizes that and doesn't add primitives to the update method in this scenario.

However, I changed both to Integer and regenerated (by the way, have to delete the target directory to get that kind of change picked up on a mvn clean compile), and still the same issue. I can go in and modify the generated output to do what I want, but that wouldn't be very elegant.

I'll just stop and let you respond, thus far; I'm probably missing something.

Not sure you'd want to embark on an Autoboxing/Unboxing rabbit hole to work around the "check for null on source" issue.

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

No branches or pull requests

2 participants