From f46a8be648b978ae0d8e727d0f97bbf2656448d0 Mon Sep 17 00:00:00 2001
From: Adirelle <adirelle@gmail.com>
Date: Mon, 30 Mar 2015 08:56:30 +0200
Subject: [PATCH] LoggerConfig::getFor always returns the same instance of
 Logger for the same $name. This avoid issues when push handlers/processors to
 that logger. Use the Monolog\ErrorHandler to log errors and exceptions.
 PHPCI/Logging/Handler becomes PHPCI/ErrorHandler. And it only throws
 ErrorException for reported errors. No need to initialize a second
 $loggerConfig in daemonise.

Close #892
---
 PHPCI/ErrorHandler.php                   | 68 ++++++++++++++++++++++++
 PHPCI/Logging/LoggerConfig.php           | 17 +++++-
 Tests/PHPCI/Logging/LoggerConfigTest.php | 11 +++-
 bootstrap.php                            |  4 +-
 daemonise                                |  2 -
 5 files changed, 95 insertions(+), 7 deletions(-)
 create mode 100644 PHPCI/ErrorHandler.php

diff --git a/PHPCI/ErrorHandler.php b/PHPCI/ErrorHandler.php
new file mode 100644
index 00000000..d268965d
--- /dev/null
+++ b/PHPCI/ErrorHandler.php
@@ -0,0 +1,68 @@
+<?php
+/**
+ * PHPCI - Continuous Integration for PHP
+ *
+ * @copyright    Copyright 2014, Block 8 Limited.
+ * @license      https://github.com/Block8/PHPCI/blob/master/LICENSE.md
+ * @link         https://www.phptesting.org/
+ */
+
+namespace PHPCI;
+
+/**
+ * Error Handler
+ *
+ * @package PHPCI\Logging
+ */
+class ErrorHandler
+{
+    /**
+     * @var array
+     */
+    protected $levels = array(
+        E_WARNING           => 'Warning',
+        E_NOTICE            => 'Notice',
+        E_USER_ERROR        => 'User Error',
+        E_USER_WARNING      => 'User Warning',
+        E_USER_NOTICE       => 'User Notice',
+        E_STRICT            => 'Runtime Notice',
+        E_RECOVERABLE_ERROR => 'Catchable Fatal Error',
+        E_DEPRECATED        => 'Deprecated',
+        E_USER_DEPRECATED   => 'User Deprecated',
+    );
+
+    /**
+     * Registers an instance of the error handler to throw ErrorException.
+     */
+    public static function register()
+    {
+        $handler = new static();
+        set_error_handler(array($handler, 'handleError'));
+    }
+
+    /**
+     * @param integer $level
+     * @param string  $message
+     * @param string  $file
+     * @param integer $line
+     *
+     * @throws \ErrorException
+     *
+     * @internal
+     */
+    public function handleError($level, $message, $file, $line)
+    {
+        if (error_reporting() & $level === 0) {
+            return;
+        }
+
+        $exceptionLevel = isset($this->levels[$level]) ? $this->levels[$level] : $level;
+        throw new \ErrorException(
+            sprintf('%s: %s in %s line %d', $exceptionLevel, $message, $file, $line),
+            0,
+            $level,
+            $file,
+            $line
+        );
+    }
+}
diff --git a/PHPCI/Logging/LoggerConfig.php b/PHPCI/Logging/LoggerConfig.php
index 69617ee6..fd929aeb 100644
--- a/PHPCI/Logging/LoggerConfig.php
+++ b/PHPCI/Logging/LoggerConfig.php
@@ -9,6 +9,7 @@
 
 namespace PHPCI\Logging;
 
+use Monolog\ErrorHandler;
 use Monolog\Logger;
 
 /**
@@ -19,6 +20,7 @@ class LoggerConfig
 {
     const KEY_ALWAYS_LOADED = "_";
     private $config;
+    private $cache = array();
 
     /**
      * The filepath is expected to return an array which will be
@@ -56,9 +58,20 @@ class LoggerConfig
      */
     public function getFor($name)
     {
+        if (isset($this->cache[$name])) {
+            return $this->cache[$name];
+        }
+
         $handlers = $this->getHandlers(self::KEY_ALWAYS_LOADED);
-        $handlers = array_merge($handlers, $this->getHandlers($name));
-        return new Logger($name, $handlers);
+        if ($name !== self::KEY_ALWAYS_LOADED) {
+            $handlers = array_merge($handlers, $this->getHandlers($name));
+        }
+
+        $logger = new Logger($name, $handlers);
+        ErrorHandler::register($logger);
+        $this->cache[$name] = $logger;
+
+        return $logger;
     }
 
     /**
diff --git a/Tests/PHPCI/Logging/LoggerConfigTest.php b/Tests/PHPCI/Logging/LoggerConfigTest.php
index 3baf10c1..7fb95ef8 100644
--- a/Tests/PHPCI/Logging/LoggerConfigTest.php
+++ b/Tests/PHPCI/Logging/LoggerConfigTest.php
@@ -81,5 +81,14 @@ class LoggerConfigTest extends \PHPUnit_Framework_TestCase
         $this->assertSame($expectedHandler, $actualHandler);
         $this->assertNotSame($alternativeHandler, $actualHandler);
     }
-}
 
+    public function testGetFor_SameInstance()
+    {
+        $config = new LoggerConfig(array());
+
+        $logger1 = $config->getFor("something");
+        $logger2 = $config->getFor("something");
+
+        $this->assertSame($logger1, $logger2);
+    }
+}
diff --git a/bootstrap.php b/bootstrap.php
index e3f9985e..18d91d07 100644
--- a/bootstrap.php
+++ b/bootstrap.php
@@ -8,7 +8,6 @@
 */
 
 // Let PHP take a guess as to the default timezone, if the user hasn't set one:
-use PHPCI\Logging\Handler;
 use PHPCI\Logging\LoggerConfig;
 
 $timezone = ini_get('date.timezone');
@@ -43,9 +42,10 @@ if (!file_exists(dirname(__FILE__) . '/vendor/autoload.php') && defined('PHPCI_I
 // Load Composer autoloader:
 require_once(dirname(__FILE__) . '/vendor/autoload.php');
 
+\PHPCI\ErrorHandler::register();
+
 if (defined('PHPCI_IS_CONSOLE') && PHPCI_IS_CONSOLE) {
     $loggerConfig = LoggerConfig::newFromFile(__DIR__ . "/loggerconfig.php");
-    Handler::register($loggerConfig->getFor('_'));
 }
 
 // Load configuration if present:
diff --git a/daemonise b/daemonise
index 5117e414..c6238568 100755
--- a/daemonise
+++ b/daemonise
@@ -15,8 +15,6 @@ require('bootstrap.php');
 use PHPCI\Command\DaemoniseCommand;
 use Symfony\Component\Console\Application;
 
-$loggerConfig = \PHPCI\Logging\LoggerConfig::newFromFile(__DIR__ . "/loggerconfig.php");
-
 $application = new Application();
 $application->add(new DaemoniseCommand($loggerConfig->getFor('DaemoniseCommand')));
 $application->run();