Merge pull request #669 from scireum/bugfix/SIRI-1041_amount_scaling

Fixes amount propery persistance scale handling
scireum · Dec 19, 2024 · fa45c9f · fa45c9f
2 parents fa6b319 + a55f163
commit fa45c9f
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 9 deletions.
diff --git a/src/main/java/sirius/db/mixing/properties/AmountProperty.java b/src/main/java/sirius/db/mixing/properties/AmountProperty.java
@@ -38,6 +38,7 @@
 import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.sql.Types;
+import java.util.Optional;
 import java.util.function.Consumer;
 
 /**
@@ -92,21 +93,31 @@ public void create(EntityDescriptor descriptor,
     @Override
     public Object transformValue(Value value) {
         if (value.isFilled()) {
-            return NLS.parseUserString(Amount.class, value.asString());
+            return applyNumericRounding(NLS.parseUserString(Amount.class, value.asString()));
         }
         if (this.isNullable() || defaultValue.isEmptyString()) {
             return Amount.NOTHING;
         }
         return defaultValue.getAmount();
     }
 
+    /**
+     * In case the property is annotated with {@link Numeric}, the value is rounded according to the annotation.
+     *
+     * @param amount the amount
+     * @return the rounded amount if the property is annotated with {@link Numeric}, otherwise the amount itself
+     */
+    private Amount applyNumericRounding(Amount amount) {
+        return getAnnotatedNumberFormat().map(amount::round).orElse(amount);
+    }
+
     @Override
     protected Object transformValueFromImport(Value value) {
         if (value.is(Amount.class)) {
             if (value.getAmount().isEmpty() && !this.isNullable()) {
                 return defaultValue.getAmount();
             }
-            return value.get();
+            return applyNumericRounding((Amount) value.get());
         }
 
         if (value.isFilled()) {
@@ -118,10 +129,10 @@ protected Object transformValueFromImport(Value value) {
 
     private Amount parseWithNLS(@Nonnull Value value) {
         try {
-            return Amount.ofMachineString(value.asString());
+            return applyNumericRounding(Amount.ofMachineString(value.asString()));
         } catch (IllegalArgumentException originalFormatException) {
             try {
-                return Amount.ofUserString(value.asString());
+                return applyNumericRounding(Amount.ofUserString(value.asString()));
             } catch (Exception ignored) {
                 throw originalFormatException;
             }
@@ -144,12 +155,17 @@ public String getColumnDefaultValue() {
         }
         // the resulting string needs to match the string representation in the DB exactly,
         // else a schema change will be issued.
-        NumberFormat format = getAnnotation(Numeric.class).map(numeric -> {
-            return new NumberFormat(numeric.scale(), RoundingMode.HALF_UP, NLS.getMachineFormatSymbols(), false, null);
-        }).orElse(NumberFormat.MACHINE_THREE_DECIMAL_PLACES);
+        NumberFormat format = getAnnotatedNumberFormat().orElse(NumberFormat.MACHINE_NO_DECIMAL_PLACES);
         return Amount.of((BigDecimal) defaultData).toString(format).asString();
     }
 
+    @Nonnull
+    private Optional<NumberFormat> getAnnotatedNumberFormat() {
+        return getAnnotation(Numeric.class).map(numeric -> {
+            return new NumberFormat(numeric.scale(), RoundingMode.HALF_UP, NLS.getMachineFormatSymbols(), false, null);
+        });
+    }
+
     @Override
     protected Object transformToElastic(Object object) {
         return object == null || ((Amount) object).isEmpty() ? null : ((Amount) object).getAmount().toPlainString();

diff --git a/src/test/java/sirius/db/jdbc/DataTypesEntity.java b/src/test/java/sirius/db/jdbc/DataTypesEntity.java
@@ -20,6 +20,8 @@
 
 public class DataTypesEntity extends SQLEntity {
 
+    public static final int AMOUNT_SCALE = 3;
+
     public enum TestEnum {
         Test1, TestTest, Test2
     }
@@ -44,7 +46,7 @@ public enum TestEnum {
 
     @DefaultValue("300")
     @NullAllowed
-    @Numeric(precision = 20, scale = 3)
+    @Numeric(precision = 20, scale = AMOUNT_SCALE)
     private Amount amountValue = Amount.NOTHING;
 
     @DefaultValue("2018-01-01")

diff --git a/src/test/java/sirius/db/mongo/properties/MongoAmountEntity.java b/src/test/java/sirius/db/mongo/properties/MongoAmountEntity.java
@@ -9,19 +9,36 @@
 package sirius.db.mongo.properties;
 
 import sirius.db.mixing.Mapping;
+import sirius.db.mixing.annotations.NullAllowed;
+import sirius.db.mixing.annotations.Numeric;
 import sirius.db.mongo.MongoEntity;
 import sirius.kernel.commons.Amount;
 
 public class MongoAmountEntity extends MongoEntity {
 
+    public static final int AMOUNT_SCALE = 3;
+
     public static final Mapping TEST_AMOUNT = Mapping.named("testAmount");
     private Amount testAmount = Amount.NOTHING;
 
+    public static final Mapping SCALED_AMOUNT = Mapping.named("scaledAmount");
+    @NullAllowed
+    @Numeric(precision = 20, scale = AMOUNT_SCALE)
+    private Amount scaledAmount = Amount.NOTHING;
+
     public Amount getTestAmount() {
         return testAmount;
     }
 
     public void setTestAmount(Amount testAmount) {
         this.testAmount = testAmount;
     }
+
+    public Amount getScaledAmount() {
+        return scaledAmount;
+    }
+
+    public void setScaledAmount(Amount scaledAmount) {
+        this.scaledAmount = scaledAmount;
+    }
 }
diff --git a/src/test/kotlin/sirius/db/jdbc/DataTypesTest.kt b/src/test/kotlin/sirius/db/jdbc/DataTypesTest.kt
@@ -14,6 +14,7 @@ import sirius.kernel.SiriusExtension
 import sirius.kernel.commons.Amount
 import sirius.kernel.commons.Value
 import sirius.kernel.di.std.Part
+import java.math.RoundingMode
 import java.time.Duration
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -86,8 +87,24 @@ class DataTypesTest {
     }
 
     @Test
-    fun `default values work`() {
+    fun `AmountProperty scaling works as excepted`() {
+        var test = DataTypesEntity()
+        val unscaledValue = Value.of("1,2345")
+
+        val amountProperty = test.descriptor.getProperty("amountValue")
+        amountProperty.parseValue(test, unscaledValue)
+        oma.update(test)
+        test = oma.refreshOrFail(test)
+        val expectedAmount = unscaledValue.amount.round(DataTypesEntity.AMOUNT_SCALE, RoundingMode.HALF_UP)
+        assertEquals(expectedAmount, test.amountValue)
 
+        // Storing the same value twice must not trigger a change
+        amountProperty.parseValue(test, unscaledValue)
+        assertFalse { test.isAnyMappingChanged }
+    }
+
+    @Test
+    fun `default values work`() {
         var test = DataTypesEntity()
 
         oma.update(test)

diff --git a/src/test/kotlin/sirius/db/mongo/properties/MongoAmountPropertyTest.kt b/src/test/kotlin/sirius/db/mongo/properties/MongoAmountPropertyTest.kt
@@ -13,8 +13,11 @@ import org.junit.jupiter.api.extension.ExtendWith
 import sirius.db.mongo.Mango
 import sirius.kernel.SiriusExtension
 import sirius.kernel.commons.Amount
+import sirius.kernel.commons.Value
 import sirius.kernel.di.std.Part
+import java.math.RoundingMode
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 
 @ExtendWith(SiriusExtension::class)
 class MongoAmountPropertyTest {
@@ -23,9 +26,31 @@ class MongoAmountPropertyTest {
         val values = listOf(-3.77, Double.MAX_VALUE, 0.00001, -0.00001)
         for (value in values) {
             assertEquals(Amount.of(value), saveAndRead(Amount.of(value)))
+            assertEquals(Amount.of(value), saveAndReadUsingPropertyParsing(Value.of(value)))
         }
     }
 
+    @Test
+    fun `property autoscaling of amount fields works`() {
+        var test = MongoAmountEntity()
+        val unscaledValue = Value.of("1,2345")
+
+        val scaledAmountProperty = test.descriptor.getProperty("scaledAmount")
+        scaledAmountProperty.parseValue(test, unscaledValue)
+        val testAmountProperty = test.descriptor.getProperty("testAmount")
+        testAmountProperty.parseValue(test, unscaledValue)
+        mango.update(test)
+        test = mango.refreshOrFail(test)
+        val expectedAmount = unscaledValue.amount.round(MongoAmountEntity.AMOUNT_SCALE, RoundingMode.HALF_UP)
+        assertEquals(expectedAmount, test.scaledAmount)
+        assertEquals(unscaledValue.amount, test.testAmount)
+
+        // Storing the same value twice must not trigger a change
+        scaledAmountProperty.parseValue(test, unscaledValue)
+        testAmountProperty.parseValue(test, unscaledValue)
+        assertFalse { test.isAnyMappingChanged }
+    }
+
     companion object {
         @Part
         private lateinit var mango: Mango
@@ -37,5 +62,14 @@ class MongoAmountPropertyTest {
             mongoAmountEntity = mango.refreshOrFail(mongoAmountEntity)
             return mongoAmountEntity.testAmount
         }
+
+        private fun saveAndReadUsingPropertyParsing(value: Value): Amount {
+            var mongoAmountEntity = MongoAmountEntity()
+            val amountProperty = mongoAmountEntity.descriptor.getProperty("testAmount")
+            amountProperty.parseValueFromImport(mongoAmountEntity, value)
+            mango.update(mongoAmountEntity)
+            mongoAmountEntity = mango.refreshOrFail(mongoAmountEntity)
+            return mongoAmountEntity.testAmount
+        }
     }
 }