From 36ee71ad0c57e65ca72c05d61b71697ce8abb179 Mon Sep 17 00:00:00 2001 From: Ruben Dijkstra Date: Tue, 15 Apr 2014 00:08:41 +0200 Subject: [PATCH] Default values for varargs array. Tests updated to reflect desired behavior. Cleanup step and improvements are scheduled next. --- .../mysema/query/util/ConstructorUtils.java | 44 +++++++++++++------ .../types/ConstructorExpressionTest.java | 18 +++++--- .../mysema/query/types/ProjectionExample.java | 4 +- .../mysema/query/types/ProjectionsTest.java | 22 ++++++++-- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/querydsl-core/src/main/java/com/mysema/query/util/ConstructorUtils.java b/querydsl-core/src/main/java/com/mysema/query/util/ConstructorUtils.java index 7a56d244d..d62507c7c 100644 --- a/querydsl-core/src/main/java/com/mysema/query/util/ConstructorUtils.java +++ b/querydsl-core/src/main/java/com/mysema/query/util/ConstructorUtils.java @@ -13,22 +13,26 @@ */ package com.mysema.query.util; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.ImmutableList.copyOf; +import static com.google.common.collect.Collections2.filter; +import static com.mysema.util.ArrayUtils.isEmpty; + import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.collect.ClassToInstanceMap; -import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableClassToInstanceMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.primitives.Primitives; import com.mysema.query.types.ExpressionException; -import com.mysema.util.ArrayUtils; import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Iterator; +import java.util.List; import java.util.Map; /** @@ -86,8 +90,8 @@ public class ConstructorUtils { Iterator> parameterIterator = Arrays .asList(parameters) .iterator(); - if (!ArrayUtils.isEmpty(givenTypes) - && !ArrayUtils.isEmpty(parameters)) { + if (!isEmpty(givenTypes) + && !isEmpty(parameters)) { Class parameter = null; for (Class argument : givenTypes) { @@ -108,8 +112,8 @@ public class ConstructorUtils { if (matches == parameters.length) { return parameters; } - } else if (ArrayUtils.isEmpty(givenTypes) - && ArrayUtils.isEmpty(parameters)) { + } else if (isEmpty(givenTypes) + && isEmpty(parameters)) { return NO_ARGS; } } @@ -128,8 +132,7 @@ public class ConstructorUtils { new VarArgsTransformer(constructor), new NullSafePrimitiveTransformer(constructor)); - return Collections2 - .filter(transformers, applicableFilter); + return copyOf(filter(transformers, applicableFilter)); } private static Class normalize(Class clazz) { @@ -189,13 +192,13 @@ public class ConstructorUtils { @Override public Object[] apply(Object[] args) { Iterator iterator = Arrays - .asList(args) + .asList(checkNotNull(args)) .iterator(); // constructor args Object[] cargs = new Object[paramTypes.length]; for (int i = 0; i < cargs.length - 1; i++) { - Array.set(cargs, i, iterator.next()); + set(cargs, i, iterator.next()); } // array with vargs int size = args.length - cargs.length + 1; @@ -203,11 +206,21 @@ public class ConstructorUtils { componentType, size); cargs[cargs.length - 1] = vargs; for (int i = 0; i < Array.getLength(vargs); i++) { - Array.set(vargs, i, iterator.next()); + set(vargs, i, iterator.next()); } return cargs; } + private void set(Object array, int index, Object value) throws IllegalArgumentException, ArrayIndexOutOfBoundsException { + try { + Array.set(array, index, value); + } catch (IllegalArgumentException primitiveArrayCantHoldNull) { + // retry with default value + Object defaultValue = defaultPrimitives.getInstance(componentType); + Array.set(array, index, defaultValue); + } + } + } private static class NullSafePrimitiveTransformer extends ConstructorArgumentTransformer { @@ -234,14 +247,17 @@ public class ConstructorUtils { @Override public Object[] apply(Object[] args) { + List argList = Arrays + .asList(checkNotNull(args)); + for (Map.Entry> primitiveEntry : primitiveLocations.entrySet()) { Integer location = primitiveEntry.getKey(); - if (args[location] == null) { + if (argList.get(location) == null) { Class primitiveClass = primitiveEntry.getValue(); - args[location] = defaultPrimitives.getInstance(primitiveClass); + argList.set(location, defaultPrimitives.getInstance(primitiveClass)); } } - return args; + return argList.toArray(); } } diff --git a/querydsl-core/src/test/java/com/mysema/query/types/ConstructorExpressionTest.java b/querydsl-core/src/test/java/com/mysema/query/types/ConstructorExpressionTest.java index 7aade09b7..1bd197bbc 100644 --- a/querydsl-core/src/test/java/com/mysema/query/types/ConstructorExpressionTest.java +++ b/querydsl-core/src/test/java/com/mysema/query/types/ConstructorExpressionTest.java @@ -21,7 +21,7 @@ import java.util.Arrays; import org.junit.Test; import com.mysema.query.types.path.StringPath; - +import static org.junit.Assert.assertTrue; public class ConstructorExpressionTest { @@ -34,15 +34,18 @@ public class ConstructorExpressionTest { public void Constructor() { Expression longVal = ConstantImpl.create(1l); Expression stringVal = ConstantImpl.create(""); - assertNotNull(new ConstructorExpression(ProjectionExample.class, - new Class[]{long.class, String.class}, longVal, stringVal).newInstance(0l,"")); + ProjectionExample instance = new ConstructorExpression(ProjectionExample.class, + new Class[]{long.class, String.class}, longVal, stringVal).newInstance(0l, ""); + assertNotNull(instance); + assertEquals((Long) 0L, instance.id); + assertTrue(instance.text.isEmpty()); } @Test public void Create() { Expression longVal = ConstantImpl.create(1l); Expression stringVal = ConstantImpl.create(""); - assertNotNull(ConstructorExpression.create(ProjectionExample.class, longVal, stringVal).newInstance(0l,"")); + assertNotNull(ConstructorExpression.create(ProjectionExample.class, longVal, stringVal).newInstance(0l, "")); } @Test @@ -72,7 +75,7 @@ public class ConstructorExpressionTest { Expression longVal = ConstantImpl.create(0L); Expression floatVal = ConstantImpl.create(0.0F); Expression doubleVal = ConstantImpl.create(0.0); - assertNotNull(ConstructorExpression.create(ProjectionExample.class, + ProjectionExample instance = ConstructorExpression.create(ProjectionExample.class, booleanVal, byteVal, charVal, shortVal, intVal, longVal, @@ -80,7 +83,8 @@ public class ConstructorExpressionTest { .newInstance(null, null, null, null, null, null, - null, null)); + null, null); + assertNotNull(instance); } @Test @@ -94,7 +98,7 @@ public class ConstructorExpressionTest { public void FactoryExpression_newInstance() { FactoryExpression constructor = ConstructorExpression.create(ProjectionExample.class, concat); constructor = FactoryExpressionUtils.wrap(constructor); - ProjectionExample projection = constructor.newInstance("12","34"); + ProjectionExample projection = constructor.newInstance("12", "34"); assertEquals("1234", projection.text); } diff --git a/querydsl-core/src/test/java/com/mysema/query/types/ProjectionExample.java b/querydsl-core/src/test/java/com/mysema/query/types/ProjectionExample.java index 85a671186..8fd22a767 100644 --- a/querydsl-core/src/test/java/com/mysema/query/types/ProjectionExample.java +++ b/querydsl-core/src/test/java/com/mysema/query/types/ProjectionExample.java @@ -43,9 +43,9 @@ public class ProjectionExample { } - public ProjectionExample(Long id, char... characters) { + public ProjectionExample(long id, char... characters) { this.id = id; - this.text = String.copyValueOf(characters); + this.text = String.valueOf(characters); } } diff --git a/querydsl-core/src/test/java/com/mysema/query/types/ProjectionsTest.java b/querydsl-core/src/test/java/com/mysema/query/types/ProjectionsTest.java index d0721027b..a35207dd0 100644 --- a/querydsl-core/src/test/java/com/mysema/query/types/ProjectionsTest.java +++ b/querydsl-core/src/test/java/com/mysema/query/types/ProjectionsTest.java @@ -88,10 +88,24 @@ public class ProjectionsTest { Constant longVal = ConstantImpl.create(1L); Constant charVal = ConstantImpl.create('\0'); ProjectionExample instance = Projections - .constructor(ProjectionExample.class, longVal, charVal, charVal, charVal, charVal, charVal, charVal) - .newInstance(1L, 'm', 'y', 's', 'e', 'm', 'a'); - assertEquals(1L, (long) instance.id); - assertEquals("mysema", instance.text); + .constructor(ProjectionExample.class, + longVal, charVal, + charVal, charVal, + charVal, charVal, + charVal, charVal, + charVal, charVal, + charVal) + .newInstance(null, 'm', + 'y', 's', + 'e', 'm', + 'a', null, + 'l', 't', + 'd'); + assertEquals(0L, (long) instance.id); + // null character cannot be inserted, so a literal String can't be used. + String expectedText = String.valueOf(new char[]{'m', 'y', 's', 'e', 'm', 'a', + '\0', 'l', 't', 'd'}); + assertEquals(expectedText, instance.text); } @Test