Parse Locales with scripts properly in LocaleTypeDescriptor.fromString

Locales with scripts (e.g. zh-Hant-HK) were not parsed properly by LocaleTypeDescriptor,
causing the script part to get lost.

Author: kilink

hibernate-core/src/main/java/org/hibernate/type/descriptor/java/LocaleTypeDescriptor.java

@@ -41,53 +41,54 @@ public String toString(Locale value) {
 	}
 
 	public Locale fromString(String string) {
-		// TODO : Ultimately switch to Locale.Builder for this. However, Locale.Builder is Java 7
-
 		if ( string == null ) {
 			return null;
 		}
 
-		if( string.isEmpty() ) {
+		if ( string.isEmpty() ) {
 			return Locale.ROOT;
 		}
 
-		boolean separatorFound = false;
-		int position = 0;
-		char[] chars = string.toCharArray();
-
-		for ( int i = 0; i < chars.length; i++ ) {
-			// We just look for separators
-			if ( chars[i] == '_' ) {
-				if ( !separatorFound ) {
-					// On the first separator we know that we have at least a language
-					string = new String( chars, position, i - position );
-					position = i + 1;
-				}
-				else {
-					// On the second separator we have to check whether there are more chars available for variant
-					if ( chars.length > i + 1 ) {
-						// There is a variant so add it to the constructor
-						return new Locale( string, new String( chars, position, i - position ), new String( chars,
-								i + 1, chars.length - i - 1 ) );
-					}
-					else {
-						// No variant given, we just have language and country
-						return new Locale( string, new String( chars, position, i - position ), "" );
-					}
-				}
-
-				separatorFound = true;
-			}
+		// Locale.toString returns a String representation matching the following pattern:
+		// language + "_" + country + "_" + (variant + "_#" | "#") + script + "_" + extensions
+		final Locale.Builder builder = new Locale.Builder();
+		final String[] parts = string.split("#");
+		final String[] langCountryVariant = parts[0].split("_");
+
+		if ( !langCountryVariant[0].isEmpty() ) {
+			builder.setLanguage( langCountryVariant[0] );
 		}
 
-		if ( !separatorFound ) {
-			// No separator found, there is only a language
-			return new Locale( string );
+		if ( langCountryVariant.length > 1 && !langCountryVariant[1].isEmpty() ) {
+			builder.setRegion( langCountryVariant[1] );
 		}
-		else {
-			// Only one separator found, there is a language and a country
-			return new Locale( string, new String( chars, position, chars.length - position ) );
+
+		if ( langCountryVariant.length > 2 && !langCountryVariant[2].isEmpty() ) {
+			builder.setVariant( langCountryVariant[2] );
 		}
+
+		if ( parts.length > 1 ) {
+			final String scriptExtension = parts[1];
+			final String maybeScript;
+			if ( scriptExtension.contains( "_" ) ) {
+				maybeScript = scriptExtension.substring( 0, scriptExtension.indexOf( "_" ) );
+			}
+			else {
+				maybeScript = scriptExtension;
+			}
+
+			if ( !isExtension(maybeScript) ) {
+				builder.setScript(maybeScript);
+			}
+		}
+
+		return builder.build();
+	}
+
+	private static boolean isExtension(String token) {
+		return token.length() >= 3
+				&& ( Character.isAlphabetic( token.charAt(0) ) || Character.isDigit( token.charAt(0) ) )
+				&& token.charAt( 1 ) == '-';
 	}
 
 	@SuppressWarnings({ "unchecked" })

hibernate-core/src/test/java/org/hibernate/test/type/descriptor/java/LocaleTypeDescriptorTest.java

@@ -25,18 +25,21 @@ public class LocaleTypeDescriptorTest extends BaseUnitTestCase {
 
 	@Test
 	public void testConversionFromString() {
-		assertEquals( toLocale( "de", null, null ), LocaleTypeDescriptor.INSTANCE.fromString( "de" ) );
-		assertEquals( toLocale( "de", "DE", null ), LocaleTypeDescriptor.INSTANCE.fromString( "de_DE" ) );
-		assertEquals( toLocale( null, "DE", null ), LocaleTypeDescriptor.INSTANCE.fromString( "_DE" ) );
-		assertEquals( toLocale( null, null, "ch123" ), LocaleTypeDescriptor.INSTANCE.fromString( "__ch123" ) );
-		assertEquals( toLocale( null, "DE", "ch123" ), LocaleTypeDescriptor.INSTANCE.fromString( "_DE_ch123" ) );
-		assertEquals( toLocale( "de", null, "ch123" ), LocaleTypeDescriptor.INSTANCE.fromString( "de__ch123" ) );
-		assertEquals( toLocale( "de", "DE", "ch123" ), LocaleTypeDescriptor.INSTANCE.fromString( "de_DE_ch123" ) );
-		assertEquals( toLocale( "", "", "" ), LocaleTypeDescriptor.INSTANCE.fromString( "" ) );
+		assertEquals( toLocale( "de", null, null, null ), LocaleTypeDescriptor.INSTANCE.fromString( "de" ) );
+		assertEquals( toLocale( "de", "DE", null, null ), LocaleTypeDescriptor.INSTANCE.fromString( "de_DE" ) );
+		assertEquals( toLocale( null, "DE", null, null ), LocaleTypeDescriptor.INSTANCE.fromString( "_DE" ) );
+		assertEquals( toLocale( null, null, "ch123", null ), LocaleTypeDescriptor.INSTANCE.fromString( "__ch123" ) );
+		assertEquals( toLocale( null, "DE", "ch123", null ), LocaleTypeDescriptor.INSTANCE.fromString( "_DE_ch123" ) );
+		assertEquals( toLocale( "de", null, "ch123", null ), LocaleTypeDescriptor.INSTANCE.fromString( "de__ch123" ) );
+		assertEquals( toLocale( "de", "DE", "ch123", null ), LocaleTypeDescriptor.INSTANCE.fromString( "de_DE_ch123" ) );
+		assertEquals( toLocale( "zh", "HK", null, "Hant" ), LocaleTypeDescriptor.INSTANCE.fromString( "zh_HK_#Hant" ) );
+		assertEquals( toLocale( "zh", "TW", null, "Hant" ), LocaleTypeDescriptor.INSTANCE.fromString( "zh_TW_#Hant_x-java" ) );
+		assertEquals( toLocale( "de", "DE", "1996", null ), LocaleTypeDescriptor.INSTANCE.fromString( "de_DE_1996_#u-co-phonebk-ka-shifted" ) );
+
 		assertEquals( Locale.ROOT, LocaleTypeDescriptor.INSTANCE.fromString( "" ) );
 	}
 
-	public Locale toLocale(String lang, String region, String variant) {
+	public Locale toLocale(String lang, String region, String variant, String script) {
 		final Locale.Builder builder = new Locale.Builder();
 		if ( StringHelper.isNotEmpty( lang ) ) {
 			builder.setLanguage( lang );
@@ -47,6 +50,9 @@ public Locale toLocale(String lang, String region, String variant) {
 		if ( StringHelper.isNotEmpty( variant ) ) {
 			builder.setVariant( variant );
 		}
+		if ( StringHelper.isNotEmpty( script ) ) {
+			builder.setScript( script );
+		}
 		return builder.build();
 	}
 }