From b4c93c76b3e292d3b4a79872d143f0814aa2d24c Mon Sep 17 00:00:00 2001 From: hengsin Date: Sun, 1 Nov 2020 19:43:36 +0800 Subject: [PATCH] IDEMPIERE-4516 InvoiceCustomerTest show NPE at console (#342) * IDEMPIERE-4516 InvoiceCustomerTest show NPE at console add NPE check * IDEMPIERE-4516 InvoiceCustomerTest show NPE at console - add way to defer the immediate posting of doc that's means to be posted in the processing of docpostprocess of the source document. - add severe log check to InvoiceCustomerTest - DocumentEngine should skip Doc from DocPostProcess that have been posted --- .../src/org/compiere/acct/Doc.java | 6 +-- .../org/compiere/acct/Doc_AllocationHdr.java | 36 +++++++------- .../src/org/compiere/model/MInOut.java | 2 + .../src/org/compiere/model/MPayment.java | 8 ++++ .../src/org/compiere/model/PO.java | 22 +++++++-- .../org/compiere/process/DocumentEngine.java | 34 ++++++++++--- .../test/model/InvoiceCustomerTest.java | 48 ++++++++++++++----- 7 files changed, 115 insertions(+), 41 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/acct/Doc.java b/org.adempiere.base/src/org/compiere/acct/Doc.java index c6818ad9da..aab3a6403c 100644 --- a/org.adempiere.base/src/org/compiere/acct/Doc.java +++ b/org.adempiere.base/src/org/compiere/acct/Doc.java @@ -1465,13 +1465,13 @@ public abstract class Doc else { - log.severe ("Not found AcctType=" + AcctType); + log.warning("Not found AcctType=" + AcctType); return 0; } // Do we have sql & Parameter if (sql == null || para_1 == 0) { - log.severe ("No Parameter for AcctType=" + AcctType + " - SQL=" + sql); + log.warning("No Parameter for AcctType=" + AcctType + " - SQL=" + sql); return 0; } @@ -1505,7 +1505,7 @@ public abstract class Doc // No account if (Account_ID == 0) { - log.severe ("NO account Type=" + log.warning("NO account Type=" + AcctType + ", Record=" + p_po.get_ID()); return 0; } diff --git a/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java b/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java index 7eb355217b..f73c14df2c 100644 --- a/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java +++ b/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java @@ -835,7 +835,7 @@ public class Doc_AllocationHdr extends Doc // For Invoice List valuesInv = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MInvoice.Table_ID, invoice.getC_Invoice_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID()); - if (valuesInv != null) { + if (valuesInv != null && valuesInv.size() >= 4) { if (invoice.getReversal_ID() == 0 || invoice.get_ID() < invoice.getReversal_ID()) { if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) @@ -963,12 +963,14 @@ public class Doc_AllocationHdr extends Doc // For Payment List valuesPay = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MPayment.Table_ID, payment.getC_Payment_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID()); - if (valuesPay != null) { + if (valuesPay != null && valuesPay.size() >= 4) { paymentSource = (BigDecimal) valuesPay.get(0); // AmtSourceDr paymentAccounted = (BigDecimal) valuesPay.get(1); // AmtAcctDr - if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) { - paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr - paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr + if (paymentSource != null && paymentAccounted != null) { + if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) { + paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr + paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr + } } } @@ -1077,7 +1079,7 @@ public class Doc_AllocationHdr extends Doc // For Invoice List valuesInv = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MInvoice.Table_ID, invoice.getC_Invoice_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID()); - if (valuesInv != null) { + if (valuesInv != null && valuesInv.size() >= 4) { BigDecimal invoiceSource = null; BigDecimal invoiceAccounted = null; if (invoice.getReversal_ID() == 0 || invoice.get_ID() < invoice.getReversal_ID()) @@ -1236,7 +1238,7 @@ public class Doc_AllocationHdr extends Doc // For Allocation List valuesAlloc = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MAllocationHdr.Table_ID, alloc.get_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID(), alloc.get_ID(), invoice.getC_Invoice_ID()); - if (valuesAlloc != null) { + if (valuesAlloc != null && valuesAlloc.size() >= 4) { totalAmtSourceDr = (BigDecimal) valuesAlloc.get(0); if (totalAmtSourceDr == null) totalAmtSourceDr = Env.ZERO; @@ -1291,7 +1293,7 @@ public class Doc_AllocationHdr extends Doc MAllocationHdr.Table_ID, alloc.get_ID(), as.getC_AcctSchema_ID(), gain.getAccount_ID(), loss.getAccount_ID(), as.getCurrencyBalancing_Acct().getAccount_ID(), alloc.get_ID(), invoice.getC_Invoice_ID()); - if (valuesAlloc != null) { + if (valuesAlloc != null && valuesAlloc.size() >= 4) { totalAmtSourceDr = (BigDecimal) valuesAlloc.get(0); if (totalAmtSourceDr == null) totalAmtSourceDr = Env.ZERO; @@ -1428,15 +1430,17 @@ public class Doc_AllocationHdr extends Doc // For Payment List valuesPay = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MPayment.Table_ID, payment.getC_Payment_ID(), as.getC_AcctSchema_ID(), htPayAcct.get(payment.getC_Payment_ID()).getAccount_ID()); - if (valuesPay != null) { + if (valuesPay != null && valuesPay.size() >= 4) { BigDecimal paymentSource = (BigDecimal) valuesPay.get(0); // AmtSourceDr BigDecimal paymentAccounted = (BigDecimal) valuesPay.get(1); // AmtAcctDr - if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) { - paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr - paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr + if (paymentSource != null && paymentAccounted != null) { + if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) { + paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr + paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr + } + htPaySource.put(payment.getC_Payment_ID(), paymentSource); + htPayAccounted.put(payment.getC_Payment_ID(), paymentAccounted); } - htPaySource.put(payment.getC_Payment_ID(), paymentSource); - htPayAccounted.put(payment.getC_Payment_ID(), paymentAccounted); } } @@ -1565,7 +1569,7 @@ public class Doc_AllocationHdr extends Doc // For Allocation List valuesAlloc = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MAllocationHdr.Table_ID, alloc.get_ID(), as.getC_AcctSchema_ID(), htPayAcct.get(payment.getC_Payment_ID()).getAccount_ID(), alloc.get_ID(), payment.getC_Payment_ID()); - if (valuesAlloc != null) { + if (valuesAlloc != null && valuesAlloc.size() >= 4) { totalAmtSourceDr = (BigDecimal) valuesAlloc.get(0); if (totalAmtSourceDr == null) totalAmtSourceDr = Env.ZERO; @@ -1620,7 +1624,7 @@ public class Doc_AllocationHdr extends Doc MAllocationHdr.Table_ID, alloc.get_ID(), as.getC_AcctSchema_ID(), gain.getAccount_ID(), loss.getAccount_ID(), as.getCurrencyBalancing_Acct().getAccount_ID(), alloc.get_ID(), payment.getC_Payment_ID()); - if (valuesAlloc != null) { + if (valuesAlloc != null && valuesAlloc.size() >= 4) { totalAmtSourceDr = (BigDecimal) valuesAlloc.get(0); if (totalAmtSourceDr == null) totalAmtSourceDr = Env.ZERO; diff --git a/org.adempiere.base/src/org/compiere/model/MInOut.java b/org.adempiere.base/src/org/compiere/model/MInOut.java index 15c934267c..9bdceb27c4 100644 --- a/org.adempiere.base/src/org/compiere/model/MInOut.java +++ b/org.adempiere.base/src/org/compiere/model/MInOut.java @@ -1816,6 +1816,8 @@ public class MInOut extends X_M_InOut implements DocAction if (log.isLoggable(Level.FINE)) log.fine(dropShipment.toString()); dropShipment.setDocAction(DocAction.ACTION_Complete); + // do not post immediate dropshipment, should post after source shipment + dropShipment.set_Attribute(DocumentEngine.DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE, Boolean.FALSE); // added AdempiereException by Zuhri if (!dropShipment.processIt(DocAction.ACTION_Complete)) throw new AdempiereException(Msg.getMsg(getCtx(), "FailedProcessingDocument") + " - " + dropShipment.getProcessMsg()); diff --git a/org.adempiere.base/src/org/compiere/model/MPayment.java b/org.adempiere.base/src/org/compiere/model/MPayment.java index 96e6958c1c..9bef309f0d 100644 --- a/org.adempiere.base/src/org/compiere/model/MPayment.java +++ b/org.adempiere.base/src/org/compiere/model/MPayment.java @@ -2355,6 +2355,8 @@ public class MPayment extends X_C_Payment pa.saveEx(); } } + //do not post immediate alloc, alloc should post after payment + alloc.set_Attribute(DocumentEngine.DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE, Boolean.FALSE); // added AdempiereException by zuhri if (!alloc.processIt(DocAction.ACTION_Complete)) throw new AdempiereException(Msg.getMsg(getCtx(), "FailedProcessingDocument") + " - " + alloc.getProcessMsg()); @@ -2395,6 +2397,8 @@ public class MPayment extends X_C_Payment aLine.setDocInfo(getC_BPartner_ID(), 0, getC_Invoice_ID()); aLine.setC_Payment_ID(getC_Payment_ID()); aLine.saveEx(get_TrxName()); + //do not post immediate alloc + alloc.set_Attribute(DocumentEngine.DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE, Boolean.FALSE); // added AdempiereException by zuhri if (!alloc.processIt(DocAction.ACTION_Complete)) throw new AdempiereException(Msg.getMsg(getCtx(), "FailedProcessingDocument") + " - " + alloc.getProcessMsg()); @@ -2492,6 +2496,8 @@ public class MPayment extends X_C_Payment } else { + //do not post immediate alloc + alloc.set_Attribute(DocumentEngine.DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE, Boolean.FALSE); // added Adempiere Exception by zuhri if (alloc.processIt(DocAction.ACTION_Complete)) { addDocsPostProcess(alloc); @@ -2783,6 +2789,8 @@ public class MPayment extends X_C_Payment if (!aLine.save(get_TrxName())) log.warning("Automatic allocation - reversal line not saved"); + //do not post immediate alloc + alloc.set_Attribute(DocumentEngine.DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE, Boolean.FALSE); // added AdempiereException by zuhri if (!alloc.processIt(DocAction.ACTION_Complete)) throw new AdempiereException(Msg.getMsg(getCtx(), "FailedProcessingDocument") + " - " + alloc.getProcessMsg()); diff --git a/org.adempiere.base/src/org/compiere/model/PO.java b/org.adempiere.base/src/org/compiere/model/PO.java index 0b12e38d3e..10c3848113 100644 --- a/org.adempiere.base/src/org/compiere/model/PO.java +++ b/org.adempiere.base/src/org/compiere/model/PO.java @@ -4880,20 +4880,34 @@ public abstract class PO p_info = initPO(p_ctx); } - public void set_Attribute(String columnName, Object value) { + /** + * set attribute value + * @param attributeName + * @param value + */ + public void set_Attribute(String attributeName, Object value) { checkImmutable(); if (m_attributes == null) m_attributes = new HashMap(); - m_attributes.put(columnName, value); + m_attributes.put(attributeName, value); } - public Object get_Attribute(String columnName) { + /** + * + * @param attributeName + * @return attribute value + */ + public Object get_Attribute(String attributeName) { if (m_attributes != null) - return m_attributes.get(columnName); + return m_attributes.get(attributeName); return null; } + /** + * + * @return map of attributes + */ public HashMap get_Attributes() { return m_attributes; } diff --git a/org.adempiere.base/src/org/compiere/process/DocumentEngine.java b/org.adempiere.base/src/org/compiere/process/DocumentEngine.java index ce9c400abb..08719ed219 100644 --- a/org.adempiere.base/src/org/compiere/process/DocumentEngine.java +++ b/org.adempiere.base/src/org/compiere/process/DocumentEngine.java @@ -73,6 +73,11 @@ import org.osgi.service.event.Event; */ public class DocumentEngine implements DocAction { + /** + * When client accounting is immediate, set this PO attribute to override the default of immediate posting after complete of document + */ + public static final String DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE = "Document.PostImmediateAfterComplete"; + /** * Doc Engine (Drafted) * @param po document @@ -343,13 +348,28 @@ public class DocumentEngine implements DocAction if (STATUS_Completed.equals(status) && MClient.isClientAccountingImmediate()) { - m_document.saveEx(); - postIt(); - - if (m_document instanceof PO && docsPostProcess.size() > 0) { - for (PO docafter : docsPostProcess) { - @SuppressWarnings("unused") - String ignoreError = DocumentEngine.postImmediate(docafter.getCtx(), docafter.getAD_Client_ID(), docafter.get_Table_ID(), docafter.get_ID(), true, docafter.get_TrxName()); + boolean postNow = true; + if (m_document instanceof PO) + { + Object attribute = ((PO) m_document).get_Attribute(DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE); + if (attribute != null && attribute instanceof Boolean) + { + postNow = (boolean) attribute; + } + } + + if (postNow) + { + m_document.saveEx(); + postIt(); + + if (m_document instanceof PO && docsPostProcess.size() > 0) { + for (PO docafter : docsPostProcess) { + if (docafter.get_ValueAsBoolean("Posted")) + continue; + @SuppressWarnings("unused") + String ignoreError = DocumentEngine.postImmediate(docafter.getCtx(), docafter.getAD_Client_ID(), docafter.get_Table_ID(), docafter.get_ID(), true, docafter.get_TrxName()); + } } } } diff --git a/org.idempiere.test/src/org/idempiere/test/model/InvoiceCustomerTest.java b/org.idempiere.test/src/org/idempiere/test/model/InvoiceCustomerTest.java index 84f75956a4..3f4e78e813 100644 --- a/org.idempiere.test/src/org/idempiere/test/model/InvoiceCustomerTest.java +++ b/org.idempiere.test/src/org/idempiere/test/model/InvoiceCustomerTest.java @@ -30,14 +30,18 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.math.BigDecimal; import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.logging.LogRecord; import org.compiere.model.MBPartner; import org.compiere.model.MDocType; import org.compiere.model.MInvoice; import org.compiere.model.MInvoiceLine; import org.compiere.model.MPayment; +import org.compiere.model.PO; import org.compiere.process.DocAction; import org.compiere.process.ProcessInfo; +import org.compiere.util.CLogErrorBuffer; import org.compiere.util.Env; import org.compiere.util.TimeUtil; import org.compiere.wf.MWorkflow; @@ -59,6 +63,11 @@ public class InvoiceCustomerTest extends AbstractTestCase { * https://idempiere.atlassian.net/browse/IDEMPIERE-829 */ public void testOpenAmt() { + int severeCount = 0; + LogRecord[] errorLogs = CLogErrorBuffer.get(true).getRecords(true); + if (errorLogs != null) + severeCount = errorLogs.length; + // Invoice $200 today MInvoice invoice = new MInvoice(Env.getCtx(), 0, getTrxName()); invoice.setBPartner(MBPartner.get(Env.getCtx(), 117)); // C&W @@ -89,9 +98,10 @@ public class InvoiceCustomerTest extends AbstractTestCase { ProcessInfo info = MWorkflow.runDocumentActionWorkflow(invoice, DocAction.ACTION_Complete); invoice.load(getTrxName()); - assertFalse(info.isError()); - assertEquals(DocAction.STATUS_Completed, invoice.getDocStatus()); - assertTrue(TWOHUNDRED.compareTo(invoice.getGrandTotal()) == 0); + assertFalse(info.isError(), "Error processing invoice: " + info.getSummary()); + assertEquals(DocAction.STATUS_Completed, invoice.getDocStatus(), "Invoice document status is not completed: " + invoice.getDocStatus()); + assertTrue(TWOHUNDRED.compareTo(invoice.getGrandTotal()) == 0, "Invoice grand total not as expected: " + invoice.getGrandTotal().toPlainString()); + assertTrue(invoice.isPosted(), "Invoice not posted"); // first $100 payment next week MPayment payment1 = new MPayment(Env.getCtx(), 0, getTrxName()); @@ -111,10 +121,16 @@ public class InvoiceCustomerTest extends AbstractTestCase { info = MWorkflow.runDocumentActionWorkflow(payment1, DocAction.ACTION_Complete); payment1.load(getTrxName()); - assertFalse(info.isError()); - assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus()); - assertEquals(false, invoice.isPaid()); - + assertFalse(info.isError(), "Error processing payment: " + info.getSummary()); + assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus(), "Payment document status is not completed: " + payment1.getDocStatus()); + assertEquals(false, invoice.isPaid(), "Invoice isPaid() is not false"); + assertTrue(payment1.isPosted(), "Payment not posted"); + + ArrayList postProcessDocs = payment1.getDocsPostProcess(); + for(PO postProcessDoc : postProcessDocs) { + assertTrue(postProcessDoc.get_ValueAsBoolean("Posted"), "Post Process Doc not posted: " + postProcessDoc); + } + // second $100 payment next two weeks MPayment payment2 = new MPayment(Env.getCtx(), 0, getTrxName()); payment2.setC_Invoice_ID(invoice.getC_Invoice_ID()); @@ -133,16 +149,26 @@ public class InvoiceCustomerTest extends AbstractTestCase { info = MWorkflow.runDocumentActionWorkflow(payment2, DocAction.ACTION_Complete); payment2.load(getTrxName()); - assertFalse(info.isError()); - assertEquals(DocAction.STATUS_Completed, payment2.getDocStatus()); + assertFalse(info.isError(), "Error processing payment: " + info.getSummary()); + assertEquals(DocAction.STATUS_Completed, payment2.getDocStatus(), "Payment document status is not completed: " + payment2.getDocStatus()); + assertTrue(payment2.isPosted(), "Payment not posted"); + postProcessDocs = payment2.getDocsPostProcess(); + for(PO postProcessDoc : postProcessDocs) { + assertTrue(postProcessDoc.get_ValueAsBoolean("Posted"), "Post Process Doc not posted: " + postProcessDoc); + } + invoice.load(getTrxName()); - assertEquals(true, invoice.isPaid()); - assertTrue(Env.ZERO.compareTo(invoice.getOpenAmt()) == 0); + assertEquals(true, invoice.isPaid(), "Invoice isPaid() is not true"); + assertTrue(Env.ZERO.compareTo(invoice.getOpenAmt()) == 0, "Invoice open amount not zero: " + invoice.getOpenAmt().toPlainString()); assertTrue(TWOHUNDRED.compareTo(invoice.getOpenAmt(false, today, true)) == 0); assertTrue(Env.ONEHUNDRED.compareTo(invoice.getOpenAmt(false, nextweek, true)) == 0); assertTrue(Env.ZERO.compareTo(invoice.getOpenAmt(false, next2weeks, true)) == 0); + errorLogs = CLogErrorBuffer.get(true).getRecords(true); + if (errorLogs != null) + assertEquals(severeCount, errorLogs.length, "Severe errors recorded in log: " + errorLogs.length); + rollback(); } }