From 67ab3e53210fd6e28dd21ec271b9a3783596645e Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 10 Oct 2025 20:30:18 +0300 Subject: [PATCH] Improvements.Fixed org identities transmogrify. --- .../Transmogrify/config/schema/tables.json | 16 +++--- .../src/Command/TransmogrifyCommand.php | 7 ++- .../src/Lib/Traits/CacheTrait.php | 1 + .../src/Lib/Traits/HookRunnersTrait.php | 6 +++ .../src/Lib/Traits/RowTransformationTrait.php | 45 ++++++++++++++--- .../src/Lib/Traits/TypeMapperTrait.php | 49 +++++++++---------- .../src/Lib/Util/RawSqlQueries.php | 25 +++++----- 7 files changed, 93 insertions(+), 56 deletions(-) diff --git a/app/plugins/Transmogrify/config/schema/tables.json b/app/plugins/Transmogrify/config/schema/tables.json index 656737920..4e84bef37 100644 --- a/app/plugins/Transmogrify/config/schema/tables.json +++ b/app/plugins/Transmogrify/config/schema/tables.json @@ -128,6 +128,8 @@ "external_identities": { "source": "cm_org_identities", "displayField": "id", + "cache": ["person_id"], + "postRow": "mapExternalIdentityToExternalIdentityRole", "fieldMap": { "co_id": null, "person_id": "&mapOrgIdentitycoPersonId", @@ -140,9 +142,7 @@ "sponsor_identifier": null, "valid_from": null, "valid_through": null - }, - "postRow": "mapExternalIdentityToExternalIdentityRole", - "cache": ["person_id"] + } }, "groups": { "source": "cm_co_groups", @@ -152,7 +152,8 @@ "fieldMap": { "auto": null, "co_group_id": "group_id", - "group_type": "?S" + "group_type": "?S", + "introduction": null }, "postTable": "createOwnersGroups" }, @@ -310,12 +311,7 @@ "source": "cm_servers", "displayField": "description", "addChangelog": false, - "cache": ["status"], - "sqlSelect": null, - "preTable": null, - "postTable": null, - "preRow": null, - "postRow": null, + "cache": ["co_id"], "fieldMap": { "plugin": "&mapServerTypeToPlugin" } diff --git a/app/plugins/Transmogrify/src/Command/TransmogrifyCommand.php b/app/plugins/Transmogrify/src/Command/TransmogrifyCommand.php index 591f855b0..0793576ea 100644 --- a/app/plugins/Transmogrify/src/Command/TransmogrifyCommand.php +++ b/app/plugins/Transmogrify/src/Command/TransmogrifyCommand.php @@ -332,16 +332,19 @@ public function execute(Arguments $args, ConsoleIo $io): int // not linked to a CO Person that was not migrated. $warns++; $progress->warn("Skipping $t record " . $row['id'] . " due to invalid foreign key: " . $e->getMessage()); + $io->ask('Press to continue...'); } catch(\InvalidArgumentException $e) { // If we can't find a value for mapping we skip the record // (ie: mapLegacyFieldNames basically requires a successful mapping) $warns++; $progress->warn("Skipping $t record " . $row['id'] . ": " . $e->getMessage()); + $io->ask('Press to continue...'); } catch(\Exception $e) { $err++; $progress->error("$t record " . $row['id'] . ": " . $e->getMessage()); + $io->ask('Press to continue...'); } $tally++; @@ -357,7 +360,7 @@ public function execute(Arguments $args, ConsoleIo $io): int $io->error(sprintf('Errors: %d', $err)); // Step 11: Execute any post-processing hooks for the table - if ($modeltableEmpty && $notSelected) { + if ($modeltableEmpty && !$notSelected) { $this->runPostTableHook($t); } @@ -370,7 +373,7 @@ public function execute(Arguments $args, ConsoleIo $io): int $io->info("This is the last table to process."); } - $io->ask('Press to continue...'); + $io->ask('Press to continue...'); } return BaseCommand::CODE_SUCCESS; diff --git a/app/plugins/Transmogrify/src/Lib/Traits/CacheTrait.php b/app/plugins/Transmogrify/src/Lib/Traits/CacheTrait.php index 6c382e9d5..84f3beebe 100644 --- a/app/plugins/Transmogrify/src/Lib/Traits/CacheTrait.php +++ b/app/plugins/Transmogrify/src/Lib/Traits/CacheTrait.php @@ -37,6 +37,7 @@ trait CacheTrait * @var array */ protected array $cache = []; + /** * Cache results as configured for the specified table. * diff --git a/app/plugins/Transmogrify/src/Lib/Traits/HookRunnersTrait.php b/app/plugins/Transmogrify/src/Lib/Traits/HookRunnersTrait.php index 888976053..9e139c230 100644 --- a/app/plugins/Transmogrify/src/Lib/Traits/HookRunnersTrait.php +++ b/app/plugins/Transmogrify/src/Lib/Traits/HookRunnersTrait.php @@ -43,6 +43,8 @@ private function runPreTableHook(string $table): void { if(!method_exists($this, $method)) { throw new \RuntimeException("Unknown preTable hook: $method"); } + + $this->io->info('Running pre-table hook: ' . ucfirst(preg_replace('/([A-Z])/', ' $1', $method))); $this->{$method}(); } @@ -60,6 +62,7 @@ private function runPostTableHook(string $table): void { if(!method_exists($this, $method)) { throw new \RuntimeException("Unknown postTable hook: $method"); } + $this->io->info('Running post-table hook: ' . ucfirst(preg_replace('/([A-Z])/', ' $1', $method))); $this->{$method}(); } @@ -79,6 +82,7 @@ private function runPreRowHook(string $table, array &$origRow, array &$row): voi if(!method_exists($this, $method)) { throw new \RuntimeException("Unknown preRow hook: $method"); } + $this->io->info('Running pre-row hook: ' . ucfirst(preg_replace('/([A-Z])/', ' $1', $method))); $this->{$method}($origRow, $row); } @@ -98,6 +102,7 @@ private function runPostRowHook(string $table, array &$origRow, array &$row): vo if(!method_exists($this, $method)) { throw new \RuntimeException("Unknown postRow hook: $method"); } + $this->io->info('Running post-row hook: ' . ucfirst(preg_replace('/([A-Z])/', ' $1', $method))); $this->{$method}($origRow, $row); } @@ -118,6 +123,7 @@ private function runSqlSelectHook(string $table, string $qualifiedTableName): st if(!method_exists(RawSqlQueries::class, $method)) { throw new \RuntimeException("Unknown sqlSelect hook: $method"); } + $this->io->info('Running SQL select hook: ' . ucfirst(preg_replace('/([A-Z])/', ' $1', $method))); return RawSqlQueries::{$method}($qualifiedTableName, $this->inconn->isMySQL()); } } \ No newline at end of file diff --git a/app/plugins/Transmogrify/src/Lib/Traits/RowTransformationTrait.php b/app/plugins/Transmogrify/src/Lib/Traits/RowTransformationTrait.php index e766b322d..35cb34b9a 100644 --- a/app/plugins/Transmogrify/src/Lib/Traits/RowTransformationTrait.php +++ b/app/plugins/Transmogrify/src/Lib/Traits/RowTransformationTrait.php @@ -298,12 +298,37 @@ protected function mapExternalIdentityToExternalIdentityRole(array $origRow, arr $roleRow[$newKey] = $origRow[$oldKey]; } - if(!empty($origRow['affiliation'])) { - $row['affiliation'] = $origRow['affiliation']; - $roleRow['affiliation_type_id'] = $this->mapAffiliationType( - row: $row, - coId: $origRow['co_id'] ?? null - ); + // Rationale: mapOrgIdentitycoPersonId accepts only the first mapping it finds + // (later mappings are treated as legacy/unpooled anomalies and ignored). + // Therefore, each ExternalIdentity produces at most one ExternalIdentityRole. + // To avoid inserting a bogus self-referential changelog link, do not carry any + // legacy key into external_identity_role_id. Start a fresh changelog chain. + $roleRow['external_identity_role_id'] = null; + $roleRow['revision'] = 0; + + + try { + if (!empty($origRow['affiliation'])) { + $row['affiliation'] = $origRow['affiliation']; + $roleRow['affiliation_type_id'] = $this->mapAffiliationType( + row: $row, + coId: $origRow['co_id'] ?? null + ); + } + } catch (\Exception $e) { + $this->io->warning("Failed to map affiliation type: " . $e->getMessage()); + if ( + isset($origRow['co_id']) + && (!isset($this->cache['cos'][$origRow['co_id']]) + || ($this->cache['cos'][$origRow['co_id']]['status']) + && $this->cache['cos'][$origRow['co_id']]['status'] == 'TR') + ) { + // This CO has been deleted, so we can't map the type. We will return null + $roleRow['affiliation_type_id'] = null; + } else { + // Rethrow the exception + throw $e; + } } $tableName = 'external_identity_roles'; @@ -313,7 +338,13 @@ protected function mapExternalIdentityToExternalIdentityRole(array $origRow, arr $this->normalizeBooleanFieldsForDb($tableName, $roleRow); $qualifiedTableName = $this->outconn->qualifyTableName($tableName); - $this->outconn->insert($qualifiedTableName, $roleRow); + + + try { + $this->outconn->insert($qualifiedTableName, $roleRow); + } catch (\Exception $e) { + $this->io->warning("record already exists: " . print_r($roleRow, true)); + } } /** diff --git a/app/plugins/Transmogrify/src/Lib/Traits/TypeMapperTrait.php b/app/plugins/Transmogrify/src/Lib/Traits/TypeMapperTrait.php index 333c45895..c3a706411 100644 --- a/app/plugins/Transmogrify/src/Lib/Traits/TypeMapperTrait.php +++ b/app/plugins/Transmogrify/src/Lib/Traits/TypeMapperTrait.php @@ -29,7 +29,7 @@ namespace Transmogrify\Lib\Traits; -use App\Lib\Enum\StatusEnum; +use Doctrine\DBAL\Exception; use Transmogrify\Lib\Util\RawSqlQueries; use Cake\Utility\Inflector; use Transmogrify\Lib\Traits\CacheTrait; @@ -176,9 +176,10 @@ protected function mapNow(array $row) { /** * Map an Org Identity ID to a CO Person ID * - * @since COmanage Registry v5.0.0 - * @param array $row Row of Org Identity table data + * @param array $row Row of Org Identity table data * @return int|null CO Person ID + * @throws Exception + * @since COmanage Registry v5.0.0 */ protected function mapOrgIdentitycoPersonId(array $row): ?int { @@ -200,7 +201,7 @@ protected function mapOrgIdentitycoPersonId(array $row): ?int // do that.) Historical information remains available in history_records, // and if the deployer keeps an archive of the old database. - $oid = (int)$row['id']; + $rowId = (int)$row['id']; if(empty($this->cache['org_identities']['co_people'])) { $this->cache['org_identities']['co_people'] = []; @@ -213,42 +214,40 @@ protected function mapOrgIdentitycoPersonId(array $row): ?int $qualifiedTableName = $this->inconn->qualifyTableName($tableName); // Only fetch current rows (historical/deleted rows are filtered out) - $mapsql = RawSqlQueries::buildSelectAll( + $mapsql = RawSqlQueries::buildSelectAllWithNoChangelong( qualifiedTableName: $qualifiedTableName, - onlyCurrent: true, changelogFK: $changelogFK ); $stmt = $this->inconn->executeQuery($mapsql); - while ($r = $stmt->fetchAssociative()) { - if (!empty($r['org_identity_id'])) { - $rOid = (int)$r['org_identity_id']; - $rOrevision = (int)$r['revision']; - $cop = isset($r['co_person_id']) ? (int)$r['co_person_id'] : null; - - if ($cop === null) { - $this->io->warning('Org Identity ' . $rOid . ' has no mapped CO Person ID, skipping'); - continue; - } + while($r = $stmt->fetchAssociative()) { + $oid = $r['org_identity_id'] ?? null; - if (isset($this->cache['org_identities']['co_people'][$rOid][$rOrevision])) { + if(!empty($oid)) { + $rowRev = $r['revision']; + if(isset($this->cache['org_identities']['co_people'][ $oid ][ $rowRev ])) { // If for some reason we already have a record, it's probably due to // improper unpooling from a legacy deployment. We'll accept only the // first record and throw warnings on the others. - $this->io->verbose('Found duplicate current CO Person mapping for Org Identity ' . $rOid . ', skipping'); - continue; - } - $this->cache['org_identities']['co_people'][$rOid][$rOrevision] = $cop; + $this->io->verbose("Found existing CO Person for Org Identity " . $oid . ", skipping"); + } else { + $this->cache['org_identities']['co_people'][ $oid ][ $rowRev ] = $r['co_person_id']; + } } } + + // Sort the array + sort($this->cache['org_identities']['co_people']); } - if (!empty($this->cache['org_identities']['co_people'][$oid])) { + if(!empty($this->cache['org_identities']['co_people'][ $rowId ])) { + // XXX OrgIdentities with no org identity link are not supported in v5 // Return the record with the highest revision number - $rev = max(array_keys($this->cache['org_identities']['co_people'][ $oid ])); - return $this->cache['org_identities']['co_people'][$oid][$rev]; + $rev = max(array_keys($this->cache['org_identities']['co_people'][ $rowId ])); + + return $this->cache['org_identities']['co_people'][ $rowId ][$rev]; } return null; @@ -302,7 +301,7 @@ protected function mapType(array $row, string $type, int $coId, string $attr = ' if(empty($this->cache['types']['co_id+attribute+value+'][$key])) { if ( !isset($this->cache['cos'][$coId]) - || ($this->cache['cos'][$coId]['status'] && $this->cache['cos'][$coId]['status'] == 'TR') + || ($this->cache['cos'][$coId]['status'] && in_array($this->cache['cos'][$coId]['status'], ['TR'])) ) { // This CO has been deleted, so we can't map the type. We will return null return null; diff --git a/app/plugins/Transmogrify/src/Lib/Util/RawSqlQueries.php b/app/plugins/Transmogrify/src/Lib/Util/RawSqlQueries.php index 6ff0bffdd..2c622ce5d 100644 --- a/app/plugins/Transmogrify/src/Lib/Util/RawSqlQueries.php +++ b/app/plugins/Transmogrify/src/Lib/Util/RawSqlQueries.php @@ -69,22 +69,23 @@ public static function buildSelectAllOrderedById(string $qualifiedTableName): st /** * Builds SQL query to select all rows, optionally filtering changelog records * @param string $qualifiedTableName Fully qualified table name - * @param bool $onlyCurrent Whether to include changelog records (default true) - * @param string|null $changelogFK Name of the changelog foreign key column * @return string SQL query string */ - public static function buildSelectAll( + public static function buildSelectAll(string $qualifiedTableName): string { + return "SELECT * FROM $qualifiedTableName"; + } + + /** + * Builds SQL query to select all rows, filtering changelog records + * @param string $qualifiedTableName Fully qualified table name + * @param string $changelogFK Changelog Foreign Key + * @return string SQL query string + */ + public static function buildSelectAllWithNoChangelong( string $qualifiedTableName, - bool $onlyCurrent = false, - string $changelogFK = null + string $changelogFK ): string { - if ($onlyCurrent) { - if (empty($changelogFK)) { - throw new \InvalidArgumentException('changelogFK is required when onlyCurrent is true'); - } - return "SELECT * FROM $qualifiedTableName WHERE $changelogFK IS NULL"; - } - return "SELECT * FROM $qualifiedTableName"; + return "SELECT * FROM $qualifiedTableName WHERE $changelogFK IS NULL"; } /**