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

Initial work on moving the Processor for messages to a separate class #21

Open
wants to merge 1 commit into
base: v2_future
Choose a base branch
from
Open

Initial work on moving the Processor for messages to a separate class #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
98 changes: 27 additions & 71 deletions lib/Handler/AbstractHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;

/**
* Abstract Handler to simplify the implementation of a Handler Interface
*/
abstract class AbstractHandler implements HandlerInterface
{
protected $logger = null;
protected $processor = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docblock:

/**
 * @var ProcessorInterface
 **/

public $verbose = 0;
public $minLevel = 7;
const LOG_LEVELS = array(
Expand All @@ -30,9 +33,20 @@ abstract class AbstractHandler implements HandlerInterface
*
* @param string $minimalLevel the minimal severity of the LogLevel to start logging with
* @param integer $verbose the Verbosity of the Log
* @param array $additionalParameters optional additional parameters required for the Handler
* @param \Productsup\Flexilog\Processor $processor the Processor to be used for the Handler, if none supplied
Copy link
Contributor

Choose a reason for hiding this comment

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

\Productsup\Flexilog\Processor can be replaced with ProcessorInterface

* the DefaultProcessor one will be used.
*/
public function __construct($minimalLevel = 'debug', $verbose = 0)
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

PSR-2 Compliant would be:

public function __construct(
    $minimalLevel = 'debug',
    $verbose = 0,
    array $additionalParameters = array(),
    ProcessorInterface $processor = null
) {

{
if (is_null($processor)) {
$this->processor = new \Productsup\Flexilog\Processor\DefaultProcessor();
} else {
$this->processor = $processor;
Copy link

Choose a reason for hiding this comment

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

$this->processor = $processor ?? new \Productsup\Flexilog\Processor\DefaultProcessor;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make the entire codebase PHP7 minimum, since this feature didn't exist before. http://php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op

Currently the codebase is PHP5.6 compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about:

if (is_null($processor)) {
    $processor = new \Productsup\Flexilog\Processor\DefaultProcessor();
}
$this->processor = $processor;

}
$this->verbose = $verbose;
if (array_key_exists($minimalLevel, self::LOG_LEVELS)) {
$this->minLevel = self::LOG_LEVELS[$minimalLevel];
Expand All @@ -46,88 +60,30 @@ public function init()
{
}

/**
* Set the Logger for the Handler
*
* @param \Productsup\Flexilog\Logger $logger
*
* @return HandlerInterface $this
*/
public function setLogger(\Productsup\Flexilog\Logger $logger)
public function setProcessor(\Productsup\Flexilog\Processor $processor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docblock.

It the property $processor now a Processor or a ProcessorInterface? Big difference imo.

{
$this->logger = $logger;
$this->processor = $processor;

return $this;
}

/**
* Interpolates context values into the message placeholders.
*
* @param string $message Message to Log with Placeholders, defined by {curly}-braces.
* @param array $context Key/Value array with properties for the Placeholders.
*
* @return string $message Message with Placeholders replaced by the context.
*/
public function interpolate($message, array $context = array())
public function getProcessor()
{
// build a replacement array with braces around the context keys
$replace = array();
foreach ($context as $key => $val) {
if (is_numeric($val)) {
$val = (string) $val;
}
if (is_string($val)) {
$replace['{'.$key.'}'] = $val;
}
}

// interpolate replacement values into the message and return
return strtr($message, $replace);
return $this->processor;
}

/**
* Prepare the Context before interpolation
* Turns Objects into String representations.
* Set the Logger for the Handler
*
* @param array $context Key/Value array with properties for the Placeholders.
* @param \Productsup\Flexilog\Logger $logger
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a Flexilog/LoggerInterface? Which theoretically another logger could create a Flexilog compatible class?

*
* @return array $conext Cleaned context
* @return HandlerInterface $this
*/
public function prepareContext(array $context)
public function setLogger(\Productsup\Flexilog\Logger $logger)
{
// cleanup any thrown exceptions
foreach ($context as $contextKey => $contextObject) {
// some reserved keywords
$reserved = array('date');
if (in_array($contextKey, $reserved)) {
// prepend with an underscore
$newkey = '_'.$contextKey;
$context[$newkey] = $context[$contextKey];
unset($context[$contextKey]);
$contextKey = $newkey;
}

if ($contextObject instanceof \Exception) {
$contextObject = $contextObject->__toString();
} elseif (is_array($contextObject)) {
$contextObject = json_encode($contextObject, true);
} elseif ($contextObject instanceof \DateTime) {
$contextObject = $contextObject->format(\DateTime::RFC3339);
} elseif (is_object($contextObject)) {
$contextObject = $contextObject->__toString();
} elseif (is_resource($contextObject)) {
$contextObject = get_resource_type($contextObject);
}

$context[$contextKey] = $contextObject;

// clean empty values
if (empty($contextObject) && (string) $contextObject !== '0') {
unset($context[$contextKey]);
}
}
$this->logger = $logger;

return $context;
return $this;
}

/**
Expand All @@ -148,8 +104,8 @@ public function prepare($level, $message, array $context = array())
$logInfo->validate();
$context = array_merge($context, $logInfo->getData());
$context['loglevel'] = $level;
$context = $this->prepareContext($context);
$message = $this->interpolate($message, $context);
$context = $this->processor->prepareContext($context);
$message = $this->processor->interpolate($message, $context);

return array($message, $context);
}
Expand Down
8 changes: 0 additions & 8 deletions lib/Handler/ArrayHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ class ArrayHandler extends AbstractHandler
{
public $array = array();

/**
* {@inheritDoc}
*/
public function __construct($minimalLevel = 'debug', $verbose = 0)
{
parent::__construct($minimalLevel, $verbose);
}

/**
* {@inheritDoc}
*/
Expand Down
16 changes: 10 additions & 6 deletions lib/Handler/FileHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;

/**
* Write to a specified File
*/
Expand All @@ -14,13 +16,16 @@ class FileHandler extends AbstractHandler
*
* @param array $additionalParameters Pass an array with the `filename` as a key/value to be used.
*/
public function __construct($minimalLevel, $verbose, $additionalParameters = array())
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR-2 compliant. See http://www.php-fig.org/psr/psr-2/#44-method-arguments

{
if (!isset($additionalParameters['filename'])) {
throw new \Exception('Filename parameter must be set');
}
$filename = $additionalParameters['filename'];
parent::__construct($minimalLevel, $verbose);
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
if ((!file_exists($filename) && file_put_contents($filename, '') === false) ||!is_writable($filename)) {
throw new \Exception('No write permission on file:'.$filename);
Copy link
Contributor

Choose a reason for hiding this comment

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

Exception not listed in docblock, please add:

/**
 * @throws Exception <Explanation when exception is thrown>
 **/

}
Expand All @@ -35,13 +40,12 @@ public function __construct($minimalLevel, $verbose, $additionalParameters = arr
*/
public function write($level, $message, array $context = array())
{
$line = sprintf('%s %s: %s%s', date('H:i:s'), strtoupper($level), $message, PHP_EOL);
$this->writeToFile($line);
$this->writeToFile($this->processor->decorateMessage($level, $message, $context));

if ($this->verbose >= 1) {
$this->writeToFile("Extra Variables:".PHP_EOL);
$this->writeToFile($this->processor->contextSeparator());
foreach ($context as $contextKey => $contextObject) {
$this->writeToFile(sprintf("\t%s: %s%s", $contextKey, $contextObject, PHP_EOL));
$this->writeToFile($this->processor->decorateContext($level, $contextKey, $contextObject));
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions lib/Handler/GelfHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;
use Productsup\Flexilog\Exception\HandlerException;
use Productsup\Flexilog\Exception\HandlerConnectionException;
use Gelf;
Expand All @@ -19,13 +20,16 @@ class GelfHandler extends AbstractHandler
*
* @param $additionalParameters array Pass the `server` and `port` as a key/value array
*/
public function __construct($minimalLevel, $verbose, $additionalParameters = array())
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.

{
if (!isset($additionalParameters['server'])) {
throw new HandlerException('Server parameter must be set inside the $additionalParameters');
Copy link
Contributor

Choose a reason for hiding this comment

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

Exception not listed in docblock, should be:

/**
 * @throws HandlerException <Text about when exception is thrown>
 **/

}
$port = isset($additionalParameters['port']) ? $additionalParameters['port'] : Gelf\Transport\UdpTransport::DEFAULT_PORT;
parent::__construct($minimalLevel, $verbose);
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
$this->transport = new Gelf\Transport\UdpTransport(
$additionalParameters['server'],
$port,
Expand Down Expand Up @@ -79,8 +83,8 @@ public function write($level, $message, array $context = array())
if (isset($context['fullMessage'])) {
$fullMessage = $context['fullMessage'];
unset($context['fullMessage']);
$fullMessage = $this->interpolate($fullMessage, $this->logger->getLogInfo()->getData());
$fullMessage = $this->interpolate($fullMessage, $context);
$fullMessage = $this->processor->interpolate($fullMessage, $this->logger->getLogInfo()->getData());
$fullMessage = $this->processor->interpolate($fullMessage, $context);
$splitFullMessage = $this->splitMessage($fullMessage);
}

Expand Down
15 changes: 15 additions & 0 deletions lib/Handler/HandlerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,26 @@

namespace Productsup\Flexilog\Handler;

use \Productsup\Flexilog\Processor\ProcessorInterface;

/**
* Handler Interface for the Flexilog endpoint handlers
*/
interface HandlerInterface
{
/**
* Construct the Handler, optionally with a minimal logging level
*
* @param string $minimalLevel the minimal severity of the LogLevel to start logging with
* @param integer $verbose the Verbosity of the Log
* @param array $additionalParameters optional additional parameters required for the Handler
* @param \Productsup\Flexilog\Processor $processor the Processor to be used for the Handler, if none supplied
Copy link
Contributor

Choose a reason for hiding this comment

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

Code states fourth parameter should be ProcessorInterface, docblock states Proccesor.

* the DefaultProcessor one will be used.
*/
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.


/**
* Write received Log information through the Handlers mechanism
Expand Down
24 changes: 24 additions & 0 deletions lib/Handler/LogfileHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;
use Productsup\Flexilog\Processor\LogfileProcessor;

/**
* Write to a specified File
*/
class LogfileHandler extends FileHandler
{
/**
* {@inheritDoc}
*/
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.

$processor = new LogfileProcessor();
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
}
}
8 changes: 6 additions & 2 deletions lib/Handler/RedisHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;
use Productsup\Flexilog\Exception\HandlerException;
use Productsup\Flexilog\Exception\HandlerConnectionException;
use Redis;
Expand All @@ -18,7 +19,10 @@ class RedisHandler extends AbstractHandler
/**
* {@inheritDoc}
*/
public function __construct($minimalLevel, $verbose, $additionalParameters = array())
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.

{
if (!class_exists('Redis')) {
throw new HandlerException('Class Redis is not found');
Copy link
Contributor

Choose a reason for hiding this comment

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

Exception not documented in docblock.

Expand All @@ -35,7 +39,7 @@ public function __construct($minimalLevel, $verbose, $additionalParameters = arr
}
$this->redis = new Redis();
$this->redisConfig = $redisConfig;
parent::__construct($minimalLevel, $verbose);
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions lib/Handler/ShellHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;
use League;

/**
Expand All @@ -14,9 +15,12 @@ class ShellHandler extends AbstractHandler
/**
* {@inheritDoc}
*/
public function __construct($minimalLevel = 'debug', $verbose = 0)
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.

{
parent::__construct($minimalLevel, $verbose);
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
$this->CLImate = new League\CLImate\CLImate();
$this->CLImate->output->defaultTo('error');
}
Expand Down
8 changes: 6 additions & 2 deletions lib/Handler/SymfonyConsoleHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Productsup\Flexilog\Handler;

use Productsup\Flexilog\Processor\ProcessorInterface;
use Productsup\Flexilog\Log\LogLevel;
use Productsup\Flexilog\Exception\HandlerException;
use Symfony\Component\Console\Output\OutputInterface;
Expand Down Expand Up @@ -43,12 +44,15 @@ class SymfonyConsoleHandler extends AbstractHandler
/**
* {@inheritDoc}
*/
public function __construct($minimalLevel = 'debug', $verbose = 0, $additionalParameters = array())
public function __construct($minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not PSR2 compliant.

{
if (!isset($additionalParameters['outputInterface'])) {
throw new HandlerException('OutputInterface parameter must be set');
}
parent::__construct($minimalLevel, $verbose);
parent::__construct($minimalLevel, $verbose, $additionalParameters, $processor);
$this->outputInterface = $additionalParameters['outputInterface'];
}

Expand Down
Loading