diff --git a/docs/en/09_migrating/00_upgrading.md b/docs/en/09_migrating/00_upgrading.md index 2e28fac1..efd1bb72 100644 --- a/docs/en/09_migrating/00_upgrading.md +++ b/docs/en/09_migrating/00_upgrading.md @@ -69,7 +69,7 @@ For databases that support transactions, the full data migration is performed wi > We strongly recommend running this task in a local development environment before trying it in production. > There may be edge cases that the migration task doesn't account for which need to be resolved. -1. Prepare the task +1. Configure the migration task - Enable the task: ```yml 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..bb477fe0 --- /dev/null +++ b/docs/en/09_migrating/02_gorriecoe-migration.md @@ -0,0 +1,294 @@ +--- +title: Migrating from gorriecoe/silverstripe-linkfield +summary: A guide for migrating from gorriecoe/silverstripe-linkfield +--- + +# Migrating from `gorriecoe/silverstripe-linkfield` + +> [!NOTE] +> If your site is running Silverstripe CMS 4.x, upgrade to CMS 5 first. +> Once you have finished upgrading to CMS 5, return to this guide and continue the linkfield upgrade. + +There are a few major changes between `gorriecoe/silverstripe-linkfield` and `silverstripe/linkfield`: + +1. Link types are defined via subclasses in `silverstripe/linkfield` as opposed to configuration within a single model. +1. `silverstripe/linkfield` doesn't support `many_many` relations - these will be migrated to `has_many` relations instead. +1. Many fields and relations have different names. +1. The default title for a link isn't stored in the database - if the `LinkText` field is left blank, nothing gets stored in the database for that field. + - This means any links migrated which had the default title set will be migrated with that title as explicit `LinkText`, which will not update automatically when you change the link URL. + - If you want the `LinkText` for those links to update automatically, you will need to either [customise the migration](#customising-the-migration) or manually unset the `LinkText` for those links afterward. + +The following additional items were identified as feature gaps, which may require some additional work to implement if you require them: + +- The phone number for `PhoneLink` isn't validated, except to ensure there is a value present. +- `PhoneLink` doesn't have template helper methods for its `Phone` database field. +- The `ExternalLink` type doesn't allow relative URLs. + - Any existing relative URLs will be migrated with their relative paths intact, but editing them will require updating them to be absolute URLs. +- There are no `addExtraClass()` or related methods for templates. If the default templates and CSS classnames don't suit your requirements you will need to override them. +- There are no `SiteTree` helpers like `isCurrent()`, `isOrphaned()` etc. You can call those methods on the `Page` relation in `SiteTreeLink` instead. +- There is no `link_to_folders` configuration - `FileLink` uses `UploadField` instead which doesn't allow linking to folders. +- There are no graphql helper methods or pre-existing graphql schema - just use regular GraphQL scaffolding if you need to fetch the links via GraphQL. +- You can't change the type of a link after creating it. +- The [`DefineableMarkupID`](https://github.com/elliot-sawyer/silverstripe-link/blob/master/src/extensions/DefineableMarkupID.php) and [`DBStringLink`](https://github.com/elliot-sawyer/silverstripe-link/blob/master/src/extensions/DBStringLink.php) classes have no equivalent in `silverstripe/linkfield`. + +This guide will help you migrate to `silverstripe/linkfield` and run a task that will automatically update your data. + +> [!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. + +## Setup + +1. Take a backup of your database +1. Remove the gorriecoe modules and add `silverstripe/linkfield`: + + ```bash + composer remove gorriecoe/silverstripe-link gorriecoe/silverstripe-linkfield + composer require silverstripe/linkfield:^4 + ``` + +1. 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: + - MyOwner: App\Model\MyClass + - belongs_many_many: + - BelongsRecord : App\Model\MyClass.LinkListTwo + ``` + +1. Update use statements and relations for the classes which own links. + - Any `many_many` relations should be swapped out for `has_many` relations, and all `has_many` relations should point to the `Owner` relation on the link class via dot notation. + - If the models that have `has_one` or `has_many` relations to link don't already use the `$owns` configuration for those relations, add that now. You may also want to set `$cascade_deletes` and `$cascade_duplicates` configuration. See [basic usage](../01_basic_usage.md) for more details. + + ```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 MyClass extends DataObject + { + private static array $has_one = [ + 'HasOneLink' => Link::class, + ]; + + private static array $has_many = [ + - 'LinkListOne' => Link::class . '.MyOwner', + + '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' => [ + - 'MySort' => 'Int', + - ] + - ]; + } + ``` + +1. If you had `many_many` through relation, delete the `DataObject` class which was used as the join table. +1. Update the usage of link fields. + + ```diff + public function getCMSFields() + { + $fields = parent::getCMSFields(); + + $fields->removeByName(['HasOneLinkID', '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)->setSortColumn('MySort'), + + LinkField::create('HasOneLink', 'Has one link'), + + MultiLinkField::create('LinkListOne', 'List list one'), + + MultiLinkField::create('LinkListTwo', 'Link list two'), + ] + ); + return $fields; + } + ``` + +1. If you applied [linkfield configuration](https://github.com/elliot-sawyer/silverstripe-linkfield?tab=readme-ov-file#configuration), update that now. + - 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); + ``` + +## Customising the migration + +There are many extension hooks in the [`GorriecoeMigrationTask`](api:SilverStripe\LinkField\Tasks\GorriecoeMigrationTask) class which you can use to change its behaviour or add additional migration steps. We strongly recommend taking a look at the source code to see if your use case requires any customisations. + +Some scenarios where you may need customisations include: + +- You had applied the [`Versioned`](api:SilverStripe\Versioned\Versioned) extension to `Link` and want to retain that versioning history +- You subclassed the base `Link` model and need to migrate data from your custom subclass +- You were relying on features of `gorriecoe/silverstripe-link` or `gorriecoe/silverstripe-linkfield` which don't have a 1-to-1 equivalent in `silverstripe/linkfield` + +Other customisations you may be using that will require manual migration or implementation include: + +- [gorriecoe/silverstripe-linkicon](https://github.com/gorriecoe/silverstripe-linkicon) +- [gorriecoe/silverstripe-ymlpresetlinks](https://github.com/gorriecoe/silverstripe-ymlpresetlinks) + +### Custom links + +If you have custom link implementations, you will need to implement an appropriate subclass of [`Link`](api:SilverStripe\LinkField\Models\Link) (or apply an extension to an existing one) with appropriate database columns and relations. + +You'll also need to add configuration to `GorriecoeMigrationTask` so it knows how to handle the migration from the old link to the new one: + +```yml +SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + link_type_columns: + # The name of the Type for your custom type as defined in gorriecoe/Link/Models/Link.types + MyCustomType: + # The FQCN for your new custom link subclass + class: 'App\Model\Link\MyCustomLink' + # An mapping of column names from the gorriecoe link to your link subclass + # Only include columns that are defined in the $db configuration for your subclass + fields: + MyOldField: 'MyNewField' +``` + +Some 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) + +## Migrating + +For databases that support transactions, the full data migration is performed within a single transaction, and any errors in the migration will result in rolling back all changes. This means you can address whatever caused the error and then run the task again. + +> [!NOTE] +> We strongly recommend running this task in a local development environment before trying it in production. +> There may be edge cases that the migration task doesn't account for which need to be resolved. + +1. Configure the migration task + - Enable the task: + + ```yml + SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + is_enabled: true + ``` + + - Declare any columns that you added to the gorriecoe link model which need to be migrated to the new base link table, for example if you added a custom sort column for your `has_many` relations: + + ```yml + SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + base_link_columns: + MySortColumn: 'Sort' + ``` + + - Declare any `has_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + has_many_links_data: + # The class where the has_many relation is declared + App\Model\MyClass: + # The key is the name of the has_many relation + # The value is the name of the old has_one relation on the gorriecoe link model + LinkListOne: 'MyOwner' + ``` + + - Declare any `many_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + many_many_links_data: + # The class where the many_many relation is declared + App\Model\MyClass: + # If it's a normal many_many relation with no extra fields, + # you can simply set the value to null and let the migration task figure it out + LinkListExample: null + # If the many_many is a many_many through, or had a $many_many_extraFields configuration defined, + # you will need to provide additional information + LinkListTwo: + # The table is required for many_many through + table: 'Page_ManyManyLinks' + # Extra fields is for $many_many_extraFields, or for any $db fields on a + # many_many through join model + extraFields: + MySort: 'Sort' + # For many_many through relations, you must add the names of the has_one relations + # from the DataObject which was used as the join model + through: + from: 'FromHasOneName', + to: 'ToHasOneName', + ``` + + - Declare any classes that may have `has_one` relations to `Link`, but which do not *own* the link. Classes declared here will include any subclasses. + For example if a custom link has a `has_many` relation to some class which does not own the link, declare that class here so it is not incorrectly identified as the owner of the link: + + ```yml + SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + classes_that_are_not_link_owners: + - App\Model\SomeClass + ``` + +1. Run dev/build and flush your cache (use the method you will be using the for next step - i.e. if you're running the task via the terminal, make sure to flush via the terminal) + - via the browser: `https://www.example.com/dev/build?flush=1` + - via a terminal: `sake dev/build flush=1` +1. Run the task + - via the browser: `https://www.example.com/dev/tasks/linkfield-tov4-migration-task` + - via a terminal: `sake dev/tasks/linkfield-tov4-migration-task` + +The task performs the following steps: + +1. Inserts new rows into the base link table, taking values from the old link table. +1. Inserts new rows into tables for link subclasses, taking values from the old link table. +1. Updates `SiteTreeLink` records, splitting out the old `Anchor` column into the separate `Anchor` and `QueryString` columns. +1. Migrates any `has_many` relations which were declared in [`GorriecoeMigrationTask.has_many_links_data`](api:SilverStripe\LinkField\Tasks\GorriecoeMigrationTask->has_many_links_data). +1. Migrates any `many_many` relations which were declared in in [`GorriecoeMigrationTask.many_many_links_data`](api:SilverStripe\LinkField\Tasks\GorriecoeMigrationTask->many_many_links_data) and drops the old join tables. +1. Set the `Owner` relation for `has_one` relations to links. +1. Drops the old link table. +1. Publishes all links, unless you have removed the `Versioned` extension. +1. Output a table with any links which are lacking the data required to generate a URL. + - You can skip this step by adding `?skipBrokenLinks=1` to the end of the URL: `https://www.example.com/dev/tasks/gorriecoe-to-linkfield-migration-task?skipBrokenLinks=1`. + - If you're running the task in a terminal, you can add `skipBrokenLinks=1` as an argument: `sake dev/tasks/gorriecoe-to-linkfield-migration-task skipBrokenLinks=1`. + +> [!WARNING] +> If the same link appears in multiple `many_many` relation lists within the same relation (with different owner records), 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 the same link appears in multiple `many_many` relation lists across *different* relations, you will need to handle the migration of this scenario yourself. The migration task will not duplicate these links. The link's owner will be whichever record is first identified, and any further owner records will simply not have that link in their `has_many` relation list. diff --git a/src/Tasks/GorriecoeMigrationTask.php b/src/Tasks/GorriecoeMigrationTask.php new file mode 100644 index 00000000..36eb26bf --- /dev/null +++ b/src/Tasks/GorriecoeMigrationTask.php @@ -0,0 +1,669 @@ + '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 = []; + + /** + * 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}"); + $subclassLinkJoins = []; + + // 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 = [ + 'ID' => $joinLinkIdColumn, + 'OwnerClass' => $db->escapeIdentifier("{$ownerBaseTable}.ClassName"), + 'OwnerID' => $db->escapeIdentifier("{$ownerBaseTable}.ID"), + ]; + // 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[$baseField] = $db->escapeIdentifier("{$baseLinkTable}.{$baseField}"); + } + } + // Select extra fields, aliased as appropriate + foreach ($extraFields as $fromField => $toField) { + $selections[$toField] = $db->escapeIdentifier("{$joinTable}.{$fromField}"); + } + // Select columns from subclasses (e.g. Email, Phone, etc) + foreach (static::config()->get('link_type_columns') as $spec) { + foreach ($spec['fields'] as $subclassField) { + $selections[$subclassField] = $schema->sqlColumnForField($spec['class'], $subclassField); + // Make sure we join the subclass table into the query + $subclassTable = $schema->tableForField($spec['class'], $subclassField); + if (!array_key_exists($subclassTable, $subclassLinkJoins)) { + $subclassLinkJoins[$subclassTable] = $db->escapeIdentifier("{$subclassTable}.ID") . ' = ' . $db->escapeIdentifier("{$baseLinkTable}.ID"); + } + } + } + + $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("{$baseLinkTable}.ID") . " = {$joinLinkIdColumn}"); + // Add joins for link subclasses + foreach ($subclassLinkJoins as $subclassTable => $onPredicate) { + if (!$select->isJoinedTo($subclassTable)) { + $select->addLeftJoin($subclassTable, $onPredicate); + } + } + $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..548b27da 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. * @@ -80,62 +56,12 @@ class LinkFieldMigrationTask extends BuildTask */ private static array $has_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 = []; - - 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 +76,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 +213,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 +279,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..3ce768cd --- /dev/null +++ b/src/Tasks/MigrationTaskTrait.php @@ -0,0 +1,443 @@ + 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..72bbfb97 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.php @@ -0,0 +1,613 @@ + 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 [ + 'no relations' => [ + 'manymanyConfig' => [], + ], + 'normal many_many, nothing specified' => [ + 'manymanyConfig' => [ + WasManyManyOwner::class => [ + 'NormalManyMany' => null, + ], + ], + ], + 'normal many_many, fully specified' => [ + 'manymanyConfig' => [ + WasManyManyOwner::class => [ + 'NormalManyMany' => [ + 'table' => 'LinkFieldTest_Tasks_WasManyManyOwner_NormalManyMany', + 'extraFields' => [ + 'CustomSort' => 'Sort', + ], + ], + ], + ], + ], + 'many_many through' => [ + 'manymanyConfig' => [ + WasManyManyOwner::class => [ + 'ManyManyThrough' => [ + 'table' => 'GorriecoeMigrationTaskTest_manymany_through', + 'extraFields' => [ + 'CustomSort' => 'Sort', + ], + 'through' => [ + 'from' => 'OldOwner', + 'to' => 'OldLink', + ], + ], + ], + ], + ], + 'many_many through' => [ + 'manymanyConfig' => [ + WasManyManyOwner::class => [ + 'ManyManyThroughPolymorphic' => [ + 'table' => 'GorriecoeMigrationTaskTest_manymany_throughpoly', + 'extraFields' => [ + 'CustomSort' => 'Sort', + ], + 'through' => [ + 'from' => 'OldOwner', + 'to' => 'OldLink', + ], + ], + ], + ], + ], + ]; + } + + /** + * @dataProvider provideMigrateManyManyRelations + */ + public function testMigrateManyManyRelations(array $manymanyConfig): void + { + GorriecoeMigrationTask::config()->set('many_many_links_data', $manymanyConfig); + + // Run the migration + $this->startCapturingOutput(); + $this->callPrivateMethod('migrateManyManyRelations'); + $output = $this->stopCapturingOutput(); + + if (empty($manymanyConfig)) { + $this->assertSame("No many_many relations to migrate.\n", $output); + return; + } + + $expectedOutput = "Migrating many_many relations.\n"; + + foreach ($manymanyConfig as $config) { + foreach ($config as $relation => $spec) { + $table = $spec['table'] ?? 'LinkFieldTest_Tasks_WasManyManyOwner_NormalManyMany'; + $hasSort = !empty($spec['extraFields']); + $expectedOutput .= "Dropping old many_many join table '{$table}'\n"; + + $owner1 = $this->objFromFixture(WasManyManyOwner::class, 'manymany-owner1'); + $owner2 = $this->objFromFixture(WasManyManyOwner::class, 'manymany-owner2'); + $owner3 = $this->objFromFixture(WasManyManyOwner::class, 'manymany-owner3'); + + // Check we have the right amount of owned links + $this->assertCount(3, $owner1->$relation()); + $this->assertCount(3, $owner2->$relation()); + $this->assertCount(1, $owner3->$relation()); + + // Check the links have the correct data + $emailLink1 = $this->objFromFixture(EmailLink::class, 'email-link01'); + $emailLink1Fields = $emailLink1->toMap(); + $relatedItem1Owner1 = $owner1->$relation(); + $relatedItem1Owner2 = $owner2->$relation(); + $relatedItem1Owner3 = $owner3->$relation(); + $this->assertListContains([$this->setSortInRecord($emailLink1Fields, 1, $hasSort)], $relatedItem1Owner1); + // These fields will vary for the other owner's links + unset($emailLink1Fields['ID']); + unset($emailLink1Fields['OwnerID']); + unset($emailLink1Fields['LastEdited']); + $this->assertListContains([$this->setSortInRecord($emailLink1Fields, 3, $hasSort)], $relatedItem1Owner2); + $this->assertListContains([$this->setSortInRecord($emailLink1Fields, 4, $hasSort)], $relatedItem1Owner3); + + $emailLink2 = $this->objFromFixture(EmailLink::class, 'email-link02'); + $emailLink2Fields = $emailLink2->toMap(); + $relatedItem2Owner1 = $owner1->$relation(); + $relatedItem2Owner2 = $owner2->$relation(); + $this->assertListContains([$this->setSortInRecord($emailLink2Fields, 2, $hasSort)], $relatedItem2Owner1); + // These fields will vary for the other owner's link + unset($emailLink2Fields['ID']); + unset($emailLink2Fields['OwnerID']); + unset($emailLink2Fields['LastEdited']); + $this->assertListContains([$this->setSortInRecord($emailLink2Fields, 1, $hasSort)], $relatedItem2Owner2); + + $sitetreeLink1 = $this->objFromFixture(SiteTreeLink::class, 'sitetree-link01'); + $this->assertListContains([$this->setSortInRecord($sitetreeLink1->toMap(), 3, $hasSort)], $owner1->$relation()); + $sitetreeLink2 = $this->objFromFixture(SiteTreeLink::class, 'sitetree-link02'); + $this->assertListContains([$this->setSortInRecord($sitetreeLink2->toMap(), 2, $hasSort)], $owner2->$relation()); + + // Check table was dropped + $this->assertArrayNotHasKey(strtolower($table), DB::table_list()); + } + } + + $this->assertSame($expectedOutput, $output); + } + + public function provideMigrateManyManyRelationsExceptions(): array + { + $ownerClass = WasManyManyOwner::class; + return [ + 'join table required' => [ + 'config' => [ + WasManyManyOwner::class => [ + 'ManyManyThrough' => [ + 'through' => [ + 'from' => 'OldOwner', + 'to' => 'OldLink', + ], + ], + ], + ], + 'expectedMessage' => "Must declare the table name for many_many through relation '{$ownerClass}.ManyManyThrough'.", + ], + 'join table not in db' => [ + 'config' => [ + WasManyManyOwner::class => [ + 'ManyManyThrough' => [ + 'table' => 'non-existant table', + ], + ], + ], + 'expectedMessage' => "Couldn't find join table for many_many relation '{$ownerClass}.ManyManyThrough'.", + ], + 'join class still exists' => [ + 'config' => [ + WasManyManyOwner::class => [ + 'ManyManyThrough' => [ + 'table' => 'LinkFieldTest_Tasks_WasManyManyJoinModel', + 'through' => [ + 'from' => 'OldOwner', + 'to' => 'OldLink', + ], + ], + ], + ], + 'expectedMessage' => "Join table 'LinkFieldTest_Tasks_WasManyManyJoinModel' for many_many through relation '{$ownerClass}.ManyManyThrough' still has a DataObject class.", + ], + ]; + } + + /** + * @dataProvider provideMigrateManyManyRelationsExceptions + */ + public function testMigrateManyManyRelationsExceptions(array $config, string $expectedMessage): void + { + GorriecoeMigrationTask::config()->set('many_many_links_data', $config); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage($expectedMessage); + + // Run the migration + $this->startCapturingOutput(); + try { + $this->callPrivateMethod('migrateManyManyRelations'); + } finally { + // If an exception is thrown we still need to make sure we stop capturing output! + $this->stopCapturingOutput(); + } + } + + private function setSortInRecord(array $record, int $sort, bool $hasSort): array + { + if (!$hasSort) { + return $record; + } + $record['Sort'] = $sort; + return $record; + } + + 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..29299fd8 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml @@ -0,0 +1,230 @@ +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' + OpenInNew: 0 + 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' + OpenInNew: 1 + 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: + join1: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + join2: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 2 + join3: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link01 + CustomSort: 3 + join4: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 3 + join5: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link02 + CustomSort: 2 + join6: + LinkFieldTest_Tasks_WasManyManyOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + GorriecoeMigrationTaskTest_OldLinkTableID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 1 + 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: + join1: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 1 + join2: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 2 + join3: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner1 + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link01 + CustomSort: 3 + join4: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 3 + join5: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\SiteTreeLink.sitetree-link02 + CustomSort: 2 + join6: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner2 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link02 + CustomSort: 1 + join7: + OldOwnerID: =>SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner.manymany-owner3 + OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 + CustomSort: 4 + +GorriecoeMigrationTaskTest_manymany_throughpoly: + 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 + 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 + 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 + 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: 3 + 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 + 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: 1 + 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);