IDEMPIERE-5707 PostgreSQL should use FOR NO KEY UPDATE instead of FOR… (#1818)

* IDEMPIERE-5707 PostgreSQL should use FOR NO KEY UPDATE instead of FOR UPDATE

* IDEMPIERE-5707 PostgreSQL should use FOR NO KEY UPDATE instead of FOR UPDATE

- add trx timeout unit test

org.adempiere.base/src/org/adempiere/exceptions/DBException.java

@@ -79,6 +79,16 @@ public class DBException extends AdempiereException
 		super(msg);
 	}
 
+	/**
+	 * Create a new DBException
+	 * @param msg Message
+	 * @param e Exception
+	 */
+	public DBException(String msg, Exception e)
+	{
+		super(msg, e);
+	}
+	
 	/**
 	 * @return SQL Query or null
 	 */

org.adempiere.base/src/org/compiere/util/Trx.java

@@ -460,20 +460,17 @@ public class Trx
 	 * 	Rollback and End Transaction, Close Connection and Throws an Exception
 	 *	@return true if success
 	 */
-	public synchronized boolean rollbackAndCloseOnTimeout() {
+	public boolean rollbackAndCloseOnTimeout() {
 		s_cache.remove(getTrxName());
 
 		//local
 		if (m_connection == null)
 			return true;
 
-		if (isActive())
-			rollback();
-
 		//	Close Connection
 		try
 		{
-			m_connection.close();
+			m_connection.abort(Runnable::run);
 			m_connection = null;
 			m_active = false;
 			fireAfterCloseEvent();

org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java

@@ -929,7 +929,7 @@ public class DB_Oracle implements AdempiereDatabase
 				}
 			} catch (Exception e) {
 				if (log.isLoggable(Level.INFO))log.log(Level.INFO, e.getLocalizedMessage(), e);
-				throw new DBException("Could not lock record for " + po.toString() + " caused by " + e.getLocalizedMessage());
+				throw new DBException("Could not lock record for " + po.toString() + " caused by " + e.getLocalizedMessage(), e);
 			} finally {
 				DB.close(rs, stmt);
 			}			

org.compiere.db.postgresql.provider/src/org/compiere/db/DB_PostgreSQL.java

@@ -977,7 +977,7 @@ public class DB_PostgreSQL implements AdempiereDatabase
 					sqlBuffer.append(" AND ");
 				sqlBuffer.append(keyColumns[i]).append("=?");
 			}
-			sqlBuffer.append(" FOR UPDATE ");
+			sqlBuffer.append(" FOR NO KEY UPDATE ");
 
 			Object[] parameters = new Object[keyColumns.length];
 			for(int i = 0; i < keyColumns.length; i++) {
@@ -1009,7 +1009,7 @@ public class DB_PostgreSQL implements AdempiereDatabase
 				}
 			} catch (Exception e) {
 				if (log.isLoggable(Level.INFO))log.log(Level.INFO, e.getLocalizedMessage(), e);
-				throw new DBException("Could not lock record for " + po.toString() + " caused by " + e.getLocalizedMessage());
+				throw new DBException("Could not lock record for " + po.toString() + " caused by " + e.getLocalizedMessage(), e);
 			} finally {
 				DB.close(rs, stmt);
 				rs = null;stmt = null;

org.idempiere.test/src/org/idempiere/test/base/DBTest.java

@@ -14,23 +14,28 @@
 package org.idempiere.test.base;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.math.BigDecimal;
+import java.sql.SQLException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.adempiere.exceptions.DBException;
+import org.compiere.model.MBPartner;
+import org.compiere.model.MOrder;
 import org.compiere.model.MTable;
 import org.compiere.model.X_Test;
 import org.compiere.util.DB;
 import org.compiere.util.Env;
 import org.compiere.util.KeyNamePair;
 import org.compiere.util.TimeUtil;
+import org.compiere.util.Trx;
 import org.compiere.util.ValueNamePair;
 import org.idempiere.test.AbstractTestCase;
 import org.idempiere.test.DictionaryIDs;
@@ -291,4 +296,81 @@ public class DBTest extends AbstractTestCase
 		assertTrue(resultStr != null);
 	}
 	
+	@Test
+	public void testForUpdateAndForeignKey() 
+	{
+		try {
+			MBPartner bp = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, getTrxName());
+			DB.getDatabase().forUpdate(bp, 0);
+			
+			SQLException sqlException = null;
+			Trx trx2 = Trx.get(Trx.createTrxName(), true);
+			MOrder order = null;
+			try {
+				order = new MOrder(Env.getCtx(), 0, trx2.getTrxName());
+				order.setC_DocTypeTarget_ID(DictionaryIDs.C_DocType.STANDARD_ORDER.id);
+				order.setC_BPartner_ID(bp.get_ID());
+				order.setDateOrdered(getLoginDate());
+				Thread thread = new Thread (() -> { 
+					try {
+						Thread.sleep(15 * 1000);
+					} catch (InterruptedException e) {
+					}
+					if (trx2.isActive()) trx2.rollbackAndCloseOnTimeout();
+				});
+				thread.start();
+				order.saveEx();		
+				trx2.commit(true);
+			} catch (SQLException e) {
+				sqlException = e;
+				order = null;
+			} finally {
+				trx2.close();
+			}
+			assertNull(sqlException, "Failed to save and commit order: " + (sqlException != null ? sqlException.getMessage() : ""));
+			if (order != null && order.get_ID() > 0) {
+				order.set_TrxName(null);
+				order.deleteEx(true);
+			}
+		} finally {
+			rollback();
+		}				
+	}
+	
+	@Test
+	public void testTrxTimeout() 
+	{
+		try {
+			MBPartner bp = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, getTrxName());
+			DB.getDatabase().forUpdate(bp, 0);
+			
+			Exception exception = null;
+			Trx trx2 = Trx.get(Trx.createTrxName(), true);
+			MBPartner bp2 = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, trx2.getTrxName());
+			try {
+				Thread thread = new Thread (() -> { 
+					try {
+						Thread.sleep(5 * 1000);
+					} catch (InterruptedException e) {
+					}
+					if (trx2.isActive()) trx2.rollbackAndCloseOnTimeout();
+				});
+				thread.start();
+				DB.getDatabase().forUpdate(bp2, 10);
+			} catch (Exception e) {
+				exception = e;
+			} finally {
+				trx2.close();
+			}
+			assertNotNull(exception, "Exception not happens as expected");
+			assertTrue(exception instanceof DBException, "Exception not instanceof DBException");
+			DBException dbe = (DBException) exception;
+			if (DB.isPostgreSQL())
+				assertTrue("08006".equals(dbe.getSQLState()), "Trx2 not timeout as expected: " + dbe.getSQLException().getMessage());
+			else if (DB.isOracle())
+				assertTrue("08003".equals(dbe.getSQLState()), "Trx2 not timeout as expected: " + dbe.getSQLException().getMessage());
+		} finally {
+			rollback();
+		}				
+	}
 }