]> granicus.if.org Git - icu/commitdiff
ICU-13353 Fixed several problems preventing horizontal resource inheritance from...
authorRich Gillam <62772518+richgillam@users.noreply.github.com>
Fri, 6 Aug 2021 21:00:41 +0000 (14:00 -0700)
committerRich Gillam <62772518+richgillam@users.noreply.github.com>
Fri, 10 Sep 2021 19:47:15 +0000 (12:47 -0700)
Java version of a unit test I'd previously only added on the C++ side.

icu4c/source/common/uresbund.cpp
icu4c/source/common/uresimp.h
icu4c/source/i18n/dtitvinf.cpp
icu4c/source/i18n/number_longnames.cpp
icu4c/source/test/intltest/numbertest_api.cpp
icu4c/source/test/intltest/restsnew.cpp
icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateTimeGeneratorTest.java

index 119faf7f293565d89a9297de59305d0f49c23823..a9c6459418cacbc4f6e97863cc3c5b5b76928d13 100644 (file)
@@ -1910,10 +1910,83 @@ static Resource getTableItemByKeyPath(const ResourceData *pResData, Resource tab
   return resource;
 }
 
-U_CAPI UResourceBundle* U_EXPORT2 
-ures_getByKeyWithFallback(const UResourceBundle *resB, 
-                          const char* inKey, 
-                          UResourceBundle *fillIn, 
+static void createPath(const char* origResPath,
+                       int32_t     origResPathLen,
+                       const char* resPath,
+                       int32_t     resPathLen,
+                       const char* inKey,
+                       CharString& path,
+                       UErrorCode* status) {
+    // This is a utility function used by ures_getByKeyWithFallback() below.  This function builds a path from
+    // resPath and inKey, returning the result in `path`.  Originally, this function just cleared `path` and
+    // appended resPath and inKey to it, but that caused problems for horizontal inheritance.
+    //
+    // In normal cases, resPath is the same as origResPath, but if ures_getByKeyWithFallback() has followed an
+    // alias, resPath may be different from origResPath.  Not only may the existing path elements be different,
+    // but resPath may also have MORE path elements than origResPath did.  If it does, those additional path
+    // elements SUPERSEDE the corresponding elements of inKey.  So this code counts the number of elements in
+    // resPath and origResPath and, for each path element in resPath that doesn't have a counterpart in origResPath,
+    // deletes a path element from the beginning of inKey.  The remainder of inKey is then appended to
+    // resPath to form the result.  (We're not using uprv_strchr() here because resPath and origResPath may
+    // not be zero-terminated.)
+    path.clear();
+    const char* key = inKey;
+    if (resPathLen > 0) {
+        path.append(resPath, resPathLen, *status);
+        if (U_SUCCESS(*status)) {
+            const char* resPathLimit = resPath + resPathLen;
+            const char* origResPathLimit = origResPath + origResPathLen;
+            const char* resPathPtr = resPath;
+            const char* origResPathPtr = origResPath;
+            
+            // Remove from the beginning of resPath the number of segments that are contained in origResPath.
+            // If origResPath has MORE segments than resPath, this will leave resPath as the empty string.
+            while (origResPathPtr < origResPathLimit && resPathPtr < resPathLimit) {
+                while (origResPathPtr < origResPathLimit && *origResPathPtr != RES_PATH_SEPARATOR) {
+                    ++origResPathPtr;
+                }
+                if (origResPathPtr < origResPathLimit && *origResPathPtr == RES_PATH_SEPARATOR) {
+                    ++origResPathPtr;
+                }
+                while (resPathPtr < resPathLimit && *resPathPtr != RES_PATH_SEPARATOR) {
+                    ++resPathPtr;
+                }
+                if (resPathPtr < resPathLimit && *resPathPtr == RES_PATH_SEPARATOR) {
+                    ++resPathPtr;
+                }
+            }
+            
+            // New remove from the beginning of `key` the number of segments remaining in resPath.
+            // If resPath has more segments than `key` does, `key` will end up empty.
+            while (resPathPtr < resPathLimit && *key != '\0') {
+                while (resPathPtr < resPathLimit && *resPathPtr != RES_PATH_SEPARATOR) {
+                    ++resPathPtr;
+                }
+                if (resPathPtr < resPathLimit && *resPathPtr == RES_PATH_SEPARATOR) {
+                    ++resPathPtr;
+                }
+                while (*key != '\0' && *key != RES_PATH_SEPARATOR) {
+                    ++key;
+                }
+                if (*key == RES_PATH_SEPARATOR) {
+                    ++key;
+                }
+            }
+        }
+        // Finally, append what's left of `key` to `path`.  What you end up with here is `resPath`, plus
+        // any pieces of `key` that aren't superseded by `resPath`.
+        // Or, to put it another way, calculate <#-segments-in-key> - (<#-segments-in-resPath> - <#-segments-in-origResPath>),
+        // and append that many segments from the end of `key` to `resPath` to produce the result.
+        path.append(key, *status);
+    } else {
+        path.append(inKey, *status);
+    }
+}
+
+U_CAPI UResourceBundle* U_EXPORT2
+ures_getByKeyWithFallback(const UResourceBundle *resB,
+                          const char* inKey,
+                          UResourceBundle *fillIn,
                           UErrorCode *status) {
     Resource res = RES_BOGUS, rootRes = RES_BOGUS;
     UResourceBundle *helper = NULL;
@@ -1928,24 +2001,32 @@ ures_getByKeyWithFallback(const UResourceBundle *resB,
 
     int32_t type = RES_GET_TYPE(resB->fRes);
     if(URES_IS_TABLE(type)) {
+        const char* origResPath = resB->fResPath;
+        int32_t origResPathLen = resB->fResPathLen;
         res = getTableItemByKeyPath(&resB->getResData(), resB->fRes, inKey);
         const char* key = inKey;
+        bool didRootOnce = false;
         if(res == RES_BOGUS) {
             UResourceDataEntry *dataEntry = resB->fData;
             CharString path;
             char *myPath = NULL;
             const char* resPath = resB->fResPath;
             int32_t len = resB->fResPathLen;
-            while(res == RES_BOGUS && dataEntry->fParent != NULL) { /* Otherwise, we'll look in parents */
-                dataEntry = dataEntry->fParent;
+            while(res == RES_BOGUS && (dataEntry->fParent != NULL || !didRootOnce)) { /* Otherwise, we'll look in parents */
+                if (dataEntry->fParent != NULL) {
+                    dataEntry = dataEntry->fParent;
+                } else {
+                    // We can't just stop when we get to a bundle whose fParent is NULL.  That'll work most of the time,
+                    // but if the bundle that the caller passed to us was "root" (which happens in getAllItemsWithFallback(),
+                    // this function will drop right out without doing anything if "root" doesn't contain the exact key path
+                    // specified.  In that case, we need one extra time through this loop to make sure we follow any
+                    // applicable aliases at the root level.
+                    didRootOnce = true;
+                }
                 rootRes = dataEntry->fData.rootRes;
 
                 if(dataEntry->fBogus == U_ZERO_ERROR) {
-                    path.clear();
-                    if (len > 0) {
-                        path.append(resPath, len, *status);
-                    }
-                    path.append(inKey, *status);
+                    createPath(origResPath, origResPathLen, resPath, len, inKey, path, status);
                     if (U_FAILURE(*status)) {
                         ures_close(helper);
                         return fillIn;
@@ -1956,7 +2037,7 @@ ures_getByKeyWithFallback(const UResourceBundle *resB,
                         res = res_findResource(&(dataEntry->fData), rootRes, &myPath, &key);
                         if (RES_GET_TYPE(res) == URES_ALIAS && *myPath) {
                             /* We hit an alias, but we didn't finish following the path. */
-                            helper = init_resb_result(dataEntry, res, NULL, -1, resB, helper, status); 
+                            helper = init_resb_result(dataEntry, res, NULL, -1, resB, helper, status);
                             /*helper = init_resb_result(dataEntry, res, inKey, -1, resB, helper, status);*/
                             if(helper) {
                               dataEntry = helper->fData;
@@ -1982,7 +2063,26 @@ ures_getByKeyWithFallback(const UResourceBundle *resB,
                     *status = U_USING_FALLBACK_WARNING;
                 }
 
-                fillIn = init_resb_result(dataEntry, res, inKey, -1, resB, fillIn, status);
+                fillIn = init_resb_result(dataEntry, res, key, -1, resB, fillIn, status);
+                if (resPath != nullptr) {
+                    createPath(origResPath, origResPathLen, resPath, len, inKey, path, status);
+                } else {
+                    const char* separator = nullptr;
+                    if (fillIn->fResPath != nullptr) {
+                        separator = uprv_strchr(fillIn->fResPath, RES_PATH_SEPARATOR);
+                    }
+                    if (separator != nullptr && separator[1] != '\0') {
+                        createPath(origResPath, origResPathLen, fillIn->fResPath,
+                                   static_cast<int32_t>(uprv_strlen(fillIn->fResPath)), inKey, path, status);
+                    } else {
+                        createPath(origResPath, origResPathLen, "", 0, inKey, path, status);
+                    }
+                }
+                ures_freeResPath(fillIn);
+                ures_appendResPath(fillIn, path.data(), path.length(), status);
+                if(fillIn->fResPath[fillIn->fResPathLen-1] != RES_PATH_SEPARATOR) {
+                    ures_appendResPath(fillIn, RES_PATH_SEPARATOR_S, 1, status);
+                }
             } else {
                 *status = U_MISSING_RESOURCE_ERROR;
             }
@@ -2001,8 +2101,7 @@ namespace {
 
 void getAllItemsWithFallback(
         const UResourceBundle *bundle, ResourceDataValue &value,
-        ResourceSink &sink,
-        UErrorCode &errorCode) {
+        ResourceSink &sink, UErrorCode &errorCode) {
     if (U_FAILURE(errorCode)) { return; }
     // We recursively enumerate child-first,
     // only storing parent items in the absence of child items.
index 399abcfe86835067f6558cc94ed063fdf32e3697..4ac09bd8c4d998221431a64fb12d652883ffb334 100644 (file)
@@ -264,7 +264,6 @@ ures_getByKeyWithFallback(const UResourceBundle *resB,
                           UResourceBundle *fillIn, 
                           UErrorCode *status);
 
-
 /**
  * Get a String with multi-level fallback. Normally only the top level resources will
  * fallback to its parent. This performs fallback on subresources. For example, when a table
index cd3ef6a57b0b6981ac1d8c9150486abca06861cc..6052894b5869cded21d2e2002a3ba88f8c680c13 100644 (file)
@@ -50,7 +50,6 @@ U_NAMESPACE_BEGIN
 UOBJECT_DEFINE_RTTI_IMPLEMENTATION(DateIntervalInfo)
 
 static const char gCalendarTag[]="calendar";
-static const char gGenericTag[]="generic";
 static const char gGregorianTag[]="gregorian";
 static const char gIntervalDateTimePatternTag[]="intervalFormats";
 static const char gFallbackPatternTag[]="fallback";
@@ -435,23 +434,6 @@ DateIntervalInfo::initializeData(const Locale& locale, UErrorCode& status)
         if ( U_SUCCESS(status) ) {
             resStr = ures_getStringByKeyWithFallback(itvDtPtnResource, gFallbackPatternTag,
                                                      &resStrLen, &status);
-            if ( U_FAILURE(status) ) {
-                // Try to find "fallback" from "generic" to work around the bug in
-                // ures_getByKeyWithFallback
-                UErrorCode localStatus = U_ZERO_ERROR;
-                UResourceBundle *genericCalBundle =
-                    ures_getByKeyWithFallback(calBundle, gGenericTag, nullptr, &localStatus);
-                UResourceBundle *genericItvDtPtnResource =
-                    ures_getByKeyWithFallback(
-                        genericCalBundle, gIntervalDateTimePatternTag, nullptr, &localStatus);
-                resStr = ures_getStringByKeyWithFallback(
-                    genericItvDtPtnResource, gFallbackPatternTag, &resStrLen, &localStatus);
-                ures_close(genericItvDtPtnResource);
-                ures_close(genericCalBundle);
-                if ( U_SUCCESS(localStatus) ) {
-                    status = U_USING_FALLBACK_WARNING;;
-                }
-            }
         }
 
         if ( U_SUCCESS(status) && (resStr != nullptr)) {
index 08078c474734be2c5e46aed230a5a4ea2e638348..25432135b35ca840c3bc159e9cecfc4bddad7e85 100644 (file)
@@ -259,20 +259,16 @@ class InflectedPluralSink : public ResourceSink {
 
     // See ResourceSink::put().
     void put(const char *key, ResourceValue &value, UBool /*noFallback*/, UErrorCode &status) U_OVERRIDE {
-        ResourceTable pluralsTable = value.getTable(status);
+        int32_t pluralIndex = getIndex(key, status);
         if (U_FAILURE(status)) { return; }
-        for (int32_t i = 0; pluralsTable.getKeyAndValue(i, key, value); ++i) {
-            int32_t pluralIndex = getIndex(key, status);
-            if (U_FAILURE(status)) { return; }
-            if (!outArray[pluralIndex].isBogus()) {
-                // We already have a pattern
-                continue;
-            }
-            ResourceTable genderTable = value.getTable(status);
-            ResourceTable caseTable; // This instance has to outlive `value`
-            if (loadForPluralForm(genderTable, caseTable, value, status)) {
-                outArray[pluralIndex] = value.getUnicodeString(status);
-            }
+        if (!outArray[pluralIndex].isBogus()) {
+            // We already have a pattern
+            return;
+        }
+        ResourceTable genderTable = value.getTable(status);
+        ResourceTable caseTable; // This instance has to outlive `value`
+        if (loadForPluralForm(genderTable, caseTable, value, status)) {
+            outArray[pluralIndex] = value.getUnicodeString(status);
         }
     }
 
@@ -370,18 +366,11 @@ void getInflectedMeasureData(StringPiece subKey,
     key.append(subKey, status);
 
     UErrorCode localStatus = status;
-    ures_getAllItemsWithFallback(unitsBundle.getAlias(), key.data(), sink, localStatus);
+    ures_getAllChildrenWithFallback(unitsBundle.getAlias(), key.data(), sink, localStatus);
     if (width == UNUM_UNIT_WIDTH_SHORT) {
         status = localStatus;
         return;
     }
-
-    // TODO(ICU-13353): The fallback to short does not work in ICU4C.
-    // Manually fall back to short (this is done automatically in Java).
-    key.clear();
-    key.append("unitsShort/", status);
-    key.append(subKey, status);
-    ures_getAllItemsWithFallback(unitsBundle.getAlias(), key.data(), sink, status);
 }
 
 class PluralTableSink : public ResourceSink {
@@ -396,20 +385,16 @@ class PluralTableSink : public ResourceSink {
     }
 
     void put(const char *key, ResourceValue &value, UBool /*noFallback*/, UErrorCode &status) U_OVERRIDE {
-        ResourceTable pluralsTable = value.getTable(status);
+        if (uprv_strcmp(key, "case") == 0) {
+            return;
+        }
+        int32_t index = getIndex(key, status);
         if (U_FAILURE(status)) { return; }
-        for (int32_t i = 0; pluralsTable.getKeyAndValue(i, key, value); ++i) {
-            if (uprv_strcmp(key, "case") == 0) {
-                continue;
-            }
-            int32_t index = getIndex(key, status);
-            if (U_FAILURE(status)) { return; }
-            if (!outArray[index].isBogus()) {
-                continue;
-            }
-            outArray[index] = value.getUnicodeString(status);
-            if (U_FAILURE(status)) { return; }
+        if (!outArray[index].isBogus()) {
+            return;
         }
+        outArray[index] = value.getUnicodeString(status);
+        if (U_FAILURE(status)) { return; }
     }
 
   private:
@@ -490,7 +475,7 @@ void getMeasureData(const Locale &locale,
         // getInflectedMeasureData after homogenizing data format? Find a unit
         // test case that demonstrates the incorrect fallback logic (via
         // regional variant of an inflected language?)
-        ures_getAllItemsWithFallback(unitsBundle.getAlias(), caseKey.data(), sink, localStatus);
+        ures_getAllChildrenWithFallback(unitsBundle.getAlias(), caseKey.data(), sink, localStatus);
     }
 
     // TODO(icu-units#138): our fallback logic is not spec-compliant: we
@@ -499,20 +484,13 @@ void getMeasureData(const Locale &locale,
     // either get the spec changed, or add unit tests that warn us if
     // case="nominative" data differs from no-case data?
     UErrorCode localStatus = U_ZERO_ERROR;
-    ures_getAllItemsWithFallback(unitsBundle.getAlias(), key.data(), sink, localStatus);
+    ures_getAllChildrenWithFallback(unitsBundle.getAlias(), key.data(), sink, localStatus);
     if (width == UNUM_UNIT_WIDTH_SHORT) {
         if (U_FAILURE(localStatus)) {
             status = localStatus;
         }
         return;
     }
-
-    // TODO(ICU-13353): The fallback to short does not work in ICU4C.
-    // Manually fall back to short (this is done automatically in Java).
-    key.clear();
-    key.append("unitsShort", status);
-    key.append(subKey, status);
-    ures_getAllItemsWithFallback(unitsBundle.getAlias(), key.data(), sink, status);
 }
 
 // NOTE: outArray MUST have a length of at least ARRAY_LENGTH.
@@ -523,7 +501,7 @@ void getCurrencyLongNameData(const Locale &locale, const CurrencyUnit &currency,
     PluralTableSink sink(outArray);
     LocalUResourceBundlePointer unitsBundle(ures_open(U_ICUDATA_CURR, locale.getName(), &status));
     if (U_FAILURE(status)) { return; }
-    ures_getAllItemsWithFallback(unitsBundle.getAlias(), "CurrencyUnitPatterns", sink, status);
+    ures_getAllChildrenWithFallback(unitsBundle.getAlias(), "CurrencyUnitPatterns", sink, status);
     if (U_FAILURE(status)) { return; }
     for (int32_t i = 0; i < StandardPlural::Form::COUNT; i++) {
         UnicodeString &pattern = outArray[i];
index d5c539372c58ca70f7c168c4f3c3fc6228371b3c..5117906b7e216f65f2fa58a665f6da297ef0823b 100644 (file)
@@ -1232,7 +1232,7 @@ void NumberFormatterApiTest::unitArbitraryMeasureUnits() {
               .locale("en-ZA");
     lnf.operator=(lnf);  // self-assignment should be a no-op
     lnf.formatInt(1, status);
-    status.expectErrorAndReset(U_RESOURCE_TYPE_MISMATCH);
+    status.expectErrorAndReset(U_INTERNAL_PROGRAM_ERROR);
 
     assertFormatSingle(
             u"kibijoule-foot-per-cubic-gigafurlong-square-second unit-width-full-name",
index 67b9f556326b94dff301ec1b89f3010fe7f877f0..a3e739a7e3e8482d0517a9f6504caa0eb4c7625c 100644 (file)
@@ -1399,22 +1399,10 @@ void NewResourceBundleTest::TestFilter() {
     }
 }
 
-/*
- * The following test for ICU-20706 has infinite loops on certain inputs for
- * locales and calendars.  In order to unblock the build (ICU-21055), those
- * specific values are temporarily removed.
- * The issue of the infinite loops and its blocking dependencies were captured
- * in ICU-21080.
- */
-
 void NewResourceBundleTest::TestIntervalAliasFallbacks() {
     const char* locales[] = {
-        // Thee will not cause infinity loop
         "en",
         "ja",
-
-        // These will cause infinity loop
-#if 0
         "fr_CA",
         "en_150",
         "es_419",
@@ -1426,15 +1414,10 @@ void NewResourceBundleTest::TestIntervalAliasFallbacks() {
         "zh_Hant",
         "zh_Hant_TW",
         "zh_TW",
-#endif
     };
     const char* calendars[] = {
-        // These won't cause infinity loop
         "gregorian",
         "chinese",
-
-        // These will cause infinity loop
-#if 0
         "islamic",
         "islamic-civil",
         "islamic-tbla",
@@ -1443,7 +1426,6 @@ void NewResourceBundleTest::TestIntervalAliasFallbacks() {
         "islamic-rgsa",
         "japanese",
         "roc",
-#endif
     };
 
     for (int lidx = 0; lidx < UPRV_LENGTHOF(locales); lidx++) {
@@ -1458,10 +1440,8 @@ void NewResourceBundleTest::TestIntervalAliasFallbacks() {
             key.append("calendar/", status);
             key.append(calendars[cidx], status);
             key.append("/intervalFormats/fallback", status);
-            if (! logKnownIssue("20400")) {
-                int32_t resStrLen = 0;
-                ures_getStringByKeyWithFallback(rb, key.data(), &resStrLen, &status);
-            }
+            int32_t resStrLen = 0;
+            ures_getStringByKeyWithFallback(rb, key.data(), &resStrLen, &status);
             if (U_FAILURE(status)) {
                 errln("Cannot ures_getStringByKeyWithFallback('%s') on locale %s",
                       key.data(), locales[lidx]);
index c1752c3919d3524c52013ea57755176d54b85f30..b12aadf0bf4441a2db19a87fa0529a8a9c207cf9 100644 (file)
@@ -1854,4 +1854,30 @@ public class DateTimeGeneratorTest extends TestFmwk {
         }
     }
 
+    // Test for ICU-21202: Make sure DateTimePatternGenerator supplies an era field for year formats using the
+    // Buddhist and Japanese calendars for all English-speaking locales.
+    @Test
+    public void testEras() {
+        String[] localeIDs = {
+            "en_US@calendar=japanese",
+            "en_GB@calendar=japanese",
+            "en_150@calendar=japanese",
+            "en_001@calendar=japanese",
+            "en@calendar=japanese",
+            "en_US@calendar=buddhist",
+            "en_GB@calendar=buddhist",
+            "en_150@calendar=buddhist",
+            "en_001@calendar=buddhist",
+            "en@calendar=buddhist",
+        };
+
+        for (String localeID : localeIDs) {
+            DateTimePatternGenerator dtpg = DateTimePatternGenerator.getInstance(new Locale(localeID));
+            String pattern = dtpg.getBestPattern("y");
+
+            if (pattern.indexOf('G') < 0) {
+                errln("missing era field for locale " + localeID);
+            }
+        }
+    }
 }