Code quality improvements

api/src/main/java/org/unitsheet/api/adapter/SpreadsheetProvider.java

@@ -5,8 +5,8 @@
 
 public interface SpreadsheetProvider {
 
-    public WorksheetType worksheetType();
+    WorksheetType worksheetType();
 
-    public SpreadsheetAdapter createSpreadsheetAdapter(InputStream inputStream) throws IOException;   //, TODO - add test context for specific config etc.
+    SpreadsheetAdapter createSpreadsheetAdapter(InputStream inputStream) throws IOException;   //, TODO - add test context for specific config etc.
 
 }

core/src/main/java/org/unitsheet/TestObjectFieldPopulator.java

@@ -4,19 +4,16 @@
 import org.unitsheet.annotations.ReadColumn;
 import org.unitsheet.api.adapter.CellInfo;
 import org.unitsheet.api.adapter.ColumnInfo;
-import org.unitsheet.types.ObjectConverter;
 import org.unitsheet.api.adapter.SpreadsheetAdapter;
+import org.unitsheet.types.ObjectConverter;
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.SortedSet;
-import java.util.TreeSet;
 
-import static java.util.Arrays.asList;
-import static org.unitsheet.utils.ReflectionUtils.getGenericTypeClass;
-import static org.unitsheet.utils.ReflectionUtils.getObjectFieldsInOrder;
-import static org.unitsheet.utils.ReflectionUtils.setObjectFieldValue;
+import static org.unitsheet.utils.ReflectionUtils.*;
 
 public class TestObjectFieldPopulator {
 
@@ -59,7 +56,12 @@ private void handleReadColumnAnnotation(Object testObject, Field field, ReadColu
         List<Object> column = spreadsheet.getColumn(columnInfo);
         List<Object> results = new ArrayList<>();
 
-        Class<?> collectionGenericTypeClass = getGenericTypeClass(field);
+        Optional<Class<?>> genericTypeClassOptional = getGenericTypeClass(field);
+        if (! genericTypeClassOptional.isPresent()) {
+            throw new RuntimeException(); // TODO - add nice exception
+        }
+
+        Class<?> collectionGenericTypeClass = genericTypeClassOptional.get();
 
         for (Object o : column) {
             Object value = objectConverter.convertType(o, collectionGenericTypeClass);

core/src/main/java/org/unitsheet/types/ObjectConverter.java

@@ -28,6 +28,7 @@ public <T> T convertType(Object source, Class<T> destinationClass) {
             }
         }
 
+        // TODO - add nice exception
         throw new RuntimeException(
                 "Can't convert object type object " + source + " from " + sourceClass + " to " + destinationClass);
     }

core/src/main/java/org/unitsheet/types/PluginTypeConverterLoader.java

@@ -11,8 +11,8 @@ public class PluginTypeConverterLoader {
     public List<TypeConverter> loadTypeConvertersFromPlugins() {
         ServiceLoader<TypeConverter> loader = ServiceLoader.load(TypeConverter.class);
 
-        List<TypeConverter> pluginTypeConverters = new ArrayList<TypeConverter>();
-        loader.iterator().forEachRemaining(converter -> pluginTypeConverters.add(converter));
+        List<TypeConverter> pluginTypeConverters = new ArrayList<>();
+        loader.iterator().forEachRemaining(pluginTypeConverters::add);
 
         return pluginTypeConverters;
     }

core/src/test/java/org/unitsheet/test/DummyTypeConverter.java

@@ -0,0 +1,11 @@
+package org.unitsheet.test;
+
+import org.unitsheet.api.types.TypeConverter;
+
+public class DummyTypeConverter implements TypeConverter {
+
+    @Override
+    public <T> T convert(Object source, Class<T> destinationClass) {
+        throw new UnsupportedOperationException();
+    }
+}

core/src/test/java/org/unitsheet/types/ObjectConverterTest.java

@@ -0,0 +1,74 @@
+package org.unitsheet.types;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.unitsheet.api.types.TypeConverter;
+import org.unitsheet.types.typeconverters.BooleanTypeConverter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ObjectConverterTest {
+
+    @InjectMocks
+    private ObjectConverter underTest;
+
+    @Mock private TypeConverterRegistry typeConverterRegistry;
+    private List<TypeConverter> typeConverters = new ArrayList<>();
+
+    @Before
+    public void setUp() {
+        typeConverters.clear();
+        when(typeConverterRegistry.getTypeConverters()).thenReturn(typeConverters);
+    }
+
+    @Test
+    public void checkConvertTypeIgnoresNull() {
+        // given
+        String convertFrom = null;
+
+        // when
+        Number result = underTest.convertType(convertFrom, Number.class);
+
+        // then
+        assertThat(result).isNull();
+    }
+
+    @Test
+    public void checkConvertTypeToSameTypeWorksForUnregisteredType() {
+        /*
+         given
+          */
+        class Banana {}
+        Banana convertFrom = new Banana();
+
+        // when
+        Banana result = underTest.convertType(convertFrom, Banana.class);
+
+        // then
+        assertThat(result).isInstanceOf(Banana.class);
+    }
+
+    @Test
+    public void checkConvertUsingRealTypeConverterReturnsResult() {
+        /*
+         given
+          */
+        typeConverters.add(new BooleanTypeConverter());
+        String convertFrom = "true";
+
+        // when
+        Boolean result = underTest.convertType(convertFrom, Boolean.class);
+
+        // then
+        assertThat(result).isTrue();
+    }
+}

core/src/test/java/org/unitsheet/types/PluginTypeConverterLoaderTest.java

@@ -0,0 +1,22 @@
+package org.unitsheet.types;
+
+import org.junit.Test;
+import org.unitsheet.api.types.TypeConverter;
+import org.unitsheet.test.DummyTypeConverter;
+
+import java.util.List;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+public class PluginTypeConverterLoaderTest {
+
+    private PluginTypeConverterLoader underTest = new PluginTypeConverterLoader();
+
+    @Test
+    public void checkLoadTypeConvertersFromPluginsFindDummyTypeConverter() {
+        List<TypeConverter> typeConverters = underTest.loadTypeConvertersFromPlugins();
+
+        assertThat(typeConverters).hasSize(1);
+        assertThat(typeConverters.get(0)).isInstanceOf(DummyTypeConverter.class);
+    }
+}

core/src/test/resources/META-INF/services/org.unitsheet.api.types.TypeConverter

@@ -0,0 +1 @@
+org.unitsheet.test.DummyTypeConverter

pom.xml

@@ -23,7 +23,7 @@
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <hamcrestVersion>1.3</hamcrestVersion>
-        <mockitoVersion>1.8.5</mockitoVersion>
+        <mockitoVersion>1.10.19</mockitoVersion>
         <junitVersion>4.11</junitVersion>
     </properties>
 

util/src/main/java/org/unitsheet/utils/ReflectionUtils.java

@@ -3,10 +3,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.SortedSet;
-import java.util.TreeSet;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import static java.util.Arrays.asList;
@@ -22,7 +19,7 @@ private ReflectionUtils(){} // non-instantiable
      *
      * TODO - use optional, handle non-happy path
      */
-    public static Class<?> getGenericTypeClass(Field field) {
+    public static Optional<Class<?>> getGenericTypeClass(Field field) {
         Class<?> typeClass = null;
 
         Type genericType = field.getGenericType();
@@ -32,7 +29,7 @@ public static Class<?> getGenericTypeClass(Field field) {
             if (actualTypeArguments.length == 1) {
                 Type actualTypeArgument = actualTypeArguments[0];
                 if (actualTypeArgument instanceof Class) {
-                    return (Class) actualTypeArgument;
+                    typeClass = (Class) actualTypeArgument;
                 }
             } else {
                 // TODO
@@ -41,7 +38,7 @@ public static Class<?> getGenericTypeClass(Field field) {
             // TODO
         }
 
-        return typeClass;
+        return Optional.ofNullable(typeClass);
     }
 
     // TODO - cleanup
@@ -60,6 +57,7 @@ public static SortedSet<Field> getObjectFieldsInOrder(Object object) {
         return sorted;
     }
 
+    // TODO - check security permissions / SecurityException
     public static void setObjectFieldValue(Field field, Object object, Object value)
             throws IllegalAccessException, SecurityException {
 

util/src/test/java/org/unitsheet/utils/ReflectionUtilsTest.java

@@ -27,7 +27,7 @@ public void checkStringListHasGenericTypeClassString() throws NoSuchFieldExcepti
         Field field = aClass.getField("stringList");
 
         // when
-        Class<?> typeClass = getGenericTypeClass(field);
+        Class<?> typeClass = getGenericTypeClass(field).get();
 
         // then
         assertThat(typeClass.getName()).isEqualTo("java.lang.String");