Skip to content

Commit d19b934

Browse files
ctruedenctrueden
authored
Merge pull request #284 from scijava/test-converter-matching
Add Tests to Ensure Proper Converter Chosen
2 parents d9e7fd0 + 556c6b2 commit d19b934

File tree

5 files changed

+164
-29
lines changed

5 files changed

+164
-29
lines changed

‎src/main/java/org/scijava/convert/CastingConverter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*
4343
* @author Mark Hiner
4444
*/
45-
@Plugin(type = Converter.class, priority = Priority.FIRST)
45+
@Plugin(type = Converter.class, priority = Priority.EXTREMELY_HIGH)
4646
public class CastingConverter extends AbstractConverter<Object, Object> {
4747

4848
@SuppressWarnings("deprecation")

‎src/main/java/org/scijava/convert/DefaultConverter.java‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import java.util.List;
4444
import java.util.Set;
4545

46+
import org.scijava.Priority;
4647
import org.scijava.plugin.Plugin;
4748
import org.scijava.util.ArrayUtils;
4849
import org.scijava.util.ClassUtils;
@@ -52,10 +53,24 @@
5253
/**
5354
* Default {@link Converter} implementation. Provides useful conversion
5455
* functionality for many common conversion cases.
56+
* <p>
57+
* Supported conversions include:
58+
* </p>
59+
* <ul>
60+
* <li>Object to Array</li>
61+
* <li>Object to Collection</li>
62+
* <li>Number to Number</li>
63+
* <li>Object to String</li>
64+
* <li>String to Character</li>
65+
* <li>String to Enum</li>
66+
* <li>Objects where the destination Class has a constructor which takes that
67+
* Object
68+
* </li>
69+
* </ul>
5570
*
5671
* @author Mark Hiner
5772
*/
58-
@Plugin(type = Converter.class)
73+
@Plugin(type = Converter.class, priority = Priority.EXTREMELY_LOW)
5974
public class DefaultConverter extends AbstractConverter<Object, Object> {
6075

6176
// -- ConversionHandler methods --
@@ -79,16 +94,9 @@ public Object convert(final Object src, final Type dest) {
7994

8095
@Override
8196
public <T> T convert(final Object src, final Class<T> dest) {
82-
if (dest == null) return null;
83-
if (src == null) return ConversionUtils.getNullValue(dest);
84-
8597
// ensure type is well-behaved, rather than a primitive type
8698
final Class<T> saneDest = ConversionUtils.getNonprimitiveType(dest);
8799

88-
// cast the existing object, if possible
89-
if (ConversionUtils.canCast(src, saneDest)) return ConversionUtils.cast(
90-
src, saneDest);
91-
92100
// Handle array types
93101
if (isArray(dest)) {
94102
@SuppressWarnings("unchecked")
@@ -301,15 +309,9 @@ public boolean canConvert(final Class<?> src, final Type dest) {
301309
@Override
302310
@Deprecated
303311
public boolean canConvert(final Class<?> src, final Class<?> dest) {
304-
305-
if (src == null || dest == null) return true;
306-
307312
// ensure type is well-behaved, rather than a primitive type
308313
final Class<?> saneDest = ConversionUtils.getNonprimitiveType(dest);
309-
310-
// OK if the existing object can be casted
311-
if (ConversionUtils.canCast(src, saneDest)) return true;
312-
314+
313315
// OK for numerical conversions
314316
if (ConversionUtils.canCast(ConversionUtils.getNonprimitiveType(src),
315317
Number.class) &&

‎src/main/java/org/scijava/convert/NullConverter.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@
4040

4141
/**
4242
* {@link Converter} implementation for handling {@code null} values. Performs
43-
* basic casting when given a {@code null} source and returns {@code} null
44-
* directly when given a {@code} null destination.
43+
* basic casting when given a {@code null} source and returns {@code null}
44+
* directly when given a {@code null} destination.
4545
* <p>
4646
* By running at {@link Priority#FIRST}, other converters should
4747
* not need to worry about {@code null} source or destination parameters.
@@ -60,23 +60,24 @@ public class NullConverter extends AbstractConverter<Object, Object> {
6060
@Override
6161
public boolean canConvert(final ConversionRequest request) {
6262
if (request == null) return false;
63-
if (request.destType() == null && request.destClass() == null) return false;
64-
return request.sourceObject() == null && request.sourceClass() == null;
63+
return (request.destType() == null && request.destClass() == null) ||
64+
(request.sourceObject() == null && request.sourceClass() == null);
6565
}
6666

6767
@Override
6868
public boolean canConvert(final Object src, final Type dest) {
69-
return src == null && dest != null;
69+
return src == null || dest == null;
7070
}
7171

7272
@Override
7373
public boolean canConvert(final Object src, final Class<?> dest) {
74-
return src == null && dest != null;
74+
return src == null || dest == null;
7575
}
7676

7777
@Override
7878
public boolean canConvert(final Class<?> src, final Class<?> dest) {
79-
return src == null && dest != null;
79+
if (src == null) return false;
80+
return dest == null;
8081
}
8182

8283
@Override

‎src/test/java/org/scijava/convert/ConvertServiceTest.java‎

Lines changed: 135 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
import static org.junit.Assert.assertTrue;
4141
import static org.junit.Assert.fail;
4242

43+
import java.lang.reflect.Field;
4344
import java.lang.reflect.Type;
45+
import java.math.BigDecimal;
4446
import java.util.ArrayList;
4547
import java.util.Collection;
4648
import java.util.Collections;
@@ -54,12 +56,22 @@
5456
import org.junit.Test;
5557
import org.scijava.Context;
5658
import org.scijava.Priority;
59+
import org.scijava.convert.ArrayConverters.ByteArrayWrapper;
60+
import org.scijava.convert.ArrayConverters.DoubleArrayUnwrapper;
61+
import org.scijava.convert.ArrayConverters.FloatArrayWrapper;
62+
import org.scijava.convert.ArrayConverters.LongArrayWrapper;
63+
import org.scijava.convert.ArrayConverters.ShortArrayUnwrapper;
64+
import org.scijava.convert.NumberConverters.ByteToLongConverter;
65+
import org.scijava.convert.NumberConverters.DoubleToBigDecimalConverter;
66+
import org.scijava.convert.NumberConverters.ShortToFloatConverter;
5767
import org.scijava.plugin.Plugin;
5868
import org.scijava.util.BoolArray;
69+
import org.scijava.util.ByteArray;
5970
import org.scijava.util.CharArray;
6071
import org.scijava.util.ClassUtils;
6172
import org.scijava.util.DoubleArray;
6273
import org.scijava.util.FloatArray;
74+
import org.scijava.util.GenericUtils;
6375
import org.scijava.util.IntArray;
6476
import org.scijava.util.LongArray;
6577
import org.scijava.util.PrimitiveArray;
@@ -135,7 +147,7 @@ public void testPrimitives() {
135147
*/
136148
@Test
137149
public void testArrays() {
138-
// Test that each primitive [] is compatible in either direciton with its
150+
// Test that each primitive [] is compatible in either direction with its
139151
// paired PrimitiveArray
140152
testIntechangeable(int[].class, IntArray.class);
141153
testIntechangeable(long[].class, LongArray.class);
@@ -145,7 +157,7 @@ public void testArrays() {
145157
testIntechangeable(char[].class, CharArray.class);
146158
testIntechangeable(boolean[].class, BoolArray.class);
147159

148-
// Test that primitive [] can not be convertied to mismatched PrimitiveArray
160+
// Test that primitive [] can not be converted to mismatched PrimitiveArray
149161
assertFalse(convertService.supports(int[].class, LongArray.class));
150162

151163
// Test that lists can be converted to any primitive []
@@ -565,6 +577,126 @@ public void testGetCompatibleInputs() {
565577
assertEquals(StringHisListConverter.S4, compatibleInputs.get(3));
566578
}
567579

580+
/**
581+
* Tests that the {@link NullConverter} is chosen for null src and/or dest.
582+
*/
583+
@Test
584+
public void testNullConverterMatching() {
585+
final Converter<?, ?> c = convertService.getHandler(new ConversionRequest(
586+
null, List.class));
587+
assertEquals(NullConverter.class, c.getClass());
588+
589+
final Converter<?, ?> cc = convertService.getHandler(new ConversionRequest(
590+
new Object(), (Class<?>) null));
591+
assertEquals(NullConverter.class, cc.getClass());
592+
593+
final Converter<?, ?> ccc = convertService.getHandler(new ConversionRequest(
594+
null, (Class<?>) null));
595+
assertEquals(NullConverter.class, ccc.getClass());
596+
}
597+
598+
/**
599+
* Tests the the appropriate wrapping ArrayConverter is chosen for converting
600+
* primitive arrays to scijava wrappers.
601+
*/
602+
@Test
603+
public void testArrayConverterWrappingMatching() {
604+
final byte[] b = new byte[] { -128, 0, 127 };
605+
final long[] l = new long[] { 13, 17, -103209, 0, 6 };
606+
final float[] f = new float[] { 12.125f, -0.0625f, 2.5f };
607+
608+
final Converter<?, ?> c = convertService.getHandler(b, ByteArray.class);
609+
assertEquals(ByteArrayWrapper.class, c.getClass());
610+
611+
final Converter<?, ?> cc = convertService.getHandler(l, LongArray.class);
612+
assertEquals(LongArrayWrapper.class, cc.getClass());
613+
614+
final Converter<?, ?> ccc = convertService.getHandler(f, FloatArray.class);
615+
assertEquals(FloatArrayWrapper.class, ccc.getClass());
616+
}
617+
618+
/**
619+
* Tests that the appropriate unwrapping ArrayConverter is chosen for
620+
* converting scijava arrays to primitive arrays.
621+
*/
622+
@Test
623+
public void testArrayConverterUnwrappingMatching() {
624+
final ShortArray s = new ShortArray();
625+
final DoubleArray d = new DoubleArray();
626+
627+
final Converter<?, ?> c = convertService.getHandler(s, short[].class);
628+
assertEquals(ShortArrayUnwrapper.class, c.getClass());
629+
630+
final Converter<?, ?> cc = convertService.getHandler(d, double[].class);
631+
assertEquals(DoubleArrayUnwrapper.class, cc.getClass());
632+
}
633+
634+
/**
635+
* Tests that the {@link CastingConverter} is called when casting is possible.
636+
*/
637+
@Test
638+
public void testCastingConverterMatching() {
639+
final ArrayList<Double> al = new ArrayList<>();
640+
641+
final Converter<?, ?> c = convertService.getHandler(al, Collection.class);
642+
assertEquals(CastingConverter.class, c.getClass());
643+
}
644+
645+
/**
646+
* Tests the that the appropriate {@link NumberToNumberConverter} is chosen.
647+
*/
648+
@Test
649+
public void testNumberConverterMatching() {
650+
final double d = -24312926.0625;
651+
final byte b = 64;
652+
final short s = 32625;
653+
654+
// Number converters only handle widening conversions
655+
final Converter<?, ?> c = convertService.getHandler(d, BigDecimal.class);
656+
assertEquals(DoubleToBigDecimalConverter.class, c.getClass());
657+
658+
final Converter<?, ?> cc = convertService.getHandler(b, long.class);
659+
assertEquals(ByteToLongConverter.class, cc.getClass());
660+
661+
final Converter<?, ?> ccc = convertService.getHandler(s, float.class);
662+
assertEquals(ShortToFloatConverter.class, ccc.getClass());
663+
}
664+
665+
/**
666+
* Tests that the {@link DefaultConverter} is chosen when no other suitable
667+
* converter is available.
668+
*/
669+
@Test
670+
public void testDefaultConverterMatching() {
671+
final float f = 13624292.25f;
672+
final List<String> l = new ArrayList<>();
673+
674+
// Narrowing number conversion
675+
final Converter<?, ?> c = convertService.getHandler(f, byte.class);
676+
assertEquals(DefaultConverter.class, c.getClass());
677+
678+
// List to Array
679+
final Converter<?, ?> cc = convertService.getHandler(l, String[].class);
680+
assertEquals(DefaultConverter.class, cc.getClass());
681+
682+
// Object to String
683+
final Converter<?, ?> os = convertService.getHandler(new Object(),
684+
String.class);
685+
assertEquals(DefaultConverter.class, os.getClass());
686+
687+
// String to Character
688+
final Converter<?, ?> ss = convertService.getHandler("hello", char.class);
689+
assertEquals(DefaultConverter.class, ss.getClass());
690+
691+
// String to Enum
692+
final Converter<?, ?> se = convertService.getHandler("bye", Words.class);
693+
assertEquals(DefaultConverter.class, se.getClass());
694+
695+
// Source which can be wrapped as destination
696+
final Converter<?, ?> w = convertService.getHandler(10122017l, Date.class);
697+
assertEquals(DefaultConverter.class, w.getClass());
698+
}
699+
568700
// -- Helper Methods --
569701

570702
/**
@@ -751,7 +883,7 @@ public <T> T convert(Object src, Class<T> dest) {
751883
// -- Helper methods --
752884

753885
/**
754-
* Verify bi-direciotnal conversion is supported between the two classes
886+
* Verify bi-directional conversion is supported between the two classes
755887
*/
756888
private void testIntechangeable(final Class<?> c1, final Class<?> c2) {
757889
assertTrue(convertService.supports(c1, c2));

‎src/test/java/org/scijava/convert/ConverterTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void testNullConverter() {
6767
final NullConverter nc = new NullConverter();
6868
assertFalse(nc.canConvert(Object.class, Object.class));
6969
assertFalse(nc.canConvert(Object.class, (Type) Object.class));
70-
assertTrue(nc.canConvert((Class<?>) null, Object.class));
70+
assertFalse(nc.canConvert((Class<?>) null, Object.class));
7171
assertTrue(nc.canConvert((Object) null, Object.class));
7272
assertTrue(nc.canConvert((ConverterTest) null, ArrayList.class));
7373
assertNull(nc.convert((Object) null, Object.class));
@@ -94,11 +94,11 @@ public void testCanConvert() {
9494

9595
@Test
9696
public void testCanConvertToGenericCollection() {
97-
final DefaultConverter dc = new DefaultConverter();
97+
final CastingConverter cc = new CastingConverter();
9898

9999
final Field destField = ClassUtils.getField(getClass(), "collection");
100100
final Type destType = GenericUtils.getFieldType(destField, getClass());
101-
assertTrue(dc.canConvert(ArrayList.class, destType));
101+
assertTrue(cc.canConvert(ArrayList.class, destType));
102102
}
103103

104104
private static class NumberConverter extends AbstractConverter<Number, Number> {

0 commit comments

Comments
 (0)