From 72f4d4fac09645c99c8443ec995b67a11eb1eeea Mon Sep 17 00:00:00 2001 From: mark_story Date: Mon, 8 Oct 2012 12:57:02 -0400 Subject: [PATCH] Fix issue with logging scopes Logging scopes should be exclusive and not allow messages matching on level alone to be logged. By using scopes + levels you opt-in to new behavior so grabbing all messages by level should not occur. Fixes #3264 --- lib/Cake/Log/CakeLog.php | 33 +++++++++------- lib/Cake/Test/Case/Log/CakeLogTest.php | 53 +++++++++++++++++--------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/lib/Cake/Log/CakeLog.php b/lib/Cake/Log/CakeLog.php index 39c6b8b08..776ea9e6f 100644 --- a/lib/Cake/Log/CakeLog.php +++ b/lib/Cake/Log/CakeLog.php @@ -426,24 +426,29 @@ class CakeLog { $logged = false; foreach (self::$_Collection->enabled() as $streamName) { $logger = self::$_Collection->{$streamName}; - $types = null; - $scopes = array(); + $types = $scopes = $config = array(); if ($logger instanceof BaseLog) { $config = $logger->config(); - if (isset($config['types'])) { - $types = $config['types']; - } - if (isset($config['scopes'])) { - $scopes = $config['scopes']; - } } - if (is_string($scope)) { - $inScope = in_array($scope, $scopes); - } else { - $intersect = array_intersect($scope, $scopes); - $inScope = !empty($intersect); + if (isset($config['types'])) { + $types = $config['types']; } - if (empty($types) || in_array($type, $types) || in_array($type, $scopes) && $inScope) { + if (isset($config['scopes'])) { + $scopes = $config['scopes']; + } + $inScope = (count(array_intersect((array)$scope, $scopes)) > 0); + $correctLevel = in_array($type, $types); + + if ( + // No config is a catch all (bc mode) + (empty($types) && empty($scopes)) || + // BC layer for mixing scope & level + (in_array($type, $scopes)) || + // no scopes, but has level + (empty($scopes) && $correctLevel) || + // exact scope + level + ($correctLevel && $inScope) + ) { $logger->write($type, $message); $logged = true; } diff --git a/lib/Cake/Test/Case/Log/CakeLogTest.php b/lib/Cake/Test/Case/Log/CakeLogTest.php index 39f715eba..e54f2c71a 100644 --- a/lib/Cake/Test/Case/Log/CakeLogTest.php +++ b/lib/Cake/Test/Case/Log/CakeLogTest.php @@ -331,21 +331,22 @@ class CakeLogTest extends CakeTestCase { /** * test backward compatible scoped logging + * + * @return void */ public function testScopedLoggingBC() { - $this->_deleteLogs(); - $this->_resetLogConfig(); + CakeLog::config('shops', array( 'engine' => 'FileLog', 'types' => array('info', 'notice', 'warning'), 'scopes' => array('transactions', 'orders'), 'file' => 'shops', - )); + )); + $this->_deleteLogs(); CakeLog::write('info', 'info message'); $this->assertFalse(file_exists(LOGS . 'error.log')); - $this->assertTrue(file_exists(LOGS . 'shops.log')); $this->assertTrue(file_exists(LOGS . 'debug.log')); $this->_deleteLogs(); @@ -375,7 +376,6 @@ class CakeLogTest extends CakeTestCase { CakeLog::write('warning', 'warning message'); $this->assertTrue(file_exists(LOGS . 'error.log')); - $this->assertTrue(file_exists(LOGS . 'shops.log')); $this->assertFalse(file_exists(LOGS . 'debug.log')); $this->_deleteLogs(); @@ -383,29 +383,48 @@ class CakeLogTest extends CakeTestCase { CakeLog::drop('shops'); } + + public function testScopedLoggingExclusive() { + $this->_deleteLogs(); + + CakeLog::config('shops', array( + 'engine' => 'FileLog', + 'types' => array('info', 'notice', 'warning'), + 'scopes' => array('transactions', 'orders'), + 'file' => 'shops.log', + )); + CakeLog::config('eggs', array( + 'engine' => 'FileLog', + 'types' => array('info', 'notice', 'warning'), + 'scopes' => array('eggs'), + 'file' => 'eggs.log', + )); + + CakeLog::write('info', 'transactions message', 'transactions'); + $this->assertFalse(file_exists(LOGS . 'eggs.log')); + $this->assertTrue(file_exists(LOGS . 'shops.log')); + + $this->_deleteLogs(); + + CakeLog::write('info', 'eggs message', 'eggs'); + $this->assertTrue(file_exists(LOGS . 'eggs.log')); + $this->assertFalse(file_exists(LOGS . 'shops.log')); + } + /** * test scoped logging * * @return void */ public function testScopedLogging() { - if (file_exists(LOGS . 'shops.log')) { - unlink(LOGS . 'shops.log'); - } - if (file_exists(LOGS . 'error.log')) { - unlink(LOGS . 'error.log'); - } - if (file_exists(LOGS . 'debug.log')) { - unlink(LOGS . 'debug.log'); - } - $this->_resetLogConfig(); + $this->_deleteLogs(); CakeLog::config('shops', array( 'engine' => 'FileLog', 'types' => array('info', 'notice', 'warning'), 'scopes' => array('transactions', 'orders'), - 'file' => 'shops', - )); + 'file' => 'shops.log', + )); CakeLog::write('info', 'info message', 'transactions'); $this->assertFalse(file_exists(LOGS . 'error.log'));