You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by et...@apache.org on 2009/08/07 18:16:52 UTC

svn commit: r802065 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/ main/java/org/apache/shindig/gadgets/spec/ test/java/org/apache/shindig/gadgets/ test/java/org/apache/shindig/gadgets/spec/

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'>" +