From af2d735cc56152b90da59b9a8ddb813a8f3d573c Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Sat, 6 May 2023 00:58:43 +0200 Subject: [PATCH] IDEMPIERE-5683 Peer review (#1825) --- .../oracle/202304280940_IDEMPIERE-5683.sql | 46 +++- .../202304280940_IDEMPIERE-5683.sql | 46 +++- .../idempiere/process/CleanOrphanCascade.java | 47 ++-- .../src/org/compiere/model/GridTable.java | 2 +- .../src/org/compiere/model/MColumn.java | 20 +- .../src/org/compiere/model/PO.java | 13 +- .../src/org/compiere/model/PO_Record.java | 244 +++++++----------- .../src/org/compiere/model/X_AD_Column.java | 14 +- .../compiere/process/CreateForeignKey.java | 8 +- .../src/org/idempiere/process/MoveClient.java | 2 +- .../test/model/ModelForeignKeyTest.java | 207 +++++++++++++++ 11 files changed, 417 insertions(+), 232 deletions(-) create mode 100644 org.idempiere.test/src/org/idempiere/test/model/ModelForeignKeyTest.java diff --git a/migration/iD11/oracle/202304280940_IDEMPIERE-5683.sql b/migration/iD11/oracle/202304280940_IDEMPIERE-5683.sql index 0c5cd8e859..a81b23df6c 100644 --- a/migration/iD11/oracle/202304280940_IDEMPIERE-5683.sql +++ b/migration/iD11/oracle/202304280940_IDEMPIERE-5683.sql @@ -5,29 +5,49 @@ SET SQLBLANKLINES ON SET DEFINE OFF -- Apr 28, 2023, 9:41:24 AM CEST -UPDATE AD_Field SET DisplayLogic='@AD_Reference_ID@=19 | @AD_Reference_ID@=30 | @AD_Reference_ID@=18 | @AD_Reference_ID@=21 | @AD_Reference_ID@=25 | @AD_Reference_ID@=31 | @AD_Reference_ID@=35 | @AD_Reference_ID@=33 | @AD_Reference_ID@=32 | @AD_Reference_ID@=53370 | @AD_Reference_ID@=200233 | @AD_Reference_ID@=200234 | @AD_Reference_ID@=200235 | @AD_Reference_ID@=200202 | @AD_Reference_ID@=200240',Updated=TO_TIMESTAMP('2023-04-28 09:41:24','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Field_ID=202519 +UPDATE AD_Field SET DisplayLogic='@AD_Reference_ID@=19 | @AD_Reference_ID@=30 | @AD_Reference_ID@=18 | @AD_Reference_ID@=21 | @AD_Reference_ID@=25 | @AD_Reference_ID@=31 | @AD_Reference_ID@=35 | @AD_Reference_ID@=33 | @AD_Reference_ID@=32 | @AD_Reference_ID@=53370 | @AD_Reference_ID@=200233 | @AD_Reference_ID@=200234 | @AD_Reference_ID@=200235 | @AD_Reference_ID@=200202',Updated=TO_TIMESTAMP('2023-04-28 09:41:24','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Field_ID=202519 ; -- update Constraint Type based on java arrays from PO_Record.java UPDATE AD_Column SET FKConstraintType = CASE - WHEN AD_Table_ID IN(254,754,389,200000,200215,200347) THEN 'C' /* AD_Attachment, AD_Archive, AD_Note, AD_RecentItem, AD_PostIt, AD_LabelAssignment - Cascade */ - WHEN AD_Table_ID IN(417,876) THEN 'N' /* R_Request, CM_Chat - No Action */ + WHEN AD_Table_ID IN(254,754,389,200000,200215,200347) THEN 'M' /* AD_Attachment, AD_Archive, AD_Note, AD_RecentItem, AD_PostIt, AD_LabelAssignment - Model Cascade */ + WHEN AD_Table_ID IN(417,876,270) THEN 'O' /* R_Request, CM_Chat, Fact_Acct - No Action - Model No Action Forbid Deletion */ ELSE 'D' END -WHERE AD_Reference_ID IN(200202,200240) /* Record ID, Record UU */ +WHERE AD_Reference_ID = 200202 /* Record ID */ ; --- May 4, 2023, 5:28:36 PM CEST +-- May 5, 2023, 7:57:28 PM CEST +UPDATE AD_Process_Para SET IsActive='N',Updated=TO_TIMESTAMP('2023-05-05 19:57:28','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Process_Para_ID=200269 +; + +-- May 5, 2023, 8:23:32 PM CEST +INSERT INTO AD_Ref_List (AD_Ref_List_ID,Name,AD_Reference_ID,Value,AD_Client_ID,AD_Org_ID,IsActive,Created,CreatedBy,Updated,UpdatedBy,EntityType,AD_Ref_List_UU) VALUES (200640,'Model Set Null',200075,'T',0,0,'Y',TO_TIMESTAMP('2023-05-05 20:23:31','YYYY-MM-DD HH24:MI:SS'),100,TO_TIMESTAMP('2023-05-05 20:23:31','YYYY-MM-DD HH24:MI:SS'),100,'D','6591e7a2-e8f9-4cd4-9ef2-afea1275e578') +; + +-- May 5, 2023, 8:23:59 PM CEST +INSERT INTO AD_Ref_List (AD_Ref_List_ID,Name,AD_Reference_ID,Value,AD_Client_ID,AD_Org_ID,IsActive,Created,CreatedBy,Updated,UpdatedBy,EntityType,AD_Ref_List_UU) VALUES (200641,'Model No Action - Forbid Deletion',200075,'O',0,0,'Y',TO_TIMESTAMP('2023-05-05 20:23:59','YYYY-MM-DD HH24:MI:SS'),100,TO_TIMESTAMP('2023-05-05 20:23:59','YYYY-MM-DD HH24:MI:SS'),100,'D','8300ca2c-50f9-4882-8776-dfdc949db534') +; + +-- May 5, 2023, 8:24:07 PM CEST +UPDATE AD_Ref_List SET Name='No Action - Forbid Deletion',Updated=TO_TIMESTAMP('2023-05-05 20:24:07','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Ref_List_ID=200162 +; + +-- May 5, 2023, 8:24:13 PM CEST +UPDATE AD_Ref_List SET Name='Do Not Create - Ignore',Updated=TO_TIMESTAMP('2023-05-05 20:24:13','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Ref_List_ID=200161 +; + +-- May 5, 2023, 8:57:26 PM CEST UPDATE AD_Val_Rule SET Code='( -(AD_Ref_List.Value!=''M'' OR @AD_Reference_ID@ IN (18,19,30,200233,200234,200235,200202,200240)) -AND -((AD_Ref_List.Value=''C'' AND @AD_Reference_ID@ NOT IN (200202,200240)) OR (AD_Ref_List.Value!=''C'')) -)',Updated=TO_TIMESTAMP('2023-05-04 17:28:36','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Val_Rule_ID=200064 -; - --- May 5, 2023, 8:15:59 AM CEST -DELETE FROM AD_Process_Para WHERE AD_Process_Para_UU='459a7f88-ec79-47cf-9c7b-7429ac565f55' + AD_Ref_List.Value = ''D'' +/* Cascade/SetNull/Forbid supported for all DB constraints */ +OR (AD_Ref_List.Value IN (''C'',''S'',''N'') AND @AD_Reference_ID@ IN (18,19,21,25,30,31,32,33,35,53370,200233,200234,200235)) +/* ModelCascade supported for Table/TableDir/Search/RecordID */ +OR (AD_Ref_List.Value = ''M'' AND @AD_Reference_ID@ IN (18,19,30,200202)) +/* ModelSetNull/ModelForbid supported for RecordID */ +OR (AD_Ref_List.Value IN (''T'',''O'') AND @AD_Reference_ID@ IN (200202)) +)',Updated=TO_TIMESTAMP('2023-05-05 20:57:26','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Val_Rule_ID=200064 ; diff --git a/migration/iD11/postgresql/202304280940_IDEMPIERE-5683.sql b/migration/iD11/postgresql/202304280940_IDEMPIERE-5683.sql index eeeb88cdd4..bc4f54c211 100644 --- a/migration/iD11/postgresql/202304280940_IDEMPIERE-5683.sql +++ b/migration/iD11/postgresql/202304280940_IDEMPIERE-5683.sql @@ -2,29 +2,49 @@ SELECT register_migration_script('202304280940_IDEMPIERE-5683.sql') FROM dual; -- Apr 28, 2023, 9:41:24 AM CEST -UPDATE AD_Field SET DisplayLogic='@AD_Reference_ID@=19 | @AD_Reference_ID@=30 | @AD_Reference_ID@=18 | @AD_Reference_ID@=21 | @AD_Reference_ID@=25 | @AD_Reference_ID@=31 | @AD_Reference_ID@=35 | @AD_Reference_ID@=33 | @AD_Reference_ID@=32 | @AD_Reference_ID@=53370 | @AD_Reference_ID@=200233 | @AD_Reference_ID@=200234 | @AD_Reference_ID@=200235 | @AD_Reference_ID@=200202 | @AD_Reference_ID@=200240',Updated=TO_TIMESTAMP('2023-04-28 09:41:24','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Field_ID=202519 +UPDATE AD_Field SET DisplayLogic='@AD_Reference_ID@=19 | @AD_Reference_ID@=30 | @AD_Reference_ID@=18 | @AD_Reference_ID@=21 | @AD_Reference_ID@=25 | @AD_Reference_ID@=31 | @AD_Reference_ID@=35 | @AD_Reference_ID@=33 | @AD_Reference_ID@=32 | @AD_Reference_ID@=53370 | @AD_Reference_ID@=200233 | @AD_Reference_ID@=200234 | @AD_Reference_ID@=200235 | @AD_Reference_ID@=200202',Updated=TO_TIMESTAMP('2023-04-28 09:41:24','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Field_ID=202519 ; -- update Constraint Type based on java arrays from PO_Record.java UPDATE AD_Column SET FKConstraintType = CASE - WHEN AD_Table_ID IN(254,754,389,200000,200215,200347) THEN 'C' /* AD_Attachment, AD_Archive, AD_Note, AD_RecentItem, AD_PostIt, AD_LabelAssignment - Cascade */ - WHEN AD_Table_ID IN(417,876) THEN 'N' /* R_Request, CM_Chat - No Action */ + WHEN AD_Table_ID IN(254,754,389,200000,200215,200347) THEN 'M' /* AD_Attachment, AD_Archive, AD_Note, AD_RecentItem, AD_PostIt, AD_LabelAssignment - Model Cascade */ + WHEN AD_Table_ID IN(417,876,270) THEN 'O' /* R_Request, CM_Chat, Fact_Acct - No Action - Model No Action Forbid Deletion */ ELSE 'D' END -WHERE AD_Reference_ID IN(200202,200240) /* Record ID, Record UU */ +WHERE AD_Reference_ID = 200202 /* Record ID */ ; --- May 4, 2023, 5:28:36 PM CEST +-- May 5, 2023, 7:57:28 PM CEST +UPDATE AD_Process_Para SET IsActive='N',Updated=TO_TIMESTAMP('2023-05-05 19:57:28','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Process_Para_ID=200269 +; + +-- May 5, 2023, 8:23:32 PM CEST +INSERT INTO AD_Ref_List (AD_Ref_List_ID,Name,AD_Reference_ID,Value,AD_Client_ID,AD_Org_ID,IsActive,Created,CreatedBy,Updated,UpdatedBy,EntityType,AD_Ref_List_UU) VALUES (200640,'Model Set Null',200075,'T',0,0,'Y',TO_TIMESTAMP('2023-05-05 20:23:31','YYYY-MM-DD HH24:MI:SS'),100,TO_TIMESTAMP('2023-05-05 20:23:31','YYYY-MM-DD HH24:MI:SS'),100,'D','6591e7a2-e8f9-4cd4-9ef2-afea1275e578') +; + +-- May 5, 2023, 8:23:59 PM CEST +INSERT INTO AD_Ref_List (AD_Ref_List_ID,Name,AD_Reference_ID,Value,AD_Client_ID,AD_Org_ID,IsActive,Created,CreatedBy,Updated,UpdatedBy,EntityType,AD_Ref_List_UU) VALUES (200641,'Model No Action - Forbid Deletion',200075,'O',0,0,'Y',TO_TIMESTAMP('2023-05-05 20:23:59','YYYY-MM-DD HH24:MI:SS'),100,TO_TIMESTAMP('2023-05-05 20:23:59','YYYY-MM-DD HH24:MI:SS'),100,'D','8300ca2c-50f9-4882-8776-dfdc949db534') +; + +-- May 5, 2023, 8:24:07 PM CEST +UPDATE AD_Ref_List SET Name='No Action - Forbid Deletion',Updated=TO_TIMESTAMP('2023-05-05 20:24:07','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Ref_List_ID=200162 +; + +-- May 5, 2023, 8:24:13 PM CEST +UPDATE AD_Ref_List SET Name='Do Not Create - Ignore',Updated=TO_TIMESTAMP('2023-05-05 20:24:13','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Ref_List_ID=200161 +; + +-- May 5, 2023, 8:57:26 PM CEST UPDATE AD_Val_Rule SET Code='( -(AD_Ref_List.Value!=''M'' OR @AD_Reference_ID@ IN (18,19,30,200233,200234,200235,200202,200240)) -AND -((AD_Ref_List.Value=''C'' AND @AD_Reference_ID@ NOT IN (200202,200240)) OR (AD_Ref_List.Value!=''C'')) -)',Updated=TO_TIMESTAMP('2023-05-04 17:28:36','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Val_Rule_ID=200064 -; - --- May 5, 2023, 8:15:59 AM CEST -DELETE FROM AD_Process_Para WHERE AD_Process_Para_UU='459a7f88-ec79-47cf-9c7b-7429ac565f55' + AD_Ref_List.Value = ''D'' +/* Cascade/SetNull/Forbid supported for all DB constraints */ +OR (AD_Ref_List.Value IN (''C'',''S'',''N'') AND @AD_Reference_ID@ IN (18,19,21,25,30,31,32,33,35,53370,200233,200234,200235)) +/* ModelCascade supported for Table/TableDir/Search/RecordID */ +OR (AD_Ref_List.Value = ''M'' AND @AD_Reference_ID@ IN (18,19,30,200202)) +/* ModelSetNull/ModelForbid supported for RecordID */ +OR (AD_Ref_List.Value IN (''T'',''O'') AND @AD_Reference_ID@ IN (200202)) +)',Updated=TO_TIMESTAMP('2023-05-05 20:57:26','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Val_Rule_ID=200064 ; diff --git a/org.adempiere.base.process/src/org/idempiere/process/CleanOrphanCascade.java b/org.adempiere.base.process/src/org/idempiere/process/CleanOrphanCascade.java index eb05130a7f..3a480b2b5e 100644 --- a/org.adempiere.base.process/src/org/idempiere/process/CleanOrphanCascade.java +++ b/org.adempiere.base.process/src/org/idempiere/process/CleanOrphanCascade.java @@ -26,15 +26,16 @@ package org.idempiere.process; import java.math.BigDecimal; import java.util.List; -import java.util.Objects; import java.util.logging.Level; import org.compiere.model.MColumn; +import org.compiere.model.MProcessPara; import org.compiere.model.MTable; import org.compiere.model.MTree_Base; import org.compiere.model.PO; import org.compiere.model.Query; import org.compiere.model.X_AD_Package_UUID_Map; +import org.compiere.process.ProcessInfoParameter; import org.compiere.process.SvrProcess; import org.compiere.util.DB; import org.compiere.util.Msg; @@ -53,6 +54,10 @@ public class CleanOrphanCascade extends SvrProcess */ protected void prepare() { + for (ProcessInfoParameter para : getParameter()) + { + MProcessPara.validateUnknownParameter(getProcessInfo().getAD_Process_ID(), para); + } } // prepare /** @@ -119,20 +124,20 @@ public class CleanOrphanCascade extends SvrProcess boolean isUUIDMap = X_AD_Package_UUID_Map.Table_Name.equals(tableName); StringBuilder sqlRef = new StringBuilder(); - sqlRef.append("SELECT DISTINCT t.AD_Table_ID, ") - .append(" t.TableName, ") - .append(" c.FKConstraintType, ") - .append(" c.IsMandatory ") - .append("FROM ").append(tableName).append(" r ") - .append(" JOIN AD_Table t ON ( r.AD_Table_ID = t.AD_Table_ID ) ") - .append(" JOIN AD_Column c ON (t.AD_Table_ID = c.AD_Table_ID AND c.ColumnName = 'Record_ID') ") - .append("ORDER BY t.Tablename"); + sqlRef.append("SELECT DISTINCT t.AD_Table_ID, "); + sqlRef.append(" t.TableName, "); + sqlRef.append(" c.FKConstraintType, "); + sqlRef.append(" c.IsMandatory "); + sqlRef.append("FROM ").append(tableName).append(" r "); + sqlRef.append(" JOIN AD_Table t ON ( r.AD_Table_ID = t.AD_Table_ID ) "); + sqlRef.append(" JOIN AD_Column c ON (t.AD_Table_ID = c.AD_Table_ID AND c.ColumnName = 'Record_ID') "); + sqlRef.append("ORDER BY t.Tablename"); List> rowTables = DB.getSQLArrayObjectsEx(get_TrxName(), sqlRef.toString()); if (rowTables != null) { for (List row : rowTables) { int refTableID = ((BigDecimal) row.get(0)).intValue(); String refTableName = row.get(1).toString(); - String constraintType = Objects.toString(row.get(2), ""); + String constraintType = row.get(2).toString(); Boolean isMandatory = row.get(3).toString().equalsIgnoreCase("Y"); MTable refTable = MTable.get(getCtx(), refTableID); @@ -157,23 +162,24 @@ public class CleanOrphanCascade extends SvrProcess } } - int noDel = 0; + int noDeleted = 0; + int noSetNull = 0; List poList = new Query(getCtx(), tableName, whereClause.toString(), get_TrxName()).list(); for (PO po : poList) { - if(MColumn.FKCONSTRAINTTYPE_ModelCascade.equals(constraintType)) { + if (MColumn.FKCONSTRAINTTYPE_ModelCascade.equals(constraintType)) { po.deleteEx(true, get_TrxName()); - } - else if(MColumn.FKCONSTRAINTTYPE_SetNull.equals(constraintType)) { - if(isMandatory) + noDeleted++; + } else if (MColumn.FKCONSTRAINTTYPE_ModelSetNull.equals(constraintType)) { + if (isMandatory) po.set_ValueOfColumn("Record_ID", 0); else po.set_ValueOfColumn("Record_ID", null); po.saveEx(get_TrxName()); - } - noDel++; + noSetNull++; + } } - if (noDel > 0) { - addLog(Msg.parseTranslation(getCtx(), noDel + " " + tableName + " " + "@Deleted@ -> " + refTableName)); + if (noDeleted > 0 || noSetNull > 0) { + addLog(Msg.parseTranslation(getCtx(), tableName + ": " + noDeleted + " @Deleted@ / " + noSetNull + " @Reset@ -> " + refTableName)); } } @@ -195,8 +201,7 @@ public class CleanOrphanCascade extends SvrProcess noDel++; } if (noDel > 0) { - addLog(Msg.parseTranslation(getCtx(), noDel + " " + treeTable + " " + "@Deleted@ -> " + foreignTable - + (treeId > 0 ? " Tree=" + treeId: "" ))); + addLog(Msg.parseTranslation(getCtx(), treeTable + ": " + noDel + " @Deleted@ -> " + foreignTable + (treeId > 0 ? " Tree=" + treeId: "" ))); } } // delTree diff --git a/org.adempiere.base/src/org/compiere/model/GridTable.java b/org.adempiere.base/src/org/compiere/model/GridTable.java index c391897929..7259023577 100644 --- a/org.adempiere.base/src/org/compiere/model/GridTable.java +++ b/org.adempiere.base/src/org/compiere/model/GridTable.java @@ -2844,7 +2844,7 @@ public class GridTable extends AbstractTableModel if (!ok) { ValueNamePair vp = CLogger.retrieveError(); - if (vp != null) + if (vp != null && !(Util.isEmpty(vp.getValue()) || Util.isEmpty(vp.getName()))) fireDataStatusEEvent(vp); else fireDataStatusEEvent("DeleteError", "", true); diff --git a/org.adempiere.base/src/org/compiere/model/MColumn.java b/org.adempiere.base/src/org/compiere/model/MColumn.java index f61e069745..7c74f9f15a 100644 --- a/org.adempiere.base/src/org/compiere/model/MColumn.java +++ b/org.adempiere.base/src/org/compiere/model/MColumn.java @@ -1014,7 +1014,7 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport else if (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeySetNull) dbDeleteRule = MColumn.FKCONSTRAINTTYPE_SetNull; else if (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyNoAction || dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyRestrict) - dbDeleteRule = MColumn.FKCONSTRAINTTYPE_NoAction; + dbDeleteRule = MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion; String fkConstraintType = column.getFKConstraintType(); if (fkConstraintType == null) { fkConstraintType = dbDeleteRule; @@ -1024,12 +1024,12 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport || "CreatedBy".equals(column.getColumnName()) || "UpdatedBy".equals(column.getColumnName()) ) - fkConstraintType = MColumn.FKCONSTRAINTTYPE_DoNotCreate; + fkConstraintType = MColumn.FKCONSTRAINTTYPE_DoNotCreate_Ignore; else - fkConstraintType = MColumn.FKCONSTRAINTTYPE_NoAction; + fkConstraintType = MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion; } } - if (!fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_DoNotCreate)) + if (!fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_DoNotCreate_Ignore)) { String fkConstraintName = column.getFKConstraintName(); if (fkConstraintName == null || fkConstraintName.trim().length() == 0) @@ -1051,7 +1051,7 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport } fkConstraint.append(")"); - if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_NoAction)) + if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion)) ; else if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_Cascade)) fkConstraint.append(" ON DELETE CASCADE"); @@ -1073,8 +1073,8 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport if ( dbForeignKey.getKeyName().equalsIgnoreCase(column.getFKConstraintName()) && ( (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyCascade && MColumn.FKCONSTRAINTTYPE_Cascade.equals(column.getFKConstraintType())) || (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeySetNull && MColumn.FKCONSTRAINTTYPE_SetNull.equals(column.getFKConstraintType())) - || (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyNoAction && MColumn.FKCONSTRAINTTYPE_NoAction.equals(column.getFKConstraintType())) - || (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyRestrict && MColumn.FKCONSTRAINTTYPE_NoAction.equals(column.getFKConstraintType())) + || (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyNoAction && MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion.equals(column.getFKConstraintType())) + || (dbForeignKey.getDeleteRule() == DatabaseMetaData.importedKeyRestrict && MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion.equals(column.getFKConstraintType())) ) ) { // nothing changed @@ -1149,9 +1149,9 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport { String fkConstraintType = column.getFKConstraintType(); if (fkConstraintType == null) - fkConstraintType = MColumn.FKCONSTRAINTTYPE_NoAction; + fkConstraintType = MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion; - if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_DoNotCreate)) + if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_DoNotCreate_Ignore)) return ""; int refid = column.getAD_Reference_ID(); @@ -1202,7 +1202,7 @@ public class MColumn extends X_AD_Column implements ImmutablePOSupport } fkConstraint.append(")"); - if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_NoAction)) + if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion)) ; else if (fkConstraintType.equals(MColumn.FKCONSTRAINTTYPE_Cascade)) fkConstraint.append(" ON DELETE CASCADE"); diff --git a/org.adempiere.base/src/org/compiere/model/PO.java b/org.adempiere.base/src/org/compiere/model/PO.java index 50c6806c66..1f10b70dbb 100644 --- a/org.adempiere.base/src/org/compiere/model/PO.java +++ b/org.adempiere.base/src/org/compiere/model/PO.java @@ -3965,17 +3965,16 @@ public abstract class PO if (get_ColumnIndex("IsSummary") >= 0) { delete_Tree(MTree_Base.TREETYPE_CustomTable); } - // Delete Cascade AD_Table_ID/Record_ID (Attachments, ..) - PO_Record.deleteCascade(AD_Table_ID, Record_ID, localTrxName); - //delete cascade only for single key column record if (m_KeyColumns != null && m_KeyColumns.length == 1) { + //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.deleteRecordIdCascade(AD_Table_ID, Record_ID, localTrxName); + // Set referencing Record_ID Null AD_Table_ID/Record_ID + PO_Record.setRecordIdNull(AD_Table_ID, Record_ID, localTrxName); } - - // Set referencing Record_ID Null AD_Table_ID/Record_ID - PO_Record.setRecordIdNull(AD_Table_ID, Record_ID, localTrxName); - + // The Delete Statement String where = isLogSQLScript() ? get_WhereClause(true, get_ValueAsString(getUUIDColumnName())) : get_WhereClause(true); List optimisticLockingParams = new ArrayList(); 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 242a29513f..9384058d67 100644 --- a/org.adempiere.base/src/org/compiere/model/PO_Record.java +++ b/org.adempiere.base/src/org/compiere/model/PO_Record.java @@ -16,9 +16,7 @@ *****************************************************************************/ package org.compiere.model; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.util.ArrayList; +import java.math.BigDecimal; import java.util.List; import java.util.logging.Level; @@ -28,6 +26,7 @@ import org.compiere.util.DB; import org.compiere.util.DisplayType; import org.compiere.util.Env; import org.compiere.util.KeyNamePair; +import org.compiere.util.Msg; /** * Maintain AD_Table_ID/Record_ID constraint @@ -38,8 +37,8 @@ import org.compiere.util.KeyNamePair; public class PO_Record { /* Cache for arrays of KeyNamePair for types of deletion: Cascade, Set Null, No Action */ - private static final CCache s_po_record_tables_cache = new CCache<>(null, "PORecordTables", 3, 120, false, 3); - + private static final CCache s_po_record_tables_cache = new CCache<>(null, "PORecordTables", 100, 120, false); + /** Parent Tables */ private static int[] s_parents = new int[]{ X_C_Order.Table_ID @@ -64,28 +63,27 @@ public class PO_Record * @param trxName transaction * @return false if could not be deleted */ - static boolean deleteCascade (int AD_Table_ID, int Record_ID, String trxName) + static boolean deleteRecordIdCascade (int AD_Table_ID, int Record_ID, String trxName) { - KeyNamePair[] cascades = getTablesWithRecordColumnFromCache(MColumn.FKCONSTRAINTTYPE_Cascade); + KeyNamePair[] cascades = getTablesWithRecordIDColumn(MColumn.FKCONSTRAINTTYPE_ModelCascade, trxName); // Table Loop for (KeyNamePair table : cascades) { // DELETE FROM table WHERE AD_Table_ID=#1 AND Record_ID=#2 - if (table.getKey() != AD_Table_ID) + List poList = new Query(Env.getCtx(), table.getName(), "AD_Table_ID=? AND Record_ID=?", trxName) + .setParameters(AD_Table_ID, Record_ID) + .list(); + + int count = 0; + for(PO po : poList) { - List poList = new Query(Env.getCtx(), table.getName(), "AD_Table_ID=? AND Record_ID=?", trxName) - .setParameters(AD_Table_ID, Record_ID) - .list(); - - int count = 0; - for(PO po : poList) - { - po.deleteEx(true); - count++; - } - if (count > 0) - if (log.isLoggable(Level.CONFIG)) log.config(table.getName() + " (" + AD_Table_ID + "/" + Record_ID + ") #" + count); + if (po.get_Table_ID() == AD_Table_ID && po.get_ID() == Record_ID) + continue; + po.deleteEx(true); + count++; } + if (count > 0) + if (log.isLoggable(Level.CONFIG)) log.config(table.getName() + " (" + AD_Table_ID + "/" + Record_ID + ") #" + count); } // Parent Loop for (int i = 0; i < s_parents.length; i++) @@ -120,10 +118,39 @@ public class PO_Record } // deleteCascade //IDEMPIERE-2060 + /** + * @param tableName + * @param Record_ID + * @param trxName + */ public static void deleteModelCascade(String tableName, int Record_ID, String trxName) { - //find dependent tables to delete cascade + KeyNamePair[] tables = getTablesWithModelCascade(tableName, trxName); + for (KeyNamePair table : tables) { + int dependentTableId = table.getKey(); + String dependentColumnName = table.getName(); + String dependentWhere = dependentColumnName + "=?"; + List poList = new Query(Env.getCtx(), + MTable.get(dependentTableId).getTableName(), + dependentWhere, + trxName).setParameters(Record_ID).list(); + for (PO po : poList) { + po.deleteEx(true, trxName); + } + } + } + + /** + * @param tableName + * @param trxName + * @return + */ + private static KeyNamePair[] getTablesWithModelCascade(String tableName, String trxName) { + StringBuilder key = new StringBuilder(MColumn.FKCONSTRAINTTYPE_ModelCascade).append("|").append(tableName); + KeyNamePair[] tables = s_po_record_tables_cache.get(key.toString()); + if (tables != null) + return tables; final String sql = "" - + "SELECT t.TableName, " + + "SELECT t.AD_Table_ID, " + " c.ColumnName " + "FROM AD_Column c " + " JOIN AD_Table t ON c.AD_Table_ID = t.AD_Table_ID " @@ -137,24 +164,23 @@ public class PO_Record + " OR ( c.AD_Reference_ID IN ( " + DisplayType.Table + ", " + DisplayType.Search + " ) " + " AND ( tr.TableName = ? OR ( tr.TableName IS NULL AND c.ColumnName = ? || '_ID' ) ) ) ) " + " AND c.FKConstraintType = '" + MColumn.FKCONSTRAINTTYPE_ModelCascade + "' "; - List> dependents = DB.getSQLArrayObjectsEx(trxName, sql, tableName, tableName, tableName); if (dependents != null) { - for (List row : dependents) { - String dependentTableName = (String) row.get(0); + tables = new KeyNamePair[dependents.size()]; + for (int i=0; i row = dependents.get(i); + int dependentTableId = ((BigDecimal)row.get(0)).intValue(); String dependentColumnName = (String) row.get(1); - String dependentWhere = dependentColumnName + "=?"; - List poList = new Query(Env.getCtx(), - dependentTableName, - dependentWhere, - trxName).setParameters(Record_ID).list(); - for (PO po : poList) { - po.deleteEx(true, trxName); - } + KeyNamePair vnp = new KeyNamePair(dependentTableId, dependentColumnName); + tables[i] = vnp; } + } else { + tables = new KeyNamePair[0]; } + s_po_record_tables_cache.put(key.toString(), tables); + return tables; } - + /** * If a referencing Record ID or Record UU exists to the deleted record, set it to NULL * @param AD_Table_ID @@ -162,19 +188,18 @@ public class PO_Record * @param trxName */ public static void setRecordIdNull(int AD_Table_ID, int Record_ID, String trxName){ - KeyNamePair[] tables = getTablesWithRecordColumnFromCache(MColumn.FKCONSTRAINTTYPE_SetNull); + KeyNamePair[] tables = getTablesWithRecordIDColumn(MColumn.FKCONSTRAINTTYPE_ModelSetNull, trxName); // Table loop for (KeyNamePair table : tables) { - if(table.getKey() == AD_Table_ID) - continue; - List poList = new Query(Env.getCtx(), table.getName(), " AD_Table_ID = ? AND Record_ID = ? ", trxName) .setParameters(AD_Table_ID, Record_ID) .list(); int count = 0; for(PO po : poList) { - if(po.isColumnMandatory(po.get_ColumnIndex("Record_ID"))) + if (po.get_Table_ID() == AD_Table_ID && po.get_ID() == Record_ID) + continue; + if (po.isColumnMandatory(po.get_ColumnIndex("Record_ID"))) po.set_Value("Record_ID", 0); else po.set_Value("Record_ID", null); @@ -196,139 +221,44 @@ public class PO_Record */ static String exists (int AD_Table_ID, int Record_ID, String trxName) { - KeyNamePair[] restricts = getTablesWithRecordColumnFromCache(MColumn.FKCONSTRAINTTYPE_NoAction); + KeyNamePair[] restricts = getTablesWithRecordIDColumn(MColumn.FKCONSTRAINTTYPE_ModelNoAction_ForbidDeletion, trxName); // Table Loop only for (int i = 0; i < restricts.length; i++) { - // SELECT COUNT(*) FROM table WHERE AD_Table_ID=#1 AND Record_ID=#2 - StringBuilder sql = new StringBuilder ("SELECT COUNT(*) FROM ") + // SELECT 1 FROM table WHERE AD_Table_ID=#1 AND Record_ID=#2 FETCH FIRST 1 ROWS ONLY + StringBuilder sqlb = new StringBuilder ("SELECT 1 FROM ") .append(restricts[i].getName()) .append(" WHERE AD_Table_ID=? AND Record_ID=?"); - int no = DB.getSQLValue(trxName, sql.toString(), AD_Table_ID, Record_ID); - if (no > 0) - return restricts[i].getName(); + String sql = DB.getDatabase().addPagingSQL(sqlb.toString(), 1, 1); + int no = DB.getSQLValueEx(trxName, sql.toString(), AD_Table_ID, Record_ID); + if (no == 1) + return Msg.getMsg(Env.getCtx(), "DeleteErrorDependent") + " -> " + restricts[i].getName(); } return null; } // exists /** - * Validate all tables for AD_Table/Record_ID relationships - */ - static void validate () - { - String sql = "SELECT AD_Table_ID, TableName FROM AD_Table WHERE IsView='N' ORDER BY TableName"; - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - rs = pstmt.executeQuery (); - while (rs.next ()) - { - validate (rs.getInt(1), rs.getString(2)); - } - } - catch (Exception e) - { - log.log (Level.SEVERE, sql, e); - } - finally { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } - } // validate - - /** - * Validate all tables for AD_Table/Record_ID relationships - * @param AD_Table_ID table - */ - static void validate (int AD_Table_ID) - { - MTable table = new MTable(Env.getCtx(), AD_Table_ID, null); - if (table.isView()) - log.warning("Ignored - View " + table.getTableName()); - else - validate (table.getAD_Table_ID(), table.getTableName()); - } // validate - - /** - * Validate Table for Table/Record - * @param AD_Table_ID table - * @param TableName Name - */ - static private void validate (int AD_Table_ID, String TableName) - { - KeyNamePair[] cascades = getTablesWithRecordColumnFromCache(MColumn.FKCONSTRAINTTYPE_Cascade); - for (int i = 0; i < cascades.length; i++) - { - StringBuilder sql = new StringBuilder ("DELETE FROM ") - .append(cascades[i].getName()) - .append(" WHERE AD_Table_ID=").append(AD_Table_ID) - .append(" AND Record_ID NOT IN (SELECT ") - .append(TableName).append("_ID FROM ").append(TableName).append(")"); - int no = DB.executeUpdate(sql.toString(), null); - if (no > 0) - if (log.isLoggable(Level.CONFIG)) log.config(cascades[i].getName() + " (" + AD_Table_ID + "/" + TableName - + ") Invalid #" + no); - } - } // validate - - /** - * Get array of tables which has a Record_ID or Record_UU column with the defined Constraint Type from cache + * Get array of tables which has a Record_ID column with the defined Constraint Type * @param constraintType - FKConstraintType of AD_Column - * @return array of KeyNamePair + * @param trxName + * @return array of KeyNamePair */ - static private KeyNamePair[] getTablesWithRecordColumnFromCache(String constraintType) { + private static KeyNamePair[] getTablesWithRecordIDColumn(String constraintType, String trxName) { KeyNamePair[] tables = s_po_record_tables_cache.get(constraintType); - if(tables != null) + if (tables != null) return tables; - tables = getTablesWithRecordColumn(constraintType); + List listTables = new Query(Env.getCtx(), MTable.Table_Name, "c.AD_Reference_ID=? AND c.FKConstraintType=?", trxName) + .addJoinClause("JOIN AD_Column c ON (c.AD_Table_ID=AD_Table.AD_Table_ID)") + .setOnlyActiveRecords(true) + .setParameters(DisplayType.RecordID, constraintType) + .list(); + tables = new KeyNamePair[listTables.size()]; + for (int i=0; i - */ - static private KeyNamePair[] getTablesWithRecordColumn(String constraintType) { - ArrayList tables = new ArrayList(); - - String sql = " SELECT t.AD_Table_ID, TableName " - + " FROM AD_Column c " - + " JOIN AD_Table t ON (c.AD_Table_ID = t.AD_Table_ID) " - + " WHERE c.ColumnName IN (?,?) AND c.FKConstraintType = ? "; - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - int idx = 1; - pstmt.setString(idx++, "Record_ID"); - pstmt.setString(idx++, "Record_UU"); - pstmt.setString(idx++, constraintType); - rs = pstmt.executeQuery (); - while (rs.next ()) - { - tables.add(new KeyNamePair(rs.getInt(1), rs.getString(2))); - } - } - catch (Exception e) - { - log.log (Level.SEVERE, sql, e); - } - finally { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } - - KeyNamePair[] tablesArray = new KeyNamePair[tables.size()]; - for(int i=0; i 0) { statusUpdate("Validating orphans for " + table.getTableName() + "." + columnName); diff --git a/org.idempiere.test/src/org/idempiere/test/model/ModelForeignKeyTest.java b/org.idempiere.test/src/org/idempiere/test/model/ModelForeignKeyTest.java new file mode 100644 index 0000000000..2429cef56e --- /dev/null +++ b/org.idempiere.test/src/org/idempiere/test/model/ModelForeignKeyTest.java @@ -0,0 +1,207 @@ +/*********************************************************************** + * This file is part of iDempiere ERP Open Source * + * http://www.idempiere.org * + * * + * Copyright (C) Contributors * + * * + * This program is free software; you can redistribute it and/or * + * modify it under the terms of the GNU General Public License * + * as published by the Free Software Foundation; either version 2 * + * of the License, or (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, * + * MA 02110-1301, USA. * + * * + * Contributors: * + * - Carlos Ruiz - globalqss - bxservice * + **********************************************************************/ + +package org.idempiere.test.model; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Properties; + +import org.adempiere.exceptions.AdempiereException; +import org.compiere.model.MColumn; +import org.compiere.model.MStyle; +import org.compiere.model.MStyleLine; +import org.compiere.model.MTest; +import org.compiere.model.PO; +import org.compiere.util.CacheMgt; +import org.compiere.util.Env; +import org.compiere.util.Trx; +import org.idempiere.test.AbstractTestCase; +import org.junit.jupiter.api.Test; + +/** + * + * @author Carlos Ruiz - globalqss - bxservice + * + */ +public class ModelForeignKeyTest extends AbstractTestCase { + + public ModelForeignKeyTest() { + } + + @Test + public void testModelCascadeRecordId() { + Properties ctx = Env.getCtx(); + String trxName = getTrxName(); + + // set model cascade on Test.Record_ID + MColumn col_test_record_id = new MColumn(ctx, MColumn.getColumn_ID(MTest.Table_Name, MTest.COLUMNNAME_Record_ID), trxName); + col_test_record_id.setFKConstraintType(MColumn.FKCONSTRAINTTYPE_ModelCascade); + try { + PO.setCrossTenantSafe(); + col_test_record_id.saveEx(); + } finally { + PO.clearCrossTenantSafe(); + } + CacheMgt.get().reset(); + + MTest test1 = new MTest(ctx, 0, trxName); + test1.setName("Test 1"); + test1.saveEx(); + MTest test2 = new MTest(ctx, 0, trxName); + test2.setName("Test 2"); + test2.saveEx(); + test1.setAD_Table_ID(MTest.Table_ID); + test1.setRecord_ID(test2.getTest_ID()); + test1.saveEx(); + + test2.deleteEx(true); + test1.load(trxName); + assertTrue(test1.get_ID() == 0); + } + + @Test + public void testModelSetNullRecordId() { + Properties ctx = Env.getCtx(); + String trxName = getTrxName(); + + // set model cascade on Test.Record_ID + MColumn col_test_record_id = new MColumn(ctx, MColumn.getColumn_ID(MTest.Table_Name, MTest.COLUMNNAME_Record_ID), trxName); + col_test_record_id.setFKConstraintType(MColumn.FKCONSTRAINTTYPE_ModelSetNull); + try { + PO.setCrossTenantSafe(); + col_test_record_id.saveEx(); + } finally { + PO.clearCrossTenantSafe(); + } + CacheMgt.get().reset(); + + MTest test1 = new MTest(ctx, 0, trxName); + test1.setName("Test 1"); + test1.saveEx(); + MTest test2 = new MTest(ctx, 0, trxName); + test2.setName("Test 2"); + test2.saveEx(); + test1.setAD_Table_ID(MTest.Table_ID); + test1.setRecord_ID(test2.getTest_ID()); + test1.saveEx(); + + test2.deleteEx(true); + test1.load(trxName); + assertTrue(test1.get_ID() > 0); + assertTrue(test1.getRecord_ID() == 0); + } + + @Test + public void testModelNoActionRecordId() { + Properties ctx = Env.getCtx(); + String trxName = getTrxName(); + + // set model cascade on Test.Record_ID + MColumn col_test_record_id = new MColumn(ctx, MColumn.getColumn_ID(MTest.Table_Name, MTest.COLUMNNAME_Record_ID), trxName); + col_test_record_id.setFKConstraintType(MColumn.FKCONSTRAINTTYPE_ModelNoAction_ForbidDeletion); + try { + PO.setCrossTenantSafe(); + col_test_record_id.saveEx(); + } finally { + PO.clearCrossTenantSafe(); + } + CacheMgt.get().reset(); + + MTest test1 = new MTest(ctx, 0, trxName); + test1.setName("Test 1"); + test1.saveEx(); + MTest test2 = new MTest(ctx, 0, trxName); + test2.setName("Test 2"); + test2.saveEx(); + test1.setAD_Table_ID(MTest.Table_ID); + test1.setRecord_ID(test2.getTest_ID()); + test1.saveEx(); + + assertThrows(AdempiereException.class, () -> { + test2.deleteEx(true); + }); + } + + @Test + public void testDBNoAction() { + Properties ctx = Env.getCtx(); + String trxName = getTrxName(); + + // set model cascade on Test.Record_ID + CacheMgt.get().reset(); + MColumn col_styleline_style = new MColumn(ctx, MColumn.getColumn_ID(MStyleLine.Table_Name, MStyleLine.COLUMNNAME_AD_Style_ID), trxName); + // this is the default already set in database + assertTrue(MColumn.FKCONSTRAINTTYPE_NoAction_ForbidDeletion.equals(col_styleline_style.getFKConstraintType())); + + MStyle style = new MStyle(ctx, 0, trxName); + style.setName("Test"); + style.saveEx(); + MStyleLine styleLine = new MStyleLine(ctx, 0, trxName); + styleLine.setAD_Style_ID(style.getAD_Style_ID()); + styleLine.setLine(10); + styleLine.setInlineStyle("Test"); + styleLine.saveEx(); + style.deleteEx(true); + + assertThrows( + Exception.class, + () -> Trx.get(trxName, false).commit(true), + "Expected Trx.commit to throw Exception, but it didn't" + ); + } + + @Test + public void testModelNoAction() { + Properties ctx = Env.getCtx(); + String trxName = getTrxName(); + + // set model cascade on Test.Record_ID + MColumn col_styleline_style = new MColumn(ctx, MColumn.getColumn_ID(MStyleLine.Table_Name, MStyleLine.COLUMNNAME_AD_Style_ID), trxName); + col_styleline_style.setFKConstraintType(MColumn.FKCONSTRAINTTYPE_ModelCascade); + try { + PO.setCrossTenantSafe(); + col_styleline_style.saveEx(); + } finally { + PO.clearCrossTenantSafe(); + } + CacheMgt.get().reset(); + + MStyle style = new MStyle(ctx, 0, trxName); + style.setName("Test"); + style.saveEx(); + MStyleLine styleLine = new MStyleLine(ctx, 0, trxName); + styleLine.setAD_Style_ID(style.getAD_Style_ID()); + styleLine.setLine(10); + styleLine.setInlineStyle("Test"); + styleLine.saveEx(); + + style.deleteEx(true); + styleLine.load(trxName); + assertTrue(styleLine.get_ID() == 0); + } + +}