Fixed bug 31419: Add parameter to control flushes in some APRM methods
authorVíctor Martínez Romanos <victor.martinez@openbravo.com>
Tue, 26 Jan 2016 17:24:27 +0100
changeset 28685 975b020c2587
parent 28684 fd863146a4f4
child 28686 6f297f8de5d6
Fixed bug 31419: Add parameter to control flushes in some APRM methods

Added a parameter doFlush to some APRM methods to control whether to flush inside the method. By default it continues to have the same behavior as before (to flush inside the method) so we should avoid any possible regression.
These flushes inside the method makes the process of several payments in a batch really slow, which is a common scenario for the remittance module. With this change (and with the necessary modifications in the remittance module), the time to process a remittance line has been decreased from 900ms to 15ms.

The idea is to create this "self-protected" methods and adapt the flows with performance problems. When the time goes by, we should clean the duplicated methods.
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/AdvPaymentMngtDao.java
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_AddPayment.java
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/AdvPaymentMngtDao.java	Fri Feb 26 08:57:54 2016 +0100
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/AdvPaymentMngtDao.java	Tue Jan 26 17:24:27 2016 +0100
@@ -11,7 +11,7 @@
  * under the License.
  * The Original Code is Openbravo ERP.
  * The Initial Developer of the Original Code is Openbravo SLU
- * All portions are Copyright (C) 2010-2015 Openbravo SLU
+ * All portions are Copyright (C) 2010-2016 Openbravo SLU
  * All Rights Reserved.
  * Contributor(s):  Enterprise Intelligence Systems (http://www.eintel.com.au).
  *************************************************************************
@@ -528,6 +528,13 @@
   public FIN_PaymentDetail getNewPaymentDetail(FIN_Payment payment,
       FIN_PaymentScheduleDetail paymentScheduleDetail, BigDecimal paymentDetailAmount,
       BigDecimal writeoffAmount, boolean isRefund, GLItem glitem) {
+    return getNewPaymentDetail(payment, paymentScheduleDetail, paymentDetailAmount, writeoffAmount,
+        isRefund, glitem, true);
+  }
+
+  public FIN_PaymentDetail getNewPaymentDetail(FIN_Payment payment,
+      FIN_PaymentScheduleDetail paymentScheduleDetail, BigDecimal paymentDetailAmount,
+      BigDecimal writeoffAmount, boolean isRefund, GLItem glitem, boolean doFlush) {
     try {
       OBContext.setAdminMode(true);
       final FIN_PaymentDetail newPaymentDetail = OBProvider.getInstance().get(
@@ -561,7 +568,9 @@
       OBDal.getInstance().save(payment);
       OBDal.getInstance().save(newPaymentDetail);
       OBDal.getInstance().save(paymentScheduleDetail);
-      OBDal.getInstance().flush();
+      if (doFlush) {
+        OBDal.getInstance().flush();
+      }
 
       if (payment.getDocumentType().getDocumentSequence() != null) {
         OBContext.getOBContext().removeWritableOrganization(
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_AddPayment.java	Fri Feb 26 08:57:54 2016 +0100
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_AddPayment.java	Tue Jan 26 17:24:27 2016 +0100
@@ -11,7 +11,7 @@
  * under the License.
  * The Original Code is Openbravo ERP.
  * The Initial Developer of the Original Code is Openbravo SLU
- * All portions are Copyright (C) 2010-2015 Openbravo SLU
+ * All portions are Copyright (C) 2010-2016 Openbravo SLU
  * All Rights Reserved.
  * Contributor(s):  ______________________________________.
  *************************************************************************
@@ -133,6 +133,8 @@
    *          this payment. Defaults to 1.0 if not supplied
    * @param finTxnAmount
    *          Amount of payment in currency of financial account
+   * @param doFlush
+   *          Force to flush inside the method after creating the payment
    * @return The FIN_Payment OBObject containing all the Payment Details.
    */
   public static FIN_Payment savePayment(FIN_Payment _payment, boolean isReceipt,
@@ -142,7 +144,7 @@
       List<FIN_PaymentScheduleDetail> selectedPaymentScheduleDetails,
       HashMap<String, BigDecimal> selectedPaymentScheduleDetailsAmounts, boolean isWriteoff,
       boolean isRefund, Currency paymentCurrency, BigDecimal finTxnConvertRate,
-      BigDecimal finTxnAmount) {
+      BigDecimal finTxnAmount, boolean doFlush) {
     dao = new AdvPaymentMngtDao();
 
     BigDecimal assignedAmount = BigDecimal.ZERO;
@@ -153,10 +155,12 @@
       payment = dao.getNewPayment(isReceipt, organization, docType, strPaymentDocumentNo,
           businessPartner, paymentMethod, finAccount, strPaymentAmount, paymentDate, referenceNo,
           paymentCurrency, finTxnConvertRate, finTxnAmount);
-      try {
-        OBDal.getInstance().flush();
-      } catch (Exception e) {
-        throw new OBException(FIN_Utility.getExceptionMessage(e));
+      if (doFlush) {
+        try {
+          OBDal.getInstance().flush();
+        } catch (Exception e) {
+          throw new OBException(FIN_Utility.getExceptionMessage(e));
+        }
       }
     }
 
@@ -174,7 +178,7 @@
         OBDal.getInstance().refresh(paymentScheduleDetail);
 
         BigDecimal assignedAmountDiff = updatePaymentDetail(paymentScheduleDetail, payment,
-            paymentDetailAmount, isWriteoff);
+            paymentDetailAmount, isWriteoff, doFlush);
         assignedAmount = assignedAmount.add(assignedAmountDiff);
       }
       // TODO: Review this condition !=0??
@@ -194,6 +198,71 @@
     return payment;
   }
 
+  /**
+   * Saves the payment and the payment details based on the given Payment Schedule Details. If no
+   * FIN_Payment is given it creates a new one.
+   * 
+   * If the Payment Scheduled Detail is not completely paid and the difference is not written a new
+   * Payment Schedule Detail is created with the difference.
+   * 
+   * If a Refund Amount is given an extra Payment Detail will be created with it.
+   * 
+   * @param _payment
+   *          FIN_Payment where new payment details will be saved.
+   * @param isReceipt
+   *          boolean to define if the Payment is a Receipt Payment (true) or a Payable Payment
+   *          (false). Used when no FIN_Payment is given.
+   * @param docType
+   *          DocumentType of the Payment. Used when no FIN_Payment is given.
+   * @param strPaymentDocumentNo
+   *          String with the Document Number of the new payment. Used when no FIN_Payment is given.
+   * @param businessPartner
+   *          BusinessPartner of the new Payment. Used when no FIN_Payment is given.
+   * @param paymentMethod
+   *          FIN_PaymentMethod of the new Payment. Used when no FIN_Payment is given.
+   * @param finAccount
+   *          FIN_FinancialAccount of the new Payment. Used when no FIN_Payment is given.
+   * @param strPaymentAmount
+   *          String with the Payment Amount of the new Payment. Used when no FIN_Payment is given.
+   * @param paymentDate
+   *          Date when the Payment is done. Used when no FIN_Payment is given.
+   * @param organization
+   *          Organization of the new Payment. Used when no FIN_Payment is given.
+   * @param selectedPaymentScheduleDetails
+   *          List of FIN_PaymentScheduleDetail to be included in the Payment. If one of the items
+   *          is contained in other payment the method will throw an exception. Prevent
+   *          invoice/order to be paid several times.
+   * @param selectedPaymentScheduleDetailsAmounts
+   *          HashMap with the Amount to be paid for each Scheduled Payment Detail.
+   * @param isWriteoff
+   *          Boolean to write off the difference when the payment amount is lower than the Payment
+   *          Scheduled PAyment Detail amount.
+   * @param isRefund
+   *          Not used.
+   * @param paymentCurrency
+   *          The currency that the payment is being made in. Will default to financial account
+   *          currency if not specified
+   * @param finTxnConvertRate
+   *          Exchange rate to convert between payment currency and financial account currency for
+   *          this payment. Defaults to 1.0 if not supplied
+   * @param finTxnAmount
+   *          Amount of payment in currency of financial account
+   * @return The FIN_Payment OBObject containing all the Payment Details.
+   */
+  public static FIN_Payment savePayment(FIN_Payment _payment, boolean isReceipt,
+      DocumentType docType, String strPaymentDocumentNo, BusinessPartner businessPartner,
+      FIN_PaymentMethod paymentMethod, FIN_FinancialAccount finAccount, String strPaymentAmount,
+      Date paymentDate, Organization organization, String referenceNo,
+      List<FIN_PaymentScheduleDetail> selectedPaymentScheduleDetails,
+      HashMap<String, BigDecimal> selectedPaymentScheduleDetailsAmounts, boolean isWriteoff,
+      boolean isRefund, Currency paymentCurrency, BigDecimal finTxnConvertRate,
+      BigDecimal finTxnAmount) {
+    return savePayment(_payment, isReceipt, docType, strPaymentDocumentNo, businessPartner,
+        paymentMethod, finAccount, strPaymentAmount, paymentDate, organization, referenceNo,
+        selectedPaymentScheduleDetails, selectedPaymentScheduleDetailsAmounts, isWriteoff,
+        isRefund, paymentCurrency, finTxnConvertRate, finTxnAmount, true);
+  }
+
   /*
    * Temporary method to supply defaults for exchange Rate and converted amount
    */
@@ -207,7 +276,65 @@
     return savePayment(_payment, isReceipt, docType, strPaymentDocumentNo, businessPartner,
         paymentMethod, finAccount, strPaymentAmount, paymentDate, organization, referenceNo,
         selectedPaymentScheduleDetails, selectedPaymentScheduleDetailsAmounts, isWriteoff,
-        isRefund, null, null, null);
+        isRefund, null, null, null, true);
+  }
+
+  /**
+   * Saves the payment and the payment details based on the given Payment Schedule Details. If no
+   * FIN_Payment is given it creates a new one.
+   * 
+   * If the Payment Scheduled Detail is not completely paid and the difference is not written a new
+   * Payment Schedule Detail is created with the difference.
+   * 
+   * If a Refund Amount is given an extra Payment Detail will be created with it.
+   * 
+   * @param _payment
+   *          FIN_Payment where new payment details will be saved.
+   * @param isReceipt
+   *          boolean to define if the Payment is a Receipt Payment (true) or a Payable Payment
+   *          (false). Used when no FIN_Payment is given.
+   * @param docType
+   *          DocumentType of the Payment. Used when no FIN_Payment is given.
+   * @param strPaymentDocumentNo
+   *          String with the Document Number of the new payment. Used when no FIN_Payment is given.
+   * @param businessPartner
+   *          BusinessPartner of the new Payment. Used when no FIN_Payment is given.
+   * @param paymentMethod
+   *          FIN_PaymentMethod of the new Payment. Used when no FIN_Payment is given.
+   * @param finAccount
+   *          FIN_FinancialAccount of the new Payment. Used when no FIN_Payment is given.
+   * @param strPaymentAmount
+   *          String with the Payment Amount of the new Payment. Used when no FIN_Payment is given.
+   * @param paymentDate
+   *          Date when the Payment is done. Used when no FIN_Payment is given.
+   * @param organization
+   *          Organization of the new Payment. Used when no FIN_Payment is given.
+   * @param selectedPaymentScheduleDetails
+   *          List of FIN_PaymentScheduleDetail to be included in the Payment. If one of the items
+   *          is contained in other payment the method will throw an exception. Prevent
+   *          invoice/order to be paid several times.
+   * @param selectedPaymentScheduleDetailsAmounts
+   *          HashMap with the Amount to be paid for each Scheduled Payment Detail.
+   * @param isWriteoff
+   *          Boolean to write off the difference when the payment amount is lower than the Payment
+   *          Scheduled PAyment Detail amount.
+   * @param isRefund
+   *          Not used.
+   * @param doFlush
+   *          Force to flush inside the method after creating the payment
+   * @return The FIN_Payment OBObject containing all the Payment Details.
+   */
+  public static FIN_Payment savePayment(FIN_Payment _payment, boolean isReceipt,
+      DocumentType docType, String strPaymentDocumentNo, BusinessPartner businessPartner,
+      FIN_PaymentMethod paymentMethod, FIN_FinancialAccount finAccount, String strPaymentAmount,
+      Date paymentDate, Organization organization, String referenceNo,
+      List<FIN_PaymentScheduleDetail> selectedPaymentScheduleDetails,
+      HashMap<String, BigDecimal> selectedPaymentScheduleDetailsAmounts, boolean isWriteoff,
+      boolean isRefund, boolean doFlush) {
+    return savePayment(_payment, isReceipt, docType, strPaymentDocumentNo, businessPartner,
+        paymentMethod, finAccount, strPaymentAmount, paymentDate, organization, referenceNo,
+        selectedPaymentScheduleDetails, selectedPaymentScheduleDetailsAmounts, isWriteoff,
+        isRefund, null, null, null, doFlush);
   }
 
   /**
@@ -231,6 +358,34 @@
    */
   public static BigDecimal updatePaymentDetail(FIN_PaymentScheduleDetail paymentScheduleDetail,
       FIN_Payment payment, BigDecimal paymentDetailAmount, boolean isWriteoff) throws OBException {
+    return updatePaymentDetail(paymentScheduleDetail, payment, paymentDetailAmount, isWriteoff,
+        true);
+  }
+
+  /**
+   * Updates the paymentScheduleDetail with the paymentDetailAmount. If it is not related to the
+   * Payment a new Payment Detail is created. If isWriteoff is true and the amount is different to
+   * the outstanding amount the difference is written off.
+   * 
+   * @param paymentScheduleDetail
+   *          the Payment Schedule Detail to be assigned to the Payment
+   * @param payment
+   *          the FIN_Payment that it is being paid
+   * @param paymentDetailAmount
+   *          the amount of this paymentScheduleDetail that it is being paid
+   * @param isWriteoff
+   *          flag to write off the difference when there is an outstanding amount remaining to pay
+   * @param doFlush
+   *          Force to flush inside the method
+   * @return a BigDecimal with the amount newly assigned to the payment. For example, when the
+   *         paymentScheduleDetail is already related to the payment and its amount is not changed
+   *         BigDecimal.ZERO is returned.
+   * @throws OBException
+   *           when the paymentDetailAmount is related to a different payment.
+   */
+  public static BigDecimal updatePaymentDetail(FIN_PaymentScheduleDetail paymentScheduleDetail,
+      FIN_Payment payment, BigDecimal paymentDetailAmount, boolean isWriteoff, boolean doFlush)
+      throws OBException {
     BigDecimal assignedAmount = paymentDetailAmount;
     if (paymentScheduleDetail.getPaymentDetails() != null) {
       if (!paymentScheduleDetail.getPaymentDetails().getFinPayment().getId()
@@ -365,7 +520,7 @@
         OBDal.getInstance().save(paymentScheduleDetail);
       }
       dao.getNewPaymentDetail(payment, paymentScheduleDetail, paymentDetailAmount,
-          amountDifference, false, null);
+          amountDifference, false, null, doFlush);
     }
     return assignedAmount;
   }
@@ -1422,6 +1577,38 @@
   }
 
   /**
+   * It calls the Payment Process using the given ProcessBundle
+   * 
+   * @param pb
+   *          ProcessBundle already created and initialized. This improves the performance when
+   *          calling this method in a loop
+   * @param payment
+   *          FIN_Payment that needs to be processed.
+   * @param comingFrom
+   *          Origin where the process is invoked
+   * @param selectedCreditLineIds
+   *          Id's of selected lines in Credit to Use grid
+   * @param doFlush
+   *          Force to flush inside the method
+   * @return a OBError with the result message of the process.
+   * @throws Exception
+   */
+  public static OBError processPayment(ProcessBundle pb, String strAction, FIN_Payment payment,
+      String comingFrom, String selectedCreditLineIds, boolean doFlush) throws Exception {
+    HashMap<String, Object> parameters = new HashMap<String, Object>();
+    parameters.put("action", strAction);
+    parameters.put("Fin_Payment_ID", payment.getId());
+    parameters.put("comingFrom", comingFrom);
+    parameters.put("selectedCreditLineIds", selectedCreditLineIds);
+    parameters.put("doFlush", doFlush);
+    pb.setParams(parameters);
+    OBError myMessage = null;
+    new FIN_PaymentProcess().execute(pb);
+    myMessage = (OBError) pb.getResult();
+    return myMessage;
+  }
+
+  /**
    * It calls the Payment Proposal Process for the given payment proposal and action.
    * 
    * @param vars
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java	Fri Feb 26 08:57:54 2016 +0100
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java	Tue Jan 26 17:24:27 2016 +0100
@@ -11,7 +11,7 @@
  * under the License.
  * The Original Code is Openbravo ERP.
  * The Initial Developer of the Original Code is Openbravo SLU
- * All portions are Copyright (C) 2010-2015 Openbravo SLU
+ * All portions are Copyright (C) 2010-2016 Openbravo SLU
  * All Rights Reserved.
  * Contributor(s):  ______________________________________.
  *************************************************************************
@@ -94,7 +94,10 @@
         isPosOrder = bundle.getParams().get("isPOSOrder").equals("Y");
       }
       final String paymentDate = (String) bundle.getParams().get("paymentdate");
-      processPayment(payment, strAction, isPosOrder, paymentDate, comingFrom, selectedCreditLineIds);
+      final boolean doFlush = bundle.getParams().get("doFlush") != null ? (Boolean) bundle
+          .getParams().get("doFlush") : true;
+      processPayment(payment, strAction, isPosOrder, paymentDate, comingFrom,
+          selectedCreditLineIds, doFlush);
       bundle.setResult(msg);
     } catch (Exception e) {
       log4j.error(e.getMessage());
@@ -110,11 +113,12 @@
   public static void doProcessPayment(FIN_Payment payment, String strAction, Boolean isPosOrder,
       String paymentDate, String comingFrom) throws OBException {
     FIN_PaymentProcess fpp = WeldUtils.getInstanceFromStaticBeanManager(FIN_PaymentProcess.class);
-    fpp.processPayment(payment, strAction, isPosOrder, paymentDate, comingFrom, null);
+    fpp.processPayment(payment, strAction, isPosOrder, paymentDate, comingFrom, null, true);
   }
 
   private void processPayment(FIN_Payment payment, String strAction, Boolean isPosOrder,
-      String paymentDate, String comingFrom, String selectedCreditLineIds) throws OBException {
+      String paymentDate, String comingFrom, String selectedCreditLineIds, boolean doFlush)
+      throws OBException {
     dao = new AdvPaymentMngtDao();
     String msg = "";
     try {
@@ -485,7 +489,7 @@
             flushDone = true;
           }
         } finally {
-          if (!flushDone) {
+          if (!flushDone && doFlush) {
             OBDal.getInstance().flush();
           }
           OBContext.restorePreviousMode();
@@ -656,7 +660,7 @@
         FIN_PaymentProcess fpp = WeldUtils
             .getInstanceFromStaticBeanManager(FIN_PaymentProcess.class);
         fpp.processPayment(reversedPayment, newStrAction, isPosOrder, paymentDate, comingFrom,
-            selectedCreditLineIds);
+            selectedCreditLineIds, true);
 
         return;