Skip to content
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.

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

Fixes amount propery persistance scale handling #669

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 23 additions & 7 deletions src/main/java/sirius/db/mixing/properties/AmountProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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()) {
Expand All @@ -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;
}
Expand All @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/sirius/db/jdbc/DataTypesEntity.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

public class DataTypesEntity extends SQLEntity {

public static final int AMOUNT_SCALE = 3;

public enum TestEnum {
Test1, TestTest, Test2
}
Expand All @@ -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")
Expand Down
17 changes: 17 additions & 0 deletions src/test/java/sirius/db/mongo/properties/MongoAmountEntity.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
19 changes: 18 additions & 1 deletion src/test/kotlin/sirius/db/jdbc/DataTypesTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
}
}
}
Loading