From f7c1be7ac1aa2268a8a25d897fdf05959fbba8f8 Mon Sep 17 00:00:00 2001 From: Rafael Marins de Sousa Date: Tue, 28 May 2019 12:28:00 +1200 Subject: [PATCH 1/8] Including canCreate in if statement so that button gets removed if user isn't to be able to create new records --- src/Forms/GridField/GridFieldDetailForm_ItemRequest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index a5bcebb52..4b58032e4 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -323,7 +323,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $rightGroup->push($previousAndNextGroup); - if ($component && $component->getShowAdd()) { + if ($component && $component->getShowAdd() && $this->record->canCreate()) { $rightGroup->push( LiteralField::create( 'new-record', From 7301b375b82664bf8cf8f05637ef9aa384208447 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 28 May 2019 17:20:36 +1200 Subject: [PATCH 2/8] NEW Clearer file migration output with colours --- composer.json | 1 + src/Dev/Tasks/MigrateFileTask.php | 120 ++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 31 deletions(-) diff --git a/composer.json b/composer.json index 70628bb75..4e76d8bf9 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ "sake" ], "require": { + "bramus/monolog-colored-line-formatter": "~2.0", "composer/installers": "~1.0", "embed/embed": "^3.0", "league/csv": "^8", diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 7bdaca5e7..54ed5ffd9 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -15,6 +15,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Logging\PreformattedEchoHandler; use SilverStripe\Dev\BuildTask; use SilverStripe\Assets\Dev\Tasks\SecureAssetsMigrationHelper; +use \Bramus\Monolog\Formatter\ColoredLineFormatter; /** * Migrates all 3.x file dataobjects to use the new DBFile field. @@ -51,6 +52,11 @@ class MigrateFileTask extends BuildTask $this->extend('preFileMigration'); + $this->logger->warn( + 'Please read https://docs.silverstripe.org/en/4/developer_guides/files/file_migration/ ' . + 'before running this task.' + ); + $subtasks = !empty($args['only']) ? explode(',', $args['only']) : $this->defaultSubtasks; $subtask = 'move-files'; @@ -59,15 +65,23 @@ class MigrateFileTask extends BuildTask $this->logger->error("No file migration helper detected"); } else { $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->logger->notice("######################################################"); + $this->logger->notice("Migrating filesystem and database records ({$subtask})"); + $this->logger->notice("######################################################"); + + FileMigrationHelper::singleton() + ->setLogger($this->logger) + ->run(); + + // TODO Split file migration helper into two tasks, + // and report back on their process counts consistently here + // if ($count) { + // $this->logger->info("{$count} File objects upgraded"); + // } else { + // $this->logger->info("No File objects needed upgrading"); + // } + $this->extend('postFileMigrationSubtask', $subtask); } } @@ -78,16 +92,19 @@ class MigrateFileTask extends BuildTask $this->logger->error("LegacyThumbnailMigrationHelper not found"); } else { $this->extend('preFileMigrationSubtask', $subtask); - $this->logger->info("### Migrating existing thumbnails ({$subtask})"); - $moved = LegacyThumbnailMigrationHelper::singleton() + $this->logger->notice("#############################################################"); + $this->logger->notice("Migrating existing thumbnails to new file format ({$subtask})"); + $this->logger->notice("#############################################################"); + + $paths = LegacyThumbnailMigrationHelper::singleton() ->setLogger($this->logger) ->run($this->getStore()); - if ($moved) { - $this->logger->info(sprintf("%d thumbnails moved", count($moved))); + if ($paths) { + $this->logger->info(sprintf("%d thumbnails moved", count($paths))); } else { - $this->logger->info("No thumbnails moved"); + $this->logger->info("No thumbnails needed to be moved"); } $this->extend('postFileMigrationSubtask', $subtask); @@ -100,8 +117,21 @@ class MigrateFileTask extends BuildTask $this->logger->error("ImageThumbnailHelper not found"); } else { $this->extend('preFileMigrationSubtask', $subtask); - $this->logger->info("### Generating new CMS UI thumbnails ({$subtask})"); - ImageThumbnailHelper::singleton()->run(); + + $this->logger->notice("#############################################"); + $this->logger->notice("Generating new CMS UI thumbnails ({$subtask})"); + $this->logger->notice("#############################################"); + + $count = ImageThumbnailHelper::singleton() + ->setLogger($this->logger) + ->run(); + + if ($count > 0) { + $this->logger->info("Created {$count} CMS UI thumbnails"); + } else { + $this->logger->info("No CMS UI thumbnails needed to be created"); + } + $this->extend('postFileMigrationSubtask', $subtask); } } @@ -113,11 +143,17 @@ class MigrateFileTask extends BuildTask } else { $this->extend('preFileMigrationSubtask', $subtask); - $this->logger->info("### Fixing folder permissions ({$subtask})"); - $updated = FixFolderPermissionsHelper::singleton()->run(); + $this->logger->notice("####################################################"); + $this->logger->notice("Fixing secure-assets folder permissions ({$subtask})"); + $this->logger->notice("####################################################"); + $this->logger->debug('Only required if the 3.x project included silverstripe/secure-assets'); - if ($updated > 0) { - $this->logger->info("Repaired {$updated} folders with broken CanViewType settings"); + $count = FixFolderPermissionsHelper::singleton() + ->setLogger($this->logger) + ->run(); + + if ($count > 0) { + $this->logger->info("Repaired {$count} folders with broken CanViewType settings"); } else { $this->logger->info("No folders required fixes"); } @@ -133,11 +169,21 @@ class MigrateFileTask extends BuildTask } else { $this->extend('preFileMigrationSubtask', $subtask); - $this->logger->info("### Fixing secure-assets ({$subtask})"); - $moved = SecureAssetsMigrationHelper::singleton() + $this->logger->notice("#####################################################"); + $this->logger->notice("Fixing secure-assets folder restrictions ({$subtask})"); + $this->logger->notice("#####################################################"); + $this->logger->debug('Only required if the 3.x project included silverstripe/secure-assets'); + + $paths = SecureAssetsMigrationHelper::singleton() ->setLogger($this->logger) ->run($this->getStore()); + if (count($paths) > 0) { + $this->logger->info(sprintf("Repaired %d folders broken folder restrictions", count($paths))); + } else { + $this->logger->info("No folders required fixes"); + } + $this->extend('postFileMigrationSubtask', $subtask); } } @@ -151,9 +197,8 @@ class MigrateFileTask extends BuildTask { return <<get(LoggerInterface::class)) { - if (Director::is_cli()) { - $logger->pushHandler(new StreamHandler('php://stdout')); - $logger->pushHandler(new StreamHandler('php://stderr', Logger::WARNING)); - } else { - $logger->pushHandler(new PreformattedEchoHandler()); - } + // Create a default logger with a custom name (less misleading than "error-log") + $logger = Injector::inst()->create(LoggerInterface::class, 'log'); + + if (Director::is_cli()) { + $formatter = new ColoredLineFormatter(); + $formatter->ignoreEmptyContextAndExtra(); + + // Don't double process WARNING or higher levels in other handlers (bubble=false) + $errorHandler = new StreamHandler('php://stderr', Logger::ERROR, false); + $errorHandler->setFormatter($formatter); + + $standardHandler = new StreamHandler('php://stdout'); + $standardHandler->setFormatter($formatter); + + $logger->pushHandler($standardHandler); + $logger->pushHandler($errorHandler); + } else { + $logger->pushHandler(new PreformattedEchoHandler()); } + + $this->logger = $logger; } } From 2d4711de0100edccdbde94dc4794037828d13b14 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 5 Jun 2019 15:09:58 +1200 Subject: [PATCH 3/8] Fixed logging Broke loggers attached by queuedjobs because it wasn't using the global service. Since the stderr handler was set to bubble=false, those messages weren't picked up by queuedjobs. Removed preformatted handler since there's no longer an ability to run this stuff via web --- src/Dev/Tasks/MigrateFileTask.php | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 54ed5ffd9..32967ce81 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -2,6 +2,7 @@ namespace SilverStripe\Dev\Tasks; +use Monolog\Handler\FilterHandler; use Monolog\Handler\StreamHandler; use Monolog\Logger; use Psr\Log\LoggerInterface; @@ -243,25 +244,28 @@ TXT; */ protected function addLogHandlers() { - // Create a default logger with a custom name (less misleading than "error-log") - $logger = Injector::inst()->create(LoggerInterface::class, 'log'); + // Using a global service here so other systems can control and redirect log output, + // for example when this task is run as part of a queuedjob + $logger = Injector::inst()->get(LoggerInterface::class)->withName('log'); - if (Director::is_cli()) { $formatter = new ColoredLineFormatter(); $formatter->ignoreEmptyContextAndExtra(); - // Don't double process WARNING or higher levels in other handlers (bubble=false) - $errorHandler = new StreamHandler('php://stderr', Logger::ERROR, false); + $errorHandler = new StreamHandler('php://stderr', Logger::ERROR); $errorHandler->setFormatter($formatter); $standardHandler = new StreamHandler('php://stdout'); $standardHandler->setFormatter($formatter); - $logger->pushHandler($standardHandler); + // Avoid double logging of errors + $standardFilterHandler = new FilterHandler( + $standardHandler, + Logger::DEBUG, + Logger::WARNING + ); + + $logger->pushHandler($standardFilterHandler); $logger->pushHandler($errorHandler); - } else { - $logger->pushHandler(new PreformattedEchoHandler()); - } $this->logger = $logger; } From 30496144b992349690be20ffdbaa3796e1e8bb5d Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 5 Jun 2019 15:10:09 +1200 Subject: [PATCH 4/8] DOCS More detail on queuedjobs file migrations --- .../14_Files/05_File_Migration.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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 59fe7b3c4..c2168f8f1 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 @@ -37,7 +37,26 @@ This task will perform a number of subtasks: One or more subtasks can be run individually through the `only` argument. Example: `only=move-files,move-thumbnails` +The output is quite verbose by default. Look for `WARNING` and `ERROR` in the log files. +When executing the task on CLI, you'll get colour coded error messages. + +## Background migration through the Queuedjobs module + You can also run this task without CLI access through the [queuedjobs](https://github.com/symbiote/silverstripe-queuedjobs) module. +Open up `admin/queuedjobs`, then create a job of type `RunBuildTaskJob`. +The only constructor parameter allowed is the full name of the task: `SilverStripe\Dev\Tasks\MigrateFileTask`. +The task output will be progressively written to the job record, and can be inspected via the "Messages" tab within the job in the CMS. +It attempts to continue running to "complete" status even if it encounters errors, so you'll need to review the logs +to ensure if everything went smoothly. Note that it's currently not possible to run specific subtasks via a queuedjob. + +While you can run the job directly through the CMS, it'll usually be more constrained by PHP `max_execution_time` settings. +Many platforms such as the New Zealand Government Common Web Platform or SilverStripe Platform +are configured to run jobs automatically without time limits +([1](https://docs.platform.silverstripe.com/development/platform-yml-file/#cron-tasks), +[2](https://www.cwp.govt.nz/developer-docs/en/2/working_with_projects/infrastructural_considerations/)). +It is not recommended to run +[multiple processes](https://github.com/symbiote/silverstripe-queuedjobs/blob/master/docs/en/configuring-multi-process-execution.md) +when executing the file migration job. ## Migration of existing thumbnails From b21e5d9e579b62b2b2e156df80657058f045c368 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 5 Jun 2019 15:10:46 +1200 Subject: [PATCH 5/8] Moved time limit increases from individual job Should apply to all file migration subtasks, not just the first one (see silverstripe/assets) --- src/Dev/Tasks/MigrateFileTask.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index 32967ce81..e23ecbf9c 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -12,6 +12,7 @@ use SilverStripe\Assets\Dev\Tasks\FileMigrationHelper; use SilverStripe\Assets\Storage\AssetStore; use SilverStripe\Assets\Storage\FileHashingService; use SilverStripe\Control\Director; +use SilverStripe\Core\Environment; use SilverStripe\Core\Injector\Injector; use SilverStripe\Logging\PreformattedEchoHandler; use SilverStripe\Dev\BuildTask; @@ -51,6 +52,10 @@ class MigrateFileTask extends BuildTask Injector::inst()->get(FileHashingService::class)->enableCache(); + // Set max time and memory limit + Environment::increaseTimeLimitTo(); + Environment::increaseMemoryLimitTo(); + $this->extend('preFileMigration'); $this->logger->warn( @@ -248,14 +253,14 @@ TXT; // for example when this task is run as part of a queuedjob $logger = Injector::inst()->get(LoggerInterface::class)->withName('log'); - $formatter = new ColoredLineFormatter(); - $formatter->ignoreEmptyContextAndExtra(); + $formatter = new ColoredLineFormatter(); + $formatter->ignoreEmptyContextAndExtra(); $errorHandler = new StreamHandler('php://stderr', Logger::ERROR); - $errorHandler->setFormatter($formatter); + $errorHandler->setFormatter($formatter); - $standardHandler = new StreamHandler('php://stdout'); - $standardHandler->setFormatter($formatter); + $standardHandler = new StreamHandler('php://stdout'); + $standardHandler->setFormatter($formatter); // Avoid double logging of errors $standardFilterHandler = new FilterHandler( @@ -265,7 +270,7 @@ TXT; ); $logger->pushHandler($standardFilterHandler); - $logger->pushHandler($errorHandler); + $logger->pushHandler($errorHandler); $this->logger = $logger; } From e8fa3d8750eccc911d0227f016a7bb8272f29600 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 6 Jun 2019 09:30:50 +1200 Subject: [PATCH 6/8] DOCS Fix incorrect deprecation reference in MonologErrorHandler --- src/Logging/MonologErrorHandler.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Logging/MonologErrorHandler.php b/src/Logging/MonologErrorHandler.php index 28f355c32..c45e96422 100644 --- a/src/Logging/MonologErrorHandler.php +++ b/src/Logging/MonologErrorHandler.php @@ -18,13 +18,13 @@ class MonologErrorHandler implements ErrorHandler * Set the PSR-3 logger to send errors & exceptions to. Will overwrite any previously configured * loggers * - * @deprecated 4.4.0:5.0.0 Use pushHandler() instead + * @deprecated 4.4.0:5.0.0 Use pushLogger() instead * @param LoggerInterface $logger * @return $this */ public function setLogger(LoggerInterface $logger) { - Deprecation::notice('4.4.0', 'Please use pushHandler() instead'); + Deprecation::notice('4.4.0', 'Please use pushLogger() instead'); $this->loggers = [$logger]; return $this; @@ -33,12 +33,12 @@ class MonologErrorHandler implements ErrorHandler /** * Get the first registered PSR-3 logger to send errors & exceptions to * - * @deprecated 4.4.0:5.0.0 Use getHandlers() instead + * @deprecated 4.4.0:5.0.0 Use getLoggers() instead * @return LoggerInterface */ public function getLogger() { - Deprecation::notice('4.4.0', 'Please use getHandlers() instead'); + Deprecation::notice('4.4.0', 'Please use getLoggers() instead'); return reset($this->loggers); } From f4cdfb06c8801e10459ba4d32713a57a091b2126 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 6 Jun 2019 15:18:12 +1200 Subject: [PATCH 7/8] Update environment timeouts See https://github.com/silverstripe/silverstripe-framework/issues/9029 --- src/Dev/Tasks/MigrateFileTask.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Dev/Tasks/MigrateFileTask.php b/src/Dev/Tasks/MigrateFileTask.php index e23ecbf9c..eda5cf33f 100644 --- a/src/Dev/Tasks/MigrateFileTask.php +++ b/src/Dev/Tasks/MigrateFileTask.php @@ -54,7 +54,8 @@ class MigrateFileTask extends BuildTask // Set max time and memory limit Environment::increaseTimeLimitTo(); - Environment::increaseMemoryLimitTo(); + Environment::setMemoryLimitMax(-1); + Environment::increaseMemoryLimitTo(-1); $this->extend('preFileMigration'); From 8324235eda1b6254b29c28c7ac418027d14c6a4e Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 6 Jun 2019 12:19:16 +1200 Subject: [PATCH 8/8] API Opt-out of in-memory caching factory In-memory caches are typically more resource constrained (number of items and storage space). Give cache consumers an opt-out if they are expecting to create large caches with long lifetimes. Use case: https://github.com/silverstripe/silverstripe-assets/pull/282 --- src/Core/Cache/DefaultCacheFactory.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Core/Cache/DefaultCacheFactory.php b/src/Core/Cache/DefaultCacheFactory.php index a9417095c..b46ff12b2 100644 --- a/src/Core/Cache/DefaultCacheFactory.php +++ b/src/Core/Cache/DefaultCacheFactory.php @@ -56,8 +56,12 @@ class DefaultCacheFactory implements CacheFactory $directory = isset($args['directory']) ? $args['directory'] : null; $version = isset($args['version']) ? $args['version'] : null; + // In-memory caches are typically more resource constrained (number of items and storage space). + // Give cache consumers an opt-out if they are expecting to create large caches with long lifetimes. + $useInMemoryCache = isset($args['useInMemoryCache']) ? $args['useInMemoryCache'] : true; + // Check support - $apcuSupported = $this->isAPCUSupported(); + $apcuSupported = ($this->isAPCUSupported() && $useInMemoryCache); $phpFilesSupported = $this->isPHPFilesSupported(); // If apcu isn't supported, phpfiles is the next best preference @@ -72,8 +76,11 @@ class DefaultCacheFactory implements CacheFactory } // Chain this cache with ApcuCache + // Note that the cache lifetime will be shorter there by default, to ensure there's enough + // resources for "hot cache" items in APCu as a resource constrained in memory cache. $apcuNamespace = $namespace . ($namespace ? '_' : '') . md5(BASE_PATH); $apcu = $this->createCache(ApcuCache::class, [$apcuNamespace, (int) $defaultLifetime / 5, $version]); + return $this->createCache(ChainCache::class, [[$apcu, $fs]]); }