From afcbf6e70fb390f336b9d9a078b57e3b6f7a2060 Mon Sep 17 00:00:00 2001 From: hengsin Date: Sat, 6 May 2023 01:41:43 +0800 Subject: [PATCH] =?UTF-8?q?IDEMPIERE-5707=20PostgreSQL=20should=20use=20FO?= =?UTF-8?q?R=20NO=20KEY=20UPDATE=20instead=20of=20FOR=E2=80=A6=20(#1818)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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/exceptions/DBException.java | 10 +++ .../src/org/compiere/util/Trx.java | 7 +- .../src/org/compiere/db/DB_Oracle.java | 2 +- .../src/org/compiere/db/DB_PostgreSQL.java | 4 +- .../src/org/idempiere/test/base/DBTest.java | 82 +++++++++++++++++++ 5 files changed, 97 insertions(+), 8 deletions(-) diff --git a/org.adempiere.base/src/org/adempiere/exceptions/DBException.java b/org.adempiere.base/src/org/adempiere/exceptions/DBException.java index 7660243644..1f7254a68d 100644 --- a/org.adempiere.base/src/org/adempiere/exceptions/DBException.java +++ b/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 */ diff --git a/org.adempiere.base/src/org/compiere/util/Trx.java b/org.adempiere.base/src/org/compiere/util/Trx.java index 060cbe24ab..1ecdecec8a 100644 --- a/org.adempiere.base/src/org/compiere/util/Trx.java +++ b/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(); diff --git a/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java b/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java index 6ad62abea2..97a4510f5f 100644 --- a/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java +++ b/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); } diff --git a/org.compiere.db.postgresql.provider/src/org/compiere/db/DB_PostgreSQL.java b/org.compiere.db.postgresql.provider/src/org/compiere/db/DB_PostgreSQL.java index 30a6c27a0a..6855148df9 100755 --- a/org.compiere.db.postgresql.provider/src/org/compiere/db/DB_PostgreSQL.java +++ b/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; diff --git a/org.idempiere.test/src/org/idempiere/test/base/DBTest.java b/org.idempiere.test/src/org/idempiere/test/base/DBTest.java index 9f45d6c1ff..a55a25982c 100644 --- a/org.idempiere.test/src/org/idempiere/test/base/DBTest.java +++ b/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(); + } + } }