From 00ab1b22acd603bf6d9a607c6adcaa8dd00c8302 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 6 Sep 2024 14:34:36 +1200 Subject: [PATCH] API Update API to reflect changes to CLI interaction --- docs/en/09_migrating/00_upgrading.md | 134 -- docs/en/09_migrating/01_linkable-migration.md | 388 ------ .../en/09_migrating/02_gorriecoe-migration.md | 14 +- docs/en/09_migrating/index.md | 4 +- src/Tasks/GorriecoeMigrationTask.php | 22 +- src/Tasks/LinkFieldMigrationTask.php | 281 ---- src/Tasks/LinkableMigrationTask.php | 129 -- src/Tasks/MigrationTaskTrait.php | 65 +- src/Tasks/ModuleMigrationTaskTrait.php | 14 +- .../php/Tasks/GorriecoeMigrationTaskTest.php | 49 +- .../php/Tasks/GorriecoeMigrationTaskTest.yml | 2 +- .../AmbiguousLinkOwner.php | 2 +- .../CustomLink.php | 2 +- .../HasManyLinkOwner.php | 2 +- .../PolymorphicLinkOwner.php | 2 +- .../ReciprocalLinkOwner.php | 2 +- .../php/Tasks/LinkFieldMigrationTaskTest.php | 1142 ----------------- .../php/Tasks/LinkFieldMigrationTaskTest.yml | 156 --- .../CustomLinkMigrationExtension.php | 17 - .../MultiLinkOwner.php | 19 - .../OverrideMigrationStepsExtension.php | 51 - .../HasManyLinkableLinkOwner.php | 16 - .../HasOneLinkableLinkOwner.php | 16 - tests/php/Tasks/LinkableMigrationTaskTest.php | 259 ---- tests/php/Tasks/LinkableMigrationTaskTest.yml | 84 -- 25 files changed, 90 insertions(+), 2782 deletions(-) delete mode 100644 docs/en/09_migrating/00_upgrading.md delete mode 100644 docs/en/09_migrating/01_linkable-migration.md delete mode 100644 src/Tasks/LinkFieldMigrationTask.php delete mode 100644 src/Tasks/LinkableMigrationTask.php rename tests/php/Tasks/{LinkFieldMigrationTaskTest => GorriecoeMigrationTaskTest}/AmbiguousLinkOwner.php (83%) rename tests/php/Tasks/{LinkFieldMigrationTaskTest => GorriecoeMigrationTaskTest}/CustomLink.php (92%) rename tests/php/Tasks/{LinkFieldMigrationTaskTest => GorriecoeMigrationTaskTest}/HasManyLinkOwner.php (88%) rename tests/php/Tasks/{LinkFieldMigrationTaskTest => GorriecoeMigrationTaskTest}/PolymorphicLinkOwner.php (92%) rename tests/php/Tasks/{LinkFieldMigrationTaskTest => GorriecoeMigrationTaskTest}/ReciprocalLinkOwner.php (88%) delete mode 100644 tests/php/Tasks/LinkFieldMigrationTaskTest.php delete mode 100644 tests/php/Tasks/LinkFieldMigrationTaskTest.yml delete mode 100644 tests/php/Tasks/LinkFieldMigrationTaskTest/CustomLinkMigrationExtension.php delete mode 100644 tests/php/Tasks/LinkFieldMigrationTaskTest/MultiLinkOwner.php delete mode 100644 tests/php/Tasks/LinkFieldMigrationTaskTest/OverrideMigrationStepsExtension.php delete mode 100644 tests/php/Tasks/LinkableMigrationTask/HasManyLinkableLinkOwner.php delete mode 100644 tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php delete mode 100644 tests/php/Tasks/LinkableMigrationTaskTest.php delete mode 100644 tests/php/Tasks/LinkableMigrationTaskTest.yml diff --git a/docs/en/09_migrating/00_upgrading.md b/docs/en/09_migrating/00_upgrading.md deleted file mode 100644 index e5ec1505..00000000 --- a/docs/en/09_migrating/00_upgrading.md +++ /dev/null @@ -1,134 +0,0 @@ ---- -title: Upgrading from older versions -summary: A guide for migrating from older versions of silverstripe/linkfield ---- - -# Upgrading from older versions - -> [!NOTE] -> If your site is running Silverstripe CMS 4.x, update your constraint for `silverstripe/linkfield` to the latest available version of linkfield v3 and upgrade to CMS 5 first. -> There should be no additional steps required for upgrading from linkfield 2.x to linkfield 3.x. -> Once you have finished upgrading to CMS 5, return to this guide and continue the linkfield upgrade. - -There are three major changes introduced in `silverstripe/linkfield` 4.0.0: - -1. The `Title` database field has been renamed to `LinkText` -1. There is now a `has_one` relation on the [`Link`](api:SilverStripe\LinkField\Models\Link) class called `Owner` which must contain the record that owns the link. -1. There is now a form field for managing `has_many` relations to link - -This guide will help you update to the latest version of `silverstripe/linkfield` and run a task that will automatically update your data. - -> [!WARNING] -> This guide does not provide an upgrade path for links which were stored using the legacy `DBLink` database field type. -> If you have any links like that in your project, you will need to write your own migration script. - -## 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. - -### Resolve deprecation warnings - -Enable [deprecation warnings](https://docs.silverstripe.org/en/upgrading/deprecations/) and resolve any deprecation warnings that are related to `silverstripe/linkfield`. When you have resolved all deprecation warnings, you can disable the deprecation warnings. - -### Update your dependencies - -Update your composer dependency for `silverstripe/linkfield` to `^4` - -```bash -composer require silverstripe/linkfield:^4 -``` - -### Configure the migration task - -1. Enable the task: - - ```yml - SilverStripe\LinkField\Tasks\LinkFieldMigrationTask: - is_enabled: true - ``` - -1. Declare any `has_many` relations that need to be migrated: - - ```yml - SilverStripe\LinkField\Tasks\LinkFieldMigrationTask: - # ... - has_many_links_data: - # The class where the has_many relation is declared - App\Model\MyClass: - # The name of the has_many relation - LinkListOne: - # The class where the old has_one relation was declared - # This will be either be Link or a Link subclass (not an extension applied to a Link class) - linkClass: 'SilverStripe\LinkField\Models\Link' - # The old name of the has_one relation on Link or a Link subclass - hasOne: 'MyOwner' - ``` - -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\LinkFieldMigrationTask: - # ... - classes_that_are_not_link_owners: - - App\Model\SomeClass - ``` - -### Update your codebase - -1. If the `has_one` relation for the record which owns the links (e.g. `Page`) has a corresponding `belongs_to` relation on the `Link` model (either added via an extension or YAML configuration), remove the `belongs_to` relation from the `Link` model. -1. For any `has_many` relations to links on the record that owns the links (e.g. `Page`), update the dot notation to point to the `Owner` relation: - - ```diff - private static array $has_many = [ - - 'LinkListOne' => Link::class . '.FirstHasOne', - + 'LinkListOne' => Link::class . '.Owner', - - 'LinkListTwo' => Link::class . '.SecondHasOne', - + 'LinkListTwo' => Link::class . '.Owner', - ]; - ``` - -1. Remove the `has_one` relation on the relevant `Link` class which was storing the `has_many` relations. - - e.g. for the above, remove the `FirstHasOne` and `SecondHasOne` relations from the `Link` class. You may have applied these via an `Extension` class or via YAML configuration. -1. If the models that have `has_one` or `has_many` relations to link don't already use `$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. - -> [!WARNING] -> `many_many` relations to `Link` are not supported. If you have any `many_many` relations to links you will need to migrate these to `has_many` relations yourself. - -## Customising the migration - -There are many extension hooks in the [`LinkFieldMigrationTask`](api:SilverStripe\LinkField\Tasks\LinkFieldMigrationTask) 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 prevent publishing links that should remain in draft. -- You have a setup that allows moving links from one page to another, and want to ensure the correct `Owner` is set for old versions of the link record. -- You have additional custom columns that need to be migrated, which the task doesn't know about. - -## 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. 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` -1. If you have any `has_many` relations to `Link`, replace the `GridField` you're currently using the manage those links with a [`MultiLinkField`](api:SilverStripe\LinkField\Form\MultiLinkField). See [basic usage](../01_basic_usage.md) for details. - -The task performs the following steps: - -1. Migrates content in the `Title` column into the new `LinkText` column and removes the `Title` column from the database. -1. Migrates any `has_many` relations to link which were declared in [`LinkFieldMigrationTask.has_many_links_data`](api:SilverStripe\LinkField\Tasks\LinkFieldMigrationTask->has_many_links_data) and removes the old `*ID` (and `*Class` for polymorphic relations) columns from the old `has_one` relations. -1. Set the `Owner` relation for `has_one` relations to links. -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/linkfield-tov4-migration-task?skipBrokenLinks=1`. - - If you're running the task in a terminal, you can add `skipBrokenLinks=1` as an argument: `sake dev/tasks/linkfield-tov4-migration-task skipBrokenLinks=1`. diff --git a/docs/en/09_migrating/01_linkable-migration.md b/docs/en/09_migrating/01_linkable-migration.md deleted file mode 100644 index b8711bb5..00000000 --- a/docs/en/09_migrating/01_linkable-migration.md +++ /dev/null @@ -1,388 +0,0 @@ ---- -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 88986569..02ebadd5 100644 --- a/docs/en/09_migrating/02_gorriecoe-migration.md +++ b/docs/en/09_migrating/02_gorriecoe-migration.md @@ -5,10 +5,6 @@ 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. @@ -109,8 +105,8 @@ composer require silverstripe/linkfield:^4 # 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', + 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. @@ -280,10 +276,10 @@ For databases that support transactions, the full data migration is performed wi 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` + - via a terminal: `sake db:build --flush` 1. Run the 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` + - via a terminal: `sake tasks:gorriecoe-to-linkfield-migration-task` The task performs the following steps: @@ -297,7 +293,7 @@ The task performs the following steps: 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`. + - If you're running the task in a terminal, you can add `--skipBrokenLinks` as an argument: `sake tasks:gorriecoe-to-linkfield-migration-task --skipBrokenLinks`. > [!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. diff --git a/docs/en/09_migrating/index.md b/docs/en/09_migrating/index.md index 9598917f..b47cfbaf 100644 --- a/docs/en/09_migrating/index.md +++ b/docs/en/09_migrating/index.md @@ -1,6 +1,6 @@ --- -title: Migrating to silverstripe/linkfield v4 -summary: Guides for migrating or upgrading to silverstripe/linkfield v4 +title: Migrating to silverstripe/linkfield +summary: Guides for migrating to silverstripe/linkfield --- # Migrating diff --git a/src/Tasks/GorriecoeMigrationTask.php b/src/Tasks/GorriecoeMigrationTask.php index 40e967e9..50108f0b 100644 --- a/src/Tasks/GorriecoeMigrationTask.php +++ b/src/Tasks/GorriecoeMigrationTask.php @@ -13,18 +13,18 @@ use SilverStripe\ORM\DB; /** - * @deprecated 4.0.0 Will be removed without equivalent functionality. + * @deprecated 5.0.0 Will be removed without equivalent functionality. */ class GorriecoeMigrationTask extends BuildTask { use MigrationTaskTrait; use ModuleMigrationTaskTrait; - private static $segment = 'gorriecoe-to-linkfield-migration-task'; + protected static string $commandName = 'gorriecoe-to-linkfield-migration-task'; - protected $title = 'Gorriecoe to Linkfield Migration Task'; + protected string $title = 'Gorriecoe to Linkfield Migration Task'; - protected $description = 'Migrate from gorriecoe/silverstripe-link to silverstripe/linkfield'; + protected static string $description = 'Migrate from gorriecoe/silverstripe-link to silverstripe/linkfield'; /** * Enable via YAML configuration if you need to run this task @@ -112,18 +112,18 @@ public function performMigration(): void $this->migrateManyManyRelations(); $this->setOwnerForHasOneLinks(); - $this->print("Dropping old link table '{$this->oldTableName}'"); + $this->output->writeln("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->output->writeln('-----------------'); + $this->output->writeln('Bulk data migration complete. All links should be correct (but unpublished) at this stage.'); + $this->output->writeln('-----------------'); $this->publishLinks(); - $this->print('-----------------'); - $this->print('Migration completed successfully.'); - $this->print('-----------------'); + $this->output->writeln('-----------------'); + $this->output->writeln('Migration completed successfully.'); + $this->output->writeln('-----------------'); $this->extend('afterPerformMigration'); } } diff --git a/src/Tasks/LinkFieldMigrationTask.php b/src/Tasks/LinkFieldMigrationTask.php deleted file mode 100644 index 01f0ac12..00000000 --- a/src/Tasks/LinkFieldMigrationTask.php +++ /dev/null @@ -1,281 +0,0 @@ - - * // SiteConfig had two has_many relationships, - * // one to Link.MyHasOne and another to ExternalLink.DifferentHasOne. - * SiteConfig::class => [ - * 'LinksListOne' => [ - * 'linkClass' => Link::class, - * 'hasOne' => 'MyHasOne', - * ] - * 'LinksListTwo' => [ - * 'linkClass' => ExternalLink::class, - * 'hasOne' => 'DifferentHasOne', - * ] - * ] - * - */ - private static array $has_many_links_data = []; - - /** - * Perform the actual data migration and publish links as appropriate - */ - public function performMigration(): void - { - $this->extend('beforePerformMigration'); - // Migrate data - $this->migrateTitleColumn(); - $this->migrateHasManyRelations(); - $this->setOwnerForHasOneLinks(); - - $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 - { - $needsMigration = false; - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - $needColumns = ['LinkText', 'Title']; - $baseDbColumns = array_keys(DB::field_list($baseTable)); - $baseNeededColumns = array_intersect($needColumns, $baseDbColumns); - - // If we have all of the requisite columns, we can proceed. - if ($baseNeededColumns === $needColumns) { - $needsMigration = true; - // Lets developers swap the true to a false if they know something we don't about their setup. - $this->extend('updateNeedsMigration', $needsMigration); - if (!$needsMigration) { - $this->print('Skipping migration due to project-level customisation.'); - } - return $needsMigration; - } - - // Lets developers swap the false to a true if they know something we don't about their setup. - $this->extend('updateNeedsMigration', $needsMigration); - if ($needsMigration) { - $this->print('Not skipping migration due to project-level customisation.'); - } - - // If we're missing anything, give clear output about what the situation is. - $missingColumns = array_diff($needColumns, $baseNeededColumns); - if (count($missingColumns) > 1) { - $this->print('Missing multiple columns in the database. This usually happens in new installations before dev/build.'); - return $needsMigration; - } - $missingColumnKey = array_key_first($missingColumns); - switch ($missingColumns[$missingColumnKey]) { - case 'Title': - $this->print('Missing "Title" column in database. This usually means you have already run this task or do not need to migrate.'); - break; - case 'LinkText': - $this->print('Missing "LinkText" column in database. This usually means you need to upgrade your silverstripe/linkfield dependency and run dev/build.'); - break; - default: - // This should never happen, but better to throw an exception here than to assume nothing went wrong. - throw new LogicException("Got unexpected missing column '{$missingColumns[$missingColumnKey]}'."); - } - return $needsMigration; - } - - /** - * Migrate the old Title column to the new LinkText column - */ - private function migrateTitleColumn(): void - { - $this->extend('beforeMigrateTitleColumn'); - - // Migrate base Link table - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - $this->print("Migrating data in '$baseTable' table."); - $this->migrateColumn($baseTable, 'Title', 'LinkText'); - - // Migrate versioned Link tables - $needColumns = ['LinkText', 'Title']; - if (Link::has_extension(Versioned::class)) { - // Migrate `_Versions` and `_Live` tables - foreach (["{$baseTable}_Versions", "{$baseTable}_Live"] as $versionedTable) { - $versionedDbColumns = array_keys(DB::field_list($versionedTable)); - // If we don't have both columns in the table, skip migrating this table - if (array_intersect($needColumns, $versionedDbColumns) !== $needColumns) { - $this->print("Nothing to migrate in '$versionedTable' table."); - continue; - } - $this->print("Migrating data in '$versionedTable' table."); - $this->migrateColumn($versionedTable, 'Title', 'LinkText'); - } - } - $this->extend('afterMigrateTitleColumn'); - } - - /** - * Set the $migrateFromColumn value to the value of the $migrateToColumn column, but don't replace values that already exist. - */ - private function migrateColumn(string $table, string $migrateFromColumn, string $migrateToColumn, bool $columnIsNumeric = false): void - { - // Give developers a chance to skip migrating this column - $shouldMigrateColumn = true; - $this->extend('updateShouldMigrateColumn', $table, $migrateFromColumn, $shouldMigrateColumn); - if (!$shouldMigrateColumn) { - $this->print("Skipping migration of '{$table}.{$migrateFromColumn}' column due to project-level customisation."); - return; - } - $db = DB::get_conn(); - $fromDbColumn = $db->escapeIdentifier($table . '.' . $migrateFromColumn); - $toDbColumn = $db->escapeIdentifier($table . '.' . $migrateToColumn); - // Migrate the data - $nullCheck = $db->nullCheckClause($toDbColumn, true); - SQLUpdate::create( - $db->escapeIdentifier($table), - [$toDbColumn => [$fromDbColumn => []]], - // Only set if there's no value in that column already - [$columnIsNumeric ? "$toDbColumn = 0 OR $nullCheck" : $nullCheck] - )->execute(); - - // Give developers a chance to skip dropping from column - $shouldDropColumn = true; - $this->extend('updateShouldDropColumn', $table, $migrateFromColumn, $shouldDropColumn); - if (!$shouldDropColumn) { - $this->print("Skipping dropping '{$table}.{$migrateFromColumn}' column due to project-level customisation."); - return; - } - // Remove the column from the db - $this->print("Dropping '{$table}.{$migrateFromColumn}' column."); - $db->query("ALTER TABLE \"$table\" DROP COLUMN \"{$migrateFromColumn}\""); - } - - 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(); - foreach ($linksList as $ownerClass => $relationData) { - foreach ($relationData as $hasManyRelation => $spec) { - $linkClass = $spec['linkClass']; - $hasOneRelation = $spec['hasOne']; - // 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}'."); - }; - // Check if HasOneID column is in the base Link table - if (!array_key_exists("{$hasOneRelation}ID", DB::field_list(DataObject::getSchema()->baseDataTable(Link::class)))) { - // This is an unusual situation, and is difficult to do generically since SQLUpdate in framework - // can't use joins. We'll leave this scenario up to the developer to handle. - $this->extend('migrateHasOneForLinkSubclass', $linkClass, $ownerClass, $hasOneRelation, $hasManyRelation); - continue; - } - $linkTable = $schema->tableForField($linkClass, 'OwnerID'); - $tables = [$linkTable]; - // Include versioned tables if link is versioned - if (Link::has_extension(Versioned::class)) { - $tables[] = "{$linkTable}_Versions"; - $tables[] = "{$linkTable}_Live"; - } - // Migrate old has_one on link to the Owner relation. - foreach ($tables as $table) { - // Only try migration if we have both columns (e.g. versioned tables may not have the old has_one column) - $needColumns = ["{$hasOneRelation}ID", 'OwnerID']; - $columnsInTable = array_keys(DB::field_list($table)); - if (array_intersect($needColumns, $columnsInTable) !== $needColumns) { - continue; - } - $this->migrateColumn($table, "{$hasOneRelation}ID", 'OwnerID', true); - // Only set Class and Relation where the ID got migrated - $ownerIdColumn = $db->escapeIdentifier($table . '.OwnerID'); - $nullCheck = $db->nullCheckClause($ownerIdColumn, false); - $whereClause = [ - "$ownerIdColumn != 0 AND $nullCheck", - $db->nullCheckClause($db->escapeIdentifier($table . '.OwnerRelation'), true), - ]; - $wasPolyMorphic = array_key_exists("{$hasOneRelation}Class", DB::field_list($table)); - if ($wasPolyMorphic) { - // For polymorphic relations, don't set the class/relation columns for records belonging - // to a different class hierarchy. - $validClasses = ClassInfo::subclassesFor($ownerClass, true); - $placeholders = DB::placeholders($validClasses); - $whereClause[] = [$db->escapeIdentifier("{$table}.{$hasOneRelation}Class") . " IN ($placeholders)" => $validClasses]; - } - // Make sure we get the actual class name, not just the base class name. - $subSelect = SQLSelect::create( - $schema->sqlColumnForField($ownerClass, 'ClassName'), - $schema->baseDataTable($ownerClass), - 'ID = ' . $ownerIdColumn - )->sql(); - SQLUpdate::create( - $db->escapeIdentifier($table), - [ - $db->escapeIdentifier($table . '.OwnerClass') => ["({$subSelect})" => []], - $db->escapeIdentifier($table . '.OwnerRelation') => $hasManyRelation, - ], - $whereClause - )->execute(); - if ($wasPolyMorphic) { - $this->print("Dropping '{$table}.{$hasOneRelation}Class' column."); - $db->query("ALTER TABLE \"$table\" DROP COLUMN \"{$hasOneRelation}Class\""); - } - } - } - } - $this->extend('afterMigrateHasManyRelations'); - } -} diff --git a/src/Tasks/LinkableMigrationTask.php b/src/Tasks/LinkableMigrationTask.php deleted file mode 100644 index 242f83bd..00000000 --- a/src/Tasks/LinkableMigrationTask.php +++ /dev/null @@ -1,129 +0,0 @@ - '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 c9c66d69..f88ab0b5 100644 --- a/src/Tasks/MigrationTaskTrait.php +++ b/src/Tasks/MigrationTaskTrait.php @@ -8,6 +8,7 @@ use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\Deprecation; +use SilverStripe\HybridExecution\HybridOutput; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; use SilverStripe\LinkField\Models\FileLink; @@ -21,9 +22,12 @@ use SilverStripe\Versioned\ChangeSet; use SilverStripe\Versioned\ChangeSetItem; use SilverStripe\Versioned\Versioned; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; /** - * @deprecated 4.0.0 Will be removed without equivalent functionality. + * @deprecated 5.0.0 Will be removed without equivalent functionality. */ trait MigrationTaskTrait { @@ -45,38 +49,48 @@ trait MigrationTaskTrait */ private array $linkRelationData = []; + private HybridOutput $output; + 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) + fn () => Deprecation::notice('5.0.0', 'Will be removed without equivalent functionality.', Deprecation::SCOPE_CLASS) ); parent::__construct(); } - public function run($request): void + protected function execute(InputInterface $input, HybridOutput $output): int { + $this->output = $output; $db = DB::get_conn(); // If we don't need to migrate, exit early. if (!$this->getNeedsMigration()) { - $this->print('Cannot perform migration.'); - return; + $this->output->writeln('Cannot perform migration.'); + return Command::SUCCESS; } if (!$db->supportsTransactions()) { - $this->print('Database transactions are not supported for this database. Errors may result in a partially-migrated state.'); + $this->output->writeln('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.'); + if ($input->getOption('skipBrokenLinks')) { + $this->output->writeln('Skipping broken link check as requested.'); } else { $this->checkForBrokenLinks(); } - $this->print('Done.'); + return Command::SUCCESS; + } + + public function getOptions(): array + { + return [ + new InputOption('skipBrokenLinks', null, InputOption::VALUE_NONE, 'Skips checking if any links are broken after migration'), + ]; } /** @@ -85,7 +99,7 @@ public function run($request): void public function failedTransaction(): void { if (DB::get_conn()->supportsTransactions()) { - $this->print('There was an error with the migration. Rolling back.'); + $this->output->writeln('There was an error with the migration. Rolling back.'); } } @@ -95,7 +109,7 @@ public function failedTransaction(): void private function setOwnerForHasOneLinks(): void { $this->extend('beforeSetOwnerForHasOneLinks'); - $this->print('Setting owners for has_one relations.'); + $this->output->writeln('Setting owners for has_one relations.'); $allDataObjectModels = ClassInfo::subclassesFor(DataObject::class, false); $allLinkModels = ClassInfo::subclassesFor(Link::class, true); foreach ($allDataObjectModels as $modelClass) { @@ -254,7 +268,7 @@ private function hasReciprocalRelation(array $linkClasses, string $hasOneName, s // 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}'"); + $this->output->writeln("Ambiguous relation '{$linkClass}.{$relationName}' found - assuming it points at '{$foreignClass}.{$hasOneName}'"); return true; } @@ -314,7 +328,7 @@ private function publishLinks(): void $shouldPublishLinks = true; $this->extend('updateShouldPublishLinks', $shouldPublishLinks); if ($shouldPublishLinks) { - $this->print('Publishing links.'); + $this->output->writeln('Publishing links.'); /** @var Versioned&Link $link */ foreach (Link::get()->chunkedFetch() as $link) { // Allow developers to skip publishing each link - this allows for scenarios @@ -326,12 +340,12 @@ private function publishLinks(): void } $link->destroy(); } - $this->print('Publishing complete.'); + $this->output->writeln('Publishing complete.'); } else { - $this->print('Skipping publish step.'); + $this->output->writeln('Skipping publish step.'); } } else { - $this->print('Links are not versioned - skipping publish step due to project-level customisation.'); + $this->output->writeln('Links are not versioned - skipping publish step due to project-level customisation.'); } } @@ -342,7 +356,7 @@ private function publishLinks(): void */ private function checkForBrokenLinks(): void { - $this->print('Checking for broken links.'); + $this->output->writeln('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 () { @@ -376,19 +390,19 @@ private function checkForBrokenLinks(): void $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."); + $this->output->writeln("Found $numBroken broken links for the '$class' class."); if ($numBroken > 0) { $brokenLinks[$class] = $ids; } } if (empty($brokenLinks)) { - $this->print('No broken links.'); + $this->output->writeln('No broken links.'); return; } // Output table of broken links - $this->print('Broken links:'); + $this->output->writeln('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. @@ -413,19 +427,10 @@ private function checkForBrokenLinks(): void } $output .= ''; } - $this->print($output); + $this->output->writeln($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 */ diff --git a/src/Tasks/ModuleMigrationTaskTrait.php b/src/Tasks/ModuleMigrationTaskTrait.php index 8a9e41eb..301bcafb 100644 --- a/src/Tasks/ModuleMigrationTaskTrait.php +++ b/src/Tasks/ModuleMigrationTaskTrait.php @@ -14,7 +14,7 @@ use SilverStripe\Versioned\Versioned; /** - * @deprecated 4.0.0 Will be removed without equivalent functionality. + * @deprecated 5.0.0 Will be removed without equivalent functionality. */ trait ModuleMigrationTaskTrait { @@ -81,7 +81,7 @@ 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.'); + $this->output->writeln('Nothing to migrate - old link table doesn\'t exist.'); return false; } $this->oldTableName = $oldTableName; @@ -232,12 +232,12 @@ private function migrateHasManyRelations(): void // Exit early if there's nothing to migrate if (empty($linksList)) { - $this->print('No has_many relations to migrate.'); + $this->output->writeln('No has_many relations to migrate.'); $this->extend('afterMigrateHasManyRelations'); return; } - $this->print('Migrating has_many relations.'); + $this->output->writeln('Migrating has_many relations.'); $schema = DataObject::getSchema(); $db = DB::get_conn(); $oldTableFields = DB::field_list($this->oldTableName); @@ -303,12 +303,12 @@ private function migrateManyManyRelations(): void // Exit early if there's nothing to migrate if (empty($linksList)) { - $this->print('No many_many relations to migrate.'); + $this->output->writeln('No many_many relations to migrate.'); $this->extend('afterMigrateManyManyRelations'); return; } - $this->print('Migrating many_many relations.'); + $this->output->writeln('Migrating many_many relations.'); $schema = DataObject::getSchema(); $db = DB::get_conn(); $baseLinkTable = $schema->baseDataTable(Link::class); @@ -393,7 +393,7 @@ private function migrateManyManyRelations(): void $update->execute(); } // Drop the join table - $this->print("Dropping old many_many join table '{$joinTable}'"); + $this->output->writeln("Dropping old many_many join table '{$joinTable}'"); DB::get_conn()->query("DROP TABLE \"{$joinTable}\""); } } diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.php b/tests/php/Tasks/GorriecoeMigrationTaskTest.php index 7c5abf5a..c0ff1197 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.php +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.php @@ -7,6 +7,7 @@ use ReflectionProperty; use RuntimeException; use SilverStripe\Dev\SapphireTest; +use SilverStripe\HybridExecution\HybridOutput; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; use SilverStripe\LinkField\Models\FileLink; @@ -17,8 +18,8 @@ use SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner; use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyJoinModel; 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\CustomLink; +use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\HasManyLinkOwner; use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasHasOneLinkOwner; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; @@ -27,6 +28,7 @@ use SilverStripe\ORM\FieldType\DBVarchar; use SilverStripe\ORM\Queries\SQLSelect; use SilverStripe\ORM\Queries\SQLUpdate; +use Symfony\Component\Console\Output\BufferedOutput; class GorriecoeMigrationTaskTest extends SapphireTest { @@ -59,9 +61,12 @@ class GorriecoeMigrationTaskTest extends SapphireTest */ protected $usesTransactions = false; + private BufferedOutput $buffer; + protected function setUp(): void { parent::setUp(); + $this->buffer = new BufferedOutput(); // Add custom link config GorriecoeMigrationTask::config()->merge('link_type_columns', [ 'Custom' => [ @@ -157,12 +162,11 @@ public function testGetNeedsMigration(string|bool $hasTable, bool $expected): vo DB::get_schema()->schemaUpdate(function () { DB::dont_require_table(self::OLD_LINK_TABLE); }); - $this->stopCapturingOutput(); + $this->startCapturingOutput(); } - $this->startCapturingOutput(); $result = $this->callPrivateMethod('getNeedsMigration'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); $this->assertSame($expected, $result); $this->assertSame($expected ? '' : "Nothing to migrate - old link table doesn't exist.\n", $output); } @@ -174,9 +178,8 @@ public function testInsertBaseRows(): void Link::get()->removeAll(); // Insert the rows - $this->startCapturingOutput(); $this->callPrivateMethod('insertBaseRows'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); $select = new SQLSelect(from: DB::get_conn()->escapeIdentifier(DataObject::getSchema()->baseDataTable(Link::class))); foreach ($select->execute() as $link) { @@ -215,14 +218,12 @@ public function testInsertTypeSpecificRows(): void // 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(); + $this->buffer->fetch(); // Insert the rows - $this->startCapturingOutput(); $this->callPrivateMethod('insertTypeSpecificRows'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE)); $oldLinkData = $oldLinkSelect->execute(); @@ -250,15 +251,13 @@ public function testUpdateSiteTreeRows(): void // 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(); + $this->buffer->fetch(); // Update the rows - $this->startCapturingOutput(); $this->callPrivateMethod('updateSiteTreeRows'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE)); foreach (SiteTreeLink::get() as $link) { @@ -355,9 +354,8 @@ public function testMigrateHasManyRelations( } // Run the migration - $this->startCapturingOutput(); $this->callPrivateMethod('migrateHasManyRelations'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); if (empty($hasManyConfig)) { $this->assertSame("No has_many relations to migrate.\n", $output); @@ -449,9 +447,8 @@ 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(); + $output = $this->buffer->fetch(); if (empty($manymanyConfig)) { $this->assertSame("No many_many relations to migrate.\n", $output); @@ -568,12 +565,11 @@ public function testMigrateManyManyRelationsExceptions(array $config, string $ex $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(); + $this->buffer->fetch(); } } @@ -583,13 +579,11 @@ public function testSetOwnerForHasOneLinks(): void // 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(); + $this->buffer->fetch(); // Insert the has_one Owner's rows - $this->startCapturingOutput(); $this->callPrivateMethod('setOwnerForHasOneLinks'); - $output = $this->stopCapturingOutput(); + $output = $this->buffer->fetch(); $ownerClass = WasHasOneLinkOwner::class; $fixtureRelationsHaveLink = [ @@ -684,6 +678,11 @@ private function stopCapturingOutput(): string private function callPrivateMethod(string $methodName, array $args = []): mixed { $task = new GorriecoeMigrationTask(); + $output = new HybridOutput(HybridOutput::FORMAT_ANSI, wrappedOutput: $this->buffer); + $reflectionProperty = new ReflectionProperty($task, 'output'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($task, $output); + // 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') { diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml index a33e5086..4cf31f07 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml @@ -136,7 +136,7 @@ GorriecoeMigrationTaskTest_OldLinkTable: OpenInNewWindow: true MySort: 3 -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\HasManyLinkOwner: +SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\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: diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest/AmbiguousLinkOwner.php b/tests/php/Tasks/GorriecoeMigrationTaskTest/AmbiguousLinkOwner.php similarity index 83% rename from tests/php/Tasks/LinkFieldMigrationTaskTest/AmbiguousLinkOwner.php rename to tests/php/Tasks/GorriecoeMigrationTaskTest/AmbiguousLinkOwner.php index 5a473874..233dfac3 100644 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest/AmbiguousLinkOwner.php +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest/AmbiguousLinkOwner.php @@ -1,6 +1,6 @@ [ - OverrideMigrationStepsExtension::class, - CustomLinkMigrationExtension::class, - ] - ]; - - /** - * Required because of all the creations and deletions of columns. - * Without this, tests pass individually but fail when run as a class. - */ - protected $usesTransactions = false; - - private bool $needsResetSchema = false; - - protected function setUp(): void - { - parent::setUp(); - - // Remove AmbiguousLinkOwner class from class manifest for all tests that it interferes with. - $name = $this->getName(false); - if (str_starts_with($name, 'testSetOwnerForHasOneLinks') && $name !== 'testSetOwnerForHasOneLinksAmbiguous') { - $this->needsResetSchema = true; - $classManifest = ClassLoader::inst()->getManifest(); - $reflectionGetState = new ReflectionMethod($classManifest, 'getState'); - $reflectionGetState->setAccessible(true); - $state = $reflectionGetState->invoke($classManifest); - $lowerCaseClass = strtolower(AmbiguousLinkOwner::class); - foreach (array_keys($state) as $property) { - switch ($property) { - case 'descendants': - unset($state[$property][$lowerCaseClass]); - unset($state[$property][strtolower(DataObject::class)][$lowerCaseClass]); - break; - case 'classes': - case 'classNames': - unset($state[$property][$lowerCaseClass]); - break; - } - } - $reflectionLoadState = new ReflectionMethod($classManifest, 'loadState'); - $reflectionLoadState->setAccessible(true); - $reflectionLoadState->invokeArgs($classManifest, [$state]); - } - } - - protected function tearDown(): void - { - // Reset static properties - OverrideMigrationStepsExtension::$shouldPublishLink = null; - OverrideMigrationStepsExtension::$shouldPublishLinks = null; - OverrideMigrationStepsExtension::$needsMigration = null; - OverrideMigrationStepsExtension::$shouldMigrateColumn = []; - OverrideMigrationStepsExtension::$shouldDropColumn = []; - // Add versioning back - should probably be done by whatever flushes cache, but apparently not. - if (!Link::has_extension(Versioned::class)) { - Link::add_extension(Versioned::class); - } - // Make sure AmbiguousLinkOwner class exists - if ($this->needsResetSchema) { - ClassLoader::inst()->getManifest()->regenerate(true); - DataObject::getSchema()->reset(); - } - parent::tearDown(); - } - - public function provideGetNeedsMigration(): array - { - $scenarios = [ - 'has both columns' => [ - 'hasTitleColumn' => true, - 'hasLinkTextColumn' => true, - 'extensionOverride' => null, - 'expected' => true, - ], - 'missing Title column' => [ - 'hasTitleColumn' => false, - 'hasLinkTextColumn' => true, - 'extensionOverride' => null, - 'expected' => false, - ], - 'missing LinkText column' => [ - 'hasTitleColumn' => true, - 'hasLinkTextColumn' => false, - 'extensionOverride' => null, - 'expected' => false, - ], - 'missing both columns' => [ - 'hasTitleColumn' => false, - 'hasLinkTextColumn' => false, - 'extensionOverride' => null, - 'expected' => false, - ], - ]; - foreach ($scenarios as $title => $scenario) { - $scenario['extensionOverride'] = true; - $scenario['expected'] = true; - $scenarios[$title . ' (override to true)'] = $scenario; - $scenario['extensionOverride'] = false; - $scenario['expected'] = false; - $scenarios[$title . ' (override to false)'] = $scenario; - } - return $scenarios; - } - - /** - * @dataProvider provideGetNeedsMigration - */ - public function testGetNeedsMigration(bool $hasTitleColumn, bool $hasLinkTextColumn, ?bool $extensionOverride, bool $expected): void - { - // Add/remove columns as necessary to replicate migration scenario - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - if (!$hasLinkTextColumn) { - DB::get_conn()->query("ALTER TABLE \"$baseTable\" DROP COLUMN \"LinkText\""); - } - if ($hasTitleColumn) { - $this->ensureColumnsExist(['Title' => DBVarchar::class], false); - } - // Set extension to override as appropriate - OverrideMigrationStepsExtension::$needsMigration = $extensionOverride; - - $this->startCapturingOutput(); - $needsMigration = $this->callPrivateMethod('getNeedsMigration'); - $output = $this->stopCapturingOutput(); - - $this->assertSame($expected, $needsMigration); - - // Extension overrides can affect the output - $expectedOutput = ''; - if ($extensionOverride && (!$hasLinkTextColumn || !$hasTitleColumn)) { - $expectedOutput = "Not skipping migration due to project-level customisation.\n"; - } elseif ($extensionOverride === false && $hasLinkTextColumn && $hasTitleColumn) { - $expectedOutput = "Skipping migration due to project-level customisation.\n"; - } - - // Output should indicate why migration can't be performed - if (!$hasLinkTextColumn && !$hasTitleColumn) { - $expectedOutput .= "Missing multiple columns in the database. This usually happens in new installations before dev/build.\n"; - } elseif (!$hasLinkTextColumn) { - $expectedOutput .= 'Missing "LinkText" column in database. This usually means you need to upgrade your silverstripe/linkfield dependency and run dev/build.' . "\n"; - } elseif (!$hasTitleColumn) { - $expectedOutput .= 'Missing "Title" column in database. This usually means you have already run this task or do not need to migrate.' . "\n"; - } - - $this->assertSame($expectedOutput, $output); - } - - public function provideMigrateTitleColumnUnversioned(): array - { - return [ - 'full migration' => [ - 'skipMigration' => false, - 'skipDropColumn' => false, - 'v3IsVersioned' => false, - ], - 'skip migration' => [ - 'skipMigration' => true, - 'skipDropColumn' => false, - 'v3IsVersioned' => false, - ], - 'skip drop column' => [ - 'skipMigration' => false, - 'skipDropColumn' => true, - 'v3IsVersioned' => false, - ], - 'full migration, v3 is versioned' => [ - 'skipMigration' => false, - 'skipDropColumn' => false, - 'v3IsVersioned' => true, - ] - ]; - } - - /** - * Tests migrating data when v2 and v3 were NOT versioned - * @dataProvider provideMigrateTitleColumnUnversioned - */ - public function testMigrateTitleColumnUnversioned(bool $skipMigration, bool $skipDropColumn, bool $v3IsVersioned): void - { - // Make sure the Title column exists before we start - $this->ensureColumnsExist(['Title' => DBVarchar::class], false); - - $schema = DataObject::getSchema(); - $baseTable = $schema->baseDataTable(Link::class); - if ($skipMigration) { - OverrideMigrationStepsExtension::$shouldMigrateColumn[$baseTable] = false; - } - if ($skipDropColumn) { - OverrideMigrationStepsExtension::$shouldDropColumn[$baseTable] = false; - } - // Add content to the "Title" column for various links - $ids = [ - 'test-email-link01' => $this->idFromFixture(EmailLink::class, 'test-email-link01'), - 'test-email-link02' => $this->idFromFixture(EmailLink::class, 'test-email-link02'), - 'test-email-link03' => $this->idFromFixture(EmailLink::class, 'test-email-link03'), - 'test-external-link01' => $this->idFromFixture(ExternalLink::class, 'test-external-link01'), - 'test-external-link02' => $this->idFromFixture(ExternalLink::class, 'test-external-link02'), - 'test-external-link03' => $this->idFromFixture(ExternalLink::class, 'test-external-link03'), - 'test-file-link01' => $this->idFromFixture(FileLink::class, 'test-file-link01'), - 'test-file-link02' => $this->idFromFixture(FileLink::class, 'test-file-link02'), - 'test-file-link03' => $this->idFromFixture(FileLink::class, 'test-file-link03'), - 'test-phone-link01' => $this->idFromFixture(PhoneLink::class, 'test-phone-link01'), - 'test-phone-link02' => $this->idFromFixture(PhoneLink::class, 'test-phone-link02'), - 'test-phone-link03' => $this->idFromFixture(PhoneLink::class, 'test-phone-link03'), - 'test-sitetree-link01' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link01'), - 'test-sitetree-link02' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link02'), - 'test-sitetree-link03' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link03'), - 'test-custom-link01' => $this->idFromFixture(CustomLink::class, 'test-custom-link01'), - 'test-custom-link02' => $this->idFromFixture(CustomLink::class, 'test-custom-link02'), - 'test-custom-link03' => $this->idFromFixture(CustomLink::class, 'test-custom-link03'), - ]; - // e.g. for a link with ID=1, the title will be "link title #1" - $db = DB::get_conn(); - $titleColumn = $db->escapeIdentifier($baseTable . '.Title'); - $idColumn = $schema->sqlColumnForField(Link::class, 'ID'); - $placeholders = DB::placeholders($ids); - $update = new SQLUpdate( - $db->escapeIdentifier($baseTable), - [$titleColumn => ["CONCAT('link title #', $idColumn)" => []]], - ["$idColumn in ($placeholders)" => array_values($ids)] - ); - $update->execute(); - - if ($this->usesTransactions) { - // This is required to allow SQLUpdate changes to be rolled back on tearDown - static::tempDB()->startTransaction(); - } - if (!$v3IsVersioned) { - Link::remove_extension(Versioned::class); - } - $this->startCapturingOutput(); - $this->callPrivateMethod('migrateTitleColumn'); - $output = $this->stopCapturingOutput(); - - // Test output, which will be different if we're skipping things - $expectedOutput = "Migrating data in '$baseTable' table.\n"; - if ($skipMigration) { - $expectedOutput .= "Skipping migration of '{$baseTable}.Title' column due to project-level customisation.\n"; - } else { - if ($skipDropColumn) { - $expectedOutput .= "Skipping dropping '{$baseTable}.Title' column due to project-level customisation.\n"; - } else { - $expectedOutput .= "Dropping '{$baseTable}.Title' column.\n"; - } - } - if ($v3IsVersioned) { - $expectedOutput .= "Nothing to migrate in '{$baseTable}_Versions' table.\n"; - $expectedOutput .= "Nothing to migrate in '{$baseTable}_Live' table.\n"; - } - $this->assertSame($expectedOutput, $output); - - // Test LinkText values - foreach ($ids as $fixtureName => $id) { - if (str_ends_with($fixtureName, '01')) { - // These fixtures already had LinkText, which should not have changed. - $expectedLinkText = $fixtureName; - } elseif ($skipMigration) { - $expectedLinkText = null; - } else { - $expectedLinkText = 'link title #' . $id; - } - $this->assertSame($expectedLinkText, Link::get()->byID($id)->LinkText); - } - - // Test whether Title column was dropped - if ($skipMigration || $skipDropColumn) { - $this->assertTrue( - in_array('Title', array_keys(DB::field_list($baseTable))), - 'Title column should NOT be removed' - ); - } else { - $this->assertFalse( - in_array('Title', array_keys(DB::field_list($baseTable))), - 'Title column should be removed' - ); - } - } - - public function provideMigrateTitleColumnVersioned(): array - { - return [ - 'full migration' => [ - 'skipMigration' => null, - 'skipDropColumn' => null, - ], - 'skip migration for Versions table' => [ - 'skipMigration' => '_Versions', - 'skipDropColumn' => null, - ], - 'skip drop column for Versions table' => [ - 'skipMigration' => null, - 'skipDropColumn' => '_Versions', - ], - ]; - } - - /** - * Tests migrating data when v2 and v3 WERE versioned - * @dataProvider provideMigrateTitleColumnVersioned - */ - public function testMigrateTitleColumnVersioned(?string $skipMigration, ?string $skipDropColumn): void - { - // Publish all links before we start so there's data in the _Versions and _Live columns - Link::get()->each(fn(Link $link) => $link->publishSingle()); - - // Make sure the Title column exists before we start - $this->ensureColumnsExist(['Title' => DBVarchar::class]); - - $schema = DataObject::getSchema(); - $baseTable = $schema->baseDataTable(Link::class); - if ($skipMigration) { - OverrideMigrationStepsExtension::$shouldMigrateColumn[$baseTable . $skipMigration] = false; - } - if ($skipDropColumn) { - OverrideMigrationStepsExtension::$shouldDropColumn[$baseTable . $skipDropColumn] = false; - } - // Add content to the "Title" column for various links - $ids = [ - 'test-email-link01' => $this->idFromFixture(EmailLink::class, 'test-email-link01'), - 'test-email-link02' => $this->idFromFixture(EmailLink::class, 'test-email-link02'), - 'test-email-link03' => $this->idFromFixture(EmailLink::class, 'test-email-link03'), - 'test-external-link01' => $this->idFromFixture(ExternalLink::class, 'test-external-link01'), - 'test-external-link02' => $this->idFromFixture(ExternalLink::class, 'test-external-link02'), - 'test-external-link03' => $this->idFromFixture(ExternalLink::class, 'test-external-link03'), - 'test-file-link01' => $this->idFromFixture(FileLink::class, 'test-file-link01'), - 'test-file-link02' => $this->idFromFixture(FileLink::class, 'test-file-link02'), - 'test-file-link03' => $this->idFromFixture(FileLink::class, 'test-file-link03'), - 'test-phone-link01' => $this->idFromFixture(PhoneLink::class, 'test-phone-link01'), - 'test-phone-link02' => $this->idFromFixture(PhoneLink::class, 'test-phone-link02'), - 'test-phone-link03' => $this->idFromFixture(PhoneLink::class, 'test-phone-link03'), - 'test-sitetree-link01' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link01'), - 'test-sitetree-link02' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link02'), - 'test-sitetree-link03' => $this->idFromFixture(SiteTreeLink::class, 'test-sitetree-link03'), - 'test-custom-link01' => $this->idFromFixture(CustomLink::class, 'test-custom-link01'), - 'test-custom-link02' => $this->idFromFixture(CustomLink::class, 'test-custom-link02'), - 'test-custom-link03' => $this->idFromFixture(CustomLink::class, 'test-custom-link03'), - ]; - // e.g. for a link with ID=1, the title will be "link title #1" - $db = DB::get_conn(); - $titleColumn = $db->escapeIdentifier($baseTable . '.Title'); - $idColumn = $schema->sqlColumnForField(Link::class, 'ID'); - $placeholders = DB::placeholders($ids); - foreach (['', '_Versions', '_Live'] as $suffix) { - $versionedTable = $baseTable . $suffix; - $versionedTitleColumn = str_replace($baseTable, $versionedTable, $titleColumn); - $versionedIdColumn = str_replace($baseTable, $versionedTable, $idColumn); - $update = new SQLUpdate( - $db->escapeIdentifier($versionedTable), - [$versionedTitleColumn => ["CONCAT('link title #', $versionedIdColumn)" => []]], - ["$versionedIdColumn in ($placeholders)" => array_values($ids)] - ); - $update->execute(); - } - - if ($this->usesTransactions) { - // This is required to allow SQLUpdate changes to be rolled back on tearDown - static::tempDB()->startTransaction(); - } - $this->startCapturingOutput(); - $this->callPrivateMethod('migrateTitleColumn'); - $output = $this->stopCapturingOutput(); - - // Test output, which will be different if we're skipping things - $expectedOutput = ''; - foreach (['', '_Versions', '_Live'] as $suffix) { - $expectedOutput .= "Migrating data in '{$baseTable}{$suffix}' table.\n"; - if ($skipMigration === $suffix) { - $expectedOutput .= "Skipping migration of '{$baseTable}{$suffix}.Title' column due to project-level customisation.\n"; - } else { - if ($skipDropColumn === $suffix) { - $expectedOutput .= "Skipping dropping '{$baseTable}{$suffix}.Title' column due to project-level customisation.\n"; - } else { - $expectedOutput .= "Dropping '{$baseTable}{$suffix}.Title' column.\n"; - } - } - } - $this->assertSame($expectedOutput, $output); - - // Test LinkText values - foreach ($ids as $fixtureName => $id) { - foreach (['', '_Versions', '_Live'] as $suffix) { - if (str_ends_with($fixtureName, '01')) { - // These fixtures already had LinkText, which should not have changed. - $expectedLinkText = $fixtureName; - } elseif ($skipMigration === $suffix) { - $expectedLinkText = null; - } else { - $expectedLinkText = 'link title #' . $id; - } - $versionedTable = $baseTable . $suffix; - $idColumn = $db->escapeIdentifier($versionedTable . '.ID'); - $select = new SQLSelect('LinkText', [$versionedTable], [$idColumn => $id]); - $this->assertSame($expectedLinkText, $select->execute()->value()); - } - } - - // Test whether Title column was dropped - foreach (['', '_Versions', '_Live'] as $suffix) { - if ($skipMigration === $suffix || $skipDropColumn === $suffix) { - $this->assertTrue( - in_array('Title', array_keys(DB::field_list($baseTable . $suffix))), - 'Title column should NOT be removed - suffix: ' . $suffix - ); - } else { - $this->assertFalse( - in_array('Title', array_keys(DB::field_list($baseTable . $suffix))), - 'Title column should be removed - suffix: ' . $suffix - ); - } - } - } - - public function provideSetOwnerForHasOneLinks(): array - { - return [ - [ - 'ownerClass' => LinkOwner::class, - 'fixtureRelationsHaveLink' => [ - 'owns-has-one' => [ - // Link exists and Owner relation is set to this record - 'Link' => true, - ], - 'owns-has-one-again' => [ - // Link exists but Owner relation is NOT set to this record - 'Link' => false, - ], - 'owns-nothing' => [ - // Link does not exist - 'Link' => null, - ], - ], - ], - [ - 'ownerClass' => MultiLinkOwner::class, - 'fixtureRelationsHaveLink' => [ - 'owns-one' => [ - 'LinkOne' => true, - 'LinkTwo' => null, - 'NotLink' => null, - ], - 'owns-another' => [ - 'LinkOne' => null, - 'LinkTwo' => true, - 'NotLink' => null, - ], - 'owns-multiple' => [ - 'LinkOne' => true, - 'LinkTwo' => true, - 'NotLink' => null, - ], - ], - ], - [ - 'ownerClass' => ReciprocalLinkOwner::class, - 'fixtureRelationsHaveLink' => [ - 'owns-many' => [ - 'BaseLink' => true, - 'CustomLink' => true, - 'BelongsToLink' => false, - 'HasManyLink' => false, - ], - ], - ], - [ - 'ownerClass' => PolymorphicLinkOwner::class, - 'fixtureRelationsHaveLink' => [ - 'owns-many' => [ - 'PolymorphicLink' => true, - 'PolymorphicReciprocalLink' => false, - 'MultiRelationalLinkOne' => true, - 'MultiRelationalLinkTwo' => false, - ], - ], - ], - ]; - } - - /** - * @dataProvider provideSetOwnerForHasOneLinks - */ - public function testSetOwnerForHasOneLinks(string $ownerClass, array $fixtureRelationsHaveLink): void - { - $this->startCapturingOutput(); - $this->callPrivateMethod('setOwnerForHasOneLinks'); - $output = $this->stopCapturingOutput(); - - $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 === LinkOwner::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(LinkOwner::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); - } - - public function provideSetOwnerForHasOneLinksSkipClass(): array - { - return [ - 'skip all DataObjects' => [ - 'skipHierarchy' => DataObject::class, - ], - 'just skip LinkOwner' => [ - 'skipHierarchy' => LinkOwner::class, - ], - ]; - } - - /** - * Tests that classes_that_are_not_link_owners skips both the class itself and all its subclasses. - * @dataProvider provideSetOwnerForHasOneLinksSkipClass - */ - public function testSetOwnerForHasOneLinksSkipClass(string $skipHierarchy): void - { - LinkFieldMigrationTask::config()->merge('classes_that_are_not_link_owners', [$skipHierarchy]); - - $this->startCapturingOutput(); - $this->callPrivateMethod('setOwnerForHasOneLinks'); - $output = $this->stopCapturingOutput(); - - $idsToFixtures = [ - $this->idFromFixture(LinkOwner::class, 'owns-has-one') => 'owns-has-one', - $this->idFromFixture(LinkOwner::class, 'owns-has-one-again') => 'owns-has-one-again', - $this->idFromFixture(LinkOwner::class, 'owns-nothing') => 'owns-nothing', - ]; - - foreach (LinkOwner::get() as $owner) { - if (!array_key_exists($owner->ID, $idsToFixtures)) { - continue; - } - $fixture = $idsToFixtures[$owner->ID]; - $link = $owner->Link(); - // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 - $this->assertSame( - [ - $fixture === 'owns-nothing' ? null : 0, - null - ], - [ - $link->OwnerID, - $link->OwnerRelation, - ], - "Link for fixture {$fixture} should not have its Owner relation set." - ); - } - - $this->assertSame("Setting owners for has_one relations.\n", $output); - } - - /** - * Tests output and result for setOwnerForHasOneLinks when the relation has an ambiguous reciprocal relation - */ - public function testSetOwnerForHasOneLinksAmbiguous(): void - { - $this->startCapturingOutput(); - $this->callPrivateMethod('setOwnerForHasOneLinks'); - $output = $this->stopCapturingOutput(); - - $record = $this->objFromFixture(AmbiguousLinkOwner::class, 'owns-one'); - /** @var Link $link */ - $link = $record->Link(); - - // Check link exists, but doesn't have its Owner relation set. - $this->assertTrue($link->isInDB()); - // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 - $this->assertSame( - [ - 0, - null - ], - [ - $link->OwnerID, - $link->OwnerRelation, - ] - ); - - $linkClass = CustomLink::class; - $foreignClass = AmbiguousLinkOwner::class; - $this->assertSame("Setting owners for has_one relations.\nAmbiguous relation '{$linkClass}.AmbiguousOwner' found - assuming it points at '{$foreignClass}.Link'\n", $output); - } - - public function provideMigrateHasManyRelations(): array - { - return [ - 'no has_many' => [ - 'hasManyConfig' => [], - ], - 'has_one for has_many still exists' => [ - 'hasManyConfig' => [ - HasManyLinkOwner::class => [ - 'ForHasOne' => [ - 'linkClass' => CustomLink::class, - 'hasOne' => 'ForHasMany', - ], - ], - ], - 'ownerFixture' => 'still-has-relation', - 'addColumns' => [], - 'relationStillExists' => true, - ], - 'regular has_one' => [ - 'hasManyConfig' => [ - HasManyLinkOwner::class => [ - 'RegularHasMany' => [ - 'linkClass' => Link::class, - 'hasOne' => 'OldHasOne', - ], - ], - ], - 'ownerFixture' => 'legacy-relations', - 'addColumns' => ['OldHasOneID' => DBInt::class], - ], - 'polymorphic has_one' => [ - 'hasManyConfig' => [ - HasManyLinkOwner::class => [ - 'PolyHasMany' => [ - 'linkClass' => Link::class, - 'hasOne' => 'OldHasOne', - ], - ], - ], - 'ownerFixture' => 'legacy-relations', - 'addColumns' => [ - 'OldHasOneID' => DBInt::class, - 'OldHasOneClass' => DBVarchar::class, - ], - ], - ]; - } - - /** - * @dataProvider provideMigrateHasManyRelations - */ - public function testMigrateHasManyRelations( - array $hasManyConfig, - string $ownerFixture = null, - array $addColumns = [], - bool $relationStillExists = false - ): void { - LinkFieldMigrationTask::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) { - $this->ensureColumnsExist($addColumns); - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - $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($baseTable), - [$db->escapeIdentifier("{$baseTable}.{$columnName}") => $value] - )->execute(); - } - } - - if ($relationStillExists) { - $message = ''; - foreach ($hasManyConfig as $ownerClass => $relationData) { - $owner = $this->objFromFixture($ownerClass, $ownerFixture); - foreach ($relationData as $hasManyRelation => $spec) { - $linkClass = $spec['linkClass']; - $hasOneRelation = $spec['hasOne']; - $message .= "has_one relation '{$linkClass}.{$hasOneRelation} still exists. Cannot migrate has_many relation '{$ownerClass}.{$hasManyRelation}'."; - } - } - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage($message); - } - - // Run the migration - $this->startCapturingOutput(); - try { - $this->callPrivateMethod('migrateHasManyRelations'); - } finally { - // If $relationStillExists is true, we will have an exception before this is run, - // but we still need to make sure we stop capturing output! - $output = $this->stopCapturingOutput(); - } - - if (empty($hasManyConfig)) { - $this->assertSame("No has_many relations to migrate.\n", $output); - return; - } - - $expectedOutput = "Migrating has_many relations.\n"; - - - if (!$relationStillExists) { - // 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')); - } - } - } - - // Make sure we report about dropped columns - foreach (['', '_Versions', '_Live'] as $suffix) { - foreach (array_keys($addColumns) as $column) { - $expectedOutput .= "Dropping '{$baseTable}{$suffix}.{$column}' column.\n"; - } - } - - $this->assertSame($expectedOutput, $output); - } - - public function providePublishLinks(): array - { - return [ - 'skip nothing' => [ - 'shouldPublishLink' => true, - 'shouldPublishLinks' => true, - ], - 'skip all links' => [ - 'shouldPublishLink' => true, - 'shouldPublishLinks' => false, - ], - 'skip individual links' => [ - 'shouldPublishLink' => false, - 'shouldPublishLinks' => true, - ], - ]; - } - - /** - * @dataProvider providePublishLinks - */ - public function testPublishLinks(bool $shouldPublishLink, bool $shouldPublishLinks): void - { - // Get the live table before calling publishLinks - $liveTable = DataObject::getSchema()->tableName(Link::class) . '_Live'; - $liveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - - OverrideMigrationStepsExtension::$shouldPublishLink = $shouldPublishLink; - OverrideMigrationStepsExtension::$shouldPublishLinks = $shouldPublishLinks; - $this->startCapturingOutput(); - $this->callPrivateMethod('publishLinks'); - $output = $this->stopCapturingOutput(); - - // Get the live table contents post-publish - $newLiveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - - // Output is based on whether we're skiping publishing for all links or not - if ($shouldPublishLinks) { - $this->assertSame("Publishing links.\nPublishing complete.\n", $output); - } else { - $this->assertSame("Skipping publish step.\n", $output); - } - - // If we're publishing the links we should have different content in the live table - if ($shouldPublishLink && $shouldPublishLinks) { - $this->assertNotSame($liveLinkContents, $newLiveLinkContents); - } else { - $this->assertSame($liveLinkContents, $newLiveLinkContents); - } - } - - /** - * Mostly this is just to validate that no errors get thrown if there's nothing to publish - */ - public function testPublishLinksNoLinks(): void - { - Link::get()->removeAll(); - // Get the live table before calling publishLinks - $liveTable = DataObject::getSchema()->tableName(Link::class) . '_Live'; - $liveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - // Call the method - $this->startCapturingOutput(); - $this->callPrivateMethod('publishLinks'); - // Check output - $this->assertSame( - "Publishing links.\nPublishing complete.\n", - $this->stopCapturingOutput() - ); - // Check the live table didn't change - $newLiveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - $this->assertSame($liveLinkContents, $newLiveLinkContents); - } - - /** - * Test that nothing changes and correct text is output for publish step when links aren't versioned - */ - public function testPublishLinksUnversioned(): void - { - // Get the live table before calling publishLinks - $liveTable = DataObject::getSchema()->tableName(Link::class) . '_Live'; - $liveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - // Remove versioning - Link::remove_extension(Versioned::class); - // Call the method - $this->startCapturingOutput(); - $this->callPrivateMethod('publishLinks'); - // Check output - $this->assertSame( - "Links are not versioned - skipping publish step due to project-level customisation.\n", - $this->stopCapturingOutput() - ); - // Check the live table didn't change - $newLiveLinkContents = SQLSelect::create() - ->setFrom(Convert::symbol2sql($liveTable)) - ->execute() - ->map(); - $this->assertSame($liveLinkContents, $newLiveLinkContents); - } - - public function provideCheckForBrokenLinks(): array - { - return [ - 'no broken links' => [ - 'hasBrokenLinks' => false, - ], - 'with broken links' => [ - 'hasBrokenLinks' => true, - ], - ]; - } - - /** - * @dataProvider provideCheckForBrokenLinks - */ - public function testCheckForBrokenLinks(bool $hasBrokenLinks): void - { - $brokenLinkFixtures = $this->getBrokenLinkFixtures($hasBrokenLinks); - $this->startCapturingOutput(); - $this->callPrivateMethod('checkForBrokenLinks'); - $output = $this->stopCapturingOutput(); - - $expectedOutputRegex = "Checking for broken links.\n"; - if ($hasBrokenLinks) { - foreach (array_keys($brokenLinkFixtures) as $class) { - $safeClass = preg_quote($class); - $expectedOutputRegex .= "Found 2 broken links for the '$safeClass' class.\n"; - } - $classNameLen = strlen(CustomLink::class); - $spaces = str_repeat(' ', $classNameLen - strlen('Link class')); - $hyphens = str_repeat('-', $classNameLen); - $expectedOutputRegex .= <<< CLI_TABLE - Broken links: - Link class$spaces | IDs of broken links - $hyphens | -------------------\n - CLI_TABLE; - foreach ($brokenLinkFixtures as $class => $ids) { - $paddedClass = str_pad($class, $classNameLen); - $safeClass = preg_quote($paddedClass); - // ID order isn't reliable, so we'll just check the format here and check the actual values separately. - $idsPlaceholder = str_repeat('\d+, ', count($ids) - 1) . '\d+'; - $expectedOutputRegex .= "$safeClass | $idsPlaceholder\n"; - } - } else { - foreach (array_keys($brokenLinkFixtures) as $class) { - $safeClass = preg_quote($class); - $expectedOutputRegex .= "Found 0 broken links for the '$safeClass' class.\n"; - } - $expectedOutputRegex .= "No broken links.\n"; - } - $this->assertMatchesRegularExpression('#' . $expectedOutputRegex . '#', $output); - - // Check the IDs are correct (regardless of order) - if ($hasBrokenLinks) { - $output = preg_replace('/.*-------------------\n/s', '', $output); - $rows = explode("\n", $output); - foreach ($rows as $row) { - if (empty(trim($row))) { - continue; - } - $parts = explode('|', $row); - if (count($parts) !== 2) { - echo ''; - } - $class = trim($parts[0]); - $ids = explode(', ', trim($parts[1])); - $toMatch = $brokenLinkFixtures[$class]; - sort($ids); - sort($toMatch); - $this->assertEquals($toMatch, $ids); - } - } - } - - /** - * @dataProvider provideCheckForBrokenLinks - */ - public function testCheckForBrokenLinksWithHtmlOutput(bool $hasBrokenLinks): void - { - // Make sure we get HTML output - $reflectionEnvironment = new ReflectionClass(Environment::class); - $reflectionEnvironment->setStaticPropertyValue('isCliOverride', false); - try { - $brokenLinkFixtures = $this->getBrokenLinkFixtures($hasBrokenLinks); - $this->startCapturingOutput(); - $this->callPrivateMethod('checkForBrokenLinks'); - $output = $this->stopCapturingOutput(); - - $expectedOutputRegex = '#Checking for broken links\.
'; - if ($hasBrokenLinks) { - foreach (array_keys($brokenLinkFixtures) as $class) { - $safeClass = preg_quote($class); - $expectedOutputRegex .= "Found 2 broken links for the '$safeClass' class\.
"; - } - $expectedOutputRegex .= 'Broken links:
'; - foreach ($brokenLinkFixtures as $class => $ids) { - $safeClass = preg_quote($class); - // ID order isn't reliable, so we'll just check the format here and check the actual values separately. - $idsPlaceholder = str_repeat('\d+, ', count($ids) - 1) . '\d+'; - $expectedOutputRegex .= ""; - } - $expectedOutputRegex .= '
Link classIDs of broken links
$safeClass$idsPlaceholder

#'; - } else { - foreach (array_keys($brokenLinkFixtures) as $class) { - $safeClass = preg_quote($class); - $expectedOutputRegex .= "Found 0 broken links for the '$safeClass' class\.
"; - } - $expectedOutputRegex .= 'No broken links\.
#'; - } - $this->assertMatchesRegularExpression($expectedOutputRegex, $output); - - // Check the IDs are correct (regardless of order) - if ($hasBrokenLinks) { - $crawler = new Crawler($output); - $tableRows = $crawler->filter('table tbody tr'); - foreach ($tableRows as $row) { - $class = trim($row->firstChild->textContent); - $ids = explode(', ', trim($row->lastChild->textContent)); - $toMatch = $brokenLinkFixtures[$class]; - sort($ids); - sort($toMatch); - $this->assertEquals($toMatch, $ids); - } - } - } finally { - // Make sure we unset the CLI override - $reflectionEnvironment->setStaticPropertyValue('isCliOverride', null); - } - } - - private function getBrokenLinkFixtures(bool $hasBrokenLinks): array - { - $fixtures = [ - EmailLink::class => [ - $this->idFromFixture(EmailLink::class, 'broken-email-link01'), - $this->idFromFixture(EmailLink::class, 'broken-email-link02'), - ], - ExternalLink::class => [ - $this->idFromFixture(ExternalLink::class, 'broken-external-link01'), - $this->idFromFixture(ExternalLink::class, 'broken-external-link02'), - ], - FileLink::class => [ - $this->idFromFixture(FileLink::class, 'broken-file-link01'), - $this->idFromFixture(FileLink::class, 'broken-file-link02'), - ], - PhoneLink::class => [ - $this->idFromFixture(PhoneLink::class, 'broken-phone-link01'), - $this->idFromFixture(PhoneLink::class, 'broken-phone-link02'), - ], - SiteTreeLink::class => [ - $this->idFromFixture(SiteTreeLink::class, 'broken-sitetree-link01'), - $this->idFromFixture(SiteTreeLink::class, 'broken-sitetree-link02'), - ], - CustomLink::class => [ - $this->idFromFixture(CustomLink::class, 'broken-custom-link01'), - $this->idFromFixture(CustomLink::class, 'broken-custom-link02'), - ], - ]; - if (!$hasBrokenLinks) { - foreach ($fixtures as $class => $ids) { - $list = new DataList($class); - $list->byIDs($ids)->removeAll(); - } - } - return $fixtures; - } - - 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 LinkFieldMigrationTask(); - $reflectionMethod = new ReflectionMethod($task, $methodName); - $reflectionMethod->setAccessible(true); - return $reflectionMethod->invoke($task, ...$args); - } - - private function ensureColumnsExist(array $columns, bool $includeVersioned = true) - { - $baseTable = DataObject::getSchema()->baseDataTable(Link::class); - $tables = [$baseTable]; - if ($includeVersioned) { - $tables[] = "{$baseTable}_Versions"; - $tables[] = "{$baseTable}_Live"; - } - DB::get_schema()->schemaUpdate(function () use ($tables, $columns) { - foreach ($tables as $table) { - foreach ($columns as $column => $fieldType) { - $dbField = DBField::create_field($fieldType, null, $column); - $dbField->setTable($table); - $dbField->requireField(); - } - } - }); - } -} diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest.yml b/tests/php/Tasks/LinkFieldMigrationTaskTest.yml deleted file mode 100644 index 84707894..00000000 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest.yml +++ /dev/null @@ -1,156 +0,0 @@ -SilverStripe\LinkField\Models\EmailLink: - broken-email-link01: - LinkText: 'Broken email link 01' - Email: null - broken-email-link02: - LinkText: 'Broken email link 02' - Email: '' - test-email-link01: - LinkText: 'test-email-link01' - Email: 'email@example.com' - test-email-link02: - LinkText: '' - Email: 'email2@example.com' - test-email-link03: - LinkText: null - Email: 'email3@example.com' - -SilverStripe\LinkField\Models\ExternalLink: - broken-external-link01: - LinkText: 'Broken external url link 01' - ExternalUrl: null - broken-external-link02: - LinkText: 'Broken external url link 02' - ExternalUrl: '' - test-external-link01: - LinkText: 'test-external-link01' - ExternalUrl: 'https://www.example.com/' - test-external-link02: - LinkText: '' - ExternalUrl: 'https://www.example.com/2' - test-external-link03: - LinkText: null - ExternalUrl: 'https://www.example.com/3' - -SilverStripe\LinkField\Models\FileLink: - broken-file-link01: - LinkText: 'Broken file link 01' - FileID: null - broken-file-link02: - LinkText: 'Broken file link 02' - FileID: 0 - test-file-link01: - LinkText: 'test-file-link01' - # Doesn't matter if this is a real file or not for our purposes - FileID: 1 - test-file-link02: - LinkText: '' - FileID: 2 - test-file-link03: - LinkText: null - FileID: 3 - -SilverStripe\LinkField\Models\PhoneLink: - broken-phone-link01: - LinkText: 'Broken phone link 01' - Phone: null - broken-phone-link02: - LinkText: 'Broken phone link 02' - Phone: '' - test-phone-link01: - LinkText: 'test-phone-link01' - Phone: '1' - test-phone-link02: - LinkText: '' - Phone: '2' - test-phone-link03: - LinkText: null - Phone: '3' - -SilverStripe\LinkField\Models\SiteTreeLink: - broken-sitetree-link01: - LinkText: 'Broken page link 01' - PageID: null - broken-sitetree-link02: - LinkText: 'Broken page link 02' - PageID: 0 - test-sitetree-link01: - LinkText: 'test-sitetree-link01' - # Doesn't matter if this is a real page or not for our purposes - PageID: 1 - test-sitetree-link02: - LinkText: '' - PageID: 2 - test-sitetree-link03: - LinkText: null - PageID: 3 - -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink: - broken-custom-link01: - LinkText: 'Broken custom link 01' - MyField: null - broken-custom-link02: - LinkText: 'Broken custom link 02' - MyField: 'broken' - test-custom-link01: - LinkText: 'test-custom-link01' - MyField: 'not broken1' - test-custom-link02: - LinkText: '' - MyField: 'not broken2' - test-custom-link03: - LinkText: null - MyField: 'not broken3' - test-custom-link04: - LinkText: 'link 4' - MyField: 'not broken4' - test-custom-link05: - LinkText: 'link 5' - MyField: 'not broken5' - -SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner: - owns-nothing: - owns-has-one: - Link: =>SilverStripe\LinkField\Models\EmailLink.test-email-link01 - # This owns the same record that "owns-has-one" owns, so the Owner relation of the link will - # be set to "owns-has-one". - owns-has-one-again: - Link: =>SilverStripe\LinkField\Models\EmailLink.test-email-link01 - -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\MultiLinkOwner: - owns-one: - LinkOne: =>SilverStripe\LinkField\Models\EmailLink.test-email-link02 - owns-another: - LinkTwo: =>SilverStripe\LinkField\Models\EmailLink.test-email-link03 - owns-multiple: - LinkOne: =>SilverStripe\LinkField\Models\ExternalLink.test-external-link01 - LinkTwo: =>SilverStripe\LinkField\Models\ExternalLink.test-external-link02 - -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\AmbiguousLinkOwner: - owns-one: - Link: =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link01 - -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\ReciprocalLinkOwner: - owns-many: - BaseLink: =>SilverStripe\LinkField\Models\ExternalLink.test-external-link03 - CustomLink: =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link02 - BelongsToLink: =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link03 - HasManyLink: =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link04 - -SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\PolymorphicLinkOwner: - owns-many: - PolymorphicLink: =>SilverStripe\LinkField\Models\FileLink.test-file-link01 - PolymorphicReciprocalLink: =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link05 - MultiRelationalLinkOne: =>SilverStripe\LinkField\Models\FileLink.test-file-link02 - MultiRelationalLinkOneRelation: 'Owner' - MultiRelationalLinkTwo: =>SilverStripe\LinkField\Models\FileLink.test-file-link03 - MultiRelationalLinkTwoRelation: 'SomethingElse' - -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: - still-has-relation: - ForHasOne: - - =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link01 - - =>SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink.test-custom-link02 diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest/CustomLinkMigrationExtension.php b/tests/php/Tasks/LinkFieldMigrationTaskTest/CustomLinkMigrationExtension.php deleted file mode 100644 index 4d8a4bc8..00000000 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest/CustomLinkMigrationExtension.php +++ /dev/null @@ -1,17 +0,0 @@ - 'MyField', - 'emptyValue' => [null, 'broken'], - ]; - } -} diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest/MultiLinkOwner.php b/tests/php/Tasks/LinkFieldMigrationTaskTest/MultiLinkOwner.php deleted file mode 100644 index cecf8638..00000000 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest/MultiLinkOwner.php +++ /dev/null @@ -1,19 +0,0 @@ - Link::class, - 'LinkTwo' => Link::class, - 'NotLink' => SiteTree::class, - ]; -} diff --git a/tests/php/Tasks/LinkFieldMigrationTaskTest/OverrideMigrationStepsExtension.php b/tests/php/Tasks/LinkFieldMigrationTaskTest/OverrideMigrationStepsExtension.php deleted file mode 100644 index 118b1357..00000000 --- a/tests/php/Tasks/LinkFieldMigrationTaskTest/OverrideMigrationStepsExtension.php +++ /dev/null @@ -1,51 +0,0 @@ - Link::class, - ]; -} diff --git a/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php b/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php deleted file mode 100644 index de343ed2..00000000 --- a/tests/php/Tasks/LinkableMigrationTask/HasOneLinkableLinkOwner.php +++ /dev/null @@ -1,16 +0,0 @@ - Link::class, - ]; -} diff --git a/tests/php/Tasks/LinkableMigrationTaskTest.php b/tests/php/Tasks/LinkableMigrationTaskTest.php deleted file mode 100644 index 1d75568b..00000000 --- a/tests/php/Tasks/LinkableMigrationTaskTest.php +++ /dev/null @@ -1,259 +0,0 @@ - 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 deleted file mode 100644 index 1ad20ff7..00000000 --- a/tests/php/Tasks/LinkableMigrationTaskTest.yml +++ /dev/null @@ -1,84 +0,0 @@ -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