diff --git a/docs/en/09_migrating/02_gorriecoe-migration.md b/docs/en/09_migrating/02_gorriecoe-migration.md new file mode 100644 index 00000000..e7b5a617 --- /dev/null +++ b/docs/en/09_migrating/02_gorriecoe-migration.md @@ -0,0 +1,194 @@ +# Title to shut my IDE linter up + +> [!WARNING] +> This guide and the associated migration task assume all of the data for your links are in the base table for `gorriecoe\Link\Models\Link` or in automatically generated tables (e.g. join tables for `many_many` relations). +> If you have subclassed `gorriecoe\Link\Models\Link`, there may be additional steps you need to take to migrate the data for your subclass. + +Remove the gorriecoe modules and add silverstripe/linkfield: + +```bash +composer require silverstripe/linkfield:^4 +composer remove gorriecoe/silverstripe-link gorriecoe/silverstripe-linkfield +``` + +If you added any database columns to the `Link` class for sorting `has_many` relations, or any `has_one` relations for storing them, remove the extension or yaml configuration for that now. + +```diff +- gorriecoe\Link\Models\Link: +- db: +- MySortColumn: Int +- has_one: +- Record: App\Model\MyRecord +- belongs_many_many: +- BelongsRecord : App\Model\MyRecord.LinkListTwo +``` + +Update namespaces and relations. + +```diff + namespace App\Model; + +- use gorriecoe\Link\Models\Link; +- use gorriecoe\LinkField\LinkField; ++ use SilverStripe\LinkField\Models\Link; ++ use SilverStripe\LinkField\Form\LinkField; ++ use SilverStripe\LinkField\Form\MultiLinkField; + use SilverStripe\ORM\DataObject; + + class MyRecord extends DataObject + { + private static array $has_one = [ + 'HasOneLink' => Link::class, + ]; + + private static array $has_many = [ +- 'LinkListOne' => Link::class . '.Record', ++ 'LinkListOne' => Link::class . '.Owner', ++ 'LinkListTwo' => Link::class . '.Owner', + ]; + ++ private static array $owns = [ ++ 'HasOneLink', ++ 'LinkListOne', ++ 'LinkListTwo', ++ ]; ++ +- private static array $many_many = [ +- 'LinkListTwo' => Link::class, +- ]; +- +- private static array $many_many_extraFields = [ +- 'LinkListTwo' => [ +- 'Sort' => 'Int', +- ] +- ]; + + public function getCMSFields() + { + $fields = parent::getCMSFields(); ++ $fields->removeByName(['LinkListOneID', 'LinkListOne', 'LinkListTwo']); + $fields->addFieldsToTab( + 'Root.Main', + [ +- LinkField::create('HasOneLink', 'Has one link', $this), +- LinkField::create('LinkListOne', 'List list one', $this)->setSortColumn('MySortColumn'), +- LinkField::create('LinkListTwo', 'Link list two', $this), ++ LinkField::create('HasOneLink', 'Has one link'), ++ MultiLinkField::create('LinkListOne', 'List list one'), ++ MultiLinkField::create('LinkListTwo', 'Link list two'), + ] + ); + return $fields; + } + } +``` + +If you had `many_many` through, delete the `DataObject` class being used as the join table. + +NOTE: If the same link appears in multiple `many_many` relation lists, the link will be duplicated so that a single link exists for each `has_many` relation list. +Unless you were doing something custom to manage links, it's unlikely this will affect you - but if it does, just be aware of this and prepare your content authors for +this change in their authoring workflow. + +If you applied [linkfield configuration](https://github.com/elliot-sawyer/silverstripe-linkfield?tab=readme-ov-file#configuration), update that now also. +See [configuring links and link fields](../02_configuration.md) for more information. + +```diff ++ use SilverStripe\LinkField\Models\ExternalLink; ++ use SilverStripe\LinkField\Models\SiteTreeLink; + +- $linkConfig = [ +- 'types' => [ +- 'SiteTree', +- 'URL', +- ], +- 'title_display' => false, +- ]; +- $linkField->setLinkConfig($linkConfig); ++ $allowedTypes = [ ++ SiteTreeLink::class, ++ ExternalLink::class, ++ ]; ++ $linkField->setAllowedTypes($allowedTypes); ++ $linkField->setExcludeLinkTextField(true); +``` + +Custom link implementations you may be using include: + +- [gorriecoe/silverstripe-securitylinks](https://github.com/gorriecoe/silverstripe-securitylinks) +- [gorriecoe/silverstripe-directionslink](https://github.com/gorriecoe/silverstripe-directionslink) +- [gorriecoe/silverstripe-advancedemaillink](https://github.com/gorriecoe/silverstripe-advancedemaillinks) + +Other customisations you may be using that will require manual migration include: + +- [gorriecoe/silverstripe-linkicon](https://github.com/gorriecoe/silverstripe-linkicon) +- [gorriecoe/silverstripe-ymlpresetlinks](https://github.com/gorriecoe/silverstripe-ymlpresetlinks) + +Things devs need to handle or be aware of: + +- Phone number validation +- External URL link doesn't allow relative URLs +- No `addExtraClass()` or related methods for templates +- `getLinkURL()` is now just `getURL()` +- No `SiteTree` helpers like `isCurrent()`, `isOrphaned()` etc (you can check those methods on the `Page` in `SiteTreeLink` instead) +- Permission checks are based on `Owner` (previously just returned `true` for everything) +- No `link_to_folders` config - uses `UploadField` instead. +- No graphql helper methods - just use regular GraphQL scaffolding if you need to fetch the links via GraphQL. +- No "Settings" tab +- Can't swap link type. +- https://github.com/elliot-sawyer/silverstripe-link/blob/master/src/extensions/DefineableMarkupID.php +- https://github.com/elliot-sawyer/silverstripe-link/blob/master/src/extensions/DBStringLink.php +- May want to update localisations. Link to transifex. + +One config for DB fields that are shared across all links + +```yml +base_link_columns: + ID: 'ID' + OpenInNewWindow: 'OpenInNew' + Title: 'LinkText' + # Can add sort for has_many here too + Sort: 'Sort' +``` + +Sort is ascending in gorriecoe's module + +One config for per-type (i.e. dependent on the "Type" column) to class + +```yml +link_type_columns: + URL: + class: 'SilverStripe\LinkField\Models\ExternalLink' + fields: + URL: 'ExternalUrl' + Email: + class: 'SilverStripe\LinkField\Models\EmailLink' + fields: + Email: 'Email' + Phone: + class: 'SilverStripe\LinkField\Models\PhoneLink' + fields: + Phone: 'Phone' + File: + class: 'SilverStripe\LinkField\Models\FileLink' + fields: + FileID: 'FileID' + SiteTree: + class: 'SilverStripe\LinkField\Models\SiteTreeLink' + fields: + SiteTreeID: 'PageID' +``` + +Note that `SiteTreeLink` also needs to take the `Anchor` column and split it out between `Anchor` and `QueryString`, and remove the `#` and `?`prefixes respectively. +If there's no prefix, make these assumptions: + It's a query string if there's a `=` or `&` anywhere in it + It's an anchor in all other cases. + +Note also that the `Title` may be equal to the default title, which differs based on the type. +We do not want to copy default titles into `LinkText`. + +This no longer does anything, and has no equivalent (the field is always `Sort` which is on Link by default): + +```yml +gorriecoe\LinkField\LinkField: + sort_column: 'SortOrder' +``` diff --git a/src/Tasks/GorriecoeMigrationTask.php b/src/Tasks/GorriecoeMigrationTask.php new file mode 100644 index 00000000..eca96029 --- /dev/null +++ b/src/Tasks/GorriecoeMigrationTask.php @@ -0,0 +1,676 @@ + 'OpenInNew', + 'Title' => 'LinkText', + ]; + + /** + * Mapping for different types of links, including the class to map to and + * database column mappings. + */ + private static array $link_type_columns = [ + 'URL' => [ + 'class' => ExternalLink::class, + 'fields' => [ + 'URL' => 'ExternalUrl', + ], + ], + 'Email' => [ + 'class' => EmailLink::class, + 'fields' => [ + 'Email' => 'Email', + ], + ], + 'Phone' => [ + 'class' => PhoneLink::class, + 'fields' => [ + 'Phone' => 'Phone', + ], + ], + 'File' => [ + 'class' => FileLink::class, + 'fields' => [ + 'FileID' => 'FileID', + ], + ], + 'SiteTree' => [ + 'class' => SiteTreeLink::class, + 'fields' => [ + 'SiteTreeID' => 'PageID', + ], + ], + ]; + + /** + * List any has_many relations that should be migrated. + * + * Keys are the FQCN for the class where the has_many is declared. + * Values are the name of the old has_one. + * + * Example: + * + * // SiteConfig had two has_many relationships, + * // one to Link.MyHasOne and another to Link.DifferentHasOne. + * SiteConfig::class => [ + * 'LinksListOne' => 'MyHasOne', + * 'LinksListTwo' => 'DifferentHasOne', + * ] + * + */ + private static array $has_many_links_data = []; + + /** + * List any many_many relations that should be migrated. + * + * Keys are the FQCN for the class where the many_many is declared. See example below for values. + * + * Example: + * + * // SiteConfig had three many_many relationships: + * // The table name for "LinksListOne" will be guessed. It wasn't a many_many through and had no extra fields + * // The table name for "LinksListTwo" will be guessed. It wasn't a many_many through but did have some extra fields + * // The table name for "LinksListThree" is explicitly provided. It was a many_many through with some extra fields + * SiteConfig::class => [ + * 'LinksListOne' => null, + * 'LinksListTwo' => [ + * 'extraFields' => [ + * 'MySort' => 'Sort', + * ], + * ], + * 'LinksListThree' => [ + * 'table' => 'App_MyThroughClassTable', + * 'extraFields' => [ + * 'MySort' => 'Sort', + * ], + * 'through' => [ + * 'from' => 'FromHasOneName', + * 'to' => 'ToHasOneName', + * ], + * ], + * ] + * + */ + private static array $many_many_links_data = []; + + /** + * in-memory cache of Link relational data so we don't keep slamming the filesystem cache + * when checking these relations in CLI + */ + private array $linkRelationData = []; + + /** + * The table name for the base gorriecoe link model. + */ + private string $oldTableName; + + /** + * Perform the actual data migration and publish links as appropriate + */ + public function performMigration(): void + { + $this->extend('beforePerformMigration'); + // Because we're using SQL INSERT with specific ID values, + // we can't perform the migration if there are existing links because there + // may be ID conflicts. + if (Link::get()->exists()) { + throw new RuntimeException('Cannot perform migration with existing silverstripe/linkfield link records.'); + } + + $this->insertBaseRows(); + $this->insertTypeSpecificRows(); + $this->updateSiteTreeRows(); + $this->migrateHasManyRelations(); + $this->migrateManyManyRelations(); + $this->setOwnerForHasOneLinks(); + + $this->print("Dropping old link table {$this->oldTableName}"); + DB::get_conn()->query("DROP TABLE \"{$this->oldTableName}\""); + + $this->print('-----------------'); + $this->print('Bulk data migration complete. All links should be correct (but unpublished) at this stage.'); + $this->print('-----------------'); + + $this->publishLinks(); + + $this->print('-----------------'); + $this->print('Migration completed successfully.'); + $this->print('-----------------'); + $this->extend('afterPerformMigration'); + } + + /** + * Check if we actually need to migrate anything, and if not give clear output as to why not. + */ + private function getNeedsMigration(): bool + { + $oldTableName = $this->getTableOrObsoleteTable(static::config()->get('old_link_table')); + if (!$oldTableName) { + $this->print('Nothing to migrate - old link table doesn\'t exist.'); + return false; + } + $this->oldTableName = $oldTableName; + return true; + } + + /** + * Insert a row into the base Link table for each link, mapping all of the columns + * that are shared across all link types. + */ + private function insertBaseRows(): void + { + $this->extend('beforeInsertBaseRows'); + $db = DB::get_conn(); + + // Get a full map of columns to migrate that applies to all link types + $baseTableColumnMap = $this->getBaseColumnMap(); + // ClassName will need to be handled per link type + unset($baseTableColumnMap['ClassName']); + + // Set the correct ClassName based on the type of link. + // Note that case statements have no abstraction, but are already used elsewhere + // so should be safe. See DataQuery::getFinalisedQuery() which is used for all + // DataList queries. + $classNameSelect = 'CASE '; + $typeColumn = $db->escapeIdentifier("{$this->oldTableName}.Type"); + foreach (static::config()->get('link_type_columns') as $type => $spec) { + $toClass = $db->quoteString($spec['class']); + $type = $db->quoteString($type); + $classNameSelect .= "WHEN {$typeColumn} = {$type} THEN {$toClass} "; + } + $classNameSelect .= 'ELSE ' . $db->quoteString(Link::class) . ' END AS ClassName'; + + // Insert rows + $baseTable = DataObject::getSchema()->baseDataTable(Link::class); + $quotedBaseTable = $db->escapeIdentifier($baseTable); + $baseColumns = implode(', ', array_values($baseTableColumnMap)); + $subQuery = SQLSelect::create( + array_keys($baseTableColumnMap), + $db->escapeIdentifier($this->oldTableName) + )->addSelect($classNameSelect)->sql(); + // We can't use the ORM to do INSERT with SELECT, but thankfully + // the syntax is generic enough that it should work for all SQL databases. + DB::query("INSERT INTO {$quotedBaseTable} ({$baseColumns}, ClassName) {$subQuery}"); + $this->extend('afterInsertBaseRows'); + } + + /** + * Insert rows for all link subclasses based on the type of the old link + */ + private function insertTypeSpecificRows(): void + { + $this->extend('beforeInsertTypeSpecificRows'); + $schema = DataObject::getSchema(); + $db = DB::get_conn(); + foreach (static::config()->get('link_type_columns') as $type => $spec) { + $type = $db->quoteString($type); + $toClass = $spec['class']; + $columnMap = $spec['fields']; + + $table = $schema->tableName($toClass); + $quotedTable = $db->escapeIdentifier($table); + $baseColumns = implode(', ', array_values($columnMap)); + $subQuery = SQLSelect::create( + ['ID', ...array_keys($columnMap)], + $db->escapeIdentifier($this->oldTableName), + [$db->escapeIdentifier("{$this->oldTableName}.Type") . " = {$type}"] + )->sql(); + // We can't use the ORM to do INSERT with SELECT, but thankfully + // the syntax is generic enough that it should work for all SQL databases. + DB::query("INSERT INTO {$quotedTable} (ID, {$baseColumns}) {$subQuery}"); + } + $this->extend('afterInsertTypeSpecificRows'); + } + + /** + * Update the Anchor column for SiteTreeLink + */ + private function updateSiteTreeRows(): void + { + $this->extend('beforeUpdateSiteTreeRows'); + // We have to split the Anchor column, which means we have to fetch and operate on the values. + $currentChunk = 0; + $chunkSize = static::config()->get('chunk_size'); + $count = $chunkSize; + $db = DB::get_conn(); + $schema = DataObject::getSchema(); + $siteTreeLinkTable = $schema->tableForField(SiteTreeLink::class, 'Anchor'); + // Keep looping until we run out of chunks + while ($count >= $chunkSize) { + // Get data about the old SiteTree links + $oldLinkRows = SQLSelect::create( + ['ID', 'Anchor'], + $db->escapeIdentifier($this->oldTableName), + [ + $db->escapeIdentifier($this->oldTableName . '.Type') => 'SiteTree', + $db->nullCheckClause($db->escapeIdentifier($this->oldTableName . '.Anchor'), false) + ] + )->setLimit($chunkSize, $chunkSize * $currentChunk)->execute(); + // Prepare for next iteration + $count = $oldLinkRows->numRecords(); + $currentChunk++; + + // Update all links which have an anchor + foreach ($oldLinkRows as $oldLink) { + // Get the query string and anchor separated + $queryString = null; + $anchor = null; + $oldAnchor = $oldLink['Anchor']; + if (str_starts_with($oldAnchor, '#')) { + $parts = explode('?', $oldAnchor, 2); + $anchor = ltrim($parts[0], '#'); + $queryString = ltrim($parts[1] ?? '', '?'); + } elseif (str_starts_with($oldAnchor, '?')) { + $parts = explode('#', $oldAnchor, 2); + $queryString = ltrim($parts[0], '?'); + $anchor = ltrim($parts[1] ?? '', '#'); + } else { + // Assume it's an anchor and they just forgot the # + // We don't need the # so just add it directly. + $anchor = $oldAnchor; + } + $this->extend('updateAnchorAndQueryString', $anchor, $queryString, $oldAnchor); + // Update the link with the correct anchor and query string + SQLUpdate::create( + $db->escapeIdentifier($siteTreeLinkTable), + [ + $schema->sqlColumnForField(SiteTreeLink::class, 'Anchor') => $anchor, + $schema->sqlColumnForField(SiteTreeLink::class, 'QueryString') => $queryString, + ], + [$db->escapeIdentifier($siteTreeLinkTable . '.ID') => $oldLink['ID']] + )->execute(); + } + + // If $chunkSize was null, we did everything in a single chunk + // but we need to break the loop artificially. + if ($chunkSize === null) { + break; + } + } + $this->extend('afterUpdateSiteTreeRows'); + } + + private function migrateHasManyRelations(): void + { + $this->extend('beforeMigrateHasManyRelations'); + $linksList = static::config()->get('has_many_links_data'); + + // Exit early if there's nothing to migrate + if (empty($linksList)) { + $this->print('No has_many relations to migrate.'); + $this->extend('afterMigrateHasManyRelations'); + return; + } + + $this->print('Migrating has_many relations.'); + $schema = DataObject::getSchema(); + $db = DB::get_conn(); + $oldTableFields = DB::field_list($this->oldTableName); + foreach ($linksList as $ownerClass => $relations) { + foreach ($relations as $hasManyRelation => $hasOneRelation) { + // Check if HasOneID column is in the old base Link table + if (!array_key_exists("{$hasOneRelation}ID", $oldTableFields)) { + // This is an unusual situation, and is difficult to do generically. + // We'll leave this scenario up to the developer to handle. + $this->extend('migrateHasOneForLinkSubclass', $linkClass, $ownerClass, $hasOneRelation, $hasManyRelation); + continue; + } + $linkTable = $schema->baseDataTable(Link::class); + $tables = [$linkTable]; + // Include versioned tables if link is versioned + if (Link::has_extension(Versioned::class)) { + $tables[] = "{$linkTable}_Versions"; + $tables[] = "{$linkTable}_Live"; + } + $wasPolyMorphic = array_key_exists("{$hasOneRelation}Class", $oldTableFields); + $wasMultiRelational = $wasPolyMorphic && array_key_exists("{$hasOneRelation}Relation", $oldTableFields); + // Migrate old has_one on link to the Owner relation. + foreach ($tables as $table) { + // Only set owner where the OwnerID is not already set + $ownerIdColumn = $db->escapeIdentifier($table . '.OwnerID'); + $nullCheck = $db->nullCheckClause($ownerIdColumn, true); + $whereClause = [ + "$ownerIdColumn = 0 OR $nullCheck", + $db->nullCheckClause($db->escapeIdentifier($table . '.OwnerRelation'), true), + ]; + if ($wasPolyMorphic) { + // For polymorphic relations, don't set the owner for records belonging + // to a different class hierarchy. + $validClasses = ClassInfo::subclassesFor($ownerClass, true); + $placeholders = DB::placeholders($validClasses); + $whereClause[] = [$db->escapeIdentifier("{$this->oldTableName}.{$hasOneRelation}Class") . " IN ($placeholders)" => $validClasses]; + if ($wasMultiRelational) { + $whereClause[] = [$db->escapeIdentifier("{$this->oldTableName}.{$hasOneRelation}Relation") => $hasManyRelation]; + } + } + $update = SQLUpdate::create( + $db->escapeIdentifier($table), + [ + $db->escapeIdentifier($table . '.OwnerID') => [$schema->sqlColumnForField($ownerClass, 'ID') => []], + $db->escapeIdentifier($table . '.OwnerClass') => [$schema->sqlColumnForField($ownerClass, 'ClassName') => []], + $db->escapeIdentifier($table . '.OwnerRelation') => $hasManyRelation, + ], + $whereClause + ) + ->addInnerJoin($this->oldTableName, $db->escapeIdentifier($this->oldTableName . '.ID') . ' = ' . $db->escapeIdentifier("{$table}.ID")) + ->addInnerJoin($schema->baseDataTable($ownerClass), $schema->sqlColumnForField($ownerClass, 'ID') . ' = ' . $db->escapeIdentifier("{$this->oldTableName}.{$hasOneRelation}ID")); + $update->execute(); + } + } + } + $this->extend('afterMigrateHasManyRelations'); + } + + private function migrateManyManyRelations(): void + { + $this->extend('beforeMigrateManyManyRelations'); + $linksList = static::config()->get('many_many_links_data'); + + // Exit early if there's nothing to migrate + if (empty($linksList)) { + $this->print('No many_many relations to migrate.'); + $this->extend('afterMigrateManyManyRelations'); + return; + } + + $this->print('Migrating many_many relations.'); + $schema = DataObject::getSchema(); + $db = DB::get_conn(); + $baseLinkTable = $schema->baseDataTable(Link::class); + $originalOldLinkTable = str_replace('_obsolete_', '', $this->oldTableName); + foreach ($linksList as $ownerClass => $relations) { + $ownerBaseTable = $schema->baseDataTable($ownerClass); + $ownerTable = $schema->tableName($ownerClass); + foreach ($relations as $manyManyRelation => $spec) { + $throughSpec = $spec['through'] ?? []; + if (!empty($throughSpec)) { + if (!isset($spec['table'])) { + throw new RuntimeException("Must declare the table name for many_many through relation '{$ownerClass}.{$manyManyRelation}'."); + } + $ownerIdField = $throughSpec['from'] . 'ID'; + $linkIdField = $throughSpec['to'] . 'ID'; + } else { + $ownerIdField = "{$ownerTable}ID"; + $linkIdField = "{$originalOldLinkTable}ID"; + } + $extraFields = $spec['extraFields'] ?? []; + $joinTable = $this->getTableOrObsoleteTable($spec['table'] ?? "{$ownerTable}_{$manyManyRelation}"); + + if ($joinTable === null) { + throw new RuntimeException("Couldn't find join table for many_many relation '{$ownerClass}.{$manyManyRelation}'."); + } + + $polymorphicWhereClause = []; + if (!empty($throughSpec)) { + $joinColumns = DB::field_list($joinTable); + if (array_key_exists($throughSpec['from'] . 'Class', $joinColumns)) { + // For polymorphic relations, don't set the owner for records belonging + // to a different class hierarchy. + $validClasses = ClassInfo::subclassesFor($ownerClass, true); + $placeholders = DB::placeholders($validClasses); + $polymorphicClassColumn = $throughSpec['from'] . 'Class'; + $polymorphicWhereClause = [$db->escapeIdentifier("{$joinTable}.{$polymorphicClassColumn}") . " IN ($placeholders)" => $validClasses]; + } + } + + // If the join table for many_many through still has an associated DataObject class, + // something is very weird and we should throw an error. + // Most likely the developer just forgot to delete it or didn't run dev/build before running this task. + if (!empty($throughSpec) && $schema->tableClass($joinTable) !== null) { + throw new RuntimeException("Join table '{$joinTable}' for many_many through relation '{$ownerClass}.{$manyManyRelation}' still has a DataObject class."); + } + + $this->copyDuplicatedLinksInThisRelation($manyManyRelation, $ownerBaseTable, $joinTable, $linkIdField, $ownerIdField, $extraFields, $polymorphicWhereClause); + + $tables = [$baseLinkTable]; + // Include versioned tables if link is versioned + if (Link::has_extension(Versioned::class)) { + $tables[] = "{$baseLinkTable}_Versions"; + $tables[] = "{$baseLinkTable}_Live"; + } + foreach ($tables as $table) { + $ownerIdColumn = $db->escapeIdentifier($table . '.OwnerID'); + $nullCheck = $db->nullCheckClause($ownerIdColumn, true); + + // Set owner fields + $assignments = [ + $ownerIdColumn => $db->escapeIdentifier("{$ownerBaseTable}.ID"), + $db->escapeIdentifier("{$table}.OwnerClass") => $db->escapeIdentifier("{$ownerBaseTable}.ClassName"), + $db->escapeIdentifier("{$table}.OwnerRelation") => $manyManyRelation, + ]; + // Set extra fields + foreach ($extraFields as $fromField => $toField) { + $assignments[$db->escapeIdentifier("{$table}.{$toField}")] = [$db->escapeIdentifier("{$joinTable}.{$fromField}") => []]; + } + + // Make the update, joining on the join table and base owner table + $update = SQLUpdate::create( + $db->escapeIdentifier($table), + $assignments, + [ + // Don't set if there's already an owner for that link + "$ownerIdColumn = 0 OR $nullCheck", + $db->nullCheckClause($db->escapeIdentifier($table . '.OwnerRelation'), true), + ...$polymorphicWhereClause, + ] + )->addInnerJoin($joinTable, $db->escapeIdentifier("{$joinTable}.{$linkIdField}") . ' = ' . $db->escapeIdentifier("{$table}.ID")) + ->addInnerJoin($ownerBaseTable, $db->escapeIdentifier("{$ownerBaseTable}.ID") . ' = ' . $db->escapeIdentifier("{$joinTable}.{$ownerIdField}")); + $update->execute(); + } + // Drop the join table + $this->print("Dropping old many_many join table {$joinTable}"); + DB::get_conn()->query("DROP TABLE \"{$joinTable}\""); + } + } + + $this->extend('afterMigrateManyManyRelations'); + } + + /** + * Duplicate any links which appear multiple times in a many_many relation + * and remove the duplicate rows from the join table + */ + private function copyDuplicatedLinksInThisRelation( + string $relationName, + string $ownerBaseTable, + string $joinTable, + string $linkIdField, + string $ownerIdField, + array $extraFields, + array $polymorphicWhereClause + ): void { + $db = DB::get_conn(); + $schema = DataObject::getSchema(); + $baseLinkTable = $schema->baseDataTable(Link::class); + $joinLinkIdColumn = $db->escapeIdentifier("{$joinTable}.{$linkIdField}"); + $joinOwnerIdColumn = $db->escapeIdentifier("{$joinTable}.{$ownerIdField}"); + + // Prepare subquery that identifies which rows are for duplicate links + $duplicates = SQLSelect::create( + $joinLinkIdColumn, + $db->escapeIdentifier($joinTable), + $polymorphicWhereClause, + groupby: $joinLinkIdColumn, + having: "COUNT({$joinLinkIdColumn}) > 1" + )->execute(); + + // Exit early if there's no duplicates + if ($duplicates->numRecords() < 1) { + return; + } + + // Get selection fields, aliased so they can be dropped straight into a link record + $selections = [ + $joinLinkIdColumn => '"ID"', + $db->escapeIdentifier("{$ownerBaseTable}.ClassName") => '"OwnerClass"', + $db->escapeIdentifier("{$ownerBaseTable}.ID") => '"OwnerID"', + ]; + // Select additional base columns except where they're mapped as extra fields (e.g. sort may come from manymany) + foreach ($this->getBaseColumnMap() as $baseField) { + if ($baseField !== 'ID' && !in_array($baseField, $extraFields)) { + $selections[$db->escapeIdentifier("{$baseLinkTable}.{$baseField}")] = '"' . $baseField . '"'; + } + } + // Select extra fields, aliased as appropriate + foreach ($extraFields as $fromField => $toField) { + $selections[$db->escapeIdentifier("{$joinTable}.{$fromField}")] = '"' . $toField . '"'; + } + // Select columns from subclasses (e.g. Email, Phone, etc) + foreach (static::config()->get('link_type_columns') as $spec) { + foreach ($spec['fields'] as $subclassField) { + $selections[$schema->sqlColumnForField($spec['class'], $subclassField)] = '"' . $subclassField . '"'; + } + } + + $toDelete = []; + $originalLinks = []; + $currentChunk = 0; + $chunkSize = static::config()->get('chunk_size'); + $count = $chunkSize; + $duplicateIDs = implode(', ', $duplicates->column()); + + // To ensure this scales well, we'll fetch and duplicate links in chunks. + while ($count >= $chunkSize) { + $select = SQLSelect::create( + $selections, + $db->escapeIdentifier($joinTable), + [ + "{$joinLinkIdColumn} in ({$duplicateIDs})", + ...$polymorphicWhereClause, + ] + ) + ->addInnerJoin($ownerBaseTable, $db->escapeIdentifier("{$ownerBaseTable}.ID") . " = {$joinOwnerIdColumn}") + ->addInnerJoin($baseLinkTable, $db->escapeIdentifier($db->escapeIdentifier("{$baseLinkTable}.ID") . " = {$joinLinkIdColumn}")); + // Add joins for link subclasses + foreach (ClassInfo::subclassesFor(Link::class) as $class) { + // Skip Link (which is already joined) and anything without a table + if ($class === Link::class || !ClassInfo::hasTable($class)) { + continue; + } + // Left join because we just want any extra data these have, rather than narrowing down the result set + $table = $schema->tableName($class); + $select->addLeftJoin( + $table, + $db->escapeIdentifier("{$table}.ID") . ' = ' . $db->escapeIdentifier("{$baseLinkTable}.ID") + ); + } + $linkData = $select->setLimit($chunkSize, $chunkSize * $currentChunk)->execute(); + // Prepare for next iteration + $count = $linkData->numRecords(); + $currentChunk++; + + foreach ($linkData as $link) { + $ownerID = $link['OwnerID']; + $linkID = $link['ID']; + unset($link['ID']); + // Skip the first of each duplicate set (i.e. the original link) + if (!array_key_exists($linkID, $originalLinks)) { + $originalLinks[$linkID] = true; + continue; + } + // Mark duplicate join row for deletion + $toDelete[] = "{$joinOwnerIdColumn} = {$ownerID} AND {$joinLinkIdColumn} = {$linkID}"; + // Create the duplicate link - note it already has its correct owner relation and other necessary data + $link['OwnerRelation'] = $relationName; + $newLink = $link['ClassName']::create($link); + $this->extend('updateNewLink', $newLink, $link); + $newLink->write(); + } + + // If $chunkSize was null, we did everything in a single chunk + // but we need to break the loop artificially. + if ($chunkSize === null) { + break; + } + } + + // Delete the duplicate rows from the join table + SQLDelete::create($db->escapeIdentifier($joinTable), $polymorphicWhereClause)->addWhereAny($toDelete)->execute(); + } + + /** + * If the table exists, returns it. If it exists but is obsolete, returned the obsolete + * prefixed name. + * Returns null if the table doesn't exist at all. + */ + private function getTableOrObsoleteTable(string $tableName): ?string + { + $allTables = DB::table_list(); + if (!array_key_exists(strtolower($tableName), $allTables)) { + $tableName = '_obsolete_' . $tableName; + if (!array_key_exists(strtolower($tableName), $allTables)) { + return null; + } + } + return $tableName; + } + + private function getBaseColumnMap(): array + { + $baseColumnMap = static::config()->get('base_link_columns'); + foreach (array_keys(DataObject::config()->uninherited('fixed_fields')) as $fixedField) { + $baseColumnMap[$fixedField] = $fixedField; + } + return $baseColumnMap; + } + + private function classIsOldLink(string $class): bool + { + return $class === 'gorriecoe\Link\Models\Link'; + } +} diff --git a/src/Tasks/LinkFieldMigrationTask.php b/src/Tasks/LinkFieldMigrationTask.php index 558cc14f..f0958d6e 100644 --- a/src/Tasks/LinkFieldMigrationTask.php +++ b/src/Tasks/LinkFieldMigrationTask.php @@ -4,27 +4,13 @@ use LogicException; use RuntimeException; -use SilverStripe\Assets\Shortcodes\FileLink as WYSIWYGFileLink; -use SilverStripe\CMS\Model\SiteTreeLink as WYSIWYGSiteTreeLink; -use SilverStripe\Control\Director; use SilverStripe\Core\ClassInfo; -use SilverStripe\Core\Config\Config; use SilverStripe\Dev\BuildTask; -use SilverStripe\Dev\Deprecation; -use SilverStripe\LinkField\Models\EmailLink; -use SilverStripe\LinkField\Models\ExternalLink; -use SilverStripe\LinkField\Models\FileLink; use SilverStripe\LinkField\Models\Link; -use SilverStripe\LinkField\Models\PhoneLink; -use SilverStripe\LinkField\Models\SiteTreeLink; -use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\DB; use SilverStripe\ORM\Queries\SQLSelect; use SilverStripe\ORM\Queries\SQLUpdate; -use SilverStripe\Versioned\ChangeSet; -use SilverStripe\Versioned\ChangeSetItem; use SilverStripe\Versioned\Versioned; /** @@ -32,6 +18,8 @@ */ class LinkFieldMigrationTask extends BuildTask { + use MigrationTaskTrait; + private static $segment = 'linkfield-tov4-migration-task'; protected $title = 'Linkfield v2/3 to v4 Migration Task'; @@ -43,18 +31,6 @@ class LinkFieldMigrationTask extends BuildTask */ private static ?bool $is_enabled = false; - /** - * Classes which should be skipped when finding owners of links. - * These classes and all of their subclasses will be skipped. - */ - private static array $classes_that_are_not_link_owners = [ - // Skip models that are used for internal tracking purposes and cannot own links - ChangeSet::class, - ChangeSetItem::class, - WYSIWYGFileLink::class, - WYSIWYGSiteTreeLink::class, - ]; - /** * List any has_many relations that should be migrated. * @@ -86,56 +62,12 @@ class LinkFieldMigrationTask extends BuildTask */ private array $linkRelationData = []; - public function __construct() - { - // Use withNoReplacement() because otherwise even viewing the dev/tasks list will trigger this warning. - Deprecation::withNoReplacement( - fn () => Deprecation::notice('4.0.0', 'Will be removed without equivalent functionality.', Deprecation::SCOPE_CLASS) - ); - parent::__construct(); - } - - public function run($request): void - { - $db = DB::get_conn(); - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - - // If we don't need to migrate, exit early. - if (!$this->getNeedsMigration($baseTable)) { - $this->print('Cannot perform migration.'); - return; - } - - if (!$db->supportsTransactions()) { - $this->print('Database transactions are not supported for this database. Errors may result in a partially-migrated state.'); - } - - $db->withTransaction([$this, 'performMigration'], [$this, 'failedTransaction']); - - if ($request->getVar('skipBrokenLinks')) { - $this->print('Skipping broken link check as requested.'); - } else { - $this->checkForBrokenLinks(); - } - - $this->print('Done.'); - } - - /** - * Used in a callback if there is an error with the migration that causes a rolled back DB transaction - */ - public function failedTransaction() - { - if (DB::get_conn()->supportsTransactions()) { - $this->print('There was an error with the migration. Rolling back.'); - } - } - /** * Perform the actual data migration and publish links as appropriate */ - public function performMigration() + public function performMigration(): void { + $this->extend('beforePerformMigration'); // Migrate data $this->migrateTitleColumn(); $this->migrateHasManyRelations(); @@ -150,14 +82,16 @@ public function performMigration() $this->print('-----------------'); $this->print('Migration completed successfully.'); $this->print('-----------------'); + $this->extend('afterPerformMigration'); } /** * Check if we actually need to migrate anything, and if not give clear output as to why not. */ - private function getNeedsMigration(string $baseTable): bool + private function getNeedsMigration(): bool { $needsMigration = false; + $baseTable = DataObject::getSchema()->baseDataTable(Link::class); $needColumns = ['LinkText', 'Title']; $baseDbColumns = array_keys(DB::field_list($baseTable)); $baseNeededColumns = array_intersect($needColumns, $baseDbColumns); @@ -285,7 +219,7 @@ private function migrateHasManyRelations(): void foreach ($relationData as $hasManyRelation => $spec) { $linkClass = $spec['linkClass']; $hasOneRelation = $spec['hasOne']; - // Skip if the has_one relation still exists + // Stop migration if the has_one relation still exists if (array_key_exists($hasOneRelation, $this->getLinkRelationData($linkClass, 'has_one'))) { throw new RuntimeException("has_one relation '{$linkClass}.{$hasOneRelation} still exists. Cannot migrate has_many relation '{$ownerClass}.{$hasManyRelation}'."); }; @@ -351,340 +285,8 @@ private function migrateHasManyRelations(): void $this->extend('afterMigrateHasManyRelations'); } - /** - * Find all `has_one` relations to link and set the corresponding `Owner` relation - */ - private function setOwnerForHasOneLinks(): void - { - $this->extend('beforeSetOwnerForHasOneLinks'); - $this->print('Setting owners for has_one relations.'); - $allDataObjectModels = ClassInfo::subclassesFor(DataObject::class, false); - $allLinkModels = ClassInfo::subclassesFor(Link::class, true); - foreach ($allDataObjectModels as $modelClass) { - if ($this->shouldSkipClassForOwnerCheck($modelClass)) { - continue; - } - $hasOnes = Config::forClass($modelClass)->uninherited('has_one') ?? []; - foreach ($hasOnes as $hasOneName => $spec) { - // Get the class of the has_one - $hasOneClass = $spec['class'] ?? null; - if (!is_array($spec)) { - $hasOneClass = $spec; - $spec = ['class' => $hasOneClass]; - } - - // Skip malformed has_one relations - if ($hasOneClass === null) { - continue; - } - - // Polymorphic has_one needs some extra handling - if ($hasOneClass === DataObject::class) { - if ($this->hasReciprocalRelation($allLinkModels, $hasOneName, $modelClass)) { - continue; - } - $this->updateOwnerForRelation(Link::class, $hasOneName, $modelClass, $spec); - continue; - } - - // Skip if the has_one isn't for Link, or points at a belongs_to or has_many on Link - if (!is_a($hasOneClass, Link::class, true)) { - continue; - } - if ($this->hasReciprocalRelation([$hasOneClass], $hasOneName, $modelClass)) { - continue; - } - - // Update Owner for the relevant links to point at this relation - $this->updateOwnerForRelation($hasOneClass, $hasOneName, $modelClass); - } - } - $this->extend('afterSetOwnerForHasOneLinks'); - } - - private function shouldSkipClassForOwnerCheck(string $modelClass): bool - { - // This is a workaround for tests, since ClassInfo will get info about all TestOnly classes, - // even if they're not in your test class's "extra_dataobjects" list. - // Some classes don't have tables and don't NEED tables - but those classes also - // won't declare has_one relations, so it's okay to skip those too. - if (!ClassInfo::hasTable(DataObject::getSchema()->tableName($modelClass))) { - return true; - } - // Skip class hierarchies that we explicitly said we want to skip - $classHierarchiesToSkip = static::config()->get('classes_that_are_not_link_owners') ?? []; - foreach ($classHierarchiesToSkip as $skipClass) { - if (is_a($modelClass, $skipClass, true)) { - return true; - } - } - return false; - } - - /** - * Store relation data in memory so we're not hitting config over and over again unnecessarily. - * The task is likely run in CLI which relies on filesystem cache for config. - */ - private function getLinkRelationData(string $linkClass, string $configName): array - { - if (!isset($this->linkRelationData[$linkClass][$configName])) { - $config = Config::forClass($linkClass); - $this->linkRelationData[$linkClass][$configName] = $config->uninherited($configName) ?? []; - } - return $this->linkRelationData[$linkClass][$configName]; - } - - private function hasReciprocalRelation(array $linkClasses, string $hasOneName, string $foreignClass): bool - { - foreach ($linkClasses as $linkClass) { - $relationData = array_merge( - $this->getLinkRelationData($linkClass, 'belongs_to'), - $this->getLinkRelationData($linkClass, 'has_many'), - ); - // Check if the given link class has a belongs_to or has_many pointing at the has_one relation - // we're asking about - foreach ($relationData as $relationName => $value) { - $parsedRelation = $this->parseRelationData($value); - - if ($foreignClass !== $parsedRelation['class']) { - continue; - } - - // If we can't tell what relation the belongs_to or has_many points at, - // assume it's for the relation we're asking about - if ($parsedRelation['reciprocalRelation'] === null) { - // Printing so developers can double check after the task is run. - // They can manually set the owner if it turns out our assumption was wrong. - // Not adding an extension point here because developers should use dot notation for the relation instead - // of working around their ambiguous relation declaration. - $this->print("Ambiguous relation '{$linkClass}.{$relationName}' found - assuming it points at '{$foreignClass}.{$hasOneName}'"); - return true; - } - - if ($hasOneName !== $parsedRelation['reciprocalRelation']) { - continue; - } - - // If we get here, then the relation points back at the has_one we're - // checking against. - return true; - } - } - return false; - } - - /** - * Parses a belongs_to or has_many relation class to separate the class from - * the reciprocal relation name. - * - * Modified from RelationValidationService in framework. - */ - private function parseRelationData(string $relationData): array - { - if (mb_strpos($relationData ?? '', '.') === false) { - return [ - 'class' => $relationData, - 'reciprocalRelation' => null, - ]; - } - - $segments = explode('.', $relationData ?? ''); - - // Theoretically this is the same as the mb_strpos check above, - // but both checks are in RelationValidationService so I'm leaving - // this here in case there's some edge case it's covering. - if (count($segments) !== 2) { - return [ - 'class' => $relationData, - 'reciprocalRelation' => null, - ]; - } - - $class = array_shift($segments); - $relation = array_shift($segments); - return [ - 'class' => $class, - 'reciprocalRelation' => $relation, - ]; - } - - /** - * Bulk update the owner for links stored in a has_one relation - */ - private function updateOwnerForRelation(string $linkClass, string $hasOneName, string $foreignClass, array $polymorphicSpec = []): void - { - $db = DB::get_conn(); - $schema = DataObject::getSchema(); - $isPolymorphic = !empty($polymorphicSpec); - - $ownerIdColumn = $schema->sqlColumnForField($linkClass, 'OwnerID'); - $ownerClassColumn = $schema->sqlColumnForField($linkClass, 'OwnerClass'); - $ownerRelationColumn = $schema->sqlColumnForField($linkClass, 'OwnerRelation'); - $linkIdColumn = $schema->sqlColumnForField($linkClass, 'ID'); - $relationIdColumn = $schema->sqlColumnForField($foreignClass, "{$hasOneName}ID"); - - $nullCheck = $db->nullCheckClause($ownerIdColumn, true); - $baseTable = $schema->tableForField($linkClass, 'OwnerID'); - $update = SQLUpdate::create( - $db->escapeIdentifier($baseTable), - [ - $ownerIdColumn => [$schema->sqlColumnForField($foreignClass, 'ID') => []], - $ownerClassColumn => [$schema->sqlColumnForField($foreignClass, 'ClassName') => []], - $ownerRelationColumn => $hasOneName, - ], - [ - $linkIdColumn . ' = ' . $relationIdColumn, - // Only set the owner if it isn't already set - // Don't check class here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 - "$ownerIdColumn = 0 OR $nullCheck", - $db->nullCheckClause($ownerRelationColumn, true), - ] - ); - // Join the table for $foreignClass - $foreignClassTable = $schema->tableName($foreignClass); - if ($foreignClassTable !== $baseTable) { - $update->addInnerJoin($foreignClassTable, $relationIdColumn . ' = ' . $linkIdColumn); - // If the table for $foreignClass is not its base table, we need to join that as well - // so we can get the ID and classname. - $baseForeignTable = $schema->baseDataTable($foreignClass); - if (!$update->isJoinedTo($baseForeignTable)) { - $update->addInnerJoin( - $baseForeignTable, - $db->escapeIdentifier($baseForeignTable . '.ID') . ' = ' . $db->escapeIdentifier($foreignClassTable . '.ID') - ); - } - // Add join and where clauses for polymorphic relations so we don't set the wrong owners - if ($isPolymorphic) { - $relationClassColumn = $schema->sqlColumnForField($foreignClass, "{$hasOneName}Class"); - $linkClassColumn = $schema->sqlColumnForField($linkClass, 'ClassName'); - $update->addFilterToJoin($foreignClassTable, $relationClassColumn . ' = ' . $linkClassColumn); - // Make sure we ignore any multi-relational has_one pointing at something other than Link.Owner - if ($polymorphicSpec[DataObjectSchema::HAS_ONE_MULTI_RELATIONAL] ?? false) { - $update->addWhere([$schema->sqlColumnForField($foreignClass, "{$hasOneName}Relation") => 'Owner']); - } - } - } - $update->execute(); - } - - /** - * Publishes links unless Link isn't versioned or developers opt out. - */ - private function publishLinks(): void - { - if (Link::has_extension(Versioned::class)) { - $shouldPublishLinks = true; - $this->extend('updateShouldPublishLinks', $shouldPublishLinks); - if ($shouldPublishLinks) { - $this->print('Publishing links.'); - /** @var Versioned&Link $link */ - foreach (Link::get()->chunkedFetch() as $link) { - // Allow developers to skip publishing each link - this allows for scenarios - // where links were Versioned in v2/v3 projects. - $shouldPublishLink = true; - $this->extend('updateShouldPublishLink', $link, $shouldPublishLink); - if ($shouldPublishLink) { - $link->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); - } - $link->destroy(); - } - $this->print('Publishing complete.'); - } else { - $this->print('Skipping publish step.'); - } - } else { - $this->print('Links are not versioned - skipping publish step due to project-level customisation.'); - } - } - - /** - * Check for broken links and output information about them. - * Doesn't actually check if file or page exists for those link types, - * this is just about whether there's data there or not. - */ - private function checkForBrokenLinks(): void - { - $this->print('Checking for broken links.'); - // Using draft stage is safe for unversioned links, and ensures we - // get all relevant data for versioned but unpublished links. - Versioned::withVersionedMode(function () { - Versioned::set_reading_mode('Stage.' . Versioned::DRAFT); - $checkForBrokenLinks = [ - EmailLink::class => [ - 'field' => 'Email', - 'emptyValue' => [null, ''], - ], - ExternalLink::class => [ - 'field' => 'ExternalUrl', - 'emptyValue' => [null, ''], - ], - FileLink::class => [ - 'field' => 'FileID', - 'emptyValue' => [null, 0], - ], - PhoneLink::class => [ - 'field' => 'Phone', - 'emptyValue' => [null, ''], - ], - SiteTreeLink::class => [ - 'field' => 'PageID', - 'emptyValue' => [null, 0], - ], - ]; - $this->extend('updateCheckForBrokenLinks', $checkForBrokenLinks); - $brokenLinks = []; - foreach ($checkForBrokenLinks as $class => $data) { - $field = $data['field']; - $emptyValue = $data['emptyValue']; - $ids = DataObject::get($class)->filter([$field => $emptyValue])->column('ID'); - $numBroken = count($ids); - $this->print("Found $numBroken broken links for the '$class' class."); - if ($numBroken > 0) { - $brokenLinks[$class] = $ids; - } - } - - if (empty($brokenLinks)) { - $this->print('No broken links.'); - return; - } - - // Output table of broken links - $this->print('Broken links:'); - if (Director::is_cli()) { - // Output in a somewhat CLI friendly table. - // Pad by the length of the longest class name so things align nicely. - $longestClassLen = max(array_map('strlen', array_keys($brokenLinks))); - $paddedClassTitle = str_pad('Link class', $longestClassLen); - $classSeparator = str_repeat('-', $longestClassLen); - $output = <<< CLI_TABLE - $paddedClassTitle | IDs of broken links - $classSeparator | ------------------- - CLI_TABLE; - foreach ($brokenLinks as $class => $ids) { - $paddedClass = str_pad($class, $longestClassLen); - $idsString = implode(', ', $ids); - $output .= "\n$paddedClass | $idsString"; - } - } else { - // Output as an HTML table - $output = ''; - foreach ($brokenLinks as $class => $ids) { - $idsString = implode(', ', $ids); - $output .= ""; - } - $output .= '
Link classIDs of broken links
$class$idsString
'; - } - $this->print($output); - }); - } - - /** - * A convenience method for printing a line to the browser or terminal with appropriate line breaks. - */ - private function print(string $message): void + private function classIsOldLink(string $class): bool { - $eol = Director::is_cli() ? "\n" : '
'; - echo $message . $eol; + return is_a($class, Link::class, true); } } diff --git a/src/Tasks/MigrationTaskTrait.php b/src/Tasks/MigrationTaskTrait.php new file mode 100644 index 00000000..a21301fa --- /dev/null +++ b/src/Tasks/MigrationTaskTrait.php @@ -0,0 +1,437 @@ + Deprecation::notice('4.0.0', 'Will be removed without equivalent functionality.', Deprecation::SCOPE_CLASS) + ); + parent::__construct(); + } + + public function run($request): void + { + $db = DB::get_conn(); + + // If we don't need to migrate, exit early. + if (!$this->getNeedsMigration()) { + $this->print('Cannot perform migration.'); + return; + } + + if (!$db->supportsTransactions()) { + $this->print('Database transactions are not supported for this database. Errors may result in a partially-migrated state.'); + } + + $db->withTransaction([$this, 'performMigration'], [$this, 'failedTransaction']); + + if ($request->getVar('skipBrokenLinks')) { + $this->print('Skipping broken link check as requested.'); + } else { + $this->checkForBrokenLinks(); + } + + $this->print('Done.'); + } + + /** + * Used in a callback if there is an error with the migration that causes a rolled back DB transaction + */ + public function failedTransaction(): void + { + if (DB::get_conn()->supportsTransactions()) { + $this->print('There was an error with the migration. Rolling back.'); + } + } + + /** + * Find all `has_one` relations to link and set the corresponding `Owner` relation + */ + private function setOwnerForHasOneLinks(): void + { + $this->extend('beforeSetOwnerForHasOneLinks'); + $this->print('Setting owners for has_one relations.'); + $allDataObjectModels = ClassInfo::subclassesFor(DataObject::class, false); + $allLinkModels = ClassInfo::subclassesFor(Link::class, true); + foreach ($allDataObjectModels as $modelClass) { + if ($this->shouldSkipClassForOwnerCheck($modelClass)) { + continue; + } + $hasOnes = Config::forClass($modelClass)->uninherited('has_one') ?? []; + foreach ($hasOnes as $hasOneName => $spec) { + // Get the class of the has_one + $hasOneClass = $spec['class'] ?? null; + if (!is_array($spec)) { + $hasOneClass = $spec; + $spec = ['class' => $hasOneClass]; + } + + // Skip malformed has_one relations + if ($hasOneClass === null) { + continue; + } + + // Polymorphic has_one needs some extra handling + if ($hasOneClass === DataObject::class) { + if ($this->hasReciprocalRelation($allLinkModels, $hasOneName, $modelClass)) { + continue; + } + $this->updateOwnerForRelation(Link::class, $hasOneName, $modelClass, $spec); + continue; + } + + // Skip if the has_one isn't for Link, or points at a belongs_to or has_many on Link + if (!$this->classIsOldLink($hasOneClass)) { + continue; + } + if ($this->hasReciprocalRelation([$hasOneClass], $hasOneName, $modelClass)) { + continue; + } + + // Update Owner for the relevant links to point at this relation + $this->updateOwnerForRelation($hasOneClass, $hasOneName, $modelClass); + } + } + $this->extend('afterSetOwnerForHasOneLinks'); + } + + /** + * Bulk update the owner for links stored in a has_one relation + */ + private function updateOwnerForRelation(string $linkClass, string $hasOneName, string $foreignClass, array $polymorphicSpec = []): void + { + $db = DB::get_conn(); + $schema = DataObject::getSchema(); + $isPolymorphic = !empty($polymorphicSpec); + + $ownerIdColumn = $schema->sqlColumnForField($linkClass, 'OwnerID'); + $ownerClassColumn = $schema->sqlColumnForField($linkClass, 'OwnerClass'); + $ownerRelationColumn = $schema->sqlColumnForField($linkClass, 'OwnerRelation'); + $linkIdColumn = $schema->sqlColumnForField($linkClass, 'ID'); + $relationIdColumn = $schema->sqlColumnForField($foreignClass, "{$hasOneName}ID"); + + $nullCheck = $db->nullCheckClause($ownerIdColumn, true); + $baseTable = $schema->tableForField($linkClass, 'OwnerID'); + $update = SQLUpdate::create( + $db->escapeIdentifier($baseTable), + [ + $ownerIdColumn => [$schema->sqlColumnForField($foreignClass, 'ID') => []], + $ownerClassColumn => [$schema->sqlColumnForField($foreignClass, 'ClassName') => []], + $ownerRelationColumn => $hasOneName, + ], + [ + $linkIdColumn . ' = ' . $relationIdColumn, + // Only set the owner if it isn't already set + // Don't check class here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + "$ownerIdColumn = 0 OR $nullCheck", + $db->nullCheckClause($ownerRelationColumn, true), + ] + ); + // Join the table for $foreignClass + $foreignClassTable = $schema->tableName($foreignClass); + if ($foreignClassTable !== $baseTable) { + $update->addInnerJoin($foreignClassTable, $relationIdColumn . ' = ' . $linkIdColumn); + // If the table for $foreignClass is not its base table, we need to join that as well + // so we can get the ID and classname. + $baseForeignTable = $schema->baseDataTable($foreignClass); + if (!$update->isJoinedTo($baseForeignTable)) { + $update->addInnerJoin( + $baseForeignTable, + $db->escapeIdentifier($baseForeignTable . '.ID') . ' = ' . $db->escapeIdentifier($foreignClassTable . '.ID') + ); + } + // Add join and where clauses for polymorphic relations so we don't set the wrong owners + if ($isPolymorphic) { + $relationClassColumn = $schema->sqlColumnForField($foreignClass, "{$hasOneName}Class"); + $linkClassColumn = $schema->sqlColumnForField($linkClass, 'ClassName'); + $update->addFilterToJoin($foreignClassTable, $relationClassColumn . ' = ' . $linkClassColumn); + // Make sure we ignore any multi-relational has_one pointing at something other than Link.Owner + if ($polymorphicSpec[DataObjectSchema::HAS_ONE_MULTI_RELATIONAL] ?? false) { + $update->addWhere([$schema->sqlColumnForField($foreignClass, "{$hasOneName}Relation") => 'Owner']); + } + } + } + $update->execute(); + } + + private function shouldSkipClassForOwnerCheck(string $modelClass): bool + { + // This is a workaround for tests, since ClassInfo will get info about all TestOnly classes, + // even if they're not in your test class's "extra_dataobjects" list. + // Some classes don't have tables and don't NEED tables - but those classes also + // won't declare has_one relations, so it's okay to skip those too. + if (!ClassInfo::hasTable(DataObject::getSchema()->tableName($modelClass))) { + return true; + } + // Skip class hierarchies that we explicitly said we want to skip + $classHierarchiesToSkip = static::config()->get('classes_that_are_not_link_owners') ?? []; + foreach ($classHierarchiesToSkip as $skipClass) { + if (is_a($modelClass, $skipClass, true)) { + return true; + } + } + return false; + } + + /** + * Store relation data in memory so we're not hitting config over and over again unnecessarily. + * The task is likely run in CLI which relies on filesystem cache for config. + */ + private function getLinkRelationData(string $linkClass, string $configName): array + { + if (!isset($this->linkRelationData[$linkClass][$configName])) { + $config = Config::forClass($linkClass); + $this->linkRelationData[$linkClass][$configName] = $config->uninherited($configName) ?? []; + } + return $this->linkRelationData[$linkClass][$configName]; + } + + private function hasReciprocalRelation(array $linkClasses, string $hasOneName, string $foreignClass): bool + { + foreach ($linkClasses as $linkClass) { + $relationData = array_merge( + $this->getLinkRelationData($linkClass, 'belongs_to'), + $this->getLinkRelationData($linkClass, 'has_many'), + ); + // Check if the given link class has a belongs_to or has_many pointing at the has_one relation + // we're asking about + foreach ($relationData as $relationName => $value) { + $parsedRelation = $this->parseRelationData($value); + + if ($foreignClass !== $parsedRelation['class']) { + continue; + } + + // If we can't tell what relation the belongs_to or has_many points at, + // assume it's for the relation we're asking about + if ($parsedRelation['reciprocalRelation'] === null) { + // Printing so developers can double check after the task is run. + // They can manually set the owner if it turns out our assumption was wrong. + // Not adding an extension point here because developers should use dot notation for the relation instead + // of working around their ambiguous relation declaration. + $this->print("Ambiguous relation '{$linkClass}.{$relationName}' found - assuming it points at '{$foreignClass}.{$hasOneName}'"); + return true; + } + + if ($hasOneName !== $parsedRelation['reciprocalRelation']) { + continue; + } + + // If we get here, then the relation points back at the has_one we're + // checking against. + return true; + } + } + return false; + } + + /** + * Parses a belongs_to or has_many relation class to separate the class from + * the reciprocal relation name. + * + * Modified from RelationValidationService in framework. + */ + private function parseRelationData(string $relationData): array + { + if (mb_strpos($relationData ?? '', '.') === false) { + return [ + 'class' => $relationData, + 'reciprocalRelation' => null, + ]; + } + + $segments = explode('.', $relationData ?? ''); + + // Theoretically this is the same as the mb_strpos check above, + // but both checks are in RelationValidationService so I'm leaving + // this here in case there's some edge case it's covering. + if (count($segments) !== 2) { + return [ + 'class' => $relationData, + 'reciprocalRelation' => null, + ]; + } + + $class = array_shift($segments); + $relation = array_shift($segments); + return [ + 'class' => $class, + 'reciprocalRelation' => $relation, + ]; + } + + /** + * Publishes links unless Link isn't versioned or developers opt out. + */ + private function publishLinks(): void + { + if (Link::has_extension(Versioned::class)) { + $shouldPublishLinks = true; + $this->extend('updateShouldPublishLinks', $shouldPublishLinks); + if ($shouldPublishLinks) { + $this->print('Publishing links.'); + /** @var Versioned&Link $link */ + foreach (Link::get()->chunkedFetch() as $link) { + // Allow developers to skip publishing each link - this allows for scenarios + // where links were Versioned in v2/v3 projects. + $shouldPublishLink = true; + $this->extend('updateShouldPublishLink', $link, $shouldPublishLink); + if ($shouldPublishLink) { + $link->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); + } + $link->destroy(); + } + $this->print('Publishing complete.'); + } else { + $this->print('Skipping publish step.'); + } + } else { + $this->print('Links are not versioned - skipping publish step due to project-level customisation.'); + } + } + + /** + * Check for broken links and output information about them. + * Doesn't actually check if file or page exists for those link types, + * this is just about whether there's data there or not. + */ + private function checkForBrokenLinks(): void + { + $this->print('Checking for broken links.'); + // Using draft stage is safe for unversioned links, and ensures we + // get all relevant data for versioned but unpublished links. + Versioned::withVersionedMode(function () { + Versioned::set_reading_mode('Stage.' . Versioned::DRAFT); + $checkForBrokenLinks = [ + EmailLink::class => [ + 'field' => 'Email', + 'emptyValue' => [null, ''], + ], + ExternalLink::class => [ + 'field' => 'ExternalUrl', + 'emptyValue' => [null, ''], + ], + FileLink::class => [ + 'field' => 'FileID', + 'emptyValue' => [null, 0], + ], + PhoneLink::class => [ + 'field' => 'Phone', + 'emptyValue' => [null, ''], + ], + SiteTreeLink::class => [ + 'field' => 'PageID', + 'emptyValue' => [null, 0], + ], + ]; + $this->extend('updateCheckForBrokenLinks', $checkForBrokenLinks); + $brokenLinks = []; + foreach ($checkForBrokenLinks as $class => $data) { + $field = $data['field']; + $emptyValue = $data['emptyValue']; + $ids = DataObject::get($class)->filter([$field => $emptyValue])->column('ID'); + $numBroken = count($ids); + $this->print("Found $numBroken broken links for the '$class' class."); + if ($numBroken > 0) { + $brokenLinks[$class] = $ids; + } + } + + if (empty($brokenLinks)) { + $this->print('No broken links.'); + return; + } + + // Output table of broken links + $this->print('Broken links:'); + if (Director::is_cli()) { + // Output in a somewhat CLI friendly table. + // Pad by the length of the longest class name so things align nicely. + $longestClassLen = max(array_map('strlen', array_keys($brokenLinks))); + $paddedClassTitle = str_pad('Link class', $longestClassLen); + $classSeparator = str_repeat('-', $longestClassLen); + $output = <<< CLI_TABLE + $paddedClassTitle | IDs of broken links + $classSeparator | ------------------- + CLI_TABLE; + foreach ($brokenLinks as $class => $ids) { + $paddedClass = str_pad($class, $longestClassLen); + $idsString = implode(', ', $ids); + $output .= "\n$paddedClass | $idsString"; + } + } else { + // Output as an HTML table + $output = ''; + foreach ($brokenLinks as $class => $ids) { + $idsString = implode(', ', $ids); + $output .= ""; + } + $output .= '
Link classIDs of broken links
$class$idsString
'; + } + $this->print($output); + }); + } + + /** + * A convenience method for printing a line to the browser or terminal with appropriate line breaks. + */ + private function print(string $message): void + { + $eol = Director::is_cli() ? "\n" : '
'; + echo $message . $eol; + } + + /** + * Perform the actual data migration and publish links as appropriate + */ + abstract public function performMigration(): void; + + /** + * Check if we actually need to migrate anything, and if not give clear output as to why not. + */ + abstract private function getNeedsMigration(): bool; + + /** + * Returns true if the class represents an old link to be migrated + */ + abstract private function classIsOldLink(string $class): bool; +} diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.php b/tests/php/Tasks/GorriecoeMigrationTaskTest.php new file mode 100644 index 00000000..9af25cf1 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.php @@ -0,0 +1,441 @@ + ExternalLink::class, + 'Email' => EmailLink::class, + 'Phone' => PhoneLink::class, + 'File' => FileLink::class, + 'SiteTree' => SiteTreeLink::class, + 'Custom' => CustomLink::class, + ]; + + protected static $fixture_file = 'GorriecoeMigrationTaskTest.yml'; + + protected static $extra_dataobjects = [ + CustomLink::class, + HasManyLinkOwner::class, + LinkOwner::class, + WasManyManyJoinModel::class, + WasManyManyOwner::class, + ]; + + /** + * Required because of the use of fixtures with a custom table. + * Without this, the table (and its fixtures) won't be recreated after each test + * so any test that tears down the table would cause future tests to fail. + */ + protected $usesTransactions = false; + + protected function setUp(): void + { + parent::setUp(); + // Add custom link config + GorriecoeMigrationTask::config()->merge('link_type_columns', [ + 'Custom' => [ + 'class' => CustomLink::class, + 'fields' => [ + 'CustomField' => 'MyField', + ], + ], + ]); + GorriecoeMigrationTask::config()->merge('base_link_columns', [ + 'MySort' => 'Sort', + ]); + } + + public function onBeforeLoadFixtures(): void + { + GorriecoeMigrationTask::config()->set('old_link_table', self::OLD_LINK_TABLE); + // Set up migration tables + DB::get_schema()->schemaUpdate(function () { + // Old link table + $linkDbColumns = [ + ...DataObject::config()->uninherited('fixed_fields'), + // Fields directly from the Link class + 'Title' => 'Varchar', + 'Type' => 'Varchar(50)', + 'URL' => 'Text', + 'Email' => 'Varchar', + 'Phone' => 'Varchar(30)', + 'OpenInNewWindow' => 'Boolean', + 'SelectedStyle' => 'Varchar', + 'FileID' => 'ForeignKey', + // Fields from the LinkSiteTree extension + 'Anchor' => 'Varchar(255)', + 'SiteTreeID' => 'ForeignKey', + // Field for a custom link type + 'CustomField' => 'Varchar', + // Field for custom sort + 'MySort' => 'Int', + ]; + DB::require_table(self::OLD_LINK_TABLE, $linkDbColumns, options: DataObject::config()->get('create_table_options')); + // many_many tables + $schema = DataObject::getSchema(); + $ownerTable = $schema->tableName(WasManyManyOwner::class); + $normalJoinColumns = [ + "{$ownerTable}ID" => 'ForeignKey', + self::OLD_LINK_TABLE . 'ID' => 'ForeignKey', + 'CustomSort' => 'Int', + ]; + DB::require_table("{$ownerTable}_NormalManyMany", $normalJoinColumns, options: DataObject::config()->get('create_table_options')); + $throughJoinColumns = [ + 'OldOwnerID' => 'ForeignKey', + 'OldLinkID' => 'ForeignKey', + 'CustomSort' => 'Int', + ]; + DB::require_table('GorriecoeMigrationTaskTest_manymany_through', $throughJoinColumns, options: DataObject::config()->get('create_table_options')); + $throughPolymorphicJoinColumns = [ + ...$throughJoinColumns, + // technically it would be a DBClassName enum but this is easier and the actual type doesn't matter + 'OldOwnerClass' => 'Varchar', + ]; + DB::require_table('GorriecoeMigrationTaskTest_manymany_throughpoly', $throughPolymorphicJoinColumns, options: DataObject::config()->get('create_table_options')); + }); + parent::onBeforeLoadFixtures(); + } + + public function provideGetNeedsMigration(): array + { + return [ + 'no old table' => [ + 'hasTable' => false, + 'expected' => false, + ], + 'original old table' => [ + 'hasTable' => true, + 'expected' => true, + ], + 'obsolete old table' => [ + 'hasTable' => 'obsolete', + 'expected' => true, + ], + ]; + } + + /** + * @dataProvider provideGetNeedsMigration + */ + public function testGetNeedsMigration(string|bool $hasTable, bool $expected): void + { + if ($hasTable === false) { + DB::query('DROP TABLE "'. self::OLD_LINK_TABLE .'"'); + } elseif ($hasTable === 'obsolete') { + $this->startCapturingOutput(); + DB::get_schema()->schemaUpdate(function () { + DB::dont_require_table(self::OLD_LINK_TABLE); + }); + $this->stopCapturingOutput(); + } + + $this->startCapturingOutput(); + $result = $this->callPrivateMethod('getNeedsMigration'); + $output = $this->stopCapturingOutput(); + $this->assertSame($expected, $result); + $this->assertSame($expected ? '' : "Nothing to migrate - old link table doesn't exist.\n", $output); + } + + public function testInsertBaseRows(): void + { + // Remove existing links which can cause ID conflicts. + // Note they would have already caused the migration to abort before this point. + Link::get()->removeAll(); + + // Insert the rows + $this->startCapturingOutput(); + $this->callPrivateMethod('insertBaseRows'); + $output = $this->stopCapturingOutput(); + + $select = new SQLSelect(from: DB::get_conn()->escapeIdentifier(DataObject::getSchema()->baseDataTable(Link::class))); + foreach ($select->execute() as $link) { + // Skip any links that already existed + if (str_starts_with($link['LinkText'], 'pre-existing')) { + continue; + } + // The owner class is likely to be some arbitrary model - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + unset($link['OwnerClass']); + $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE), where: ['ID' => $link['ID']]); + $oldLinkData = $oldLinkSelect->execute()->record(); + $expectedDataForLink = [ + 'ID' => $oldLinkData['ID'], + 'ClassName' => self::TYPE_MAP[$oldLinkData['Type']], + 'LastEdited' => $oldLinkData['LastEdited'], + 'Created' => $oldLinkData['Created'], + 'LinkText' => $oldLinkData['Title'], + 'OpenInNew' => $oldLinkData['OpenInNewWindow'], + 'Sort' => $oldLinkData['MySort'], + // All of the below are just left as the default values + 'OwnerID' => 0, + 'OwnerRelation' => null, + 'Version' => 0, + ]; + ksort($expectedDataForLink); + ksort($link); + $this->assertSame($expectedDataForLink, $link); + } + + $this->assertEmpty($output); + } + + public function testInsertTypeSpecificRows(): void + { + // Remove existing links which can cause ID conflicts. + // Note they would have already caused the migration to abort before this point. + Link::get()->removeAll(); + // This test is dependent on the base rows being inserted + $this->startCapturingOutput(); + $this->callPrivateMethod('insertBaseRows'); + $this->stopCapturingOutput(); + + // Insert the rows + $this->startCapturingOutput(); + $this->callPrivateMethod('insertTypeSpecificRows'); + $output = $this->stopCapturingOutput(); + + $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE)); + $oldLinkData = $oldLinkSelect->execute(); + $this->assertCount($oldLinkData->numRecords(), Link::get()); + + $typeColumnMaps = GorriecoeMigrationTask::config()->get('link_type_columns'); + foreach ($oldLinkData as $oldLink) { + $link = Link::get()->byID($oldLink['ID']); + $this->assertInstanceOf(self::TYPE_MAP[$oldLink['Type']], $link); + foreach ($typeColumnMaps[$oldLink['Type']]['fields'] as $oldField => $newField) { + $this->assertSame( + $oldLink[$oldField], + $link->$newField, + "'$newField' field on Link must be the same as '$oldField' field in the old table" + ); + } + } + + $this->assertEmpty($output); + } + + public function testUpdateSiteTreeRows(): void + { + // Remove existing links which can cause ID conflicts. + // Note they would have already caused the migration to abort before this point. + Link::get()->removeAll(); + // This test is dependent on the base and type-specific rows being inserted + $this->startCapturingOutput(); + $this->callPrivateMethod('insertBaseRows'); + $this->callPrivateMethod('insertTypeSpecificRows'); + $this->stopCapturingOutput(); + + // Update the rows + $this->startCapturingOutput(); + $this->callPrivateMethod('updateSiteTreeRows'); + $output = $this->stopCapturingOutput(); + + $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE)); + foreach (SiteTreeLink::get() as $link) { + $oldLinkSelect = new SQLSelect( + from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE), + where: ['ID' => $link->ID] + ); + $oldLink = $oldLinkSelect->execute()->record(); + $oldAnchor = $oldLink['Anchor']; + if ($oldAnchor === null) { + $anchor = null; + $queryString = null; + } elseif (str_starts_with($oldAnchor, '?')) { + $anchor = 'anchor-second'; + $queryString = 'querystring=first&awesome'; + } elseif (str_starts_with($oldAnchor, '#')) { + $anchor = 'anchor-first'; + $queryString = 'querystring=second&awesome'; + } else { + $anchor = 'this-will-be?treated&like-just-an-anchor=1#okiedoke'; + $queryString = null; + } + $this->assertSame($anchor, $link->Anchor, 'Anchor must be set correctly'); + $this->assertSame($queryString, $link->QueryString, 'Query string must be set correctly'); + } + + $this->assertEmpty($output); + } + + public function provideMigrateHasManyRelations(): array + { + return [ + 'no has_many' => [ + 'hasManyConfig' => [], + ], + 'regular has_one' => [ + 'hasManyConfig' => [ + HasManyLinkOwner::class => [ + 'RegularHasMany' => 'OldHasOne', + ], + ], + 'ownerFixture' => 'legacy-relations', + 'addColumns' => ['OldHasOneID' => DBInt::class], + ], + 'polymorphic has_one' => [ + 'hasManyConfig' => [ + HasManyLinkOwner::class => [ + 'PolyHasMany' => 'OldHasOne', + ], + ], + 'ownerFixture' => 'legacy-relations', + 'addColumns' => [ + 'OldHasOneID' => DBInt::class, + 'OldHasOneClass' => DBVarchar::class, + ], + ], + ]; + } + + /** + * @dataProvider provideMigrateHasManyRelations + */ + public function testMigrateHasManyRelations( + array $hasManyConfig, + string $ownerFixture = null, + array $addColumns = [] + ): void { + GorriecoeMigrationTask::config()->set('has_many_links_data', $hasManyConfig); + + if (!empty($addColumns) && !$ownerFixture) { + throw new LogicException('Test scenario is broken - need owner if we are adding columns.'); + } + + // Set up legacy has_one columns and data + if ($ownerFixture) { + $oldTable = self::OLD_LINK_TABLE; + DB::get_schema()->schemaUpdate(function () use ($oldTable, $addColumns) { + foreach ($addColumns as $column => $fieldType) { + $dbField = DBField::create_field($fieldType, null, $column); + $dbField->setTable($oldTable); + $dbField->requireField(); + } + }); + $db = DB::get_conn(); + $ownerClass = array_key_first($hasManyConfig); + $owner = $this->objFromFixture($ownerClass, $ownerFixture); + foreach (array_keys($addColumns) as $columnName) { + $value = str_ends_with($columnName, 'ID') ? $owner->ID : $owner->ClassName; + SQLUpdate::create( + $db->escapeIdentifier($oldTable), + [$db->escapeIdentifier("{$oldTable}.{$columnName}") => $value] + )->execute(); + } + } + + // Run the migration + $this->startCapturingOutput(); + $this->callPrivateMethod('migrateHasManyRelations'); + $output = $this->stopCapturingOutput(); + + if (empty($hasManyConfig)) { + $this->assertSame("No has_many relations to migrate.\n", $output); + return; + } + + $expectedOutput = "Migrating has_many relations.\n"; + + // Owner SHOULD have been set + foreach ($hasManyConfig as $ownerClass => $relationData) { + $owner = $this->objFromFixture($ownerClass, $ownerFixture); + foreach ($relationData as $hasManyRelation => $spec) { + $list = $owner->$hasManyRelation(); + // Check that the Owner relation got set correctly for these + $this->assertSame([$owner->ID], $list->columnUnique('OwnerID')); + $this->assertSame([$hasManyRelation], $list->columnUnique('OwnerRelation')); + $this->assertSame([$owner->ClassName], $list->columnUnique('OwnerClass')); + } + } + + $this->assertSame($expectedOutput, $output); + } + + public function provideMigrateManyManyRelations(): array + { + return [ + [ + 'table' => 'LinkFieldTest_Tasks_WasManyManyOwner_NormalManyMany', + ], + [ + 'table' => 'GorriecoeMigrationTaskTest_manymany_through', + ], + [ + 'table' => 'GorriecoeMigrationTaskTest_manymany_throughpoly', + ], + ]; + } + + /** + * @dataProvider provideMigrateManyManyRelations + */ + public function testMigrateManyManyRelations($table): void + { + /* + @TODO + many_many + many_many through + many_many through polymorphic + All of the above with duplicate links in the same relation + */ + } + + public function testMigrateManyManyRelationsJoinClassStillExists(): void + { + // @TODO many_many through but the join class still exists + } + + private function startCapturingOutput(): void + { + flush(); + ob_start(); + } + + private function stopCapturingOutput(): string + { + return ob_get_clean(); + } + + private function callPrivateMethod(string $methodName, array $args = []): mixed + { + $task = new GorriecoeMigrationTask(); + // getNeedsMigration() sets the table to pull from. + // If we're not testing that method, we need to set the table ourselves. + if ($this->getName() !== 'testGetNeedsMigration') { + $reflectionProperty = new ReflectionProperty($task, 'oldTableName'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($task, self::OLD_LINK_TABLE); + } + $reflectionMethod = new ReflectionMethod($task, $methodName); + $reflectionMethod->setAccessible(true); + return $reflectionMethod->invoke($task, ...$args); + } +} diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml new file mode 100644 index 00000000..8e43a753 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml @@ -0,0 +1,228 @@ +SilverStripe\LinkField\Models\EmailLink: + email-link01: + LinkText: 'pre-existing link 01' + Email: 'email@example.com' + email-link02: + LinkText: 'pre-existing link 02' + Email: 'another-email@example.com' + +SilverStripe\LinkField\Models\SiteTreeLink: + sitetree-link01: + LinkText: 'pre-existing link 01' + Anchor: 'an-anchor' + # Doesn't matter if this is a real page or not for our purposes + PageID: 7 + sitetree-link02: + LinkText: 'pre-existing link 02' + PageID: 1 + +GorriecoeMigrationTaskTest_OldLinkTable: + url-link01: + Title: 'url link 01' + Created: '2019-12-02T12:21:12' + LastEdited: '2019-12-23T12:21:18' + Type: 'URL' + URL: null + OpenInNewWindow: true + MySort: 1 + url-link02: + Title: 'url link 02' + Type: 'URL' + URL: '/some-relative-path/hahaha' + OpenInNewWindow: false + MySort: 2 + url-link03: + Title: 'url link03' + Type: 'URL' + URL: 'https://www.example.com/' + MySort: 3 + email-link01: + Title: 'email link 01' + Type: 'Email' + Email: null + OpenInNewWindow: true + email-link02: + Title: 'email link 02' + Type: 'Email' + Email: 'email@example.com' + OpenInNewWindow: false + email-link03: + Title: 'email link03' + Type: 'Email' + Email: 'email2@example.com' + phone-link01: + Title: 'phone link 01' + Type: 'Phone' + Phone: null + OpenInNewWindow: true + phone-link02: + Title: 'phone link 02' + Type: 'Phone' + Phone: '123456789' + OpenInNewWindow: false + phone-link03: + Title: 'phone link03' + Type: 'Phone' + Phone: '04-555-call-me' + file-link01: + Title: 'file link 01' + Type: 'File' + FileID: 0 + OpenInNewWindow: true + file-link02: + Title: 'file link 02' + Type: 'File' + FileID: 0 + OpenInNewWindow: false + file-link03: + Title: 'file link03' + Type: 'File' + # Doesn't matter if this is a real file or not for our purposes + FileID: 1 + sitetree-link01: + Title: 'sitetree link 01' + Type: 'SiteTree' + SiteTreeID: 0 + OpenInNewWindow: true + Anchor: null + sitetree-link02: + Title: 'sitetree link 02' + Type: 'SiteTree' + SiteTreeID: 0 + OpenInNewWindow: false + Anchor: 'this-will-be?treated&like-just-an-anchor=1#okiedoke' + sitetree-link03: + Title: 'sitetree link03' + Type: 'SiteTree' + # Doesn't matter if this is a real page or not for our purposes + SiteTreeID: 1 + Anchor: '#anchor-first?querystring=second&awesome' + sitetree-link04: + Title: 'sitetree link04' + Type: 'SiteTree' + Anchor: '?querystring=first&awesome#anchor-second' + custom-link01: + Title: 'custom link 01' + Type: 'Custom' + CustomField: null + OpenInNewWindow: true + custom-link02: + Title: 'custom link 02' + Type: 'Custom' + CustomField: 'Some value' + OpenInNewWindow: false + custom-link03: + Title: 'custom link03' + Type: 'Custom' + CustomField: 'another value' + +SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\HasManyLinkOwner: + # We can't add the relations here, because that would set them against a real has_one, but we want + # them to be added against columns that aren't added through the regular ORM to simulate legacy data + legacy-relations: + +# many_many relation migrations +SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner: + manymany-owner1: + Title: 'Owner 1' + manymany-owner2: + Title: 'Owner 2' + manymany-owner3: + Title: 'Owner 3' + +LinkFieldTest_Tasks_WasManyManyOwner_NormalManyMany: + normal-join1: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + normal-join2: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 2 + normal-join3: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link01 + CustomSort: 3 + normal-join4: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + normal-join5: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link02 + CustomSort: 2 + normal-join6: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 3 + normal-join7: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner3 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 4 + +GorriecoeMigrationTaskTest_manymany_through: + through-join1: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + through-join2: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 2 + through-join3: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link01 + CustomSort: 3 + through-join4: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + through-join5: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link02 + CustomSort: 2 + through-join6: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 3 + through-join7: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner3 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 4 + +GorriecoeMigrationTaskTest_manymany_throughpoly: + through-poly-join1: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + through-poly-join2: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 2 + through-poly-join3: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link01 + CustomSort: 3 + through-poly-join4: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + through-poly-join5: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link02 + CustomSort: 2 + through-poly-join6: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 3 + through-poly-join7: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner3 + OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 4 diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyJoinModel.php b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyJoinModel.php new file mode 100644 index 00000000..d31fb455 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyJoinModel.php @@ -0,0 +1,17 @@ + WasManyManyOwner::class, + 'Link' => Link::class, + ]; +} diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyOwner.php b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyOwner.php new file mode 100644 index 00000000..7e564716 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasManyManyOwner.php @@ -0,0 +1,26 @@ + 'Varchar', + ]; + + private static array $has_many = [ + 'NormalManyMany' => Link::class . '.Owner', + 'ManyManyThrough' => Link::class . '.Owner', + 'ManyManyThroughPolymorphic' => Link::class . '.Owner', + // These two are here just as a sanity check that additional relationships don't affect the task + 'LinkButNotIncluded' => Link::class . '.Owner', + 'NotLink' => SiteTree::class, + ]; +} diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest.php b/tests/php/Tasks/LinkFieldMigrationTaskTest.php index a3ef9dba..b03e5edc 100644 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest.php +++ b/tests/php/Tasks/LinkFieldMigrationTaskTest.php @@ -9,7 +9,6 @@ use SilverStripe\Core\Convert; use SilverStripe\Core\Environment; use SilverStripe\Core\Manifest\ClassLoader; -use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\SapphireTest; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; @@ -174,7 +173,7 @@ public function testGetNeedsMigration(bool $hasTitleColumn, bool $hasLinkTextCol OverrideMigrationStepsExtension::$needsMigration = $extensionOverride; $this->startCapturingOutput(); - $needsMigration = $this->callPrivateMethod('getNeedsMigration', [$baseTable]); + $needsMigration = $this->callPrivateMethod('getNeedsMigration'); $output = $this->stopCapturingOutput(); $this->assertSame($expected, $needsMigration); @@ -1117,7 +1116,7 @@ private function stopCapturingOutput(): string private function callPrivateMethod(string $methodName, array $args = []): mixed { - $task = Deprecation::withNoReplacement(fn() => new LinkFieldMigrationTask()); + $task = new LinkFieldMigrationTask(); $reflectionMethod = new ReflectionMethod($task, $methodName); $reflectionMethod->setAccessible(true); return $reflectionMethod->invoke($task, ...$args);