From 73c989ee674224a9549d3bfbd1e34aaaf120ce14 Mon Sep 17 00:00:00 2001 From: Stephen Abello Date: Tue, 9 Jan 2024 09:43:22 +0100 Subject: [PATCH] =?UTF-8?q?N=C2=B07130=20-=20Allows=20to=20ignore=20existi?= =?UTF-8?q?ng=20column=20field=20in=20setup's=20data=20migration=20method?= =?UTF-8?q?=20(#597)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allows to ignore existing column field in setup's data migration method * Update setup/moduleinstaller.class.inc.php * N°7130 - Fix typo in variable extraction * N°7130 - Add unit tests * Update setup/moduleinstaller.class.inc.php --------- Co-authored-by: Molkobain --- setup/moduleinstaller.class.inc.php | 25 ++-- .../setup/ModuleInstallerAPITest.php | 132 ++++++++++++++++++ 2 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php diff --git a/setup/moduleinstaller.class.inc.php b/setup/moduleinstaller.class.inc.php index d47376af3..e8e5b4668 100644 --- a/setup/moduleinstaller.class.inc.php +++ b/setup/moduleinstaller.class.inc.php @@ -242,11 +242,15 @@ abstract class ModuleInstallerAPI * @param $sOrigColumn * @param $sDstTable * @param $sDstColumn + * @param bool $bIgnoreExistingDstColumn * - * @throws \MySQLException * @throws \CoreException + * @throws \MySQLException + * @throws \MySQLHasGoneAwayException + * + * @since 3.2.0 N°7130 Add parameter $bIgnoreExistingDstColumn */ - public static function MoveColumnInDB($sOrigTable, $sOrigColumn, $sDstTable, $sDstColumn) + public static function MoveColumnInDB($sOrigTable, $sOrigColumn, $sDstTable, $sDstColumn, $bIgnoreExistingDstColumn = false) { if (!MetaModel::DBExists(false)) { @@ -259,17 +263,20 @@ abstract class ModuleInstallerAPI // Original field is not present return; } - - if (!CMDBSource::IsTable($sDstTable) || CMDBSource::IsField($sDstTable, $sDstColumn)) + + $bDstTableFieldExists = CMDBSource::IsField($sDstTable, $sDstColumn); + if (!CMDBSource::IsTable($sDstTable) || ($bDstTableFieldExists && !$bIgnoreExistingDstColumn)) { - // Destination field is already created + // Destination field is already created, and we are not ignoring it return; } - // Create the destination field - $sSpec = CMDBSource::GetFieldSpec($sOrigTable, $sOrigColumn); - $sQueryAdd = "ALTER TABLE `{$sDstTable}` ADD `{$sDstColumn}` {$sSpec}"; - CMDBSource::Query($sQueryAdd); + // Create the destination field if necessary + if($bDstTableFieldExists === false){ + $sSpec = CMDBSource::GetFieldSpec($sOrigTable, $sOrigColumn); + $sQueryAdd = "ALTER TABLE `{$sDstTable}` ADD `{$sDstColumn}` {$sSpec}"; + CMDBSource::Query($sQueryAdd); + } // Copy the data $sQueryUpdate = "UPDATE `{$sDstTable}` AS d LEFT JOIN `{$sOrigTable}` AS o ON d.id = o.id SET d.`{$sDstColumn}` = o.`{$sOrigColumn}` WHERE 1"; diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php new file mode 100644 index 000000000..cb27843d9 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php @@ -0,0 +1,132 @@ +RequireOnceItopFile('setup/moduleinstaller.class.inc.php'); + } + + public function tearDown(): void + { + if (CMDBSource::IsTable(static::$sWorkTable)) { + CMDBSource::DropTable(static::$sWorkTable); + } + + parent::tearDown(); + } + + /** + * Test that the new $bIgnoreExistingDstColumn parameter works as expected and doesn't break the previous behavior + * + * @covers \ModuleInstallerAPI::MoveColumnInDB + * @dataProvider MoveColumnInDB_IgnoreExistingDstColumnParamProvider + * + * @param bool $bDstColAlreadyExists + * @param bool $bIgnoreExistingDstColumn + * @param bool $bShouldWork + * + * @return void + * @throws \CoreException + * @throws \MySQLException + * @throws \MySQLHasGoneAwayException + */ + public function testMoveColumnInDB_IgnoreExistingDstColumnParam(bool $bDstColAlreadyExists, bool $bIgnoreExistingDstColumn, bool $bShouldWork): void + { + // Info from the original table + $sOrigClass = "Person"; + $sOrigTable = MetaModel::DBGetTable($sOrigClass); + $sOrigAttCode = "first_name"; + $oOrigAttDef = MetaModel::GetAttributeDef($sOrigClass, $sOrigAttCode); + $sOrigColName = array_key_first($oOrigAttDef->GetSQLColumns()); + + // Info for the destination table + $sDstTable = static::$sWorkTable; + $sDstNonExistingColName = "non_existing_column"; + $sDstExistingColName = "existing_column"; + + // Create work table with an empty $sDstExistingColName column + // Data will then either be moved to the $sDstExistingColName or $sDstNonExistingColName column depending on the test case + CMDBSource::Query( + <<Get($sOrigAttCode); + + // Try to move data + $sDstColName = $bDstColAlreadyExists ? $sDstExistingColName : $sDstNonExistingColName; + ModuleInstallerAPI::MoveColumnInDB($sOrigTable, $sOrigColName, $sDstTable, $sDstColName, $bIgnoreExistingDstColumn); + + // Check if data was actually moved + // - Either way, the column should exist + $sDstValue = CMDBSource::QueryToScalar( + <<assertEquals($sOrigValue, $sDstValue, "Data was not moved as expected"); + } else { + $this->assertEquals(null, $sDstValue, "Data should NOT have moved"); + } + } + + public function MoveColumnInDB_IgnoreExistingDstColumnParamProvider(): array + { + return [ + "Nominal use case, move data to a non-existing column" => [ + "Dest. col. already exists?" => false, + "bIgnoreExistingDstColumn param" => false, + "Should work" => true, + ], + "Move data to non-existing table fails if not explicitly wanted" => [ + "Dest. col. already exists?" => true, + "bIgnoreExistingDstColumn param" => false, + "Should work" => false, + ], + "Move data to non-existing table on purpose" => [ + "Dest. col. already exists?" => true, + "bIgnoreExistingDstColumn param" => true, + "Should work" => true, + ], + ]; + } + +}