details: https://code.openbravo.com/erp/devel/pi/rev/d1e5ce861ccc changeset: 29273:d1e5ce861ccc user: Atul Gaware <atul.gaware <at> openbravo.com> date: Fri Apr 22 12:04:27 2016 +0200 summary: Fixes issue 32510: Improve performance of Payment Report
- Improve hql query. - Avoid joins in query if they are not necessary. - Retrieve only id instead of dal object in query. - Use !isEmpty() instead of size()>0. - Use setMaxResults(1) and uniqueResult() instead of list().get(0). - Improve simple queries to return needed column instead of full dal object and avoid looping through results. details: https://code.openbravo.com/erp/devel/pi/rev/ec5b60a23972 changeset: 29274:ec5b60a23972 user: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com> date: Fri Apr 22 15:20:33 2016 +0200 summary: Related to issue 32510: Code review improvements diffstat: modules/org.openbravo.financial.paymentreport/src/org/openbravo/financial/paymentreport/erpCommon/ad_reports/PaymentReportDao.java | 300 ++++----- 1 files changed, 129 insertions(+), 171 deletions(-) diffs (truncated from 512 to 300 lines): diff -r 31f0af79e203 -r ec5b60a23972 modules/org.openbravo.financial.paymentreport/src/org/openbravo/financial/paymentreport/erpCommon/ad_reports/PaymentReportDao.java --- a/modules/org.openbravo.financial.paymentreport/src/org/openbravo/financial/paymentreport/erpCommon/ad_reports/PaymentReportDao.java Wed Apr 20 15:20:32 2016 +0200 +++ b/modules/org.openbravo.financial.paymentreport/src/org/openbravo/financial/paymentreport/erpCommon/ad_reports/PaymentReportDao.java Fri Apr 22 15:20:33 2016 +0200 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SL - * All portions are Copyright (C) 2009-2014 Openbravo SL + * All portions are Copyright (C) 2009-2016 Openbravo SL * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -259,32 +259,37 @@ ArrayList<FieldProvider> totalData = new ArrayList<FieldProvider>(); int numberOfElements = 0; int lastElement = 0; - boolean existsConvRate = false; ScrollableResults scroller = null; OBContext.setAdminMode(true); try { - hsqlScript.append(" from FIN_Payment_ScheduleDetail as fpsd "); - hsqlScript.append(" left outer join fpsd.paymentDetails.finPayment pay"); - hsqlScript.append(" left outer join pay.businessPartner paybp"); - hsqlScript.append(" left outer join paybp.businessPartnerCategory paybpc"); - hsqlScript.append(" left outer join fpsd.invoicePaymentSchedule invps"); - hsqlScript.append(" left outer join invps.invoice inv"); - hsqlScript.append(" left outer join inv.businessPartner invbp"); - hsqlScript.append(" left outer join invbp.businessPartnerCategory invbpc"); - hsqlScript.append(" left outer join fpsd.paymentDetails.finPayment.currency paycur"); - hsqlScript.append(" left outer join fpsd.invoicePaymentSchedule.invoice.currency invcur"); - hsqlScript.append(" left outer join pay.project paypro"); - hsqlScript.append(" left outer join inv.project invpro"); - hsqlScript.append(" where (fpsd."); - hsqlScript.append(FIN_PaymentScheduleDetail.PROPERTY_PAYMENTDETAILS); - hsqlScript.append(" is not null or invps is not null "); - hsqlScript.append(") "); - - hsqlScript.append(" and fpsd."); - hsqlScript.append(FIN_PaymentScheduleDetail.PROPERTY_ORGANIZATION); - hsqlScript.append(".id in "); + hsqlScript.append(" left join fpsd.paymentDetails as fpd"); + hsqlScript.append(" left join fpd.finPayment as pay"); + hsqlScript.append(" left join fpsd.invoicePaymentSchedule as invps"); + hsqlScript.append(" left join invps.invoice as inv"); + if (strGroupCrit.equalsIgnoreCase("INS_CURRENCY") || strOrdCrit.contains("INS_CURRENCY")) { + hsqlScript.append(" left join pay.currency as paycur"); + hsqlScript.append(" left join inv.currency as invcur"); + } + if (strGroupCrit.equalsIgnoreCase("Project") || strOrdCrit.contains("Project")) { + hsqlScript.append(" left join pay.project as paypro"); + hsqlScript.append(" left join inv.project as invpro"); + } + if (strGroupCrit.equalsIgnoreCase("APRM_FATS_BPARTNER") + || strOrdCrit.contains("APRM_FATS_BPARTNER") + || strGroupCrit.equalsIgnoreCase("FINPR_BPartner_Category") + || strOrdCrit.contains("FINPR_BPartner_Category")) { + hsqlScript.append(" left join pay.businessPartner as paybp"); + hsqlScript.append(" left join inv.businessPartner as invbp"); + if (strGroupCrit.equalsIgnoreCase("FINPR_BPartner_Category") + || strOrdCrit.contains("FINPR_BPartner_Category")) { + hsqlScript.append(" left join paybp.businessPartnerCategory as paybpc"); + hsqlScript.append(" left join invbp.businessPartnerCategory as invbpc"); + } + } + hsqlScript.append(" where (fpd is not null or invps is not null)"); + hsqlScript.append(" and fpsd.organization.id in "); hsqlScript.append(concatOrganizations(OBContext.getOBContext().getReadableOrganizations())); // organization + include sub-organization @@ -589,7 +594,7 @@ final StringBuilder firstLineQuery = new StringBuilder(); firstLineQuery - .append("select fpsd, (select a.sequenceNumber from ADList a where a.reference.id = '575BCB88A4694C27BC013DE9C73E6FE7' and a.searchKey = coalesce(pay.status, 'RPAP')) as a"); + .append("select fpsd.id, (select a.sequenceNumber from ADList a where a.reference.id = '575BCB88A4694C27BC013DE9C73E6FE7' and a.searchKey = coalesce(pay.status, 'RPAP')) as a"); hsqlScript = firstLineQuery.append(hsqlScript); hsqlScript.append(" order by "); @@ -730,11 +735,9 @@ int i = 0; while (scroller.next()) { i++; - // get 1st column (idx=0) - Object value = scroller.get(0); - // TODO: rename variable to not have same name as class - FIN_PaymentScheduleDetail FIN_PaymentScheduleDetail = (FIN_PaymentScheduleDetail) value; + FIN_PaymentScheduleDetail fpsd = OBDal.getInstance().get(FIN_PaymentScheduleDetail.class, + scroller.get(0)); // make a empty FieldProvider instead of saving link to DAL-object FieldProvider data = FieldProviderFactory.getFieldProvider(null); @@ -746,24 +749,27 @@ .getInstance() .getSession() .buildLockRequest(LockOptions.NONE) - .lock(org.openbravo.model.financialmgmt.payment.FIN_PaymentScheduleDetail.ENTITY_NAME, - FIN_PaymentScheduleDetail); + .lock(FIN_PaymentScheduleDetail.ENTITY_NAME, + fpsd); // search for fin_finacc_transaction for this payment FIN_FinaccTransaction trx = null; - FIN_PaymentDetail detail = FIN_PaymentScheduleDetail.getPaymentDetails(); - if (detail != null) { + FIN_Payment payment = null; + Invoice invoice = null; + if (fpsd.getPaymentDetails() != null) { + payment = fpsd.getPaymentDetails().getFinPayment(); OBCriteria<FIN_FinaccTransaction> trxQuery = OBDal.getInstance().createCriteria( FIN_FinaccTransaction.class); - trxQuery.add(Restrictions.eq(FIN_FinaccTransaction.PROPERTY_FINPAYMENT, - detail.getFinPayment())); + trxQuery.add(Restrictions.eq(FIN_FinaccTransaction.PROPERTY_FINPAYMENT, payment)); // uniqueness guaranteed via unique constraint in db trx = (FIN_FinaccTransaction) trxQuery.uniqueResult(); + } else { + invoice = fpsd.getInvoicePaymentSchedule().getInvoice(); } // If the payment schedule detail has a payment detail, then, the information is taken from // the payment. If not, the information is taken from the invoice (the else). - if (FIN_PaymentScheduleDetail.getPaymentDetails() != null) { - BusinessPartner bp = getDocumentBusinessPartner(FIN_PaymentScheduleDetail); + if (fpsd.getPaymentDetails() != null) { + BusinessPartner bp = getDocumentBusinessPartner(fpsd); if (bp == null) { FieldProviderFactory.setField(data, "BP_GROUP", ""); FieldProviderFactory.setField(data, "BPARTNER", ""); @@ -776,57 +782,38 @@ } // transCurrency - transCurrency = FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() - .getCurrency(); + transCurrency = payment.getCurrency(); FieldProviderFactory.setField(data, "TRANS_CURRENCY", transCurrency.getISOCode()); // paymentMethod - FieldProviderFactory.setField(data, "PAYMENT_METHOD", FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getPaymentMethod().getIdentifier()); + FieldProviderFactory.setField(data, "PAYMENT_METHOD", payment.getPaymentMethod() + .getIdentifier()); // payment - FieldProviderFactory - .setField( - data, - "PAYMENT", - ((FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment().getPaymentDate() != null) ? dateFormat - .format(FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() - .getPaymentDate()) : "Null") - + " - " - + FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() - .getDocumentNo()); + FieldProviderFactory.setField(data, "PAYMENT", + ((payment.getPaymentDate() != null) ? dateFormat.format(payment.getPaymentDate()) + : "Null") + " - " + payment.getDocumentNo()); // payment description - FieldProviderFactory.setField(data, "PAYMENT_DESC", FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getDescription()); + FieldProviderFactory.setField(data, "PAYMENT_DESC", payment.getDescription()); // payment_id - FieldProviderFactory.setField(data, "PAYMENT_ID", FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getId().toString()); + FieldProviderFactory.setField(data, "PAYMENT_ID", payment.getId().toString()); // payment_date - FieldProviderFactory - .setField( - data, - "PAYMENT_DATE", - (FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment().getPaymentDate() != null) ? dateFormat - .format(FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() - .getPaymentDate()) : "Null"); + FieldProviderFactory.setField(data, "PAYMENT_DATE", + (payment.getPaymentDate() != null) ? dateFormat.format(payment.getPaymentDate()) + : "Null"); // payment_docNo - FieldProviderFactory.setField(data, "PAYMENT_DOCNO", FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getDocumentNo()); + FieldProviderFactory.setField(data, "PAYMENT_DOCNO", payment.getDocumentNo()); // payment yes / no FieldProviderFactory.setField(data, "PAYMENT_Y_N", ""); // financialAccount FieldProviderFactory.setField(data, "FINANCIAL_ACCOUNT", - FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() - .getFINFinaccTransactionList().size() != 0 ? FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getFINFinaccTransactionList().get(0) - .getAccount().getName() : FIN_PaymentScheduleDetail.getPaymentDetails() - .getFinPayment().getAccount().getName()); + !payment.getFINFinaccTransactionList().isEmpty() ? payment + .getFINFinaccTransactionList().get(0).getAccount().getName() : payment + .getAccount().getName()); // status - FieldProviderFactory.setField(data, "STATUS", translateRefList(FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getStatus())); - FieldProviderFactory.setField(data, "STATUS_CODE", FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment().getStatus()); + FieldProviderFactory.setField(data, "STATUS", translateRefList(payment.getStatus())); + FieldProviderFactory.setField(data, "STATUS_CODE", payment.getStatus()); // is receipt - if (FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment().isReceipt()) { + if (payment.isReceipt()) { FieldProviderFactory.setField(data, "ISRECEIPT", "Y"); isReceipt = true; } else { @@ -843,19 +830,16 @@ } else { // bp_group -- bp_category - FieldProviderFactory.setField(data, "BP_GROUP", FIN_PaymentScheduleDetail - .getInvoicePaymentSchedule().getInvoice().getBusinessPartner() + FieldProviderFactory.setField(data, "BP_GROUP", invoice.getBusinessPartner() .getBusinessPartnerCategory().getName()); // bpartner - FieldProviderFactory.setField(data, "BPARTNER", FIN_PaymentScheduleDetail - .getInvoicePaymentSchedule().getInvoice().getBusinessPartner().getName()); + FieldProviderFactory.setField(data, "BPARTNER", invoice.getBusinessPartner().getName()); // transCurrency - transCurrency = FIN_PaymentScheduleDetail.getInvoicePaymentSchedule().getInvoice() - .getCurrency(); + transCurrency = invoice.getCurrency(); FieldProviderFactory.setField(data, "TRANS_CURRENCY", transCurrency.getISOCode()); // paymentMethod - FieldProviderFactory.setField(data, "PAYMENT_METHOD", FIN_PaymentScheduleDetail - .getInvoicePaymentSchedule().getFinPaymentmethod().getIdentifier()); + FieldProviderFactory.setField(data, "PAYMENT_METHOD", fpsd.getInvoicePaymentSchedule() + .getFinPaymentmethod().getIdentifier()); // payment FieldProviderFactory.setField(data, "PAYMENT", ""); // payment_id @@ -872,8 +856,7 @@ FieldProviderFactory.setField(data, "STATUS", translateRefList("RPAP")); FieldProviderFactory.setField(data, "STATUS_CODE", "RPAP"); // is receipt - if (FIN_PaymentScheduleDetail.getInvoicePaymentSchedule().getInvoice() - .isSalesTransaction()) { + if (invoice.isSalesTransaction()) { FieldProviderFactory.setField(data, "ISRECEIPT", "Y"); isReceipt = true; } else { @@ -894,16 +877,13 @@ * * - Otherwise, it is filled empty. */ - if (FIN_PaymentScheduleDetail.getInvoicePaymentSchedule() != null) { - fillLine(dateFormat, data, FIN_PaymentScheduleDetail, - FIN_PaymentScheduleDetail.getInvoicePaymentSchedule(), false); - } else if (FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() != null) { - java.util.List<Invoice> invoices = getInvoicesUsingCredit(FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment()); + if (fpsd.getInvoicePaymentSchedule() != null) { + fillLine(dateFormat, data, fpsd, fpsd.getInvoicePaymentSchedule(), false); + } else if (payment != null) { + java.util.List<Invoice> invoices = getInvoicesUsingCredit(payment); if (invoices.size() == 1) { - java.util.List<FIN_PaymentSchedule> ps = getInvoicePaymentSchedules(FIN_PaymentScheduleDetail - .getPaymentDetails().getFinPayment()); - fillLine(dateFormat, data, FIN_PaymentScheduleDetail, ps.get(0), true); + java.util.List<FIN_PaymentSchedule> ps = getInvoicePaymentSchedules(payment); + fillLine(dateFormat, data, fpsd, ps.get(0), true); } else { // project FieldProviderFactory.setField(data, "PROJECT", ""); @@ -962,7 +942,7 @@ } // transactional and base amounts - transAmount = FIN_PaymentScheduleDetail.getAmount(); + transAmount = fpsd.getAmount(); Currency baseCurrency = OBDal.getInstance().get(Currency.class, strConvertCurrency); @@ -1013,20 +993,14 @@ // Balance String status = "RPAE"; try { - status = FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment().getStatus(); + status = payment.getStatus(); } catch (NullPointerException e) { } - final boolean isCreditPayment = FIN_PaymentScheduleDetail.getInvoicePaymentSchedule() == null - && FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment() != null; + final boolean isCreditPayment = fpsd.getInvoicePaymentSchedule() == null && payment != null; BigDecimal balance = BigDecimal.ZERO; if (isCreditPayment && status != null && "PWNC RPR RPPC PPM RDNC".indexOf(status) >= 0) { - balance = FIN_PaymentScheduleDetail - .getPaymentDetails() - .getFinPayment() - .getGeneratedCredit() - .subtract( - FIN_PaymentScheduleDetail.getPaymentDetails().getFinPayment().getUsedCredit()); + balance = payment.getGeneratedCredit().subtract(payment.getUsedCredit()); if (isReceipt) { balance = balance.negate(); } @@ -1041,28 +1015,24 @@ } FieldProviderFactory.setField(data, "BALANCE", balance.toString()); ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
