MVC Route: @SessionParam fix #1387

Author: jknack

TODO

@@ -1,3 +1,4 @@
+* review ValueNode return type on Context, some must be Value not ValueNode.
 * Make Session.toMap() read-only
 * review toString on Value API (removal of QueryStringValue?)
 * tests and coverage

jooby/src/main/java/io/jooby/Context.java

@@ -127,6 +127,15 @@ public interface Context extends Registry {
    */
   @Nonnull Session session();
 
+  /**
+   * Find a session attribute using the given name. If there is no session or attribute under that
+   * name a missing value is returned.
+   *
+   * @param name Attribute's name.
+   * @return Session's attribute or missing.
+   */
+  @Nonnull Value session(@Nonnull String name);
+
   /**
    * Find an existing session.
    *

jooby/src/main/java/io/jooby/DefaultContext.java

@@ -86,6 +86,14 @@ public interface DefaultContext extends Context {
     return Value.create(this, name, flash().get(name));
   }
 
+  @Override default @Nonnull Value session(@Nonnull String name) {
+    Session session = sessionOrNull();
+    if (session != null) {
+      return session.get(name);
+    }
+    return Value.missing(name);
+  }
+
   @Override default @Nonnull Session session() {
     Session session = sessionOrNull();
     if (session == null) {

jooby/src/main/java/io/jooby/ForwardingContext.java

@@ -65,6 +65,10 @@ public ForwardingContext(@Nonnull Context context) {
     return ctx.flash(name);
   }
 
+  @Nonnull @Override public Value session(@Nonnull String name) {
+    return ctx.session(name);
+  }
+
   @Nonnull @Override public Session session() {
     return ctx.session();
   }

modules/jooby-apt/src/main/java/io/jooby/apt/Annotations.java

@@ -23,6 +23,7 @@
 import io.jooby.annotations.PathParam;
 import io.jooby.annotations.Produces;
 import io.jooby.annotations.QueryParam;
+import io.jooby.annotations.SessionParam;
 import io.jooby.annotations.TRACE;
 
 import javax.annotation.Nonnull;
@@ -93,6 +94,10 @@ public interface Annotations {
       javax.ws.rs.QueryParam.class.getName()
   )));
 
+  Set<String> SESSION_PARAMS = unmodifiableSet(new LinkedHashSet<>(asList(
+      SessionParam.class.getName()
+  )));
+
   /**
    * Cookie parameters.
    */

modules/jooby-apt/src/main/java/io/jooby/internal/apt/ParamKind.java

@@ -8,6 +8,7 @@
 import io.jooby.Context;
 import io.jooby.Formdata;
 import io.jooby.Multipart;
+import io.jooby.Session;
 import io.jooby.apt.Annotations;
 import io.jooby.internal.apt.asm.ContextParamWriter;
 import io.jooby.internal.apt.asm.NamedParamWriter;
@@ -77,7 +78,26 @@ public enum ParamKind {
       return new ContextParamWriter();
     }
   },
+  SESSION_ATTRIBUTE_PARAM {
+    @Override public Set<String> annotations() {
+      return Annotations.SESSION_PARAMS;
+    }
 
+    @Override public Method valueObject(ParamDefinition param) throws NoSuchMethodException {
+      if (param.isOptional()) {
+        return Context.class.getDeclaredMethod("sessionOrNull");
+      }
+      return Context.class.getDeclaredMethod("session");
+    }
+
+    @Override public Method singleValue(ParamDefinition param) throws NoSuchMethodException {
+      return Context.class.getDeclaredMethod("session", String.class);
+    }
+
+    @Override public ParamWriter newWriter() {
+      return new NamedParamWriter();
+    }
+  },
   QUERY_PARAM {
     @Override public Set<String> annotations() {
       return Annotations.QUERY_PARAMS;
@@ -163,18 +183,7 @@ public enum ParamKind {
       return new NamedParamWriter();
     }
   },
-  SESSION_PARAM {
-    @Override public Set<String> annotations() {
-      return Collections.emptySet();
-    }
 
-    @Override public Method valueObject(ParamDefinition param) throws NoSuchMethodException {
-      if (param.isOptional()) {
-        return Context.class.getDeclaredMethod("sessionOrNull");
-      }
-      return Context.class.getDeclaredMethod("session");
-    }
-  },
   ROUTE_PARAM {
     @Override public Set<String> annotations() {
       return Collections.emptySet();

modules/jooby-apt/src/test/java/source/Issue1387.java

@@ -3,6 +3,7 @@
 import io.jooby.annotations.ContextParam;
 import io.jooby.annotations.GET;
 import io.jooby.annotations.Path;
+import io.jooby.annotations.SessionParam;
 
 import java.util.Map;
 
@@ -30,4 +31,14 @@ public Data1387 attributeComplex(@ContextParam Data1387 data) {
   public Map<String, Object> attributes(@ContextParam Map<String, Object> attributes) {
     return attributes;
   }
+
+  @GET("/session")
+  public String session(@SessionParam String userId) {
+    return userId;
+  }
+
+  @GET("/session/int")
+  public int session(@SessionParam int userId) {
+    return userId;
+  }
 }

modules/jooby-apt/src/test/java/tests/Issue1387.java

@@ -2,6 +2,7 @@
 
 import io.jooby.Context;
 import io.jooby.MockContext;
+import io.jooby.Session;
 import io.jooby.apt.MvcHandlerCompilerRunner;
 import io.jooby.internal.MockContextHelper;
 import org.junit.jupiter.api.Test;
@@ -48,4 +49,25 @@ public void shouldInjectContextParam() throws Exception {
     ;
   }
 
+  @Test
+  public void shouldInjectSessionParam() throws Exception {
+    new MvcHandlerCompilerRunner(new source.Issue1387())
+        .compile("/1387/session", handler -> {
+          MockContext ctx = MockContextHelper.mockContext();
+          assertEquals(null, handler.apply(ctx));
+
+          Session session = ctx.session();
+          session.put("userId", "abc");
+          assertEquals("abc", handler.apply(ctx));
+        })
+        .compile("/1387/session/int", handler -> {
+          MockContext ctx = MockContextHelper.mockContext();
+          Session session = ctx.session();
+          session.put("userId", 123);
+
+          assertEquals(123, handler.apply(ctx));
+        })
+    ;
+  }
+
 }

modules/jooby-test/src/main/java/io/jooby/MockContext.java

@@ -87,7 +87,7 @@ MockContext setMethod(@Nonnull String method) {
 
   @Nonnull @Override public Session session() {
     if (session == null) {
-      session = new MockSession();
+      session = new MockSession(this);
     }
     return session;
   }

modules/jooby-test/src/main/java/io/jooby/MockSession.java

@@ -16,6 +16,7 @@
  * Mock session.
  */
 public class MockSession implements Session {
+  private MockContext ctx;
   private String sessionId;
 
   private Map<String, String> data = new HashMap<>();
@@ -29,7 +30,8 @@ public class MockSession implements Session {
    *
    * @param sessionId Session ID.
    */
-  public MockSession(@Nonnull String sessionId) {
+  public MockSession(@Nonnull MockContext ctx, @Nonnull String sessionId) {
+    this.ctx = ctx;
     this.sessionId = sessionId;
     this.creationTime = Instant.now();
     this.lastAccessedTime = Instant.now();
@@ -38,8 +40,8 @@ public MockSession(@Nonnull String sessionId) {
   /**
    * Mock session with a random ID.
    */
-  public MockSession() {
-    this(UUID.randomUUID().toString());
+  public MockSession(@Nonnull MockContext ctx) {
+    this(ctx, UUID.randomUUID().toString());
   }
 
   @Nonnull @Override public String getId() {
@@ -48,7 +50,7 @@ public MockSession() {
 
   @Nonnull @Override public ValueNode get(@Nonnull String name) {
     return Optional.ofNullable(data.get(name))
-        .map(value -> Value.create(null, name, value))
+        .map(value -> Value.create(ctx, name, value))
         .orElse(Value.missing(name));
   }