#2952 Do not treat a getter as an alternative write accessor when using CollectionMappingStrategy#TARGET_IMMUTABLE

This commit is contained in:
Filip Hrisafov 2022-11-03 23:29:47 +01:00 committed by GitHub
parent 93f7c3b8ea
commit 16e3ceadec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 159 additions and 17 deletions

View File

@ -7,6 +7,54 @@ package org.mapstruct;
/**
* Strategy for propagating the value of collection-typed properties from source to target.
* <p>
* In the table below, the dash {@code -} indicates a property name.
* Next, the trailing {@code s} indicates the plural form.
* The table explains the options and how they are applied to the presence / absence of a
* {@code set-s}, {@code add-} and / or {@code get-s} method on the target object.
* <table>
* <caption>Collection mapping strategy options</caption>
* <tr>
* <th>Option</th>
* <th>Only target set-s Available</th>
* <th>Only target add- Available</th>
* <th>Both set-s/add- Available</th>
* <th>No set-s/add- Available</th>
* <th>Existing Target ({@code @TargetType})</th>
* </tr>
* <tr>
* <td>{@link #ACCESSOR_ONLY}</td>
* <td>set-s</td>
* <td>get-s</td>
* <td>set-s</td>
* <td>get-s</td>
* <td>get-s</td>
* </tr>
* <tr>
* <td>{@link #SETTER_PREFERRED}</td>
* <td>set-s</td>
* <td>add-</td>
* <td>set-s</td>
* <td>get-s</td>
* <td>get-s</td>
* </tr>
* <tr>
* <td>{@link #ADDER_PREFERRED}</td>
* <td>set-s</td>
* <td>add-</td>
* <td>add-</td>
* <td>get-s</td>
* <td>get-s</td>
* </tr>
* <tr>
* <td>{@link #TARGET_IMMUTABLE}</td>
* <td>set-s</td>
* <td>exception</td>
* <td>set-s</td>
* <td>exception</td>
* <td>set-s</td>
* </tr>
* </table>
*
* @author Sjaak Derksen
*/

View File

@ -788,6 +788,13 @@ public class Type extends ModelElement implements Comparable<Type> {
// an adder has been found (according strategy) so overrule current choice.
candidate = adderMethod;
}
if ( cmStrategy == CollectionMappingStrategyGem.TARGET_IMMUTABLE
&& candidate.getAccessorType() == AccessorType.GETTER ) {
// If the collection mapping strategy is target immutable
// then the getter method cannot be used as a setter
continue;
}
}
else if ( candidate.getAccessorType() == AccessorType.FIELD && ( Executables.isFinal( candidate ) ||
result.containsKey( targetPropertyName ) ) ) {

View File

@ -0,0 +1,62 @@
/*
* 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._2952;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.mapstruct.CollectionMappingStrategy;
import org.mapstruct.Mapper;
import org.mapstruct.factory.Mappers;
/**
* @author Filip Hrisafov
*/
@Mapper(collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE)
public interface Issue2952Mapper {
Issue2952Mapper INSTANCE = Mappers.getMapper( Issue2952Mapper.class );
Target map(Source source);
class Source {
private final String value;
public Source(String value) {
this.value = value;
}
public String getValue() {
return value;
}
}
class Target {
private final Map<String, String> attributes = new HashMap<>();
private final List<String> values = new ArrayList<>();
private String value;
public Map<String, String> getAttributes() {
return Collections.unmodifiableMap( attributes );
}
public List<String> getValues() {
return Collections.unmodifiableList( values );
}
public String getValue() {
return value;
}
public void setValue(String value) {
this.value = value;
}
}
}

View File

@ -0,0 +1,29 @@
/*
* 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._2952;
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 Filip Hrisafov
*/
@IssueKey("2952")
@WithClasses({
Issue2952Mapper.class
})
class Issue2952Test {
@ProcessorTest
void shouldCorrectIgnoreImmutableIterable() {
Issue2952Mapper.Target target = Issue2952Mapper.INSTANCE.map( new Issue2952Mapper.Source( "test" ) );
assertThat( target.getValue() ).isEqualTo( "test" );
}
}

View File

@ -5,6 +5,7 @@
*/
package org.mapstruct.ap.test.collection.immutabletarget;
import java.util.ArrayList;
import java.util.List;
/**
@ -13,7 +14,7 @@ import java.util.List;
*/
public class CupboardEntityOnlyGetter {
private List<String> content;
private List<String> content = new ArrayList<>();
public List<String> getContent() {
return content;

View File

@ -15,9 +15,9 @@ import org.mapstruct.factory.Mappers;
* @author Sjaak Derksen
*/
@Mapper( collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE )
public interface ErroneousCupboardMapper {
public interface CupboardNoSetterMapper {
ErroneousCupboardMapper INSTANCE = Mappers.getMapper( ErroneousCupboardMapper.class );
CupboardNoSetterMapper INSTANCE = Mappers.getMapper( CupboardNoSetterMapper.class );
void map( CupboardDto in, @MappingTarget CupboardEntityOnlyGetter out );
}

View File

@ -11,9 +11,6 @@ import java.util.Collections;
import org.mapstruct.ap.testutil.IssueKey;
import org.mapstruct.ap.testutil.ProcessorTest;
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 static org.assertj.core.api.Assertions.assertThat;
@ -41,19 +38,17 @@ public class ImmutableProductTest {
@ProcessorTest
@WithClasses({
ErroneousCupboardMapper.class,
CupboardNoSetterMapper.class,
CupboardEntityOnlyGetter.class
})
@ExpectedCompilationOutcome(
value = CompilationResult.FAILED,
diagnostics = {
@Diagnostic(type = ErroneousCupboardMapper.class,
kind = javax.tools.Diagnostic.Kind.ERROR,
line = 22,
message = "No write accessor found for property \"content\" in target type.")
}
)
public void testShouldFailOnPropertyMappingNoPropertySetterOnlyGetter() {
public void shouldIgnoreImmutableTarget() {
CupboardDto in = new CupboardDto();
in.setContent( Arrays.asList( "flour", "peas" ) );
CupboardEntityOnlyGetter out = new CupboardEntityOnlyGetter();
out.getContent().add( "bread" );
CupboardNoSetterMapper.INSTANCE.map( in, out );
assertThat( out.getContent() ).containsExactly( "bread" );
}
}