Skip to content

fix: Ensure NullValuePropertyMappingStrategy.SET_TO_DEFAULT initializes empty collection/map when target is null#3871

Merged
filiphr merged 3 commits intomapstruct:mainfrom
tangyang9464:fix/3865
Jun 15, 2025
Merged

fix: Ensure NullValuePropertyMappingStrategy.SET_TO_DEFAULT initializes empty collection/map when target is null#3871
filiphr merged 3 commits intomapstruct:mainfrom
tangyang9464:fix/3865

Conversation

@tangyang9464
Copy link
Contributor

@tangyang9464 tangyang9464 commented May 27, 2025

fix: #3884

When both source and target Map/List properties are null and nullValuePropertyMappingStrategy is SET_TO_DEFAULT, the target property should be set to an empty collection instead of remaining null.

This commit adds the necessary else branch to ExistingInstanceSetterWrapperForCollectionsAndMaps.ftl to handle this case correctly.

…es empty collection/map when target is null

Signed-off-by: TangYang <[email protected]>
@tangyang9464
Copy link
Contributor Author

@filiphr There was a fixture test about SET_TO_DEFAULT that failed, and I changed the fixture.

I think the bug may be related to this comment,but to be honest, I don’t quite understand what this sentence means.

* <b>Note</b>: some types of mappings (collections, maps), in which MapStruct is instructed to use a getter or adder
* as target accessor see {@link CollectionMappingStrategy}, MapStruct will always generate a source property
* null check, regardless the value of the {@link NullValuePropertyMappingStrategy} to avoid addition of {@code null}
* to the target collection or map. Since the target is assumed to be initialised this strategy will not be applied.

It's correct for SET_TO_NULL and IGNORE, and the else branch should not be generated. But it doesn't apply to SET_TO_DEFAULT

Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @tangyang9464. Some small comments to clean up the test a bit before we merge it

1. Remove useless properties and assertions
2. Replace isNotNull with isNotEmpty
@tangyang9464
Copy link
Contributor Author

@filiphr added two commit. plz review again

@tangyang9464 tangyang9464 requested a review from filiphr June 15, 2025 05:19
Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes @tangyang9464

@filiphr filiphr merged commit e4bc1cd into mapstruct:main Jun 15, 2025
4 checks passed
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

Successfully merging this pull request may close these issues.

NullValuePropertyMappingStrategy.SET_TO_DEFAULT should set target Map/Collection to default when source and target are all null

2 participants