From 9c8165a44db669f3093960692d7ae0f55f1e6115 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Sat, 30 Sep 2017 00:21:07 +0000 Subject: [PATCH] ICU-13368 Overhauling currency data loading to resolve incorrect fallback bug. Adding back a test for the current number parser for behavior testing of the currency data loading change. X-SVN-Rev: 40519 --- .../src/com/ibm/icu/impl/CurrencyData.java | 13 +- .../ibm/icu/text/CurrencyDisplayNames.java | 24 +- .../ibm/icu/text/DecimalFormatSymbols.java | 2 +- .../impl/ICUCurrencyDisplayInfoProvider.java | 500 +++++++++++------- .../data/numberformattestspecification.txt | 57 +- .../test/format/CompactDecimalFormatTest.java | 1 + .../DataDrivenNumberFormatTestUtility.java | 5 +- .../format/NumberFormatDataDrivenTest.java | 179 ++++++- .../icu/dev/test/format/NumberFormatTest.java | 6 +- 9 files changed, 550 insertions(+), 237 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/CurrencyData.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/CurrencyData.java index 09fbdd9e52..d7587c583e 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/CurrencyData.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/CurrencyData.java @@ -29,17 +29,18 @@ public class CurrencyData { public abstract Map getUnitPatterns(); public abstract CurrencyFormatInfo getFormatInfo(String isoCode); public abstract CurrencySpacingInfo getSpacingInfo(); + public abstract String getNarrowSymbol(String isoCode); } public static final class CurrencyFormatInfo { public final String currencyPattern; - public final String monetarySeparator; + public final String monetaryDecimalSeparator; public final String monetaryGroupingSeparator; public CurrencyFormatInfo(String currencyPattern, String monetarySeparator, String monetaryGroupingSeparator) { this.currencyPattern = currencyPattern; - this.monetarySeparator = monetarySeparator; + this.monetaryDecimalSeparator = monetarySeparator; this.monetaryGroupingSeparator = monetaryGroupingSeparator; } } @@ -47,6 +48,9 @@ public class CurrencyData { public static final class CurrencySpacingInfo { private final String[][] symbols = new String[SpacingType.COUNT.ordinal()][SpacingPattern.COUNT.ordinal()]; + public boolean hasBeforeCurrency = false; + public boolean hasAfterCurrency = false; + public static enum SpacingType { BEFORE, AFTER, COUNT }; public static enum SpacingPattern { CURRENCY_MATCH(DecimalFormatSymbols.CURRENCY_SPC_CURRENCY_MATCH), @@ -144,6 +148,11 @@ public class CurrencyData { return fallback ? isoCode : null; } + @Override + public String getNarrowSymbol(String isoCode) { + return fallback ? isoCode : null; + } + @Override public Map symbolMap() { return Collections.emptyMap(); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyDisplayNames.java b/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyDisplayNames.java index 0f9c59469d..448957700e 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyDisplayNames.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyDisplayNames.java @@ -16,9 +16,9 @@ import com.ibm.icu.util.ULocale; /** * Returns currency names localized for a locale. - * + * * This class is not intended for public subclassing. - * + * * @stable ICU 4.4 */ public abstract class CurrencyDisplayNames { @@ -27,9 +27,9 @@ public abstract class CurrencyDisplayNames { * localized for display in the provided locale. If there is no data for the * provided locale, this falls back to the current default locale; if there * is no data for that either, it falls back to the root locale. Substitute - * values are returned from APIs when there is no data for the requested ISO + * values are returned from APIs when there is no data for the requested ISO * code. - * + * * @param locale the locale into which to localize the names * @return a CurrencyDisplayNames * @stable ICU 4.4 @@ -43,9 +43,9 @@ public abstract class CurrencyDisplayNames { * localized for display in the provided locale. If there is no data for the * provided locale, this falls back to the current default locale; if there * is no data for that either, it falls back to the root locale. Substitute - * values are returned from APIs when there is no data for the requested ISO + * values are returned from APIs when there is no data for the requested ISO * code. - * + * * @param locale the locale into which to localize the names * @return a CurrencyDisplayNames * @stable ICU 54 @@ -62,7 +62,7 @@ public abstract class CurrencyDisplayNames { * the default locale or root, and null is returned, and 2) if there is data * for the locale, but not data for the requested ISO code, null is returned * from those APIs instead of a substitute value. - * + * * @param locale the locale into which to localize the names * @param noSubstitute if true, do not return substitute values. * @return a CurrencyDisplayNames @@ -80,7 +80,7 @@ public abstract class CurrencyDisplayNames { * the default locale or root, and null is returned, and 2) if there is data * for the locale, but not data for the requested ISO code, null is returned * from those APIs instead of a substitute value. - * + * * @param locale the {@link java.util.Locale} into which to localize the names * @param noSubstitute if true, do not return substitute values. * @return a CurrencyDisplayNames @@ -112,7 +112,7 @@ public abstract class CurrencyDisplayNames { /** * Returns the symbol for the currency with the provided ISO code. If * there is no data for the ISO code, substitutes isoCode or returns null. - * + * * @param isoCode the three-letter ISO code. * @return the display name. * @stable ICU 4.4 @@ -122,7 +122,7 @@ public abstract class CurrencyDisplayNames { /** * Returns the 'long name' for the currency with the provided ISO code. * If there is no data for the ISO code, substitutes isoCode or returns null. - * + * * @param isoCode the three-letter ISO code * @return the display name * @stable ICU 4.4 @@ -132,9 +132,9 @@ public abstract class CurrencyDisplayNames { /** * Returns a 'plural name' for the currency with the provided ISO code corresponding to * the pluralKey. If there is no data for the ISO code, substitutes isoCode or - * returns null. If there is data for the ISO code but no data for the plural key, + * returns null. If there is data for the ISO code but no data for the plural key, * substitutes the 'other' value (and failing that the isoCode) or returns null. - * + * * @param isoCode the three-letter ISO code * @param pluralKey the plural key, for example "one", "other" * @return the display name diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java index 984afc83f7..8bba53a13d 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java @@ -1414,7 +1414,7 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { CurrencyFormatInfo fmtInfo = info.getFormatInfo(intlCurrencySymbol); if (fmtInfo != null) { currencyPattern = fmtInfo.currencyPattern; - setMonetaryDecimalSeparatorString(fmtInfo.monetarySeparator); + setMonetaryDecimalSeparatorString(fmtInfo.monetaryDecimalSeparator); setMonetaryGroupingSeparatorString(fmtInfo.monetaryGroupingSeparator); } } else { diff --git a/icu4j/main/classes/currdata/src/com/ibm/icu/impl/ICUCurrencyDisplayInfoProvider.java b/icu4j/main/classes/currdata/src/com/ibm/icu/impl/ICUCurrencyDisplayInfoProvider.java index ceabeea4d5..81eb849bed 100644 --- a/icu4j/main/classes/currdata/src/com/ibm/icu/impl/ICUCurrencyDisplayInfoProvider.java +++ b/icu4j/main/classes/currdata/src/com/ibm/icu/impl/ICUCurrencyDisplayInfoProvider.java @@ -11,17 +11,15 @@ package com.ibm.icu.impl; import java.lang.ref.SoftReference; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.MissingResourceException; -import java.util.Set; -import java.util.TreeMap; import com.ibm.icu.impl.CurrencyData.CurrencyDisplayInfo; import com.ibm.icu.impl.CurrencyData.CurrencyDisplayInfoProvider; import com.ibm.icu.impl.CurrencyData.CurrencyFormatInfo; import com.ibm.icu.impl.CurrencyData.CurrencySpacingInfo; import com.ibm.icu.impl.ICUResourceBundle.OpenType; +import com.ibm.icu.util.ICUException; import com.ibm.icu.util.ULocale; import com.ibm.icu.util.UResourceBundle; @@ -54,17 +52,68 @@ public class ICUCurrencyDisplayInfoProvider implements CurrencyDisplayInfoProvid static class ICUCurrencyDisplayInfo extends CurrencyDisplayInfo { private final boolean fallback; private final ICUResourceBundle rb; - private final ICUResourceBundle currencies; - private final ICUResourceBundle plurals; - private SoftReference> _symbolMapRef; - private SoftReference> _nameMapRef; + private volatile SoftReference rawDataCache; + + /** + * The primary data structure is isoCodeToCurrencyStrings. In that structure, + * the String arrays contain the following elements: + * + * [DISPLAY_NAME] => display name + * [SYMBOL] => symbol + * [NARROW_SYMBOL] => narrow symbol + * [FORMAT_PATTERN] => currency format pattern + * [DECIMAL_SEPARATOR] => currency decimal separator + * [GROUPING_SEPARATOR] => currency grouping separator + * [PLURALS_OFFSET+p] => plural name where p=standardPlural.ordinal() + */ + private static class RawCurrencyData { + static final int DISPLAY_NAME = 0; + static final int SYMBOL = 1; + static final int NARROW_SYMBOL = 2; + static final int FORMAT_PATTERN = 3; + static final int DECIMAL_SEPARATOR = 4; + static final int GROUPING_SEPARATOR = 5; + static final int PLURALS_OFFSET = 6; + static final int CURRENCY_STRINGS_LENGTH = 6 + StandardPlural.COUNT; + + Map isoCodeToCurrencyStrings = new HashMap(); + + // The following maps are redundant data with the above map, but the API for CurrencyDisplayNames + // restricts us to using these data structures. + Map symbolToIsoCode = new HashMap(); + Map nameToIsoCode = new HashMap(); + + // Other currency-related data + CurrencySpacingInfo spacingInfo = new CurrencySpacingInfo(); + Map currencyUnitPatterns = new HashMap(); + + /** + * Gets an entry out of isoCodeToCurrencyStrings or creates it if it does not exist yet. + */ + String[] getOrCreateCurrencyStrings(String isoCode) { + String[] currencyStrings = isoCodeToCurrencyStrings.get(isoCode); + if (currencyStrings == null) { + currencyStrings = new String[CURRENCY_STRINGS_LENGTH]; + isoCodeToCurrencyStrings.put(isoCode, currencyStrings); + } + return currencyStrings; + } + + /** + * Called after all data is loaded to convert the externally visible Maps to Unmodifiable. + */ + void freezeMaps() { + symbolToIsoCode = Collections.unmodifiableMap(symbolToIsoCode); + nameToIsoCode = Collections.unmodifiableMap(nameToIsoCode); + currencyUnitPatterns = Collections.unmodifiableMap(currencyUnitPatterns); + } + } public ICUCurrencyDisplayInfo(ICUResourceBundle rb, boolean fallback) { this.fallback = fallback; this.rb = rb; - this.currencies = rb.findTopLevel("Currencies"); - this.plurals = rb.findTopLevel("CurrencyPlurals"); - } + rawDataCache = new SoftReference(null); + } @Override public ULocale getULocale() { @@ -73,129 +122,286 @@ public class ICUCurrencyDisplayInfoProvider implements CurrencyDisplayInfoProvid @Override public String getName(String isoCode) { - return getName(isoCode, false); + return getName(isoCode, RawCurrencyData.DISPLAY_NAME); } @Override public String getSymbol(String isoCode) { - return getName(isoCode, true); + return getName(isoCode, RawCurrencyData.SYMBOL); } - private String getName(String isoCode, boolean symbolName) { - if (currencies != null) { - ICUResourceBundle result = currencies.findWithFallback(isoCode); - if (result != null) { - if (!fallback && !rb.isRoot() && result.isRoot()) { - return null; - } - return result.getString(symbolName ? 0 : 1); - } - } + @Override + public String getNarrowSymbol(String isoCode) { + // TODO: Should this fall back to the regular symbol instead of the ISO code? + return getName(isoCode, RawCurrencyData.NARROW_SYMBOL); + } - return fallback ? isoCode : null; + private String getName(String isoCode, int index) { + String[] currencyStrings = getRawCurrencyData().isoCodeToCurrencyStrings.get(isoCode); + String result = null; + if (currencyStrings != null) { + result = currencyStrings[index]; + } + // If fallback is true, don't return null; return the ISO code + if (result == null && fallback) { + result = isoCode; + } + return result; } @Override public String getPluralName(String isoCode, String pluralKey ) { - // See http://unicode.org/reports/tr35/#Currencies, especially the fallback rule. - if (plurals != null) { - ICUResourceBundle pluralsBundle = plurals.findWithFallback(isoCode); - if (pluralsBundle != null) { - String pluralName = pluralsBundle.findStringWithFallback(pluralKey); - if (pluralName == null) { - if (!fallback) { - return null; - } - pluralName = pluralsBundle.findStringWithFallback("other"); - if (pluralName == null) { - return getName(isoCode); - } - } - return pluralName; - } + StandardPlural plural = StandardPlural.orNullFromString(pluralKey); + String[] currencyStrings = getRawCurrencyData().isoCodeToCurrencyStrings.get(isoCode); + String result = null; + if (currencyStrings != null && plural != null) { + result = currencyStrings[RawCurrencyData.PLURALS_OFFSET + plural.ordinal()]; } - - return fallback ? getName(isoCode) : null; + // See http://unicode.org/reports/tr35/#Currencies, especially the fallback rule. + if (result == null && currencyStrings != null && fallback) { + // First fall back to the "other" plural variant + // Note: If plural is already "other", this fallback is benign + result = currencyStrings[RawCurrencyData.PLURALS_OFFSET + StandardPlural.OTHER.ordinal()]; + } + if (result == null && currencyStrings != null && fallback) { + // If that fails, fall back to the display name + result = currencyStrings[0]; + } + if (result == null && fallback) { + // If all else fails, return the ISO code + result = isoCode; + } + return result; } @Override public Map symbolMap() { - Map map = _symbolMapRef == null ? null : _symbolMapRef.get(); - if (map == null) { - map = _createSymbolMap(); - // atomic and idempotent - _symbolMapRef = new SoftReference>(map); - } - return map; + return getRawCurrencyData().symbolToIsoCode; } @Override public Map nameMap() { - Map map = _nameMapRef == null ? null : _nameMapRef.get(); - if (map == null) { - map = _createNameMap(); - // atomic and idempotent - _nameMapRef = new SoftReference>(map); - } - return map; + return getRawCurrencyData().nameToIsoCode; } @Override public Map getUnitPatterns() { - Map result = new HashMap(); - - ULocale locale = rb.getULocale(); - for (;locale != null; locale = locale.getFallback()) { - ICUResourceBundle r = (ICUResourceBundle) UResourceBundle.getBundleInstance( - ICUData.ICU_CURR_BASE_NAME, locale); - if (r == null) { - continue; - } - ICUResourceBundle cr = r.findWithFallback("CurrencyUnitPatterns"); - if (cr == null) { - continue; - } - for (int index = 0, size = cr.getSize(); index < size; ++index) { - ICUResourceBundle b = (ICUResourceBundle) cr.get(index); - String key = b.getKey(); - if (result.containsKey(key)) { - continue; - } - result.put(key, b.getString()); - } - } - // Default result is the empty map. Callers who require a pattern will have to // supply a default. - return Collections.unmodifiableMap(result); + return getRawCurrencyData().currencyUnitPatterns; } @Override public CurrencyFormatInfo getFormatInfo(String isoCode) { - ICUResourceBundle crb = currencies.findWithFallback(isoCode); - if (crb != null && crb.getSize() > 2) { - crb = crb.at(2); - if (crb != null) { - String pattern = crb.getString(0); - String separator = crb.getString(1); - String groupingSeparator = crb.getString(2); - return new CurrencyFormatInfo(pattern, separator, groupingSeparator); - } + String[] currencyStrings = getRawCurrencyData().isoCodeToCurrencyStrings.get(isoCode); + if (currencyStrings == null || currencyStrings[RawCurrencyData.FORMAT_PATTERN] == null) { + return null; } - return null; + String pattern = currencyStrings[RawCurrencyData.FORMAT_PATTERN]; + String decimalSeparator = currencyStrings[RawCurrencyData.DECIMAL_SEPARATOR]; + String groupingSeparator = currencyStrings[RawCurrencyData.GROUPING_SEPARATOR]; + return new CurrencyFormatInfo(pattern, decimalSeparator, groupingSeparator); } @Override public CurrencySpacingInfo getSpacingInfo() { - SpacingInfoSink sink = new SpacingInfoSink(); - rb.getAllItemsWithFallback("currencySpacing", sink); - return sink.getSpacingInfo(fallback); + CurrencySpacingInfo result = getRawCurrencyData().spacingInfo; + if (result != null && (!result.hasBeforeCurrency || !result.hasAfterCurrency) && fallback) { + result = CurrencySpacingInfo.DEFAULT; + } + if (result == null && fallback) { + result = CurrencySpacingInfo.DEFAULT; + } + return result; } - private final class SpacingInfoSink extends UResource.Sink { - CurrencySpacingInfo spacingInfo = new CurrencySpacingInfo(); - boolean hasBeforeCurrency = false; - boolean hasAfterCurrency = false; + /** + * If the soft cache is populated, returns the data stored there. + * Otherwise, computes the data, stores it in the cache, and returns it. + * Never returns null. + */ + private RawCurrencyData getRawCurrencyData() { + RawCurrencyData data = rawDataCache.get(); + if (data == null) { + data = new RawCurrencyData(); + RawCurrencyDataSink sink = new RawCurrencyDataSink(data, !fallback); + rb.getAllItemsWithFallback("", sink); + data.freezeMaps(); + rawDataCache = new SoftReference(data); + } + return data; + } + + private static final class RawCurrencyDataSink extends UResource.Sink { + private final RawCurrencyData data; + private final boolean noRoot; + + RawCurrencyDataSink(RawCurrencyData data, boolean noRoot) { + this.data = data; + this.noRoot = noRoot; + } + + /** + * The entrypoint method delegates to helper methods for each of the types of tables + * found in the currency data. + */ + @Override + public void put(UResource.Key key, UResource.Value value, boolean noFallback) { + if (noRoot && noFallback) { + // Don't consume the root bundle + return; + } + + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + if (key.contentEquals("Currencies")) { + consumeCurrencies(key, value, noFallback); + } else if (key.contentEquals("Currencies%narrow")) { + consumeCurrenciesNarrow(key, value, noFallback); + } else if (key.contentEquals("Currencies%variant")) { + consumeCurrenciesVariant(key, value, noFallback); + } else if (key.contentEquals("CurrencyPlurals")) { + consumeCurrencyPlurals(key, value, noFallback); + } else if (key.contentEquals("currencySpacing")) { + consumeCurrencySpacing(key, value, noFallback); + } else if (key.contentEquals("CurrencyUnitPatterns")) { + consumeCurrencyUnitPatterns(key, value, noFallback); + } + } + } + + /* + * Currencies{ + * ... + * USD{ + * "US$", => symbol + * "US Dollar", => display name + * } + * ... + * ESP{ + * "₧", => symbol + * "pesseta espanyola", => display name + * { + * "¤ #,##0.00", => currency-specific pattern + * ",", => currency-specific grouping separator + * ".", => currency-specific decimal separator + * } + * } + * ... + * } + */ + private void consumeCurrencies(UResource.Key key, UResource.Value value, boolean noFallback) { + + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + String isoCode = key.toString(); + String[] currencyStrings = data.getOrCreateCurrencyStrings(isoCode); + if (value.getType() != UResourceBundle.ARRAY) { + throw new ICUException("Unexpected data type in Currencies table for " + isoCode); + } + UResource.Array array = value.getArray(); + + // First element is the symbol. + array.getValue(0, value); + String symbol = value.getString(); + if (currencyStrings[RawCurrencyData.SYMBOL] == null) { + currencyStrings[RawCurrencyData.SYMBOL] = symbol; + } + + // Second element is the display name. + array.getValue(1, value); + String name = value.getString(); + if (currencyStrings[RawCurrencyData.DISPLAY_NAME] == null) { + currencyStrings[RawCurrencyData.DISPLAY_NAME] = name; + } + + // If present, the third element is the currency format info. + // TODO: Write unit test to ensure that this data is being used by number formatting. + if (array.getSize() > 2 && currencyStrings[RawCurrencyData.FORMAT_PATTERN] == null) { + array.getValue(2, value); + UResource.Array formatArray = value.getArray(); + formatArray.getValue(0, value); + currencyStrings[RawCurrencyData.FORMAT_PATTERN] = value.getString(); + assert currencyStrings[RawCurrencyData.DECIMAL_SEPARATOR] == null; + formatArray.getValue(1, value); + currencyStrings[RawCurrencyData.DECIMAL_SEPARATOR] = value.getString(); + assert currencyStrings[RawCurrencyData.GROUPING_SEPARATOR] == null; + formatArray.getValue(2, value); + currencyStrings[RawCurrencyData.GROUPING_SEPARATOR] = value.getString(); + } + + // Add the name and symbols to the other two maps (used for parsing). + data.nameToIsoCode.put(name, isoCode); + data.symbolToIsoCode.put(isoCode, isoCode); // Add the ISO code itself as a symbol + data.symbolToIsoCode.put(symbol, isoCode); + } + } + + /* + * Currencies%narrow{ + * AOA{"Kz"} + * ARS{"$"} + * ... + * } + */ + private void consumeCurrenciesNarrow(UResource.Key key, UResource.Value value, boolean noFallback) { + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + String isoCode = key.toString(); + String[] currencyStrings = data.getOrCreateCurrencyStrings(isoCode); + if (currencyStrings[RawCurrencyData.NARROW_SYMBOL] == null) { + currencyStrings[RawCurrencyData.NARROW_SYMBOL] = value.getString(); + } + + // Note: This data is used for formatting but not parsing. + } + } + + /* + * Currencies%variant{ + * TRY{"TL"} + * } + */ + private void consumeCurrenciesVariant(UResource.Key key, UResource.Value value, boolean noFallback) { + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + String isoCode = key.toString(); + + // Note: This data is used for parsing but not formatting. + data.symbolToIsoCode.put(value.getString(), isoCode); + } + } + + /* + * CurrencyPlurals{ + * BYB{ + * one{"Belarusian new rouble (1994–1999)"} + * other{"Belarusian new roubles (1994–1999)"} + * } + * ... + * } + */ + private void consumeCurrencyPlurals(UResource.Key key, UResource.Value value, boolean noFallback) { + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + String isoCode = key.toString(); + String[] currencyStrings = data.getOrCreateCurrencyStrings(isoCode); + UResource.Table pluralsTable = value.getTable(); + for (int j=0; pluralsTable.getKeyAndValue(j, key, value); j++) { + StandardPlural plural = StandardPlural.orNullFromString(key.toString()); + if (plural == null) { + throw new ICUException("Could not make StandardPlural from keyword " + key); + } + String valueString = value.getString(); + if (currencyStrings[RawCurrencyData.PLURALS_OFFSET + plural.ordinal()] == null) { + currencyStrings[RawCurrencyData.PLURALS_OFFSET + plural.ordinal()] = valueString; + } + + // Add the name to the name-to-currency map (used for parsing) + data.nameToIsoCode.put(valueString, isoCode); + } + } + } /* * currencySpacing{ @@ -211,17 +417,16 @@ public class ICUCurrencyDisplayInfoProvider implements CurrencyDisplayInfoProvid * } * } */ - @Override - public void put(UResource.Key key, UResource.Value value, boolean noFallback) { + private void consumeCurrencySpacing(UResource.Key key, UResource.Value value, boolean noFallback) { UResource.Table spacingTypesTable = value.getTable(); for (int i = 0; spacingTypesTable.getKeyAndValue(i, key, value); ++i) { CurrencySpacingInfo.SpacingType type; if (key.contentEquals("beforeCurrency")) { type = CurrencySpacingInfo.SpacingType.BEFORE; - hasBeforeCurrency = true; + data.spacingInfo.hasBeforeCurrency = true; } else if (key.contentEquals("afterCurrency")) { type = CurrencySpacingInfo.SpacingType.AFTER; - hasAfterCurrency = true; + data.spacingInfo.hasAfterCurrency = true; } else { continue; } @@ -239,95 +444,26 @@ public class ICUCurrencyDisplayInfoProvider implements CurrencyDisplayInfoProvid continue; } - spacingInfo.setSymbolIfNull(type, pattern, value.getString()); + data.spacingInfo.setSymbolIfNull(type, pattern, value.getString()); } } } - CurrencySpacingInfo getSpacingInfo(boolean fallback) { - if (hasBeforeCurrency && hasAfterCurrency) { - return spacingInfo; - } else if (fallback) { - return CurrencySpacingInfo.DEFAULT; - } else { - return null; - } - } - } - - private Map _createSymbolMap() { - Map result = new HashMap(); - - for (ULocale locale = rb.getULocale(); locale != null; locale = locale.getFallback()) { - ICUResourceBundle bundle = (ICUResourceBundle) - UResourceBundle.getBundleInstance(ICUData.ICU_CURR_BASE_NAME, locale); - ICUResourceBundle curr = bundle.findTopLevel("Currencies"); - if (curr == null) { - continue; - } - for (int i = 0; i < curr.getSize(); ++i) { - ICUResourceBundle item = curr.at(i); - String isoCode = item.getKey(); - if (!result.containsKey(isoCode)) { - // put the code itself - result.put(isoCode, isoCode); - // 0 == symbol element - String symbol = item.getString(0); - result.put(symbol, isoCode); + /* + * CurrencyUnitPatterns{ + * other{"{0} {1}"} + * ... + * } + */ + private void consumeCurrencyUnitPatterns(UResource.Key key, UResource.Value value, boolean noFallback) { + UResource.Table table = value.getTable(); + for (int i = 0; table.getKeyAndValue(i, key, value); i++) { + String pluralKeyword = key.toString(); + if (data.currencyUnitPatterns.get(pluralKeyword) == null) { + data.currencyUnitPatterns.put(pluralKeyword, value.getString()); } } } - - return Collections.unmodifiableMap(result); - } - - private Map _createNameMap() { - // ignore case variants - Map result = new TreeMap(String.CASE_INSENSITIVE_ORDER); - - Set visited = new HashSet(); - Map> visitedPlurals = new HashMap>(); - for (ULocale locale = rb.getULocale(); locale != null; locale = locale.getFallback()) { - ICUResourceBundle bundle = (ICUResourceBundle) - UResourceBundle.getBundleInstance(ICUData.ICU_CURR_BASE_NAME, locale); - ICUResourceBundle curr = bundle.findTopLevel("Currencies"); - if (curr != null) { - for (int i = 0; i < curr.getSize(); ++i) { - ICUResourceBundle item = curr.at(i); - String isoCode = item.getKey(); - if (!visited.contains(isoCode)) { - visited.add(isoCode); - // 1 == name element - String name = item.getString(1); - result.put(name, isoCode); - } - } - } - - ICUResourceBundle plurals = bundle.findTopLevel("CurrencyPlurals"); - if (plurals != null) { - for (int i = 0; i < plurals.getSize(); ++i) { - ICUResourceBundle item = plurals.at(i); - String isoCode = item.getKey(); - Set pluralSet = visitedPlurals.get(isoCode); - if (pluralSet == null) { - pluralSet = new HashSet(); - visitedPlurals.put(isoCode, pluralSet); - } - for (int j = 0; j < item.getSize(); ++j) { - ICUResourceBundle plural = item.at(j); - String pluralType = plural.getKey(); - if (!pluralSet.contains(pluralType)) { - String pluralName = plural.getString(); - result.put(pluralName, isoCode); - pluralSet.add(pluralType); - } - } - } - } - } - - return Collections.unmodifiableMap(result); } } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt b/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt index 8801d986c5..0eef9e3db4 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt @@ -240,8 +240,8 @@ $**####,##0 1234 $***1\u00a0234 K // In J ICU adds padding as if 'EUR' is only 2 chars (2 * 0xa4) \u00a4\u00a4 **####0.00 433.0 EUR *433,00 JK // In J ICU adds padding as if 'EUR' is only 2 chars (2 * 0xa4) -// S and Q fail this one because the test code bypasses CurrencyUsage -\u00a4\u00a4 **#######0 433.0 EUR *433,00 JKSQ +// Q fails this one because the test code bypasses CurrencyUsage +\u00a4\u00a4 **#######0 433.0 EUR *433,00 JKQ test padding and currencies begin @@ -317,10 +317,9 @@ minIntegerDigits maxIntegerDigits minFractionDigits maxFractionDigits output bre 0 1 0 0 2.99792458E8 K 1 1 0 0 3E8 // JDK gives E0 instead of allowing for unlimited precision -// S obeys the maximum integer digits and returns .299792458E9 -0 0 0 0 2.99792458E8 KS -// JDK and S give .299792E9; Q gives 2.99792E8 -0 1 0 5 2.9979E8 KSQ +0 0 0 0 2.99792458E8 K +// JDK gives .299792E9; Q gives 2.99792E8 +0 1 0 5 2.9979E8 KQ // JDK gives 300E6 0 3 0 0 299.792458E6 K // JDK gives 299.8E6 (maybe maxInt + maxFrac instead of minInt + maxFrac)? @@ -335,10 +334,9 @@ minIntegerDigits maxIntegerDigits minFractionDigits maxFractionDigits output bre 4 4 0 0 2998E5 0 0 1 5 .29979E9 // JDK gives E0 -// S obeys the maximum integer digits -0 0 1 0 2.99792458E8 KS -// JDK and S give .2998E9 -0 0 0 4 2.998E8 KSQ +0 0 1 0 2.99792458E8 K +// JDK and Q give .2998E9 +0 0 0 4 2.998E8 KQ // According to the spec, if maxInt>minInt and minInt>1, then set // Context: #13289 2 8 1 6 2.9979246E8 K @@ -371,8 +369,7 @@ set format 29979245.0 begin minIntegerDigits maxIntegerDigits minFractionDigits maxFractionDigits output breaks // JDK gives E0 -// S obeys the max integer digits and prints 0.299... -0 0 0 0 2.9979245E7 KS +0 0 0 0 2.9979245E7 K // JDK gives .3E8 0 1 0 0 2.9979245E7 K // JDK gives 2998E4. @@ -385,12 +382,12 @@ begin format maxIntegerDigits output breaks 123 1 3 0 0 0 -// S and Q ignore max integer if it is less than zero and prints "123" -123 -2147483648 0 SQ +// Q ignores max integer if it is less than zero and prints "123" +123 -2147483648 0 Q 12345 1 5 -12345 -2147483648 0 SQ +12345 -2147483648 0 Q 5.3 1 5.3 -5.3 -2147483648 .3 SQ +5.3 -2147483648 .3 Q test patterns with zero set locale en @@ -567,7 +564,7 @@ set locale en set currency USD begin pattern format output breaks -# 123 123 SQ +# 123 123 Q // Currency rounding should always override the pattern. // K prints the currency in ISO format for some reason. \u00a4# 123 $123.00 K @@ -1028,7 +1025,7 @@ begin parse output outputCurrency breaks 53.45 fail GBP £53.45 53.45 GBP -$53.45 fail USD +$53.45 fail USD J 53.45 USD 53.45 USD 53.45 GBP 53.45 GBP USD 53.45 53.45 USD J @@ -1044,7 +1041,7 @@ USD (7.92) -7.92 USD CJ (7.92)USD -7.92 USD CJ USD(7.92) -7.92 USD CJ (8) USD -8 USD --8 USD -8 USD CJ +-8 USD -8 USD C 67 USD 67 USD 53.45$ fail USD US Dollars 53.45 53.45 USD J @@ -1075,7 +1072,7 @@ begin parse output outputCurrency breaks 53.45 fail GBP £53.45 53.45 GBP -$53.45 fail USD +$53.45 fail USD J 53.45 USD 53.45 USD 53.45 GBP 53.45 GBP USD 53.45 53.45 USD J @@ -1109,7 +1106,7 @@ parse output outputCurrency breaks // J throws a NullPointerException on the first case 53.45 fail GBP £53.45 53.45 GBP -$53.45 fail USD +$53.45 fail USD J 53.45 USD 53.45 USD 53.45 GBP 53.45 GBP USD 53.45 53.45 USD J @@ -1125,7 +1122,7 @@ USD (7.92) -7.92 USD CJ (7.92)USD -7.92 USD CJ USD(7.92) -7.92 USD CJ (8) USD -8 USD --8 USD -8 USD CJ +-8 USD -8 USD C 67 USD 67 USD // J throws a NullPointerException on the next case 53.45$ fail USD @@ -1147,7 +1144,7 @@ begin parse output outputCurrency breaks 53.45 fail GBP £53.45 53.45 GBP -$53.45 fail USD +$53.45 fail USD J 53.45 USD 53.45 USD 53.45 GBP 53.45 GBP USD 53.45 53.45 USD J @@ -1163,7 +1160,7 @@ USD (7.92) -7.92 USD CJ (7.92)USD -7.92 USD CJ USD(7.92) -7.92 USD CJ (8) USD -8 USD --8 USD -8 USD CJ +-8 USD -8 USD C 67 USD 67 USD 53.45$ fail USD US Dollars 53.45 53.45 USD J @@ -1184,9 +1181,9 @@ begin parse output outputCurrency breaks 53.45 fail GBP £53.45 53.45 GBP -$53.45 fail USD -53.45 USD 53.45 USD CJ -53.45 GBP 53.45 GBP CJ +$53.45 fail USD J +53.45 USD 53.45 USD C +53.45 GBP 53.45 GBP C USD 53.45 53.45 USD J 53.45USD 53.45 USD CJ USD53.45 53.45 USD @@ -1201,8 +1198,8 @@ USD (7.92) -7.92 USD CJS (7.92)USD -7.92 USD CJS USD(7.92) -7.92 USD CJS (8) USD -8 USD CJS --8 USD -8 USD CJ -67 USD 67 USD CJ +-8 USD -8 USD C +67 USD 67 USD C 53.45$ fail USD US Dollars 53.45 53.45 USD J 53.45 US Dollars 53.45 USD @@ -1325,7 +1322,7 @@ USD (7.92) fail USD (7.92)USD fail USD USD(7.92) fail USD (8) USD -8 USD --8 USD fail USD +-8 USD fail USD J 67 USD 67 USD 53.45$ fail USD US Dollars 53.45 fail USD diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java index 455a9dc8f4..bae5b5324b 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/CompactDecimalFormatTest.java @@ -587,6 +587,7 @@ public class CompactDecimalFormatTest extends TestFmwk { public void TestLongShortFallback() { // smn, dz have long but not short // es_US, es_GT, es_419, ee have short but not long + // TODO: This test is out-of-date. The locales have more data as of ICU 60. ULocale[] locs = new ULocale[] { new ULocale("smn"), new ULocale("es_US"), diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DataDrivenNumberFormatTestUtility.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DataDrivenNumberFormatTestUtility.java index 0ee21a952a..06995968b3 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DataDrivenNumberFormatTestUtility.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DataDrivenNumberFormatTestUtility.java @@ -204,10 +204,7 @@ public class DataDrivenNumberFormatTestUtility extends TestFmwk { return; } } - if (tuple.output != null && tuple.output.equals("fail") && tuple.parse != null && tuple.parse.equals("$53.45") && - logKnownIssue("13368", "en_GB parsing of $53.45 as USD works, shouldn't") ) { - // skip test - } else if (runMode == RunMode.INCLUDE_KNOWN_FAILURES || !breaks(codeUnderTestId)) { + if (runMode == RunMode.INCLUDE_KNOWN_FAILURES || !breaks(codeUnderTestId)) { String errorMessage; Exception err = null; boolean shouldFail = (tuple.output != null && tuple.output.equals("fail")) diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java index c33e1513c9..7fcafc290c 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java @@ -4,6 +4,7 @@ package com.ibm.icu.dev.test.format; import java.math.BigDecimal; import java.math.RoundingMode; +import java.text.ParseException; import java.text.ParsePosition; import org.junit.Test; @@ -11,11 +12,14 @@ import org.junit.Test; import com.ibm.icu.dev.test.TestUtil; import com.ibm.icu.impl.number.DecimalFormatProperties; import com.ibm.icu.impl.number.Padder.PadPosition; +import com.ibm.icu.impl.number.Parse; import com.ibm.icu.impl.number.Parse.ParseMode; import com.ibm.icu.impl.number.PatternStringParser; import com.ibm.icu.impl.number.PatternStringUtils; import com.ibm.icu.number.LocalizedNumberFormatter; import com.ibm.icu.number.NumberFormatter; +import com.ibm.icu.text.DecimalFormat; +import com.ibm.icu.text.DecimalFormat.PropertySetter; import com.ibm.icu.text.DecimalFormatSymbols; import com.ibm.icu.text.DecimalFormat_ICU58; import com.ibm.icu.util.CurrencyAmount; @@ -529,6 +533,9 @@ public class NumberFormatDataDrivenTest { } } + /** + * Formatting, but no other features. + */ private DataDrivenNumberFormatTestUtility.CodeUnderTest ICU60 = new DataDrivenNumberFormatTestUtility.CodeUnderTest() { @@ -566,6 +573,170 @@ public class NumberFormatDataDrivenTest { } }; + /** + * All features except formatting. + */ + private DataDrivenNumberFormatTestUtility.CodeUnderTest ICU60_Other = + new DataDrivenNumberFormatTestUtility.CodeUnderTest() { + + @Override + public Character Id() { + return 'S'; + } + + /** + * Runs a single toPattern test. On success, returns null. On failure, returns the error. This implementation + * just returns null. Subclasses should override. + * + * @param tuple + * contains the parameters of the format test. + */ + @Override + public String toPattern(DataDrivenNumberFormatTestData tuple) { + String pattern = (tuple.pattern == null) ? "0" : tuple.pattern; + final DecimalFormatProperties properties; + DecimalFormat df; + try { + properties = PatternStringParser.parseToProperties( + pattern, + tuple.currency != null ? PatternStringParser.IGNORE_ROUNDING_ALWAYS + : PatternStringParser.IGNORE_ROUNDING_NEVER); + propertiesFromTuple(tuple, properties); + // TODO: Use PatternString.propertiesToString() directly. (How to deal with CurrencyUsage?) + df = new DecimalFormat(); + df.setProperties(new PropertySetter() { + @Override + public void set(DecimalFormatProperties props) { + props.copyFrom(properties); + } + }); + } catch (IllegalArgumentException e) { + e.printStackTrace(); + return e.getLocalizedMessage(); + } + + if (tuple.toPattern != null) { + String expected = tuple.toPattern; + String actual = df.toPattern(); + if (!expected.equals(actual)) { + return "Expected toPattern='" + expected + "'; got '" + actual + "'"; + } + } + if (tuple.toLocalizedPattern != null) { + String expected = tuple.toLocalizedPattern; + String actual = PatternStringUtils.propertiesToPatternString(properties); + if (!expected.equals(actual)) { + return "Expected toLocalizedPattern='" + expected + "'; got '" + actual + "'"; + } + } + return null; + } + + /** + * Runs a single parse test. On success, returns null. On failure, returns the error. This implementation just + * returns null. Subclasses should override. + * + * @param tuple + * contains the parameters of the format test. + */ + @Override + public String parse(DataDrivenNumberFormatTestData tuple) { + String pattern = (tuple.pattern == null) ? "0" : tuple.pattern; + DecimalFormatProperties properties; + ParsePosition ppos = new ParsePosition(0); + Number actual; + try { + properties = PatternStringParser.parseToProperties( + pattern, + tuple.currency != null ? PatternStringParser.IGNORE_ROUNDING_ALWAYS + : PatternStringParser.IGNORE_ROUNDING_NEVER); + propertiesFromTuple(tuple, properties); + actual = Parse.parse(tuple.parse, ppos, properties, DecimalFormatSymbols.getInstance(tuple.locale)); + } catch (IllegalArgumentException e) { + return "parse exception: " + e.getMessage(); + } + if (actual == null && ppos.getIndex() != 0) { + throw new AssertionError("Error: value is null but parse position is not zero"); + } + if (ppos.getIndex() == 0) { + return "Parse failed; got " + actual + ", but expected " + tuple.output; + } + if (tuple.output.equals("NaN")) { + if (!Double.isNaN(actual.doubleValue())) { + return "Expected NaN, but got: " + actual; + } + return null; + } else if (tuple.output.equals("Inf")) { + if (!Double.isInfinite(actual.doubleValue()) || Double.compare(actual.doubleValue(), 0.0) < 0) { + return "Expected Inf, but got: " + actual; + } + return null; + } else if (tuple.output.equals("-Inf")) { + if (!Double.isInfinite(actual.doubleValue()) || Double.compare(actual.doubleValue(), 0.0) > 0) { + return "Expected -Inf, but got: " + actual; + } + return null; + } else if (tuple.output.equals("fail")) { + return null; + } else if (new BigDecimal(tuple.output).compareTo(new BigDecimal(actual.toString())) != 0) { + return "Expected: " + tuple.output + ", got: " + actual; + } else { + return null; + } + } + + /** + * Runs a single parse currency test. On success, returns null. On failure, returns the error. This + * implementation just returns null. Subclasses should override. + * + * @param tuple + * contains the parameters of the format test. + */ + @Override + public String parseCurrency(DataDrivenNumberFormatTestData tuple) { + String pattern = (tuple.pattern == null) ? "0" : tuple.pattern; + DecimalFormatProperties properties; + ParsePosition ppos = new ParsePosition(0); + CurrencyAmount actual; + try { + properties = PatternStringParser.parseToProperties( + pattern, + tuple.currency != null ? PatternStringParser.IGNORE_ROUNDING_ALWAYS + : PatternStringParser.IGNORE_ROUNDING_NEVER); + propertiesFromTuple(tuple, properties); + actual = Parse + .parseCurrency(tuple.parse, ppos, properties, DecimalFormatSymbols.getInstance(tuple.locale)); + } catch (ParseException e) { + e.printStackTrace(); + return "parse exception: " + e.getMessage(); + } + if (ppos.getIndex() == 0 || actual.getCurrency().getCurrencyCode().equals("XXX")) { + return "Parse failed; got " + actual + ", but expected " + tuple.output; + } + BigDecimal expectedNumber = new BigDecimal(tuple.output); + if (expectedNumber.compareTo(new BigDecimal(actual.getNumber().toString())) != 0) { + return "Wrong number: Expected: " + expectedNumber + ", got: " + actual; + } + String expectedCurrency = tuple.outputCurrency; + if (!expectedCurrency.equals(actual.getCurrency().toString())) { + return "Wrong currency: Expected: " + expectedCurrency + ", got: " + actual; + } + return null; + } + + /** + * Runs a single select test. On success, returns null. On failure, returns the error. This implementation just + * returns null. Subclasses should override. + * + * @param tuple + * contains the parameters of the format test. + */ + @Override + public String select(DataDrivenNumberFormatTestData tuple) { + return null; + } + }; + @Test public void TestDataDrivenICU58() { // Android can't access DecimalFormat_ICU58 for testing (ticket #13283). @@ -595,8 +766,14 @@ public class NumberFormatDataDrivenTest { } @Test - public void TestDataDrivenICU60() { + public void TestDataDrivenICULatest_Format() { DataDrivenNumberFormatTestUtility.runFormatSuiteIncludingKnownFailures( "numberformattestspecification.txt", ICU60); } + + @Test + public void TestDataDrivenICULatest_Other() { + DataDrivenNumberFormatTestUtility.runFormatSuiteIncludingKnownFailures( + "numberformattestspecification.txt", ICU60_Other); + } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java index 54492f41ea..3696bd4367 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java @@ -871,7 +871,7 @@ public class NumberFormatTest extends TestFmwk { new ParseCurrencyItem( "en_GB", "euros4", "4,00\u00A0\u20AC", 4,400, 6,400, "EUR" ), new ParseCurrencyItem( "en_GB", "euros6", "6\u00A0\u20AC", 1, 6, 3, 6, "EUR" ), new ParseCurrencyItem( "en_GB", "euros8", "\u20AC8", 0, 0, 2, 8, "EUR" ), - new ParseCurrencyItem( "en_GB", "dollars4", "US$4", 0, 0, 4, 4, "USD" ), // With CLDR 32/ICU 60, US$4 fails, $4 works, #13368 + new ParseCurrencyItem( "en_GB", "dollars4", "US$4", 0, 0, 4, 4, "USD" ), new ParseCurrencyItem( "fr_FR", "euros4", "4,00\u00A0\u20AC", 6, 4, 6, 4, "EUR" ), new ParseCurrencyItem( "fr_FR", "euros6", "6\u00A0\u20AC", 3, 6, 3, 6, "EUR" ), @@ -907,10 +907,6 @@ public class NumberFormatTest extends TestFmwk { parsePos.setIndex(0); int curExpectPos = item.getCurExpectPos(); - if (currStr.equals("US$4") && logKnownIssue("13368", "en_GB parsing of US$4 fails but $4 works")) { - currStr = "$4"; - curExpectPos = 2; - } CurrencyAmount currAmt = fmt.parseCurrency(currStr, parsePos); if ( parsePos.getIndex() != curExpectPos || (currAmt != null && (currAmt.getNumber().intValue() != item.getCurExpectVal() || currAmt.getCurrency().getCurrencyCode().compareTo(item.getCurExpectCurr()) != 0)) ) {