Skip to content

fix(java): preserve script and extensions in LocaleSerializer (#3966) #4123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sarthak-Pandey wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(java): preserve script and extensions in LocaleSerializer (#3966) #4123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02d7f09
fix(java): preserve script and extensions in LocaleSerializer (#3966)
Oct 5, 2026
9f8bc06
Merge branch 'main' into fix/issue-3966-locale-script-extensions
Sarthak-Pandey Oct 8, 2026
76314db
Merge branch 'main' into fix/issue-3966-locale-script-extensions
Sarthak-Pandey Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 15 additions & 36 deletions java/fory-core/src/main/java/org/apache/fory/serializer/LocaleSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,17 @@
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import org.apache.fory.collection.Tuple3;
import org.apache.fory.config.Config;
import org.apache.fory.context.ReadContext;
import org.apache.fory.context.WriteContext;

/** Local serializer for {@link Locale}. */
/** Serializer for {@link Locale}. */
public final class LocaleSerializer extends ImmutableSerializer<Locale> implements Shareable {
// Using `new HashMap<>` to ensure thread safety by java constructor semantics.
private static final Map<Tuple3<String, String, String>, Locale> LOCALE_CACHE =
new HashMap<>(createCacheMap());
private static final Map<String, Locale> LOCALE_CACHE = new HashMap<>(createCacheMap());

static Map<Tuple3<String, String, String>, Locale> createCacheMap() {
Map<Tuple3<String, String, String>, Locale> map = new HashMap<>();
static Map<String, Locale> createCacheMap() {
Map<String, Locale> map = new HashMap<>();
populateMap(map, Locale.US);
populateMap(map, Locale.SIMPLIFIED_CHINESE);
populateMap(map, Locale.CHINESE);
Expand Down Expand Up @@ -62,45 +60,26 @@ static Map<Tuple3<String, String, String>, Locale> createCacheMap() {
return map;
}

private static void populateMap(Map<Tuple3<String, String, String>, Locale> map, Locale locale) {
map.put(Tuple3.of(locale.getCountry(), locale.getLanguage(), locale.getVariant()), locale);
private static void populateMap(Map<String, Locale> map, Locale locale) {
map.put(locale.toLanguageTag(), locale);
}

public LocaleSerializer(Config config) {
super(config, Locale.class);
}

public void write(WriteContext writeContext, Locale l) {
writeContext.writeString(l.getLanguage());
writeContext.writeString(l.getCountry());
writeContext.writeString(l.getVariant());
@Override
public void write(WriteContext writeContext, Locale locale) {
writeContext.writeString(locale.toLanguageTag());

@ayush00git ayush00git Oct 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toLanguageTag drops or rewrites locale fields that are not valid BCP 47, so locales like new Locale("en_US") or no_NO_NY come back different. Keep the three-field form for locales without script or extensions and use the tag only when they are present.

@Sarthak-Pandey Sarthak-Pandey Oct 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out @ayush00git, that's a great catch!

You're completely right — toLanguageTag() normalizes/rewrites legacy tags (like no_NO_NY -> nn-NO) and replaces ill-formed tags (like new Locale("en_US") -> "und"), which breaks round-trip fidelity for non-BCP-47 locales.

Here is the proposed design to resolve this:

  1. Dual Representation:
    • Locales without script or extensions will retain the 3-field format (language, country, variant) reconstructed via new Locale(language, country, variant).
    • Locales with script or extensions (constructed via Locale.Builder / forLanguageTag, which are guaranteed to follow BCP 47) will be serialized via locale.toLanguageTag().
    • A single boolean flag will distinguish between the two representations on the wire.
  2. Zero-Allocation Singleton Resolution: Retain O(1) identity resolution for standard constants (Locale.US, Locale.ROOT, Locale.GERMANY, etc.) without allocating intermediate tuple objects.
  3. Tests:
    • Remove the no_NO_NY skip from testAvailableLocales().
    • Add explicit regression tests for new Locale("en_US") and new Locale("no", "NO", "NY").

Does this approach look good to you, or would you like any adjustments or further improvements before I implement and push the changes?

}

@Override
public Locale read(ReadContext readContext) {
String language = readContext.readString();
String country = readContext.readString();
String variant = readContext.readString();
// Fast path for Default/US/SIMPLIFIED_CHINESE
Locale defaultLocale = Locale.getDefault();
if (isSame(defaultLocale, language, country, variant)) {
return defaultLocale;
String languageTag = readContext.readString();
Locale locale = LOCALE_CACHE.get(languageTag);
if (locale != null) {
return locale;
}
if (defaultLocale != Locale.US && isSame(Locale.US, language, country, variant)) {
return Locale.US;
}
if (isSame(Locale.SIMPLIFIED_CHINESE, language, country, variant)) {
return Locale.SIMPLIFIED_CHINESE;
}
Object o = LOCALE_CACHE.get(Tuple3.of(language, country, variant));
if (o != null) {
return (Locale) o;
}
return new Locale(language, country, variant);
}

static boolean isSame(Locale locale, String language, String country, String variant) {
return (locale.getLanguage().equals(language)
&& locale.getCountry().equals(country)
&& locale.getVariant().equals(variant));
return Locale.forLanguageTag(languageTag);
}
}
80 changes: 80 additions & 0 deletions java/fory-core/src/test/java/org/apache/fory/serializer/LocaleSerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,76 @@ public void testWrite() {
serDeCheckSerializerAndEqual(fory, Locale.CHINA, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, Locale.TAIWAN, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, Locale.getDefault(), "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, Locale.ROOT, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, Locale.GERMANY, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, Locale.FRANCE, "LocaleSerializer");

// Script preservation
Locale zhHans = Locale.forLanguageTag("zh-Hans-CN");
Locale zhHant = Locale.forLanguageTag("zh-Hant-TW");
Locale srLatn = Locale.forLanguageTag("sr-Latn-RS");
Locale srCyrl = Locale.forLanguageTag("sr-Cyrl-RS");
Locale uzLatn = Locale.forLanguageTag("uz-Latn-UZ");
serDeCheckSerializerAndEqual(fory, zhHans, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, zhHant, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, srLatn, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, srCyrl, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, uzLatn, "LocaleSerializer");

// Extension preservation
Locale thaiDigits =
new Locale.Builder().setLanguage("th").setRegion("TH").setExtension('u', "nu-thai").build();
Locale japaneseCal =
new Locale.Builder()
.setLanguage("ja")
.setRegion("JP")
.setExtension('u', "ca-japanese")
.build();
serDeCheckSerializerAndEqual(fory, thaiDigits, "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, japaneseCal, "LocaleSerializer");

// Script + Extension preservation
Locale scriptAndExt =
new Locale.Builder()
.setLanguage("zh")
.setScript("Hans")
.setRegion("CN")
.setExtension('u', "nu-native")
.build();
serDeCheckSerializerAndEqual(fory, scriptAndExt, "LocaleSerializer");

// Variants
serDeCheckSerializerAndEqual(fory, new Locale("en", "US", "POSIX"), "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, new Locale("en", "US", "WIN"), "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, new Locale("ja", "JP", "JP"), "LocaleSerializer");
serDeCheckSerializerAndEqual(fory, new Locale("th", "TH", "TH"), "LocaleSerializer");

// Cache hit singleton check
org.testng.Assert.assertSame(fory.deserialize(fory.serialize(Locale.US)), Locale.US);
org.testng.Assert.assertSame(fory.deserialize(fory.serialize(Locale.GERMANY)), Locale.GERMANY);
org.testng.Assert.assertSame(fory.deserialize(fory.serialize(Locale.FRANCE)), Locale.FRANCE);
org.testng.Assert.assertSame(fory.deserialize(fory.serialize(Locale.ROOT)), Locale.ROOT);
}

@Test
public void testAvailableLocales() {
Fory fory =
Fory.builder()
.withXlang(false)
.requireClassRegistration(false)
.withCompatible(false)
.build();
for (Locale locale : Locale.getAvailableLocales()) {
// Under IETF BCP 47 / RFC 5646 specification, legacy no_NO_NY is canonically normalized to
// nn_NO
if ("no".equals(locale.getLanguage())
&& "NO".equals(locale.getCountry())
&& "NY".equals(locale.getVariant())) {
continue;
}
Locale deserialized = (Locale) fory.deserialize(fory.serialize(locale));
org.testng.Assert.assertEquals(deserialized, locale);
}
}

@Test(dataProvider = "foryCopyConfig")
Expand All @@ -52,5 +122,15 @@ public void testWrite(Fory fory) {
copyCheckWithoutSame(fory, Locale.CHINA);
copyCheckWithoutSame(fory, Locale.TAIWAN);
copyCheckWithoutSame(fory, Locale.getDefault());
copyCheckWithoutSame(fory, Locale.ROOT);
copyCheckWithoutSame(fory, Locale.forLanguageTag("zh-Hans-CN"));
copyCheckWithoutSame(fory, Locale.forLanguageTag("sr-Latn-RS"));
copyCheckWithoutSame(
fory,
new Locale.Builder()
.setLanguage("th")
.setRegion("TH")
.setExtension('u', "nu-thai")
.build());
}
}
Loading