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
cakephp · Oct 8, 2012 · 72f4d4f · 72f4d4f
1 parent a59db11
commit 72f4d4f
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 31 deletions.
diff --git a/lib/Cake/Log/CakeLog.php b/lib/Cake/Log/CakeLog.php
@@ -426,24 +426,29 @@ public static function write($type, $message, $scope = array()) {
 		$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
@@ -331,21 +331,22 @@ protected function _deleteLogs() {
 
 /**
  * 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,37 +376,55 @@ public function testScopedLoggingBC() {
 
 		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();
 
 		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'));