#39 - Add unused report and factory method selection

publicissapient-france · Mar 16, 2016 · 800e8dc · 800e8dc
1 parent 94010e5
commit 800e8dc
Show file tree

Hide file tree

Showing 13 changed files with 237 additions and 27 deletions.
diff --git a/processor/src/main/java/fr/xebia/extras/selma/codegen/FactoryWrapper.java b/processor/src/main/java/fr/xebia/extras/selma/codegen/FactoryWrapper.java
@@ -46,7 +46,7 @@ public class FactoryWrapper {
     private final Element annotatedElement;
     private final AnnotationWrapper annotationWrapper;
     private final MapperGeneratorContext context;
-    private final HashMap unusedFactories;
+    private final HashMap<Factory, String> unusedFactories;
     private final IoC ioC;
     private final List<TypeElement> factoryFields;
     private final ArrayList<Factory> staticFactories;
@@ -205,21 +205,38 @@ public void emitFactoryFields(JavaWriter writer, boolean assign) throws IOExcept
         }
     }
 
+    public void reportUnused() {
+        for (String field : unusedFactories.values()) {
+            context.warn(annotatedElement, "Factory method \"%s\" is never used", field);
+        }
+    }
+
+
+    /**
+     * Called to search for a matching factory and return the corresponding MappingSourceNode
+     * @param inOutType         the type association we are currently mapping
+     * @param outBeanWrapper    The Bean wrapper of the bean we want to build
+     * @return  Null if no factory matches was found or the corresponding MappingSourceNode
+     */
     public MappingSourceNode generateNewInstanceSourceNodes(InOutType inOutType, BeanWrapper outBeanWrapper) {
 
+        TypeMirror out = inOutType.out();
         for (Factory factory : staticFactories){
-            if(factory.provide(inOutType.out())){
+            if(factory.provide(out)){
                 unusedFactories.remove(factory);
                 return factory.buildNewInstanceSourceNode(inOutType);
             }
         }
+        Factory found = null;
         for (Factory factory : genericFactories){
-            if(factory.provide(inOutType.out())){
-                unusedFactories.remove(factory);
-                return factory.buildNewInstanceSourceNode(inOutType);
+            if(factory.provide(out) && factory.isLowerClass(found)){
+                found = factory;
             }
         }
-
+        if (found != null){
+            unusedFactories.remove(found);
+            return found.buildNewInstanceSourceNode(inOutType);
+        }
         return null;
     }
 
@@ -257,19 +274,36 @@ public boolean provide(TypeMirror out) {
                 if (context.type.isSameType(out, method.returnType())) {
                     return true;
                 }
-            } else if (context.type.isAssignable(out, ((TypeVariable)method.returnType()).getUpperBound())){
+            } else if (context.type.isAssignable(out, getUpperBound())){
                 return true;
             }
             return false;
         }
 
         public MappingSourceNode buildNewInstanceSourceNode(InOutType inOutType) {
             if (!method.hasTypeParameter()){
+
                 return callStaticFactoryOut(inOutType, methodCall);
             } else {
                 return callGenericFactoryOut(inOutType, methodCall);
             }
         }
+
+        public boolean isLowerClass(Factory factory) {
+            boolean res;
+            if (factory == null){
+                res = true;
+            } else {
+                TypeMirror upperBound = getUpperBound();
+                TypeMirror upperBoundCmp = factory.getUpperBound();
+                res = context.type.isAssignable(upperBound, upperBoundCmp);
+            }
+            return res;
+        }
+
+        private TypeMirror getUpperBound() {
+            return ((TypeVariable) method.returnType()).getUpperBound();
+        }
     }
 }
 
diff --git a/processor/src/main/java/fr/xebia/extras/selma/codegen/MapperWrapper.java b/processor/src/main/java/fr/xebia/extras/selma/codegen/MapperWrapper.java
@@ -150,6 +150,7 @@ public void reportUnused() {
         // Report unused enumMapper
         enumMappers.reportUnused();
         immutablesMapper.reportUnused();
+        factory.reportUnused();
     }
 
     public void emitCustomMappersFields(JavaWriter writer, boolean assign) throws IOException {

diff --git a/processor/src/test/java/fr/xebia/extras/selma/beans/AddressIn.java b/processor/src/test/java/fr/xebia/extras/selma/beans/AddressIn.java
@@ -23,7 +23,6 @@
  * User: slemesle
  * Date: 21/11/2013
  * Time: 09:31
- * To change this template use File | Settings | File Templates.
  */
 public class AddressIn {
 

diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/BeanFactory.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/BeanFactory.java
@@ -18,24 +18,31 @@
 
 import fr.xebia.extras.selma.beans.CityOut;
 
-import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * Generic bean factory
  */
 public class BeanFactory {
 
-    Map<String, AtomicLong> buildMap = new HashMap<String, AtomicLong>();
+    AtomicInteger newInstance = new AtomicInteger();
+    AtomicInteger newCityCalled = new AtomicInteger();
+    AtomicInteger newOutObjectCalled = new AtomicInteger();
 
     public <T>  T newInstance(Class<T> targetType) {
-
-        if (buildMap.containsKey(targetType.getName())){
-            buildMap.get(targetType.getName()).incrementAndGet();
-        } else {
-            buildMap.put(targetType.getName(), new AtomicLong(1));
+        newInstance.incrementAndGet();
+        try {
+            return targetType.newInstance();
+        } catch (InstantiationException e) {
+            throw new RuntimeException(e);
+        } catch (IllegalAccessException e) {
+            throw new RuntimeException(e);
         }
+    }
+
+    public <T extends OutObject>  T newOutObjectInstance(Class<T> targetType) {
+        newOutObjectCalled.incrementAndGet();
         try {
             return targetType.newInstance();
         } catch (InstantiationException e) {
@@ -45,13 +52,12 @@ public <T>  T newInstance(Class<T> targetType) {
         }
     }
 
-/*
     public CityOut newCity(){
+        newCityCalled.incrementAndGet();
         return new CityOut();
     }
-*/
 
-    public Map<String, AtomicLong> getBuildMap() {
-        return buildMap;
+    public Map getBuildMap(){
+        return null;
     }
 }
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/ExtendedAddressIn.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/ExtendedAddressIn.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2013  Séven Le Mesle
+ *
+ * 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 fr.xebia.extras.selma.it.factory;
+
+import fr.xebia.extras.selma.beans.AddressIn;
+
+/**
+ * Created by slemesle on 15/03/2016.
+ */
+public class ExtendedAddressIn extends AddressIn {
+}
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/ExtendedAddressOut.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/ExtendedAddressOut.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2013  Séven Le Mesle
+ *
+ * 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 fr.xebia.extras.selma.it.factory;
+
+import fr.xebia.extras.selma.beans.AddressOut;
+
+/**
+ * Created by slemesle on 15/03/2016.
+ */
+public class ExtendedAddressOut extends AddressOut implements OutObject {
+}
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/FactoryMapper.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/FactoryMapper.java
@@ -27,4 +27,6 @@
 public interface FactoryMapper {
 
     AddressOut asAddressOut(AddressIn in);
+
+    ExtendedAddressOut asExtendedAddressOut(ExtendedAddressIn in);
 }
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/FactoryMapperIT.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/FactoryMapperIT.java
@@ -22,15 +22,22 @@
 import fr.xebia.extras.selma.beans.CityIn;
 import fr.xebia.extras.selma.it.utils.Compile;
 import fr.xebia.extras.selma.it.utils.IntegrationTestBase;
-import org.junit.Assert;
 import org.junit.Test;
 
 import java.util.Arrays;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
 /**
  *
  */
-@Compile(withClasses = {BeanFactory.class, FactoryMapper.class})
+@Compile(withClasses = {
+         BeanFactory.class,
+         FactoryMapper.class,
+         OutObject.class,
+         ExtendedAddressIn.class,
+         ExtendedAddressOut.class})
 public class FactoryMapperIT extends IntegrationTestBase {
 
     @Test
@@ -50,8 +57,39 @@ public void given_a_valid_factory_class_mapper_should_use_it_to_instantiate_bean
 
         AddressOut res = mapper.asAddressOut(address);
 
-        Assert.assertNotNull(res);
-        Assert.assertEquals(2, factory.getBuildMap().size());
+        assertNotNull(res);
+        assertEquals("Generic factory should have been called once for AddressOut", 1, factory.newInstance.get());
+        assertEquals("Static factory should have been called once for CityOut", 1, factory.newCityCalled.get());
+    }
+
+    @Test
+    public void given_a_factory_method_never_used_compiler_should_report_it() throws Exception {
+        assertCompilationWarning(FactoryMapper.class,
+                "public interface FactoryMapper {",
+                "Factory method \"fr.xebia.extras.selma.it.factory.BeanFactory.getBuildMap\" is never used");
+    }
+
+    @Test
+    public void given_a_valid_factory_class_mapper_should_use_closest_factory_to_instantiate_beans() throws Exception {
+        BeanFactory factory = new BeanFactory();
+        FactoryMapper mapper = Selma.builder(FactoryMapper.class).withFactories(factory).build();
+
+        ExtendedAddressIn address = new ExtendedAddressIn();
+        address.setCity(new CityIn());
+        address.setExtras(Arrays.asList(new String []{"134", "1234", "543"}));
+        address.setPrincipal(false);
+        address.setNumber(42);
+        address.setStreet("Victor Hugo");
+        address.getCity().setName("Paris");
+        address.getCity().setCapital(true);
+        address.getCity().setPopulation(10);
+
+        ExtendedAddressOut res = mapper.asExtendedAddressOut(address);
+
+        assertNotNull(res);
+        assertEquals("Generic newOutObjectInstance factory should have been called once for ExtendedAddressOut",
+                     1, factory.newOutObjectCalled.get());
+        assertEquals("Static factory should have been called once for CityOut", 1, factory.newCityCalled.get());
     }
 
 }
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/factory/OutObject.java b/processor/src/test/java/fr/xebia/extras/selma/it/factory/OutObject.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2013  Séven Le Mesle
+ *
+ * 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 fr.xebia.extras.selma.it.factory;
+
+/**
+ * Created by slemesle on 15/03/2016.
+ */
+public interface OutObject {
+}
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/inject/AddressMapper.java b/processor/src/test/java/fr/xebia/extras/selma/it/inject/AddressMapper.java
@@ -24,7 +24,10 @@
 /**
  * Created by slemesle on 25/03/15.
  */
-@Mapper(withIgnoreFields = "extras", withCustom =  CustomImmutableMapperClass.class, withIoC = IoC.SPRING)
+@Mapper(withIgnoreFields = "extras",
+        withCustom =  CustomImmutableMapperClass.class,
+        withFactories = BeanFactoryClass.class,
+        withIoC = IoC.SPRING)
 public interface AddressMapper {
 
     AddressOut asAddressOut(AddressIn in);

diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/inject/BeanFactoryClass.java b/processor/src/test/java/fr/xebia/extras/selma/it/inject/BeanFactoryClass.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2013  Séven Le Mesle
+ *
+ * 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 fr.xebia.extras.selma.it.inject;
+
+import fr.xebia.extras.selma.beans.CityOut;
+import org.springframework.stereotype.Service;
+
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Generic bean factory
+ */
+@Service
+public class BeanFactoryClass {
+
+    AtomicInteger newInstance = new AtomicInteger();
+    AtomicInteger newCityCalled = new AtomicInteger();
+
+    public <T>  T newInstance(Class<T> targetType) {
+        newInstance.incrementAndGet();
+        try {
+            return targetType.newInstance();
+        } catch (InstantiationException e) {
+            throw new RuntimeException(e);
+        } catch (IllegalAccessException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    public CityOut newCity(){
+        newCityCalled.incrementAndGet();
+        return new CityOut();
+    }
+
+    public Map getBuildMap(){
+        return null;
+    }
+}
diff --git a/processor/src/test/java/fr/xebia/extras/selma/it/inject/CustomMapperUsingSpringIoCIT.java b/processor/src/test/java/fr/xebia/extras/selma/it/inject/CustomMapperUsingSpringIoCIT.java
@@ -30,7 +30,7 @@
 import java.util.Arrays;
 
 
-@Compile(withClasses = {AddressMapper.class, CustomImmutableMapperClass.class})
+@Compile(withClasses = {AddressMapper.class, CustomImmutableMapperClass.class, BeanFactoryClass.class})
 public class CustomMapperUsingSpringIoCIT extends IntegrationTestBase {
 
 

diff --git a/selma/src/main/java/fr/xebia/extras/selma/Selma.java b/selma/src/main/java/fr/xebia/extras/selma/Selma.java
@@ -148,7 +148,8 @@ private synchronized static <T, V, U> T getMapper(Class<T> mapperClass,
                                                       boolean useCache,
                                                       List factories) throws IllegalArgumentException {
 
-        final String mapperKey = String.format("%s-%s-%s", mapperClass.getCanonicalName(), source, customMappers);
+        final String mapperKey = String.format("%s-%s-%s-%s", mapperClass.getCanonicalName(), source, customMappers,
+                                                factories);
 
         Object mapperInstance = mappers.get(mapperKey);
         if ((mapperInstance == null || !mapperClass.isAssignableFrom(mapperInstance.getClass())) || !useCache) {