Refactor transformation

* Transform types when a transformation is required instead of keeping a list of types to transform * Specify concrete types for transformations. Makes code simpler (eg, use numbers directly instead of formatting->parsing->formatting->parsing) and more explicit * Handle chars correctly, char-inputs didn't work before * Handle BigInteger/BigDecimal correctly, previously those didn't work correctly if formatted (truncated to double or simply failing) * Handle arrays of formatted types correctly, didn't worked before * Build a foundation for eclipse/microprofile-graphql#251 Signed-off-by: Yannick Bröker <[email protected]>
smallrye · May 16, 2020 · 6dc0803 · 6dc0803
1 parent cfa37d4
commit 6dc0803
Show file tree

Hide file tree

Showing 10 changed files with 181 additions and 88 deletions.
diff --git a/server/implementation/pom.xml b/server/implementation/pom.xml
@@ -71,10 +71,24 @@
 
         <!-- Testing -->
         <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>5.6.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.vintage</groupId>
+            <artifactId>junit-vintage-engine</artifactId>
+            <version>5.6.2</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>

diff --git a/...tation/src/main/java/io/smallrye/graphql/execution/datafetcher/helper/AbstractHelper.java b/...tation/src/main/java/io/smallrye/graphql/execution/datafetcher/helper/AbstractHelper.java
@@ -95,8 +95,12 @@ private Object recursiveTransformArray(Object array, Field field) throws Transfo
             array = ((Collection) array).toArray();
         }
 
-        String classNameInCollection = field.getReference().getClassName();
-        Class classInCollection = classloadingService.loadClass(classNameInCollection);
+        Class classInCollection = getArrayType(field);
+
+        //Skip transform if not needed
+        if (array.getClass().getComponentType().equals(classInCollection)) {
+            return array;
+        }
 
         int length = Array.getLength(array);
         Object targetArray = Array.newInstance(classInCollection, length);
@@ -111,6 +115,12 @@ private Object recursiveTransformArray(Object array, Field field) throws Transfo
         return targetArray;
     }
 
+    protected Class<?> getArrayType(Field field) {
+        String classNameInCollection = field.getReference().getClassName();
+        Class classInCollection = classloadingService.loadClass(classNameInCollection);
+        return classInCollection;
+    }
+
     /**
      * This just creates a new correct type collection and add values to it by calling the recursiveTransform method.
      * This allows collections of collections and transformation inside collections

diff --git a/...mentation/src/main/java/io/smallrye/graphql/execution/datafetcher/helper/FieldHelper.java b/...mentation/src/main/java/io/smallrye/graphql/execution/datafetcher/helper/FieldHelper.java
@@ -27,9 +27,7 @@ public FieldHelper(Field field) {
 
     public Object transformResponse(Object argumentValue)
             throws TransformException {
-        if (Transformer.shouldTransform(field)) {
-            argumentValue = super.recursiveTransform(argumentValue, field);
-        }
+        argumentValue = super.recursiveTransform(argumentValue, field);
         return argumentValue;
     }
 
@@ -50,4 +48,8 @@ protected Object afterRecursiveTransform(Object fieldValue, Field field) {
         return fieldValue;
     }
 
+    @Override
+    protected Class<?> getArrayType(final Field field) {
+        return classloadingService.loadClass(field.getReference().getGraphQlClassName());
+    }
 }
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/transformation/CharTransformer.java b/server/implementation/src/main/java/io/smallrye/graphql/transformation/CharTransformer.java
@@ -0,0 +1,20 @@
+package io.smallrye.graphql.transformation;
+
+/**
+ * Transforms between char and String.
+ */
+public class CharTransformer implements Transformer<Character, String> {
+
+    @Override
+    public Character in(final String o) {
+        return o.charAt(0);
+    }
+
+    @Override
+    public String out(final Character o) {
+        if (o == null) {
+            return null;
+        }
+        return String.valueOf(o);
+    }
+}
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/transformation/DateTransformer.java b/server/implementation/src/main/java/io/smallrye/graphql/transformation/DateTransformer.java
@@ -21,7 +21,7 @@
 /**
  * Handles date and time-types from {@linkplain java.time}.
  */
-public class DateTransformer implements Transformer {
+public class DateTransformer implements Transformer<Temporal, String> {
 
     private static final Map<String, DateTimeFormatter> DEFAULT_FORMATTER = createDefaultFormatter();
     private static final Map<String, TemporalQuery<?>> TEMPORAL_QUERYS = createTemporalQuerys();
@@ -41,7 +41,7 @@ public DateTransformer(Field field) {
     }
 
     @Override
-    public Temporal in(final Object o) {
+    public Temporal in(final String o) {
         TemporalQuery<?> temporalAccessor = TEMPORAL_QUERYS.get(targetClassName);
 
         if (temporalAccessor == null || dateTimeFormatter == null) {
@@ -52,11 +52,8 @@ public Temporal in(final Object o) {
     }
 
     @Override
-    public Object out(final Object dateType) {
-        if (dateType instanceof Temporal) {
-            return dateTimeFormatter.format((Temporal) dateType);
-        }
-        return dateType.toString();
+    public String out(final Temporal dateType) {
+        return dateTimeFormatter.format(dateType);
     }
 
     private static Map<String, TemporalQuery<?>> createTemporalQuerys() {
@@ -72,7 +69,7 @@ private static Map<String, TemporalQuery<?>> createTemporalQuerys() {
         return defaultFormatter;
     }
 
-    private DateTimeFormatter getDateFormat(TransformInfo formatter, String className) {
+    private static DateTimeFormatter getDateFormat(TransformInfo formatter, String className) {
         if (formatter != null) {
             String format = formatter.getFormat();
             String locale = formatter.getLocale();
@@ -99,4 +96,4 @@ private static Map<String, DateTimeFormatter> createDefaultFormatter() {
 
         return defaultFormatter;
     }
-}
+}
diff --git a/...entation/src/main/java/io/smallrye/graphql/transformation/FormattedNumberTransformer.java b/...entation/src/main/java/io/smallrye/graphql/transformation/FormattedNumberTransformer.java
@@ -12,25 +12,27 @@
 /**
  * Parses and formats numbers in the needed format.
  */
-public class FormattedNumberTransformer implements Transformer {
+public class FormattedNumberTransformer implements Transformer<Number, String> {
 
-    private final NumberFormat numberFormat;
+    private final DecimalFormat numberFormat;
 
     private final NumberTransformer numberTransformer;
 
     protected FormattedNumberTransformer(Field field) {
         this.numberTransformer = new NumberTransformer(field);
         this.numberFormat = getNumberFormat(field.getTransformInfo());
+        numberFormat.setParseBigDecimal(true);
     }
 
-    private NumberFormat getNumberFormat(TransformInfo formatter) {
+    private DecimalFormat getNumberFormat(TransformInfo formatter) {
         String format = formatter.getFormat();
         String locale = formatter.getLocale();
 
         if (format == null && locale == null) {
-            return NumberFormat.getInstance();
+            return new DecimalFormat();
         } else if (format == null) {
-            return NumberFormat.getInstance(Locale.forLanguageTag(locale));
+            //Should work: https://docs.oracle.com/javase/tutorial/i18n/format/decimalFormat.html
+            return (DecimalFormat) NumberFormat.getInstance(Locale.forLanguageTag(locale));
         } else if (locale == null) {
             return new DecimalFormat(format);
         } else {
@@ -40,16 +42,12 @@ private NumberFormat getNumberFormat(TransformInfo formatter) {
     }
 
     @Override
-    public Object in(final Object o) throws ParseException {
-        return numberTransformer.in(numberFormat.parse(o.toString()).toString());
+    public Number in(final String o) throws ParseException {
+        Number parsed = numberFormat.parse(o);
+        return numberTransformer.in(parsed);
     }
 
-    public Object out(final Object object) {
-        if (object instanceof Number) {
-            Number number = (Number) object;
-            return this.numberFormat.format(number);
-        }
-        throw new RuntimeException(String.format("[%s] is no valid number-type", object.getClass()));
-
+    public String out(final Number object) {
+        return this.numberFormat.format(object);
     }
 }
diff --git a/...mplementation/src/main/java/io/smallrye/graphql/transformation/LegacyDateTransformer.java b/...mplementation/src/main/java/io/smallrye/graphql/transformation/LegacyDateTransformer.java
@@ -15,7 +15,7 @@
 /**
  * Handles legacy-date-formats (which aren't required by spec).
  */
-public class LegacyDateTransformer implements Transformer {
+public class LegacyDateTransformer implements Transformer<Date, String> {
 
     /**
      * Mappings between Legacy-Date-Type and java.time-Type.
@@ -34,7 +34,7 @@ public LegacyDateTransformer(final Field field) {
     }
 
     @Override
-    public Object in(final Object o) throws Exception {
+    public Date in(final String o) throws Exception {
         if (targetClassName.equals(java.sql.Date.class.getName())) {
             LocalDate localdate = (LocalDate) dateTransformer.in(o);
             return java.sql.Date.valueOf(localdate);
@@ -53,7 +53,7 @@ public Object in(final Object o) throws Exception {
     }
 
     @Override
-    public Object out(final Object dateType) {
+    public String out(final Date dateType) {
         if (dateType instanceof java.sql.Date) {
             java.sql.Date casted = (java.sql.Date) dateType;
             return dateTransformer.out(casted.toLocalDate());
@@ -63,11 +63,10 @@ public Object out(final Object dateType) {
         } else if (dateType instanceof java.sql.Timestamp) {
             java.sql.Timestamp casted = (java.sql.Timestamp) dateType;
             return dateTransformer.out(casted.toLocalDateTime());
-        } else if (dateType instanceof Date) {
+        } else {
             Date casted = (Date) dateType;
             return dateTransformer.out(casted.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime());
         }
-        throw new RuntimeException("Can't format [" + dateType.getClass().getName() + "] from [" + targetClassName + "]");
     }
 
     private static Map<String, String> createClassMappings() {

diff --git a/...er/implementation/src/main/java/io/smallrye/graphql/transformation/NumberTransformer.java b/...er/implementation/src/main/java/io/smallrye/graphql/transformation/NumberTransformer.java
@@ -2,68 +2,73 @@
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
-import java.text.ParseException;
 
 import io.smallrye.graphql.schema.model.Field;
 
 /**
- * Parses incoming numbers to the needed class, doesn't touch outgoing values.
+ * Converts between number-types.
  */
-public class NumberTransformer implements Transformer {
+public class NumberTransformer implements Transformer<Number, Number> {
 
-    private final Field field;
+    private final String typeClassName;
 
     protected NumberTransformer(Field field) {
-        this.field = field;
+        this(field.getReference().getClassName());
     }
 
-    @Override
-    public Object in(final Object o) throws ParseException {
-        return parseNumber(o.toString(), field.getReference().getClassName());
+    public NumberTransformer(String targetClassName) {
+        this.typeClassName = targetClassName;
     }
 
-    public Number parseNumber(String input, String typeClassName) throws ParseException {
+    @Override
+    public Number in(final Number input) {
         // Integer
-        if (typeClassName.equals(int.class.getName())) {
-            return (Integer.parseInt(input));
-        } else if (typeClassName.equals(Integer.class.getName())) {
-            return (Integer.valueOf(input));
-        } else if (typeClassName.equals(short.class.getName())) {
-            return (Short.parseShort(input));
-        } else if (typeClassName.equals(Short.class.getName())) {
-            return (Short.valueOf(input));
-        } else if (typeClassName.equals(byte.class.getName())) {
-            return (Byte.parseByte(input));
-        } else if (typeClassName.equals(Byte.class.getName())) {
-            return (Byte.valueOf(input));
-
+        if (typeClassName.equals(int.class.getName()) || typeClassName.equals(Integer.class.getName())) {
+            return input.intValue();
+        } else if (typeClassName.equals(short.class.getName()) || typeClassName.equals(Short.class.getName())) {
+            return (short) input.intValue();
+        } else if (typeClassName.equals(byte.class.getName()) || typeClassName.equals(Byte.class.getName())) {
+            return (byte) input.intValue();
             // Float
         } else if (typeClassName.equals(float.class.getName())) {
-            return (Float.parseFloat(input));
+            return input.floatValue();
         } else if (typeClassName.equals(Float.class.getName())) {
-            return (Float.valueOf(input));
+            return input.floatValue();
         } else if (typeClassName.equals(double.class.getName())) {
-            return (Double.parseDouble(input));
+            return input.doubleValue();
         } else if (typeClassName.equals(Double.class.getName())) {
-            return (Double.valueOf(input));
+            return input.doubleValue();
 
             // BigInteger
         } else if (typeClassName.equals(BigInteger.class.getName())) {
-            return (new BigInteger(input));
+            if (input instanceof BigDecimal) {
+                return ((BigDecimal) input).toBigInteger();
+            }
+            if (input instanceof BigInteger) {
+                return input;
+            }
+            return BigInteger.valueOf(input.longValue());
         } else if (typeClassName.equals(long.class.getName())) {
-            return (Long.parseLong(input));
+            return input.longValue();
         } else if (typeClassName.equals(Long.class.getName())) {
-            return (Long.valueOf(input));
+            return input.longValue();
 
             // BigDecimal
         } else if (typeClassName.equals(BigDecimal.class.getName())) {
-            return (new BigDecimal(input));
+            if (input instanceof BigDecimal) {
+                return (input);
+            }
+            if (input instanceof BigInteger) {
+                return new BigDecimal(((BigInteger) input));
+            }
+            return BigDecimal.valueOf(input.doubleValue());
         }
 
         throw new RuntimeException(String.format("[%s] is no valid number-type", typeClassName));
+
     }
 
-    public Object out(final Object object) {
+    public Number out(final Number object) {
         return object;
     }
 }