From 80f1feabe5ecf9b09b76627ba4b114ae10ddcaf6 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 11 Feb 2019 21:13:53 +0100 Subject: [PATCH] Refactoring Logging to use Configuration --- bin/auth_ejabberd.php | 3 +- bin/console.php | 3 +- bin/daemon.php | 3 +- bin/worker.php | 3 +- index.php | 3 +- src/App.php | 29 ------ src/Core/Config.php | 4 - src/Core/Config/Cache/ConfigCache.php | 12 ++- src/Core/Logger.php | 61 +----------- src/Factory/DBFactory.php | 8 ++ src/Factory/LoggerFactory.php | 97 +++++++++++-------- tests/include/ApiTest.php | 2 +- .../src/Core/Config/Cache/ConfigCacheTest.php | 59 +++++++++-- 13 files changed, 141 insertions(+), 146 deletions(-) diff --git a/bin/auth_ejabberd.php b/bin/auth_ejabberd.php index 7682674d7..003faae1f 100755 --- a/bin/auth_ejabberd.php +++ b/bin/auth_ejabberd.php @@ -59,7 +59,8 @@ $configLoader = new Cache\ConfigCacheLoader($basedir); $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); -$pconfig = Factory\ConfigFactory::createPConfig($configCache); +// needed to call PConfig::init() +Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('auth_ejabberd', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); diff --git a/bin/console.php b/bin/console.php index 9fee605b2..2b0d58821 100755 --- a/bin/console.php +++ b/bin/console.php @@ -12,7 +12,8 @@ $configLoader = new Cache\ConfigCacheLoader($basedir); $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); -$pconfig = Factory\ConfigFactory::createPConfig($configCache); +// needed to call PConfig::init() +Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('console', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); diff --git a/bin/daemon.php b/bin/daemon.php index 2b106b62d..f2970a518 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -39,7 +39,8 @@ $configLoader = new Cache\ConfigCacheLoader($basedir); $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); -$pconfig = Factory\ConfigFactory::createPConfig($configCache); +// needed to call PConfig::init() +Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('daemon', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); diff --git a/bin/worker.php b/bin/worker.php index 0a9b4cb38..530333127 100755 --- a/bin/worker.php +++ b/bin/worker.php @@ -37,7 +37,8 @@ $configLoader = new Cache\ConfigCacheLoader($basedir); $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); -$pconfig = Factory\ConfigFactory::createPConfig($configCache); +// needed to call PConfig::init() +Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('worker', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); diff --git a/index.php b/index.php index 9ef9490b3..66f023965 100644 --- a/index.php +++ b/index.php @@ -20,7 +20,8 @@ $configLoader = new Cache\ConfigCacheLoader($basedir); $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); -$pconfig = Factory\ConfigFactory::createPConfig($configCache); +// needed to call PConfig::init() +Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('index', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); diff --git a/src/App.php b/src/App.php index e715e6ced..3189b2da7 100644 --- a/src/App.php +++ b/src/App.php @@ -109,11 +109,6 @@ class App */ public $mobileDetect; - /** - * @var LoggerInterface The current logger of this App - */ - private $logger; - /** * @var Configuration The config */ @@ -339,35 +334,13 @@ class App return $this->mode; } - /** - * Returns the Logger of the Application - * - * @return LoggerInterface The Logger - * @throws InternalServerErrorException when the logger isn't created - */ - public function getLogger() - { - if (empty($this->logger)) { - throw new InternalServerErrorException('Logger of the Application is not defined'); - } - - return $this->logger; - } - /** * Reloads the whole app instance */ public function reload() { - $this->getMode()->determine($this->basePath); - $this->determineURLPath(); - if ($this->getMode()->has(App\Mode::DBCONFIGAVAILABLE)) { - Core\Config::load(); - } - - // again because DB-config could change the config $this->getMode()->determine($this->basePath); if ($this->getMode()->has(App\Mode::DBAVAILABLE)) { @@ -382,8 +355,6 @@ class App Core\L10n::init(); $this->process_id = Core\System::processID('log'); - - Core\Logger::setLogger($this->logger); } /** diff --git a/src/Core/Config.php b/src/Core/Config.php index 405d14d12..4bf9c5b11 100644 --- a/src/Core/Config.php +++ b/src/Core/Config.php @@ -8,10 +8,6 @@ */ namespace Friendica\Core; -use Friendica\Core\Config\ConfigCache; -use Friendica\Core\Config\IConfigAdapter; -use Friendica\Core\Config\IConfigCache; - /** * @brief Arbitrary system configuration storage * diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index b1172c0c8..54da327db 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -55,6 +55,8 @@ class ConfigCache implements IConfigCache, IPConfigCache { if (isset($this->config[$cat][$key])) { return $this->config[$cat][$key]; + } elseif ($key == null && isset($this->config[$cat])) { + return $this->config[$cat]; } else { return '!!'; } @@ -65,8 +67,8 @@ class ConfigCache implements IConfigCache, IPConfigCache */ public function has($cat, $key = null) { - return isset($this->config[$cat][$key]) - && $this->config[$cat][$key] !== '!!'; + return (isset($this->config[$cat][$key]) && $this->config[$cat][$key] !== '!!') || + ($key == null && isset($this->config[$cat]) && $this->config[$cat] !== '!!' && is_array($this->config[$cat])); } /** @@ -105,8 +107,8 @@ class ConfigCache implements IConfigCache, IPConfigCache */ public function hasP($uid, $cat, $key = null) { - return isset($this->config[$uid][$cat][$key]) - && $this->config[$uid][$cat][$key] !== '!!'; + return (isset($this->config[$uid][$cat][$key]) && $this->config[$uid][$cat][$key] !== '!!') || + ($key == null && isset($this->config[$uid][$cat]) && $this->config[$uid][$cat] !== '!!' && is_array($this->config[$uid][$cat])); } /** @@ -144,6 +146,8 @@ class ConfigCache implements IConfigCache, IPConfigCache { if (isset($this->config[$uid][$cat][$key])) { return $this->config[$uid][$cat][$key]; + } elseif ($key == null && isset($this->config[$uid][$cat])) { + return $this->config[$uid][$cat]; } else { return '!!'; } diff --git a/src/Core/Logger.php b/src/Core/Logger.php index e42f2f033..a2e587342 100644 --- a/src/Core/Logger.php +++ b/src/Core/Logger.php @@ -5,8 +5,6 @@ namespace Friendica\Core; use Friendica\BaseObject; -use Friendica\Factory\LoggerFactory; -use Friendica\Network\HTTPException\InternalServerErrorException; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; @@ -67,73 +65,22 @@ class Logger extends BaseObject /** * Sets the default logging handler for Friendica. - * @todo Can be combined with other handlers too if necessary, could be configurable. * * @param LoggerInterface $logger The Logger instance of this Application - * - * @throws InternalServerErrorException if the logger factory is incompatible to this logger */ public static function setLogger($logger) { - $debugging = Config::get('system', 'debugging'); - $logfile = Config::get('system', 'logfile'); - $loglevel = Config::get('system', 'loglevel'); - - if (!$debugging || !$logfile) { - return; - } - - $loglevel = self::mapLegacyConfigDebugLevel((string)$loglevel); - - LoggerFactory::addStreamHandler($logger, $logfile, $loglevel); - self::$logger = $logger; - - $logfile = Config::get('system', 'dlogfile'); - - if (!$logfile) { - return; - } - - $developIp = Config::get('system', 'dlogip'); - - self::$devLogger = LoggerFactory::createDev('develop', $developIp); - LoggerFactory::addStreamHandler(self::$devLogger, $logfile, LogLevel::DEBUG); } /** - * Mapping a legacy level to the PSR-3 compliant levels - * @see https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#5-psrlogloglevel + * Sets the default dev-logging handler for Friendica. * - * @param string $level the level to be mapped - * - * @return string the PSR-3 compliant level + * @param LoggerInterface $logger The Logger instance of this Application */ - private static function mapLegacyConfigDebugLevel($level) + public static function setDevLogger($logger) { - switch ($level) { - // legacy WARNING - case "0": - return LogLevel::ERROR; - // legacy INFO - case "1": - return LogLevel::WARNING; - // legacy TRACE - case "2": - return LogLevel::NOTICE; - // legacy DEBUG - case "3": - return LogLevel::INFO; - // legacy DATA - case "4": - return LogLevel::DEBUG; - // legacy ALL - case "5": - return LogLevel::DEBUG; - // default if nothing set - default: - return $level; - } + self::$devLogger = $logger; } /** diff --git a/src/Factory/DBFactory.php b/src/Factory/DBFactory.php index 5970541c1..abfe6f99d 100644 --- a/src/Factory/DBFactory.php +++ b/src/Factory/DBFactory.php @@ -7,6 +7,14 @@ use Friendica\Database; class DBFactory { + /** + * Initialize the DBA connection + * + * @param Cache\ConfigCache $configCache The configuration cache + * @param array $server The $_SERVER variables + * + * @throws \Exception if connection went bad + */ public static function init(Cache\ConfigCache $configCache, array $server) { if (Database\DBA::connected()) { diff --git a/src/Factory/LoggerFactory.php b/src/Factory/LoggerFactory.php index e9bacb38e..74f55e637 100644 --- a/src/Factory/LoggerFactory.php +++ b/src/Factory/LoggerFactory.php @@ -35,16 +35,17 @@ class LoggerFactory $logger->pushProcessor(new Monolog\Processor\UidProcessor()); $logger->pushProcessor(new FriendicaIntrospectionProcessor(LogLevel::DEBUG, [Logger::class, Profiler::class])); - if (isset($config)) { - $debugging = $config->get('system', 'debugging'); - $stream = $config->get('system', 'logfile'); - $level = $config->get('system', 'loglevel'); + $debugging = $config->get('system', 'debugging'); + $stream = $config->get('system', 'logfile'); + $level = $config->get('system', 'loglevel'); - if ($debugging) { - static::addStreamHandler($logger, $stream, $level); - } + if ($debugging) { + $loglevel = self::mapLegacyConfigDebugLevel((string)$level); + static::addStreamHandler($logger, $stream, $loglevel); } + Logger::setLogger($logger); + return $logger; } @@ -56,25 +57,71 @@ class LoggerFactory * * It should never get filled during normal usage of Friendica * - * @param string $channel The channel of the logger instance - * @param string $developerIp The IP of the developer who wants to use the logger + * @param string $channel The channel of the logger instance + * @param Configuration $config The config * * @return LoggerInterface The PSR-3 compliant logger instance */ - public static function createDev($channel, $developerIp) + public static function createDev($channel, Configuration $config) { + $debugging = $config->get('system', 'debugging'); + $stream = $config->get('system', 'dlogfile'); + $developerIp = $config->get('system', 'dlogip'); + + if (!isset($developerIp) || !$debugging) { + return null; + } + $logger = new Monolog\Logger($channel); $logger->pushProcessor(new Monolog\Processor\PsrLogMessageProcessor()); $logger->pushProcessor(new Monolog\Processor\ProcessIdProcessor()); $logger->pushProcessor(new Monolog\Processor\UidProcessor()); $logger->pushProcessor(new FriendicaIntrospectionProcessor(LogLevel::DEBUG, ['Friendica\\Core\\Logger'])); - $logger->pushHandler(new FriendicaDevelopHandler($developerIp)); + static::addStreamHandler($logger, $stream, LogLevel::DEBUG); + + Logger::setDevLogger($logger); + return $logger; } + /** + * Mapping a legacy level to the PSR-3 compliant levels + * @see https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#5-psrlogloglevel + * + * @param string $level the level to be mapped + * + * @return string the PSR-3 compliant level + */ + private static function mapLegacyConfigDebugLevel($level) + { + switch ($level) { + // legacy WARNING + case "0": + return LogLevel::ERROR; + // legacy INFO + case "1": + return LogLevel::WARNING; + // legacy TRACE + case "2": + return LogLevel::NOTICE; + // legacy DEBUG + case "3": + return LogLevel::INFO; + // legacy DATA + case "4": + return LogLevel::DEBUG; + // legacy ALL + case "5": + return LogLevel::DEBUG; + // default if nothing set + default: + return $level; + } + } + /** * Adding a handler to a given logger instance * @@ -106,32 +153,4 @@ class LoggerFactory throw new InternalServerErrorException('Logger instance incompatible for MonologFactory'); } } - - /** - * This method enables the test mode of a given logger - * - * @param LoggerInterface $logger The logger - * - * @return Monolog\Handler\TestHandler the Handling for tests - * - * @throws InternalServerErrorException if the logger is incompatible to the logger factory - */ - public static function enableTest($logger) - { - if ($logger instanceof Monolog\Logger) { - // disable every handler so far - $logger->pushHandler(new Monolog\Handler\NullHandler()); - - // enable the test handler - $fileHandler = new Monolog\Handler\TestHandler(); - $formatter = new Monolog\Formatter\LineFormatter("%datetime% %channel% [%level_name%]: %message% %context% %extra%\n"); - $fileHandler->setFormatter($formatter); - - $logger->pushHandler($fileHandler); - - return $fileHandler; - } else { - throw new InternalServerErrorException('Logger instance incompatible for MonologFactory'); - } - } } diff --git a/tests/include/ApiTest.php b/tests/include/ApiTest.php index 7f2299d51..59096f5e5 100644 --- a/tests/include/ApiTest.php +++ b/tests/include/ApiTest.php @@ -41,7 +41,7 @@ class ApiTest extends DatabaseTest $configCache = Factory\ConfigFactory::createCache($configLoader); Factory\DBFactory::init($configCache, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); - $pconfig = Factory\ConfigFactory::createPConfig($configCache); + Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config); $profiler = Factory\ProfilerFactory::create($logger, $config); $this->app = new App($config, $logger, $profiler, false); diff --git a/tests/src/Core/Config/Cache/ConfigCacheTest.php b/tests/src/Core/Config/Cache/ConfigCacheTest.php index 7b56ca646..ac9fae540 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheTest.php @@ -149,13 +149,34 @@ class ConfigCacheTest extends MockedTest $configCache = new ConfigCache(); $this->assertFalse($configCache->has('system', 'test')); + $this->assertFalse($configCache->has('system')); $configCache->set('system', 'test', 'it'); $this->assertTrue($configCache->has('system', 'test')); + $this->assertTrue($configCache->has('system')); + } - $this->assertFalse($configCache->has('system', null)); - $configCache->set('system', null, 'it'); - $this->assertTrue($configCache->has('system', null)); + /** + * Test the get() method with a category + */ + public function testGetCat() + { + $configCache = new ConfigCache([ + 'system' => [ + 'key1' => 'value1', + 'key2' => 'value2', + ], + 'config' => [ + 'key3' => 'value3', + ], + ]); + + $this->assertTrue($configCache->has('system')); + + $this->assertEquals([ + 'key1' => 'value1', + 'key2' => 'value2', + ], $configCache->get('system')); } /** @@ -194,6 +215,32 @@ class ConfigCacheTest extends MockedTest } + /** + * Test the getP() method with a category + */ + public function testGetPCat() + { + $configCache = new ConfigCache(); + $uid = 345; + + $configCache->loadP($uid, [ + 'system' => [ + 'key1' => 'value1', + 'key2' => 'value2', + ], + 'config' => [ + 'key3' => 'value3', + ], + ]); + + $this->assertTrue($configCache->hasP($uid,'system')); + + $this->assertEquals([ + 'key1' => 'value1', + 'key2' => 'value2', + ], $configCache->get($uid, 'system')); + } + /** * Test the deleteP() method * @dataProvider dataTests @@ -227,12 +274,10 @@ class ConfigCacheTest extends MockedTest $uid = 345; $this->assertFalse($configCache->hasP($uid, 'system', 'test')); + $this->assertFalse($configCache->hasP($uid, 'system')); $configCache->setP($uid, 'system', 'test', 'it'); $this->assertTrue($configCache->hasP($uid, 'system', 'test')); - - $this->assertFalse($configCache->hasP($uid, 'system', null)); - $configCache->setP($uid, 'system', null, 'it'); - $this->assertTrue($configCache->hasP($uid, 'system', null)); + $this->assertTrue($configCache->hasP($uid, 'system')); } }