From 4da8be3bf5b00adeae00ec458cd6f61bbf6dd703 Mon Sep 17 00:00:00 2001 From: shoosah Date: Tue, 14 May 2019 09:47:14 +1200 Subject: [PATCH 1/7] Add extend function in getSchemaValidation function This allows to create extensions which add validation list --- src/Forms/FormField.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index 5a9b65bef..ca9034049 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -1609,9 +1609,11 @@ class FormField extends RequestHandler */ public function getSchemaValidation() { + $validationList = []; if ($this->Required()) { - return [ 'required' => true ]; + $validationList['required'] = true; } - return []; + $this->extend('updateSchemaValidation', $validationList); + return $validationList; } } From 3f1479edbbe406a6b9ca1c5284f2daabf455c8b5 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 15 May 2019 11:42:10 +1200 Subject: [PATCH 2/7] BUGFIX: DataQuery overwriting _SortColumn selects (#8974) * BUGFIX: DataQuery overwriting _SortColumn selects * FIX DataQuery _SortColumn handling --- src/ORM/DataQuery.php | 13 ++++++++----- tests/php/ORM/DataQueryTest.php | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index dd21d2bc2..2b4224be7 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -354,7 +354,6 @@ class DataQuery { if ($orderby = $query->getOrderBy()) { $newOrderby = array(); - $i = 0; foreach ($orderby as $k => $dir) { $newOrderby[$k] = $dir; @@ -372,7 +371,6 @@ class DataQuery if (isset($originalSelect[$col])) { $query->selectField($originalSelect[$col], $col); } - continue; } @@ -402,10 +400,15 @@ class DataQuery if (!in_array($qualCol, $query->getSelect())) { unset($newOrderby[$k]); - $newOrderby["\"_SortColumn$i\""] = $dir; - $query->selectField($qualCol, "_SortColumn$i"); + // Find the first free "_SortColumnX" slot + // and assign it to $key + $i = 0; + while (isset($orderby[$key = "\"_SortColumn$i\""])) { + ++$i; + } - $i++; + $newOrderby[$key] = $dir; + $query->selectField($qualCol, "_SortColumn$i"); } } } diff --git a/tests/php/ORM/DataQueryTest.php b/tests/php/ORM/DataQueryTest.php index b9278a496..0c8a6bf41 100644 --- a/tests/php/ORM/DataQueryTest.php +++ b/tests/php/ORM/DataQueryTest.php @@ -296,6 +296,32 @@ class DataQueryTest extends SapphireTest static::resetDBSchema(true); } + public function testSurrogateFieldSort() + { + $query = new DataQuery(DataQueryTest\ObjectE::class); + $query->sort( + sprintf( + '(case when "Title" = %s then 1 else 0 end)', + DB::get_conn()->quoteString('Second') + ), + 'DESC', + true + ); + $query->sort('SortOrder', 'ASC', false); + $query->sort( + sprintf( + '(case when "Title" = %s then 0 else 1 end)', + DB::get_conn()->quoteString('Fourth') + ), + 'DESC', + false + ); + $this->assertEquals( + $query->execute()->column('Title'), + $query->column('Title') + ); + } + public function testComparisonClauseDateStartsWith() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyDate\") VALUES ('1988-03-04 06:30')"); From 25aa3af032f24314ac458743db78028e1aa66ead Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Fri, 24 May 2019 13:44:01 +1200 Subject: [PATCH 3/7] FIX HeaderField requires the optional Title field FormField marks the Title constructor argument as optional, and DatalessField does not override the __construct method. HeaderField on the other hand goes against the grain of FormFields as a whole and requires the Title field, seemingly for no good reason (at least, not that the commit message for a68ba384781086f902708c7364550cc996c15b16 indicates) - this seems like an accidental ommision. This commit looks to reinstate the optionality of this constructor argument for consistency's sake. Plus it broke a module I was investigating. --- src/Forms/HeaderField.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/HeaderField.php b/src/Forms/HeaderField.php index d350d41f5..52c260fb4 100644 --- a/src/Forms/HeaderField.php +++ b/src/Forms/HeaderField.php @@ -30,7 +30,7 @@ class HeaderField extends DatalessField * @param string $title * @param int $headingLevel */ - public function __construct($name, $title, $headingLevel = 2) + public function __construct($name, $title = null, $headingLevel = 2) { $this->setHeadingLevel($headingLevel); parent::__construct($name, $title); From 5b6d0946f446387ec1a52f17aa474a9f6a867ab9 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 28 May 2019 09:19:05 +1200 Subject: [PATCH 4/7] API Add extension points to MigrateFileTask (#8994) * API Add extension points to MigrateFileTask * Apply suggestions from code review Co-Authored-By: Guy Marriott --- .../14_Files/05_File_Migration.md | 70 ++++++++++- src/Dev/Tasks/MigrateFileTask.php | 113 ++++++++++-------- 2 files changed, 134 insertions(+), 49 deletions(-) diff --git a/docs/en/02_Developer_Guides/14_Files/05_File_Migration.md b/docs/en/02_Developer_Guides/14_Files/05_File_Migration.md index f74917041..59fe7b3c4 100644 --- a/docs/en/02_Developer_Guides/14_Files/05_File_Migration.md +++ b/docs/en/02_Developer_Guides/14_Files/05_File_Migration.md @@ -55,7 +55,7 @@ but will no longer be attached to a dataobject anymore, and should be cleaned up To disable this, set the following config: ```yaml -SilverStripe\Assets\FileMigrationHelper: +SilverStripe\Assets\Dev\Tasks\FileMigrationHelper: delete_invalid_files: false ``` @@ -113,3 +113,71 @@ Use the following estimates to decide how you will run your file migration: | 10000+ | Command Line or contact support | n/a | n/a | Your exact experience will vary based on your host server, the size of your files and other conditions. If your site is hosted on a managed environement (e.g.: [Common Web Platform](https://www.cwp.govt.nz/service-desk) or [SilverStripe Platform](https://docs.platform.silverstripe.com/support/)), you may not have access to the command line to manually run the migration task. Contact your hosting provider's helpdesk if that's your case. + +## Customise the File Migration Task (Advanced) + +In some context, you may want to disable some other process when the file migration is running. For example, if you have a module that indexes files when they get modified, you'll probably want to wait until the file migration is done to reindex. + +The `MigrateFileTask` exposes 4 extension point that can be use to detect the progress of the migration. +* `preFileMigration` that gets fired at the start of the task +* `postFileMigration` that gets fired at the end of the task +* `preFileMigrationSubtask` that gets fired at the start of each subtasks +* `postFileMigrationSubtask` that gets fired at the end of each subtasks. + +`preFileMigrationSubtask` and `postFileMigrationSubtask` will provide a single string parameter matching the name of the subtask (e.g.: `move-files`) + +### Example migrate file task extension +```php + '%$' . LoggerInterface::class . '.quiet', + ]; + + /** @var LoggerInterface */ + private $logger; + + /** + * @param LoggerInterface $logger + */ + public function setLogger(LoggerInterface $logger) + { + $this->logger = $logger; + } + + public function preFileMigration() + { + $this->logger->info('Run some extension code BEFORE the Migrate File Task'); + } + + public function postFileMigration() + { + $this->logger->info('Run some extension code AFTER the Migrate File Task'); + } + + public function preFileMigrationSubtask($subtaskName) + { + $this->logger->info(sprintf('Run some extension code BEFORE the %s subtask', $subtaskName)); + } + + public function postFileMigrationSubtask($subtaskName) + { + $this->logger->info(sprintf('Run some extension code AFTER the %s subtask', $subtaskName)); + } + +} +``` + +Add the following snippet to your YML config to enable the extension. + +```yaml +SilverStripe\Dev\Tasks\MigrateFileTask: + extensions: + - MigrateFileTaskExtension +``` diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 559900de2..67e7aca9a 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -46,85 +46,102 @@ class MigrateFileTask extends BuildTask $args = $request->getVars(); $this->validateArgs($args); + $this->extend('preFileMigration'); + $subtasks = !empty($args['only']) ? explode(',', $args['only']) : $this->defaultSubtasks; - if (in_array('move-files', $subtasks)) { + $subtask = 'move-files'; + if (in_array($subtask, $subtasks)) { if (!class_exists(FileMigrationHelper::class)) { $this->logger->error("No file migration helper detected"); - return; - } - - $this->logger->info('### Migrating filesystem and database records (move-files)'); - - $this->logger->info('If the task fails or times out, run it again and it will start where it left off.'); - - $migrated = FileMigrationHelper::singleton()->run(); - if ($migrated) { - $this->logger->info("{$migrated} File DataObjects upgraded"); } else { - $this->logger->info("No File DataObjects need upgrading"); + $this->extend('preFileMigrationSubtask', $subtask); + $this->logger->info("### Migrating filesystem and database records ({$subtask})"); + $this->logger->info('If the task fails or times out, run it again and it will start where it left off.'); + + $migrated = FileMigrationHelper::singleton()->run(); + if ($migrated) { + $this->logger->info("{$migrated} File DataObjects upgraded"); + } else { + $this->logger->info("No File DataObjects need upgrading"); + } + $this->extend('postFileMigrationSubtask', $subtask); } } - if (in_array('move-thumbnails', $subtasks)) { + $subtask = 'move-thumbnails'; + if (in_array($subtask, $subtasks)) { if (!class_exists(LegacyThumbnailMigrationHelper::class)) { $this->logger->error("LegacyThumbnailMigrationHelper not found"); - return; - } - - $this->logger->info('### Migrating existing thumbnails (move-thumbnails)'); - - $moved = LegacyThumbnailMigrationHelper::singleton() - ->setLogger($this->logger) - ->run($this->getStore()); - - if ($moved) { - $this->logger->info(sprintf("%d thumbnails moved", count($moved))); } else { - $this->logger->info("No thumbnails moved"); + $this->extend('preFileMigrationSubtask', $subtask); + $this->logger->info("### Migrating existing thumbnails ({$subtask})"); + + $moved = LegacyThumbnailMigrationHelper::singleton() + ->setLogger($this->logger) + ->run($this->getStore()); + + if ($moved) { + $this->logger->info(sprintf("%d thumbnails moved", count($moved))); + } else { + $this->logger->info("No thumbnails moved"); + } + + $this->extend('postFileMigrationSubtask', $subtask); } } - if (in_array('generate-cms-thumbnails', $subtasks)) { + $subtask = 'generate-cms-thumbnails'; + if (in_array($subtask, $subtasks)) { if (!class_exists(ImageThumbnailHelper::class)) { $this->logger->error("ImageThumbnailHelper not found"); - return; + } else { + $this->extend('preFileMigrationSubtask', $subtask); + $this->logger->info("### Generating new CMS UI thumbnails ({$subtask})"); + ImageThumbnailHelper::singleton()->run(); + $this->extend('postFileMigrationSubtask', $subtask); } - $this->logger->info('### Generating new CMS UI thumbnails (generate-cms-thumbnails)'); - ImageThumbnailHelper::singleton()->run(); } - if (in_array('fix-folder-permissions', $subtasks)) { + $subtask = 'fix-folder-permissions'; + if (in_array($subtask, $subtasks)) { if (!class_exists(FixFolderPermissionsHelper::class)) { $this->logger->error("FixFolderPermissionsHelper not found"); - return; - } - - $this->logger->info('### Fixing folder permissions (fix-folder-permissions)'); - - $updated = FixFolderPermissionsHelper::singleton()->run(); - - if ($updated > 0) { - $this->logger->info("Repaired {$updated} folders with broken CanViewType settings"); } else { - $this->logger->info("No folders required fixes"); + $this->extend('preFileMigrationSubtask', $subtask); + + $this->logger->info("### Fixing folder permissions ({$subtask})"); + $updated = FixFolderPermissionsHelper::singleton()->run(); + + if ($updated > 0) { + $this->logger->info("Repaired {$updated} folders with broken CanViewType settings"); + } else { + $this->logger->info("No folders required fixes"); + } + + $this->extend('postFileMigrationSubtask', $subtask); } } - if (in_array('fix-secureassets', $subtasks)) { + $subtask = 'fix-secureassets'; + if (in_array($subtask, $subtasks)) { if (!class_exists(SecureAssetsMigrationHelper::class)) { $this->logger->error("SecureAssetsMigrationHelper not found"); - return; + } else { + $this->extend('preFileMigrationSubtask', $subtask); + + $this->logger->info("### Fixing secure-assets ({$subtask})"); + $moved = SecureAssetsMigrationHelper::singleton() + ->setLogger($this->logger) + ->run($this->getStore()); + + $this->extend('postFileMigrationSubtask', $subtask); } - - $this->logger->info('### Fixing secure-assets (fix-secureassets)'); - - $moved = SecureAssetsMigrationHelper::singleton() - ->setLogger($this->logger) - ->run($this->getStore()); } + + $this->extend('postFileMigration'); } public function getDescription() From 4f39e59aff00ca57e9ebb9694a29f69774964943 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 28 May 2019 09:21:11 +1200 Subject: [PATCH 5/7] BUG Enable file hash caching when running the file migration task (#8993) --- src/Dev/Tasks/MigrateFileTask.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 67e7aca9a..2d552be3b 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -7,8 +7,9 @@ use Monolog\Logger; use Psr\Log\LoggerInterface; use SilverStripe\AssetAdmin\Helper\ImageThumbnailHelper; use SilverStripe\Assets\Dev\Tasks\LegacyThumbnailMigrationHelper; -use SilverStripe\Assets\FileMigrationHelper; +use SilverStripe\Assets\Dev\Tasks\FileMigrationHelper; use SilverStripe\Assets\Storage\AssetStore; +use SilverStripe\Assets\Storage\FileHashingService; use SilverStripe\Control\Director; use SilverStripe\Core\Injector\Injector; use SilverStripe\Logging\PreformattedEchoHandler; @@ -46,6 +47,8 @@ class MigrateFileTask extends BuildTask $args = $request->getVars(); $this->validateArgs($args); + Injector::inst()->get(FileHashingService::class)->enableCache(); + $this->extend('preFileMigration'); $subtasks = !empty($args['only']) ? explode(',', $args['only']) : $this->defaultSubtasks; From 2c8c643ce39414f56a6bc72dca7a8a88b03095f9 Mon Sep 17 00:00:00 2001 From: Andre Kiste Date: Tue, 28 May 2019 09:45:13 +1200 Subject: [PATCH 6/7] MigrateFileTask now outputs "Done" when it has finished running (#8995) --- src/Dev/Tasks/MigrateFileTask.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 2d552be3b..e61714703 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -144,6 +144,8 @@ class MigrateFileTask extends BuildTask } } + $this->logger->info("Done!"); + $this->extend('postFileMigration'); } From 3e2fc6aa0bf21892f3944e56c2819b0601cc41dc Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 30 May 2019 09:34:34 +1200 Subject: [PATCH 7/7] Automated phpcbf linting --- src/Dev/Tasks/MigrateFileTask.php | 2 -- tests/php/ORM/SQLSelectTest.php | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index e61714703..7bdaca5e7 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -104,8 +104,6 @@ class MigrateFileTask extends BuildTask ImageThumbnailHelper::singleton()->run(); $this->extend('postFileMigrationSubtask', $subtask); } - - } $subtask = 'fix-folder-permissions'; diff --git a/tests/php/ORM/SQLSelectTest.php b/tests/php/ORM/SQLSelectTest.php index 7b36fa335..a97792cfc 100755 --- a/tests/php/ORM/SQLSelectTest.php +++ b/tests/php/ORM/SQLSelectTest.php @@ -854,7 +854,7 @@ class SQLSelectTest extends SapphireTest $sql = $query->sql(); $this->assertSQLEquals( - 'SELECT * FROM "MyTable" AS "MyTableAlias" , '. + 'SELECT * FROM "MyTable" AS "MyTableAlias" , ' . '(SELECT * FROM "MyTable" where "something" = "whatever") as "CrossJoin"', $sql );