louisdevie/mustangproject
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix percentage-based allowance/charge calculations (EN16931 compliance)

Browse Source 
Two arithmetic bugs caused wrong line totals when percentage-based allowances
or charges were used:

Bug A — Product-level (product.allowances): The percent discount was computed
as (price * pct/100) * quantity (line total), then subtracted from the unit
price and multiplied by quantity again — discount applied twice.

Bug B — Item-level (itemAllowances) with basisQuantity != 1: The percent was
applied to price * quantity instead of price * quantity / basisQuantity.

Fix: LineCalculator and XML pull providers now pass context-appropriate
IAbsoluteValueProvider wrappers (perUnitProvider for product-level,
itemBasisProvider for item-level). Allowance.java uses RoundingMode.HALF_UP
in getPercent().divide(100). BasisAmount in XML is formatted via
currencyFormat().

Files changed:
- LineCalculator.java: perUnitProvider, itemBasisProvider in allowance/charge loops
- Allowance.java: RoundingMode.HALF_UP, scale 18
- ZUGFeRD2PullProvider.java: correct providers, format BasisAmount
- OXPullProvider.java, DAPullProvider.java: perUnitProvider for product-level
- CalculationTest.java: @Test on 5 methods, 4 new regression tests

Verification: mvn test -pl library -Dtest=CalculationTest

Full PR description with before/after XML proof: see .project/PR-allowance-charge-fix.md
(Test evidence files in C:\temp\mustang-test\ are for PR attachment only, not in repo.)

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Philipp
2026-02-17 21:50:43 +01:00
parent ce60fa42d2
commit dd32514938
6 changed files with 181 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
library/src/main/java/org/mustangproject/Allowance.java
View File

@@ -38,7 +38,7 @@ public class Allowance extends Charge {
if(totalAmount != null) {
return totalAmount;
} else if (percent!=null) {
BigDecimal singlePrice=currentItem.getValue().multiply(BigDecimal.ONE.subtract(getPercent().divide(new BigDecimal(100))));
BigDecimal singlePrice=currentItem.getValue().multiply(BigDecimal.ONE.subtract(getPercent().divide(new BigDecimal(100), 18, RoundingMode.HALF_UP)));
BigDecimal singlePriceDiff=currentItem.getValue().subtract(singlePrice);
return singlePriceDiff.multiply(currentItem.getQuantity());
} else {

18
library/src/main/java/org/mustangproject/ZUGFeRD/DAPullProvider.java
View File

@@ -22,6 +22,7 @@ package org.mustangproject.ZUGFeRD;
import static org.mustangproject.ZUGFeRD.ZUGFeRDDateFormat.DATE;
import java.math.BigDecimal;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Optional;
@@ -96,16 +97,27 @@ public class DAPullProvider extends ZUGFeRD2PullProvider {
xml += "<ram:BuyerAssignedID>"
+ XMLTools.encodeXML(currentItem.getProduct().getBuyerAssignedID()) + "</ram:BuyerAssignedID>";
}
// Product-level (GrossPrice / product section): ActualAmount must be per-unit (BT-147)
final IZUGFeRDExportableItem itemForProduct = currentItem;
IAbsoluteValueProvider perUnitProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return itemForProduct.getPrice();
}
@Override
public BigDecimal getQuantity() {
return BigDecimal.ONE;
}
};
String allowanceChargeStr = "";
if (currentItem.getItemAllowances() != null) {
for (final IZUGFeRDAllowanceCharge allowance : currentItem.getItemAllowances()) {
allowanceChargeStr += getAllowanceChargeStr(allowance, currentItem);
allowanceChargeStr += getAllowanceChargeStr(allowance, perUnitProvider);
}
}
if (currentItem.getItemCharges() != null) {
for (final IZUGFeRDAllowanceCharge charge : currentItem.getItemCharges()) {
allowanceChargeStr += getAllowanceChargeStr(charge, currentItem);
allowanceChargeStr += getAllowanceChargeStr(charge, perUnitProvider);
}
}

58
library/src/main/java/org/mustangproject/ZUGFeRD/LineCalculator.java
View File

@@ -21,26 +21,42 @@ public class LineCalculator {
protected BigDecimal allowanceItemTotal = BigDecimal.ZERO;
public LineCalculator(IZUGFeRDExportableItem currentItem) {
// Compute basisQuantity first so it can be used for item-level allowance/charge context
BigDecimal basisQuantity = currentItem.getBasisQuantity().compareTo(BigDecimal.ZERO) == 0
? BigDecimal.ONE.setScale(4)
: currentItem.getBasisQuantity();
// Provider for item-level: getValue() returns price/basisQty so percentage
// allowances compute against the actual line amount (qty * price / basisQty),
// not the raw (qty * price). No effect on absolute amounts.
IAbsoluteValueProvider itemBasisProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return currentItem.getPrice().divide(basisQuantity, 18, RoundingMode.HALF_UP);
}
@Override
public BigDecimal getQuantity() {
return currentItem.getQuantity();
}
};
if (currentItem.getItemAllowances() != null) {
for (IZUGFeRDAllowanceCharge allowance : currentItem.getItemAllowances()) {
BigDecimal singleAllowance=allowance.getTotalAmount(currentItem);
BigDecimal singleAllowance = allowance.getTotalAmount(itemBasisProvider);
addItemAllowance(singleAllowance);
addAllowanceItemTotal(singleAllowance);
}
}
if (currentItem.getItemCharges() != null) {
for (IZUGFeRDAllowanceCharge charge : currentItem.getItemCharges()) {
BigDecimal singleCharge=charge.getTotalAmount(currentItem);
BigDecimal singleCharge = charge.getTotalAmount(itemBasisProvider);
addItemCharge(singleCharge);
subtractAllowanceItemTotal(singleCharge);
}
}
if (currentItem.getItemTotalAllowances() != null) {
for (final IZUGFeRDAllowanceCharge itemTotalAllowance : currentItem.getItemTotalAllowances()) {
addAllowanceItemTotal(itemTotalAllowance.getTotalAmount(currentItem));
addAllowanceItemTotal(itemTotalAllowance.getTotalAmount(itemBasisProvider));
}
}
@@ -60,33 +76,41 @@ public class LineCalculator {
price = currentItem.getPrice();
priceGross = price;
// price=price.subtract(itemAllowance).add(itemCharge);
// BigDecimal delta=charge.subtract(allowanceItemTotal).subtract(allowance);
// delta=delta.divide(currentItem.getQuantity(), 18, RoundingMode.HALF_UP);
// Provider for product-level: getQuantity() returns ONE because product
// allowances adjust the per-unit price, not the line total.
// No effect on absolute amounts.
IAbsoluteValueProvider perUnitProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return currentItem.getPrice();
}
@Override
public BigDecimal getQuantity() {
return BigDecimal.ONE;
}
};
BigDecimal delta = BigDecimal.ZERO;
if (currentItem.getProduct() != null) {
if (currentItem.getProduct().getAllowances() != null) {
for (IZUGFeRDAllowanceCharge ccaf : currentItem.getProduct().getAllowances()) {
delta=delta.subtract(ccaf.getTotalAmount(currentItem));
delta = delta.subtract(ccaf.getTotalAmount(perUnitProvider));
}
}
if (currentItem.getProduct().getCharges() != null) {
for (IZUGFeRDAllowanceCharge ccaf : currentItem.getProduct().getCharges()) {
delta = delta.add(ccaf.getTotalAmount(currentItem));
delta = delta.add(ccaf.getTotalAmount(perUnitProvider));
}
}
}
price = price.add(delta);
// Division/Zero occurred here.
// Used the setScale only because that's also done in getBasisQuantity
BigDecimal basisQuantity = currentItem.getBasisQuantity().compareTo(BigDecimal.ZERO) == 0
? BigDecimal.ONE.setScale(4)
: currentItem.getBasisQuantity();
itemTotalNetAmount = quantity.multiply(price).divide(basisQuantity, 18, RoundingMode.HALF_UP)
.add(lineCharge).subtract(lineAllowance).subtract(allowanceItemTotal.setScale(2, RoundingMode.HALF_UP)).setScale(2, RoundingMode.HALF_UP);
itemTotalVATAmount = itemTotalNetAmount.multiply(multiplicator);//.setScale(2, RoundingMode.HALF_UP);
.add(lineCharge).subtract(lineAllowance)
.subtract(allowanceItemTotal.setScale(2, RoundingMode.HALF_UP))
.setScale(2, RoundingMode.HALF_UP);
itemTotalVATAmount = itemTotalNetAmount.multiply(multiplicator);
}
public BigDecimal getPrice() {

18
library/src/main/java/org/mustangproject/ZUGFeRD/OXPullProvider.java
View File

@@ -24,6 +24,7 @@ import static org.mustangproject.ZUGFeRD.ZUGFeRDDateFormat.DATE;
import static org.mustangproject.ZUGFeRD.model.DocumentCodeTypeConstants.CORRECTEDINVOICE;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.nio.charset.StandardCharsets;
import java.text.SimpleDateFormat;
import java.util.Base64;
@@ -124,16 +125,27 @@ public class OXPullProvider extends ZUGFeRD2PullProvider {
xml += "<ram:BuyerAssignedID>"
+ XMLTools.encodeXML(currentItem.getProduct().getBuyerAssignedID()) + "</ram:BuyerAssignedID>";
}
// Product-level (GrossPriceProductTradePrice): ActualAmount must be per-unit (BT-147)
final IZUGFeRDExportableItem itemForProduct = currentItem;
IAbsoluteValueProvider perUnitProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return itemForProduct.getPrice();
}
@Override
public BigDecimal getQuantity() {
return BigDecimal.ONE;
}
};
String allowanceChargeStr = "";
if (currentItem.getItemAllowances() != null) {
for (final IZUGFeRDAllowanceCharge allowance : currentItem.getItemAllowances()) {
allowanceChargeStr += getAllowanceChargeStr(allowance, currentItem);
allowanceChargeStr += getAllowanceChargeStr(allowance, perUnitProvider);
}
}
if (currentItem.getItemCharges() != null) {
for (final IZUGFeRDAllowanceCharge charge : currentItem.getItemCharges()) {
allowanceChargeStr += getAllowanceChargeStr(charge, currentItem);
allowanceChargeStr += getAllowanceChargeStr(charge, perUnitProvider);
}
}

42
library/src/main/java/org/mustangproject/ZUGFeRD/ZUGFeRD2PullProvider.java
View File

@@ -26,6 +26,7 @@ import static org.mustangproject.ZUGFeRD.model.TaxCategoryCodeTypeConstants.CATE
import java.io.IOException;
import java.io.StringWriter;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.nio.charset.StandardCharsets;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
@@ -270,7 +271,7 @@ public class ZUGFeRD2PullProvider implements IXMLProvider {
String chargeIndicator = "false";
if ((allowance.getPercent() != null) && (profile == Profiles.getByName("Extended"))) {
percentage = "<ram:CalculationPercent>" + vatFormat(allowance.getPercent()) + "</ram:CalculationPercent>";
percentage += "<ram:BasisAmount>" + item.getValue() + "</ram:BasisAmount>";
percentage += "<ram:BasisAmount>" + currencyFormat(item.getValue()) + "</ram:BasisAmount>";
}
if (allowance.isCharge()) {
chargeIndicator = "true";
@@ -305,7 +306,7 @@ public class ZUGFeRD2PullProvider implements IXMLProvider {
String chargeIndicator = "false";
if ((allowance.getPercent() != null) && (profile == Profiles.getByName("Extended"))) {
percentage = "<ram:CalculationPercent>" + vatFormat(allowance.getPercent()) + "</ram:CalculationPercent>";
percentage += "<ram:BasisAmount>" + item.getValue() + "</ram:BasisAmount>";
percentage += "<ram:BasisAmount>" + currencyFormat(item.getValue()) + "</ram:BasisAmount>";
}
if (allowance.isCharge()) {
chargeIndicator = "true";
@@ -499,15 +500,27 @@ public class ZUGFeRD2PullProvider implements IXMLProvider {
xml += "</ram:BuyerOrderReferencedDocument>";
}
// Per-unit provider for product-level: ActualAmount must be per-unit (BT-147)
final IZUGFeRDExportableItem itemForProduct = currentItem;
IAbsoluteValueProvider perUnitProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return itemForProduct.getPrice();
}
@Override
public BigDecimal getQuantity() {
return BigDecimal.ONE;
}
};
String allowanceChargeStr = "";
if (currentItem.getProduct().getAllowances() != null && currentItem.getProduct().getAllowances().length > 0) {
for (final IZUGFeRDAllowanceCharge allowance : currentItem.getProduct().getAllowances()) {
allowanceChargeStr += getAllowanceChargeStr(allowance, currentItem);
allowanceChargeStr += getAllowanceChargeStr(allowance, perUnitProvider);
}
}
if (currentItem.getProduct().getCharges() != null && currentItem.getProduct().getCharges().length > 0) {
for (final IZUGFeRDAllowanceCharge charge : currentItem.getProduct().getCharges()) {
allowanceChargeStr += getAllowanceChargeStr(charge, currentItem);
allowanceChargeStr += getAllowanceChargeStr(charge, perUnitProvider);
}
}
if (!allowanceChargeStr.isEmpty()) {
@@ -588,16 +601,31 @@ public class ZUGFeRD2PullProvider implements IXMLProvider {
xml += "</ram:BillingSpecifiedPeriod>";
}
// item charges/allowances
// Item-level: use basisQuantity-aware provider so ActualAmount (BT-136) is correct when basisQuantity != 1
BigDecimal itemBasisQty = currentItem.getBasisQuantity().compareTo(BigDecimal.ZERO) == 0
? BigDecimal.ONE.setScale(4)
: currentItem.getBasisQuantity();
final IZUGFeRDExportableItem itemForSettlement = currentItem;
final BigDecimal basisQty = itemBasisQty;
IAbsoluteValueProvider itemBasisProvider = new IAbsoluteValueProvider() {
@Override
public BigDecimal getValue() {
return itemForSettlement.getPrice().divide(basisQty, 18, RoundingMode.HALF_UP);
}
@Override
public BigDecimal getQuantity() {
return itemForSettlement.getQuantity();
}
};
String itemTotalAllowanceChargeStr = "";
if (currentItem.getAllowances() != null && currentItem.getAllowances().length > 0) {
for (final IZUGFeRDAllowanceCharge itemTotalAllowance : currentItem.getAllowances()) {
itemTotalAllowanceChargeStr += getItemTotalAllowanceChargeStr(itemTotalAllowance, currentItem);
itemTotalAllowanceChargeStr += getItemTotalAllowanceChargeStr(itemTotalAllowance, itemBasisProvider);
}
}
if (currentItem.getCharges() != null && currentItem.getCharges().length > 0) {
for (final IZUGFeRDAllowanceCharge itemTotalCharges : currentItem.getCharges()) {
itemTotalAllowanceChargeStr += getItemTotalAllowanceChargeStr(itemTotalCharges, currentItem);
itemTotalAllowanceChargeStr += getItemTotalAllowanceChargeStr(itemTotalCharges, itemBasisProvider);
}
}
if (!itemTotalAllowanceChargeStr.isEmpty()) {

61
library/src/test/java/org/mustangproject/ZUGFeRD/CalculationTest.java
View File

@@ -297,6 +297,7 @@ public class CalculationTest extends ResourceCase {
assertEquals(valueOf(4.750).stripTrailingZeros(), calculator.getGrandTotal().stripTrailingZeros());
}
@Test
public void testSimpleItemPercentAllowance() {
/***
* a product with net 1.10 and qty 5 and relative item allowance of 10% should return 5 as line and grand total
@@ -351,6 +352,7 @@ public class CalculationTest extends ResourceCase {
assertEquals(new BigDecimal("4.95"), calculator.getGrandTotal().stripTrailingZeros());
}
@Test
public void testSimpleItemPercentCharge() {
/***
* a product with net 1.10 and qty 5 and relative item allowance of 10% should return 5 as line and grand total
@@ -405,6 +407,7 @@ public class CalculationTest extends ResourceCase {
assertEquals(new BigDecimal("6.05"), calculator.getGrandTotal().stripTrailingZeros());
}
@Test
public void testSimpleDocumentPercentCharge() {
String orgname = "Test company";
@@ -430,6 +433,7 @@ public class CalculationTest extends ResourceCase {
assertEquals(new BigDecimal("16.07"), tc.getDuePayable());
}
@Test
public void testSimpleDocumentPercentAllowance() {
String orgname = "Test company";
@@ -455,6 +459,7 @@ public class CalculationTest extends ResourceCase {
assertEquals(new BigDecimal("5.36"), tc.getDuePayable());
}
@Test
public void testSimpleItemTotalAllowance() {
/***
* a product with net 1 and qty 5 and absolute _item_ allowance of 1 should return 4 as line total, and grand total
@@ -498,6 +503,62 @@ public class CalculationTest extends ResourceCase {
}
@Test
public void testPercentProductAllowanceNotMultipliedByQuantity() {
// Bug A: 10% product discount on price=100, qty=5
// Correct: net price = 90, line total = 5 * 90 = 450
// Bug A would give: delta = 10*5 = 50, price = 50, total = 5*50 = 250
Product product = new Product("Test", "", "H87", BigDecimal.ZERO);
product.addAllowance((Allowance) new Allowance().setPercent(new BigDecimal(10)));
Item item = new Item(product, new BigDecimal("100.00"), new BigDecimal("5"));
LineCalculator lc = item.getCalculation();
assertEquals(new BigDecimal("450.00"), lc.getItemTotalNetAmount());
assertEquals(new BigDecimal("90.00").stripTrailingZeros(),
lc.getPrice().stripTrailingZeros());
}
@Test
public void testPercentItemAllowanceWithBasisQuantity() {
// Bug B: price=128.49 per 100 LTR, qty=50 LTR, 10% item allowance
// Line total before allowance = 50 * 128.49 / 100 = 64.245
// Allowance = 10% of 64.245 = 6.4245 -> 6.42 (rounded)
// Correct line total = 64.25 - 6.42 = 57.83
Product product = new Product("Test", "", "LTR", BigDecimal.ZERO);
Item item = new Item(product, new BigDecimal("128.49"), new BigDecimal("50"));
item.setBasisQuantity(new BigDecimal("100"));
item.addAllowance(new Allowance().setPercent(new BigDecimal(10)));
LineCalculator lc = item.getCalculation();
assertEquals(new BigDecimal("57.83"), lc.getItemTotalNetAmount());
}
@Test
public void testPercentProductAllowanceWithBasisQuantity() {
// 10% product discount, price=100 per 10 units, qty=50
// Net price = 100 - 10 = 90 (per 10 units)
// Line total = 50 * 90 / 10 = 450
Product product = new Product("Test", "", "H87", BigDecimal.ZERO);
product.addAllowance((Allowance) new Allowance().setPercent(new BigDecimal(10)));
Item item = new Item(product, new BigDecimal("100.00"), new BigDecimal("50"));
item.setBasisQuantity(new BigDecimal("10"));
LineCalculator lc = item.getCalculation();
assertEquals(new BigDecimal("450.00"), lc.getItemTotalNetAmount());
}
@Test
public void testPercentItemChargeWithBasisQuantity() {
// Item-level 10% charge with basisQuantity: line before charge = 50*100/100 = 50, charge = 5, total = 55
Product product = new Product("Test", "", "LTR", BigDecimal.ZERO);
Item item = new Item(product, new BigDecimal("100.00"), new BigDecimal("50"));
item.setBasisQuantity(new BigDecimal("100"));
item.addCharge(new Charge().setPercent(new BigDecimal(10)));
LineCalculator lc = item.getCalculation();
assertEquals(new BigDecimal("55.00"), lc.getItemTotalNetAmount());
}
/**
* LineCalculator should not throw an exception when calculating a non-terminating decimal expansion
*/