From e0576f654b5a89530ca17c802bbe55f4707c17b6 Mon Sep 17 00:00:00 2001 From: thunderhook <8238759+thunderhook@users.noreply.github.com> Date: Wed, 3 Jan 2024 23:26:38 +0100 Subject: [PATCH] #3495 when updating targets with collections without a setter, clear them only if the null check or condition is satisfied --- .../GetterWrapperForCollectionsAndMaps.ftl | 6 +-- .../ap/test/bugs/_289/Issue289Test.java | 5 +- .../ap/test/bugs/_3495/Issue3495Mapper.java | 54 +++++++++++++++++++ .../ap/test/bugs/_3495/Issue3495Test.java | 33 ++++++++++++ 4 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Test.java diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl index 4ef55f3b4..cf33e6470 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl @@ -10,10 +10,10 @@ <@lib.sourceLocalVarAssignment/> if ( ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing /> != null ) { <@lib.handleExceptions> - <#if ext.existingInstanceMapping> - ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear(); - <@lib.handleLocalVarNullCheck needs_explicit_local_var=false> + <#if ext.existingInstanceMapping> + ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear(); + ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.collectionType>addAll<#else>putAll( <@lib.handleWithAssignmentOrNullCheckVar/> ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_289/Issue289Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_289/Issue289Test.java index 4b95513fe..efd9f40fe 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_289/Issue289Test.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_289/Issue289Test.java @@ -44,11 +44,12 @@ public class Issue289Test { Source source = new Source(); source.setCollection( null ); TargetWithoutSetter target = new TargetWithoutSetter(); - target.getCollection().add( new TargetElement() ); + TargetElement existingElement = new TargetElement(); + target.getCollection().add( existingElement ); Issue289Mapper.INSTANCE.sourceToTargetWithoutSetter( source, target ); - assertThat( target.getCollection() ).isEmpty(); + assertThat( target.getCollection() ).containsExactly( existingElement ); } @ProcessorTest diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Mapper.java new file mode 100644 index 000000000..bf0d8b6cc --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Mapper.java @@ -0,0 +1,54 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._3495; + +import java.util.ArrayList; +import java.util.List; + +import org.mapstruct.Condition; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.Named; +import org.mapstruct.factory.Mappers; + +@Mapper +public interface Issue3495Mapper { + + Issue3495Mapper INSTANCE = Mappers.getMapper( Issue3495Mapper.class ); + + @Mapping(target = "names", source = "names", conditionQualifiedByName = "alwaysFalse") + void update(Source source, @MappingTarget TargetWithoutSetter target); + + @Condition + @Named("alwaysFalse") + default boolean alwaysFalse(@MappingTarget TargetWithoutSetter target) { + return false; + } + + class Source { + + private List names; + + public List getNames() { + return names; + } + + public void setNames(List names) { + this.names = names; + } + } + + class TargetWithoutSetter { + + private final List names = new ArrayList<>(); + + public List getNames() { + return names; + } + + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Test.java new file mode 100644 index 000000000..e3cb10421 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3495/Issue3495Test.java @@ -0,0 +1,33 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._3495; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Oliver Erhart + */ +@IssueKey("3495") +@WithClasses(Issue3495Mapper.class) +class Issue3495Test { + + @ProcessorTest + void shouldNotClearCollectionBecauseConditionWasNotMet() { + + Issue3495Mapper.Source source = new Issue3495Mapper.Source(); + + Issue3495Mapper.TargetWithoutSetter target = new Issue3495Mapper.TargetWithoutSetter(); + target.getNames().add( "name" ); + + Issue3495Mapper.INSTANCE.update( source, target ); + assertThat( target ).isNotNull(); + assertThat( target.getNames() ).containsExactly( "name" ); + } +}