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

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;
 		}

org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java

@@ -835,7 +835,7 @@ public class Doc_AllocationHdr extends Doc
 		// For Invoice
 		List<Object> 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<Object> 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) {
+			if (paymentSource != null && paymentAccounted != null) {
-				paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr
+				if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) {
-				paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr
+					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<Object> 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<Object> 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<Object> 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) {
+				if (paymentSource != null && paymentAccounted != null) {
-					paymentSource = (BigDecimal) valuesPay.get(2); // AmtSourceCr
+					if (paymentSource.signum() == 0 && paymentAccounted.signum() == 0) {
-					paymentAccounted = (BigDecimal) valuesPay.get(3); // AmtAcctCr
+						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<Object> 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;

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());

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());

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<String, Object>();
-		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<String,Object> get_Attributes() {
 		return m_attributes;
 	}

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();
+					boolean postNow = true;
-					postIt();
+					if (m_document instanceof PO)
-
+					{
-					if (m_document instanceof PO && docsPostProcess.size() > 0) {
+						Object attribute = ((PO) m_document).get_Attribute(DOCUMENT_POST_IMMEDIATE_AFTER_COMPLETE);
-						for (PO docafter : docsPostProcess) {
+						if (attribute != null && attribute instanceof Boolean)
-							@SuppressWarnings("unused")
+						{
-							String ignoreError = DocumentEngine.postImmediate(docafter.getCtx(), docafter.getAD_Client_ID(), docafter.get_Table_ID(), docafter.get_ID(), true, docafter.get_TrxName());
+							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());
+							}
 						}
 					}
 				}

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());
+		assertFalse(info.isError(), "Error processing invoice: " + info.getSummary());
-		assertEquals(DocAction.STATUS_Completed, invoice.getDocStatus());
+		assertEquals(DocAction.STATUS_Completed, invoice.getDocStatus(), "Invoice document status is not completed: " + invoice.getDocStatus());
-		assertTrue(TWOHUNDRED.compareTo(invoice.getGrandTotal()) == 0);
+		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());
+		assertFalse(info.isError(), "Error processing payment: " + info.getSummary());
-		assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus());
+		assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus(), "Payment document status is not completed: " + payment1.getDocStatus());
-		assertEquals(false, invoice.isPaid());
+		assertEquals(false, invoice.isPaid(), "Invoice isPaid() is not false");
-
+		assertTrue(payment1.isPosted(), "Payment not posted");
+		
+		ArrayList<PO> 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());
+		assertFalse(info.isError(), "Error processing payment: " + info.getSummary());
-		assertEquals(DocAction.STATUS_Completed, payment2.getDocStatus());
+		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());
+		assertEquals(true, invoice.isPaid(), "Invoice isPaid() is not true");
-		assertTrue(Env.ZERO.compareTo(invoice.getOpenAmt()) == 0);
+		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();
 	}
 }