details: https://code.openbravo.com/erp/devel/pi/rev/217fb3a426fe changeset: 28180:217fb3a426fe user: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com> date: Tue Dec 01 16:44:30 2015 +0100 summary: Fixes issue 31580: Java Heap Space error when posting payment with many details
Add a new FIN_Utility.getOrderedPaymentDetailList method and deprecate the old one, to return a List<String> instead of a List<FIN_PaymentDetail> and avoid loading all the entities in memory. Adapt loadLines methods in DocFINFinAccTransaction, DocFINPayment and DocFINReconciliation to the new FIN_Utility.getOrderedPaymentDetailList method. Create a new getPaymentDetailWriteOffAndAmount method in AcctServer to adapt it to new parameters. details: https://code.openbravo.com/erp/devel/pi/rev/1817418c55af changeset: 28181:1817418c55af user: Víctor Martínez Romanos <victor.martinez <at> openbravo.com> date: Tue Dec 29 11:32:34 2015 +0100 summary: Fixed bug 31580: code review improvements Properly passing parameter to the HQL query Force to check client+org when using admin mode diffstat: modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java | 39 +- src/org/openbravo/erpCommon/ad_forms/AcctServer.java | 82 ++- src/org/openbravo/erpCommon/ad_forms/DocFINFinAccTransaction.java | 245 ++++----- src/org/openbravo/erpCommon/ad_forms/DocFINPayment.java | 244 ++++----- src/org/openbravo/erpCommon/ad_forms/DocFINReconciliation.java | 182 +++--- 5 files changed, 429 insertions(+), 363 deletions(-) diffs (truncated from 1029 to 300 lines): diff -r 0759c894c939 -r 1817418c55af modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java --- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java Mon Jan 11 13:20:26 2016 +0100 +++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java Tue Dec 29 11:32:34 2015 +0100 @@ -1310,8 +1310,11 @@ } /** - * Returns Payment Details from a Payment ordered by Invoice and Order + * Returns Payment Details from a Payment ordered by Invoice and Order. This method is deprecated + * as it does not perform well. Use {@link #getOrderedPaymentDetailList(String paymentId)} instead + * */ + @Deprecated public static List<FIN_PaymentDetail> getOrderedPaymentDetailList(FIN_Payment payment) { List<FIN_PaymentDetail> pdList = null; @@ -1343,6 +1346,40 @@ } /** + * Returns Payment Details from a Payment ordered by Invoice and Order + */ + @SuppressWarnings("unchecked") + public static List<String> getOrderedPaymentDetailList(String paymentId) { + + List<String> pdList = null; + + OBContext.setAdminMode(true); + try { + final StringBuilder whereClause = new StringBuilder(); + whereClause.append(" select pd." + FIN_PaymentDetail.PROPERTY_ID); + whereClause.append(" from " + FIN_PaymentDetail.ENTITY_NAME + " as pd"); + whereClause.append(" left join pd." + FIN_PaymentDetail.PROPERTY_FINPAYMENTSCHEDULEDETAILLIST + + " as psd"); + whereClause + .append(" where pd." + FIN_PaymentDetail.PROPERTY_FINPAYMENT + ".id = :paymentId "); + whereClause.append(" and pd." + FIN_PaymentDetail.PROPERTY_ACTIVE + " = true"); + whereClause.append(" order by psd." + + FIN_PaymentScheduleDetail.PROPERTY_INVOICEPAYMENTSCHEDULE); + whereClause.append(", coalesce(psd." + + FIN_PaymentScheduleDetail.PROPERTY_ORDERPAYMENTSCHEDULE + ",'0')"); + + Query query = OBDal.getInstance().getSession().createQuery(whereClause.toString()); + query.setParameter("paymentId", paymentId); + pdList = query.list(); + + } finally { + OBContext.restorePreviousMode(); + } + + return pdList; + } + + /** * Returns true if the payment is a reverse payment not a reversed one. */ public static boolean isReversePayment(FIN_Payment payment) { diff -r 0759c894c939 -r 1817418c55af src/org/openbravo/erpCommon/ad_forms/AcctServer.java --- a/src/org/openbravo/erpCommon/ad_forms/AcctServer.java Mon Jan 11 13:20:26 2016 +0100 +++ b/src/org/openbravo/erpCommon/ad_forms/AcctServer.java Tue Dec 29 11:32:34 2015 +0100 @@ -2954,24 +2954,74 @@ public HashMap<String, BigDecimal> getPaymentDetailWriteOffAndAmount( List<FIN_PaymentDetail> paymentDetails, FIN_PaymentSchedule ps, FIN_PaymentSchedule psi, FIN_PaymentSchedule pso, int currentPaymentDetailIndex, final FieldProvider fieldProvider) { + FIN_PaymentDetail paymentDetail = OBDal.getInstance().get(FIN_PaymentDetail.class, + paymentDetails.get(currentPaymentDetailIndex)); + String paymentDetailNextId = null; + String paymentDetailPreviousId = null; + if (currentPaymentDetailIndex < paymentDetails.size() - 1) { + paymentDetailNextId = paymentDetails.get(currentPaymentDetailIndex + 1).getId(); + } + if (currentPaymentDetailIndex > 0) { + paymentDetailPreviousId = paymentDetails.get(currentPaymentDetailIndex - 1).getId(); + } + return getPaymentDetailWriteOffAndAmount(paymentDetail, paymentDetailNextId, + paymentDetailPreviousId, ps, psi, pso, fieldProvider); + } + + /** + * Returns the writeoff and the amount of a Payment Detail. In case the related Payment Schedule + * Detail was generated for compensate the difference between an Order and a related Invoice, it + * merges it's amount with the next Payment Schedule Detail. Issue 19567: + * https://issues.openbravo.com/view.php?id=19567 <br /> + * It does exactly the same as the + * {@link #getPaymentDetailWriteOffAndAmount(List, FIN_PaymentSchedule, FIN_PaymentSchedule, FIN_PaymentSchedule, int)} + * method, but it also stores a new field "MergedPaymentDetailId" inside the fieldProvider with + * the merged payment detail id (if any). + * + * @param paymentDetail + * Payment Detail + * @param paymentDetailNextId + * Next Payment Detail id + * @param paymentDetailPreviousId + * Previous Payment Detail id + * @param ps + * Previous Payment Schedule + * @param psi + * Invoice Payment Schedule of actual Payment Detail + * @param pso + * Order Payment Schedule of actual Payment Detail + * @param fieldProvider + * contains the FieldProvider with the Payment Detail currently being processed. Used to + * store the "MergedPaymentDetailId" (if any) as a new field of the fieldProvider + */ + public HashMap<String, BigDecimal> getPaymentDetailWriteOffAndAmount( + FIN_PaymentDetail paymentDetail, String paymentDetailNextId, String paymentDetailPreviousId, + FIN_PaymentSchedule ps, FIN_PaymentSchedule psi, FIN_PaymentSchedule pso, + final FieldProvider fieldProvider) { HashMap<String, BigDecimal> amountAndWriteOff = new HashMap<String, BigDecimal>(); // Default return values - amountAndWriteOff.put("amount", paymentDetails.get(currentPaymentDetailIndex).getAmount()); - amountAndWriteOff.put("writeoff", paymentDetails.get(currentPaymentDetailIndex) - .getWriteoffAmount()); + amountAndWriteOff.put("amount", paymentDetail.getAmount()); + amountAndWriteOff.put("writeoff", paymentDetail.getWriteoffAmount()); // If the Payment Detail has either an Invoice or an Order associated to it if (psi != null || pso != null) { // If the Payment Detail has no Order associated to it, or it has an Invoice associated and is // the same one as the previous Payment Detail if ((psi != null && psi.equals(ps)) || pso == null) { - FIN_PaymentScheduleDetail psdNext = (currentPaymentDetailIndex == paymentDetails.size() - 1) ? null - : paymentDetails.get(currentPaymentDetailIndex + 1).getFINPaymentScheduleDetailList() - .get(0); - FIN_PaymentScheduleDetail psdPrevious = (currentPaymentDetailIndex == 0) ? null - : paymentDetails.get(currentPaymentDetailIndex - 1).getFINPaymentScheduleDetailList() - .get(0); + FIN_PaymentDetail paymentDetailNext = null; + FIN_PaymentDetail paymentDetailPrevious = null; + FIN_PaymentScheduleDetail psdNext = null; + FIN_PaymentScheduleDetail psdPrevious = null; + if (paymentDetailNextId != null) { + paymentDetailNext = OBDal.getInstance().get(FIN_PaymentDetail.class, paymentDetailNextId); + psdNext = paymentDetailNext.getFINPaymentScheduleDetailList().get(0); + } + if (paymentDetailPreviousId != null) { + paymentDetailPrevious = OBDal.getInstance().get(FIN_PaymentDetail.class, + paymentDetailPreviousId); + psdPrevious = paymentDetailPrevious.getFINPaymentScheduleDetailList().get(0); + } // If the Payment Detail has no Order associated, and the next Payment Detail belongs to the // same Invoice and it has an Order related, then return null if (pso == null && psdNext != null && psdNext.getInvoicePaymentSchedule() == psi @@ -2982,15 +3032,13 @@ // related to it, return the sum of amounts. } else if (psdPrevious != null && psdPrevious.getInvoicePaymentSchedule() == psi && psdPrevious.getOrderPaymentSchedule() == null) { - amountAndWriteOff.put("amount", paymentDetails.get(currentPaymentDetailIndex).getAmount() - .add(paymentDetails.get(currentPaymentDetailIndex - 1).getAmount())); - amountAndWriteOff.put( - "writeoff", - paymentDetails.get(currentPaymentDetailIndex).getWriteoffAmount() - .add(paymentDetails.get(currentPaymentDetailIndex - 1).getWriteoffAmount())); + amountAndWriteOff.put("amount", + paymentDetail.getAmount().add(paymentDetailPrevious.getAmount())); + amountAndWriteOff.put("writeoff", + paymentDetail.getWriteoffAmount().add(paymentDetailPrevious.getWriteoffAmount())); if (fieldProvider != null) { - FieldProviderFactory.setField(fieldProvider, "MergedPaymentDetailId", paymentDetails - .get(currentPaymentDetailIndex - 1).getId()); + FieldProviderFactory.setField(fieldProvider, "MergedPaymentDetailId", + paymentDetailPreviousId); } } } diff -r 0759c894c939 -r 1817418c55af src/org/openbravo/erpCommon/ad_forms/DocFINFinAccTransaction.java --- a/src/org/openbravo/erpCommon/ad_forms/DocFINFinAccTransaction.java Mon Jan 11 13:20:26 2016 +0100 +++ b/src/org/openbravo/erpCommon/ad_forms/DocFINFinAccTransaction.java Tue Dec 29 11:32:34 2015 +0100 @@ -114,38 +114,48 @@ public FieldProviderFactory[] loadLinesPaymentDetailsFieldProvider( FIN_FinaccTransaction transaction) { FIN_Payment payment = transaction.getFinPayment(); - List<FIN_PaymentDetail> paymentDetails = FIN_Utility.getOrderedPaymentDetailList(payment); + List<String> paymentDetails = FIN_Utility.getOrderedPaymentDetailList(payment.getId()); FieldProviderFactory[] data = new FieldProviderFactory[paymentDetails.size()]; FIN_PaymentSchedule ps = null; FIN_PaymentDetail pd = null; OBContext.setAdminMode(); try { for (int i = 0; i < data.length; i++) { + FIN_PaymentDetail paymentDetail = OBDal.getInstance().get(FIN_PaymentDetail.class, + paymentDetails.get(i)); /* * if (!getPaymentConfirmation(payment)) continue; */ // Details refunded used credit are excluded as the entry will be created using the credit // used - if (paymentDetails.get(i).isRefund() && paymentDetails.get(i).isPrepayment()) { + if (paymentDetail.isRefund() && paymentDetail.isPrepayment()) { continue; } // If the Payment Detail has already been processed, skip it - if (paymentDetails.get(i).equals(pd)) { + if (paymentDetail.equals(pd)) { continue; } - pd = paymentDetails.get(i); + pd = paymentDetail; data[i] = new FieldProviderFactory(null); - FIN_PaymentSchedule psi = paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) + FIN_PaymentSchedule psi = paymentDetail.getFINPaymentScheduleDetailList().get(0) .getInvoicePaymentSchedule(); - FIN_PaymentSchedule pso = paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) + FIN_PaymentSchedule pso = paymentDetail.getFINPaymentScheduleDetailList().get(0) .getOrderPaymentSchedule(); // Related to Issue Issue 19567. Some Payment Detail's amount and writeoff amount are merged // into one. // https://issues.openbravo.com/view.php?id=19567 + String paymentDetailNextId = null; + String paymentDetailPreviousId = null; + if (i < paymentDetails.size() - 1) { + paymentDetailNextId = paymentDetails.get(i + 1); + } + if (i > 0) { + paymentDetailPreviousId = paymentDetails.get(i - 1); + } HashMap<String, BigDecimal> amountAndWriteOff = getPaymentDetailWriteOffAndAmount( - paymentDetails, ps, psi, pso, i, data[i]); + paymentDetail, paymentDetailNextId, paymentDetailPreviousId, ps, psi, pso, data[i]); BigDecimal amount = amountAndWriteOff.get("amount"); BigDecimal writeOff = amountAndWriteOff.get("writeoff"); if (amount == null) { @@ -158,150 +168,131 @@ ps = psi; FieldProviderFactory.setField(data[i], "FIN_Finacc_Transaction_ID", transaction.getId()); - FieldProviderFactory.setField(data[i], "AD_Client_ID", paymentDetails.get(i).getClient() - .getId()); - FieldProviderFactory.setField(data[i], "AD_Org_ID", paymentDetails.get(i).getOrganization() - .getId()); - FieldProviderFactory.setField(data[i], "FIN_Payment_Detail_ID", paymentDetails.get(i) - .getId()); + FieldProviderFactory.setField(data[i], "AD_Client_ID", paymentDetail.getClient().getId()); + FieldProviderFactory + .setField(data[i], "AD_Org_ID", paymentDetail.getOrganization().getId()); + FieldProviderFactory.setField(data[i], "FIN_Payment_Detail_ID", paymentDetail.getId()); FieldProviderFactory.setField(data[i], "FIN_Payment_ID", payment.getId()); FieldProviderFactory.setField(data[i], "DepositAmount", transaction.getDepositAmount() .toString()); FieldProviderFactory.setField(data[i], "PaymentAmount", transaction.getPaymentAmount() .toString()); - FieldProviderFactory.setField(data[i], "DoubtFulDebtAmount", paymentDetails.get(i) + FieldProviderFactory.setField(data[i], "DoubtFulDebtAmount", paymentDetail .getFINPaymentScheduleDetailList().get(0).getDoubtfulDebtAmount().toString()); - FieldProviderFactory.setField(data[i], "isprepayment", - paymentDetails.get(i).isPrepayment() ? "Y" : "N"); + FieldProviderFactory.setField(data[i], "isprepayment", paymentDetail.isPrepayment() ? "Y" + : "N"); // Check if payment against invoice is in a previous date than invoice accounting date - boolean isPaymentDatePriorToInvoiceDate = isPaymentDatePriorToInvoiceDate(paymentDetails - .get(i)); + boolean isPaymentDatePriorToInvoiceDate = isPaymentDatePriorToInvoiceDate(paymentDetail); FieldProviderFactory.setField(data[i], "isPaymentDatePriorToInvoiceDate", - isPaymentDatePriorToInvoiceDate && !paymentDetails.get(i).isPrepayment() ? "Y" : "N"); + isPaymentDatePriorToInvoiceDate && !paymentDetail.isPrepayment() ? "Y" : "N"); FieldProviderFactory.setField(data[i], "WriteOffAmt", writeOff.toString()); FieldProviderFactory.setField(data[i], "cGlItemId", - paymentDetails.get(i).getGLItem() != null ? paymentDetails.get(i).getGLItem().getId() - : ""); + paymentDetail.getGLItem() != null ? paymentDetail.getGLItem().getId() : ""); // Calculate Business Partner from payment header or from details if header is null BusinessPartner bPartner = payment.getBusinessPartner() != null ? payment - .getBusinessPartner() : (paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) - .getInvoicePaymentSchedule() != null ? paymentDetails.get(i) - .getFINPaymentScheduleDetailList().get(0).getInvoicePaymentSchedule().getInvoice() - .getBusinessPartner() : (paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) - .getOrderPaymentSchedule() != null ? paymentDetails.get(i) - .getFINPaymentScheduleDetailList().get(0).getOrderPaymentSchedule().getOrder() - .getBusinessPartner() : (paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) - .getBusinessPartner()))); + .getBusinessPartner() + : (paymentDetail.getFINPaymentScheduleDetailList().get(0).getInvoicePaymentSchedule() != null ? paymentDetail + .getFINPaymentScheduleDetailList().get(0).getInvoicePaymentSchedule().getInvoice() + .getBusinessPartner() + : (paymentDetail.getFINPaymentScheduleDetailList().get(0).getOrderPaymentSchedule() != null ? paymentDetail + .getFINPaymentScheduleDetailList().get(0).getOrderPaymentSchedule().getOrder() + .getBusinessPartner() + : (paymentDetail.getFINPaymentScheduleDetailList().get(0).getBusinessPartner()))); FieldProviderFactory.setField(data[i], "cBpartnerId", bPartner != null ? bPartner.getId() : ""); - FieldProviderFactory.setField(data[i], "Refund", paymentDetails.get(i).isRefund() ? "Y" - : "N"); - FieldProviderFactory.setField(data[i], "adOrgId", paymentDetails.get(i).getOrganization() - .getId()); + FieldProviderFactory.setField(data[i], "Refund", paymentDetail.isRefund() ? "Y" : "N"); + FieldProviderFactory.setField(data[i], "adOrgId", paymentDetail.getOrganization().getId()); FieldProviderFactory.setField(data[i], "description", transaction.getDescription()); FieldProviderFactory.setField(data[i], "cCurrencyId", transaction.getCurrency().getId()); - FieldProviderFactory.setField(data[i], "cProjectId", paymentDetails.get(i) + FieldProviderFactory.setField(data[i], "cProjectId", paymentDetail .getFINPaymentScheduleDetailList().get(0).getInvoicePaymentSchedule() != null - && paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) - .getInvoicePaymentSchedule().getInvoice().getProject() != null ? paymentDetails - .get(i).getFINPaymentScheduleDetailList().get(0).getInvoicePaymentSchedule() - .getInvoice().getProject().getId() : (paymentDetails.get(i) - .getFINPaymentScheduleDetailList().get(0).getOrderPaymentSchedule() != null - && paymentDetails.get(i).getFINPaymentScheduleDetailList().get(0) ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
