Skip to content

Commit

Permalink
Merge pull request #727 from FgForrest/726-automatic-rounding-for-att…
Browse files Browse the repository at this point in the history
…ributes-prices-with-incompatible-number-of-decimal-places

fix(#726): automatic rounding for attributes prices with incompatible number of decimal places
  • Loading branch information
novoj authored Nov 1, 2024
2 parents 5fab69b + 2ec9917 commit a962b4a
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 22 deletions.
28 changes: 26 additions & 2 deletions evita_common/src/main/java/io/evitadb/utils/NumberUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@

package io.evitadb.utils;

import io.evitadb.exception.EvitaInvalidUsageException;

import javax.annotation.Nonnull;
import java.math.BigDecimal;
import java.math.RoundingMode;

/**
* String utils contains shared utility method for working with Numbers.
Expand Down Expand Up @@ -54,6 +57,7 @@ public static boolean isIntConvertibleNumber(@Nonnull Class<?> parameterType) {
* converted to the same type and applied. Method checks that there is no loss of precision during sum.
*/
@SuppressWarnings("RedundantCast")
@Nonnull
public static Number sum(@Nonnull Number a, @Nonnull Number b) {
if (a instanceof Byte) {
final long longResult = convertToLong(a) + convertToLong(b);
Expand Down Expand Up @@ -150,15 +154,34 @@ public static int convertToInt(@Nonnull Number number) {
*/
public static int convertToInt(@Nonnull BigDecimal number, int acceptDecimalPlaces) {
try {
return number.stripTrailingZeros().scaleByPowerOfTen(acceptDecimalPlaces).intValueExact();
return number.stripTrailingZeros()
.scaleByPowerOfTen(acceptDecimalPlaces)
.setScale(0, RoundingMode.HALF_UP)
.intValueExact();
} catch (ArithmeticException ex) {
throw new IllegalArgumentException(
throw new ArithmeticException(
"Cannot convert big decimal " + number +
" to exact integer by using " + acceptDecimalPlaces + " decimal places!"
);
}
}

/**
* Converts passed {@link BigDecimal} number to integer value with rounding and overflow handling.
*
* @param number number to convert
* @param indexedPricePlaces number of decimal places to keep in the integer value
* @return converted integer value
* @throws EvitaInvalidUsageException if the number is too large to be converted to integer
*/
public static int convertExternalNumberToInt(@Nonnull BigDecimal number, int indexedPricePlaces) {
try {
return convertToInt(number, indexedPricePlaces);
} catch (ArithmeticException ex) {
throw new EvitaInvalidUsageException(ex.getMessage(), ex);
}
}

/**
* Converts unknown number to {@link long}.
*/
Expand All @@ -173,6 +196,7 @@ public static long convertToLong(@Nonnull Number number) {
/**
* Converts unknown number to {@link BigDecimal}.
*/
@Nonnull
public static BigDecimal convertToBigDecimal(@Nonnull Number number) {
if (number instanceof Byte) {
return new BigDecimal(number.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ protected PricePredicate(
int indexedPricePlaces
) {
this.queryPriceMode = queryPriceMode;
this.fromAsInt = from == null ? Integer.MIN_VALUE : NumberUtils.convertToInt(from, indexedPricePlaces);
this.toAsInt = to == null ? Integer.MAX_VALUE : NumberUtils.convertToInt(to, indexedPricePlaces);
this.fromAsInt = from == null ? Integer.MIN_VALUE : NumberUtils.convertExternalNumberToInt(from, indexedPricePlaces);
this.toAsInt = to == null ? Integer.MAX_VALUE : NumberUtils.convertExternalNumberToInt(to, indexedPricePlaces);
this.indexedPricePlaces = indexedPricePlaces;
if (queryPriceMode == null) {
this.description = "NO FILTER PREDICATE";
Expand Down Expand Up @@ -109,7 +109,7 @@ protected PricePredicate(
this.amountPredicate = new PriceAmountPredicate(
queryPriceMode, from, to, indexedPricePlaces,
amount -> {
final int amountAsInt = NumberUtils.convertToInt(amount, indexedPricePlaces);
final int amountAsInt = NumberUtils.convertExternalNumberToInt(amount, indexedPricePlaces);
return amountAsInt >= fromAsInt && amountAsInt <= toAsInt;
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,17 @@ public SellingPriceAvailableBitmapFilter(
innerRecordPrice.priceId(),
entityPrimaryKey,
innerRecordPrice.innerRecordId(),
NumberUtils.convertToInt(innerRecordPrice.priceWithTax(), indexedPricePlaces),
NumberUtils.convertToInt(innerRecordPrice.priceWithoutTax(), indexedPricePlaces)
NumberUtils.convertExternalNumberToInt(innerRecordPrice.priceWithTax(), indexedPricePlaces),
NumberUtils.convertExternalNumberToInt(innerRecordPrice.priceWithoutTax(), indexedPricePlaces)
)
);
}
}
return new CumulatedVirtualPriceRecord(
entityPrimaryKey,
priceQueryMode == QueryPriceMode.WITH_TAX ?
NumberUtils.convertToInt(cumulatedPrice.priceWithTax(), indexedPricePlaces) :
NumberUtils.convertToInt(cumulatedPrice.priceWithoutTax(), indexedPricePlaces),
NumberUtils.convertExternalNumberToInt(cumulatedPrice.priceWithTax(), indexedPricePlaces) :
NumberUtils.convertExternalNumberToInt(cumulatedPrice.priceWithoutTax(), indexedPricePlaces),
priceQueryMode,
intSetInnerRecordIds
);
Expand All @@ -147,8 +147,8 @@ public SellingPriceAvailableBitmapFilter(
priceWithInternalIds.getInternalPriceId() : -1,
priceContract.priceId(),
entityPrimaryKey,
NumberUtils.convertToInt(priceContract.priceWithTax(), indexedPricePlaces),
NumberUtils.convertToInt(priceContract.priceWithoutTax(), indexedPricePlaces)
NumberUtils.convertExternalNumberToInt(priceContract.priceWithTax(), indexedPricePlaces),
NumberUtils.convertExternalNumberToInt(priceContract.priceWithoutTax(), indexedPricePlaces)
);
} else {
return new PriceRecordInnerRecordSpecific(
Expand All @@ -157,8 +157,8 @@ public SellingPriceAvailableBitmapFilter(
priceContract.priceId(),
entityPrimaryKey,
priceContract.innerRecordId(),
NumberUtils.convertToInt(priceContract.priceWithTax(), indexedPricePlaces),
NumberUtils.convertToInt(priceContract.priceWithoutTax(), indexedPricePlaces)
NumberUtils.convertExternalNumberToInt(priceContract.priceWithTax(), indexedPricePlaces),
NumberUtils.convertExternalNumberToInt(priceContract.priceWithoutTax(), indexedPricePlaces)
);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import io.evitadb.store.entity.model.entity.price.PriceInternalIdContainer;
import io.evitadb.store.entity.model.entity.price.PriceWithInternalIds;
import io.evitadb.utils.Assert;
import io.evitadb.utils.NumberUtils;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand All @@ -41,8 +42,6 @@
import java.util.function.BiFunction;
import java.util.function.Consumer;

import static io.evitadb.utils.NumberUtils.convertToInt;

/**
* This interface is used to co-locate price mutating routines which are rather procedural and long to avoid excessive
* amount of code in {@link EntityIndexLocalMutationExecutor}.
Expand Down Expand Up @@ -108,8 +107,8 @@ static void priceUpsert(
final Integer formerInternalPriceId = Objects.requireNonNull(formerPrice.getInternalPriceId());
final Integer formerInnerRecordId = formerPrice.innerRecordId();
final DateTimeRange formerValidity = formerPrice.validity();
final int formerPriceWithoutTax = convertToInt(formerPrice.priceWithoutTax(), indexedPricePlaces);
final int formerPriceWithTax = convertToInt(formerPrice.priceWithTax(), indexedPricePlaces);
final int formerPriceWithoutTax = NumberUtils.convertExternalNumberToInt(formerPrice.priceWithoutTax(), indexedPricePlaces);
final int formerPriceWithTax = NumberUtils.convertExternalNumberToInt(formerPrice.priceWithTax(), indexedPricePlaces);
entityIndex.priceRemove(
entityPrimaryKey,
formerInternalPriceId,
Expand All @@ -135,8 +134,8 @@ static void priceUpsert(
if (indexed) {
final PriceInternalIdContainer internalPriceIds = internalIdSupplier.apply(priceKey, innerRecordId);
final Integer internalPriceId = internalPriceIds.getInternalPriceId();
final int priceWithoutTaxAsInt = convertToInt(priceWithoutTax, indexedPricePlaces);
final int priceWithTaxAsInt = convertToInt(priceWithTax, indexedPricePlaces);
final int priceWithoutTaxAsInt = NumberUtils.convertExternalNumberToInt(priceWithoutTax, indexedPricePlaces);
final int priceWithTaxAsInt = NumberUtils.convertExternalNumberToInt(priceWithTax, indexedPricePlaces);
final PriceInternalIdContainer priceId = entityIndex.addPrice(
entityPrimaryKey,
internalPriceId,
Expand Down Expand Up @@ -198,8 +197,8 @@ static void priceRemove(
final int internalPriceId = formerPrice.getInternalPriceId();
final Integer innerRecordId = formerPrice.innerRecordId();
final DateTimeRange validity = formerPrice.validity();
final int priceWithoutTax = convertToInt(formerPrice.priceWithoutTax(), indexedPricePlaces);
final int priceWithTax = convertToInt(formerPrice.priceWithTax(), indexedPricePlaces);
final int priceWithoutTax = NumberUtils.convertExternalNumberToInt(formerPrice.priceWithoutTax(), indexedPricePlaces);
final int priceWithTax = NumberUtils.convertExternalNumberToInt(formerPrice.priceWithTax(), indexedPricePlaces);
entityIndex.priceRemove(
entityPrimaryKey,
internalPriceId,
Expand Down Expand Up @@ -244,4 +243,5 @@ static BiFunction<PriceKey, Integer, PriceInternalIdContainer> createPriceProvid
return price;
};
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ void shouldConvertNumbersToInt() {
assertEquals(2, NumberUtils.convertToInt((short) 2));
assertEquals(2, NumberUtils.convertToInt(2));
assertEquals(2, NumberUtils.convertToInt((long) 2));
assertEquals(22314, NumberUtils.convertToInt(new BigDecimal("223.1405"), 2));
assertEquals(2231405, NumberUtils.convertToInt(new BigDecimal("223.1405"), 4));
assertEquals(223, NumberUtils.convertToInt(new BigDecimal("223.1405"), 0));
}

@Test
Expand All @@ -160,7 +163,7 @@ void shouldConvertBigDecimalToInt() {
assertEquals(11020, NumberUtils.convertToInt(new BigDecimal("110.2"), 2));
assertEquals(11020, NumberUtils.convertToInt(new BigDecimal("110.20"), 2));
assertEquals(11020, NumberUtils.convertToInt(new BigDecimal("110.2000"), 2));
assertThrows(IllegalArgumentException.class, () -> NumberUtils.convertToInt(new BigDecimal("110.202"), 2));
assertThrows(ArithmeticException.class, () -> NumberUtils.convertToInt(new BigDecimal("21474836471"), 2));
}

@Test
Expand Down

0 comments on commit a962b4a

Please sign in to comment.