bean parser: support null values fix #408

Author: jknack

coverage-report/src/test/java/org/jooby/hbv/BeanValidationFeature.java

@@ -9,6 +9,7 @@
 import javax.validation.ConstraintViolationException;
 import javax.validation.Path;
 
+import org.jooby.Parser;
 import org.jooby.hbm.data.Car;
 import org.jooby.json.Jackson;
 import org.jooby.test.ServerFeature;
@@ -17,6 +18,8 @@
 public class BeanValidationFeature extends ServerFeature {
 
   {
+    parser(Parser.bean(true));
+
     use(new Jackson());
 
     use(new Hbv(Car.class));

coverage-report/src/test/java/org/jooby/issues/Issue408.java

@@ -0,0 +1,41 @@
+package org.jooby.issues;
+
+import java.util.Date;
+
+import org.jooby.Parser;
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue408 extends ServerFeature {
+
+  public static class Bean408 {
+    public Integer id;
+
+    public String title;
+
+    public Date releaseDate;
+
+    @Override
+    public String toString() {
+      return id + ":" + title + ":" + releaseDate;
+    }
+  }
+
+  {
+    parser(Parser.bean(true));
+
+    get("/408", req -> req.params().to(Bean408.class).toString());
+  }
+
+  @Test
+  public void shouldIgnoreEmptyValues() throws Exception {
+    request()
+        .get("/408?id=1&title=Title&releaseDate=")
+        .expect("1:Title:null");
+
+    request()
+        .get("/408")
+        .expect("null:null:null");
+  }
+
+}

jooby/src/main/java/org/jooby/Err.java

@@ -41,6 +41,14 @@
 @SuppressWarnings("serial")
 public class Err extends RuntimeException {
 
+  public static class Missing extends Err {
+
+    public Missing(final String name) {
+      super(Status.BAD_REQUEST, name);
+    }
+
+  }
+
   /**
    * Default err handler it does content negotation. On <code>text/html</code> requests the err
    * handler creates an <code>err</code> view and set as model the {@link Err#toMap()}.

jooby/src/main/java/org/jooby/Jooby.java

@@ -640,6 +640,9 @@ public EnvDep(final Predicate<String> predicate, final Consumer<Config> callback
   /** Don't add same mapper twice . */
   private Set<String> mappers = new HashSet<>();
 
+  /** Bean parser . */
+  private Optional<Parser> beanParser = Optional.empty();
+
   public Jooby() {
     this(null);
   }
@@ -1120,7 +1123,11 @@ public Session.Definition session(final Session.Store store) {
    * @return This jooby instance.
    */
   public Jooby parser(final Parser parser) {
-    bag.add(requireNonNull(parser, "A parser is required."));
+    if (parser instanceof BeanParser) {
+      beanParser = Optional.of(parser);
+    } else {
+      bag.add(requireNonNull(parser, "A parser is required."));
+    }
     return this;
   }
 
@@ -3919,7 +3926,7 @@ private Injector bootstrap(final Config args,
       parsers.addBinding().toInstance(new DateParser(dateFormat));
       parsers.addBinding().toInstance(new LocalDateParser(dateTimeFormatter));
       parsers.addBinding().toInstance(new LocaleParser());
-      parsers.addBinding().toInstance(new BeanParser());
+      parsers.addBinding().toInstance(beanParser.orElseGet(() -> new BeanParser(false)));
       parsers.addBinding().toInstance(new StaticMethodParser("valueOf"));
       parsers.addBinding().toInstance(new StaticMethodParser("fromString"));
       parsers.addBinding().toInstance(new StaticMethodParser("forName"));

jooby/src/main/java/org/jooby/Parser.java

@@ -24,6 +24,8 @@
 import java.util.Map;
 import java.util.Optional;
 
+import org.jooby.internal.parser.BeanParser;
+
 import com.google.inject.Key;
 import com.google.inject.TypeLiteral;
 
@@ -417,4 +419,44 @@ interface Context extends Builder {
    */
   Object parse(TypeLiteral<?> type, Context ctx) throws Throwable;
 
+  /**
+   * Overwrite the default bean parser with <code>null</code> supports. The default bean parser
+   * doesn't allow <code>null</code>, so if a parameter is optional you must declare it as
+   * {@link Optional} otherwise parsing fails with a <code>404</code> status code.
+   *
+   * For example:
+   * <pre>{@code
+   *
+   * public class Book {
+   *
+   *   public String title;
+   *
+   *   public Date releaseDate;
+   *
+   *   public String toString() {
+   *     return title + ":" + releaseDate;
+   *   }
+   * }
+   *
+   * {
+   *   parser(Parser.bean(true));
+   *
+   *   post("/", req -> {
+   *     return req.params(Book.class).toString();
+   *   });
+   * }
+   * }</pre>
+   *
+   * With <code>/?title=Title&releaseDate=</code> prints <code>Title:null</code>.
+   *
+   * Now, same call with <code>allowNulls=false</code> results in <code>Bad Request: 400</code>
+   * because <code>releaseDate</code> if required and isn't present in the HTTP request.
+   *
+   * @param allowNulls Enabled null supports while parsing HTTP params as Java Beans.
+   * @return A new parser.
+   */
+  static Parser bean(final boolean allowNulls) {
+    return new BeanParser(allowNulls);
+  }
+
 }

jooby/src/main/java/org/jooby/internal/MutantImpl.java

@@ -89,7 +89,7 @@ public <T> T to(final TypeLiteral<T> type, final MediaType mtype) {
         results.put(type, result);
       } catch (ParamNotFound ex) {
         Tuple3<String, String, Status> md = md();
-        throw new Err(parser.statusCode(ex), String.format(REQUIRED, md._2));
+        throw new Err.Missing(String.format(REQUIRED, md._2));
       } catch (Err ex) {
         throw ex;
       } catch (Throwable ex) {

jooby/src/main/java/org/jooby/internal/parser/BeanParser.java

@@ -25,10 +25,12 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
+import org.jooby.Err;
 import org.jooby.Mutant;
 import org.jooby.Parser;
 import org.jooby.Request;
@@ -46,8 +48,22 @@
 import com.google.common.reflect.Reflection;
 import com.google.inject.TypeLiteral;
 
+import javaslang.control.Try;
+
 public class BeanParser implements Parser {
 
+  private Function<? super Throwable, Try<? extends Object>> MISSING = x -> {
+    return x instanceof Err.Missing ? Try.success(null) : Try.failure(x);
+  };
+
+  private Function<? super Throwable, Try<? extends Object>> RETHROW = Try::failure;
+
+  private Function<? super Throwable, Try<? extends Object>> recoverMissing;
+
+  public BeanParser(final boolean allowNulls) {
+    this.recoverMissing = allowNulls ? MISSING : RETHROW;
+  }
+
   @Override
   public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwable {
     Class<?> beanType = type.getRawType();
@@ -58,7 +74,7 @@ public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwab
     return ctx.ifparams(map -> {
       final Object bean;
       if (beanType.isInterface()) {
-        bean = newBeanInterface(ctx.require(Request.class), beanType);
+        bean = newBeanInterface(ctx.require(Request.class), ctx.require(Response.class), beanType);
       } else {
         bean = newBean(ctx.require(Request.class), ctx.require(Response.class), map, beanType);
       }
@@ -85,17 +101,16 @@ private Object newBean(final Request req, final Response rsp,
     if (constructors.size() > 1) {
       return null;
     }
-    final Object bean;
     Constructor<?> constructor = constructors.get(0);
     RequestParamProvider provider = new RequestParamProviderImpl(
         new RequestParamNameProviderImpl(classInfo));
     List<RequestParam> parameters = provider.parameters(constructor);
     Object[] args = new Object[parameters.size()];
     for (int i = 0; i < args.length; i++) {
-      args[i] = parameters.get(i).value(req, rsp);
+      args[i] = value(parameters.get(i), req, rsp);
     }
     // inject args
-    bean = constructor.newInstance(args);
+    final Object bean = constructor.newInstance(args);
 
     // inject fields
     for (Entry<String, Mutant> param : params.entrySet()) {
@@ -109,9 +124,7 @@ private Object newBean(final Request req, final Response rsp,
         int mods = field.getModifiers();
         if (!Modifier.isFinal(mods) && !Modifier.isStatic(mods) && !Modifier.isTransient(mods)) {
           // get
-          RequestParam fparam = new RequestParam(field, pname, field.getGenericType());
-          Object value = fparam.value(req, rsp);
-
+          Object value = value(new RequestParam(field, pname, field.getGenericType()), req, rsp);
           // set
           field.setAccessible(true);
           field.set(root, value);
@@ -148,17 +161,24 @@ private Object seek(final Object bean, final List<String> path) throws Exception
     return it;
   }
 
-  private Object newBeanInterface(final Request req, final Class<?> beanType) {
-
+  private Object newBeanInterface(final Request req, final Response rsp, final Class<?> beanType) {
     return Reflection.newProxy(beanType, (proxy, method, args) -> {
       StringBuilder name = new StringBuilder(method.getName()
           .replace("get", "")
           .replace("is", ""));
       name.setCharAt(0, Character.toLowerCase(name.charAt(0)));
-      return req.param(name.toString()).to(TypeLiteral.get(method.getGenericReturnType()));
+      return value(new RequestParam(method, name.toString(), method.getGenericReturnType()), req,
+          rsp);
     });
   }
 
+  private Object value(final RequestParam param, final Request req, final Response rsp)
+      throws Throwable {
+    return Try.of(() -> param.value(req, rsp))
+        .recoverWith(recoverMissing)
+        .getOrElseThrow(Function.identity());
+  }
+
   private static List<String> name(final String name) {
     return Splitter.on(new CharMatcher() {
       @Override

md/req.md

@@ -235,7 +235,7 @@ public class Contact {
 ```java
 {
   post("/save", req -> {
-    Contact contact = req.body().to(Contact.class);
+    Contact contact = req.params().to(Contact.class);
     // save contact...
   });
 }