Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PreformattedEchoHandler #8930

Merged
merged 3 commits into from
Apr 18, 2019
Merged

Add PreformattedEchoHandler #8930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d24ff4
Add `PreformattedEchoHandler`
bergice Apr 16, 2019
71964e0
Batch log messages for every 100 file.
bergice Apr 17, 2019
3442530
Update src/Logging/PreformattedEchoHandler.php
ScopeyNZ Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/Dev/Tasks/MigrateFileTask.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

namespace SilverStripe\Dev\Tasks;

use Monolog\Handler\StreamHandler;
use Monolog\Logger;
use Psr\Log\LoggerInterface;
use SilverStripe\AssetAdmin\Helper\ImageThumbnailHelper;
use SilverStripe\Control\Director;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Logging\PreformattedEchoHandler;
use SilverStripe\ORM\DB;
use SilverStripe\Assets\FileMigrationHelper;
use SilverStripe\Dev\BuildTask;
Expand All @@ -12,7 +18,6 @@
*/
class MigrateFileTask extends BuildTask
{

private static $segment = 'MigrateFileTask';

protected $title = 'Migrate File dataobjects from 3.x';
Expand All @@ -23,6 +28,8 @@ class MigrateFileTask extends BuildTask

public function run($request)
{
$this->addLogHandlers();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we do that in the constructor rather than the run method?


if (!class_exists(FileMigrationHelper::class)) {
DB::alteration_message("No file migration helper detected", "notice");
return;
Expand All @@ -46,4 +53,22 @@ public function run($request)
}
ImageThumbnailHelper::singleton()->run();
}

/**
* TODO Refactor this whole mess into Symfony Console on a TaskRunner level,
* with a thin wrapper to show coloured console output via a browser:
* https://github.com/silverstripe/silverstripe-framework/issues/5542
* @throws \Exception
*/
protected function addLogHandlers()
{
if ($logger = Injector::inst()->get(LoggerInterface::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we have a prototype specialisation, like

  Psr\Log\LoggerInterface.monolog:
    type: prototype
    class: Monolog\Logger

rather than taking the service and mutate its global state in here?

if (Director::is_cli()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might worth having a separate logger class implementation rather than encoding this logic in here

$logger->pushHandler(new StreamHandler('php://stdout'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoggerInterface doesn't have pushHandler methods

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this is one of those cases where we squint a bit, and just assume that people will not apply loggers other than Monolog for now. We're effectively using LoggerInterface as a global singleton here, since @bergice other PR on queuedjobs is configuring that singleton with additional handlers, so both of these are assuming to work off the same instance. It's pretty messy, but the answer is not to use log handlers here at all, but rather Symfony Console.

$logger->pushHandler(new StreamHandler('php://stderr', Logger::WARNING));
} else {
$logger->pushHandler(new PreformattedEchoHandler());
}
}
}
}
27 changes: 27 additions & 0 deletions src/Logging/PreformattedEchoHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace SilverStripe\Logging;

use Monolog\Handler\AbstractProcessingHandler;

/**
* Echo the output as preformatted HTML, emulating console output in a browser.
* Tiding us over until we can properly decoupled web from CLI output.
* Do not use this API outside of core modules,
* it'll likely be removed as part of a larger refactor.
*
* See https://github.com/silverstripe/silverstripe-framework/issues/5542
*
* @internal
*/
class PreformattedEchoHandler extends AbstractProcessingHandler
{

/**
* @param array $record
*/
protected function write(array $record)
{
echo sprintf('<pre>%s</pre>', htmlspecialchars($record['formatted'], ENT_QUOTES, 'UTF-8'));
}
}