From 7573fa3148cf4ef99075e5373e40b776c0939f17 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 5 Jan 2017 12:47:27 +0100 Subject: [PATCH 1/3] Handle log_type "nextcloud" more gracefully Signed-off-by: Joas Schilling --- lib/private/Log.php | 27 ++++++++++++++++++++++----- lib/private/Server.php | 5 ++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index ef1b70d3cb..af0ca140cb 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -106,12 +106,8 @@ class Log implements ILogger { // FIXME: Add this for backwards compatibility, should be fixed at some point probably if($logger === null) { - // TODO: Drop backwards compatibility for config in the future $logType = $this->config->getValue('log_type', 'file'); - if($logType==='owncloud') { - $logType = 'file'; - } - $this->logger = 'OC\\Log\\'.ucfirst($logType); + $this->logger = static::getLogClass($logType); call_user_func(array($this->logger, 'init')); } else { $this->logger = $logger; @@ -327,4 +323,25 @@ class Log implements ILogger { $msg .= ': ' . json_encode($exception); $this->error($msg, $context); } + + /** + * @param string $logType + * @return string + * @internal + */ + public static function getLogClass($logType) { + // TODO: Drop backwards compatibility for config in the future + switch (strtolower($logType)) { + case 'owncloud': + case 'nextcloud': + $logType = 'file'; + } + $logClass = 'OC\\Log\\' . ucfirst($logType); + + if (!class_exists($logClass)) { + $logClass = 'OC\\Log\\File'; + } + + return $logClass; + } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 5bc72e3614..a2a2eb607f 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -418,9 +418,8 @@ class Server extends ServerContainer implements IServerContainer { ); }); $this->registerService('Logger', function (Server $c) { - $logClass = $c->query('AllConfig')->getSystemValue('log_type', 'file'); - // TODO: Drop backwards compatibility for config in the future - $logger = 'OC\\Log\\' . ucfirst($logClass=='owncloud' ? 'file' : $logClass); + $logType = $c->query('AllConfig')->getSystemValue('log_type', 'file'); + $logger = Log::getLogClass($logType); call_user_func(array($logger, 'init')); return new Log($logger); From e7ff1ba548ad4f8661f8309157336d376d6fb937 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Jan 2017 10:55:40 +0100 Subject: [PATCH 2/3] Add tests Signed-off-by: Joas Schilling --- lib/private/Log.php | 2 +- tests/lib/LoggerTest.php | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index af0ca140cb..5ba1969861 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -339,7 +339,7 @@ class Log implements ILogger { $logClass = 'OC\\Log\\' . ucfirst($logType); if (!class_exists($logClass)) { - $logClass = 'OC\\Log\\File'; + $logClass = \OC\Log\File::class; } return $logClass; diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index abb9deebd5..8bae8d1822 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -138,4 +138,20 @@ class LoggerTest extends TestCase { } } + public function dataGetLogClass() { + return [ + ['owncloud', \OC\Log\File::class], + ['nextcloud', \OC\Log\File::class], + ['file', \OC\Log\File::class], + ['errorlog', \OC\Log\Errorlog::class], + ['syslog', \OC\Log\Syslog::class], + ]; + } + + /** + * @dataProvider dataGetLogClass + */ + public function testGetLogClass($type, $class) { + $this->assertEquals($class, Log::getLogClass($type)); + } } From 7fa063ceca5d78741c47f645c6dfdf65e4be7330 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 10 Jan 2017 12:58:23 +0100 Subject: [PATCH 3/3] Better fallback for unknown log types Signed-off-by: Joas Schilling --- lib/private/Log.php | 19 ++++++++++--------- tests/lib/LoggerTest.php | 6 ++++-- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index 5ba1969861..fddd359312 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -330,18 +330,19 @@ class Log implements ILogger { * @internal */ public static function getLogClass($logType) { - // TODO: Drop backwards compatibility for config in the future switch (strtolower($logType)) { + case 'errorlog': + return \OC\Log\Errorlog::class; + case 'syslog': + return \OC\Log\Syslog::class; + case 'file': + return \OC\Log\File::class; + + // Backwards compatibility for old and fallback for unknown log types case 'owncloud': case 'nextcloud': - $logType = 'file'; + default: + return \OC\Log\File::class; } - $logClass = 'OC\\Log\\' . ucfirst($logType); - - if (!class_exists($logClass)) { - $logClass = \OC\Log\File::class; - } - - return $logClass; } } diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index 8bae8d1822..da9cedc9f5 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -140,11 +140,13 @@ class LoggerTest extends TestCase { public function dataGetLogClass() { return [ - ['owncloud', \OC\Log\File::class], - ['nextcloud', \OC\Log\File::class], ['file', \OC\Log\File::class], ['errorlog', \OC\Log\Errorlog::class], ['syslog', \OC\Log\Syslog::class], + + ['owncloud', \OC\Log\File::class], + ['nextcloud', \OC\Log\File::class], + ['foobar', \OC\Log\File::class], ]; }