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

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

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

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());
+				int cnt = DB.getSQLValueEx(get_TrxName(), "SELECT COUNT(*) FROM AD_Ref_Table WHERE AD_Reference_ID=?", getAD_Reference_Value_ID());
-				if (rt != null)
+				if (cnt == 1) {
-					foreignTable = rt.getAD_Table().getTableName();
+					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";

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());
 	}
 	
 	/**

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

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())
+				if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) {
-					return -1;
+					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;
 			}

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())
+		if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) {
-			return 0;
+			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();

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())
+		if (MSysConfig.getBooleanValue(MSysConfig.TWOPACK_COMMIT_DDL, false)) {
-			return 0;
+			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();

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

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
+    		updPackageImpNoTrx();
-    		packageImp.setPK_Status(packageStatus);
+    		updPackageImpInstNoTrx();
-    		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();
 
     		//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
+    	    		updPackageImpNoTrx();
-    	    		packageImp.setPK_Status(packageStatus);
-    	    		packageImp.saveEx();
     	    		throw re;
     			} catch (SAXException se) {
     				packageStatus = "Import Failed";
     				packIn.getNotifier().addStatusLine(packageStatus);
-    				//Update package history log with package status
+    	    		updPackageImpNoTrx();
-    	    		packageImp.setPK_Status(packageStatus);
-    	    		packageImp.saveEx();
     	    		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;
 

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