From f0aa65c877a9bbe1edd629a5cc47f39a3b029650 Mon Sep 17 00:00:00 2001 From: Yoshito Umaoka Date: Mon, 4 Oct 2010 19:42:01 +0000 Subject: [PATCH] ICU-7996 Fixed resource bundle alias resolution problems. Also restored resource alias test cases accidentally disabled before. X-SVN-Rev: 28749 --- .../com/ibm/icu/impl/ICUResourceBundle.java | 68 ++++++++------ .../ibm/icu/impl/ICUResourceBundleImpl.java | 4 +- icu4j/main/shared/data/testdata.jar | 4 +- .../dev/test/util/ICUResourceBundleTest.java | 92 +++++++++++-------- 4 files changed, 97 insertions(+), 71 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundle.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundle.java index 74198b9c5d1..9c9b5c468e7 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundle.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundle.java @@ -1129,7 +1129,9 @@ public class ICUResourceBundle extends UResourceBundle { private static final char HYPHEN = '-'; private static final String LOCALE = "LOCALE"; - protected ICUResourceBundle findResource(String _key, int _resource, + protected ICUResourceBundle findResource(String key, + String resPath, + int _resource, HashMap table, UResourceBundle requested) { ClassLoader loaderToUse = loader; @@ -1150,6 +1152,9 @@ public class ICUResourceBundle extends UResourceBundle { bundleName = rpath.substring(1, i); if (j < 0) { locale = rpath.substring(i + 1); + // if key path is not available, + // use the given key path + keyPath = resPath; } else { locale = rpath.substring(i + 1, j); keyPath = rpath.substring(j + 1, rpath.length()); @@ -1168,12 +1173,14 @@ public class ICUResourceBundle extends UResourceBundle { } else { //no path start with locale int i = rpath.indexOf(RES_PATH_SEP_CHAR); - keyPath = rpath.substring(i + 1); if (i != -1) { locale = rpath.substring(0, i); + keyPath = rpath.substring(i + 1); } else { - locale = keyPath; - keyPath = null;//keyPath.substring(i, keyPath.length()); + locale = rpath; + // if key path is not available, + // use the given key path + keyPath = resPath; } bundleName = baseName; } @@ -1181,12 +1188,23 @@ public class ICUResourceBundle extends UResourceBundle { ICUResourceBundle sub = null; if(bundleName.equals(LOCALE)){ bundleName = baseName; - bundle = (ICUResourceBundle)requested; keyPath = rpath.substring(LOCALE.length() + 2/* prepending and appending / */, rpath.length()); locale = ((ICUResourceBundle)requested).getLocaleID(); - sub = ICUResourceBundle.findResourceWithFallback(keyPath, requested, null); - if (sub != null) { - sub.resPath = "/" + sub.getLocaleID() + "/" + keyPath; + + // Get the top bundle of the requested bundle + bundle = (ICUResourceBundle)getBundleInstance(bundleName, locale, loaderToUse, false); + if (bundle != null) { + sub = ICUResourceBundle.findResourceWithFallback(keyPath, bundle, null); + // TODO + // The resPath of the resolved bundle should reflect the resource path + // requested by caller. However, overwriting resPath here will affect cached + // resource instance. The resPath is exposed by ICUResourceBundle#getResPath, + // but there are no call sites in ICU (and ICUResourceBundle is an implementation + // class). We may create a safe clone to overwrite the resPath field, but + // it has no benefit at least for now. -Yoshito + //if (sub != null) { + // sub.resPath = resPath; + //} } }else{ if (locale == null) { @@ -1197,29 +1215,25 @@ public class ICUResourceBundle extends UResourceBundle { bundle = (ICUResourceBundle) getBundleInstance(bundleName, locale, loaderToUse, false); } - if (keyPath != null) { - StringTokenizer st = new StringTokenizer(keyPath, "/"); - ICUResourceBundle current = bundle; - while (st.hasMoreTokens()) { - String subKey = st.nextToken(); - sub = (ICUResourceBundle)current.get(subKey, table, requested); - if (sub == null) { - break; - } - current = sub; + + StringTokenizer st = new StringTokenizer(keyPath, "/"); + ICUResourceBundle current = bundle; + while (st.hasMoreTokens()) { + String subKey = st.nextToken(); + sub = (ICUResourceBundle)current.get(subKey, table, requested); + if (sub == null) { + break; } - } else { - // if the sub resource is not found - // try fetching the sub resource with - // the key of this alias resource - sub = (ICUResourceBundle)bundle.get(_key); - } - if (sub != null) { - sub.resPath = rpath; + current = sub; } + // TODO + // See the comments above. + //if (sub != null) { + // sub.resPath = resPath; + //} } if (sub == null) { - throw new MissingResourceException(localeID, baseName, _key); + throw new MissingResourceException(localeID, baseName, key); } return sub; } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleImpl.java index fe40d438780..5d2ed741454 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleImpl.java @@ -1,6 +1,6 @@ /* ******************************************************************************* - * Copyright (C) 2004-2009, International Business Machines Corporation and * + * Copyright (C) 2004-2010, International Business Machines Corporation and * * others. All Rights Reserved. * ******************************************************************************* */ @@ -39,7 +39,7 @@ class ICUResourceBundleImpl extends ICUResourceBundle { if (isAlias != null) { isAlias[0] = true; } - return findResource(_key, _resource, table, requested); + return findResource(_key, _resPath, _resource, table, requested); case INT: return new ICUResourceBundleImpl.ResourceInt(reader, _key, _resPath, _resource, this); case INT_VECTOR: diff --git a/icu4j/main/shared/data/testdata.jar b/icu4j/main/shared/data/testdata.jar index 3becdcafbed..84c9550a436 100755 --- a/icu4j/main/shared/data/testdata.jar +++ b/icu4j/main/shared/data/testdata.jar @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bb0fa63c44ccf590eb50381805731bedd26e7251cf1c2bc98ea97004a76f76eb -size 717903 +oid sha256:2ffe39028595ba7f73253cd8ade3c4bfe34bac854cf1a3cda26a8b6b3dd3e2b6 +size 718100 diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/ICUResourceBundleTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/ICUResourceBundleTest.java index 3abf829428c..16304518038 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/ICUResourceBundleTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/ICUResourceBundleTest.java @@ -439,7 +439,6 @@ public final class ICUResourceBundleTest extends TestFmwk { } public void TestAliases(){ -/* String simpleAlias = "Open"; UResourceBundle rb = (UResourceBundle)UResourceBundle.getBundleInstance("com/ibm/icu/dev/data/testdata","testaliases", testLoader); @@ -549,34 +548,48 @@ public final class ICUResourceBundleTest extends TestFmwk { errln("Did not get the expected output for testGetStringByIndexAliasing/3. Got: "+s1); } } + +// Note: Following test cases are no longer working because collation data is now in the collation module +// { +// sub = rb.get("testAliasToTree" ); +// +// ByteBuffer buf = sub.get("standard").get("%%CollationBin").getBinary(); +// if(buf==null){ +// errln("Did not get the expected output for %%CollationBin"); +// } +// } +// +// rb = (UResourceBundle) UResourceBundle.getBundleInstance(ICUResourceBundle.ICU_COLLATION_BASE_NAME,"zh_TW"); +// UResourceBundle b = (UResourceBundle) rb.getObject("collations"); +// if(b != null){ +// if(b.get(0).getKey().equals( "default")){ +// logln("Alias mechanism works"); +// }else{ +// errln("Alias mechanism failed for zh_TW collations"); +// } +// }else{ +// errln("Did not get the expected object for collations"); +// } + + // Test case for #7996 { - sub = rb.get("testAliasToTree" ); - - ByteBuffer buf = sub.get("standard").get("%%CollationBin").getBinary(); - if(buf==null){ - errln("Did not get the expected output for %%CollationBin"); + UResourceBundle bundle = UResourceBundle.getBundleInstance("com/ibm/icu/dev/data/testdata", "te"); + UResourceBundle table = bundle.get("tableT7996"); + try { + String s = table.getString("a7996"); + logln("Alias in nested table referring one in sh worked - " + s); + } catch (MissingResourceException e) { + errln("Alias in nested table referring one in sh failed"); + } + + try { + String s = ((ICUResourceBundle)table).getStringWithFallback("b7996"); + logln("Alias with /LOCALE/ in nested table in root referring back to another key in the current locale bundle worked - " + s); + } catch (MissingResourceException e) { + errln("Alias with /LOCALE/ in nested table in root referring back to another key in the current locale bundle failed"); } } - // should not get an exception - rb = (UResourceBundle) UResourceBundle.getBundleInstance(ICUResourceBundle.ICU_RBNF_BASE_NAME,"fr_BE"); - String str = rb.getString("SpelloutRules"); - if(str !=null && str.length()>0){ - logln("Alias mechanism works"); - }else{ - errln("Alias mechanism failed for fr_BE SpelloutRules"); - } - rb = (UResourceBundle) UResourceBundle.getBundleInstance(ICUResourceBundle.ICU_COLLATION_BASE_NAME,"zh_TW"); - UResourceBundle b = (UResourceBundle) rb.getObject("collations"); - if(b != null){ - if(b.get(0).getKey().equals( "default")){ - logln("Alias mechanism works"); - }else{ - errln("Alias mechanism failed for zh_TW collations"); - } - }else{ - errln("Did not get the expected object for collations"); - } -*/ + } public void TestAlias(){ logln("Testing %%ALIAS"); @@ -613,20 +626,19 @@ public final class ICUResourceBundleTest extends TestFmwk { } } public void TestCircularAliases(){ -// Aliases no longer supported -// try{ -// UResourceBundle rb = (UResourceBundle)UResourceBundle.getBundleInstance("com/ibm/icu/dev/data/testdata","testaliases",testLoader); -// UResourceBundle sub = rb.get("aaa"); -// String s1 = sub.getString(); -// if(s1!=null){ -// errln("Did not get the expected exception"); -// } -// }catch(IllegalArgumentException ex){ -// logln("got expected exception for circular references"); -// } -// catch (MissingResourceException ex) { -// warnln("could not load resource data: " + ex.getMessage()); -// } + try{ + UResourceBundle rb = (UResourceBundle)UResourceBundle.getBundleInstance("com/ibm/icu/dev/data/testdata","testaliases",testLoader); + UResourceBundle sub = rb.get("aaa"); + String s1 = sub.getString(); + if(s1!=null){ + errln("Did not get the expected exception"); + } + }catch(IllegalArgumentException ex){ + logln("got expected exception for circular references"); + } + catch (MissingResourceException ex) { + warnln("could not load resource data: " + ex.getMessage()); + } } public void TestGetWithFallback(){