Fixed bug 29904 Performance improvements for orderloading in finance core
authorSandra Huguet <sandra.huguet@openbravo.com>
Wed, 27 May 2015 09:41:59 +0200
changeset 26817 83734f0b006e
parent 26816 56a6bfc4e4c7
child 26818 9eb109c1f121
Fixed bug 29904 Performance improvements for orderloading in finance core

review flush () and .list () in FIN_PaymentProcess and FIN_TransactionProcess
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/TransactionsDao.java
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java
modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_TransactionProcess.java
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/TransactionsDao.java	Wed May 27 16:05:43 2015 +0200
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/dao/TransactionsDao.java	Wed May 27 09:41:59 2015 +0200
@@ -131,6 +131,7 @@
         newTransaction.setForeignConversionRate(payment.getFinancialTransactionConvertRate());
         newTransaction.setForeignAmount(payment.getAmount());
       }
+      payment.getFINFinaccTransactionList().add(newTransaction);
       OBDal.getInstance().save(newTransaction);
       OBDal.getInstance().flush();
     } finally {
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java	Wed May 27 16:05:43 2015 +0200
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProcess.java	Wed May 27 09:41:59 2015 +0200
@@ -21,6 +21,8 @@
 import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -155,14 +157,14 @@
         }
       }
 
-      OBDal.getInstance().flush();
       if (strAction.equals("P") || strAction.equals("D")) {
         // Guess if this is a refund payment
         boolean isRefund = false;
         OBContext.setAdminMode(false);
         try {
-          if (payment.getFINPaymentDetailList().size() > 0) {
-            for (FIN_PaymentDetail det : payment.getFINPaymentDetailList()) {
+          List<FIN_PaymentDetail> paymentDetailList = payment.getFINPaymentDetailList();
+          if (paymentDetailList.size() > 0) {
+            for (FIN_PaymentDetail det : paymentDetailList) {
               if (det.isRefund()) {
                 isRefund = true;
                 break;
@@ -178,13 +180,14 @@
           OBDal.getInstance().save(payment);
         }
 
-        boolean orgLegalWithAccounting = FIN_Utility.periodControlOpened(FIN_Payment.TABLE_NAME,
-            payment.getId(), FIN_Payment.TABLE_NAME + "_ID", "LE");
         boolean documentEnabled = getDocumentConfirmation(null, payment.getId());
-        if (documentEnabled
+        boolean periodNotAvailable = documentEnabled
             && !FIN_Utility.isPeriodOpen(payment.getClient().getId(), payment.getDocumentType()
                 .getDocumentCategory(), payment.getOrganization().getId(), OBDateUtils
-                .formatDate(payment.getPaymentDate())) && orgLegalWithAccounting) {
+                .formatDate(payment.getPaymentDate()))
+            && FIN_Utility.periodControlOpened(FIN_Payment.TABLE_NAME, payment.getId(),
+                FIN_Payment.TABLE_NAME + "_ID", "LE");
+        if (periodNotAvailable) {
           msg = OBMessageUtils.messageBD("PeriodNotAvailable");
           throw new OBException(msg);
         }
@@ -204,6 +207,7 @@
         // FIXME: added to access the FIN_PaymentSchedule and FIN_PaymentScheduleDetail tables to be
         // removed when new security implementation is done
         OBContext.setAdminMode();
+        boolean flushDone = false;
         try {
           String strRefundCredit = "";
           // update payment schedule amount
@@ -313,8 +317,9 @@
             }
           }
           // Execution Process
-          if (dao.isAutomatedExecutionPayment(payment.getAccount(), payment.getPaymentMethod(),
-              payment.isReceipt())) {
+          if (!isPosOrder
+              && dao.isAutomatedExecutionPayment(payment.getAccount(), payment.getPaymentMethod(),
+                  payment.isReceipt())) {
             try {
               payment.setStatus("RPAE");
 
@@ -359,16 +364,26 @@
                 decreaseCustomerCredit(businessPartner, payment.getUsedCredit());
               }
             }
+
             for (FIN_PaymentDetail paymentDetail : payment.getFINPaymentDetailList()) {
+
+              List<FIN_PaymentScheduleDetail> orderPaymentScheduleDetails = new ArrayList<FIN_PaymentScheduleDetail>(
+                  paymentDetail.getFINPaymentScheduleDetailList());
+
               // Get payment schedule detail list ordered by amount asc.
               // First negative if they exist and then positives
-              OBCriteria<FIN_PaymentScheduleDetail> obcPSD = OBDal.getInstance().createCriteria(
-                  FIN_PaymentScheduleDetail.class);
-              obcPSD.add(Restrictions.eq(FIN_PaymentScheduleDetail.PROPERTY_PAYMENTDETAILS,
-                  paymentDetail));
-              obcPSD.addOrderBy(FIN_PaymentScheduleDetail.PROPERTY_AMOUNT, true);
+              if (orderPaymentScheduleDetails.size() > 1) {
+                Collections.sort(orderPaymentScheduleDetails,
+                    new Comparator<FIN_PaymentScheduleDetail>() {
+                      @Override
+                      public int compare(FIN_PaymentScheduleDetail o1, FIN_PaymentScheduleDetail o2) {
+                        // TODO Auto-generated method stub
+                        return o1.getAmount().compareTo(o2.getAmount());
+                      }
+                    });
+              }
 
-              for (FIN_PaymentScheduleDetail paymentScheduleDetail : obcPSD.list()) {
+              for (FIN_PaymentScheduleDetail paymentScheduleDetail : orderPaymentScheduleDetails) {
                 BigDecimal amount = paymentScheduleDetail.getAmount().add(
                     paymentScheduleDetail.getWriteoffAmount());
                 // Do not restore paid amounts if the payment is awaiting execution.
@@ -378,7 +393,7 @@
                 paymentScheduleDetail.setInvoicePaid(false);
                 // Payment = 0 when the payment is generated by a invoice that consume credit
                 if (invoicePaidAmounts
-                    | (payment.getAmount().compareTo(new BigDecimal("0.00")) == 0)) {
+                    || (payment.getAmount().compareTo(new BigDecimal("0.00")) == 0)) {
                   if (paymentScheduleDetail.getInvoicePaymentSchedule() != null) {
                     // BP SO_CreditUsed
                     businessPartner = paymentScheduleDetail.getInvoicePaymentSchedule()
@@ -458,14 +473,18 @@
                 && payment.getAmount().compareTo(BigDecimal.ZERO) != 0
                 && !"TRANSACTION".equals(comingFrom)) {
               triggerAutomaticFinancialAccountTransaction(payment);
+              flushDone = true;
             }
           }
           if (!payment.getAccount().getCurrency().equals(payment.getCurrency())
               && getConversionRateDocument(payment).size() == 0) {
             insertConversionRateDocument(payment);
+            flushDone = true;
           }
         } finally {
-          OBDal.getInstance().flush();
+          if (!flushDone) {
+            OBDal.getInstance().flush();
+          }
           OBContext.restorePreviousMode();
         }
 
@@ -1320,14 +1339,8 @@
     OBContext.setAdminMode();
     try {
       FIN_Payment payment = OBDal.getInstance().get(FIN_Payment.class, strRecordId);
-      OBCriteria<FinAccPaymentMethod> obCriteria = OBDal.getInstance().createCriteria(
-          FinAccPaymentMethod.class);
-      obCriteria.add(Restrictions.eq(FinAccPaymentMethod.PROPERTY_ACCOUNT, payment.getAccount()));
-      obCriteria.add(Restrictions.eq(FinAccPaymentMethod.PROPERTY_PAYMENTMETHOD,
-          payment.getPaymentMethod()));
-      obCriteria.setFilterOnReadableClients(false);
-      obCriteria.setFilterOnReadableOrganization(false);
-      List<FinAccPaymentMethod> lines = obCriteria.list();
+      List<FinAccPaymentMethod> lines = payment.getPaymentMethod()
+          .getFinancialMgmtFinAccPaymentMethodList();
       List<FIN_FinancialAccountAccounting> accounts = payment.getAccount()
           .getFINFinancialAccountAcctList();
       for (FIN_FinancialAccountAccounting account : accounts) {
@@ -1360,6 +1373,7 @@
         }
       }
     } catch (Exception e) {
+      // TODO no logging... ??
       return confirmation;
     } finally {
       OBContext.restorePreviousMode();
--- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_TransactionProcess.java	Wed May 27 16:05:43 2015 +0200
+++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_TransactionProcess.java	Wed May 27 09:41:59 2015 +0200
@@ -99,6 +99,7 @@
         // ***********************
         // Process Transaction
         // ***********************
+
         boolean orgLegalWithAccounting = FIN_Utility.periodControlOpened(
             FIN_FinaccTransaction.TABLE_NAME, transaction.getId(), FIN_FinaccTransaction.TABLE_NAME
                 + "_ID", "LE");
@@ -110,6 +111,7 @@
           msg = OBMessageUtils.messageBD("PeriodNotAvailable");
           throw new OBException(msg);
         }
+
         final FIN_FinancialAccount financialAccount = transaction.getAccount();
         financialAccount.setCurrentBalance(financialAccount.getCurrentBalance().add(
             transaction.getDepositAmount().subtract(transaction.getPaymentAmount())));
@@ -165,7 +167,6 @@
         transaction.setAprmProcessed("R");
         OBDal.getInstance().save(financialAccount);
         OBDal.getInstance().save(transaction);
-        OBDal.getInstance().flush();
 
       } else if (strAction.equals("R")) {
         // ***********************
@@ -196,10 +197,14 @@
               ConversionRateDoc.class);
           obc.add(Restrictions.eq(ConversionRateDoc.PROPERTY_FINANCIALACCOUNTTRANSACTION,
               transaction));
+          boolean dataRemoved = false;
           for (ConversionRateDoc conversionRateDoc : obc.list()) {
+            dataRemoved = true;
             OBDal.getInstance().remove(conversionRateDoc);
           }
-          OBDal.getInstance().flush();
+          if (dataRemoved) {
+            OBDal.getInstance().flush();
+          }
         } finally {
           OBContext.restorePreviousMode();
         }
@@ -209,7 +214,6 @@
             .subtract(transaction.getDepositAmount()).add(transaction.getPaymentAmount()));
         OBDal.getInstance().save(financialAccount);
         OBDal.getInstance().save(transaction);
-        OBDal.getInstance().flush();
         if (payment != null) {
           Boolean invoicePaidold = false;
           for (FIN_PaymentDetail pd : payment.getFINPaymentDetailList()) {
@@ -233,9 +237,9 @@
         }
         transaction.setAprmProcessed("P");
         OBDal.getInstance().save(transaction);
-        OBDal.getInstance().flush();
       }
     } finally {
+      OBDal.getInstance().flush();
       OBContext.restorePreviousMode();
     }
   }