Fix decimal separator issues when creating QuantityType from String (o…

…penhab#2362) * Fix decimal separator issues when creating QuantityType from String Fixes openhab#2360 Signed-off-by: Wouter Born <[email protected]>
ConnectorIO · May 14, 2021 · 568881a · 568881a
1 parent a8f469e
commit 568881a
Show file tree

Hide file tree

Showing 4 changed files with 243 additions and 90 deletions.
diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java
@@ -15,6 +15,7 @@
 import static org.eclipse.jdt.annotation.DefaultLocation.*;
 
 import java.math.BigDecimal;
+import java.text.DecimalFormatSymbols;
 import java.time.Instant;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
@@ -58,6 +59,7 @@ public class QuantityType<T extends Quantity<T>> extends Number
         implements PrimitiveType, State, Command, Comparable<QuantityType<T>> {
 
     private static final long serialVersionUID = 8828949721938234629L;
+    private static final char DOT_DECIMAL_SEPARATOR = '.';
     private static final BigDecimal HUNDRED = BigDecimal.valueOf(100);
 
     public static final QuantityType<Dimensionless> ZERO = new QuantityType<>(0, AbstractUnit.ONE);
@@ -102,6 +104,14 @@ public QuantityType(String value) {
             BigDecimal bd = new BigDecimal(value);
             quantity = (Quantity<T>) Quantities.getQuantity(bd, AbstractUnit.ONE, Scale.RELATIVE);
         } else {
+            char defaultDecimalSeparator = DecimalFormatSymbols.getInstance().getDecimalSeparator();
+            // The quantity is parsed using a NumberFormat based on the default locale.
+            // To prevent issues, any dot decimal separators are replaced by the default locale decimal separator.
+            if (DOT_DECIMAL_SEPARATOR != defaultDecimalSeparator
+                    && formatted.contains(String.valueOf(DOT_DECIMAL_SEPARATOR))) {
+                formatted = formatted.replace(DOT_DECIMAL_SEPARATOR, defaultDecimalSeparator);
+            }
+
             Quantity<T> absoluteQuantity = (Quantity<T>) Quantities.getQuantity(formatted);
             quantity = Quantities.getQuantity(absoluteQuantity.getValue(), absoluteQuantity.getUnit(), Scale.RELATIVE);
         }

diff --git a/...penhab.core/src/test/java/org/openhab/core/internal/items/ItemStateConverterImplTest.java b/...penhab.core/src/test/java/org/openhab/core/internal/items/ItemStateConverterImplTest.java
@@ -17,12 +17,16 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.*;
 
+import java.util.Locale;
+import java.util.stream.Stream;
+
 import javax.measure.quantity.Length;
 import javax.measure.quantity.Temperature;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.openhab.core.i18n.UnitProvider;
 import org.openhab.core.items.Item;
 import org.openhab.core.library.items.NumberItem;
@@ -44,50 +48,78 @@ public class ItemStateConverterImplTest {
 
     private @NonNullByDefault({}) ItemStateConverterImpl itemStateConverter;
 
+    /**
+     * Locales having a different decimal separator to test string parsing and generation.
+     */
+    static Stream<Locale> locales() {
+        return Stream.of(
+                // ٫ (Arabic, Egypt)
+                Locale.forLanguageTag("ar-EG"),
+                // , (German, Germany)
+                Locale.forLanguageTag("de-DE"),
+                // . (English, United States)
+                Locale.forLanguageTag("en-US"));
+    }
+
     @BeforeEach
     public void setup() {
         UnitProvider unitProvider = mock(UnitProvider.class);
         when(unitProvider.getUnit(Temperature.class)).thenReturn(ImperialUnits.FAHRENHEIT);
         itemStateConverter = new ItemStateConverterImpl(unitProvider);
     }
 
-    @Test
-    public void testNullState() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void testNullState(Locale locale) {
+        Locale.setDefault(locale);
+
         State undef = itemStateConverter.convertToAcceptedState(null, null);
 
         assertThat(undef, is(UnDefType.NULL));
     }
 
-    @Test
+    @ParameterizedTest
+    @MethodSource("locales")
     @SuppressWarnings("PMD.CompareObjectsWithEquals")
-    public void testNoConversion() {
+    public void testNoConversion(Locale locale) {
+        Locale.setDefault(locale);
+
         Item item = new NumberItem("number");
         State originalState = new DecimalType(12.34);
         State state = itemStateConverter.convertToAcceptedState(originalState, item);
 
         assertTrue(originalState == state);
     }
 
-    @Test
-    public void testStateConversion() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void testStateConversion(Locale locale) {
+        Locale.setDefault(locale);
+
         Item item = new NumberItem("number");
         State originalState = new PercentType("42");
         State convertedState = itemStateConverter.convertToAcceptedState(originalState, item);
 
         assertThat(convertedState, is(new DecimalType("0.42")));
     }
 
-    @Test
-    public void numberItemWithoutDimensionShouldConvertToDecimalType() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void numberItemWithoutDimensionShouldConvertToDecimalType(Locale locale) {
+        Locale.setDefault(locale);
+
         Item item = new NumberItem("number");
         State originalState = new QuantityType<>("12.34 °C");
         State convertedState = itemStateConverter.convertToAcceptedState(originalState, item);
 
         assertThat(convertedState, is(new DecimalType("12.34")));
     }
 
-    @Test
-    public void numberItemWitDimensionShouldConvertToItemStateDescriptionUnit() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void numberItemWitDimensionShouldConvertToItemStateDescriptionUnit(Locale locale) {
+        Locale.setDefault(locale);
+
         NumberItem item = mock(NumberItem.class);
         StateDescription stateDescription = mock(StateDescription.class);
         when(item.getStateDescription()).thenReturn(stateDescription);
@@ -100,8 +132,11 @@ public void numberItemWitDimensionShouldConvertToItemStateDescriptionUnit() {
         assertThat(convertedState, is(new QuantityType<>("285.49 K")));
     }
 
-    @Test
-    public void numberItemWitDimensionShouldConvertToLocaleBasedUnit() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void numberItemWitDimensionShouldConvertToLocaleBasedUnit(Locale locale) {
+        Locale.setDefault(locale);
+
         NumberItem item = mock(NumberItem.class);
         doReturn(Temperature.class).when(item).getDimension();
 
@@ -111,8 +146,11 @@ public void numberItemWitDimensionShouldConvertToLocaleBasedUnit() {
         assertThat(convertedState, is(new QuantityType<>("54.212 °F")));
     }
 
-    @Test
-    public void numberItemShouldNotConvertUnitsWhereMeasurmentSystemEquals() {
+    @ParameterizedTest
+    @MethodSource("locales")
+    public void numberItemShouldNotConvertUnitsWhereMeasurmentSystemEquals(Locale locale) {
+        Locale.setDefault(locale);
+
         NumberItem item = mock(NumberItem.class);
         doReturn(Length.class).when(item).getDimension();