From b4298c68ca3b79abb7413452311daf074206c31c Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Wed, 7 May 2014 01:55:06 +0200 Subject: [PATCH 1/4] - make logger available in the container - inject logger class into log - adding PHPDoc comments and fixing typos --- lib/private/log.php | 30 ++++++++---- lib/private/server.php | 27 +++++++++-- lib/public/ilogger.php | 101 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 13 deletions(-) create mode 100644 lib/public/ilogger.php diff --git a/lib/private/log.php b/lib/private/log.php index e0b9fe3c696..c7a3b99a5e0 100644 --- a/lib/private/log.php +++ b/lib/private/log.php @@ -8,6 +8,8 @@ namespace OC; +use \OCP\ILogger; + /** * logging utilities * @@ -18,8 +20,24 @@ namespace OC; * MonoLog is an example implementing this interface. */ -class Log { - private $logClass; +class Log implements ILogger { + + private $logger; + + /** + * @param string $logger The logger that should be used + */ + public function __construct($logger=null) { + // FIXME: Add this for backwards compatibility, should be fixed at some point probably + if($logger === null) { + $this->logger = 'OC_Log_'.ucfirst(\OC_Config::getValue('log_type', 'owncloud')); + call_user_func(array($this->logger, 'init')); + } else { + $this->logger = $logger; + } + + } + /** * System is unusable. @@ -112,10 +130,6 @@ class Log { $this->log(\OC_Log::DEBUG, $message, $context); } - public function __construct() { - $this->logClass = 'OC_Log_'.ucfirst(\OC_Config::getValue('log_type', 'owncloud')); - call_user_func(array($this->logClass, 'init')); - } /** * Logs with an arbitrary level. @@ -130,7 +144,7 @@ class Log { } else { $app = 'no app in context'; } - $logClass=$this->logClass; - $logClass::write($app, $message, $level); + $logger=$this->logger; + $logger::write($app, $message, $level); } } diff --git a/lib/private/server.php b/lib/private/server.php index 4c29092cf44..52dd56e291e 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -30,9 +30,9 @@ class Server extends SimpleContainer implements IServerContainer { } if (\OC::$session->exists('requesttoken')) { - $requesttoken = \OC::$session->get('requesttoken'); + $requestToken = \OC::$session->get('requesttoken'); } else { - $requesttoken = false; + $requestToken = false; } if (defined('PHPUNIT_RUN') && PHPUNIT_RUN @@ -54,7 +54,7 @@ class Server extends SimpleContainer implements IServerContainer { ? $_SERVER['REQUEST_METHOD'] : null, 'urlParams' => $urlParams, - 'requesttoken' => $requesttoken, + 'requesttoken' => $requestToken, ), $stream ); }); @@ -158,6 +158,14 @@ class Server extends SimpleContainer implements IServerContainer { $this->registerService('AvatarManager', function($c) { return new AvatarManager(); }); + $this->registerService('Logger', function($c) { + /** @var $c SimpleContainer */ + $logClass = $c->query('AllConfig')->getSystemValue('log_type', 'owncloud'); + $logger = 'OC_Log_' . ucfirst($logClass); + call_user_func(array($logger, 'init')); + + return new Log($logger); + }); $this->registerService('JobList', function ($c) { /** * @var Server $c @@ -325,14 +333,14 @@ class Server extends SimpleContainer implements IServerContainer { } /** - * @return \OC\URLGenerator + * @return \OCP\IURLGenerator */ function getURLGenerator() { return $this->query('URLGenerator'); } /** - * @return \OC\Helper + * @return \OCP\IHelper */ function getHelper() { return $this->query('AppHelper'); @@ -392,6 +400,15 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('JobList'); } + /** + * Returns a logger instance + * + * @return \OCP\ILogger + */ + function getLogger(){ + return $this->query('Logger'); + } + /** * Returns a router for generating and matching urls * diff --git a/lib/public/ilogger.php b/lib/public/ilogger.php new file mode 100644 index 00000000000..ad0fcd05a1d --- /dev/null +++ b/lib/public/ilogger.php @@ -0,0 +1,101 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP; + +/** + * Interface ILogger + * @package OCP + * + * This logger interface follows the design guidelines of PSR-3 + * https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#3-psrlogloggerinterface + */ +interface ILogger { + /** + * System is unusable. + * + * @param string $message + * @param array $context + * @return null + */ + function emergency($message, array $context = array()); + + /** + * Action must be taken immediately. + * + * @param string $message + * @param array $context + * @return null + */ + function alert($message, array $context = array()); + + /** + * Critical conditions. + * + * @param string $message + * @param array $context + * @return null + */ + function critical($message, array $context = array()); + + /** + * Runtime errors that do not require immediate action but should typically + * be logged and monitored. + * + * @param string $message + * @param array $context + * @return null + */ + function error($message, array $context = array()); + + /** + * Exceptional occurrences that are not errors. + * + * @param string $message + * @param array $context + * @return null + */ + function warning($message, array $context = array()); + + /** + * Normal but significant events. + * + * @param string $message + * @param array $context + * @return null + */ + function notice($message, array $context = array()); + + /** + * Interesting events. + * + * @param string $message + * @param array $context + * @return null + */ + function info($message, array $context = array()); + + /** + * Detailed debug information. + * + * @param string $message + * @param array $context + * @return null + */ + function debug($message, array $context = array()); + + /** + * Logs with an arbitrary level. + * + * @param mixed $level + * @param string $message + * @param array $context + * @return mixed + */ + function log($level, $message, array $context = array()); +} From d853c60d7e4eba63c8cb402249b7b77b94c4c764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 12 May 2014 10:54:09 +0200 Subject: [PATCH 2/4] adding interpolation as requested by PSR-3 --- lib/private/log.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/private/log.php b/lib/private/log.php index c7a3b99a5e0..682f6321474 100644 --- a/lib/private/log.php +++ b/lib/private/log.php @@ -144,6 +144,15 @@ class Log implements ILogger { } else { $app = 'no app in context'; } + // interpolate $message as defined in PSR-3 + $replace = array(); + foreach ($context as $key => $val) { + $replace['{' . $key . '}'] = $val; + } + + // interpolate replacement values into the message and return + $message = strtr($message, $replace); + $logger=$this->logger; $logger::write($app, $message, $level); } From 9d95fff427e5f09e5a31b9da7b85ca852688851d Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 12 May 2014 13:32:03 +0200 Subject: [PATCH 3/4] fix missing spaces --- lib/private/log.php | 4 ++-- lib/private/server.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/log.php b/lib/private/log.php index 682f6321474..98465ec40ea 100644 --- a/lib/private/log.php +++ b/lib/private/log.php @@ -21,7 +21,7 @@ use \OCP\ILogger; */ class Log implements ILogger { - + private $logger; /** @@ -153,7 +153,7 @@ class Log implements ILogger { // interpolate replacement values into the message and return $message = strtr($message, $replace); - $logger=$this->logger; + $logger = $this->logger; $logger::write($app, $message, $level); } } diff --git a/lib/private/server.php b/lib/private/server.php index 52dd56e291e..4ee0238a1e6 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -405,7 +405,7 @@ class Server extends SimpleContainer implements IServerContainer { * * @return \OCP\ILogger */ - function getLogger(){ + function getLogger() { return $this->query('Logger'); } From 93dbb39e775bf599f19445b27a10b0862cc3bab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 12 May 2014 14:16:54 +0200 Subject: [PATCH 4/4] adding unit test for message interpolation --- tests/lib/logger.php | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/lib/logger.php diff --git a/tests/lib/logger.php b/tests/lib/logger.php new file mode 100644 index 00000000000..7d5d4049b28 --- /dev/null +++ b/tests/lib/logger.php @@ -0,0 +1,40 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test; + +use OC\Log; + +class Logger extends \PHPUnit_Framework_TestCase { + /** + * @var \OCP\ILogger + */ + private $logger; + static private $logs = array(); + + public function setUp() { + self::$logs = array(); + $this->logger = new Log($this); + } + + public function testInterpolation() { + $logger = $this->logger; + $logger->info('{Message {nothing} {user} {foo.bar} a}', array('user' => 'Bob', 'foo.bar' => 'Bar')); + + $expected = array('1 {Message {nothing} Bob Bar a}'); + $this->assertEquals($expected, $this->getLogs()); + } + + private function getLogs() { + return self::$logs; + } + + public static function write($app, $message, $level) { + self::$logs[]= "$level $message"; + } +}