From e2dbd6ae1d9d09ff147af234e7af9dfc87d2ad9d Mon Sep 17 00:00:00 2001 From: sjaakd Date: Mon, 10 Nov 2014 20:45:53 +0100 Subject: [PATCH] #341 method annotated with qualifier should not qualify for mappings without qualifiedBy --- .../source/selector/QualifierSelector.java | 125 ++++++++++++------ .../qualifier/MapperWithoutQualifiedBy.java | 46 +++++++ .../selection/qualifier/QualifierTest.java | 21 +++ .../qualifier/handwritten/Reverse.java | 32 +++++ 4 files changed, 181 insertions(+), 43 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/MapperWithoutQualifiedBy.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/handwritten/Reverse.java diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java index ef344a506..712f9dd12 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java @@ -36,6 +36,19 @@ import org.mapstruct.ap.prism.QualifierPrism; /** * This selector selects a best match based on qualifiers name. * + *

+ * A method is said to be marked with a qualifier annotation if the class in which it resides is annotated with a + * qualifier annotation or if the method itself is annotated with a qualifier annotation or both. + *

+ * + *

+ * Rules: + *

    + *
  1. 1. If a method is marked with a qualifier annotation, it does not contribute to a match otherwise and + * is hence removed from the list of potential mapping methods
    2. + *
  2. 2. If multiple qualifiers (qualifedBy) are specified, all should match to make a match.
    3. + *
+ *

* @author Sjaak Derksen */ public class QualifierSelector implements MethodSelector { @@ -52,66 +65,92 @@ public class QualifierSelector implements MethodSelector { Type parameterType, Type returnType, List qualifiers, String targetPropertyName) { - List matches = new ArrayList(); if ( qualifiers == null || qualifiers.isEmpty() ) { - return methods; - } + // remove the method marked as qualifier from the list + List nonQualiferAnnotatedMethods = new ArrayList(); + for ( T candidate : methods ) { - for ( T candidate : methods ) { - - if ( !( candidate instanceof SourceMethod ) ) { - continue; - } - - // retrieve annotations - Set combinedAnnotations = new HashSet(); - - // first from the method itself - SourceMethod candidateSM = (SourceMethod) candidate; - List methodAnnotations = candidateSM.getExecutable().getAnnotationMirrors(); - for ( AnnotationMirror methodAnnotation : methodAnnotations ) { - addOnlyWhenQualifier( combinedAnnotations, methodAnnotation ); - } - - // then from the mapper (if declared) - Type mapper = candidate.getDeclaringMapper(); - if ( mapper != null ) { - List mapperAnnotations = mapper.getTypeElement().getAnnotationMirrors(); - for ( AnnotationMirror mapperAnnotation : mapperAnnotations ) { - addOnlyWhenQualifier( combinedAnnotations, mapperAnnotation ); - } - } - - // now count if all qualifiers are machted - int matchingQualifierCounter = 0; - for ( TypeMirror qualifier : qualifiers) { - for ( TypeMirror annotationType : combinedAnnotations ) { - if ( typeUtils.isSameType( qualifier, annotationType ) ) { - matchingQualifierCounter++; - break; + if ( candidate instanceof SourceMethod ) { + Set qualifierAnnotations = getQualifierAnnotations( candidate ); + if ( qualifierAnnotations.isEmpty() ) { + nonQualiferAnnotatedMethods.add( candidate ); } } - } + else { + nonQualiferAnnotatedMethods.add( candidate ); + } - if ( matchingQualifierCounter == qualifiers.size() ) { - // all qualifiers are matched with a qualifying annotation, add candidate - matches.add( candidate ); } - } - if ( !matches.isEmpty() ) { - return matches; + return nonQualiferAnnotatedMethods; } else { - return methods; + + List matches = new ArrayList(); + for ( T candidate : methods ) { + + if ( !( candidate instanceof SourceMethod ) ) { + continue; + } + + // retrieve annotations + Set qualifierAnnotations = getQualifierAnnotations( candidate ); + + // now count if all qualifiers are matched + int matchingQualifierCounter = 0; + for ( TypeMirror qualifier : qualifiers ) { + for ( TypeMirror annotationType : qualifierAnnotations ) { + if ( typeUtils.isSameType( qualifier, annotationType ) ) { + matchingQualifierCounter++; + break; + } + } + } + + if ( matchingQualifierCounter == qualifiers.size() ) { + // all qualifiers are matched with a qualifying annotation, add candidate + matches.add( candidate ); + } + } + if ( !matches.isEmpty() ) { + return matches; + } + else { + return methods; + } } } + private Set getQualifierAnnotations( Method candidate ) { + + // retrieve annotations + Set qualiferAnnotations = new HashSet(); + + // first from the method itself + SourceMethod candidateSM = (SourceMethod) candidate; + List methodAnnotations = candidateSM.getExecutable().getAnnotationMirrors(); + for ( AnnotationMirror methodAnnotation : methodAnnotations ) { + addOnlyWhenQualifier( qualiferAnnotations, methodAnnotation ); + } + + // then from the mapper (if declared) + Type mapper = candidate.getDeclaringMapper(); + if ( mapper != null ) { + List mapperAnnotations = mapper.getTypeElement().getAnnotationMirrors(); + for ( AnnotationMirror mapperAnnotation : mapperAnnotations ) { + addOnlyWhenQualifier( qualiferAnnotations, mapperAnnotation ); + } + } + + return qualiferAnnotations; + } + private void addOnlyWhenQualifier( Set annotationSet, AnnotationMirror candidate ) { // only add the candidate annotation when the candidate itself has the annotation 'Qualifier' if ( QualifierPrism.getInstanceOn( candidate.getAnnotationType().asElement() ) != null ) { annotationSet.add( candidate.getAnnotationType() ); } } + } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/MapperWithoutQualifiedBy.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/MapperWithoutQualifiedBy.java new file mode 100644 index 000000000..c046a7280 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/MapperWithoutQualifiedBy.java @@ -0,0 +1,46 @@ +/** + * Copyright 2012-2014 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.selection.qualifier; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.Mappings; +import org.mapstruct.ap.test.selection.qualifier.bean.GermanRelease; +import org.mapstruct.ap.test.selection.qualifier.bean.OriginalRelease; +import org.mapstruct.ap.test.selection.qualifier.handwritten.Facts; +import org.mapstruct.ap.test.selection.qualifier.handwritten.Reverse; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper( uses = { Facts.class, Reverse.class } ) +public interface MapperWithoutQualifiedBy { + + MapperWithoutQualifiedBy INSTANCE = Mappers.getMapper( MapperWithoutQualifiedBy.class ); + + @Mappings( { + @Mapping( target = "title", source = "title" ), + @Mapping( target = "keyWords", ignore = true ), + @Mapping( target = "facts", ignore = true ) + } ) + GermanRelease map( OriginalRelease movies ); + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java index e262049b5..6c82db2f6 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java @@ -36,6 +36,7 @@ import org.mapstruct.ap.test.selection.qualifier.annotation.NonQualifierAnnotate import org.mapstruct.ap.test.selection.qualifier.annotation.TitleTranslator; import org.mapstruct.ap.test.selection.qualifier.handwritten.Facts; import org.mapstruct.ap.test.selection.qualifier.handwritten.PlotWords; +import org.mapstruct.ap.test.selection.qualifier.handwritten.Reverse; import org.mapstruct.ap.test.selection.qualifier.handwritten.SomeOtherMapper; import org.mapstruct.ap.test.selection.qualifier.handwritten.YetAnotherMapper; import org.mapstruct.ap.testutil.IssueKey; @@ -116,4 +117,24 @@ public class QualifierTest { public void shouldNotProduceMatchingMethod() { } + + @Test + @WithClasses( { + MapperWithoutQualifiedBy.class, + Facts.class, + EnglishToGerman.class, + Reverse.class + } ) + @IssueKey( "341" ) + public void shouldNotUseQualifierAnnotatedMethod() { + + + OriginalRelease foreignMovies = new OriginalRelease(); + foreignMovies.setTitle( "Sixth Sense, The" ); + + GermanRelease result = MapperWithoutQualifiedBy.INSTANCE.map( foreignMovies ); + assertThat( result ).isNotNull(); + assertThat( result.getTitle() ).isEqualTo( "ehT ,esneS htxiS"); + + } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/handwritten/Reverse.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/handwritten/Reverse.java new file mode 100644 index 000000000..537cad10e --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/handwritten/Reverse.java @@ -0,0 +1,32 @@ +/** + * Copyright 2012-2014 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.selection.qualifier.handwritten; + +/** + * + * @author Sjaak Derksen + */ +public class Reverse { + + public String reverse( String in ) { + StringBuilder b = new StringBuilder(in); + return b.reverse().toString(); + } + +}