From 3b7d1aa8aa3f1d429c6498e487414ac02b3bae1e Mon Sep 17 00:00:00 2001 From: Markus Scherer Date: Wed, 2 Sep 2015 02:08:44 +0000 Subject: [PATCH] ICU-11879 change to child-first resource enumeration: avoid deserializing overridden resource values X-SVN-Rev: 37865 --- .../src/com/ibm/icu/impl/ICUResource.java | 8 +- .../com/ibm/icu/impl/ICUResourceBundle.java | 33 +++++---- .../ibm/icu/impl/ICUResourceBundleReader.java | 2 +- .../com/ibm/icu/impl/TimeZoneNamesImpl.java | 74 +++++++++++++------ .../dev/test/format/TimeZoneFormatTest.java | 4 +- 5 files changed, 77 insertions(+), 44 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResource.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResource.java index 248fa0bbc5..122d59ec81 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResource.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResource.java @@ -357,15 +357,15 @@ public final class ICUResource { public void put(Key key, Value value) {} /** - * Removes any value for this key. - * Typically used for CLDR no-fallback data values of "∅∅∅" - * when enumerating tables with fallback from root to the specific resource bundle. + * Adds a no-fallback/no-inheritance marker for this key. + * Used for CLDR no-fallback data values of "∅∅∅" + * when enumerating tables with fallback from the specific resource bundle to root. * *

The default implementation does nothing. * * @param key to be removed */ - public void remove(Key key) {} + public void putNoFallback(Key key) {} /** * Returns a nested resource array for the key as another sink. 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 49a18f87fd..a4b645ed3d 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 @@ -492,17 +492,27 @@ public class ICUResourceBundle extends UResourceBundle { private void getAllContainerItemsWithFallback( ICUResource.Key key, ReaderValue readerValue, ICUResource.ArraySink arraySink, ICUResource.TableSink tableSink) { - // We recursively enumerate parent-first, - // overriding parent items with child items. - // When we see the no-inheritance marker, then we remove the parent's item. - // - // It would be possible to recursively enumerate child-first, - // only storing parent items in the absence of child items, - // but then we would need to store the no-inheritance marker - // (or some placeholder for it) + // We recursively enumerate child-first, + // only storing parent items in the absence of child items. + // We store a placeholder value for the no-fallback/no-inheritance marker // to prevent a parent item from being stored. + // + // It would be possible to recursively enumerate parent-first, + // overriding parent items with child items. + // When we see the no-fallback/no-inheritance marker, + // then we would remove the parent's item. + // We would deserialize parent values even though they are overridden in a child bundle. int expectedType = arraySink != null ? ARRAY : TABLE; + if (getType() == expectedType) { + if (arraySink != null) { + ((ICUResourceBundleImpl.ResourceArray)this).getAllItems(key, readerValue, arraySink); + } else if (tableSink != null) { + ((ICUResourceBundleImpl.ResourceTable)this).getAllItems(key, readerValue, tableSink); + } + } if (parent != null) { + // We might try to query the sink whether + // any fallback from the parent bundle is still possible. ICUResourceBundle parentBundle = (ICUResourceBundle)parent; ICUResourceBundle rb; int depth = getResDepth(); @@ -519,13 +529,6 @@ public class ICUResourceBundle extends UResourceBundle { rb.getAllContainerItemsWithFallback(key, readerValue, arraySink, tableSink); } } - if (getType() == expectedType) { - if (arraySink != null) { - ((ICUResourceBundleImpl.ResourceArray)this).getAllItems(key, readerValue, arraySink); - } else if (tableSink != null) { - ((ICUResourceBundleImpl.ResourceTable)this).getAllItems(key, readerValue, tableSink); - } - } } /** diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleReader.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleReader.java index 1d93f9cc16..6e6e4a2069 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleReader.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/ICUResourceBundleReader.java @@ -1051,7 +1051,7 @@ public final class ICUResourceBundleReader { throw new UnsupportedOperationException( "aliases not handled in resource enumeration"); } else if (reader.isNoInheritanceMarker(res)) { - sink.remove(key); + sink.putNoFallback(key); } else { value.res = res; sink.put(key, value); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/TimeZoneNamesImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/TimeZoneNamesImpl.java index 8550ffc51e..3c1320a45e 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/TimeZoneNamesImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/TimeZoneNamesImpl.java @@ -298,7 +298,9 @@ public class TimeZoneNamesImpl extends TimeZoneNames { for (Map.Entry entry : keyToLoader.entrySet()) { ICUResource.Key key = entry.getKey(); ZNamesLoader loader = entry.getValue(); - if (key.startsWith(MZ_PREFIX)) { + if (loader == ZNamesLoader.DUMMY_LOADER) { + // skip + } else if (key.startsWith(MZ_PREFIX)) { String mzID = mzIDFromKey(key).intern(); ZNames mzNames = ZNames.getInstance(loader.getNames(), null); _mzNamesMap.put(mzID, mzNames); @@ -314,28 +316,38 @@ public class TimeZoneNamesImpl extends TimeZoneNames { public TableSink getOrCreateTableSink(ICUResource.Key key, int initialSize) { ZNamesLoader loader = keyToLoader.get(key); if (loader != null) { + if (loader == ZNamesLoader.DUMMY_LOADER) { + return null; + } return loader; } + ZNamesLoader result = null; if (key.startsWith(MZ_PREFIX)) { String mzID = mzIDFromKey(key); if (_mzNamesMap.containsKey(mzID)) { - return null; // We have already loaded the names for this meta zone. + // We have already loaded the names for this meta zone. + loader = ZNamesLoader.DUMMY_LOADER; + } else { + result = loader = ZNamesLoader.forMetaZoneNames(); } - loader = ZNamesLoader.forMetaZoneNames(); } else { String tzID = tzIDFromKey(key); if (_tzNamesMap.containsKey(tzID)) { - return null; // We have already loaded the names for this time zone. + // We have already loaded the names for this time zone. + loader = ZNamesLoader.DUMMY_LOADER; + } else { + result = loader = ZNamesLoader.forTimeZoneNames(); } - loader = ZNamesLoader.forTimeZoneNames(); } keyToLoader.put(key.clone(), loader); - return loader; + return result; } @Override - public void remove(ICUResource.Key key) { - keyToLoader.remove(key); + public void putNoFallback(ICUResource.Key key) { + if (!keyToLoader.containsKey(key)) { + keyToLoader.put(key.clone(), ZNamesLoader.DUMMY_LOADER); + } } /** @@ -552,6 +564,13 @@ public class TimeZoneNamesImpl extends TimeZoneNames { private static int NUM_META_ZONE_NAMES = 6; private static int NUM_TIME_ZONE_NAMES = 7; // incl. EXEMPLAR_LOCATION + private static String NO_NAME = ""; + + /** + * Does not load any names, for no-fallback handling. + */ + private static ZNamesLoader DUMMY_LOADER = new ZNamesLoader(0); + private String[] names; private int numNames; @@ -605,23 +624,24 @@ public class TimeZoneNamesImpl extends TimeZoneNames { @Override public void put(ICUResource.Key key, ICUResource.Value value) { if (value.getType() == UResourceBundle.STRING) { + if (names == null) { + names = new String[numNames]; + } NameType type = nameTypeFromKey(key); - if (type != null && type.ordinal() < numNames) { - if (names == null) { - names = new String[numNames]; - } + if (type != null && type.ordinal() < numNames && names[type.ordinal()] == null) { names[type.ordinal()] = value.getString(); } } } @Override - public void remove(ICUResource.Key key) { - if (names != null) { - NameType type = nameTypeFromKey(key); - if (type != null && type.ordinal() < numNames) { - names[type.ordinal()] = null; - } + public void putNoFallback(ICUResource.Key key) { + if (names == null) { + names = new String[numNames]; + } + NameType type = nameTypeFromKey(key); + if (type != null && type.ordinal() < numNames && names[type.ordinal()] == null) { + names[type.ordinal()] = NO_NAME; } } @@ -629,13 +649,21 @@ public class TimeZoneNamesImpl extends TimeZoneNames { if (names == null) { return null; } - int length = names.length; - while (names[length - 1] == null) { - if (--length == 0) { - return null; // no names + int length = 0; + for (int i = 0; i < numNames; ++i) { + String name = names[i]; + if (name != null) { + if (name == NO_NAME) { + names[i] = null; + } else { + length = i + 1; + } } } - if (length == names.length || numNames == NUM_TIME_ZONE_NAMES) { + if (length == 0) { + return null; + } + if (length == numNames || numNames == NUM_TIME_ZONE_NAMES) { // Return the full array if the last name is set. // Also return the full *time* zone names array, // so that the exemplar location can be set. diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeZoneFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeZoneFormatTest.java index 9f5d7aef17..77dada4fa1 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeZoneFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/TimeZoneFormatTest.java @@ -650,7 +650,9 @@ public class TimeZoneFormatTest extends com.ibm.icu.dev.test.TestFmwk { } if (errMsg != null) { - errln("Fail: " + errMsg + " [text=" + text + ", pos=" + inPos + ", style=" + style + "]"); + errln("Fail: " + errMsg + + " [text=" + text + ", pos=" + inPos + + ", locale=" + loc + ", style=" + style + "]"); } } }