From 4d9c84f4a1187bbe800b7b0225476ee82c965057 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Wed, 20 Mar 2024 13:25:43 +1300 Subject: [PATCH] NEW LinkableMigrationTask for migrating sheadawson-linkable links --- docs/en/09_migrating/01_linkable-migration.md | 388 ++++++++++++ .../en/09_migrating/02_gorriecoe-migration.md | 9 +- src/Tasks/GorriecoeMigrationTask.php | 544 +---------------- src/Tasks/LinkFieldMigrationTask.php | 5 - src/Tasks/LinkableMigrationTask.php | 129 ++++ src/Tasks/MigrationTaskTrait.php | 7 +- src/Tasks/ModuleMigrationTaskTrait.php | 550 ++++++++++++++++++ .../php/Tasks/GorriecoeMigrationTaskTest.php | 87 ++- .../php/Tasks/GorriecoeMigrationTaskTest.yml | 29 + .../WasHasOneLinkOwner.php | 16 + .../HasManyLinkableLinkOwner.php | 16 + .../HasOneLinkableLinkOwner.php | 16 + tests/php/Tasks/LinkableMigrationTaskTest.php | 259 +++++++++ tests/php/Tasks/LinkableMigrationTaskTest.yml | 84 +++ 14 files changed, 1583 insertions(+), 556 deletions(-) create mode 100644 docs/en/09_migrating/01_linkable-migration.md create mode 100644 src/Tasks/LinkableMigrationTask.php create mode 100644 src/Tasks/ModuleMigrationTaskTrait.php create mode 100644 tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkOwner.php create mode 100644 tests/php/Tasks/LinkableMigrationTask/HasManyLinkableLinkOwner.php create mode 100644 tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php create mode 100644 tests/php/Tasks/LinkableMigrationTaskTest.php create mode 100644 tests/php/Tasks/LinkableMigrationTaskTest.yml diff --git a/docs/en/09_migrating/01_linkable-migration.md b/docs/en/09_migrating/01_linkable-migration.md new file mode 100644 index 00000000..b8711bb5 --- /dev/null +++ b/docs/en/09_migrating/01_linkable-migration.md @@ -0,0 +1,388 @@ +--- +title: Migrating from sheadawson/silverstripe-linkable module +summary: A guide for migrating from sheadawson/silverstripe-linkable to silverstripe/linkfield +--- + +# Migrating from sheadawson/silverstripe-linkable module + +> [!NOTE] +> If your site is running Silverstripe CMS 4.x, upgrade to CMS 5 first. +> You will most likely need to use a fork of `sheadawson/silverstripe-linkable` that is compatible with Silverstripe CMS 5 as part of this upgrade. +> Once you have finished upgrading to CMS 5, return to this guide and continue the linkfield upgrade. + +The [`sheadawson/silverstripe-linkable` module](https://github.com/sheadawson/silverstripe-linkable) was a much loved, and much used module. It is, unfortunately, no longer maintained. We have provided some steps and tasks that we hope can be used to migrate your project from Linkable to LinkField. + +There are a few major changes between `sheadawson/silverstripe-linkable` 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: + +- Adding custom CSS classes to link. The `setCSSClass()` method does not exist in Linkfield. You can still add an `Extension` to the `Link` class or develop your own custom link class and implement the logic of this method. +- Customizing link templates. You can still call the `renderWith()` method and pass the name of your custom template, or use a template with the file path of the FQCN of the link subclass, but `LinkField` doesn't support the `templates` configuration. +- Limit allowed Link types. The `silverstripe/linkfield` module does not support the `allowed_types` configuration. Now, in order to set a limited list of link types available to the user, you should use the `LinkField::setAllowedTypes()` method. Use `allowed_by_default` configuration to globally limit link types. +- Custom query params. This functionality is not supported. You can no longer set the `data-extra-query` attribute to a link. But you can still add an extension to the link and template that will allow you to implement this logic. +- The `EmbeddedObject` and `EmbeddedObjectField` classes have no equivalent functionality in `silverstripe/linkfield` +- If you have subclassed `Sheadawson\Linkable\Models\Link`, there may be additional steps you need to take to migrate the data for your subclass. + +> [!WARNING] +> This guide and the associated migration task assume all of the data for your links are in the base table for `Sheadawson\Linkable\Models\Link` or in automatically generated tables (e.g. join tables for `many_many` relations). + +## Setup + +> [!TIP] +> We strongly recommend taking a backup of your database before doing anything else. +> This will ensure you have a known state to revert to in case anything goes wrong. + +### Update your dependencies + +Remove the Linkable module and add `silverstripe/linkfield`: + +```bash +composer remove sheadawson/silverstripe-linkable +composer require silverstripe/linkfield:^4 +``` + +### Configure the migration task + +> [!NOTE] +> Be sure to check how the old module classes are referenced in config `yml` files (eg: `app/_config`). Update appropriately. + +1. Enable the task: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + is_enabled: true + ``` + +> [!WARNING] +> The sheadawson/silverstripe-linkable documentation does not provide guidance or advice on setting up and maintaining `has_many` and `many_many` link relationships. This guide and the corresponding migration task only make an assumption how this setting was made. It is your responsibility to check that this assumption suits your case and customising the migration task as required. + +1. Declare any columns that you added to the linkable 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\LinkableMigrationTask: + # ... + base_link_columns: + MySortColumn: 'Sort' + ``` + +1. Declare any `has_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + 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 Linkable link model + LinkListOne: 'MyOwner' + ``` + +1. Declare any `many_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + 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', + ``` + +1. 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\LinkableMigrationTask: + # ... + classes_that_are_not_link_owners: + - App\Model\SomeClass + ``` + +### Update your codebase + +You should review how you are using the original `Link` model and `LinkField`, but if you don't have any customisations, then replacing the old with the new might be quite simple. + +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 + - Sheadawson\Linkable\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.ed. + + ```diff + namespace App\Model; + + - use Sheadawson\Linkable\Models\Link; + - use Sheadawson\Linkable\Forms\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 field (and `GridField` if you were using that to manage `has_many` or `many_many` relations). + - Note that the linkable module's `LinkField` required you to specify the related field with `ID` appended (e.g. `HasOneLinkID`), whereas the new `LinkField` requires you to specify the field without `ID` appended (e.g. `HasOneLink`). + + ```diff + public function getCMSFields() + { + $fields = parent::getCMSFields(); + + $fields->removeByName(['HasOneLinkID', 'LinkListOne', 'LinkListTwo']); + $fields->addFieldsToTab( + 'Root.Main', + [ + - LinkField::create('HasOneLinkID', 'Has one link') + + LinkField::create('HasOneLink', 'Has one link'), + - GridField::create( + - 'LinkListTwo', + - 'Link List Two', + - $this->LinkListTwo(), + - GridFieldConfig_RelationEditor::create() + - ->removeComponentsByType(GridFieldAddExistingAutocompleter::class) + - ), + + MultiLinkField::create('LinkListOne', 'List list one'), + + MultiLinkField::create('LinkListTwo', 'List list two'), + ] + ); + return $fields; + } + + ``` + +1. In `sheadawson/silverstripe-linkable`, the list of allowed link types was listed in the configuration file. `LinkField` uses a different approach, it is necessary to specify in the configuration those types of links that will be unavailable to the user. If you need to make a certain type of link available, you must use the `LinkField::setAllowedTypes()` method and pass an array of class names as a parameter. Use `allowed_by_default` if it's needed to globally limit link types. + - See [configuring links and link fields](../02_configuration.md) for more information. + + ```diff + - Sheadawson\Linkable\Models\Link: + - allowed_types: + - - URL + - - SiteTree + + // Now you should exclude all link types that are not allowed + + SilverStripe\LinkField\Models\EmilLink: + + allowed_by_default: false + + SilverStripe\LinkField\Models\PhoneLink: + + allowed_by_default: false + + SilverStripe\LinkField\Models\FileLink: + + allowed_by_default: false + + ``` + + ```diff + + use SilverStripe\LinkField\Models\ExternalLink; + + use SilverStripe\LinkField\Models\SiteTreeLink; + + - $allowedTypes = [ + - 'SiteTree', + - 'URL', + - ]; + + + $allowedTypes = [ + + SiteTreeLink::class, + + ExternalLink::class, + + ]; + $linkField->setAllowedTypes($allowedTypes); + ``` + +### Populate module + +If you use the `dnadesign/silverstripe-populate` module, you will not be able to simply "replace" the namespace. Fixture definitions for the new Linkfield module are quite different. There are entirely different models for different link types, whereas before it was just a DB field to specify the type. + +See below for example: + +```diff +- Sheadawson\Linkable\Models\Link: +- internal: +- Title: Internal link +- Type: SiteTree +- SiteTreeID: 1 +- OpenInNewWindow: true ++ SilverStripe\LinkField\Models\SiteTreeLink: ++ internal: ++ LinkText: Internal link ++ Page: =>Page.home ++ OpenInNew: true +- external: +- Title: External link +- Type: URL +- URL: https://example.org +- OpenInNewWindow: true ++ SilverStripe\LinkField\Models\ExternalLink: ++ external: ++ LinkText: External link ++ ExternalUrl: https://example.org ++ OpenInNew: true +- file: +- Title: File link +- Type: File +- File: =>SilverStripe\Assets\File.example ++ SilverStripe\LinkField\Models\FileLink: ++ file: ++ LinkText: File link ++ File: =>SilverStripe\Assets\File.example +- phone: +- Title: Phone link +- Type: Phone +- Phone: +64 1 234 567 ++ SilverStripe\LinkField\Models\PhoneLink: ++ phone: ++ LinkText: Phone link ++ Phone: +64 1 234 567 +- email: +- Title: Email link +- Type: Email +- Email: foo@example.org ++ SilverStripe\LinkField\Models\EmailLink: ++ email: ++ LinkText: Email link ++ Email: foo@example.org + +``` + +## Replace template usages + +The link element structure is rendered using the `SilverStripe/LinkField/Models/Link.ss` template. You can override this template by copying it to the theme or project folder and making the necessary changes. You still can also specify a custom template to display the link by using the `renderWith()` method and passing the name of your custom template. +You can also provide templates for specific subclasses of `Link` - the file path for those templates is the FQCN for the link. + +When working on your own template, you should consider the following differences in variable names. + +**Before:** You might have had references to `$LinkURL` or `$Link.LinkURL`.\ +**After:** These would need to be updated to `$URL` or `$Link.URL` respectively. + +**Before:** `$OpenInNewWindow` or `$Link.OpenInNewWindow`.\ +**After:** `$OpenInNew` or `$Link.OpenInNew` respectively. + +**Before:** `$Link.TargetAttr` or `$TargetAttr` would output the appropriate `target="xx"`.\ +**After:** There is no direct replacement. + +## Customising the migration + +There are many extension hooks in the [`LinkableMigrationTask`](api:SilverStripe\LinkField\Tasks\LinkableMigrationTask) 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 `sheadawson/silverstripe-linkable` or `sheadawson/silverstripe-linkable` which don't have a 1-to-1 equivalent in `silverstripe/linkfield` + +### 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 need to update configuration `LinkableMigrationTask` so it knows how to handle the migration from the old link to the new one: + +```yml +SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + link_type_columns: + # The name of the Type for your custom type as defined in ===== + MyCustomType: + # The FQCN for your new custom link subclass + class: 'App\Model\Link\MyCustomLink' + # An mapping of column names from the old link to your new link subclass + # Only include columns that are defined in the $db configuration for your subclass + fields: + MyOldField: 'MyNewField' +``` + +## Migrating + +> [!NOTE] +> This migration process covers shifting data from the `LinkableLink` tables to the appropriate `LinkField` tables. + +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. 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/linkable-to-linkfield-migration-task` + - via a terminal: `sake dev/tasks/linkable-to-linkfield-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 [`LinkableMigrationTask.has_many_links_data`](api:SilverStripe\LinkField\Tasks\LinkableMigrationTask->has_many_links_data). +1. Migrates any `many_many` relations which were declared in in [`LinkableMigrationTask.many_many_links_data`](api:SilverStripe\LinkField\Tasks\LinkableMigrationTask->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/linkable-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/linkable-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/docs/en/09_migrating/02_gorriecoe-migration.md b/docs/en/09_migrating/02_gorriecoe-migration.md index e600e7cb..88986569 100644 --- a/docs/en/09_migrating/02_gorriecoe-migration.md +++ b/docs/en/09_migrating/02_gorriecoe-migration.md @@ -54,6 +54,9 @@ composer require silverstripe/linkfield:^4 ### Configure the migration task +> [!NOTE] +> Be sure to check how the old module classes are referenced in config `yml` files (eg: `app/_config`). Update appropriately. + 1. Enable the task: ```yml @@ -122,6 +125,8 @@ composer require silverstripe/linkfield:^4 ### Update your codebase +You should review how you are using the original `Link` model and `LinkField`, but if you don't have any customisations, then replacing the old with the new might be quite simple. + 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 @@ -277,8 +282,8 @@ For databases that support transactions, the full data migration is performed wi - 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` + - via the browser: `https://www.example.com/dev/tasks/gorriecoe-to-linkfield-migration-task` + - via a terminal: `sake dev/tasks/gorriecoe-to-linkfield-migration-task` The task performs the following steps: diff --git a/src/Tasks/GorriecoeMigrationTask.php b/src/Tasks/GorriecoeMigrationTask.php index 36eb26bf..40e967e9 100644 --- a/src/Tasks/GorriecoeMigrationTask.php +++ b/src/Tasks/GorriecoeMigrationTask.php @@ -3,20 +3,14 @@ namespace SilverStripe\LinkField\Tasks; use RuntimeException; -use SilverStripe\Core\ClassInfo; use SilverStripe\Dev\BuildTask; +use SilverStripe\LinkField\Models\Link; 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\DataObject; use SilverStripe\ORM\DB; -use SilverStripe\ORM\Queries\SQLDelete; -use SilverStripe\ORM\Queries\SQLSelect; -use SilverStripe\ORM\Queries\SQLUpdate; -use SilverStripe\Versioned\Versioned; /** * @deprecated 4.0.0 Will be removed without equivalent functionality. @@ -24,6 +18,7 @@ class GorriecoeMigrationTask extends BuildTask { use MigrationTaskTrait; + use ModuleMigrationTaskTrait; private static $segment = 'gorriecoe-to-linkfield-migration-task'; @@ -97,62 +92,6 @@ class GorriecoeMigrationTask extends BuildTask ], ]; - /** - * 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 */ @@ -187,483 +126,4 @@ public function performMigration(): void $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 548b27da..01f0ac12 100644 --- a/src/Tasks/LinkFieldMigrationTask.php +++ b/src/Tasks/LinkFieldMigrationTask.php @@ -278,9 +278,4 @@ private function migrateHasManyRelations(): void } $this->extend('afterMigrateHasManyRelations'); } - - private function classIsOldLink(string $class): bool - { - return is_a($class, Link::class, true); - } } diff --git a/src/Tasks/LinkableMigrationTask.php b/src/Tasks/LinkableMigrationTask.php new file mode 100644 index 00000000..242f83bd --- /dev/null +++ b/src/Tasks/LinkableMigrationTask.php @@ -0,0 +1,129 @@ + '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', + ], + ], + ]; + + /** + * 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'); + } +} diff --git a/src/Tasks/MigrationTaskTrait.php b/src/Tasks/MigrationTaskTrait.php index 3ce768cd..c9c66d69 100644 --- a/src/Tasks/MigrationTaskTrait.php +++ b/src/Tasks/MigrationTaskTrait.php @@ -126,7 +126,7 @@ private function setOwnerForHasOneLinks(): void } // Skip if the has_one isn't for Link, or points at a belongs_to or has_many on Link - if (!$this->classIsOldLink($hasOneClass)) { + if (!is_a($hasOneClass, Link::class, true)) { continue; } if ($this->hasReciprocalRelation([$hasOneClass], $hasOneName, $modelClass)) { @@ -435,9 +435,4 @@ 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/src/Tasks/ModuleMigrationTaskTrait.php b/src/Tasks/ModuleMigrationTaskTrait.php new file mode 100644 index 00000000..8a9e41eb --- /dev/null +++ b/src/Tasks/ModuleMigrationTaskTrait.php @@ -0,0 +1,550 @@ + + * // 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 link model. + */ + private string $oldTableName; + + /** + * 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; + } +} diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.php b/tests/php/Tasks/GorriecoeMigrationTaskTest.php index 72bbfb97..7c5abf5a 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.php +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.php @@ -6,7 +6,6 @@ use ReflectionMethod; use ReflectionProperty; use RuntimeException; -use SilverStripe\Dev\Debug; use SilverStripe\Dev\SapphireTest; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; @@ -20,6 +19,7 @@ use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner; use SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink; use SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\HasManyLinkOwner; +use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasHasOneLinkOwner; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBField; @@ -49,6 +49,7 @@ class GorriecoeMigrationTaskTest extends SapphireTest LinkOwner::class, WasManyManyJoinModel::class, WasManyManyOwner::class, + WasHasOneLinkOwner::class, ]; /** @@ -576,6 +577,90 @@ public function testMigrateManyManyRelationsExceptions(array $config, string $ex } } + public function testSetOwnerForHasOneLinks(): 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 has_one Owner's rows + $this->startCapturingOutput(); + $this->callPrivateMethod('setOwnerForHasOneLinks'); + $output = $this->stopCapturingOutput(); + + $ownerClass = WasHasOneLinkOwner::class; + $fixtureRelationsHaveLink = [ + 'owner-1' => ['Link' => true], + 'owner-2' => ['Link' => true], + 'owner-3' => ['Link' => true], + ]; + + $relationsByID = []; + foreach ($fixtureRelationsHaveLink as $fixture => $data) { + $data['fixture'] = $fixture; + $relationsByID[$this->idFromFixture($ownerClass, $fixture)] = $data; + } + + foreach (DataObject::get($ownerClass) as $owner) { + if (array_key_exists($owner->ID, $relationsByID)) { + $data = $relationsByID[$owner->ID]; + $ownerFixture = $data['fixture']; + $record = $this->objFromFixture($ownerClass, $ownerFixture); + foreach ($data as $relation => $hasLink) { + if ($relation === 'fixture') { + continue; + } + /** @var Link $link */ + $link = $record->$relation(); + if ($hasLink === null) { + // Relation should either not be for link, or should not be in the DB. + if (is_a($link->ClassName, Link::class, true)) { + $this->assertFalse($link->isInDB(), "Relation {$relation} should not have a link saved in it"); + } + continue; + } elseif ($hasLink === false) { + // The special case is where the Owner relation was already set to a different record. + $isSpecialCase = $ownerClass === WasHasOneLinkOwner::class && $ownerFixture === 'owns-has-one-again'; + // Relation should be for link, but should not have its Owner set. + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + $this->assertSame( + [ + $isSpecialCase ? $this->idFromFixture(WasHasOneLinkOwner::class, 'owns-has-one') : 0, + $isSpecialCase ? 'Link' : null + ], + [ + $link->OwnerID, + $link->OwnerRelation, + ], + "Relation {$relation} should not have an Owner set" + ); + continue; + } + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + $this->assertSame( + [ + $owner->ID, + $owner->ClassName, + $relation, + ], + [ + $link->OwnerID, + $link->OwnerClass, + $link->OwnerRelation, + ], + "Relation {$relation} should have an Owner set" + ); + } + } + } + + $this->assertSame("Setting owners for has_one relations.\n", $output); + } + private function setSortInRecord(array $record, int $sort, bool $hasSort): array { if (!$hasSort) { diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml index 29299fd8..a33e5086 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml @@ -117,6 +117,24 @@ GorriecoeMigrationTaskTest_OldLinkTable: Title: 'custom link03' Type: 'Custom' CustomField: 'another value' + has-one-link-1: + Title: 'HasOne Link 1' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 1 + has-one-link-2: + Title: 'HasOne Link 2' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 2 + has-one-link-3: + Title: 'HasOne Link 3' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 3 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 @@ -228,3 +246,14 @@ GorriecoeMigrationTaskTest_manymany_throughpoly: OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 CustomSort: 4 + +SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasHasOneLinkOwner: + owner-1: + Title: 'HasOne Link Owner 1' + Link: =>GorriecoeMigrationTaskTest_OldLinkTable.has-one-link-1 + owner-2: + Title: 'HasOne Link Owner 2' + Link: =>GorriecoeMigrationTaskTest_OldLinkTable.has-one-link-2 + owner-3: + Title: 'HasOne Link Owner 3' + Link: =>GorriecoeMigrationTaskTest_OldLinkTable.has-one-link-3 diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkOwner.php b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkOwner.php new file mode 100644 index 00000000..28dcb3a5 --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkOwner.php @@ -0,0 +1,16 @@ + Link::class, + ]; +} diff --git a/tests/php/Tasks/LinkableMigrationTask/HasManyLinkableLinkOwner.php b/tests/php/Tasks/LinkableMigrationTask/HasManyLinkableLinkOwner.php new file mode 100644 index 00000000..57f8a1da --- /dev/null +++ b/tests/php/Tasks/LinkableMigrationTask/HasManyLinkableLinkOwner.php @@ -0,0 +1,16 @@ + Link::class, + ]; +} diff --git a/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php b/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php new file mode 100644 index 00000000..de343ed2 --- /dev/null +++ b/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php @@ -0,0 +1,16 @@ + Link::class, + ]; +} diff --git a/tests/php/Tasks/LinkableMigrationTaskTest.php b/tests/php/Tasks/LinkableMigrationTaskTest.php new file mode 100644 index 00000000..1d75568b --- /dev/null +++ b/tests/php/Tasks/LinkableMigrationTaskTest.php @@ -0,0 +1,259 @@ + ExternalLink::class, + 'Email' => EmailLink::class, + 'Phone' => PhoneLink::class, + 'File' => FileLink::class, + 'SiteTree' => SiteTreeLink::class, + ]; + + protected static $fixture_file = 'LinkableMigrationTaskTest.yml'; + + protected static $extra_dataobjects = [ + HasManyLinkableLinkOwner::class, + HasOneLinkableLinkOwner::class, + ]; + + protected function setUp(): void + { + parent::setUp(); + + LinkableMigrationTask::config()->merge('base_link_columns', [ + 'MySort' => 'Sort', + ]); + } + + public function onBeforeLoadFixtures(): void + { + LinkableMigrationTask::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(255)', + 'Type' => 'Varchar', + 'URL' => 'Varchar(255)', + 'Email' => 'Varchar(255)', + 'Phone' => 'Varchar(255)', + 'OpenInNewWindow' => 'Boolean', + 'FileID' => 'ForeignKey', + // Fields from the LinkableSiteTreeExtension + 'Anchor' => 'Varchar(255)', + 'SiteTreeID' => 'ForeignKey', + // Field for custom fixture + 'MySort' => 'Int', + ]; + DB::require_table(self::OLD_LINK_TABLE, $linkDbColumns, options: DataObject::config()->get('create_table_options')); + }); + parent::onBeforeLoadFixtures(); + } + + 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) { + // 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 = LinkableMigrationTask::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 testSetOwnerForHasOneLinks(): 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 has_one Owner's rows + $this->startCapturingOutput(); + $this->callPrivateMethod('setOwnerForHasOneLinks'); + $output = $this->stopCapturingOutput(); + + $ownerClass = HasOneLinkableLinkOwner::class; + $fixtureRelationsHaveLink = [ + 'owner-1' => ['Link' => true], + 'owner-2' => ['Link' => true], + 'owner-3' => ['Link' => true], + ]; + + $relationsByID = []; + foreach ($fixtureRelationsHaveLink as $fixture => $data) { + $data['fixture'] = $fixture; + $relationsByID[$this->idFromFixture($ownerClass, $fixture)] = $data; + } + + foreach (DataObject::get($ownerClass) as $owner) { + if (array_key_exists($owner->ID, $relationsByID)) { + $data = $relationsByID[$owner->ID]; + $ownerFixture = $data['fixture']; + $record = $this->objFromFixture($ownerClass, $ownerFixture); + foreach ($data as $relation => $hasLink) { + if ($relation === 'fixture') { + continue; + } + /** @var Link $link */ + $link = $record->$relation(); + if ($hasLink === null) { + // Relation should either not be for link, or should not be in the DB. + if (is_a($link->ClassName, Link::class, true)) { + $this->assertFalse($link->isInDB(), "Relation {$relation} should not have a link saved in it"); + } + continue; + } elseif ($hasLink === false) { + // The special case is where the Owner relation was already set to a different record. + $isSpecialCase = $ownerClass === HasOneLinkableLinkOwner::class && $ownerFixture === 'owns-has-one-again'; + // Relation should be for link, but should not have its Owner set. + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + $this->assertSame( + [ + $isSpecialCase ? $this->idFromFixture(HasOneLinkableLinkOwner::class, 'owns-has-one') : 0, + $isSpecialCase ? 'Link' : null + ], + [ + $link->OwnerID, + $link->OwnerRelation, + ], + "Relation {$relation} should not have an Owner set" + ); + continue; + } + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + $this->assertSame( + [ + $owner->ID, + $owner->ClassName, + $relation, + ], + [ + $link->OwnerID, + $link->OwnerClass, + $link->OwnerRelation, + ], + "Relation {$relation} should have an Owner set" + ); + } + } + } + + $this->assertSame("Setting owners for has_one relations.\n", $output); + } + + 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 LinkableMigrationTask(); + $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/LinkableMigrationTaskTest.yml b/tests/php/Tasks/LinkableMigrationTaskTest.yml new file mode 100644 index 00000000..1ad20ff7 --- /dev/null +++ b/tests/php/Tasks/LinkableMigrationTaskTest.yml @@ -0,0 +1,84 @@ +LinkableMigrationTaskTest_OldLinkTable: + custom-link-1: + Title: 'HasMany Link 1' + Type: 'Email' + Email: 'test1@test.com' + OpenInNewWindow: true + MySort: 1 + custom-link-2: + Title: 'HasMany Link 2' + Type: 'Email' + Email: 'test1@test.com' + OpenInNewWindow: true + MySort: 2 + custom-link-3: + Title: 'HasMany Link 3' + Type: 'Email' + Email: 'test2@test.com' + OpenInNewWindow: true + MySort: 3 + custom-link-4: + Title: 'HasMany Link 4' + Type: 'Email' + Email: 'test3@test.com' + OpenInNewWindow: true + MySort: 4 + custom-link-5: + Title: 'HasMany Link 5' + Type: 'Email' + Email: 'test4@test.com' + OpenInNewWindow: true + MySort: 5 + custom-link-6: + Title: 'HasMany Link 6' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 6 + custom-link-7: + Title: 'HasMany Link 7' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 7 + custom-link-8: + Title: 'HasMany Link 8' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 8 + custom-link-9: + Title: 'HasMany Link 9' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 9 + custom-link-10: + Title: 'HasOne Link 10' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 10 + custom-link-11: + Title: 'HasOne Link 11' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 11 + custom-link-12: + Title: 'HasOne Link 12' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 12 + +SilverStripe\LinkField\Tests\Tasks\LinkableMigrationTaskTest\HasOneLinkableLinkOwner: + owner-1: + Title: 'HasOne Link Owner 1' + LinkID: =>LinkableMigrationTaskTest_OldLinkTable.custom-link-10 + owner-2: + Title: 'HasOne Link Owner 2' + LinkID: =>LinkableMigrationTaskTest_OldLinkTable.custom-link-11 + owner-3: + Title: 'HasOne Link Owner 3' + LinkID: =>LinkableMigrationTaskTest_OldLinkTable.custom-link-12