Repository: jooby-project/jooby
Pull request #1386 (2019-09-07 20:53:49)

Feature/more value types 1377
Work to make a Value extension point for data conversion. The extension uses the ServiceLoader as discussed. It is mainly focused for object conversion and thus might fit nicely with the simple string conversion you @jknack are working on.

This PR is for discussion. Its not complete but I wanted to get it in before the end of the weekend as promised.

The big thing I think I want to add is more conversion methods to ValueContainer. Also I might make it so this extension loader only run for `Value#isObject()` given your work but unsure right now.


Patch:
diff --git a/jooby/src/main/java/io/jooby/Value.java b/jooby/src/main/java/io/jooby/Value.java
index e5d511c5f..834173cce 100644
--- a/jooby/src/main/java/io/jooby/Value.java
+++ b/jooby/src/main/java/io/jooby/Value.java
@@ -10,6 +10,7 @@ import io.jooby.internal.HashValue;
 import io.jooby.internal.MissingValue;
 import io.jooby.internal.SingleValue;
 import io.jooby.internal.ValueInjector;
+import io.jooby.spi.ValueContainer;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -52,7 +53,7 @@ import java.util.function.Function;
  * @since 2.0.0
  * @author edgar
  */
-public interface Value extends Iterable<Value> {
+public interface Value extends Iterable<Value>, ValueContainer {
 
   /**
    * Convert this value to long (if possible).
diff --git a/jooby/src/main/java/io/jooby/internal/ValueInjector.java b/jooby/src/main/java/io/jooby/internal/ValueInjector.java
index 66665a0e6..348045f00 100644
--- a/jooby/src/main/java/io/jooby/internal/ValueInjector.java
+++ b/jooby/src/main/java/io/jooby/internal/ValueInjector.java
@@ -14,6 +14,7 @@ import io.jooby.StatusCode;
 import io.jooby.TypeMismatchException;
 import io.jooby.Value;
 import io.jooby.internal.reflect.$Types;
+import io.jooby.spi.ValueConverters;
 
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -147,6 +148,10 @@ public final class ValueInjector {
       throws IllegalAccessException, InvocationTargetException, InstantiationException,
       NoSuchMethodException {
     if (scope.isObject() || scope.isSingle()) {
+      Object o = ValueConverters.getInstance().convert(scope, type);
+      if (o != null) {
+        return o;
+      }
       return newInstance(type, scope);
     } else if (scope.isMissing()) {
       if (type.isPrimitive()) {
@@ -250,6 +255,9 @@ public final class ValueInjector {
     if (FileUpload.class == rawType) {
       return true;
     }
+    if (rawType.isEnum()) {
+      return true;
+    }
     /**********************************************************************************************
      * Static method: valueOf
      * ********************************************************************************************
diff --git a/jooby/src/main/java/io/jooby/spi/ValueContainer.java b/jooby/src/main/java/io/jooby/spi/ValueContainer.java
new file mode 100644
index 000000000..0af3a6ca3
--- /dev/null
+++ b/jooby/src/main/java/io/jooby/spi/ValueContainer.java
@@ -0,0 +1,80 @@
+package io.jooby.spi;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import io.jooby.Value;
+
+/**
+ *  A restricted base type of {@link Value} for the {@link ValueConverter} SPI.
+ * @author agentgt
+ *
+ */
+public interface ValueContainer {
+
+  /**
+   * Get a value at the given position.
+   *
+   * @param index Position.
+   * @return A value at the given position.
+   */
+  @Nonnull ValueContainer get(@Nonnull int index);
+
+  /**
+   * Get a value that matches the given name.
+   *
+   * @param name Field name.
+   * @return Field value.
+   */
+  @Nonnull ValueContainer get(@Nonnull String name);
+
+  /**
+   * Get string value.
+   *
+   * @return String value.
+   */
+  @Nonnull String value();
+
+  /**
+   * Convert this value to String (if possible) or <code>null</code> when missing.
+   *
+   * @return Convert this value to String (if possible) or <code>null</code> when missing.
+   */
+  @Nullable String valueOrNull();
+
+  /**
+   * True for missing values.
+   *
+   * @return True for missing values.
+   */
+  boolean isMissing();
+
+  /**
+   * The number of values this one has. For single values size is <code>0</code>.
+   *
+   * @return Number of values. Mainly for array and hash values.
+   */
+  int size();
+
+  /**
+   * True if this value is an array/sequence (not single or hash).
+   *
+   * @return True if this value is an array/sequence.
+   */
+  boolean isArray();
+
+  /**
+   * True if this is a single value (not a hash or array).
+   *
+   * @return True if this is a single value (not a hash or array).
+   */
+  boolean isSingle();
+
+  /**
+   * True if this is a hash/object value (not single or array).
+   *
+   * @return True if this is a hash/object value (not single or array).
+   */
+  boolean isObject();
+
+}
diff --git a/jooby/src/main/java/io/jooby/spi/ValueConverter.java b/jooby/src/main/java/io/jooby/spi/ValueConverter.java
new file mode 100644
index 000000000..598029cf8
--- /dev/null
+++ b/jooby/src/main/java/io/jooby/spi/ValueConverter.java
@@ -0,0 +1,35 @@
+package io.jooby.spi;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import io.jooby.TypeMismatchException;
+import io.jooby.Value;
+
+/**
+ * An SPI for value conversion.
+ * @author agentgt
+ */
+public interface ValueConverter {
+  /**
+   * A short circuit to see if the converter supports the given type.
+   *
+   * This defaults to true to allow a functional interface since convert can return null
+   * to indicate it does not support the type.
+   *
+   * @param type class or interface
+   * @return true if the converter can convert for the type
+   */
+  default boolean supportsType(@Nonnull Class<?> type) {
+    return true;
+  }
+  /**
+   * Converts values for {@link Value#to} and friends. Returning null indicates the type is not supported
+   * or the converter chose not to do the conversion.
+   * @param value the value to be converted
+   * @param type the desired type. The type should be the equal or a super of the resulting instance returned.
+   * @return <code>null</code> indicates that the converter chose to delegate to other converters down the chain.
+   * @throws TypeMismatchException if the converter cannot convert the type and does not want to delegate.
+   */
+  @Nullable Object convert(@Nonnull ValueContainer value, @Nonnull Class<?> type) throws TypeMismatchException;
+}
diff --git a/jooby/src/main/java/io/jooby/spi/ValueConverters.java b/jooby/src/main/java/io/jooby/spi/ValueConverters.java
new file mode 100644
index 000000000..81b782d61
--- /dev/null
+++ b/jooby/src/main/java/io/jooby/spi/ValueConverters.java
@@ -0,0 +1,114 @@
+package io.jooby.spi;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.ServiceLoader;
+
+import javax.annotation.Nullable;
+
+import io.jooby.TypeMismatchException;
+
+/**
+ * Contains the {@link ValueConverter}s loaded via the ServiceLoader. It is is a
+ * singleton and an instance can be retrieved with {@link #getInstance()}. The
+ * ValueConverters are stored in an ordered collection and thus resolution of
+ * type to be converted is based on the order of the converters.
+ *
+ * @author agentgt
+ *
+ */
+public final class ValueConverters {
+
+  // Allow thread safe adding of ValueConverters.
+  private final Iterable<ValueConverter> valueConverters;
+
+  // Initialization on demand
+  private static final class Hidden {
+
+    private static volatile ValueConverters instance = ValueConverters.builder().fromServiceLoader().build();
+  }
+
+  private ValueConverters(Iterable<ValueConverter> valueConverters) {
+    super();
+    this.valueConverters = valueConverters;
+  }
+
+  static Builder builder() {
+    return new Builder();
+  }
+
+  static final class Builder {
+
+    private final List<ValueConverter> valueConverters = new ArrayList<>();
+
+    Builder fromServiceLoader() {
+      ServiceLoader<ValueConverter> sl = ServiceLoader.load(ValueConverter.class);
+      // If any failes to load we will fail entirely.
+      // The value converters found earlier in the classpath take precedence.
+      sl.forEach(this::add);
+      return this;
+    }
+
+    /**
+     * You can add value converters programmatic. For now its protected. Its
+     * also to aid unit testing since serviceloader is inherently static
+     * singleton.
+     *
+     * @param vc
+     * @return
+     */
+    Builder add(ValueConverter vc) {
+      valueConverters.add(vc);
+      return this;
+    }
+
+    Builder clear() {
+      valueConverters.clear();
+      return this;
+    }
+
+    ValueConverters build() {
+      return new ValueConverters(valueConverters);
+    }
+
+    ValueConverters set() {
+      ValueConverters vc = build();
+      Hidden.instance = vc;
+      return vc;
+    }
+  }
+
+  /**
+   * Attempts to convert values to an object based on the provided type.
+   *
+   * @param v
+   *          value
+   * @param c
+   *          desired type
+   * @return the type if converted or null if conversion was not possible.
+   * @throws TypeMismatchException
+   *           failure in a converter
+   */
+  public @Nullable Object convert(ValueContainer v, Class<?> c) throws TypeMismatchException {
+    Object result = null;
+    for (ValueConverter vc : valueConverters) {
+      if (vc.supportsType(c)) {
+        result = vc.convert(v, c);
+        if (result != null) {
+          return result;
+        }
+      }
+    }
+    return result;
+  }
+
+  /**
+   * The ValueConverters singleton usually preloaded by the ServiceLoader.
+   *
+   * @return the shared singleton used by Jooby
+   */
+  public static ValueConverters getInstance() {
+    return ValueConverters.Hidden.instance;
+  }
+
+}


Test patch:
diff --git a/jooby/src/test/java/io/jooby/spi/ValueConvertersTest.java b/jooby/src/test/java/io/jooby/spi/ValueConvertersTest.java
new file mode 100644
index 000000000..2357af2ae
--- /dev/null
+++ b/jooby/src/test/java/io/jooby/spi/ValueConvertersTest.java
@@ -0,0 +1,62 @@
+package io.jooby.spi;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.function.Consumer;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Test;
+
+import io.jooby.MissingValueException;
+import io.jooby.QueryString;
+import io.jooby.internal.UrlParser;
+
+class ValueConvertersTest {
+
+  @AfterAll
+  static void restoreFromServiceLoader() {
+    // Restore ValueConvert list for other unit tests.
+    ValueConverters.builder().fromServiceLoader().set();
+  }
+
+  @Test
+  void testConvert() {
+    ValueConverters.builder().add((value, type) -> {
+      if (type == MyValue.class) {
+        MyValue mv = new MyValue();
+        // we have chosen simple parameters names to make sure we don't get a
+        // false positve
+        // from the reflection converter.
+        mv.name = value.get("n").value();
+        // TODO: ValueContainer probably should have primitive convert methods.
+        mv.order = Integer.parseInt(value.get("o").value());
+        return mv;
+      }
+      return null;
+    }).set();
+    queryString("n=stuff&o=1", queryString -> {
+      MyValue mv = queryString.to(MyValue.class);
+      assertEquals("stuff", mv.name);
+      assertEquals(1, mv.order);
+    });
+    queryString("n=stuff&missingOrder=1", queryString -> {
+      try {
+        queryString.to(MyValue.class);
+        fail();
+      } catch (MissingValueException mve) {
+        assertEquals("Missing value: 'o'", mve.getMessage());
+      }
+    });
+  }
+
+  static class MyValue {
+
+    public String name;
+    public int order;
+  }
+
+  private void queryString(String queryString, Consumer<QueryString> consumer) {
+    consumer.accept(UrlParser.queryString(queryString));
+  }
+
+}