Avoid using C_UOM_Conversion.DivideRate

backend/de.metas.adempiere.libero.libero/src/test/java/de/metas/material/planning/pporder/impl/PPOrderBOMBLTest.java

@@ -41,6 +41,9 @@
 
 import de.metas.material.planning.pporder.IPPOrderBOMBL;
 import de.metas.material.planning.pporder.IPPOrderBOMDAO;
+import de.metas.product.ProductId;
+import de.metas.uom.CreateUOMConversionRequest;
+import de.metas.uom.UomId;
 import de.metas.util.Services;
 
 /**
@@ -89,12 +92,12 @@ public void qualityMultiplierTest()
 		final I_M_Product pSalad = helper.createProduct("Salad", uomStuck); // AB Alicesalat 250g - the big product bom
 
 		// Conversion Salad
-		helper.createUOMConversion(
-				pSalad.getM_Product_ID(),
-				uomStuck,
-				uomKillogram,
-				new BigDecimal(0.25),
-				new BigDecimal(4));
+		helper.createUOMConversion(CreateUOMConversionRequest.builder()
+				.productId(ProductId.ofRepoId(pSalad.getM_Product_ID()))
+				.fromUomId(UomId.ofRepoId(uomStuck.getC_UOM_ID()))
+				.toUomId(UomId.ofRepoId(uomKillogram.getC_UOM_ID()))
+				.fromToMultiplier(new BigDecimal(0.25))
+				.build());
 
 		// Components
 		final I_M_Product pCarrot = helper.createProduct("Carrot", uomKillogram); // Karotten Julienne 3.2 mm Halbfabrikat
@@ -108,12 +111,12 @@ public void qualityMultiplierTest()
 
 		//
 		// Conversion for Folie
-		helper.createUOMConversion(
-				pFolie.getM_Product_ID(),
-				uomRolle,
-				uomMillimeter,
-				new BigDecimal(1500000),
-				new BigDecimal(0.000000666667));
+		helper.createUOMConversion(CreateUOMConversionRequest.builder()
+				.productId(ProductId.ofRepoId(pFolie.getM_Product_ID()))
+				.fromUomId(UomId.ofRepoId(uomRolle.getC_UOM_ID()))
+				.toUomId(UomId.ofRepoId(uomMillimeter.getC_UOM_ID()))
+				.fromToMultiplier(new BigDecimal(1500000))
+				.build());
 
 		//
 		// Define BOM

backend/de.metas.adempiere.libero.libero/src/test/java/org/eevolution/mrp/api/impl/MRPTestHelper.java

@@ -112,7 +112,6 @@
 import de.metas.uom.CreateUOMConversionRequest;
 import de.metas.uom.IUOMConversionDAO;
 import de.metas.uom.IUOMDAO;
-import de.metas.uom.UomId;
 import de.metas.util.Services;
 import de.metas.util.time.SystemTime;
 
@@ -448,40 +447,10 @@ public I_C_UOM createUOM(final String name, final int stdPrecision)
 		return uom;
 	}
 
-	public void createUOMConversion(
-			final int productId,
-			final I_C_UOM uomFrom,
-			final I_C_UOM uomTo,
-			final BigDecimal fromToMultipler,
-			final BigDecimal toFromMultiplier)
+	public void createUOMConversion(final CreateUOMConversionRequest request)
 	{
-		Services.get(IUOMConversionDAO.class).createUOMConversion(CreateUOMConversionRequest.builder()
-				.productId(ProductId.ofRepoIdOrNull(productId))
-				.fromUomId(UomId.ofRepoId(uomFrom.getC_UOM_ID()))
-				.toUomId(UomId.ofRepoId(uomTo.getC_UOM_ID()))
-				.fromToMultiplier(fromToMultipler)
-				.toFromMultiplier(toFromMultiplier)
-				.build());
-	}
-
-	public void createUOMConversion(
-			final I_M_Product product,
-			final I_C_UOM uomFrom,
-			final I_C_UOM uomTo,
-			final String fromToMultiplierStr,
-			final String toFromMultiplierStr)
-	{
-		final ProductId productId = product != null ? ProductId.ofRepoId(product.getM_Product_ID()) : null;
-		final BigDecimal fromToMultiplier = new BigDecimal(fromToMultiplierStr);
-		final BigDecimal toFromMultiplier = new BigDecimal(toFromMultiplierStr);
-
-		Services.get(IUOMConversionDAO.class).createUOMConversion(CreateUOMConversionRequest.builder()
-				.productId(productId)
-				.fromUomId(UomId.ofRepoId(uomFrom.getC_UOM_ID()))
-				.toUomId(UomId.ofRepoId(uomTo.getC_UOM_ID()))
-				.fromToMultiplier(fromToMultiplier)
-				.toFromMultiplier(toFromMultiplier)
-				.build());
+		final IUOMConversionDAO uomConversionDAO = Services.get(IUOMConversionDAO.class);
+		uomConversionDAO.createUOMConversion(request);
 	}
 
 	public I_M_Product createProduct(final String name)

backend/de.metas.business/src/main/java/de/metas/uom/CreateUOMConversionRequest.java

@@ -42,11 +42,9 @@ public class CreateUOMConversionRequest
 	UomId fromUomId;
 	@NonNull
 	UomId toUomId;
-	
+
 	@NonNull
 	BigDecimal fromToMultiplier;
-	@NonNull
-	BigDecimal toFromMultiplier;
-
+
 	boolean catchUOMForProduct;
 }

backend/de.metas.business/src/main/java/de/metas/uom/IUOMConversionBL.java

@@ -72,6 +72,12 @@ BigDecimal convertQty(
 
 	BigDecimal convertQty(UOMConversionContext conversionCtx, BigDecimal qty, UomId uomFrom, UomId uomTo);
 
+	default BigDecimal convertQty(final ProductId productId, final BigDecimal qty, final UomId uomFrom, final UomId uomTo)
+	{
+		final UOMConversionContext conversionCtx = UOMConversionContext.of(productId);
+		return convertQty(conversionCtx, qty, uomFrom, uomTo);
+	}
+
 	/**
 	 * @deprecated please consider using {@link #convertQuantityTo(Quantity, UOMConversionContext, UomId)}
 	 */

backend/de.metas.business/src/main/java/de/metas/uom/UOMConversionRate.java

@@ -1,9 +1,17 @@
 package de.metas.uom;
 
 import java.math.BigDecimal;
+import java.math.RoundingMode;
+
+import javax.annotation.Nullable;
+
+import org.adempiere.exceptions.AdempiereException;
 
 import de.metas.util.Check;
+import de.metas.util.NumberUtils;
+import lombok.AccessLevel;
 import lombok.Builder;
+import lombok.Getter;
 import lombok.NonNull;
 import lombok.Value;
 
@@ -35,7 +43,10 @@ public class UOMConversionRate
 {
 	UomId fromUomId;
 	UomId toUomId;
+
+	@Getter(AccessLevel.NONE)
 	BigDecimal fromToMultiplier;
+	@Getter(AccessLevel.NONE)
 	BigDecimal toFromMultiplier;
 
 	boolean catchUOMForProduct;
@@ -44,12 +55,13 @@ public class UOMConversionRate
 	private UOMConversionRate(
 			@NonNull final UomId fromUomId,
 			@NonNull final UomId toUomId,
-			@NonNull final BigDecimal fromToMultiplier,
-			@NonNull final BigDecimal toFromMultiplier,
+			@Nullable final BigDecimal fromToMultiplier,
+			@Nullable final BigDecimal toFromMultiplier,
 			final boolean catchUOMForProduct)
 	{
-		Check.assume(fromToMultiplier.signum() != 0, "invalid fromToMultiplier: {}", fromToMultiplier);
-		Check.assume(toFromMultiplier.signum() != 0, "invalid toFromMultiplier: {}", toFromMultiplier);
+		Check.assume(fromToMultiplier != null || toFromMultiplier != null, "at least fromToMultiplier={} or toFromMultiplier={} shall be not null", fromToMultiplier, toFromMultiplier);
+		Check.assume(fromToMultiplier == null || fromToMultiplier.signum() != 0, "invalid fromToMultiplier: {}", fromToMultiplier);
+		Check.assume(toFromMultiplier == null || toFromMultiplier.signum() != 0, "invalid toFromMultiplier: {}", toFromMultiplier);
 
 		this.fromUomId = fromUomId;
 		this.toUomId = toUomId;
@@ -85,23 +97,45 @@ public UOMConversionRate invert()
 
 	public boolean isOne()
 	{
-		return fromToMultiplier.compareTo(BigDecimal.ONE) == 0
-				&& toFromMultiplier.compareTo(BigDecimal.ONE) == 0;
+		return (fromToMultiplier == null || fromToMultiplier.compareTo(BigDecimal.ONE) == 0)
+				&& (toFromMultiplier == null || toFromMultiplier.compareTo(BigDecimal.ONE) == 0);
 	}
 
-	public BigDecimal convert(@NonNull final BigDecimal qty, @NonNull final UOMPrecision precision)
+	public BigDecimal getFromToMultiplier()
+	{
+		if (fromToMultiplier != null)
+		{
+			return fromToMultiplier;
+		}
+		else
+		{
+			return computeInvertedMultiplier(toFromMultiplier);
+		}
+	}
+
+	public static BigDecimal computeInvertedMultiplier(@NonNull final BigDecimal multiplier)
 	{
-		BigDecimal qtyConverted = convert(qty);
-		return precision.round(qtyConverted);
+		if (multiplier.signum() == 0)
+		{
+			throw new AdempiereException("Multiplier shall not be ZERO");
+		}
+		return NumberUtils.stripTrailingDecimalZeros(BigDecimal.ONE.divide(multiplier, 12, RoundingMode.HALF_UP));
 	}
 
-	public BigDecimal convert(@NonNull final BigDecimal qty)
+	public BigDecimal convert(@NonNull final BigDecimal qty, @NonNull final UOMPrecision precision)
 	{
 		if (qty.signum() == 0)
 		{
 			return qty;
 		}
 
-		return qty.multiply(fromToMultiplier);
+		if (fromToMultiplier != null)
+		{
+			return precision.round(qty.multiply(fromToMultiplier));
+		}
+		else
+		{
+			return qty.divide(toFromMultiplier, precision.toInt(), precision.getRoundingMode());
+		}
 	}
 }

backend/de.metas.business/src/main/java/de/metas/uom/UOMPrecision.java

@@ -52,6 +52,7 @@ public static UOMPrecision ofInt(final int precision)
 
 	public static final UOMPrecision ZERO = new UOMPrecision(0);
 	public static final UOMPrecision TWO = new UOMPrecision(2);
+	public static final UOMPrecision TWELVE = new UOMPrecision(12);
 
 	private static final UOMPrecision[] cachedValues = new UOMPrecision[] {
 			ZERO,
@@ -66,7 +67,7 @@ public static UOMPrecision ofInt(final int precision)
 			new UOMPrecision(9),
 			new UOMPrecision(10),
 			new UOMPrecision(11),
-			new UOMPrecision(12),
+			TWELVE,
 	};
 
 	private final int precision;

backend/de.metas.business/src/main/java/de/metas/uom/impl/UOMConversionBL.java

@@ -507,7 +507,6 @@ private Optional<UOMConversionRate> getTimeConversionRate(@NonNull final I_C_UOM
 				.fromUomId(fromTimeUomId)
 				.toUomId(toTimeUomId)
 				.fromToMultiplier(fromToMultiplier)
-				.toFromMultiplier(BigDecimal.ONE.divide(fromToMultiplier, 12, RoundingMode.HALF_UP))
 				.build());
 
 	}
@@ -802,7 +801,7 @@ public ProductPrice convertProductPriceToUom(
 		}
 
 		final UOMConversionRate rate = getRate(price.getProductId(), toUomId, price.getUomId());
-		final BigDecimal priceConv = pricePrecision.round(rate.convert(price.toBigDecimal()));
+		final BigDecimal priceConv = pricePrecision.round(rate.convert(price.toBigDecimal(), UOMPrecision.TWELVE));
 
 		return price.withValueAndUomId(priceConv, toUomId);
 	}

backend/de.metas.business/src/main/java/de/metas/uom/impl/UOMConversionDAO.java

@@ -4,12 +4,13 @@
 import static org.adempiere.model.InterfaceWrapperHelper.saveRecord;
 
 import java.math.BigDecimal;
+import java.util.Objects;
 
 import org.adempiere.ad.dao.IQueryBL;
 import org.compiere.model.I_C_UOM_Conversion;
 import org.slf4j.Logger;
 
-import java.util.Objects;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 
 import de.metas.cache.CCache;
@@ -80,46 +81,40 @@ public UOMConversionsMap getGenericConversions()
 				.build();
 	}
 
-	private static UOMConversionRate toUOMConversionOrNull(@NonNull final I_C_UOM_Conversion record)
+	@VisibleForTesting
+	static UOMConversionRate toUOMConversionOrNull(@NonNull final I_C_UOM_Conversion record)
 	{
 		final BigDecimal fromToMultiplier = record.getMultiplyRate();
-		BigDecimal toFromMultiplier = record.getDivideRate();
-		if (toFromMultiplier.signum() == 0 && fromToMultiplier.signum() != 0)
-		{
-			// In case divide rate is not available, calculate divide rate as 1/multiplyRate (precision=12)
-			toFromMultiplier = BigDecimal.ONE.divide(fromToMultiplier, 12, BigDecimal.ROUND_HALF_UP);
-		}
-
 		if (fromToMultiplier.signum() == 0)
 		{
 			logger.warn("Invalid conversion {}: multiplyRate is zero", record);
 			return null;
 		}
-		if (toFromMultiplier.signum() == 0)
-		{
-			logger.warn("Invalid conversion {}: divideRate is zero", record);
-			return null;
-		}
+
+		// NOTE: Don't use C_UOM_Conversion.DivideRate because that's always calculated as 1/MultiplyRate, rounded HALF_UP at 12 decimals.
+		// This approach can introduce calculation errors in some cases, so better let the API calculate it when needed, using the correct precision.
 
 		return UOMConversionRate.builder()
 				.fromUomId(UomId.ofRepoId(record.getC_UOM_ID()))
 				.toUomId(UomId.ofRepoId(record.getC_UOM_To_ID()))
 				.fromToMultiplier(fromToMultiplier)
-				.toFromMultiplier(toFromMultiplier)
 				.catchUOMForProduct(record.isCatchUOMForProduct())
 				.build();
 	}
 
 	@Override
 	public void createUOMConversion(@NonNull final CreateUOMConversionRequest request)
 	{
+		final BigDecimal fromToMultiplier = request.getFromToMultiplier();
+		final BigDecimal toFromMultiplier = UOMConversionRate.computeInvertedMultiplier(fromToMultiplier);
+
 		final I_C_UOM_Conversion record = newInstance(I_C_UOM_Conversion.class);
 
 		record.setM_Product_ID(ProductId.toRepoId(request.getProductId()));
 		record.setC_UOM_ID(request.getFromUomId().getRepoId());
 		record.setC_UOM_To_ID(request.getToUomId().getRepoId());
-		record.setMultiplyRate(request.getFromToMultiplier());
-		record.setDivideRate(request.getToFromMultiplier());
+		record.setMultiplyRate(fromToMultiplier);
+		record.setDivideRate(toFromMultiplier);
 		record.setIsCatchUOMForProduct(request.isCatchUOMForProduct());
 
 		saveRecord(record);

backend/de.metas.business/src/test/java/de/metas/business/BusinessTestHelper.java

@@ -77,7 +77,6 @@ public final class BusinessTestHelper
 	 */
 	private static final int UOM_Precision_3 = 3;
 
-
 	public static CountryId createCountry(@NonNull final String countryCode)
 	{
 		final I_C_Country record = newInstance(I_C_Country.class);
@@ -142,35 +141,10 @@ public static I_C_UOM createUOM(final String name, final String x12de355)
 		return uom;
 	}
 
-	public static void createUOMConversion(
-			@Nullable final I_M_Product product,
-			@NonNull final I_C_UOM uomFrom,
-			@NonNull final I_C_UOM uomTo,
-			@NonNull final BigDecimal fromToMultiplier,
-			@NonNull final BigDecimal toFromMultiplier)
-	{
-		createUOMConversion(
-				product != null ? ProductId.ofRepoId(product.getM_Product_ID()) : null,
-				UomId.ofRepoId(uomFrom.getC_UOM_ID()),
-				UomId.ofRepoId(uomTo.getC_UOM_ID()),
-				fromToMultiplier,
-				toFromMultiplier);
-	}
-
-	public static void createUOMConversion(
-			@Nullable final ProductId productId,
-			@NonNull final UomId uomFromId,
-			@NonNull final UomId uomToId,
-			@NonNull final BigDecimal fromToMultiplier,
-			@NonNull final BigDecimal toFromMultiplier)
+	public static void createUOMConversion(@NonNull final CreateUOMConversionRequest request)
 	{
-		Services.get(IUOMConversionDAO.class).createUOMConversion(CreateUOMConversionRequest.builder()
-				.productId(productId)
-				.fromUomId(uomFromId)
-				.toUomId(uomToId)
-				.fromToMultiplier(fromToMultiplier)
-				.toFromMultiplier(toFromMultiplier)
-				.build());
+		final IUOMConversionDAO uomConversionDAO = Services.get(IUOMConversionDAO.class);
+		uomConversionDAO.createUOMConversion(request);
 	}
 
 	public static CurrencyId getEURCurrencyId()

backend/de.metas.business/src/test/java/de/metas/customs/CustomsInvoiceServiceTest.java

@@ -100,7 +100,6 @@ public class CustomsInvoiceServiceTest
 	private UomId uom1;
 	private UomId uom2;
 	private final BigDecimal convertionMultiplier = new BigDecimal("2");
-	private final BigDecimal conversionDivisor = new BigDecimal("0.5");
 
 	@BeforeEach
 	public void init()
@@ -133,7 +132,6 @@ public void init()
 				.fromUomId(uom2)
 				.toUomId(uom1)
 				.fromToMultiplier(convertionMultiplier)
-				.toFromMultiplier(conversionDivisor)
 				.build());
 
 		final PlainCurrencyDAO currencyDAO = (PlainCurrencyDAO)Services.get(ICurrencyDAO.class);