From 69995b178b47af0437273d94f993d3af35e490d6 Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Tue, 28 May 2024 16:24:22 +0200 Subject: [PATCH] IDEMPIERE-6150 NON-DB attachments are disappearing when a DB deletion fails (#2377) * IDEMPIERE-6150 NON-DB attachments are disappearing when a DB deletion fails - delete AD_Attachment and AD_Archive after commit of deletion Trx * - add patch from Heng Sin to convert string concatenation to text block --------- Co-authored-by: hengsin --- .../src/org/compiere/model/PO.java | 35 ++++++++++-- .../src/org/compiere/model/PO_Record.java | 55 ++++++++++++------- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/model/PO.java b/org.adempiere.base/src/org/compiere/model/PO.java index 47bda21f69..33834a22b6 100644 --- a/org.adempiere.base/src/org/compiere/model/PO.java +++ b/org.adempiere.base/src/org/compiere/model/PO.java @@ -4078,14 +4078,14 @@ public abstract class PO if (m_KeyColumns != null && m_KeyColumns.length == 1 && !getTable().isUUIDKeyTable()) { //delete cascade only for single key column record PO_Record.deleteModelCascade(p_info.getTableName(), Record_ID, localTrxName); - // Delete Cascade AD_Table_ID/Record_ID (Attachments, ..) - PO_Record.deleteRecordCascade(AD_Table_ID, Record_ID, localTrxName); + // Delete Cascade AD_Table_ID/Record_ID except Attachments/Archive (that's postponed until trx commit) + PO_Record.deleteRecordCascade(AD_Table_ID, Record_ID, "AD_Table.TableName NOT IN ('AD_Attachment','AD_Archive')", localTrxName); // Set referencing Record_ID Null AD_Table_ID/Record_ID PO_Record.setRecordNull(AD_Table_ID, Record_ID, localTrxName); } if (Record_UU != null) { PO_Record.deleteModelCascade(p_info.getTableName(), Record_UU, localTrxName); - PO_Record.deleteRecordCascade(AD_Table_ID, Record_UU, localTrxName); + PO_Record.deleteRecordCascade(AD_Table_ID, Record_UU, "AD_Table.TableName NOT IN ('AD_Attachment','AD_Archive')", localTrxName); PO_Record.setRecordNull(AD_Table_ID, Record_UU, localTrxName); } @@ -4226,9 +4226,10 @@ public abstract class PO } else { - if (CacheMgt.get().hasCache(p_info.getTableName())) { - Trx trxdel = Trx.get(get_TrxName(), false); - if (trxdel != null) { + Trx trxdel = Trx.get(get_TrxName(), false); + if (trxdel != null) { + // Schedule the reset cache for after committed the delete + if (CacheMgt.get().hasCache(p_info.getTableName())) { trxdel.addTrxEventListener(new TrxEventListener() { @Override public void afterRollback(Trx trxdel, boolean success) { @@ -4245,6 +4246,28 @@ public abstract class PO } }); } + // trigger the deletion of attachments and archives for after committed the delete + trxdel.addTrxEventListener(new TrxEventListener() { + @Override + public void afterRollback(Trx trxdel, boolean success) { + trxdel.removeTrxEventListener(this); + } + @Override + public void afterCommit(Trx trxdel, boolean success) { + if (success) { + if (m_KeyColumns != null && m_KeyColumns.length == 1 && !getTable().isUUIDKeyTable()) + // Delete Cascade AD_Table_ID/Record_ID on Attachments/Archive + // after commit because operations on external storage providers don't have rollback + PO_Record.deleteRecordCascade(AD_Table_ID, Record_ID, "AD_Table.TableName IN ('AD_Attachment','AD_Archive')", null); + if (Record_UU != null) + PO_Record.deleteRecordCascade(AD_Table_ID, Record_UU, "AD_Table.TableName IN ('AD_Attachment','AD_Archive')", null); + } + trxdel.removeTrxEventListener(this); + } + @Override + public void afterClose(Trx trxdel) { + } + }); } if (localTrx != null) { diff --git a/org.adempiere.base/src/org/compiere/model/PO_Record.java b/org.adempiere.base/src/org/compiere/model/PO_Record.java index d26daaa888..0e65126c6a 100644 --- a/org.adempiere.base/src/org/compiere/model/PO_Record.java +++ b/org.adempiere.base/src/org/compiere/model/PO_Record.java @@ -28,6 +28,7 @@ import org.compiere.util.DisplayType; import org.compiere.util.Env; import org.compiere.util.KeyNamePair; import org.compiere.util.Msg; +import org.compiere.util.Util; /** * Maintain AD_Table_ID/Record_ID constraint @@ -47,10 +48,11 @@ public class PO_Record * Delete Cascade including (selected)parent relationships * @param AD_Table_ID table * @param Record_IDorUU record ID (int) or UUID (String) + * @param whereTables filter for the Tables * @param trxName transaction * @return false if could not be deleted */ - protected static boolean deleteRecordCascade (int AD_Table_ID, Serializable Record_IDorUU, String trxName) + protected static boolean deleteRecordCascade (int AD_Table_ID, Serializable Record_IDorUU, String whereTables, String trxName) { int refId; String columnName; @@ -63,7 +65,7 @@ public class PO_Record } else { throw new IllegalArgumentException(Record_IDorUU.getClass().getName() + " not supported for ID/UUID"); } - KeyNamePair[] cascades = getTablesWithConstraintType(refId, MColumn.FKCONSTRAINTTYPE_ModelCascade, trxName); + KeyNamePair[] cascades = getTablesWithConstraintType(refId, MColumn.FKCONSTRAINTTYPE_ModelCascade, whereTables, trxName); // Table Loop StringBuilder whereClause = new StringBuilder("AD_Table_ID=? AND ").append(columnName).append("=?"); for (KeyNamePair table : cascades) @@ -146,21 +148,21 @@ public class PO_Record KeyNamePair[] tables = s_po_record_tables_cache.get(key.toString()); if (tables != null) return tables; - final String sql = "" - + "SELECT t.AD_Table_ID, " - + " c.ColumnName " - + "FROM AD_Column c " - + " JOIN AD_Table t ON c.AD_Table_ID = t.AD_Table_ID " - + " LEFT JOIN AD_Ref_Table r ON c.AD_Reference_Value_ID = r.AD_Reference_ID " - + " LEFT JOIN AD_Table tr ON r.AD_Table_ID = tr.AD_Table_ID " - + "WHERE t.IsView = 'N' " - + " AND t.IsActive = 'Y' " - + " AND c.IsActive = 'Y' " - + " AND ( ( c.AD_Reference_ID = ? " - + " AND c.ColumnName = ? || '_ID' ) " - + " OR ( c.AD_Reference_ID IN (? , ?) " - + " AND ( tr.TableName = ? OR ( tr.TableName IS NULL AND c.ColumnName = ? || '_ID' ) ) ) ) " - + " AND c.FKConstraintType = ?"; + final String sql = """ + SELECT t.AD_Table_ID, + c.ColumnName + FROM AD_Column c + JOIN AD_Table t ON c.AD_Table_ID = t.AD_Table_ID + LEFT JOIN AD_Ref_Table r ON c.AD_Reference_Value_ID = r.AD_Reference_ID + LEFT JOIN AD_Table tr ON r.AD_Table_ID = tr.AD_Table_ID + WHERE t.IsView = 'N' + AND t.IsActive = 'Y' + AND c.IsActive = 'Y' + AND ( ( c.AD_Reference_ID = ? + AND c.ColumnName = ? || '_ID' ) + OR ( c.AD_Reference_ID IN (? , ?) + AND ( tr.TableName = ? OR ( tr.TableName IS NULL AND c.ColumnName = ? || '_ID' ) ) ) ) + AND c.FKConstraintType = ?"""; List> dependents = DB.getSQLArrayObjectsEx(trxName, sql, refTableDirId, tableName, refTableId, refTableSearchId, tableName, tableName, MColumn.FKCONSTRAINTTYPE_ModelCascade); if (dependents != null) { @@ -275,6 +277,18 @@ public class PO_Record * @return array of KeyNamePair */ private static KeyNamePair[] getTablesWithConstraintType(int refId, String constraintType, String trxName) { + return getTablesWithConstraintType(refId, constraintType, null, trxName); + } + + /** + * Get array of tables which has a refId column with the defined Constraint Type + * @param refId AD_Reference_ID - Record_ID or Record_UU + * @param constraintType - FKConstraintType of AD_Column + * @param whereTables - optional filter + * @param trxName + * @return array of KeyNamePair + */ + private static KeyNamePair[] getTablesWithConstraintType(int refId, String constraintType, String whereTables, String trxName) { String columnName; if (refId == DisplayType.RecordID) { columnName = "Record_ID"; @@ -284,11 +298,14 @@ public class PO_Record log.warning(refId + " not supported for ID/UUID"); return null; } - StringBuilder key = new StringBuilder(constraintType).append("|").append(refId); + StringBuilder key = new StringBuilder(constraintType).append("|").append(refId).append("|").append(whereTables); KeyNamePair[] tables = s_po_record_tables_cache.get(key.toString()); if (tables != null) return tables; - List listTables = new Query(Env.getCtx(), MTable.Table_Name, "c.AD_Reference_ID=? AND c.FKConstraintType=? AND AD_Table.IsView='N' AND c.ColumnName=?", trxName) + String whereClause = "c.AD_Reference_ID=? AND c.FKConstraintType=? AND AD_Table.IsView='N' AND c.ColumnName=?"; + if (! Util.isEmpty(whereTables)) + whereClause = whereClause + " AND (" + whereTables + ")"; + List listTables = new Query(Env.getCtx(), MTable.Table_Name, whereClause, trxName) .addJoinClause("JOIN AD_Column c ON (c.AD_Table_ID=AD_Table.AD_Table_ID)") .setOnlyActiveRecords(true) .setParameters(refId, constraintType, columnName)