#1830 NullValuePropertyMappingStrategy.CLEAR #3592

zyberzebra · 2024-05-03T11:15:52Z

Fixes #1830

adds CLEAR as a new strategy under NullValuePropertyMappingStrategy for handling null values
clears collections or maps when the source property is null
includes tests for functionality and error handling

zyberzebra · 2024-05-03T11:17:53Z

@filiphr or any Reviewer who wants to Review:

I added some TODO where I need some clarification

I tried to come up with, what is wanted from all the discussion that point to the issue i try to fix here.

If I missed something some examples would be nice.

zyberzebra · 2024-05-04T08:14:19Z

It's still not clear to me what should be done for every case.
I need some input on the following:

Works with this PR:

clearing

@Mapper
public interface BeanMapper {
    @Mapping(source = "list", target = "list", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR)
    BeanDTO map(Bean source, @MappingTarget BeanDTO target);

generates:

[...]
        if ( target.getList() != null ) {
            Collection<String> collection = source.getList();
            if ( collection != null ) {
                target.getList().clear();
                target.getList().addAll( collection );
            }
            else {
                target.getList().clear();
            }
        }    
[...]

Error on compile:

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR)
public interface MapperWithErrorInConfig {

-> "The strategy: "CLEAR" can't be used for: "nullValuePropertyMapping"."

    @Mapping(target = "id", source = "id", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR) //id is an integer
    BeanDTOWithId map(BeanWithId source, @MappingTarget BeanDTOWithId target);

-> "Target property: "id" is not a Collection or Map and can't be used with NullValuePropertyMappingStrategy: 'CLEAR'"

Should be valid(?):

@Mapper(
//    nullValueMapMappingStrategy = NullValueMappingStrategy.CLEAR, // TODO: IS this feature wanted? (calling .clear())
//    nullValueIterableMappingStrategy = NullValueMappingStrategy.CLEAR // TODO: IS this feature wanted? an do we iterate and remove? 
)
public interface BeanMapperWithValidConfig {

filiphr · 2024-05-18T06:24:30Z

Thanks for your work on this @zyberzebra. I'm traveling the next month, so I'm not sure if I'll get the chance to review this properly. Just giving you a heads up.

Regarding your questions:

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR)
public interface MapperWithErrorInConfig {

I didn't understand, does this work or not? It should work, basically things like this are inherited based on what is defined. So if you define in @Mapping it is using from there, then it goes to @BeanMapping, then @Mapper, @MapperConfig etc.

    @Mapping(target = "id", source = "id", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR) //id is an integer
    BeanDTOWithId map(BeanWithId source, @MappingTarget BeanDTOWithId target);

I'm not sure what to do in this case, I would say it need to behave the same as it is now without the clear.

@Mapper(
//    nullValueMapMappingStrategy = NullValueMappingStrategy.CLEAR, // TODO: IS this feature wanted? (calling .clear())
//    nullValueIterableMappingStrategy = NullValueMappingStrategy.CLEAR // TODO: IS this feature wanted? an do we iterate and remove? 
)
public interface BeanMapperWithValidConfig {

are we ever doing a clear for a regular mapping? I don't think so, so I would not go there right now.

zyberzebra · 2024-05-18T09:59:03Z

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR)
public interface MapperWithErrorInConfig {
I didn't understand, does this work or not? It should work, basically things like this are inherited based on what is defined. So if you define in @Mapping it is using from there, then it goes to @BeanMapping, then @Mapper, @MapperConfig etc.

I implemented it to throw an compile Error.

I'm not sure what to do in this case, I would say it need to behave the same as it is now without the clear.

I'm going to change the compile Error to your suggestions that is does nothing, if there is nothing "to clear"

@Mapper(
//    nullValueMapMappingStrategy = NullValueMappingStrategy.CLEAR, // TODO: IS this feature wanted? (calling .clear())
//    nullValueIterableMappingStrategy = NullValueMappingStrategy.CLEAR // TODO: IS this feature wanted? an do we iterate and remove? 
)
public interface BeanMapperWithValidConfig {

are we ever doing a clear for a regular mapping? I don't think so, so I would not go there right now.

alright!

Thanks for answering the questions. I'm going to try to make some changes and write some good Tests and tasks here that help with understanding what's going on if we come back to this PR in a month. :)

zyberzebra · 2024-05-19T07:59:06Z

    @Mapping(target = "id", source = "id", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.CLEAR) //id is an integer
    BeanDTOWithId map(BeanWithId source, @MappingTarget BeanDTOWithId target);

I'm not sure what to do in this case, I would say it need to behave the same as it is now without the clear.

In the reference Issue your opinion was:

#1830 (comment)

In the implementation if NullValuePropertyMappingStrategy.CLEAR is used for nullValuePropertyMapingStrategy then we should provide a compile error, since the strategy does not make sense for non iterable / map types.

I removed it for now, and it behaves like the default behavior.

Compile Error or not?

filiphr

Sorry for the confusion @zyberzebra.

I think I was referring to the fact that the issue you are working on, would be done after #3387. Which means that the clear would be fine for the 2 new places, but not for the existing place.

processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java

...pstruct/ap/internal/model/assignment/ExistingInstanceSetterWrapperForCollectionsAndMaps.java

filiphr

Sorry for the wait @zyberzebra. The implementation looks good to me. I only have some small comments around the tests.

Additionally, can we please move the tests to another package. This is not a bug, but a new feature. e.g. you can put them under org.mapstruct.ap.test.nullvaluepropertymapping or a new package under that.

processor/src/test/java/org/mapstruct/ap/test/bugs/_1830/BeanMapperTest.java

processor/src/test/java/org/mapstruct/ap/test/bugs/_1830/BeanMapper.java

processor/src/test/java/org/mapstruct/ap/test/bugs/_1830/BeanDTO.java

...or/src/test/java/org/mapstruct/ap/test/bugs/_1830/BeanMapperWithStrategyDefinedInConfig.java

- adds CLEAR as a new strategy under NullValuePropertyMappingStrategy for handling null values - clears collections or maps when the source property is null - includes tests for functionality and error handling

- prevents the usage of "CLEAR" as a nullValuePropertyMapping strategy in config - new error message for the invalid use of mapping strategies - test case to verify the error handling.

- removes the needsImport method - reduce the enum visibility

noshua · 2025-07-23T09:51:22Z

@filiphr Any update on this? Can't see what holds this essential feature back from being merged for months.

filiphr · 2025-07-27T20:17:56Z

@noshua, as you can see the build is not passing for it. Which means, that we need to take it up on our side and try to finish it. Since we are doing this in our free time, we are a bit limited with how much we can do.

zyberzebra · 2025-08-01T04:48:16Z

@filiphr o can try to finish the work. Haven't worked with mapstruct for a while but I can try to have a look on the weekend. Do you already know what's the issue? Is maybe just a Rebase needed?

filiphr · 2025-08-04T05:36:00Z

I'm not sure what the problem is @zyberzebra, I can't see the logs anymore. If you have time rebase it and we can see. Otherwise, we can also take it over and bring it over the finish line to main.

noshua · 2025-08-04T10:25:49Z

I'm also willing to help if there is anything I could do. This feature is basically missing to make MapStruct usable in an Jakarta Persistence stack.

…ppingStrategey#Clear

zyberzebra changed the title ~~[#1830] NullValuePropertyMappingStrategy.CLEAR~~ #1830 NullValuePropertyMappingStrategy.CLEAR May 3, 2024

filiphr reviewed Jun 1, 2024

View reviewed changes

filiphr requested changes Sep 2, 2024

View reviewed changes

zyberzebra requested a review from filiphr September 7, 2024 08:47

zyberzebra added 10 commits September 7, 2024 11:04

adds NullValuePropertyMappingStrategy.CLEAR strategy

dda452b

- adds CLEAR as a new strategy under NullValuePropertyMappingStrategy for handling null values - clears collections or maps when the source property is null - includes tests for functionality and error handling

Adds validation for nullValuePropertyMapping strategy in config

d44f827

- prevents the usage of "CLEAR" as a nullValuePropertyMapping strategy in config - new error message for the invalid use of mapping strategies - test case to verify the error handling.

remove compile errors

1b1502c

adss missing copyright headers

0d38b3b

deletes unused mapper

211b638

refactor: ExistingInstanceSetterWrapperForCollectionsAndMaps

6781d96

- removes the needsImport method - reduce the enum visibility

revert method extraction

a314746

refactor: remove unnecessary stuff

7296899

refactor(test): renaming, restructuring, styling

abf0c99

move(test): move tests to different package

00c2d2a

zyberzebra force-pushed the #1830-nullValuePropertyMappingStrategey#Clear branch from 92ee4c4 to 00c2d2a Compare September 7, 2024 09:05

zyberzebra added 2 commits November 20, 2025 23:34

Merge branch 'mapstruct:main' into mapstruct#1830-nullValuePropertyMa…

718f710

…ppingStrategey#Clear

style: fix checkstyle format issues

5ed9c0a

Uh oh!

#1830 NullValuePropertyMappingStrategy.CLEAR #3592

Are you sure you want to change the base?

#1830 NullValuePropertyMappingStrategy.CLEAR #3592

Conversation

zyberzebra commented May 3, 2024 • edited by filiphr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zyberzebra commented May 3, 2024

Uh oh!

zyberzebra commented May 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Works with this PR:

clearing

Error on compile:

Should be valid(?):

Uh oh!

filiphr commented May 18, 2024

Uh oh!

zyberzebra commented May 18, 2024

Uh oh!

zyberzebra commented May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filiphr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

filiphr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

noshua commented Jul 23, 2025

Uh oh!

filiphr commented Jul 27, 2025

Uh oh!

zyberzebra commented Aug 1, 2025

Uh oh!

filiphr commented Aug 4, 2025

Uh oh!

noshua commented Aug 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zyberzebra commented May 3, 2024 •

edited by filiphr

Loading

zyberzebra commented May 4, 2024 •

edited

Loading

zyberzebra commented May 19, 2024 •

edited

Loading