From 8bf236657c89bb2acc6909d1605b58e678f25ddc Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Wed, 23 Mar 2016 01:50:03 +0100 Subject: [PATCH] IDEMPIERE-3058 Make 2Pack transaction safe (for postgres) - added sysconfig 2PACK_COMMIT_DDL - by default now in postgresql the 2pack process everything in one transaction and rollback all in case of failure - now 2Pack is marked as failed if there are unresolved elements - fix a locking issue when creating change log and 2pack has locked ad_table foreign key - the Incremental2PackActivator now reprocess the packages that are not marked as Completed successfully - the Incremental2PackActivator stop processing more packin versions if one fails --- .../oracle/201603222022_IDEMPIERE-3058.sql | 23 ++++++++++++ .../201603222022_IDEMPIERE-3058.sql | 20 ++++++++++ .../src/org/compiere/model/MColumn.java | 34 ++++++++++++----- .../src/org/compiere/model/MIndexColumn.java | 2 +- .../src/org/compiere/model/MSysConfig.java | 5 ++- .../pipo2/handler/ColumnElementHandler.java | 11 ++++-- .../pipo2/handler/TableElementHandler.java | 11 ++++-- .../handler/TableIndexElementHandler.java | 11 ++++-- .../src/org/adempiere/pipo2/PackIn.java | 7 +++- .../org/adempiere/pipo2/PackInHandler.java | 37 ++++++++++--------- .../utils/Incremental2PackActivator.java | 11 ++++-- 11 files changed, 129 insertions(+), 43 deletions(-) create mode 100644 migration/i3.1/oracle/201603222022_IDEMPIERE-3058.sql create mode 100644 migration/i3.1/postgresql/201603222022_IDEMPIERE-3058.sql diff --git a/migration/i3.1/oracle/201603222022_IDEMPIERE-3058.sql b/migration/i3.1/oracle/201603222022_IDEMPIERE-3058.sql new file mode 100644 index 0000000000..a403ca18e7 --- /dev/null +++ b/migration/i3.1/oracle/201603222022_IDEMPIERE-3058.sql @@ -0,0 +1,23 @@ +SET SQLBLANKLINES ON +SET DEFINE OFF + +-- IDEMPIERE-3058 Make 2Pack transaction safe (for postgres) +-- Mar 22, 2016 7:30:44 PM CET +INSERT INTO AD_SysConfig (AD_SysConfig_ID,AD_Client_ID,AD_Org_ID,Created,Updated,CreatedBy,UpdatedBy,IsActive,Name,Value,Description,EntityType,ConfigurationLevel,AD_SysConfig_UU) VALUES (200076,0,0,TO_DATE('2016-03-22 19:30:38','YYYY-MM-DD HH24:MI:SS'),TO_DATE('2016-03-22 19:30:38','YYYY-MM-DD HH24:MI:SS'),100,100,'Y','2PACK_COMMIT_DDL','N','If set to Y 2Pack tries to behave in PostgreSQL same as with Oracle - committing before and after DDL statements','D','S','112f7659-c30f-45df-a505-ba85c4b6f83a') +; + +-- Mar 22, 2016 8:30:02 PM CET +UPDATE AD_Column SET AD_Reference_ID=30, FKConstraintType=NULL,Updated=TO_DATE('2016-03-22 20:30:02','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Column_ID=50025 +; + +-- Mar 22, 2016 9:59:58 PM CET +UPDATE AD_Column SET FieldLength=2000,Updated=TO_DATE('2016-03-22 21:59:58','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Column_ID=50073 +; + +-- Mar 22, 2016 10:00:02 PM CET +ALTER TABLE AD_Package_Imp_Detail MODIFY Name VARCHAR2(2000) DEFAULT NULL +; + +SELECT register_migration_script('201603222022_IDEMPIERE-3058.sql') FROM dual +; + diff --git a/migration/i3.1/postgresql/201603222022_IDEMPIERE-3058.sql b/migration/i3.1/postgresql/201603222022_IDEMPIERE-3058.sql new file mode 100644 index 0000000000..f71a241f8c --- /dev/null +++ b/migration/i3.1/postgresql/201603222022_IDEMPIERE-3058.sql @@ -0,0 +1,20 @@ +-- IDEMPIERE-3058 Make 2Pack transaction safe (for postgres) +-- Mar 22, 2016 7:30:44 PM CET +INSERT INTO AD_SysConfig (AD_SysConfig_ID,AD_Client_ID,AD_Org_ID,Created,Updated,CreatedBy,UpdatedBy,IsActive,Name,Value,Description,EntityType,ConfigurationLevel,AD_SysConfig_UU) VALUES (200076,0,0,TO_TIMESTAMP('2016-03-22 19:30:38','YYYY-MM-DD HH24:MI:SS'),TO_TIMESTAMP('2016-03-22 19:30:38','YYYY-MM-DD HH24:MI:SS'),100,100,'Y','2PACK_COMMIT_DDL','N','If set to Y 2Pack tries to behave in PostgreSQL same as with Oracle - committing before and after DDL statements','D','S','112f7659-c30f-45df-a505-ba85c4b6f83a') +; + +-- Mar 22, 2016 8:30:02 PM CET +UPDATE AD_Column SET AD_Reference_ID=30, FKConstraintType=NULL,Updated=TO_TIMESTAMP('2016-03-22 20:30:02','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Column_ID=50025 +; + +-- Mar 22, 2016 9:59:58 PM CET +UPDATE AD_Column SET FieldLength=2000,Updated=TO_TIMESTAMP('2016-03-22 21:59:58','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Column_ID=50073 +; + +-- Mar 22, 2016 10:00:02 PM CET +INSERT INTO t_alter_column values('ad_package_imp_detail','Name','VARCHAR(2000)',null,'NULL') +; + +SELECT register_migration_script('201603222022_IDEMPIERE-3058.sql') FROM dual +; + diff --git a/org.adempiere.base/src/org/compiere/model/MColumn.java b/org.adempiere.base/src/org/compiere/model/MColumn.java index 9b90e4dd40..c98f4d1b5e 100644 --- a/org.adempiere.base/src/org/compiere/model/MColumn.java +++ b/org.adempiere.base/src/org/compiere/model/MColumn.java @@ -51,7 +51,12 @@ public class MColumn extends X_AD_Column /** * */ - private static final long serialVersionUID = -7261365443985547106L; + private static final long serialVersionUID = 3082823885314140209L; + + public static MColumn get (Properties ctx, int AD_Column_ID) + { + return get(ctx, AD_Column_ID, null); + } /** * Get MColumn from Cache @@ -59,13 +64,15 @@ public class MColumn extends X_AD_Column * @param AD_Column_ID id * @return MColumn */ - public static MColumn get (Properties ctx, int AD_Column_ID) + public static MColumn get(Properties ctx, int AD_Column_ID, String trxName) { Integer key = new Integer (AD_Column_ID); MColumn retValue = (MColumn) s_cache.get (key); - if (retValue != null) + if (retValue != null) { + retValue.set_TrxName(trxName); return retValue; - retValue = new MColumn (ctx, AD_Column_ID, null); + } + retValue = new MColumn (ctx, AD_Column_ID, trxName); if (retValue.get_ID () != 0) s_cache.put (key, retValue); return retValue; @@ -84,15 +91,21 @@ public class MColumn extends X_AD_Column return table.getColumn(columnName); } // get + public static String getColumnName (Properties ctx, int AD_Column_ID) + { + return getColumnName (ctx, AD_Column_ID, null); + } + /** * Get Column Name * @param ctx context * @param AD_Column_ID id + * @param trxName transaction * @return Column Name or null */ - public static String getColumnName (Properties ctx, int AD_Column_ID) + public static String getColumnName (Properties ctx, int AD_Column_ID, String trxName) { - MColumn col = MColumn.get(ctx, AD_Column_ID); + MColumn col = MColumn.get(ctx, AD_Column_ID, trxName); if (col.get_ID() == 0) return null; return col.getColumnName(); @@ -709,9 +722,12 @@ public class MColumn extends X_AD_Column } else if (DisplayType.Table == refid || DisplayType.Search == refid) { X_AD_Reference ref = new X_AD_Reference(getCtx(), getAD_Reference_Value_ID(), get_TrxName()); if (X_AD_Reference.VALIDATIONTYPE_TableValidation.equals(ref.getValidationType())) { - MRefTable rt = new MRefTable(getCtx(), getAD_Reference_Value_ID(), get_TrxName()); - if (rt != null) - foreignTable = rt.getAD_Table().getTableName(); + int cnt = DB.getSQLValueEx(get_TrxName(), "SELECT COUNT(*) FROM AD_Ref_Table WHERE AD_Reference_ID=?", getAD_Reference_Value_ID()); + if (cnt == 1) { + MRefTable rt = new MRefTable(getCtx(), getAD_Reference_Value_ID(), get_TrxName()); + if (rt != null) + foreignTable = rt.getAD_Table().getTableName(); + } } } else if (DisplayType.List == refid || DisplayType.Payment == refid) { foreignTable = "AD_Ref_List"; diff --git a/org.adempiere.base/src/org/compiere/model/MIndexColumn.java b/org.adempiere.base/src/org/compiere/model/MIndexColumn.java index 9d86b2b2b0..cbc64eb0c3 100644 --- a/org.adempiere.base/src/org/compiere/model/MIndexColumn.java +++ b/org.adempiere.base/src/org/compiere/model/MIndexColumn.java @@ -74,7 +74,7 @@ public class MIndexColumn extends X_AD_IndexColumn { if (sql != null && sql.length() > 0) return sql; int AD_Column_ID = getAD_Column_ID(); - return MColumn.getColumnName(getCtx(), AD_Column_ID); + return MColumn.getColumnName(getCtx(), AD_Column_ID, get_TrxName()); } /** diff --git a/org.adempiere.base/src/org/compiere/model/MSysConfig.java b/org.adempiere.base/src/org/compiere/model/MSysConfig.java index 510205cbb7..79b5764038 100644 --- a/org.adempiere.base/src/org/compiere/model/MSysConfig.java +++ b/org.adempiere.base/src/org/compiere/model/MSysConfig.java @@ -39,10 +39,10 @@ import org.compiere.util.DisplayType; */ public class MSysConfig extends X_AD_SysConfig { - /** + /** * */ - private static final long serialVersionUID = -5124493725187310483L; + private static final long serialVersionUID = 5139119853639605887L; public static final String ADDRESS_VALIDATION = "ADDRESS_VALIDATION"; public static final String ALERT_SEND_ATTACHMENT_AS_XLS = "ALERT_SEND_ATTACHMENT_AS_XLS"; @@ -130,6 +130,7 @@ public class MSysConfig extends X_AD_SysConfig public static final String SYSTEM_IN_MAINTENANCE_MODE = "SYSTEM_IN_MAINTENANCE_MODE"; public static final String SYSTEM_INSERT_CHANGELOG = "SYSTEM_INSERT_CHANGELOG"; public static final String SYSTEM_NATIVE_SEQUENCE = "SYSTEM_NATIVE_SEQUENCE"; + public static final String TWOPACK_COMMIT_DDL = "2PACK_COMMIT_DDL"; public static final String TWOPACK_HANDLE_TRANSLATIONS = "2PACK_HANDLE_TRANSLATIONS"; public static final String USE_EMAIL_FOR_LOGIN = "USE_EMAIL_FOR_LOGIN"; public static final String USER_LOCKING_MAX_ACCOUNT_LOCK_MINUTES = "USER_LOCKING_MAX_ACCOUNT_LOCK_MINUTES"; diff --git a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/ColumnElementHandler.java b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/ColumnElementHandler.java index 89cc6b344a..50bd809429 100644 --- a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/ColumnElementHandler.java +++ b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/ColumnElementHandler.java @@ -36,6 +36,7 @@ import org.adempiere.pipo2.exception.POSaveFailedException; import org.compiere.model.I_AD_Column; import org.compiere.model.I_AD_Table; import org.compiere.model.MColumn; +import org.compiere.model.MSysConfig; import org.compiere.model.MTable; import org.compiere.model.X_AD_Column; import org.compiere.model.X_AD_Element; @@ -260,8 +261,10 @@ public class ColumnElementHandler extends AbstractElementHandler { log.info(sql); //make it consistent for oracle and postgresql - if (!trx.commit()) - return -1; + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + if (!trx.commit()) + return -1; + } if (sql.indexOf(DB.SQLSTATEMENT_SEPARATOR) == -1) { int ret = DB.executeUpdate(sql, false, trx.getTrxName()); @@ -279,7 +282,9 @@ public class ColumnElementHandler extends AbstractElementHandler { } } } - trx.commit(true); + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + trx.commit(true); + } } else { return 0; } diff --git a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableElementHandler.java b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableElementHandler.java index 39f779d536..ba2769a5d2 100644 --- a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableElementHandler.java +++ b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableElementHandler.java @@ -36,6 +36,7 @@ import org.adempiere.pipo2.PoFiller; import org.adempiere.pipo2.exception.DatabaseAccessException; import org.adempiere.pipo2.exception.POSaveFailedException; import org.compiere.model.I_AD_Table; +import org.compiere.model.MSysConfig; import org.compiere.model.MTable; import org.compiere.model.MTableIndex; import org.compiere.model.MViewComponent; @@ -126,12 +127,16 @@ public class TableElementHandler extends AbstractElementHandler { private int validateDatabaseView(PIPOContext ctx, MTable table) { Trx trx = Trx.get(getTrxName(ctx), true); - if (!trx.commit()) - return 0; + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + if (!trx.commit()) + return 0; + } try { DatabaseViewValidate.validateDatabaseView(ctx.ctx, table, trx.getTrxName(), null); - trx.commit(true); + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + trx.commit(true); + } } catch (Exception e) { log.log(Level.SEVERE, e.getLocalizedMessage(), e); trx.rollback(); diff --git a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableIndexElementHandler.java b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableIndexElementHandler.java index 2c353699d5..006725fd66 100644 --- a/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableIndexElementHandler.java +++ b/org.adempiere.pipo.handlers/src/org/adempiere/pipo2/handler/TableIndexElementHandler.java @@ -30,6 +30,7 @@ import org.adempiere.pipo2.exception.DatabaseAccessException; import org.adempiere.pipo2.exception.POSaveFailedException; import org.compiere.model.MIndexColumn; import org.compiere.model.MMessage; +import org.compiere.model.MSysConfig; import org.compiere.model.MTableIndex; import org.compiere.model.X_AD_Package_Imp_Detail; import org.compiere.process.TableIndexValidate; @@ -102,12 +103,16 @@ public class TableIndexElementHandler extends AbstractElementHandler { private int validateTableIndex(PIPOContext ctx, MTableIndex tableIndex) { Trx trx = Trx.get(getTrxName(ctx), true); - if (!trx.commit()) - return 0; + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + if (!trx.commit()) + return 0; + } try { TableIndexValidate.validateTableIndex(ctx.ctx, tableIndex, trx.getTrxName(), null); - trx.commit(true); + if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) { + trx.commit(true); + } } catch (Exception e) { log.log(Level.SEVERE, e.getLocalizedMessage(), e); trx.rollback(); diff --git a/org.adempiere.pipo/src/org/adempiere/pipo2/PackIn.java b/org.adempiere.pipo/src/org/adempiere/pipo2/PackIn.java index ff2c95685c..e9d4294750 100644 --- a/org.adempiere.pipo/src/org/adempiere/pipo2/PackIn.java +++ b/org.adempiere.pipo/src/org/adempiere/pipo2/PackIn.java @@ -37,6 +37,7 @@ import java.util.zip.ZipFile; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; +import org.adempiere.exceptions.AdempiereException; import org.compiere.model.MSysConfig; import org.compiere.model.PO; import org.compiere.model.X_AD_Package_Imp_Detail; @@ -175,10 +176,12 @@ public class PackIn { } msg = "End Parser"; log.info(msg); - if (handler.getUnresolvedCount() > 0) - handler.dumpUnresolvedElements(); msg = "Processed="+handler.getElementsProcessed()+" Un-Resolved="+handler.getUnresolvedCount(); getNotifier().addStatusLine(msg); + if (handler.getUnresolvedCount() > 0) { + handler.dumpUnresolvedElements(); + throw new AdempiereException("Unresolved elements"); + } return msg; } catch (Exception e) { log.log(Level.SEVERE, "importXML:", e); diff --git a/org.adempiere.pipo/src/org/adempiere/pipo2/PackInHandler.java b/org.adempiere.pipo/src/org/adempiere/pipo2/PackInHandler.java index 466bf47061..d75a152d85 100644 --- a/org.adempiere.pipo/src/org/adempiere/pipo2/PackInHandler.java +++ b/org.adempiere.pipo/src/org/adempiere/pipo2/PackInHandler.java @@ -297,8 +297,6 @@ public class PackInHandler extends DefaultHandler { else elementValue = uri + localName; - X_AD_Package_Imp packageImp = new X_AD_Package_Imp(m_ctx.ctx, AD_Package_Imp_ID, null); - packageImp.setProcessed(true); if (elementValue.equals("idempiere")){ processDeferElements(); @@ -314,14 +312,8 @@ public class PackInHandler extends DefaultHandler { } packIn.getNotifier().addStatusLine(packageStatus); - //Update package history log with package status - packageImp.setPK_Status(packageStatus); - packageImp.saveEx(); - - //Update package list with package status - X_AD_Package_Imp_Inst packageInst = new X_AD_Package_Imp_Inst(m_ctx.ctx, AD_Package_Imp_Inst_ID, null); - packageInst.setPK_Status(packageStatus); - packageInst.saveEx(); + updPackageImpNoTrx(); + updPackageImpInstNoTrx(); //reset setupHandlers(); @@ -334,23 +326,34 @@ public class PackInHandler extends DefaultHandler { } catch (RuntimeException re) { packageStatus = "Import Failed"; packIn.getNotifier().addStatusLine(packageStatus); - //Update package history log with package status - packageImp.setPK_Status(packageStatus); - packageImp.saveEx(); + updPackageImpNoTrx(); throw re; } catch (SAXException se) { packageStatus = "Import Failed"; packIn.getNotifier().addStatusLine(packageStatus); - //Update package history log with package status - packageImp.setPK_Status(packageStatus); - packageImp.saveEx(); + updPackageImpNoTrx(); throw se; } } } } // endElement - private void processDeferElements() throws SAXException { + private void updPackageImpNoTrx() { + // NOTE: Updating out of model to avoid change log insert that can cause locks + //Update package history log with package status + DB.executeUpdateEx("UPDATE AD_Package_Imp SET Processed=?, PK_Status=?, UpdatedBy=?, Updated=SYSDATE WHERE AD_Package_Imp_ID=?", + new Object[] {"Y", packageStatus, Env.getAD_User_ID(m_ctx.ctx), AD_Package_Imp_ID}, + null); + } + + private void updPackageImpInstNoTrx() { + // NOTE: Updating out of model to avoid change log insert that can cause locks + DB.executeUpdateEx("UPDATE AD_Package_Imp_Inst SET PK_Status=?, UpdatedBy=?, Updated=SYSDATE WHERE AD_Package_Imp_Inst_ID=?", + new Object[] {packageStatus, Env.getAD_User_ID(m_ctx.ctx), AD_Package_Imp_Inst_ID}, + null); + } + + private void processDeferElements() throws SAXException { if (defer.isEmpty()) return; diff --git a/org.adempiere.plugin.utils/src/org/adempiere/plugin/utils/Incremental2PackActivator.java b/org.adempiere.plugin.utils/src/org/adempiere/plugin/utils/Incremental2PackActivator.java index 7e5754b62d..a168bd9877 100644 --- a/org.adempiere.plugin.utils/src/org/adempiere/plugin/utils/Incremental2PackActivator.java +++ b/org.adempiere.plugin.utils/src/org/adempiere/plugin/utils/Incremental2PackActivator.java @@ -90,7 +90,7 @@ public class Incremental2PackActivator implements BundleActivator, ServiceTracke // e.g. 1.0.0.qualifier, check only the "1.0.0" part String bundleVersionPart = getVersion(); String installedVersionPart = null; - String where = "Name=?"; + String where = "Name=? AND PK_Status = 'Completed successfully'"; Query q = new Query(Env.getCtx(), X_AD_Package_Imp.Table_Name, where.toString(), null); q.setParameters(new Object[] { getName() }); @@ -163,7 +163,10 @@ public class Incremental2PackActivator implements BundleActivator, ServiceTracke }); for(TwoPackEntry entry : list) { - packIn(entry.url); + if (!packIn(entry.url)) { + // stop processing further packages if one fail + break; + } } } @@ -175,7 +178,7 @@ public class Incremental2PackActivator implements BundleActivator, ServiceTracke return v; } - protected void packIn(URL packout) { + protected boolean packIn(URL packout) { if (packout != null && service != null) { String path = packout.getPath(); String suffix = path.substring(path.lastIndexOf("_")); @@ -195,6 +198,7 @@ public class Incremental2PackActivator implements BundleActivator, ServiceTracke service.merge(context, zipfile); } catch (Throwable e) { logger.log(Level.SEVERE, "Pack in failed.", e); + return false; } finally{ if (zipstream != null) { try { @@ -204,6 +208,7 @@ public class Incremental2PackActivator implements BundleActivator, ServiceTracke } System.out.println(getName() + " " + packout.getPath() + " installed"); } + return true; } protected BundleContext getContext() {