Request local changes

jknack · jknack · commit 698a14f6edab · 2016-02-20T18:30:02.000-03:00
* supports injection of local attributes fix jooby-project#278 * request local attributes: change return type of req.get from Optional<T> to <T> fix jooby-project#279
diff --git a/coverage-report/src/test/java/org/jooby/CsrfFeature.java b/coverage-report/src/test/java/org/jooby/CsrfFeature.java
@@ -11,9 +11,9 @@ public class CsrfFeature extends ServerFeature {
   {
     use("*", new CsrfHandler());
 
-    get("/csrf", req -> req.get("csrf").get());
+    get("/csrf", req -> req.ifGet("csrf").get());
 
-    post("/csrf", req -> req.get("csrf").get());
+    post("/csrf", req -> req.ifGet("csrf").get());
   }
 
   @Test
diff --git a/coverage-report/src/test/java/org/jooby/RequestLocalsFeature.java b/coverage-report/src/test/java/org/jooby/RequestLocalsFeature.java
@@ -21,7 +21,7 @@ public class RequestLocalsFeature extends ServerFeature {
       assertFalse(req.isSet("l1"));
       req.set("l1", "v1");
       assertTrue(req.isSet("l1"));
-      return req.get("l1");
+      return req.ifGet("l1");
     });
 
     get("/locals/unset", (req) -> {
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue25.java b/coverage-report/src/test/java/org/jooby/issues/Issue25.java
@@ -11,13 +11,13 @@ public class Issue25 extends ServerFeature {
 
   {
     get("/contextPath", req -> {
-      assertEquals("", req.get("contextPath").get());
+      assertEquals("", req.ifGet("contextPath").get());
       assertEquals("/", req.require(Config.class).getString("application.path"));
       return req.path();
     });
 
     get("/req-path", req -> {
-      assertEquals("/req-path", req.get("path").get());
+      assertEquals("/req-path", req.ifGet("path").get());
       assertEquals("/req-path", req.path());
       return req.path();
     });
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue278.java b/coverage-report/src/test/java/org/jooby/issues/Issue278.java
@@ -0,0 +1,33 @@
+package org.jooby.issues;
+
+import org.jooby.mvc.GET;
+import org.jooby.mvc.Local;
+import org.jooby.mvc.Path;
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue278 extends ServerFeature {
+
+  @Path("/issue278")
+  public static class Resource {
+
+    @GET
+    public Object local(@Local final int local) {
+      return local;
+    }
+  }
+
+  {
+    use("*", (req, rsp) -> {
+      req.set("local", 678);
+    });
+
+    use(Resource.class);
+  }
+
+  @Test
+  public void shouldNotDisplayStacktrace() throws Exception {
+    request().get("/issue278").expect("678");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue278b.java b/coverage-report/src/test/java/org/jooby/issues/Issue278b.java
@@ -0,0 +1,35 @@
+package org.jooby.issues;
+
+import java.util.Optional;
+
+import org.jooby.mvc.GET;
+import org.jooby.mvc.Local;
+import org.jooby.mvc.Path;
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue278b extends ServerFeature {
+
+  @Path("/issue278")
+  public static class Resource {
+
+    @GET
+    public Object local(@Local final Optional<Integer> local) {
+      return local;
+    }
+  }
+
+  {
+    use("*", (req, rsp) -> {
+      req.set("local", 678);
+    });
+
+    use(Resource.class);
+  }
+
+  @Test
+  public void shouldNotDisplayStacktrace() throws Exception {
+    request().get("/issue278").expect("Optional[678]");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue278c.java b/coverage-report/src/test/java/org/jooby/issues/Issue278c.java
@@ -0,0 +1,35 @@
+package org.jooby.issues;
+
+import java.util.Map;
+
+import org.jooby.mvc.GET;
+import org.jooby.mvc.Local;
+import org.jooby.mvc.Path;
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue278c extends ServerFeature {
+
+  @Path("/issue278")
+  public static class Resource {
+
+    @GET
+    public Object local(@Local final Map<String, Object> local) {
+      return local;
+    }
+  }
+
+  {
+    use("*", (req, rsp) -> {
+      req.set("local", 678);
+    });
+
+    use(Resource.class);
+  }
+
+  @Test
+  public void shouldNotDisplayStacktrace() throws Exception {
+    request().get("/issue278").expect("{path=/issue278, contextPath=, local=678}");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue279.java b/coverage-report/src/test/java/org/jooby/issues/Issue279.java
@@ -0,0 +1,37 @@
+package org.jooby.issues;
+
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue279 extends ServerFeature {
+
+  {
+    use("*", (req, rsp) -> {
+      req.set("local", 678);
+    });
+
+    use("/issue279")
+        .get("/get", req -> {
+          int value = req.get("local");
+          return value;
+        })
+        .get("/getdef", req -> {
+          String value = req.get("def", "def");
+          return value;
+        })
+        .get("/geterr", req -> {
+          String value = req.get("def");
+          return value;
+        });
+  }
+
+  @Test
+  public void getRequestLocals() throws Exception {
+    request().get("/issue279/get").expect("678");
+
+    request().get("/issue279/getdef").expect("def");
+
+    request().get("/issue279/geterr").expect(400);
+  }
+
+}
diff --git a/jooby-hbs/src/main/java/org/jooby/internal/hbs/RequestValueResolver.java b/jooby-hbs/src/main/java/org/jooby/internal/hbs/RequestValueResolver.java
@@ -37,7 +37,7 @@ public class RequestValueResolver implements ValueResolver {
   public Object resolve(final Object context, final String name) {
     Object value = null;
     if (context instanceof Request) {
-      value = ((Request) context).get(name).orElse(null);
+      value = ((Request) context).ifGet(name).orElse(null);
     }
     return value == null ? UNRESOLVED : value;
   }
diff --git a/jooby-hbs/src/test/java/org/jooby/internal/hbs/RequestValueResolverTest.java b/jooby-hbs/src/test/java/org/jooby/internal/hbs/RequestValueResolverTest.java
@@ -23,7 +23,7 @@ public void resolveProperty() throws Exception {
     new MockUnit(Request.class)
         .expect(unit -> {
           Request Request = unit.get(Request.class);
-          expect(Request.get("prop")).andReturn(Optional.of("x"));
+          expect(Request.ifGet("prop")).andReturn(Optional.of("x"));
         })
         .run(unit -> {
           assertEquals("x", new RequestValueResolver().resolve(unit.get(Request.class), "prop"));
@@ -35,7 +35,7 @@ public void resolveMissingProperty() throws Exception {
     new MockUnit(Request.class)
         .expect(unit -> {
           Request Request = unit.get(Request.class);
-          expect(Request.get("prop")).andReturn(Optional.empty());
+          expect(Request.ifGet("prop")).andReturn(Optional.empty());
         })
         .run(
             unit -> {
diff --git a/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthCallback.java b/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthCallback.java
@@ -92,7 +92,7 @@ public void handle(final Request req, final Response rsp, final Chain chain) thr
 
       // where to go? if there is a local var set, it use that. If there is a session var set, it
       // use that. Otherwise, it use the global property: "auth.login.redirectTo".
-      String requestedUrl = req.<String> get(Pac4jConstants.REQUESTED_URL).orElseGet(() -> {
+      String requestedUrl = req.<String> ifGet(Pac4jConstants.REQUESTED_URL).orElseGet(() -> {
         return session.unset(Pac4jConstants.REQUESTED_URL).toOptional()
             .map(url -> url.equals("/") ? this.redirectTo : url)
             .orElse(this.redirectTo);
diff --git a/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthContext.java b/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthContext.java
@@ -154,7 +154,7 @@ public String toString() {
 
   @Override
   public Object getRequestAttribute(final String name) {
-    Optional<Object> attr = req.get(name);
+    Optional<Object> attr = req.ifGet(name);
     return attr.orElse(null);
   }
 
diff --git a/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthFilter.java b/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthFilter.java
@@ -124,7 +124,7 @@ public void handle(final Request req, final Response rsp) throws Exception {
   }
 
   private String profileID(final boolean useSession, final Request req) {
-    return req.<String> get(Auth.ID)
+    return req.<String> ifGet(Auth.ID)
         .orElseGet(() -> useSession ? req.session().get(Auth.ID).value(null) : null);
   }
 
diff --git a/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthLogout.java b/jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthLogout.java
@@ -57,7 +57,7 @@ public void handle(final Request req, final Response rsp) throws Exception {
     } else {
       log.debug("nothing to logout from session");
     }
-    String redirectTo = req.<String> get("auth.logout.redirectTo").orElse(this.redirectTo);
+    String redirectTo = req.<String> ifGet("auth.logout.redirectTo").orElse(this.redirectTo);
     rsp.redirect(redirectTo);
   }
 
diff --git a/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthCallbackTest.java b/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthCallbackTest.java
@@ -42,7 +42,7 @@ public class AuthCallbackTest {
 
   private Block localRedirect = unit -> {
     Request req = unit.get(Request.class);
-    expect(req.get("pac4jRequestedUrl")).andReturn(Optional.empty());
+    expect(req.ifGet("pac4jRequestedUrl")).andReturn(Optional.empty());
   };
 
   @SuppressWarnings({"unchecked", "rawtypes" })
diff --git a/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthContextTest.java b/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthContextTest.java
@@ -213,7 +213,7 @@ public void getReqAttr() throws Exception {
         .expect(params1)
         .expect(unit -> {
           Request req = unit.get(Request.class);
-          expect(req.get("r")).andReturn(Optional.of("v"));
+          expect(req.ifGet("r")).andReturn(Optional.of("v"));
         })
         .run(unit -> {
           AuthContext ctx = new AuthContext(unit.get(Request.class), unit.get(Response.class));
@@ -224,7 +224,7 @@ public void getReqAttr() throws Exception {
         .expect(params1)
         .expect(unit -> {
           Request req = unit.get(Request.class);
-          expect(req.get("r")).andReturn(Optional.empty());
+          expect(req.ifGet("r")).andReturn(Optional.empty());
         })
         .run(unit -> {
           AuthContext ctx = new AuthContext(unit.get(Request.class), unit.get(Response.class));
diff --git a/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthFilterTest.java b/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthFilterTest.java
@@ -89,7 +89,7 @@ private <C extends Client> Block findNoClient(final Class<C> clientType, final S
   private Block reqAuthID(final String id) {
     return unit -> {
       Request req = unit.get(Request.class);
-      expect(req.get(Auth.ID)).andReturn(Optional.ofNullable(id));
+      expect(req.ifGet(Auth.ID)).andReturn(Optional.ofNullable(id));
     };
   }
 
diff --git a/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthLogoutTest.java b/jooby-pac4j/src/test/java/org/jooby/internal/pac4j/AuthLogoutTest.java
@@ -26,7 +26,7 @@ public void unset() throws Exception {
           Request req = unit.get(Request.class);
           expect(req.ifSession()).andReturn(Optional.of(session));
           expect(req.require(AuthStore.class)).andReturn(unit.get(AuthStore.class));
-          expect(req.get("auth.logout.redirectTo")).andReturn(Optional.empty());
+          expect(req.ifGet("auth.logout.redirectTo")).andReturn(Optional.empty());
         })
         .expect(unit -> {
           Optional<String> id = Optional.of("1");
@@ -60,7 +60,7 @@ public void unsetIgnore() throws Exception {
 
           Request req = unit.get(Request.class);
           expect(req.ifSession()).andReturn(Optional.of(session));
-          expect(req.get("auth.logout.redirectTo")).andReturn(Optional.empty());
+          expect(req.ifGet("auth.logout.redirectTo")).andReturn(Optional.empty());
         })
         .expect(unit -> {
           Optional<String> id = Optional.empty();
@@ -87,7 +87,7 @@ public void unsetNoSession() throws Exception {
         .expect(unit -> {
           Request req = unit.get(Request.class);
           expect(req.ifSession()).andReturn(Optional.empty());
-          expect(req.get("auth.logout.redirectTo")).andReturn(Optional.empty());
+          expect(req.ifGet("auth.logout.redirectTo")).andReturn(Optional.empty());
         })
         .expect(unit -> {
           Response rsp = unit.get(Response.class);
diff --git a/jooby/src/main/java/org/jooby/Request.java b/jooby/src/main/java/org/jooby/Request.java
@@ -258,10 +258,20 @@ public Map<String, Object> attributes() {
     }
 
     @Override
-    public <T> Optional<T> get(final String name) {
+    public <T> Optional<T> ifGet(final String name) {
+      return req.ifGet(name);
+    }
+
+    @Override
+    public <T> T get(final String name) {
       return req.get(name);
     }
 
+    @Override
+    public <T> T get(final String name, final T def) {
+      return req.get(name, def);
+    }
+
     @Override
     public Request set(final String name, final Object value) {
       req.set(name, value);
@@ -737,7 +747,29 @@ default boolean xhr() {
    * @return True if the local attribute is set.
    */
   default boolean isSet(final String name) {
-    return get(name).isPresent();
+    return ifGet(name).isPresent();
+  }
+
+  /**
+   * Get a request local attribute.
+   *
+   * @param name Attribute's name.
+   * @param <T> Target type.
+   * @return A local attribute.
+   */
+  <T> Optional<T> ifGet(String name);
+
+  /**
+   * Get a request local attribute.
+   *
+   * @param name Attribute's name.
+   * @param def A default value.
+   * @param <T> Target type.
+   * @return A local attribute.
+   */
+  default <T> T get(final String name, final T def) {
+    Optional<T> opt = ifGet(name);
+    return opt.orElse(def);
   }
 
   /**
@@ -746,8 +778,12 @@ default boolean isSet(final String name) {
    * @param name Attribute's name.
    * @param <T> Target type.
    * @return A local attribute.
+   * @throws Err with {@link Status#BAD_REQUEST}.
    */
-  <T> Optional<T> get(String name);
+  default <T> T get(final String name) {
+    Optional<T> opt = ifGet(name);
+    return opt.orElseThrow(() -> new Err(Status.BAD_REQUEST, "Not found: " + name));
+  }
 
   /**
    * Remove a request local attribute.
diff --git a/jooby/src/main/java/org/jooby/internal/RequestImpl.java b/jooby/src/main/java/org/jooby/internal/RequestImpl.java
@@ -122,7 +122,7 @@ public String contextPath() {
 
   @SuppressWarnings("unchecked")
   @Override
-  public <T> Optional<T> get(final String name) {
+  public <T> Optional<T> ifGet(final String name) {
     requireNonNull(name, "A local's name is required.");
     return Optional.ofNullable((T) locals.get(name));
   }
diff --git a/jooby/src/main/java/org/jooby/internal/mvc/RequestParam.java b/jooby/src/main/java/org/jooby/internal/mvc/RequestParam.java
diff --git a/jooby/src/main/java/org/jooby/mvc/Local.java b/jooby/src/main/java/org/jooby/mvc/Local.java
diff --git a/jooby/src/test/java/org/jooby/RequestForwardingTest.java b/jooby/src/test/java/org/jooby/RequestForwardingTest.java
diff --git a/jooby/src/test/java/org/jooby/RequestTest.java b/jooby/src/test/java/org/jooby/RequestTest.java
diff --git a/md/req.md b/md/req.md

Original file line number	Diff line number	Diff line change
`@@ -11,9 +11,9 @@ public class CsrfFeature extends ServerFeature {`
`11`	`11`	`{`
`12`	`12`	`use("*", new CsrfHandler());`
`13`	`13`
`14`		`- get("/csrf", req -> req.get("csrf").get());`
	`14`	`+ get("/csrf", req -> req.ifGet("csrf").get());`
`15`	`15`
`16`		`- post("/csrf", req -> req.get("csrf").get());`
	`16`	`+ post("/csrf", req -> req.ifGet("csrf").get());`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public class RequestValueResolver implements ValueResolver {`
`37`	`37`	`public Object resolve(final Object context, final String name) {`
`38`	`38`	`Object value = null;`
`39`	`39`	`if (context instanceof Request) {`
`40`		`- value = ((Request) context).get(name).orElse(null);`
	`40`	`+ value = ((Request) context).ifGet(name).orElse(null);`
`41`	`41`	`}`
`42`	`42`	`return value == null ? UNRESOLVED : value;`
`43`	`43`	`}`