Skip to content

Commit f59b177

Browse files
jknackjknack
committed
Flash scope improvements
* flash scope get lost between (sub)paths fix jooby-project#468 * allow to customize flash cookie fix jooby-project#470
1 parent b5addee commit f59b177

11 files changed

Lines changed: 186 additions & 22 deletions

File tree

‎coverage-report/src/test/java/apps/flashscope/FlashScopeApp.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ public class FlashScopeApp extends Jooby {
2222

2323
get("/", () -> Results.html("flash"));
2424

25-
post("/", req -> {
26-
req.flash("success", req.param("message").value());
25+
get("/send", req -> {
26+
req.flash("success", req.param("message").value("It works"));
2727
return Results.redirect("/");
2828
});
2929

‎coverage-report/src/test/java/org/jooby/issues/Issue397.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ public void shouldCreateAndDestroyFlashCookie() throws Exception {
4949
.post("/397/flash")
5050
.expect(302)
5151
.header("Set-Cookie", setCookie -> {
52-
assertEquals("flash=success=Thank+you%21;Version=1", setCookie);
52+
assertEquals("jooby.flash=success=Thank+you%21;Version=1;Path=/;HttpOnly", setCookie);
5353
request()
5454
.get("/397/flash")
5555
.header("Cookie", setCookie)
5656
.expect("{success=Thank you!}")
5757
.header("Set-Cookie", clearCookie -> {
58-
assertTrue(clearCookie.startsWith("flash=;Version=1;Max-Age=0;"));
58+
assertTrue(clearCookie.startsWith("jooby.flash=;Version=1;Path=/;HttpOnly;Max-Age=0;"));
5959
});
6060
});
6161
}
@@ -76,19 +76,19 @@ public void shouldRecreateCookieOnReset() throws Exception {
7676
.post("/397/flash")
7777
.expect(302)
7878
.header("Set-Cookie", setCookie1 -> {
79-
assertEquals("flash=success=Thank+you%21;Version=1", setCookie1);
79+
assertEquals("jooby.flash=success=Thank+you%21;Version=1;Path=/;HttpOnly", setCookie1);
8080
request()
8181
.post("/397/reset")
8282
.header("Cookie", setCookie1)
8383
.expect(302)
8484
.header("Set-Cookie", setCookie2 -> {
85-
assertEquals("flash=success=Thank+you%21&foo=bar;Version=1", setCookie2);
85+
assertEquals("jooby.flash=success=Thank+you%21&foo=bar;Version=1;Path=/;HttpOnly", setCookie2);
8686
request()
8787
.get("/397/flash")
8888
.header("Cookie", setCookie2)
8989
.expect("{success=Thank you!, foo=bar}")
9090
.header("Set-Cookie", clearCookie -> {
91-
assertTrue(clearCookie.startsWith("flash=;Version=1;Max-Age=0;"));
91+
assertTrue(clearCookie.startsWith("jooby.flash=;Version=1;Path=/;HttpOnly;Max-Age=0;"));
9292
});
9393
});
9494
});
@@ -98,10 +98,10 @@ public void shouldRecreateCookieOnReset() throws Exception {
9898
public void shouldClearFlashCookieWhenEmpty() throws Exception {
9999
request()
100100
.get("/397/discard")
101-
.header("Cookie", "flash=success=OK;Version=1")
101+
.header("Cookie", "jooby.flash=success=OK;Version=1")
102102
.expect(200)
103103
.header("Set-Cookie", setCookie -> {
104-
assertTrue(setCookie.startsWith("flash=;Version=1;Max-Age=0;"));
104+
assertTrue(setCookie.startsWith("jooby.flash=;Version=1;Path=/;HttpOnly;Max-Age=0;"));
105105
});
106106
}
107107
}

‎coverage-report/src/test/java/org/jooby/issues/Issue397b.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ public void flashScopeOnMvc() throws Exception {
5555
.get("/397/flash")
5656
.expect("{foo=bar}")
5757
.header("Set-Cookie", setCookie -> {
58-
assertEquals("flash=foo=bar;Version=1", setCookie);
58+
assertEquals("jooby.flash=foo=bar;Version=1;Path=/;HttpOnly", setCookie);
5959
request()
6060
.get("/397/flash/attr")
6161
.expect("bar")
6262
.header("Set-Cookie", clearCookie -> {
63-
assertTrue(clearCookie.startsWith("flash=;Version=1;Max-Age=0;"));
63+
assertTrue(clearCookie.startsWith("jooby.flash=;Version=1;Path=/;HttpOnly;Max-Age=0;"));
6464
});
6565
});
6666
}

‎coverage-report/src/test/java/org/jooby/issues/Issue468.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.FlashScope;
4+
import org.jooby.Results;
5+
import org.jooby.test.ServerFeature;
6+
import org.junit.Test;
7+
8+
import com.typesafe.config.ConfigFactory;
9+
import com.typesafe.config.ConfigValueFactory;
10+
11+
public class Issue468 extends ServerFeature {
12+
13+
{
14+
use(ConfigFactory.empty()
15+
.withValue("application.path", ConfigValueFactory.fromAnyRef("/468")));
16+
17+
use(new FlashScope());
18+
19+
get("/", req -> req.flash().get("foo"));
20+
21+
get("/redirect", req -> {
22+
req.flash("foo", "bar");
23+
return Results.redirect(req.contextPath() + "/");
24+
});
25+
}
26+
27+
@Test
28+
public void flashAttributeIsPresentBetweenDiffPaths() throws Exception {
29+
request()
30+
.dontFollowRedirect()
31+
.get("/468/redirect")
32+
.execute()
33+
.header("Set-Cookie", "jooby.flash=foo=bar;Version=1;Path=/468;HttpOnly");
34+
}
35+
36+
@Test
37+
public void flashAttributeIsPresentBetweenDiffPathsOnRedirect() throws Exception {
38+
request()
39+
.get("/468/redirect")
40+
.expect("bar");
41+
}
42+
43+
}

‎coverage-report/src/test/java/org/jooby/issues/Issue468a.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.Cookie;
4+
import org.jooby.FlashScope;
5+
import org.jooby.Results;
6+
import org.jooby.test.ServerFeature;
7+
import org.junit.Test;
8+
9+
public class Issue468a extends ServerFeature {
10+
11+
{
12+
use(new FlashScope(new Cookie.Definition("x").httpOnly(false)));
13+
14+
get("/468", req -> {
15+
req.flash("foo", "bar");
16+
return Results.redirect(req.contextPath() + "/");
17+
});
18+
}
19+
20+
@Test
21+
public void flashAttributeIsPresentBetweenDiffPaths() throws Exception {
22+
request()
23+
.dontFollowRedirect()
24+
.get("/468")
25+
.execute()
26+
.header("Set-Cookie", "x=foo=bar;Version=1;Path=/");
27+
}
28+
29+
}

‎coverage-report/src/test/java/org/jooby/issues/Issue468b.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.FlashScope;
4+
import org.jooby.Results;
5+
import org.jooby.test.ServerFeature;
6+
import org.junit.Test;
7+
8+
public class Issue468b extends ServerFeature {
9+
10+
{
11+
use(new FlashScope());
12+
13+
get("/468", req -> req.flash().get("foo"));
14+
15+
get("/468/redirect", req -> {
16+
req.flash("foo", "bar");
17+
return Results.redirect("/468");
18+
});
19+
}
20+
21+
@Test
22+
public void flashAttributeIsPresentBetweenDiffPaths() throws Exception {
23+
request()
24+
.dontFollowRedirect()
25+
.get("/468/redirect")
26+
.execute()
27+
.header("Set-Cookie", "jooby.flash=foo=bar;Version=1;Path=/;HttpOnly");
28+
}
29+
30+
@Test
31+
public void flashAttributeIsPresentBetweenDiffPathsOnRedirect() throws Exception {
32+
request()
33+
.get("/468/redirect")
34+
.expect("bar");
35+
}
36+
37+
}

‎coverage-report/src/test/resources/apps/flashscope/flash.html‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
{{flash.success}}!!
66
{{else}}
77
Welcome
8-
<form action="/" method="post">
9-
<input name="message">
10-
</form>
8+
<a href="/send">Redirect me</a>
119
{{/if}}
1210
</body>
1311
</html>

‎jooby/src/main/java/org/jooby/Cookie.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,15 @@ public Definition(final String name, final String value) {
182182
value(value);
183183
}
184184

185+
/**
186+
* Creates a new {@link Definition cookie's definition}.
187+
*
188+
* @param name Cookie's name.
189+
*/
190+
public Definition(final String name) {
191+
name(name);
192+
}
193+
185194
/**
186195
* Produces a cookie from current definition.
187196
*

‎jooby/src/main/java/org/jooby/FlashScope.java‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
*/
1919
package org.jooby;
2020

21+
import static java.util.Objects.requireNonNull;
22+
2123
import java.util.Map;
24+
import java.util.Optional;
2225
import java.util.function.Function;
2326

2427
import org.jooby.internal.handlers.FlashScopeHandler;
@@ -101,14 +104,41 @@ public class FlashScope implements Jooby.Module {
101104

102105
private Function<Map<String, String>, String> encoder = Cookie.URL_ENCODER;
103106

104-
private String cookie = "flash";
107+
private Optional<Cookie.Definition> cookie = Optional.empty();
105108

106109
private String method = "*";
107110

108111
private String path = "*";
109112

113+
/**
114+
* Creates a new {@link FlashScope} and customize the flash cookie.
115+
*
116+
* @param cookie Cookie template for flash scope.
117+
*/
118+
public FlashScope(final Cookie.Definition cookie) {
119+
this.cookie = Optional.of(requireNonNull(cookie, "Cookie required."));
120+
}
121+
122+
/**
123+
* Creates a new {@link FlashScope}.
124+
*/
125+
public FlashScope() {
126+
}
127+
110128
@Override
111129
public void configure(final Env env, final Config conf, final Binder binder) {
130+
Config $cookie = conf.getConfig("flash.cookie");
131+
String cpath = $cookie.getString("path");
132+
boolean chttp = $cookie.getBoolean("httpOnly");
133+
boolean csecure = $cookie.getBoolean("secure");
134+
Cookie.Definition cookie = this.cookie
135+
.orElseGet(() -> new Cookie.Definition($cookie.getString("name")));
136+
137+
// uses user provided or fallback to defaults
138+
cookie.path(cookie.path().orElse(cpath))
139+
.httpOnly(cookie.httpOnly().orElse(chttp))
140+
.secure(cookie.secure().orElse(csecure));
141+
112142
env.routes().use(method, path, new FlashScopeHandler(cookie, decoder, encoder))
113143
.name("flash-scope");
114144
}

‎jooby/src/main/java/org/jooby/internal/handlers/FlashScopeHandler.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,26 @@
3131

3232
public class FlashScopeHandler implements Route.Filter {
3333

34-
private String name;
34+
private Cookie.Definition template;
35+
36+
private String cname;
3537

3638
private Function<String, Map<String, String>> decoder;
3739

3840
private Function<Map<String, String>, String> encoder;
3941

40-
public FlashScopeHandler(final String cname, final Function<String, Map<String, String>> decoder,
42+
public FlashScopeHandler(final Cookie.Definition cookie, final Function<String, Map<String, String>> decoder,
4143
final Function<Map<String, String>, String> encoder) {
42-
this.name = cname;
44+
this.template = cookie;
45+
this.cname = cookie.name().get();
4346
this.decoder = decoder;
4447
this.encoder = encoder;
4548
}
4649

4750
@Override
4851
public void handle(final Request req, final Response rsp, final Route.Chain chain)
4952
throws Throwable {
50-
Optional<String> value = req.cookie(name).toOptional();
53+
Optional<String> value = req.cookie(cname).toOptional();
5154
Map<String, String> flashScope = value.map(decoder::apply)
5255
.orElseGet(HashMap::new);
5356
Map<String, String> copy = new HashMap<>(flashScope);
@@ -67,16 +70,16 @@ private Route.After finalizeFlash(final Map<String, String> initialScope,
6770
if (scope.equals(initialScope)) {
6871
// 1.a. existing data available, discard
6972
if (scope.size() > 0) {
70-
rsp.cookie(new Cookie.Definition(name, "").maxAge(0));
73+
rsp.cookie(new Cookie.Definition(template).maxAge(0));
7174
}
7275
} else {
7376
// 2. change detected
7477
if (scope.size() == 0) {
7578
// 2.a everything was removed from app logic
76-
rsp.cookie(new Cookie.Definition(name, "").maxAge(0));
79+
rsp.cookie(new Cookie.Definition(template).maxAge(0));
7780
} else {
7881
// 2.b there is something to see in the next request
79-
rsp.cookie(name, encoder.apply(scope));
82+
rsp.cookie(new Cookie.Definition(template).value(encoder.apply(scope)));
8083
}
8184
}
8285
return result;

0 commit comments

Comments
 (0)