Fix foreign key constraint on moddb report_template_acls

classes/ETL/DbModel/ForeignKeyConstraint.php

@@ -188,15 +188,15 @@ public function compare(iEntity $cmp)
         }
 
         if ($this->on_delete != $cmp->on_delete
-            && !in_array($this->on_delete, array(null, 'RESTRICT', 'NO ACTION'))
-            && !in_array($cmp->on_delete, array(null, 'RESTRICT', 'NO ACTION'))
+            && !(in_array($this->on_delete, array(null, 'RESTRICT', 'NO ACTION'))
+                 && in_array($cmp->on_delete, array(null, 'RESTRICT', 'NO ACTION')))
         ) {
             return -2;
         }
 
         if ($this->on_update != $cmp->on_update
-            && !in_array($this->on_update, array(null, 'RESTRICT', 'NO ACTION'))
-            && !in_array($cmp->on_update, array(null, 'RESTRICT', 'NO ACTION'))
+            && !(in_array($this->on_update, array(null, 'RESTRICT', 'NO ACTION'))
+                 && in_array($cmp->on_update, array(null, 'RESTRICT', 'NO ACTION')))
         ) {
             return -3;
         }

classes/ETL/DbModel/Table.php

@@ -735,6 +735,7 @@ public function getAlterSql($destination, $includeSchema = true)
         $alterList = array();
         $changeList = array();
         $triggerList = array();
+        $foreignKeyList = array();
 
         // --------------------------------------------------------------------------------
         // Process columns
@@ -954,8 +955,8 @@ public function getAlterSql($destination, $includeSchema = true)
             if ( 0 == $destForeignKeyConstraint->compare($this->getForeignKeyConstraint($name)) ) {
                 continue;
             }
-            $alterList[] = 'DROP FOREIGN KEY ' . $destForeignKeyConstraint->getName(true);
-            $alterList[] = 'ADD ' . $destForeignKeyConstraint->getSql($includeSchema);
+            $foreignKeyList[] = 'DROP FOREIGN KEY ' . $destForeignKeyConstraint->getName(true);
+            $foreignKeyList[] = 'ADD ' . $destForeignKeyConstraint->getSql($includeSchema);
         }
 
         // --------------------------------------------------------------------------------
@@ -998,7 +999,7 @@ public function getAlterSql($destination, $includeSchema = true)
         // --------------------------------------------------------------------------------
         // Put it all together
 
-        if ( 0 == count($alterList) && 0 == count($changeList) && 0 == count($triggerList) ) {
+        if ( 0 == count($alterList) && 0 == count($changeList) && 0 == count($triggerList) && 0 == count($foreignKeyList) ) {
             return false;
         }
 
@@ -1013,6 +1014,10 @@ public function getAlterSql($destination, $includeSchema = true)
             $sqlList[] = sprintf("ALTER TABLE %s\n%s;", $tableName, implode(",\n", $changeList));
         }
 
+        foreach ($foreignKeyList as $fkey) {
+            $sqlList[] = sprintf("ALTER TABLE %s\n%s;", $tableName, $fkey);
+        }
+
         if ( 0 != count($triggerList) ) {
             foreach ( $triggerList as $trigger ) {
                 $sqlList[] = $trigger;

configuration/etl/etl_tables.d/acls/report_template_acls.json

@@ -52,7 +52,8 @@
         "referenced_table": "acls",
         "referenced_columns": [
           "acl_id"
-        ]
+        ],
+        "on_delete": "CASCADE"
       }
     ]
   }

tests/artifacts/xdmod/etlv2/dbmodel/output/alter_table-charset.sql

@@ -2,9 +2,11 @@ ALTER TABLE `test_db_model`
 CHARSET = utf8mb4,
 COLLATE = utf8mb4_general_ci,
 DROP INDEX `fk_instance`,
-ADD INDEX `fk_instance` USING BTREE (`instance_id`, `inferred`),
-DROP FOREIGN KEY `con_col1`,
-ADD CONSTRAINT `con_col1` FOREIGN KEY (`col1`) REFERENCES `db_test_model2` (`col4`);
+ADD INDEX `fk_instance` USING BTREE (`instance_id`, `inferred`);
 ALTER TABLE `test_db_model`
 CHANGE COLUMN `col1` `col1` varchar(32) CHARSET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT 'mydefault' ,
 CHANGE COLUMN `instance_id` `instance_id` int(11) NULL DEFAULT -1 ;
+ALTER TABLE `test_db_model`
+DROP FOREIGN KEY `con_col1`;
+ALTER TABLE `test_db_model`
+ADD CONSTRAINT `con_col1` FOREIGN KEY (`col1`) REFERENCES `db_test_model2` (`col4`);

tests/artifacts/xdmod/etlv2/dbmodel/output/alter_table_manually-charset.sql

@@ -6,11 +6,13 @@ ADD COLUMN `new_column2` char(64) CHARSET utf8mb4 COLLATE utf8mb4_general_ci NUL
 ADD INDEX `index_new_column` (`new_column`),
 DROP INDEX `fk_instance`,
 ADD INDEX `fk_instance` USING BTREE (`instance_id`, `inferred`),
-ADD CONSTRAINT `fk_new_column` FOREIGN KEY (`new_column`) REFERENCES `other_table` (`other_column`),
-DROP FOREIGN KEY `con_col1`,
-ADD CONSTRAINT `con_col1` FOREIGN KEY (`col1`) REFERENCES `db_test_model2` (`col4`);
+ADD CONSTRAINT `fk_new_column` FOREIGN KEY (`new_column`) REFERENCES `other_table` (`other_column`);
 ALTER TABLE `test_db_model`
 CHANGE COLUMN `col1` `col1` varchar(32) CHARSET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT 'mydefault' ,
 CHANGE COLUMN `instance_id` `instance_id` int(11) NULL DEFAULT -1 ;
+ALTER TABLE `test_db_model`
+DROP FOREIGN KEY `con_col1`;
+ALTER TABLE `test_db_model`
+ADD CONSTRAINT `con_col1` FOREIGN KEY (`col1`) REFERENCES `db_test_model2` (`col4`);
 CREATE TRIGGER `before_ins` BEFORE INSERT ON `jobfact` FOR EACH ROW
  BEGIN DELETE FROM jobfactstatus WHERE job_id = NEW.job_id; END

tests/ci/validate.sh

@@ -38,4 +38,10 @@ then
     exitcode=1
 fi
 
+if ! mysqldump -d moddb report_template_acls | grep -q "ON DELETE CASCADE"
+then
+    echo "Missing/incorrect foreign key constraint on report_template_acls table"
+    exitcode=1
+fi
+
 exit $exitcode

tests/unit/lib/ETL/DbModel/DbModelTest.php

@@ -259,6 +259,38 @@ public function testCreateSql()
 
     }
 
+    /**
+     * Confirm foreign key contraint changes work.
+     */
+    public function testAlterTableContraints()
+    {
+        $config = self::TEST_ARTIFACT_INPUT_PATH . '/table_def-charset.json';
+
+        $origTable = new Table($config, '`', self::$logger);
+        $fkconfig = (object) array(
+            'columns' => array('new_column'),
+            'referenced_table' => 'other_table',
+            'referenced_columns' => array('other_column'),
+            'on_delete' => 'NO ACTION'
+        );
+        $origTable->addForeignKeyConstraint($fkconfig);
+
+        $targetTable = new Table($config, '`', self::$logger);
+        $fkconfig1 = (object) array(
+            'columns' => array('new_column'),
+            'referenced_table' => 'other_table',
+            'referenced_columns' => array('other_column'),
+            'on_delete' => 'CASCADE'
+        );
+        $targetTable->addForeignKeyConstraint($fkconfig1);
+
+        $sql = $origTable->getAlterSql($targetTable);
+
+        $this->assertCount(2, $sql);
+        $this->assertEquals("ALTER TABLE `test_db_model`\nDROP FOREIGN KEY `fk_new_column`;", trim($sql[0]));
+        $this->assertEquals("ALTER TABLE `test_db_model`\nADD CONSTRAINT `fk_new_column` FOREIGN KEY (`new_column`) REFERENCES `other_table` (`other_column`) ON DELETE CASCADE;", trim($sql[1]));
+    }
+
     /**
      * Test comparing 2 tables and the ALTER TABLE statement needed to go from one to the other.
      * Also manually add elements and verify the ALTER TABLE statement generated.