Author: etnu
Date: Fri Aug 7 16:16:50 2009
New Revision: 802065
URL: http://svn.apache.org/viewvc?rev=802065&view=rev
Log:
Fixed edge cases that were causing one additional bundle fetch plus improper
merging on country-only matches.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/ModulePrefsTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java?rev=802065&r1=802064&r2=802065&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
Fri Aug 7 16:16:50 2009
@@ -83,7 +83,8 @@
country = getBundleFor(spec, new Locale("all", locale.getCountry()),
ignoreCache);
}
- if (isAllCountry && isAllLanguage) {
+ if (isAllCountry || isAllLanguage) {
+ // If either of these is true, we already picked up both anyway.
all = MessageBundle.EMPTY;
} else {
all = getBundleFor(spec, ALL_ALL, ignoreCache);
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java?rev=802065&r1=802064&r2=802065&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
Fri Aug 7 16:16:50 2009
@@ -458,7 +458,7 @@
public OAuthSpec getOAuthSpec() {
return oauth;
}
-
+
/**
* Not part of the spec. Indicates whether UserPref-substitutable
* fields in this prefs require __UP_ substitution.
@@ -468,28 +468,12 @@
}
/**
- * Attempts to retrieve a valid LocaleSpec for the given Locale.
- * First tries to find an exact language / country match.
- * Then tries to find a match for language / all.
- * Then tries to find a match for all / all.
- * Finally gives up.
- * @param locale
+ * Gets the locale spec for the given locale, if any exists.
+ *
* @return The locale spec, if there is a matching one, or null.
*/
public LocaleSpec getLocale(Locale locale) {
- if (locales.isEmpty()) {
- return null;
- }
- LocaleSpec localeSpec = locales.get(locale);
- if (localeSpec == null) {
- locale = new Locale(locale.getLanguage(), "ALL");
- localeSpec = locales.get(locale);
- if (localeSpec == null) {
- localeSpec = locales.get(GadgetSpec.DEFAULT_LOCALE);
- }
- }
-
- return localeSpec;
+ return locales.get(locale);
}
/**
@@ -546,7 +530,7 @@
buf.append("</ModulePrefs>");
return buf.toString();
}
-
+
/**
* @param prefs ModulePrefs object
* @return true if any UserPref-substitutable fields in the given
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java?rev=802065&r1=802064&r2=802065&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java
Fri Aug 7 16:16:50 2009
@@ -49,6 +49,9 @@
*/
public class DefaultMessageBundleFactoryTest {
private static final Uri BUNDLE_URI =
Uri.parse("http://example.org/messagex.xml";);
+ private static final Uri LANG_BUNDLE_URI =
Uri.parse("http://example.org/messagex.xml";);
+ private static final Uri COUNTRY_BUNDLE_URI =
Uri.parse("http://example.org/messagex.xml";);
+ private static final Uri ALL_BUNDLE_URI =
Uri.parse("http://example.org/messagex.xml";);
private static final Uri SPEC_URI =
Uri.parse("http://example.org/gadget.xml";);
private static final String MSG_0_NAME = "messageZero";
@@ -73,13 +76,31 @@
" <msg name='" + MSG_1_NAME + "'>" + MSG_1_VALUE + "</msg>" +
"</messagebundle>";
+ private static final String LANG_BUNDLE
+ = "<messagebundle>" +
+ " <msg name='" + MSG_0_NAME + "'>" + MSG_0_LANG_VALUE + "</msg>" +
+ " <msg name='lang'>true</msg>" +
+ "</messagebundle>";
+
+ private static final String COUNTRY_BUNDLE
+ = "<messagebundle>" +
+ " <msg name='" + MSG_0_NAME + "'>" + MSG_0_COUNTRY_VALUE + "</msg>" +
+ " <msg name='country'>true</msg>" +
+ "</messagebundle>";
+
+ private static final String ALL_ALL_BUNDLE
+ = "<messagebundle>" +
+ " <msg name='" + MSG_0_NAME + "'>" + MSG_0_ALL_VALUE + "</msg>" +
+ " <msg name='all'>true</msg>" +
+ "</messagebundle>";
+
private static final String BASIC_SPEC
= "<Module>" +
"<ModulePrefs title='foo'>" +
- " <Locale lang='all' country='ALL'>" +
+ " <Locale>" +
" <msg name='" + MSG_0_NAME + "'>" + MSG_0_ALL_VALUE + "</msg>" +
" </Locale>" +
- " <Locale lang='all' country='" + LOCALE.getCountry() + "'>" +
+ " <Locale country='" + LOCALE.getCountry() + "'>" +
" <msg name='" + MSG_0_NAME + "'>" + MSG_0_COUNTRY_VALUE + "</msg>" +
" <msg name='" + MSG_3_NAME + "'>" + MSG_3_VALUE + "</msg>" +
" </Locale>" +
@@ -97,12 +118,12 @@
private static final String ALL_EXTERNAL_SPEC
= "<Module>" +
"<ModulePrefs title='foo'>" +
- " <Locale lang='all' country='ALL' messages='" + BUNDLE_URI + "'/>" +
- " <Locale lang='all' country='" + LOCALE.getCountry() + "'" +
- " messages='" + BUNDLE_URI + "'/>" +
- " <Locale lang='" + LOCALE.getLanguage() + "' messages='" + BUNDLE_URI
+ "'/>" +
+ " <Locale messages='" + BUNDLE_URI + "'/>" +
+ " <Locale country='" + LOCALE.getCountry() + "'" +
+ " messages='" + COUNTRY_BUNDLE_URI + "'/>" +
+ " <Locale lang='" + LOCALE.getLanguage() + "' messages='" +
LANG_BUNDLE_URI + "'/>" +
" <Locale lang='" + LOCALE.getLanguage() + "' country='" +
LOCALE.getCountry() + "' " +
- " messages='" + BUNDLE_URI + "'/>" +
+ " messages='" + ALL_BUNDLE_URI + "'/>" +
"</ModulePrefs>" +
"<Content type='html'/>" +
"</Module>";
@@ -173,41 +194,67 @@
@Test
public void getExactBundleAllExternal() throws Exception {
HttpResponse response = new HttpResponse(BASIC_BUNDLE);
-
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).times(4);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response);
+ HttpResponse langResponse = new HttpResponse(LANG_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(langResponse);
+ HttpResponse countryResponse = new HttpResponse(COUNTRY_BUNDLE);
+
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(countryResponse);
+ HttpResponse allAllResponse = new HttpResponse(ALL_ALL_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(allAllResponse);
replay(pipeline);
- bundleFactory.getBundle(externalSpec, LOCALE, true);
+ MessageBundle bundle = bundleFactory.getBundle(externalSpec, LOCALE, true);
verify(pipeline);
+
+ assertEquals("true", bundle.getMessages().get("lang"));
+ assertEquals("true", bundle.getMessages().get("country"));
+ assertEquals("true", bundle.getMessages().get("all"));
+ assertEquals(MSG_0_VALUE, bundle.getMessages().get(MSG_0_NAME));
}
@Test
public void getLangBundleAllExternal() throws Exception {
- HttpResponse response = new HttpResponse(BASIC_BUNDLE);
-
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).times(3);
+ HttpResponse langResponse = new HttpResponse(LANG_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(langResponse);
+ HttpResponse allAllResponse = new HttpResponse(ALL_ALL_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(allAllResponse);
replay(pipeline);
- bundleFactory.getBundle(externalSpec, LANG_LOCALE, true);
+ MessageBundle bundle = bundleFactory.getBundle(externalSpec, LANG_LOCALE,
true);
verify(pipeline);
+
+ assertEquals("true", bundle.getMessages().get("lang"));
+ assertEquals("true", bundle.getMessages().get("all"));
+ assertEquals(MSG_0_LANG_VALUE, bundle.getMessages().get(MSG_0_NAME));
}
@Test
public void getCountryBundleAllExternal() throws Exception {
- HttpResponse response = new HttpResponse(BASIC_BUNDLE);
-
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).times(3);
+ HttpResponse countryResponse = new HttpResponse(COUNTRY_BUNDLE);
+
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(countryResponse);
+ HttpResponse allAllResponse = new HttpResponse(ALL_ALL_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(allAllResponse);
replay(pipeline);
- bundleFactory.getBundle(externalSpec, COUNTRY_LOCALE, true);
+ MessageBundle bundle = bundleFactory.getBundle(externalSpec,
COUNTRY_LOCALE, true);
verify(pipeline);
+
+ assertEquals("true", bundle.getMessages().get("country"));
+ assertEquals("true", bundle.getMessages().get("all"));
+ assertEquals(MSG_0_COUNTRY_VALUE, bundle.getMessages().get(MSG_0_NAME));
}
@Test
public void getAllAllExternal() throws Exception {
- HttpResponse response = new HttpResponse(BASIC_BUNDLE);
-
expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).once();
+ HttpResponse allAllResponse = new HttpResponse(ALL_ALL_BUNDLE);
+ expect(pipeline.execute(isA(HttpRequest.class))).andReturn(allAllResponse);
replay(pipeline);
- bundleFactory.getBundle(externalSpec, new Locale("all", "ALL"), true);
+ MessageBundle bundle = bundleFactory.getBundle(externalSpec, new
Locale("all", "ALL"), true);
verify(pipeline);
+
+ assertEquals("true", bundle.getMessages().get("all"));
+ assertEquals(MSG_0_ALL_VALUE, bundle.getMessages().get(MSG_0_NAME));
}
@Test
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/ModulePrefsTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/ModulePrefsTest.java?rev=802065&r1=802064&r2=802065&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/ModulePrefsTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/spec/ModulePrefsTest.java
Fri Aug 7 16:16:50 2009
@@ -139,15 +139,18 @@
@Test
public void getLocale() throws Exception {
String xml = "<ModulePrefs title='locales'>" +
- " <Locale lang='en' messages='en.xml'/>" +
+ " <Locale lang='en' country='UK' messages='en.xml'/>" +
" <Locale lang='foo' language_direction='rtl'/>" +
"</ModulePrefs>";
ModulePrefs prefs = new ModulePrefs(XmlUtil.parse(xml), SPEC_URL);
- LocaleSpec spec = prefs.getLocale(new Locale("en", "uk"));
+ LocaleSpec spec = prefs.getLocale(new Locale("en", "UK"));
assertEquals("http://example.org/en.xml";, spec.getMessages().toString());
- spec = prefs.getLocale(new Locale("foo", "bar"));
+ spec = prefs.getLocale(new Locale("foo", "ALL"));
assertEquals("rtl", spec.getLanguageDirection());
+
+ spec = prefs.getLocale(new Locale("en", "notexist"));
+ assertNull(spec);
}
@Test
@@ -219,21 +222,21 @@
ModulePrefs prefs = new ModulePrefs(XmlUtil.parse(FULL_XML), SPEC_URL);
doAsserts(new ModulePrefs(XmlUtil.parse(prefs.toString()), SPEC_URL));
}
-
+
@Test
public void needsUserPrefSubstInTitle() throws Exception {
String xml = "<ModulePrefs title='Title __UP_foo__'/>";
ModulePrefs prefs = new ModulePrefs(XmlUtil.parse(xml), SPEC_URL);
assertTrue(prefs.needsUserPrefSubstitution());
}
-
+
@Test
public void needsUserPrefSubstInTitleUrl() throws Exception {
String xml = "<ModulePrefs title='foo' title_url='http://__UP_url__'/>";
ModulePrefs prefs = new ModulePrefs(XmlUtil.parse(xml), SPEC_URL);
assertTrue(prefs.needsUserPrefSubstitution());
}
-
+
@Test
public void needsUserPrefSubstInPreload() throws Exception {
String xml = "<ModulePrefs title='foo'>" +
