You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2007/09/21 21:55:15 UTC
Re: svn commit: r571291 - in /ofbiz/trunk/applications/accounting:
config/ script/org/ofbiz/accounting/finaccount/
src/org/ofbiz/accounting/finaccount/ src/org/ofbiz/accounting/payment/
Same question : "I wonder if we should not back port this in release. There are changes but mostly it's a fix."
WDYT ?
Jacques
> Author: jonesde
> Date: Thu Aug 30 13:08:43 2007
> New Revision: 571291
>
> URL: http://svn.apache.org/viewvc?rev=571291&view=rev
> Log:
> A number of fixes/improvements related to handling funny numbers properly using BigDecimal for calculations, and rounding incoming
Double values which can be (often are) weird
>
> Modified:
> ofbiz/trunk/applications/accounting/config/arithmetic.properties
> ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java
>
> Modified: ofbiz/trunk/applications/accounting/config/arithmetic.properties
> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/config/arithmetic.properties?rev=571291&r1=571290&r2=571291&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/config/arithmetic.properties (original)
> +++ ofbiz/trunk/applications/accounting/config/arithmetic.properties Thu Aug 30 13:08:43 2007
> @@ -37,6 +37,7 @@
> # such as Financial Accounts
> finaccount.decimals = 2
> finaccount.rounding = ROUND_HALF_UP
> +finaccount.roundingSimpleMethod = HalfUp
>
> # Most companies would want their sales tax calculations ALWAYS to round up (ie, 100.081 becomes 100.09)
> # This could be ROUND_CEILING or ROUND_UP. (The difference is that ROUND_CEILING rounds towards positive infinity,
>
> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml
> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml?rev=571291&r1=571290&r2=571291&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml (original)
> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/finaccount/FinAccountServices.xml Thu Aug 30 13:08:43 2007
> @@ -20,8 +20,16 @@
>
> <simple-methods xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
> xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/simple-methods.xsd">
> +
> + <simple-method method-name="getArithmeticSettingsInline" short-description="getArithmeticSettingsInline">
> + <property-to-field resource="arithmetic" property="finaccount.decimals" field-name="roundingDecimals" default="2"/>
> + <property-to-field resource="arithmetic" property="finaccount.roundingSimpleMethod" field-name="roundingMode"
default="HalfUp"/>
> + <log level="info" message="Got settings from arithmetic.properties: roundingDecimals=${roundingDecimals},
roundingMode=${roundingMode}"/>
> + </simple-method>
>
> <simple-method method-name="createFinAccount" short-description="Create a Financial Account">
> + <call-simple-method method-name="getArithmeticSettingsInline"/>
> +
> <!-- if no statusId set to default to FNACT_ACTIVE -->
> <if-empty field-name="parameters.statusId">
> <set field="parameters.statusId" value="FNACT_ACTIVE"/>
> @@ -63,6 +71,11 @@
> </then>
> </if>
> </if-empty>
> +
> + <!-- make sure the replenishLevel is a sane number -->
> + <calculate field-name="newEntity.replenishLevel" decimal-scale="${roundingDecimals}" rounding-mode="${roundingMode}">
> + <calcop operator="get" field-name="newEntity.replenishLevel"/>
> + </calculate>
>
> <create-value value-name="newEntity"/>
> <field-to-result field-name="finAccountId" map-name="newEntity" result-name="finAccountId"/>
> @@ -71,6 +84,8 @@
> <call-service service-name="createFinAccountStatus" in-map-name="createFinAccountStatusMap"/>
> </simple-method>
> <simple-method method-name="updateFinAccount" short-description="Update a Financial Account">
> + <call-simple-method method-name="getArithmeticSettingsInline"/>
> +
> <entity-one entity-name="FinAccount" value-name="lookedUpValue"/>
> <field-to-result field-name="lookedUpValue.replenishPaymentId" result-name="oldReplenishPaymentId"/>
> <field-to-result field-name="lookedUpValue.replenishLevel" result-name="oldReplenishLevel"/>
> @@ -106,6 +121,11 @@
> </if>
>
> <set-nonpk-fields map-name="parameters" value-name="lookedUpValue"/>
> +
> + <!-- make sure the replenishLevel is a sane number -->
> + <calculate field-name="lookedUpValue.replenishLevel" decimal-scale="${roundingDecimals}" rounding-mode="${roundingMode}">
> + <calcop operator="get" field-name="lookedUpValue.replenishLevel"/>
> + </calculate>
> <store-value value-name="lookedUpValue"/>
>
> <field-to-result field-name="lookedUpValue.replenishPaymentId" result-name="replenishPaymentId"/>
> @@ -140,6 +160,8 @@
>
> <!-- FinAccountTrans Services -->
> <simple-method method-name="createFinAccountTrans" short-description="Create a Financial Account Transaction">
> + <call-simple-method method-name="getArithmeticSettingsInline"/>
> +
> <make-value value-name="newEntity" entity-name="FinAccountTrans"/>
> <set-nonpk-fields value-name="newEntity" map-name="parameters"/>
> <sequenced-id-to-env sequence-name="FinAccountTrans" env-name="newEntity.finAccountTransId"/>
> @@ -154,6 +176,15 @@
>
> <set field="newEntity.performedByPartyId" from-field="userLogin.partyId"/>
>
> + <!-- make sure the amount is a sane number -->
> + <set field="originalAmount" from-field="newEntity.amount"/>
> + <calculate field-name="newEntity.amount" decimal-scale="${roundingDecimals}" rounding-mode="${roundingMode}">
> + <calcop operator="get" field-name="newEntity.amount"/>
> + </calculate>
> + <if-compare-field field-name="newEntity.amount" operator="not-equals" to-field-name="originalAmount" type="BigDecimal">
> + <log level="warning" message="In createFinAccountTrans had to round the amount from [${originalAmount}] to
[${newEntity.amount}]"></log>
> + </if-compare-field>
> +
> <create-value value-name="newEntity"/>
> <field-to-result field-name="newEntity.finAccountTransId" result-name="finAccountTransId"/>
> </simple-method>
> @@ -182,6 +213,8 @@
>
> <!-- FinAccountAuth Services -->
> <simple-method method-name="createFinAccountAuth" short-description="Create a Financial Account Authorization">
> + <call-simple-method method-name="getArithmeticSettingsInline"/>
> +
> <make-value value-name="newEntity" entity-name="FinAccountAuth"/>
> <set-nonpk-fields value-name="newEntity" map-name="parameters"/>
>
> @@ -194,6 +227,16 @@
> <if-empty field-name="newEntity.fromDate">
> <set from-field="nowTimestamp" field="newEntity.fromDate"/>
> </if-empty>
> +
> + <!-- make sure the amount is a sane number -->
> + <set field="originalAmount" from-field="newEntity.amount"/>
> + <calculate field-name="newEntity.amount" decimal-scale="${roundingDecimals}" rounding-mode="${roundingMode}">
> + <calcop operator="get" field-name="newEntity.amount"/>
> + </calculate>
> + <if-compare-field field-name="newEntity.amount" operator="not-equals" to-field-name="originalAmount" type="BigDecimal">
> + <log level="warning" message="In createFinAccountAuth had to round the amount from [${originalAmount}] to
[${newEntity.amount}]"></log>
> + </if-compare-field>
> +
> <create-value value-name="newEntity"/>
> <field-to-result field-name="newEntity.finAccountAuthId" result-name="finAccountAuthId"/>
> </simple-method>
> @@ -232,6 +275,8 @@
> <call-simple-method method-name="inlineUpdateFinAccountActualAndAvailableBalance"/>
> </simple-method>
> <simple-method method-name="inlineUpdateFinAccountActualAndAvailableBalance" short-description="">
> + <call-simple-method method-name="getArithmeticSettingsInline"/>
> +
> <!-- NOTE: finAccountId should always be in place -->
> <!-- Get all of the records for the account from the database and sum everything up again, make sure we don't have any
errors propagated over time -->
>
> @@ -252,7 +297,7 @@
> <then><set field="amountForCalc" from-field="finAccountTrans.amount"/></then>
> </else-if>
> </if>
> - <calculate field-name="actualBalanceSum" type="BigDecimal">
> + <calculate field-name="actualBalanceSum" type="BigDecimal" decimal-scale="${roundingDecimals}"
rounding-mode="${roundingMode}">
> <calcop operator="add" field-name="actualBalanceSum">
> <calcop operator="get" field-name="amountForCalc"/>
> </calcop>
> @@ -265,7 +310,7 @@
> <condition-expr field-name="finAccountId" env-name="finAccountId"/>
> </entity-condition>
> <iterate entry-name="finAccountAuth" list-name="finAccountAuthList">
> - <calculate field-name="availableBalanceSum" type="BigDecimal">
> + <calculate field-name="availableBalanceSum" type="BigDecimal" decimal-scale="${roundingDecimals}"
rounding-mode="${roundingMode}">
> <calcop operator="add" field-name="availableBalanceSum">
> <calcop operator="negative" field-name="finAccountAuth.amount"/>
> </calcop>
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java
> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java?rev=571291&r1=571290&r2=571291&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java Thu Aug 30 13:08:43
2007
> @@ -197,8 +197,15 @@
> BigDecimal availableBalance = finAccount.getBigDecimal("availableBalance");
> if (availableBalance == null) {
> availableBalance = FinAccountHelper.ZERO;
> + } else {
> + BigDecimal availableBalanceOriginal = availableBalance;
> + availableBalance = (availableBalance).setScale(FinAccountHelper.decimals, FinAccountHelper.rounding);
> + if (availableBalance != availableBalanceOriginal) {
> + Debug.logWarning("In finAccountPreAuth for finAccountId [" + finAccountId + "] availableBalance [" +
availableBalanceOriginal + "] was different after rounding [" + availableBalance + "]; it should never have made it into the
database this way, so check whatever put it there.", module);
> + }
> }
>
> +
> Map result = ServiceUtil.returnSuccess();
> String authMessage = null;
> Boolean processResult;
> @@ -232,7 +239,7 @@
> // refresh the account
> finAccount.refresh();
> } else {
> - Debug.logError("Attempted to authorize [" + amount + "] against a balance of only [" + availableBalance + "]",
module);
> + Debug.logWarning("Attempted to authorize [" + amount + "] against a balance of only [" + availableBalance + "]
for finAccountId [" + finAccountId + "]", module);
> refNum = "0"; // a refNum is always required from authorization
> authMessage = "Insufficient funds";
> processResult = Boolean.FALSE;
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java
> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java?rev=571291&r1=571290&r2=571291&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java Thu Aug 30 13:08:43 2007
> @@ -396,7 +396,7 @@
> result.put("processResult", "APPROVED");
> return result;
> } else {
> - Debug.logInfo("Only (" + finished + ") passed auth; returning FAILED", module);
> + Debug.logInfo("Only [" + finished + "/" + paymentPrefs.size() + "] OrderPaymentPreference authorizations passed;
returning processResult=FAILED with no message so that message from ProductStore will be used", module);
> result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_SUCCESS);
> result.put("processResult", "FAILED");
> return result;
>