From 197dd4327afe1f9cf55a10579491aa3fe162fb0b Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Mon, 15 Mar 2021 00:21:24 +0100 Subject: [PATCH] #2339 Polish PR #2362 Use MappingEntry for defaultTarget and nullTarget in ValueMappingMethod to simplify certain things --- .../ap/internal/model/ValueMappingMethod.java | 76 +++++++------------ .../ap/internal/model/ValueMappingMethod.ftl | 6 +- .../mapstruct/ap/test/gem/ConstantTest.java | 1 + .../value/spi/CustomEnumMappingStrategy.java | 21 +++++ .../spi/CustomEnumMappingStrategyTest.java | 23 ++++++ .../value/spi/CustomThrowingCheeseMapper.java | 22 ++++++ .../value/spi/CustomThrowingCheeseType.java | 17 +++++ .../value/spi/CustomThrowingEnumMarker.java | 12 +++ 8 files changed, 125 insertions(+), 53 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseType.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingEnumMarker.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ValueMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ValueMappingMethod.java index e292d38e3..6e7fe3f0d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ValueMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ValueMappingMethod.java @@ -42,10 +42,8 @@ import static org.mapstruct.ap.internal.util.Collections.first; public class ValueMappingMethod extends MappingMethod { private final List valueMappings; - private final String defaultTarget; - private final String nullTarget; - private boolean nullAsException; - private boolean defaultAsException; + private final MappingEntry defaultTarget; + private final MappingEntry nullTarget; private final Type unexpectedValueMappingException; @@ -122,12 +120,10 @@ public class ValueMappingMethod extends MappingMethod { return new ValueMappingMethod( method, mappingEntries, valueMappings.nullValueTarget, - valueMappings.hasNullValueAsException, valueMappings.defaultTargetValue, determineUnexpectedValueMappingException(), beforeMappingMethods, - afterMappingMethods, - determineExceptionMappingForDefaultCase() + afterMappingMethods ); } @@ -417,6 +413,7 @@ public class ValueMappingMethod extends MappingMethod { foundIncorrectMapping = true; } else if ( valueMappings.nullTarget == null && valueMappings.nullValueTarget != null + && !valueMappings.nullValueTarget.equals( THROW_EXCEPTION ) && !targetEnumConstants.contains( valueMappings.nullValueTarget ) ) { // if there is no nullTarget, but nullValueTarget has a value it means that there was an SPI // which returned an enum for the target enum @@ -433,28 +430,13 @@ public class ValueMappingMethod extends MappingMethod { } private Type determineUnexpectedValueMappingException() { - boolean noDefaultValueForSwitchCase = !valueMappings.hasDefaultValue; - if ( noDefaultValueForSwitchCase || valueMappings.hasAtLeastOneExceptionValue - || valueMappings.hasNullValueAsException ) { - TypeMirror unexpectedValueMappingException = enumMapping.getUnexpectedValueMappingException(); - if ( unexpectedValueMappingException != null ) { - return ctx.getTypeFactory().getType( unexpectedValueMappingException ); - } - - return ctx.getTypeFactory() - .getType( ctx.getEnumMappingStrategy().getUnexpectedValueMappingExceptionType() ); + TypeMirror unexpectedValueMappingException = enumMapping.getUnexpectedValueMappingException(); + if ( unexpectedValueMappingException != null ) { + return ctx.getTypeFactory().getType( unexpectedValueMappingException ); } - return null; - } - - private boolean determineExceptionMappingForDefaultCase() { - if ( valueMappings.hasDefaultValue ) { - return THROW_EXCEPTION.equals( valueMappings.defaultTargetValue ); - } - else { - return true; - } + return ctx.getTypeFactory() + .getType( ctx.getEnumMappingStrategy().getUnexpectedValueMappingExceptionType() ); } } @@ -493,7 +475,6 @@ public class ValueMappingMethod extends MappingMethod { boolean hasMapAnyUnmapped = false; boolean hasMapAnyRemaining = false; boolean hasDefaultValue = false; - boolean hasNullValueAsException = false; boolean hasAtLeastOneExceptionValue = false; ValueMappings(List valueMappings) { @@ -501,22 +482,19 @@ public class ValueMappingMethod extends MappingMethod { for ( ValueMappingOptions valueMapping : valueMappings ) { if ( ANY_REMAINING.equals( valueMapping.getSource() ) ) { defaultTarget = valueMapping; - defaultTargetValue = getValue( defaultTarget ); + defaultTargetValue = defaultTarget.getTarget(); hasDefaultValue = true; hasMapAnyRemaining = true; } else if ( ANY_UNMAPPED.equals( valueMapping.getSource() ) ) { defaultTarget = valueMapping; - defaultTargetValue = getValue( defaultTarget ); + defaultTargetValue = defaultTarget.getTarget(); hasDefaultValue = true; hasMapAnyUnmapped = true; } else if ( NULL.equals( valueMapping.getSource() ) ) { nullTarget = valueMapping; nullValueTarget = getValue( nullTarget ); - if ( THROW_EXCEPTION.equals( nullValueTarget ) ) { - hasNullValueAsException = true; - } } else { regularValueMappings.add( valueMapping ); @@ -536,17 +514,14 @@ public class ValueMappingMethod extends MappingMethod { private ValueMappingMethod(Method method, List enumMappings, String nullTarget, - boolean hasNullTargetAsException, String defaultTarget, Type unexpectedValueMappingException, List beforeMappingMethods, - List afterMappingMethods, boolean defaultAsException) { + List afterMappingMethods) { super( method, beforeMappingMethods, afterMappingMethods ); this.valueMappings = enumMappings; - this.nullTarget = nullTarget; - this.nullAsException = hasNullTargetAsException; - this.defaultTarget = defaultTarget; - this.defaultAsException = defaultAsException; + this.nullTarget = new MappingEntry( null, nullTarget ); + this.defaultTarget = new MappingEntry( null, defaultTarget != null ? defaultTarget : THROW_EXCEPTION); this.unexpectedValueMappingException = unexpectedValueMappingException; this.overridden = method.overridesMethod(); } @@ -556,36 +531,37 @@ public class ValueMappingMethod extends MappingMethod { Set importTypes = super.getImportTypes(); if ( unexpectedValueMappingException != null && !unexpectedValueMappingException.isJavaLangType() ) { - importTypes.addAll( unexpectedValueMappingException.getImportTypes() ); + if ( defaultTarget.isTargetAsException() || nullTarget.isTargetAsException() || + hasMappingWithTargetAsException() ) { + importTypes.addAll( unexpectedValueMappingException.getImportTypes() ); + } } return importTypes; } + protected boolean hasMappingWithTargetAsException() { + return getValueMappings() + .stream() + .anyMatch( MappingEntry::isTargetAsException ); + } + public List getValueMappings() { return valueMappings; } - public String getDefaultTarget() { + public MappingEntry getDefaultTarget() { return defaultTarget; } - public String getNullTarget() { + public MappingEntry getNullTarget() { return nullTarget; } - public boolean isNullAsException() { - return nullAsException; - } - public Type getUnexpectedValueMappingException() { return unexpectedValueMappingException; } - public boolean isDefaultAsException() { - return defaultAsException; - } - public Parameter getSourceParameter() { return first( getSourceParameters() ); } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/ValueMappingMethod.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/ValueMappingMethod.ftl index 2844b43e3..4d961c40a 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/ValueMappingMethod.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/ValueMappingMethod.ftl @@ -15,7 +15,7 @@ if ( ${sourceParameter.name} == null ) { - <#if nullAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} );<#else>return <@writeTarget target=nullTarget/>; + <#if nullTarget.targetAsException>throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} );<#else>return <@writeTarget target=nullTarget.target/>; } <@includeModel object=resultType/> ${resultName}; @@ -25,7 +25,7 @@ case <@writeSource source=valueMapping.source/>: <#if valueMapping.targetAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} );<#else>${resultName} = <@writeTarget target=valueMapping.target/>; break; - default: <#if defaultAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} )<#else>${resultName} = <@writeTarget target=defaultTarget/>; + default: <#if defaultTarget.targetAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} )<#else>${resultName} = <@writeTarget target=defaultTarget.target/>; } <#list beforeMappingReferencesWithMappingTarget as callback> <#if callback_index = 0> @@ -40,7 +40,7 @@ <@includeModel object=callback targetBeanName=resultName targetType=resultType/> - <#if !(valueMappings.empty && unexpectedValueMappingException??)> + <#if !(valueMappings.empty && defaultTarget.targetAsException)> return ${resultName}; } diff --git a/processor/src/test/java/org/mapstruct/ap/test/gem/ConstantTest.java b/processor/src/test/java/org/mapstruct/ap/test/gem/ConstantTest.java index 69fa56a31..711d6d467 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/gem/ConstantTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/gem/ConstantTest.java @@ -23,6 +23,7 @@ public class ConstantTest { assertThat( MappingConstants.ANY_REMAINING ).isEqualTo( MappingConstantsGem.ANY_REMAINING ); assertThat( MappingConstants.ANY_UNMAPPED ).isEqualTo( MappingConstantsGem.ANY_UNMAPPED ); assertThat( MappingConstants.NULL ).isEqualTo( MappingConstantsGem.NULL ); + assertThat( MappingConstants.THROW_EXCEPTION ).isEqualTo( MappingConstantsGem.THROW_EXCEPTION ); assertThat( MappingConstants.SUFFIX_TRANSFORMATION ).isEqualTo( MappingConstantsGem.SUFFIX_TRANSFORMATION ); assertThat( MappingConstants.STRIP_SUFFIX_TRANSFORMATION ) .isEqualTo( MappingConstantsGem.STRIP_SUFFIX_TRANSFORMATION ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategy.java b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategy.java index b505dd640..7cf7a2493 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategy.java +++ b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategy.java @@ -8,6 +8,7 @@ package org.mapstruct.ap.test.value.spi; import javax.lang.model.element.TypeElement; import javax.lang.model.type.TypeMirror; +import org.mapstruct.MappingConstants; import org.mapstruct.ap.internal.gem.MappingConstantsGem; import org.mapstruct.ap.spi.DefaultEnumMappingStrategy; import org.mapstruct.ap.spi.EnumMappingStrategy; @@ -20,6 +21,10 @@ public class CustomEnumMappingStrategy extends DefaultEnumMappingStrategy implem @Override public String getDefaultNullEnumConstant(TypeElement enumType) { + if ( isCustomThrowingEnum( enumType ) ) { + return MappingConstants.THROW_EXCEPTION; + } + if ( isCustomEnum( enumType ) ) { return "UNSPECIFIED"; } @@ -29,6 +34,10 @@ public class CustomEnumMappingStrategy extends DefaultEnumMappingStrategy implem @Override public String getEnumConstant(TypeElement enumType, String enumConstant) { + if ( isCustomThrowingEnum( enumType ) ) { + return getCustomEnumConstant( enumConstant ); + } + if ( isCustomEnum( enumType ) ) { return getCustomEnumConstant( enumConstant ); } @@ -53,6 +62,18 @@ public class CustomEnumMappingStrategy extends DefaultEnumMappingStrategy implem return false; } + protected boolean isCustomThrowingEnum(TypeElement enumType) { + for ( TypeMirror enumTypeInterface : enumType.getInterfaces() ) { + if ( typeUtils.asElement( enumTypeInterface ) + .getSimpleName() + .contentEquals( "CustomThrowingEnumMarker" ) ) { + return true; + } + } + + return false; + } + @Override protected Class getUnexpectedValueMappingExceptionClass() { return CustomIllegalArgumentException.class; diff --git a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategyTest.java b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategyTest.java index 9cce52d7b..313a7553b 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategyTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomEnumMappingStrategyTest.java @@ -9,12 +9,14 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mapstruct.ap.test.value.CustomIllegalArgumentException; +import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.WithClasses; import org.mapstruct.ap.testutil.WithServiceImplementation; import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; import org.mapstruct.ap.testutil.runner.GeneratedSource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * @author Filip Hrisafov @@ -24,6 +26,8 @@ import static org.assertj.core.api.Assertions.assertThat; CheeseType.class, CustomCheeseType.class, CustomEnumMarker.class, + CustomThrowingCheeseType.class, + CustomThrowingEnumMarker.class, CustomIllegalArgumentException.class, }) @WithServiceImplementation(CustomEnumMappingStrategy.class) @@ -122,4 +126,23 @@ public class CustomEnumMappingStrategyTest { assertThat( mapper.mapStringToCustom( "ROQUEFORT" ) ).isEqualTo( CustomCheeseType.CUSTOM_ROQUEFORT ); assertThat( mapper.mapStringToCustom( "UNKNOWN" ) ).isEqualTo( CustomCheeseType.CUSTOM_BRIE ); } + + @Test + @IssueKey("2339") + @WithClasses({ + CustomThrowingCheeseMapper.class + }) + public void shouldApplyCustomEnumMappingStrategyWithThrowingException() { + CustomThrowingCheeseMapper mapper = CustomThrowingCheeseMapper.INSTANCE; + + // CustomCheeseType -> CustomThrowingCheeseType + assertThatThrownBy( () -> mapper.map( (CheeseType) null ) ) + .isInstanceOf( CustomIllegalArgumentException.class ) + .hasMessage( "Unexpected enum constant: null" ); + assertThat( mapper.map( CheeseType.BRIE ) ).isEqualTo( CustomThrowingCheeseType.CUSTOM_BRIE ); + + // CustomThrowingCheeseType -> CustomCheeseType + assertThat( mapper.map( (CustomThrowingCheeseType) null ) ).isNull(); + assertThat( mapper.map( CustomThrowingCheeseType.CUSTOM_BRIE ) ).isEqualTo( CheeseType.BRIE ); + } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseMapper.java b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseMapper.java new file mode 100644 index 000000000..a62dba55f --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseMapper.java @@ -0,0 +1,22 @@ +/* + * 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.value.spi; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface CustomThrowingCheeseMapper { + + CustomThrowingCheeseMapper INSTANCE = Mappers.getMapper( CustomThrowingCheeseMapper.class ); + + CustomThrowingCheeseType map(CheeseType cheese); + + CheeseType map(CustomThrowingCheeseType cheese); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseType.java b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseType.java new file mode 100644 index 000000000..f97b39a6d --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingCheeseType.java @@ -0,0 +1,17 @@ +/* + * 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.value.spi; + +/** + * @author Filip Hrisafov + */ +public enum CustomThrowingCheeseType implements CustomThrowingEnumMarker { + + UNSPECIFIED, + CUSTOM_BRIE, + CUSTOM_ROQUEFORT, + UNRECOGNIZED, +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingEnumMarker.java b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingEnumMarker.java new file mode 100644 index 000000000..aa5943bbc --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/value/spi/CustomThrowingEnumMarker.java @@ -0,0 +1,12 @@ +/* + * 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.value.spi; + +/** + * @author Filip Hrisafov + */ +public interface CustomThrowingEnumMarker { +}