Silverstripe-framework: RFC: Generic (PSR-3) output interface for BuildTasks and controllers to send output to CLI

Created on 1 Jan 2017 · 10Comments · Source: silverstripe/silverstripe-framework

Introduction

There are numerous places throughout the framework that you find things like echo "Processed {$someVar}\n"; - examples are in DatabaseAdmin::doBuild and most instances of BuildTask.

This RFC proposes to provide a common interface to use instead of direct output.

Why?

Because stdout output is not each to handle from anything using it externally. For example, if I'm writing a Symfony console application that triggers DatabaseAdmin::doBuild, the output from the build is going to be sent straight to stdout. I can buffer it, but then I won't see any results until the buffer is flushed (at the end).

Proposal

Provide a common method (probably from Object) which can be used instead of direct echo calls. This method would abstract any output to the CLI so that it can be handled in whatever way is required at the time.

Perhaps using a PSR-3 compatible interface might make sense since we have it in place already. In this instance, the Monolog StreamHandler could be configured to send messages to stdout. It would probably need a custom implementation to remove any additional information such as dates and times from the output message.

Quick example

I can imagine the logging.yml configuration getting a new CliLogger which is configured to return a new instance of Monolog with a configured StreamHandler pushed. In that case, the example method above might look like this:

# Class: Object
protected function output($message, $newline = true)
{
    if ($newline) {
        $message .= PHP_EOL;
    }
    Injector::inst()->get('CliLogger')->addInfo($message);
}

# File: logging.yml
  CliLogger:
    type: singleton
    class: Monolog\Logger
    constructor:
      - "cli-log"
    calls:
      CliOutputHandler: [ pushHandler, [ %$CliOutputHandler ]]
  CliOutputHandler:
    class: Monolog\Handler\StreamHandler
    constructor:
      - 'php://stdout'
      - 100 # Minimum log level. Would be better as a constant!
    calls:
      SetFormatter: [ setFormatter, [ %$CliOutputFormatter ]]
  CliOutputFormatter:
    class: Monolog\Formatter\LineFormatter
    constructor:
      - '%message%' # Specifies the format - no dates, etc
      - null 
      - true        # Allow inline line breaks

Using the above, an Object instance can call $this->output('Hello world') and have it written to stdout when running from the command line.

Other considerations

Add some kind of abstraction between SilverStripe's Object and Monolog's Logger - a CliLoggerService class maybe - sole purpose is to interface a method like output to addWarn
Add setter and getter for the logger service

This would allow me to define my own logger service if I didn't want those messages going to stdout by default.

Alternatively expose the CLI Logger instance with a getter, allowing people to configure the Logger without needing a service class in SilverStripe. The default configuration would be as provided in the example YAML above.

affectv4 changmajor rfdraft

Source

robbieaverill

👍3

All 10 comments

This seems like a good idea - we really should avoid using "echo" where we can.

I don't think Object is the right place to put the output method, perhaps it should be some dedicated global response/output interface...

dhensby on 3 Jan 2017

I don't think Object is the right place to put the output method, perhaps it should be some dedicated global response/output interface...

Agreed. I wonder whether it is worth going so far as a PSR-7 interface, or whether a standalone class like Direct::output might be sufficient.

robbieaverill on 3 Jan 2017

I think that noting PSR-3 here is incorrect, since that would add many levels that we probably don't need here.

I wonder if PSR-7 would be more suitable. That's probably overkill too though. We really just need to abstract out the output so I could do something like $this->getOutput()->write('This is the equivalent of "echo"') or ->error('This would write to stderr') with a getter and setter for the output interface.

I'd like to see the input ($request) abstracted out as well, but this would be a little different since BuildTask classes would be used to expecting it to be an HTTPRequest class.

Only changing the output would be B/C and extensible.

I've added a bit of a POC mockup here: https://github.com/silverstripe/silverstripe-framework/compare/master...robbieaverill:feature/cli-output-interface?expand=1

Usage from a task would be:

public function run($request)
{
    $output = $this->getOutputInterface();

    $output->write('Test message');
    $output->error('Test error message');
    $output->write('Another Test message');
    $output->error('Another Test error message');
}

robbieaverill on 11 Jan 2017

PSR-3/LoggerInterface added back in again to minimise any extra dependencies required for this to be achieved.

robbieaverill on 18 Jan 2017

Core things:

interface Task {
  /**
   * @return array Map of available parameters and their descriptions
   */
  function getParams();
  function run($params, LoggerInterface $log);
}