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/

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

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/

Jacques Le Roux
Administrator
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;
>