Skip to content

Commit b02e65d

Browse files
mbknormbknor
committed
[playframework#1412] Modified 'Implemented resolving nearest matching locale.' to not break API + added tests
Also had to fix an existing test because it was unpredictable - It tests the same thing after the modification but in a predictable way
1 parent a6833b4 commit b02e65d

File tree

6 files changed

+187
-14
lines changed

6 files changed

+187
-14
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ eclipse
3030
*.iml
3131
*.ipr
3232
*.iws
33+
atlassian-ide-plugin.xml
3334

3435
# modules/: exclude except for core modules
3536
modules/*

‎framework/src/play/i18n/Lang.java

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,27 @@ public static void clear() {
6969
*/
7070
public static void change(String locale) {
7171
String closestLocale = findClosestMatch(Collections.singleton(locale));
72-
boolean success = set(closestLocale);
73-
//findClosestMatch should always return a valid locale, so success should always be true.
74-
Response.current().setCookie(Play.configuration.getProperty("application.lang.cookie", "PLAY_LANG"), locale);
72+
if ( closestLocale == null ) {
73+
// Give up
74+
return ;
75+
}
76+
if ( set(closestLocale) ) {
77+
Response response = Response.current();
78+
if ( response != null ) {
79+
// We have a current response in scope - set the language-cookie to store the selected language for the next requests
80+
response.setCookie(Play.configuration.getProperty("application.lang.cookie", "PLAY_LANG"), locale);
81+
}
82+
}
83+
7584
}
7685

7786
/**
7887
* Given a set of desired locales, searches the set of locales supported by this Play! application and returns the closest match.
7988
*
80-
* @param desiredLocales a collection of desired locales. If the collection is ordered, earlier locales are preferred over later ones. Locales should be of the form "[language]_[country" or "[language]", e.g. "en_CA" or "en". The locale strings are case insensitive (e.g. "EN_CA" is considered the same as "en_ca").
81-
* @return the closest matching locale. If no locales are defined in this Play! application, the empty string is returned.
89+
* @param desiredLocales a collection of desired locales. If the collection is ordered, earlier locales are preferred over later ones.
90+
* Locales should be of the form "[language]_[country" or "[language]", e.g. "en_CA" or "en".
91+
* The locale strings are case insensitive (e.g. "EN_CA" is considered the same as "en_ca").
92+
* @return the closest matching locale. If no closest match for a language/country is found, null is returned
8293
*/
8394
private static String findClosestMatch(Iterable<String> desiredLocales) {
8495
//look for an exact match
@@ -106,12 +117,9 @@ private static String findClosestMatch(Iterable<String> desiredLocales) {
106117
}
107118
}
108119
}
109-
//No matches found. Return default locale
110-
if (Play.langs.isEmpty()) {
111-
return "";
112-
} else {
113-
return Play.langs.get(0);
114-
}
120+
121+
// We did not find a anything
122+
return null;
115123
}
116124

117125
/**
@@ -140,7 +148,13 @@ private static void resolvefrom(Request request) {
140148

141149
}
142150
String closestLocaleMatch = findClosestMatch(request.acceptLanguage());
143-
set(closestLocaleMatch);
151+
if ( closestLocaleMatch != null ) {
152+
set(closestLocaleMatch);
153+
} else {
154+
// Did not find anything - use default
155+
setDefaultLocale();
156+
}
157+
144158
}
145159

146160
public static void setDefaultLocale() {

‎framework/test-src/play/i18n/LangTest.java

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package play.i18n;
2+
3+
import org.junit.Test;
4+
import play.Play;
5+
import play.PlayBuilder;
6+
import play.mvc.Http;
7+
import play.test.FunctionalTest;
8+
9+
import java.util.ArrayList;
10+
import java.util.Arrays;
11+
12+
import static org.fest.assertions.Assertions.*;
13+
14+
public class LangTest {
15+
16+
@Test
17+
public void testChange() {
18+
new PlayBuilder().build();
19+
Play.langs = Arrays.asList("no", "en", "fr");
20+
Lang.current.set(null);
21+
assertThat(Lang.current.get()).isNull();
22+
23+
Lang.change("no");
24+
assertThat(Lang.current.get()).isEqualTo("no");
25+
Lang.change("nox");
26+
assertThat(Lang.current.get()).isEqualTo("no");
27+
Lang.change("EN");
28+
assertThat(Lang.current.get()).isEqualTo("en");
29+
Lang.change("fr");
30+
assertThat(Lang.current.get()).isEqualTo("fr");
31+
32+
Lang.change("xx");
33+
assertThat(Lang.current.get()).isEqualTo("fr");
34+
35+
Lang.change("en_uk");
36+
assertThat(Lang.current.get()).isEqualTo("en");
37+
38+
Play.langs = Arrays.asList("no", "en", "en_uk", "fr");
39+
Lang.current.set(null);
40+
Lang.change("en_uk");
41+
assertThat(Lang.current.get()).isEqualTo("en_uk");
42+
Lang.change("en");
43+
assertThat(Lang.current.get()).isEqualTo("en");
44+
Lang.change("en_qw");
45+
assertThat(Lang.current.get()).isEqualTo("en");
46+
}
47+
48+
@Test
49+
public void testGet() {
50+
new PlayBuilder().build();
51+
Play.langs = Arrays.asList("no", "en", "en_uk", "fr");
52+
Lang.current.set(null);
53+
54+
Http.Response.current.set( new Http.Response());
55+
56+
// check default when missing request
57+
Http.Request.current.set(null);
58+
assertThat(Lang.get()).isEqualTo("no");
59+
60+
// check default when missing info in request
61+
Http.Request req = FunctionalTest.newRequest();
62+
Http.Request.current.set(req);
63+
Lang.current.set(null);
64+
assertThat(Lang.get()).isEqualTo("no");
65+
66+
// check only with accept-language, without cookie value
67+
req = FunctionalTest.newRequest();
68+
req.headers.put("accept-language", new Http.Header("accept-language", "x"));
69+
Http.Request.current.set(req);
70+
Lang.current.set(null);
71+
assertThat(Lang.get()).isEqualTo("no");
72+
73+
req = FunctionalTest.newRequest();
74+
req.headers.put("accept-language", new Http.Header("accept-language", "no"));
75+
Http.Request.current.set(req);
76+
Lang.current.set(null);
77+
assertThat(Lang.get()).isEqualTo("no");
78+
79+
req = FunctionalTest.newRequest();
80+
req.headers.put("accept-language", new Http.Header("accept-language", "en"));
81+
Http.Request.current.set(req);
82+
Lang.current.set(null);
83+
assertThat(Lang.get()).isEqualTo("en");
84+
85+
req = FunctionalTest.newRequest();
86+
req.headers.put("accept-language", new Http.Header("accept-language", "x,en"));
87+
Http.Request.current.set(req);
88+
Lang.current.set(null);
89+
assertThat(Lang.get()).isEqualTo("en");
90+
91+
req = FunctionalTest.newRequest();
92+
req.headers.put("accept-language", new Http.Header("accept-language", "en_uk"));
93+
Http.Request.current.set(req);
94+
Lang.current.set(null);
95+
assertThat(Lang.get()).isEqualTo("en_uk");
96+
97+
req = FunctionalTest.newRequest();
98+
req.headers.put("accept-language", new Http.Header("accept-language", "x,en_uk"));
99+
Http.Request.current.set(req);
100+
Lang.current.set(null);
101+
assertThat(Lang.get()).isEqualTo("en_uk");
102+
103+
// check with cookie value
104+
105+
req = FunctionalTest.newRequest();
106+
107+
Http.Cookie cookie = new Http.Cookie();
108+
cookie.name = "PLAY_LANG";
109+
cookie.value = "x";//not found in cookie
110+
req.cookies.put(cookie.name, cookie);
111+
req.headers.put("accept-language", new Http.Header("accept-language", "en"));
112+
Http.Request.current.set(req);
113+
Lang.current.set(null);
114+
assertThat(Lang.get()).isEqualTo("en");
115+
116+
cookie = new Http.Cookie();
117+
cookie.name = "PLAY_LANG";
118+
cookie.value = "en";
119+
req.cookies.put(cookie.name, cookie);
120+
Http.Request.current.set(req);
121+
Lang.current.set(null);
122+
assertThat(Lang.get()).isEqualTo("en");
123+
124+
cookie = new Http.Cookie();
125+
cookie.name = "PLAY_LANG";
126+
cookie.value = "en_q";
127+
req.cookies.put(cookie.name, cookie);
128+
Http.Request.current.set(req);
129+
Lang.current.set(null);
130+
assertThat(Lang.get()).isEqualTo("en");
131+
132+
cookie = new Http.Cookie();
133+
cookie.name = "PLAY_LANG";
134+
cookie.value = "en_uk";
135+
req.cookies.put(cookie.name, cookie);
136+
Http.Request.current.set(req);
137+
Lang.current.set(null);
138+
assertThat(Lang.get()).isEqualTo("en_uk");
139+
140+
141+
}
142+
}

‎samples-and-tests/just-test-cases/app/controllers/Application.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ public static void tagContexts() {
129129
render();
130130
}
131131

132+
133+
public static void generateBookWithDateLink(long timeLong) {
134+
render(timeLong);
135+
}
136+
132137
public static void book(Date at) {
133138
java.text.SimpleDateFormat df = new java.text.SimpleDateFormat("dd/MM/yy");
134139
df.setTimeZone(TimeZone.getTimeZone("UTC"));

‎samples-and-tests/just-test-cases/app/views/Application/generateBookWithDateLink.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<html>
2+
<body>
3+
<a id="timeLink" href="@{Application.book(new java.util.Date(timeLong))}">link</a>
4+
</body>
5+
</html>

‎samples-and-tests/just-test-cases/test/binding.test.html

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,14 @@
7575

7676
<!-- Now unbind dates -->
7777
open('/databinding/changeLanguage/en/')
78-
open('@{Application.book(new java.util.Date(2879877856556))}')
79-
assertTextPresent('Booked at 04/04/61 !!')
78+
<!-- Since it is unclear which locale is current when rendering this selenium page,
79+
we cannot rely on the auto-date-unbinding when generating the url with patams here.
80+
To test the same stuff´, we therefor request a page from the server (which has the correct lang, en)
81+
which generates the link we need, then we click it.
82+
We end up testing the exact same stuff, only under known locale conditions. -->
83+
open('@{Application.generateBookWithDateLink(2879877856556)}')
84+
clickAndWait('timeLink')
85+
assertTextPresent('Booked at 04/04/61 !!')
8086

8187
<!-- Binding internal enum with empty string -->
8288
open('/databinding/createFactory?factory.name=Nestle&factory.color=RED')

0 commit comments

Comments
 (0)