Merge branch 'feature/errorcode-i18n-simplify' into '5.5.12'

gitlab · gitlab · commit d0ccc44a7b8c · 2026-03-18T01:48:10.000Z
&lt;feature&gt;[errorcode]: simplify i18n — guarantee message is never null

See merge request zstackio/zstack!9354
diff --git a/core/src/main/java/org/zstack/core/Platform.java b/core/src/main/java/org/zstack/core/Platform.java
@@ -17,6 +17,7 @@
 import org.zstack.core.db.DatabaseGlobalProperty;
 import org.zstack.core.encrypt.EncryptRSA;
 import org.zstack.core.errorcode.ErrorFacade;
+import org.zstack.core.errorcode.GlobalErrorCodeI18nService;
 import org.zstack.core.propertyvalidator.ValidatorTool;
 import org.zstack.core.search.SearchGlobalProperty;
 import org.zstack.core.statemachine.StateMachine;
@@ -988,6 +989,23 @@ public static ErrorCode err(String globalErrorCode, Enum errCode, ErrorCode caus
                     .toArray(String[]::new));
         }
 
+        // populate message at creation time with default locale;
+        // RestServer will override with client's Accept-Language if different
+        try {
+            ComponentLoader currentLoader = loader;
+            if (currentLoader != null) {
+                GlobalErrorCodeI18nService i18nService = currentLoader.getComponent(GlobalErrorCodeI18nService.class);
+                if (i18nService != null) {
+                    i18nService.localizeErrorCode(result, org.zstack.core.errorcode.LocaleUtils.DEFAULT_LOCALE);
+                }
+            }
+        } catch (Exception e) {
+            // i18n service not initialized during early startup
+        }
+        if (result.getMessage() == null) {
+            result.setMessage(details != null ? details : result.getDescription());
+        }
+
         return result;
     }
 
diff --git a/core/src/main/java/org/zstack/core/errorcode/GlobalErrorCodeI18nServiceImpl.java b/core/src/main/java/org/zstack/core/errorcode/GlobalErrorCodeI18nServiceImpl.java
@@ -56,7 +56,7 @@ private void loadAllJsonFiles() {
                     String content = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8);
                     @SuppressWarnings("unchecked")
                     Map<String, String> messages = JSONObjectUtil.toObject(content, LinkedHashMap.class);
-                    localeMessages.put(locale, messages);
+                    localeMessages.put(locale, Collections.unmodifiableMap(messages));
                     logger.info(String.format("loaded %d i18n error messages for locale [%s]",
                             messages.size(), locale));
                 } catch (Exception e) {
@@ -125,26 +125,34 @@ private String formatTemplate(String template, String[] formatArgs) {
 
     @Override
     public void localizeErrorCode(ErrorCode error, String locale) {
-        if (error == null || locale == null) {
+        if (error == null) {
             return;
         }
 
+        String resolvedLocale = locale != null ? locale : LocaleUtils.DEFAULT_LOCALE;
+
         if (error.getGlobalErrorCode() != null) {
-            String message = getLocalizedMessage(error.getGlobalErrorCode(), locale, error.getFormatArgs());
+            String message = getLocalizedMessage(error.getGlobalErrorCode(), resolvedLocale, error.getFormatArgs());
             if (message != null) {
                 error.setMessage(message);
             }
         }
 
+        // guarantee: message is never null
+        if (error.getMessage() == null) {
+            String fallback = error.getDetails() != null ? error.getDetails() : error.getDescription();
+            error.setMessage(fallback != null ? fallback : (error.getCode() != null ? error.getCode() : ""));
+        }
+
         if (error.getCause() != null) {
-            localizeErrorCode(error.getCause(), locale);
+            localizeErrorCode(error.getCause(), resolvedLocale);
         }
 
         if (error instanceof ErrorCodeList) {
             List<ErrorCode> causes = ((ErrorCodeList) error).getCauses();
             if (causes != null) {
                 for (ErrorCode cause : causes) {
-                    localizeErrorCode(cause, locale);
+                    localizeErrorCode(cause, resolvedLocale);
                 }
             }
         }
diff --git a/core/src/main/java/org/zstack/core/errorcode/LocaleUtils.java b/core/src/main/java/org/zstack/core/errorcode/LocaleUtils.java
@@ -30,6 +30,9 @@ public class LocaleUtils {
      * Parse Accept-Language header and return the best matching locale key
      * from the set of available locales.
      *
+     * Uses {@link Locale.LanguageRange#parse(String)} for RFC 7231 compliant
+     * parsing with proper q-value priority sorting.
+     *
      * @param acceptLanguage the Accept-Language header value
      * @param availableLocales the set of locale keys loaded from JSON files
      * @return the best matching locale key, or en_US as fallback
@@ -39,18 +42,27 @@ public static String resolveLocale(String acceptLanguage, Set<String> availableL
             return DEFAULT_LOCALE;
         }
 
-        List<LocaleEntry> entries = parseAcceptLanguage(acceptLanguage);
-        for (LocaleEntry entry : entries) {
-            if (entry.quality <= 0) {
+        List<Locale.LanguageRange> ranges;
+        try {
+            ranges = Locale.LanguageRange.parse(acceptLanguage);
+        } catch (IllegalArgumentException e) {
+            logger.debug(String.format("failed to parse Accept-Language [%s]: %s", acceptLanguage, e.getMessage()));
+            return DEFAULT_LOCALE;
+        }
+
+        // ranges are already sorted by q-value descending
+        for (Locale.LanguageRange range : ranges) {
+            if (range.getWeight() <= 0) {
                 continue;
             }
 
-            String normalized = normalizeTag(entry.tag);
+            String tag = range.getRange();
+            String normalized = normalizeTag(tag);
             if (availableLocales.contains(normalized)) {
                 return normalized;
             }
 
-            String lang = entry.tag.split("[-_]")[0].toLowerCase();
+            String lang = tag.split("[-_]")[0].toLowerCase();
             String mapped = LANGUAGE_TO_LOCALE.get(lang);
             if (mapped != null && availableLocales.contains(mapped)) {
                 return mapped;
@@ -78,48 +90,4 @@ static String normalizeTag(String tag) {
         }
         return tag;
     }
-
-    private static List<LocaleEntry> parseAcceptLanguage(String header) {
-        List<LocaleEntry> entries = new ArrayList<>();
-        String[] parts = header.split(",");
-        for (String part : parts) {
-            part = part.trim();
-            if (part.isEmpty()) {
-                continue;
-            }
-            String[] tagAndParams = part.split(";");
-            if (tagAndParams.length == 0) {
-                continue;
-            }
-            String tag = tagAndParams[0].trim();
-            if (tag.isEmpty()) {
-                continue;
-            }
-            double quality = 1.0;
-            for (int i = 1; i < tagAndParams.length; i++) {
-                String param = tagAndParams[i].trim();
-                if (param.startsWith("q=")) {
-                    try {
-                        quality = Double.parseDouble(param.substring(2).trim());
-                    } catch (NumberFormatException e) {
-                        logger.debug(String.format("failed to parse quality value [%s]: %s", param, e.getMessage()));
-                        quality = 0;
-                    }
-                }
-            }
-            entries.add(new LocaleEntry(tag, quality));
-        }
-        entries.sort((a, b) -> Double.compare(b.quality, a.quality));
-        return entries;
-    }
-
-    private static class LocaleEntry {
-        final String tag;
-        final double quality;
-
-        LocaleEntry(String tag, double quality) {
-            this.tag = tag;
-            this.quality = quality;
-        }
-    }
 }
diff --git a/rest/src/main/java/org/zstack/rest/RestServer.java b/rest/src/main/java/org/zstack/rest/RestServer.java
@@ -408,8 +408,11 @@ private void callWebHook(RequestData d) throws IllegalAccessException, NoSuchMet
 
             writeResponse(response, w, ret.getResult());
         } else {
+            // localize with webhook caller's locale (message already populated by Platform.err)
             String locale = resolveLocale();
-            i18nService.localizeErrorCode(evt.getError(), locale);
+            if (!LocaleUtils.DEFAULT_LOCALE.equals(locale)) {
+                i18nService.localizeErrorCode(evt.getError(), locale);
+            }
             response.setError(evt.getError());
         }
 
@@ -917,14 +920,18 @@ private void handleJobQuery(HttpServletRequest req, HttpServletResponse rsp) thr
             writeResponse(response, w, ret.getResult());
             sendResponse(HttpStatus.OK.value(), response, rsp);
         } else {
-            String locale = resolveLocaleFromRequest(req);
-            i18nService.localizeErrorCode(evt.getError(), locale);
             response.setError(evt.getError());
             sendResponse(HttpStatus.SERVICE_UNAVAILABLE.value(), response, rsp);
         }
     }
 
     private void sendResponse(int statusCode, ApiResponse response, HttpServletResponse rsp) throws IOException {
+        // centralized localization: override message with client's preferred locale
+        if (response.getError() != null) {
+            String locale = resolveLocale();
+            i18nService.localizeErrorCode(response.getError(), locale);
+        }
+
         RequestInfo info = requestInfo.get();
         if (requestLogger.isTraceEnabled() && needLog(info)) {
             String body = CloudBusGson.toJson(response);
@@ -1428,18 +1435,16 @@ private String resolveLocale() {
         return LocaleUtils.resolveLocale(acceptLanguage, i18nService.getAvailableLocales());
     }
 
-    private String resolveLocaleFromRequest(HttpServletRequest req) {
-        String acceptLanguage = req.getHeader("Accept-Language");
-        return LocaleUtils.resolveLocale(acceptLanguage, i18nService.getAvailableLocales());
-    }
-
     private void sendReplyResponse(MessageReply reply, Api api, HttpServletResponse rsp) throws IOException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
         ApiResponse response = new ApiResponse();
 
         if (!reply.isSuccess()) {
             String locale = resolveLocale();
             i18nService.localizeErrorCode(reply.getError(), locale);
             response.setError(reply.getError());
+            // use JSONObjectUtil (which disables HTML escaping) to keep the same
+            // serialization behavior as before; CloudBusGson.httpGson escapes '\'' to
+            // '\u0027' which breaks SDK-side string assertions (ZSTAC-71075 etc.)
             sendResponse(HttpStatus.SERVICE_UNAVAILABLE.value(), JSONObjectUtil.toJsonString(response), rsp);
             return;
         }
diff --git a/test/src/test/groovy/org/zstack/test/integration/core/ErrorCodeI18nCase.groovy b/test/src/test/groovy/org/zstack/test/integration/core/ErrorCodeI18nCase.groovy
@@ -1,7 +1,9 @@
 package org.zstack.test.integration.core
 
+import org.zstack.core.errorcode.GlobalErrorCodeI18nServiceImpl
 import org.zstack.core.errorcode.LocaleUtils
 import org.zstack.header.errorcode.ErrorCode
+import org.zstack.header.errorcode.ErrorCodeList
 import org.zstack.testlib.SubCase
 
 class ErrorCodeI18nCase extends SubCase {
@@ -24,8 +26,12 @@ class ErrorCodeI18nCase extends SubCase {
         testLocaleUtilsNoMatch()
         testLocaleUtilsCaseInsensitive()
         testLocaleUtilsMalformedHeader()
+        testLocaleUtilsComplexBrowserHeader()
         testErrorCodeCopyConstructor()
         testErrorCodeCopyConstructorWithNulls()
+        testMessageGuaranteeFallbackToDetails()
+        testMessageGuaranteeFallbackToDescription()
+        testMessageGuaranteeOnCauseChain()
     }
 
     @Override
@@ -78,6 +84,14 @@ class ErrorCodeI18nCase extends SubCase {
         assert LocaleUtils.resolveLocale(";;;,,,", available) == "en_US"
     }
 
+    void testLocaleUtilsComplexBrowserHeader() {
+        def available = ["zh_CN", "en_US"] as Set
+        // real Chrome header: zh-CN,zh;q=0.9,en;q=0.8,en-GB;q=0.7,en-US;q=0.6
+        assert LocaleUtils.resolveLocale("zh-CN,zh;q=0.9,en;q=0.8,en-GB;q=0.7,en-US;q=0.6", available) == "zh_CN"
+        // q=0 means "not acceptable" — should be skipped
+        assert LocaleUtils.resolveLocale("zh-CN;q=0,en-US;q=1.0", available) == "en_US"
+    }
+
     // ---- ErrorCode copy constructor ----
 
     void testErrorCodeCopyConstructor() {
@@ -127,4 +141,42 @@ class ErrorCodeI18nCase extends SubCase {
         assert copy.formatArgs == null
     }
 
+    // ---- message guarantee (localizeErrorCode always populates message) ----
+
+    void testMessageGuaranteeFallbackToDetails() {
+        def i18n = new GlobalErrorCodeI18nServiceImpl()
+        // no i18n JSON loaded, so getLocalizedMessage returns null
+        // localizeErrorCode should fall back to details
+        def error = new ErrorCode("SYS.1000", "System Error", "disk full on /dev/sda1")
+        assert error.getMessage() == null
+
+        i18n.localizeErrorCode(error, "en_US")
+
+        assert error.getMessage() == "disk full on /dev/sda1"
+    }
+
+    void testMessageGuaranteeFallbackToDescription() {
+        def i18n = new GlobalErrorCodeI18nServiceImpl()
+        // no details, should fall back to description
+        def error = new ErrorCode("SYS.1000", "System Error")
+        assert error.getMessage() == null
+
+        i18n.localizeErrorCode(error, "en_US")
+
+        assert error.getMessage() == "System Error"
+    }
+
+    void testMessageGuaranteeOnCauseChain() {
+        def i18n = new GlobalErrorCodeI18nServiceImpl()
+        def root = new ErrorCode("INTERNAL.1001", "Internal Error", "root cause detail")
+        def mid = new ErrorCode("SYS.1000", "System Error")
+        mid.setCause(root)
+
+        i18n.localizeErrorCode(mid, "en_US")
+
+        // both should have message populated
+        assert mid.getMessage() == "System Error"
+        assert root.getMessage() == "root cause detail"
+    }
+
 }
diff --git a/test/src/test/java/org/zstack/test/core/errorcode/TestGlobalErrorCodeI18n.java b/test/src/test/java/org/zstack/test/core/errorcode/TestGlobalErrorCodeI18n.java
@@ -128,8 +128,27 @@ public void testLocalizeErrorCodeList() {
     @Test
     public void testNoGlobalErrorCode() {
         ErrorCode error = new ErrorCode("SYS.1000", "test error");
-        // no globalErrorCode set
+        // no globalErrorCode set — message should fall back to description
         i18nService.localizeErrorCode(error, "zh_CN");
-        Assert.assertNull("message should remain null", error.getMessage());
+        Assert.assertEquals("message should fall back to description",
+                "test error", error.getMessage());
+    }
+
+    @Test
+    public void testMessageGuaranteeFallbackToDetails() {
+        ErrorCode error = new ErrorCode("SYS.1000", "System Error", "disk full on /dev/sda1");
+        i18nService.localizeErrorCode(error, "en_US");
+        Assert.assertEquals("message should fall back to details",
+                "disk full on /dev/sda1", error.getMessage());
+    }
+
+    @Test
+    public void testMessageNeverNull() {
+        ErrorCode error = new ErrorCode("SYS.1000", "System Error");
+        error.setDetails(null);
+        i18nService.localizeErrorCode(error, "en_US");
+        Assert.assertNotNull("message must never be null after localizeErrorCode",
+                error.getMessage());
+        Assert.assertEquals("System Error", error.getMessage());
     }
 }
