Skip to content

Commit a1cba08

Browse files
committed
Issue #2814 [Bug]: changing GUI language changes also decimal separator
* The interface language was installed as the JVM-wide default locale, so it also decided decimal separator, grouping separator, currency and date formats. The language now writes only Locale.Category.DISPLAY and the regional settings write Locale.Category.FORMAT. * Regional settings had no representation of their own. Added RegionalSettings in core, resolving a single source - OPERATING_SYSTEM (the default), CUSTOM or LANGUAGE - from RegionalSettingsSource and RegionalSettingsLocale in hop-config.json; an unreadable or unknown configuration degrades to OPERATING_SYSTEM. * Locale.setDefault lived in HopGui.main, so hop-run and hop-server never applied it and the same pipeline formatted differently in the editor and in production. The regional settings are now applied from HopEnvironment.init(), the bootstrap common to every entry point. * Const.DEFAULT_DECIMAL_SEPARATOR and its siblings are static final, computed at class load before any locale was installed, and ValueMetaBase copied them into every value, overwriting correctly localised symbols. Added getDefaultDecimalSeparator(), getDefaultGroupingSeparator(), getDefaultCurrencySymbol() and getDefaultNumberFormat(), which read the live FORMAT category and cache the symbols against the locale they were built from; the constants are kept and deprecated because they are public API. * The setting was not reachable from the GUI. Added a Regional settings tab carrying the interface language combo moved out of GUI options, two mutually exclusive checkboxes, a type-to-filter combo over the available locales and a live preview of the resulting formats. * ValueMetaBase and ValueMetaTimestamp compared an explicitly chosen date format locale against Locale.getDefault(), so a field locale that happened to match the interface language was discarded and the date rendered with the regional locale instead. The comparison now reads the FORMAT category. Fixes #2814
1 parent 08c0721 commit a1cba08

21 files changed

Lines changed: 1545 additions & 58 deletions

File tree

core/src/main/java/org/apache/hop/core/Condition.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,7 @@ public IValueMeta createValueMeta() throws HopPluginException {
847847
IValueMeta valueMeta = ValueMetaFactory.createValueMeta(name, getHopType());
848848
valueMeta.setLength(length, precision);
849849
valueMeta.setConversionMask(mask);
850-
valueMeta.setDecimalSymbol(String.valueOf(Const.DEFAULT_DECIMAL_SEPARATOR));
850+
valueMeta.setDecimalSymbol(String.valueOf(Const.getDefaultDecimalSeparator()));
851851
valueMeta.setGroupingSymbol(null);
852852
valueMeta.setCurrencySymbol(null);
853853
return valueMeta;

core/src/main/java/org/apache/hop/core/Const.java

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,22 +280,100 @@ public String getMessage() {
280280
/** The default locale for the hop environment (system defined) */
281281
public static final Locale DEFAULT_LOCALE = Locale.getDefault();
282282

283-
/** The default decimal separator . or , */
283+
/**
284+
* The default decimal separator . or ,
285+
*
286+
* @deprecated captured at class-load time from {@link #DEFAULT_LOCALE}; use {@link
287+
* #getDefaultDecimalSeparator()} to read the active regional settings at call time.
288+
*/
289+
@Deprecated(since = "2.20")
284290
public static final char DEFAULT_DECIMAL_SEPARATOR =
285291
(new DecimalFormatSymbols(DEFAULT_LOCALE)).getDecimalSeparator();
286292

287-
/** The default grouping separator , or . */
293+
/**
294+
* The default grouping separator , or .
295+
*
296+
* @deprecated captured at class-load time from {@link #DEFAULT_LOCALE}; use {@link
297+
* #getDefaultGroupingSeparator()} to read the active regional settings at call time.
298+
*/
299+
@Deprecated(since = "2.20")
288300
public static final char DEFAULT_GROUPING_SEPARATOR =
289301
(new DecimalFormatSymbols(DEFAULT_LOCALE)).getGroupingSeparator();
290302

291-
/** The default currency symbol */
303+
/**
304+
* The default currency symbol
305+
*
306+
* @deprecated captured at class-load time from {@link #DEFAULT_LOCALE}; use {@link
307+
* #getDefaultCurrencySymbol()} to read the active regional settings at call time.
308+
*/
309+
@Deprecated(since = "2.20")
292310
public static final String DEFAULT_CURRENCY_SYMBOL =
293311
(new DecimalFormatSymbols(DEFAULT_LOCALE)).getCurrencySymbol();
294312

295-
/** The default number format */
313+
/**
314+
* The default number format
315+
*
316+
* @deprecated captured at class-load time from {@link #DEFAULT_LOCALE}; use {@link
317+
* #getDefaultNumberFormat()} to read the active regional settings at call time.
318+
*/
319+
@Deprecated(since = "2.20")
296320
public static final String DEFAULT_NUMBER_FORMAT =
297321
((DecimalFormat) (NumberFormat.getInstance())).toPattern();
298322

323+
/**
324+
* Cached symbols for the regional locale they were built from.
325+
*
326+
* <p>These accessors are called from the {@code ValueMetaBase} constructor, so they sit on a hot
327+
* path: building a {@link DecimalFormatSymbols} on every call would be a real cost — the same one
328+
* {@code ValueMetaBase.getDecimalFormat()} already warns about for {@code DecimalFormat}. The
329+
* symbols are therefore cached and rebuilt only when the FORMAT locale actually changes.
330+
*
331+
* <p>Both fields are written together under {@code synchronized} and read together, so a racing
332+
* reader can never pair one locale's symbols with another locale's marker.
333+
*/
334+
private static DecimalFormatSymbols cachedFormatSymbols;
335+
336+
private static Locale cachedFormatSymbolsLocale;
337+
338+
private static synchronized DecimalFormatSymbols getFormatSymbols() {
339+
Locale formatLocale = Locale.getDefault(Locale.Category.FORMAT);
340+
if (cachedFormatSymbols == null || !formatLocale.equals(cachedFormatSymbolsLocale)) {
341+
cachedFormatSymbols = new DecimalFormatSymbols(formatLocale);
342+
cachedFormatSymbolsLocale = formatLocale;
343+
}
344+
return cachedFormatSymbols;
345+
}
346+
347+
/**
348+
* The decimal separator of the active regional settings, read at call time.
349+
*
350+
* <p>Prefer this over {@link #DEFAULT_DECIMAL_SEPARATOR}, which is captured when the class is
351+
* loaded and therefore predates the regional settings being installed.
352+
*/
353+
public static char getDefaultDecimalSeparator() {
354+
return getFormatSymbols().getDecimalSeparator();
355+
}
356+
357+
/** The grouping separator of the active regional settings, read at call time. */
358+
public static char getDefaultGroupingSeparator() {
359+
return getFormatSymbols().getGroupingSeparator();
360+
}
361+
362+
/** The currency symbol of the active regional settings, read at call time. */
363+
public static String getDefaultCurrencySymbol() {
364+
return getFormatSymbols().getCurrencySymbol();
365+
}
366+
367+
/**
368+
* The number format pattern of the active regional settings, read at call time. In practice the
369+
* returned pattern is locale-invariant (locale-specific separators are applied later via
370+
* DecimalFormatSymbols), so callers do not generally need to re-read it when the locale changes.
371+
*/
372+
public static String getDefaultNumberFormat() {
373+
return ((DecimalFormat) NumberFormat.getInstance(Locale.getDefault(Locale.Category.FORMAT)))
374+
.toPattern();
375+
}
376+
299377
/** Default string representing Null String values (empty) */
300378
public static final String NULL_STRING = "";
301379

core/src/main/java/org/apache/hop/core/row/value/ValueMetaBase.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,9 @@ protected ValueMetaBase(
313313
this.storageType = STORAGE_TYPE_NORMAL;
314314
this.sortedDescending = false;
315315
this.outputPaddingEnabled = false;
316-
this.decimalSymbol = "" + Const.DEFAULT_DECIMAL_SEPARATOR;
317-
this.groupingSymbol = "" + Const.DEFAULT_GROUPING_SEPARATOR;
318-
this.currencySymbol = "" + Const.DEFAULT_CURRENCY_SYMBOL;
316+
this.decimalSymbol = "" + Const.getDefaultDecimalSeparator();
317+
this.groupingSymbol = "" + Const.getDefaultGroupingSeparator();
318+
this.currencySymbol = "" + Const.getDefaultCurrencySymbol();
319319
this.dateFormatLocale = Locale.getDefault();
320320
this.collatorDisabled = true;
321321
this.collatorLocale = Locale.getDefault();
@@ -1296,7 +1296,13 @@ private synchronized SimpleDateFormat getDateFormat(int valueMetaType) {
12961296

12971297
// Do we have a locale?
12981298
//
1299-
if (dateFormatLocale == null || dateFormatLocale.equals(Locale.getDefault())) {
1299+
// Compared against the FORMAT category, not against Locale.getDefault(): that one carries the
1300+
// interface language, so a locale deliberately picked on the field would be dismissed as "no
1301+
// locale set" whenever it happened to match the language, and the field would silently follow
1302+
// the regional settings instead of the choice.
1303+
//
1304+
if (dateFormatLocale == null
1305+
|| dateFormatLocale.equals(Locale.getDefault(Locale.Category.FORMAT))) {
13001306
if (mask != null) {
13011307
dateFormat = new SimpleDateFormat(mask);
13021308
}

core/src/main/java/org/apache/hop/core/row/value/ValueMetaTimestamp.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,13 @@ private synchronized SimpleDateFormat getDateFormat(int valueMetaType) {
666666

667667
// Do we have a locale?
668668
//
669-
if (dateFormatLocale == null || dateFormatLocale.equals(Locale.getDefault())) {
669+
// Compared against the FORMAT category, not against Locale.getDefault(): that one carries the
670+
// interface language, so a locale deliberately picked on the field would be dismissed as "no
671+
// locale set" whenever it happened to match the language, and the field would silently follow
672+
// the regional settings instead of the choice.
673+
//
674+
if (dateFormatLocale == null
675+
|| dateFormatLocale.equals(Locale.getDefault(Locale.Category.FORMAT))) {
670676
dateFormat = new SimpleTimestampFormat(mask);
671677
} else {
672678
dateFormat = new SimpleTimestampFormat(mask, dateFormatLocale);
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.hop.i18n;
19+
20+
import java.util.Arrays;
21+
import java.util.Locale;
22+
import org.apache.hop.core.config.HopConfig;
23+
import org.apache.hop.core.logging.LogChannel;
24+
import org.apache.hop.core.util.EnvUtil;
25+
import org.apache.hop.core.util.Utils;
26+
27+
/**
28+
* Holds the regional settings (decimal and grouping separators, currency, date formats) as a
29+
* concern separate from the interface language, which stays under {@link LanguageChoice}.
30+
*
31+
* <p>The effective locale is installed in the JVM as {@link Locale.Category#FORMAT}, while {@link
32+
* Locale#getDefault()} — the locale {@code ResourceBundle} resolves messages with — keeps carrying
33+
* the interface language.
34+
*/
35+
public class RegionalSettings {
36+
37+
/** Where the regional settings come from. */
38+
public enum Source {
39+
/**
40+
* Follow the selected interface language, so that changing the language changes the formats
41+
* with it. This is a deliberate choice a user makes, not the source an unconfigured
42+
* installation falls back to.
43+
*/
44+
LANGUAGE,
45+
/** Inherit them from the operating system Hop is running on. */
46+
OPERATING_SYSTEM,
47+
/** Use an explicitly selected locale. */
48+
CUSTOM
49+
}
50+
51+
public static final String STRING_REGIONAL_SETTINGS_SOURCE = "RegionalSettingsSource";
52+
public static final String STRING_REGIONAL_SETTINGS_LOCALE = "RegionalSettingsLocale";
53+
54+
/**
55+
* The locale the JVM started with, captured before anything can overwrite it. The first {@code
56+
* Locale.setDefault(language)} destroys this value and it cannot be recovered afterwards, so
57+
* {@link Source#OPERATING_SYSTEM} would have nothing to read without this field.
58+
*/
59+
private static final Locale OPERATING_SYSTEM_LOCALE = Locale.getDefault();
60+
61+
private static RegionalSettings instance;
62+
63+
private Source source;
64+
private Locale customLocale;
65+
66+
private RegionalSettings() {
67+
reload();
68+
}
69+
70+
public static synchronized RegionalSettings getInstance() {
71+
if (instance == null) {
72+
instance = new RegionalSettings();
73+
}
74+
return instance;
75+
}
76+
77+
/**
78+
* Re-reads the configuration, degrading to {@link Source#OPERATING_SYSTEM} on anything unusable.
79+
*/
80+
public void reload() {
81+
String sourceValue =
82+
HopConfig.readOptionString(STRING_REGIONAL_SETTINGS_SOURCE, Source.OPERATING_SYSTEM.name());
83+
try {
84+
source = Source.valueOf(sourceValue);
85+
} catch (IllegalArgumentException e) {
86+
LogChannel.GENERAL.logBasic(
87+
"Unknown value '"
88+
+ sourceValue
89+
+ "' for option "
90+
+ STRING_REGIONAL_SETTINGS_SOURCE
91+
+ ", deriving regional settings from the operating system instead.");
92+
source = Source.OPERATING_SYSTEM;
93+
}
94+
95+
String localeValue = HopConfig.readOptionString(STRING_REGIONAL_SETTINGS_LOCALE, null);
96+
customLocale = Utils.isEmpty(localeValue) ? null : EnvUtil.createLocale(localeValue);
97+
98+
if (source == Source.CUSTOM && !isUsable(customLocale)) {
99+
LogChannel.GENERAL.logBasic(
100+
"Regional settings locale '"
101+
+ localeValue
102+
+ "' is not available in this JVM, deriving regional settings from the operating"
103+
+ " system instead.");
104+
source = Source.OPERATING_SYSTEM;
105+
}
106+
}
107+
108+
/** Persists the current source and custom locale. */
109+
public void save() {
110+
HopConfig.getInstance().saveOption(STRING_REGIONAL_SETTINGS_SOURCE, source.name());
111+
HopConfig.getInstance()
112+
.saveOption(
113+
STRING_REGIONAL_SETTINGS_LOCALE, customLocale == null ? null : customLocale.toString());
114+
}
115+
116+
/** The locale actually used to format numbers, currencies and dates. */
117+
public Locale getEffectiveLocale() {
118+
return switch (source) {
119+
case OPERATING_SYSTEM -> OPERATING_SYSTEM_LOCALE;
120+
case CUSTOM -> customLocale;
121+
case LANGUAGE -> LanguageChoice.getInstance().getDefaultLocale();
122+
};
123+
}
124+
125+
/**
126+
* Applies the regional settings for a headless run (hop-run, hop-server, REST), so those runs
127+
* honour the configuration of the machine they run on.
128+
*
129+
* <p>Distributed Beam and Spark workers are not covered by this method: they never load a {@code
130+
* hop-config.json} in the first place, so they fall back to the default source and format with
131+
* their own operating system locale regardless of what this method would apply.
132+
*/
133+
public void applyHeadless() {
134+
// Under the default source this writes OPERATING_SYSTEM_LOCALE, which was captured from the
135+
// JVM's own initial default — precisely what a headless run already carries, including when it
136+
// was set with -Duser.language. Writing it back is therefore a no-op in practice.
137+
Locale formatLocale = getEffectiveLocale();
138+
if (formatLocale == null) {
139+
LogChannel.GENERAL.logBasic(
140+
"No usable regional settings locale is configured; leaving the format settings alone.");
141+
return;
142+
}
143+
Locale.setDefault(Locale.Category.FORMAT, formatLocale);
144+
}
145+
146+
/**
147+
* Applies the interface language and then the regional settings, in that order.
148+
*
149+
* <p>The order is mandatory: {@code Locale.setDefault(Locale)} writes all three categories, so
150+
* setting the language after the regional settings would wipe the FORMAT category. For the same
151+
* reason the FORMAT category is always written back, even when the regional settings are derived
152+
* from the language and the two carry the same value.
153+
*/
154+
public void applyGui() {
155+
Locale.setDefault(LanguageChoice.getInstance().getDefaultLocale());
156+
Locale formatLocale = getEffectiveLocale();
157+
if (formatLocale == null) {
158+
LogChannel.GENERAL.logBasic(
159+
"No usable regional settings locale is configured; leaving the format settings alone.");
160+
return;
161+
}
162+
Locale.setDefault(Locale.Category.FORMAT, formatLocale);
163+
}
164+
165+
private static boolean isUsable(Locale locale) {
166+
return locale != null && Arrays.asList(Locale.getAvailableLocales()).contains(locale);
167+
}
168+
169+
public Source getSource() {
170+
return source;
171+
}
172+
173+
public void setSource(Source source) {
174+
this.source = source;
175+
}
176+
177+
public Locale getCustomLocale() {
178+
return customLocale;
179+
}
180+
181+
public void setCustomLocale(Locale customLocale) {
182+
this.customLocale = customLocale;
183+
}
184+
185+
public Locale getOperatingSystemLocale() {
186+
return OPERATING_SYSTEM_LOCALE;
187+
}
188+
}

0 commit comments

Comments
 (0)