diff --git a/core-common/src/main/java/org/mapstruct/CollectionMappingStrategy.java b/core-common/src/main/java/org/mapstruct/CollectionMappingStrategy.java index 55979672e..556e8aab0 100644 --- a/core-common/src/main/java/org/mapstruct/CollectionMappingStrategy.java +++ b/core-common/src/main/java/org/mapstruct/CollectionMappingStrategy.java @@ -30,7 +30,8 @@ public enum CollectionMappingStrategy { * {@code orderDto.setOrderLines(order.getOrderLines)}. *

* If no setter is available but a getter method, this will be used, under the assumption it has been initialized: - * {@code orderDto.getOrderLines().addAll(order.getOrderLines)}. + * {@code orderDto.getOrderLines().addAll(order.getOrderLines)}. This will also be the case when using + * {@link MappingTarget} (updating existing instances). */ ACCESSOR_ONLY, @@ -51,5 +52,13 @@ public enum CollectionMappingStrategy { * Identical to {@link #SETTER_PREFERRED}, only that adder methods will be preferred over setter methods, if both * are present for a given collection-typed property. */ - ADDER_PREFERRED; + ADDER_PREFERRED, + + /** + * Identical to {@link #SETTER_PREFERRED}, however the target collection will not be cleared and accessed via + * addAll in case of updating existing bean instances, see: {@link MappingTarget}. + * + * Instead the target accessor (e.g. set) will be used on the target bean to set the collection. + */ + TARGET_IMMUTABLE; } diff --git a/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc b/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc index b4d537b80..c70b53a67 100644 --- a/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc +++ b/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc @@ -466,7 +466,7 @@ Specifying the parameter in which the property resides is mandatory when using t Mapping methods with several source parameters will return `null` in case all the source parameters are `null`. Otherwise the target object will be instantiated and all properties from the provided parameters will be propagated. ==== -MapStruct also offers the possibility to directly refer to a source parameter. +MapStruct also offers the possibility to directly refer to a source parameter. .Mapping method directly referring to a source parameter ==== @@ -1338,7 +1338,7 @@ public Map stringStringMapToLongDateMap(Map source) [[collection-mapping-strategies]] === Collection mapping strategies -MapStruct has a `CollectionMappingStrategy`, with the possible values: `ACCESSOR_ONLY`, `SETTER_PREFERRED` and `ADDER_PREFERRED`. +MapStruct has a `CollectionMappingStrategy`, with the possible values: `ACCESSOR_ONLY`, `SETTER_PREFERRED`, `ADDER_PREFERRED` and `TARGET_IMMUTABLE`. In the table below, the dash `-` indicates a property name. Next, the trailing `s` indicates the plural form. The table explains the options and how they are apply to the presence/absense of a `set-s`, `add-` and / or `get-s` method on the target object: @@ -1366,6 +1366,13 @@ In the table below, the dash `-` indicates a property name. Next, the trailing ` |add- |get-s |get-s + +|`TARGET_IMMUTABLE` +|set-s +|exception +|set-s +|exception +|set-s |=== Some background: An `adder` method is typically used in case of http://www.eclipse.org/webtools/dali/[generated (JPA) entities], to add a single element (entity) to an underlying collection. Invoking the adder establishes a parent-child relation between parent - the bean (entity) on which the adder is invoked - and its child(ren), the elements (entities) in the collection. To find the appropriate `adder`, MapStruct will try to make a match between the generic parameter type of the underlying collection and the single argument of a candidate `adder`. When there are more candidates, the plural `setter` / `getter` name is converted to singular and will be used in addition to make a match. diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java index 2e235cd26..d8edebf6a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java @@ -52,6 +52,7 @@ import org.mapstruct.ap.internal.model.source.ParameterProvidedMethods; import org.mapstruct.ap.internal.model.source.PropertyEntry; import org.mapstruct.ap.internal.model.source.SelectionParameters; import org.mapstruct.ap.internal.model.source.SourceReference; +import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism; import org.mapstruct.ap.internal.util.Executables; import org.mapstruct.ap.internal.util.MapperConfiguration; @@ -400,10 +401,15 @@ public class PropertyMapping extends ModelElement { Assignment result = rhs; - if ( targetAccessorType == TargetWriteAccessorType.SETTER || - targetAccessorType == TargetWriteAccessorType.FIELD ) { + CollectionMappingStrategyPrism cms = method.getMapperConfiguration().getCollectionMappingStrategy(); + boolean targetImmutable = cms == CollectionMappingStrategyPrism.TARGET_IMMUTABLE; + + if ( targetAccessorType == TargetWriteAccessorType.SETTER || + targetAccessorType == TargetWriteAccessorType.FIELD ) { + + + if ( result.isCallingUpdateMethod() && !targetImmutable) { - if ( result.isCallingUpdateMethod() ) { // call to an update method if ( targetReadAccessor == null ) { ctx.getMessager().printMessage( @@ -423,6 +429,7 @@ public class PropertyMapping extends ModelElement { ); } else { + // target accessor is setter, so wrap the setter in setter map/ collection handling result = new SetterWrapperForCollectionsAndMaps( result, @@ -430,11 +437,20 @@ public class PropertyMapping extends ModelElement { targetType, method.getMapperConfiguration().getNullValueCheckStrategy(), ctx.getTypeFactory(), - targetWriteAccessorType == TargetWriteAccessorType.FIELD + targetWriteAccessorType == TargetWriteAccessorType.FIELD, + targetImmutable ); } } else { + if ( targetImmutable ) { + ctx.getMessager().printMessage( + method.getExecutable(), + Message.PROPERTYMAPPING_NO_WRITE_ACCESSOR_FOR_TARGET_TYPE, + targetPropertyName + ); + } + // target accessor is getter, so wrap the setter in getter map/ collection handling result = new GetterWrapperForCollectionsAndMaps( result, method.getThrownTypes(), diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java index db5faa074..76817d208 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java @@ -47,13 +47,15 @@ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd private final boolean includeSourceNullCheck; private final Type targetType; private final TypeFactory typeFactory; + private final boolean targetImmutable; public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment, List thrownTypesToExclude, Type targetType, NullValueCheckStrategyPrism nvms, TypeFactory typeFactory, - boolean fieldAssignment) { + boolean fieldAssignment, + boolean targetImmutable ) { super( decoratedAssignment, @@ -64,6 +66,7 @@ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd this.includeSourceNullCheck = ALWAYS == nvms; this.targetType = targetType; this.typeFactory = typeFactory; + this.targetImmutable = targetImmutable; } @Override @@ -96,4 +99,8 @@ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd return "java.util.EnumSet".equals( targetType.getFullyQualifiedName() ); } + public boolean isTargetImmutable() { + return targetImmutable; + } + } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index 4440ee461..3be805723 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -469,7 +469,8 @@ public class Type extends ModelElement implements Comparable { // the current target accessor can also be a getter method. // The following if block, checks if the target accessor should be overruled by an add method. if ( cmStrategy == CollectionMappingStrategyPrism.SETTER_PREFERRED - || cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { + || cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED + || cmStrategy == CollectionMappingStrategyPrism.TARGET_IMMUTABLE ) { // first check if there's a setter method. Accessor adderMethod = null; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/prism/CollectionMappingStrategyPrism.java b/processor/src/main/java/org/mapstruct/ap/internal/prism/CollectionMappingStrategyPrism.java index 96851100e..a68fa477b 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/prism/CollectionMappingStrategyPrism.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/prism/CollectionMappingStrategyPrism.java @@ -27,5 +27,6 @@ public enum CollectionMappingStrategyPrism { ACCESSOR_ONLY, SETTER_PREFERRED, - ADDER_PREFERRED; + ADDER_PREFERRED, + TARGET_IMMUTABLE; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java index 78169e9df..9b2a37bdc 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java @@ -55,6 +55,7 @@ public enum Message { PROPERTYMAPPING_INVALID_PROPERTY_NAME( "No property named \"%s\" exists in source parameter(s)." ), PROPERTYMAPPING_NO_PRESENCE_CHECKER_FOR_SOURCE_TYPE( "Using custom source value presence checking strategy, but no presence checker found for %s in source type." ), PROPERTYMAPPING_NO_READ_ACCESSOR_FOR_TARGET_TYPE( "No read accessor found for property \"%s\" in target type." ), + PROPERTYMAPPING_NO_WRITE_ACCESSOR_FOR_TARGET_TYPE( "No write accessor found for property \"%s\" in target type." ), CONSTANTMAPPING_MAPPING_NOT_FOUND( "Can't map \"%s %s\" to \"%s %s\"." ), CONSTANTMAPPING_NO_READ_ACCESSOR_FOR_TARGET_TYPE( "No read accessor found for property \"%s\" in target type." ), diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl index c681e2a8e..a31bf3dae 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl @@ -22,7 +22,7 @@ <#import "../macro/CommonMacros.ftl" as lib> <@lib.sourceLocalVarAssignment/> <@lib.handleExceptions> - <#if ext.existingInstanceMapping> + <#if ext.existingInstanceMapping && !targetImmutable> if ( ${ext.targetBeanName}.${ext.targetReadAccessorName} != null ) { <@lib.handleLocalVarNullCheck> ${ext.targetBeanName}.${ext.targetReadAccessorName}.clear(); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardDto.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardDto.java new file mode 100644 index 000000000..10fdda78b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardDto.java @@ -0,0 +1,38 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class CupboardDto { + + private List content; + + public List getContent() { + return content; + } + + public void setContent(List content) { + this.content = content; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntity.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntity.java new file mode 100644 index 000000000..357921553 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntity.java @@ -0,0 +1,38 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class CupboardEntity { + + private List content; + + public List getContent() { + return content; + } + + public void setContent(List content) { + this.content = content; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntityOnlyGetter.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntityOnlyGetter.java new file mode 100644 index 000000000..1552ccc89 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardEntityOnlyGetter.java @@ -0,0 +1,35 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class CupboardEntityOnlyGetter { + + private List content; + + public List getContent() { + return content; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardMapper.java new file mode 100644 index 000000000..de536286c --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/CupboardMapper.java @@ -0,0 +1,36 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import org.mapstruct.CollectionMappingStrategy; +import org.mapstruct.Mapper; +import org.mapstruct.MappingTarget; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper( collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE ) +public interface CupboardMapper { + + CupboardMapper INSTANCE = Mappers.getMapper( CupboardMapper.class ); + + void map( CupboardDto in, @MappingTarget CupboardEntity out ); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ErroneousCupboardMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ErroneousCupboardMapper.java new file mode 100644 index 000000000..4f59ca702 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ErroneousCupboardMapper.java @@ -0,0 +1,36 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import org.mapstruct.CollectionMappingStrategy; +import org.mapstruct.Mapper; +import org.mapstruct.MappingTarget; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper( collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE ) +public interface ErroneousCupboardMapper { + + ErroneousCupboardMapper INSTANCE = Mappers.getMapper( ErroneousCupboardMapper.class ); + + void map( CupboardDto in, @MappingTarget CupboardEntityOnlyGetter out ); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ImmutableTargetTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ImmutableTargetTest.java new file mode 100644 index 000000000..5d331a4f8 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/immutabletarget/ImmutableTargetTest.java @@ -0,0 +1,75 @@ +/** + * Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.collection.immutabletarget; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.Collections; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult; +import org.mapstruct.ap.testutil.compilation.annotation.Diagnostic; +import org.mapstruct.ap.testutil.compilation.annotation.ExpectedCompilationOutcome; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +/** + * + * @author Sjaak Derksen + */ +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses({CupboardDto.class, CupboardEntity.class, CupboardMapper.class}) +@IssueKey( "1126" ) +public class ImmutableTargetTest { + + @Test + public void shouldHandleImmutableTarget() { + + CupboardDto in = new CupboardDto(); + in.setContent( Arrays.asList( "cups", "soucers" ) ); + CupboardEntity out = new CupboardEntity(); + out.setContent( Collections.emptyList() ); + + CupboardMapper.INSTANCE.map( in, out ); + + assertThat( out.getContent() ).isNotNull(); + assertThat( out.getContent() ).containsExactly( "cups", "soucers" ); + } + + @Test + @WithClasses({ + ErroneousCupboardMapper.class, + CupboardEntityOnlyGetter.class + }) + @ExpectedCompilationOutcome( + value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic(type = ErroneousCupboardMapper.class, + kind = javax.tools.Diagnostic.Kind.ERROR, + line = 35, + messageRegExp = "No write accessor found for property \"content\" in target type.") + } + ) + public void testShouldFailOnPropertyMappingNoPropertySetterOnlyGetter() { + } + +}