Wrong scale used in `AbstractCurrencyConversion#roundFactor` method?

[peterquiel]

I belive the roundFactor method in AbstractCurrencyConversion uses a wrong scale in case the amount has a context and the allowes a much higher scale then the factor has.

The documentation says:

    /**
     * Optionally rounds the factor to be used. By default this method will only round
     * as much as it is needed, so the factor can be handled by the target amount instance based on its
     * numeric capabilities. Rounding is applied only if {@code amount.getContext().getMaxScale() > 0} as follows:
     * <ul>
     *     <li>If the amount provides a {@link MathContext} as context property this is used.</li>
     *     <li>If the amount provides a {@link RoundingMode}, this is used (default is
     *     {@code RoundingMode.HALF_EVEN}).</li>
     *     <li>By default the scale used is scale of the conversion factor. If the acmount allows a higher
     *     scale based on {@code amount.getContext().getMaxScale()}, this higher scale is used.</li>
     * </ul>

The last point is interesting:

If the acmount allows a higher scale based on {@code amount.getContext().getMaxScale()}, this higher scale is used.</li>

The code:

                int scale = factor.getScale();
                if (factor.getScale() > amount.getContext().getMaxScale()) {
                    scale = amount.getContext().getMaxScale();
                }

Looks wrong to me. Should be a < instead of >, don't you think?

[keilw]

If that's all, it is a minor thing, should be easy to apply with the next RI release. Thanks.

[Shenker93]

Seemed strange to me, so I've done some debug & found out that @peterquiel is right. This bug affects FastMoney subclass of MonetaryAmount only, Money BigDecimal-based implementation doesn't use this branch of roundFactor() at all, amount.getContext().getMaxScale() returns -1 on it. I've used this implementation in my project, so haven't noticed such behavior too. Now I'm going to open PR with this fix & some basic unit test for currencies conversion.

[LucCappellaro]

This is a show stopper because the conversion is by default just plain wrong.

@Test
public void testECBv1_4_2() {
    Number value = BigDecimal.valueOf(1000);
    CurrencyUnit base = Monetary.getCurrency("EUR");
    CurrencyUnit term = Monetary.getCurrency("USD");
    LocalDate date = LocalDate.of(2022, 4, 20);// ECB rate=1.083

    MonetaryAmount amount = Monetary.getAmountFactory(Money.class).setNumber(value).setCurrency(base).create();
    ConversionQuery q = ConversionQueryBuilder.of().setBaseCurrency(base).setTermCurrency(term).set(date).build();
    CurrencyConversion convert = MonetaryConversions.getExchangeRateProvider("ECB-HIST").getCurrencyConversion(q);

    assertEquals("USD 1083.00", amount.with(convert).toString()); // But returns "USD 1080.00"
}

It does impact Money.class too because the scale is by default 63. https://github.com/JavaMoney/jsr354-ri/blob/ac0afde6f37ecc8a9a30c3386b2350dbe7d37459/moneta-core/src/main/java/org/javamoney/moneta/spi/MoneyAmountFactory.java#L36