From d9821a0cc89c8276090f78918db59548d15ac3ed Mon Sep 17 00:00:00 2001 From: Andreas Gudian Date: Tue, 11 Jul 2017 23:03:44 +0200 Subject: [PATCH] #1242 Fix favoring of a single factory method with source params before others without source params. From the Javadoc of @ObjectFactory: If there are two factory methods, both serving the same type, one with no parameters and one taking sources as input, then the one with the source parameters is favored. If there are multiple such factories, an ambiguity error is shown. --- .../selector/FactoryParameterSelector.java | 62 ++++++++++++++ .../source/selector/MethodSelectors.java | 3 +- ...roneousIssue1242MapperMultipleSources.java | 39 +++++++++ .../ap/test/bugs/_1242/Issue1242Mapper.java | 38 +++++++++ .../ap/test/bugs/_1242/Issue1242Test.java | 83 +++++++++++++++++++ .../mapstruct/ap/test/bugs/_1242/SourceA.java | 34 ++++++++ .../mapstruct/ap/test/bugs/_1242/SourceB.java | 25 ++++++ .../mapstruct/ap/test/bugs/_1242/TargetA.java | 34 ++++++++ .../mapstruct/ap/test/bugs/_1242/TargetB.java | 34 ++++++++ .../ap/test/bugs/_1242/TargetFactories.java | 43 ++++++++++ 10 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/FactoryParameterSelector.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/ErroneousIssue1242MapperMultipleSources.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceA.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceB.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetA.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetB.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetFactories.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/FactoryParameterSelector.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/FactoryParameterSelector.java new file mode 100644 index 000000000..8571f5183 --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/FactoryParameterSelector.java @@ -0,0 +1,62 @@ +/** + * 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.internal.model.source.selector; + +import java.util.ArrayList; +import java.util.List; + +import org.mapstruct.ap.internal.model.common.Type; +import org.mapstruct.ap.internal.model.source.Method; + +/** + * For factory methods, the candidate list is checked if it contains a method with a source parameter which is to be + * favored compared to factory methods without a source parameter. It returns the original list of candidates in case of + * ambiguities. + * + * @author Andreas Gudian + */ +public class FactoryParameterSelector implements MethodSelector { + + @Override + public List> getMatchingMethods(Method mappingMethod, + List> methods, + ListsourceTypes, + Type targetType, + SelectionCriteria criteria) { + if ( !criteria.isObjectFactoryRequired() || methods.size() <= 1 ) { + return methods; + } + + List> sourceParamFactoryMethods = new ArrayList>( methods.size() ); + + for ( SelectedMethod candidate : methods ) { + if ( !candidate.getMethod().getSourceParameters().isEmpty() ) { + sourceParamFactoryMethods.add( candidate ); + } + } + + if ( sourceParamFactoryMethods.size() == 1 ) { + // there is exactly one candidate with source params, so favor that one. + return sourceParamFactoryMethods; + } + + // let the caller produce an ambiguity error referencing all possibly matching methods + return methods; + } +} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java index 4fca9c9c8..e651082f1 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java @@ -46,7 +46,8 @@ public class MethodSelectors { new TargetTypeSelector( typeUtils, elementUtils ), new XmlElementDeclSelector( typeUtils, elementUtils ), new InheritanceSelector(), - new CreateOrUpdateSelector() ); + new CreateOrUpdateSelector(), + new FactoryParameterSelector() ); } /** diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/ErroneousIssue1242MapperMultipleSources.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/ErroneousIssue1242MapperMultipleSources.java new file mode 100644 index 000000000..3732aca92 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/ErroneousIssue1242MapperMultipleSources.java @@ -0,0 +1,39 @@ +/** + * 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.bugs._1242; + +import org.mapstruct.Mapper; +import org.mapstruct.ObjectFactory; + +/** + * Results in an ambiguous factory method error, as there are two methods with matching source types available. + * + * @author Andreas Gudian + */ +@Mapper(uses = TargetFactories.class) +public abstract class ErroneousIssue1242MapperMultipleSources { + abstract TargetA toTargetA(SourceA source); + + abstract TargetB toTargetB(SourceB source); + + @ObjectFactory + protected TargetB anotherTargetBCreator(SourceB source) { + throw new RuntimeException( "never to be called" ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Mapper.java new file mode 100644 index 000000000..45e7547fd --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Mapper.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.bugs._1242; + +import org.mapstruct.Mapper; +import org.mapstruct.MappingTarget; + +/** + * Test mapper for properly resolving the best fitting factory method + * + * @author Andreas Gudian + */ +@Mapper(uses = TargetFactories.class) +public abstract class Issue1242Mapper { + abstract TargetA toTargetA(SourceA source); + + abstract TargetB toTargetB(SourceB source); + + abstract void mergeA(SourceA source, @MappingTarget TargetA target); + + abstract void mergeB(SourceB source, @MappingTarget TargetB target); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Test.java new file mode 100644 index 000000000..2a65e99d9 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/Issue1242Test.java @@ -0,0 +1,83 @@ +/** + * 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.bugs._1242; + +import static org.assertj.core.api.Assertions.assertThat; + +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; +import org.mapstruct.factory.Mappers; + +/** + * Tests that if multiple factory methods are applicable but only one of them has a source parameter, the one with the + * source param is chosen. + * + * @author Andreas Gudian + */ +@IssueKey("1242") +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses({ + Issue1242Mapper.class, + SourceA.class, + SourceB.class, + TargetA.class, + TargetB.class, + TargetFactories.class +}) +public class Issue1242Test { + @Test + public void factoryMethodWithSourceParamIsChosen() { + SourceA sourceA = new SourceA(); + sourceA.setB( new SourceB() ); + + TargetA targetA = new TargetA(); + Mappers.getMapper( Issue1242Mapper.class ).mergeA( sourceA, targetA ); + + assertThat( targetA.getB() ).isNotNull(); + assertThat( targetA.getB().getPassedViaConstructor() ).isEqualTo( "created by factory" ); + + targetA = Mappers.getMapper( Issue1242Mapper.class ).toTargetA( sourceA ); + + assertThat( targetA.getB() ).isNotNull(); + assertThat( targetA.getB().getPassedViaConstructor() ).isEqualTo( "created by factory" ); + } + + @Test + @WithClasses(ErroneousIssue1242MapperMultipleSources.class) + @ExpectedCompilationOutcome(value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic(type = ErroneousIssue1242MapperMultipleSources.class, + kind = javax.tools.Diagnostic.Kind.ERROR, + line = 33, + messageRegExp = "Ambiguous factory methods found for creating .*TargetB:" + + " .*TargetB anotherTargetBCreator\\(.*SourceB source\\)," + + " .*TargetB .*TargetFactories\\.createTargetB\\(.*SourceB source," + + " @TargetType .*Class<.*TargetB> clazz\\)," + + " .*TargetB .*TargetFactories\\.createTargetB\\(@TargetType java.lang.Class<.*TargetB> clazz\\)," + + " .*TargetB .*TargetFactories\\.createTargetB\\(\\).") + }) + public void ambiguousMethodErrorForTwoFactoryMethodsWithSourceParam() { + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceA.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceA.java new file mode 100644 index 000000000..874732508 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceA.java @@ -0,0 +1,34 @@ +/** + * 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.bugs._1242; + +/** + * @author Andreas Gudian + */ +class SourceA { + private SourceB b; + + public SourceB getB() { + return b; + } + + public void setB(SourceB b) { + this.b = b; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceB.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceB.java new file mode 100644 index 000000000..caf301c80 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/SourceB.java @@ -0,0 +1,25 @@ +/** + * 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.bugs._1242; + +/** + * @author Andreas Gudian + */ +class SourceB { +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetA.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetA.java new file mode 100644 index 000000000..a100c51a7 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetA.java @@ -0,0 +1,34 @@ +/** + * 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.bugs._1242; + +/** + * @author Andreas Gudian + */ +class TargetA { + private TargetB b; + + public TargetB getB() { + return b; + } + + public void setB(TargetB b) { + this.b = b; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetB.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetB.java new file mode 100644 index 000000000..f6b690073 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetB.java @@ -0,0 +1,34 @@ +/** + * 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.bugs._1242; + +/** + * @author Andreas Gudian + */ +class TargetB { + private final String passedViaConstructor; + + TargetB(String passedViaConstructor) { + this.passedViaConstructor = passedViaConstructor; + } + + String getPassedViaConstructor() { + return passedViaConstructor; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetFactories.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetFactories.java new file mode 100644 index 000000000..b4476bcd2 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1242/TargetFactories.java @@ -0,0 +1,43 @@ +/** + * 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.bugs._1242; + +import org.mapstruct.ObjectFactory; +import org.mapstruct.TargetType; + +/** + * Contains non-conflicting factory methods for {@link TargetB}. + * + * @author Andreas Gudian + */ +public class TargetFactories { + + @ObjectFactory + protected TargetB createTargetB(SourceB source, @TargetType Class clazz) { + return new TargetB( "created by factory" ); + } + + protected TargetB createTargetB(@TargetType Class clazz) { + throw new RuntimeException( "This method is not to be called" ); + } + + protected TargetB createTargetB() { + throw new RuntimeException( "This method is not to be called" ); + } +}