composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-05-16 05:48:47 +02:00
Code Issues Packages Projects Releases Wiki Activity

N°4261 - implement feedbacks from the team

Browse Source 
plus:
 - the entrypoint is now `LogException()` instead of `FromException()` which sounded more like a factory and less like an active method.
 - merge conflicting commit with @molkobain (CC fix)
 - remove the writing of the exception object in the error.log context (adding it was an error, it's way too verbose!!).
   - Technical note: The context is still used to propagate the exception across several call stack, so it now uses a less generic naming in order to avoid conflicts (see `ExceptionLog::CONTEXT_EXCEPTION`). another solution would have been to add a new parameter to `ExceptionLog::Log()`, but I didn't want to add constraint over the hypothetical evolution of the base class method.
This commit is contained in:
bruno-ds
2021-09-20 12:27:13 +02:00
parent 6e0d570d41
commit daf24d8cb3
2 changed files with 26 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
core/log.class.inc.php
View File

@@ -656,9 +656,9 @@ abstract class LogAPI
* @throws \ConfigException if log wrongly configured
* @uses GetMinLogLevel
*/
final public static function IsLogLevelEnabled(string $sLevel, string $sChannel, string $code = 'log_level_min'): bool
final public static function IsLogLevelEnabled(string $sLevel, string $sChannel, string $sCode = 'log_level_min'): bool
{
$sMinLogLevel = self::GetMinLogLevel($sChannel, $code);
$sMinLogLevel = self::GetMinLogLevel($sChannel, $sCode);
if ($sMinLogLevel === false || $sMinLogLevel === 'false') {
return false;
@@ -678,6 +678,7 @@ abstract class LogAPI
/**
* @param string $sChannel
* @param string $sCode
*
* @return string one of the LEVEL_* const value : the one configured it if exists, otherwise default log level for this channel
* Config can be set :
@@ -697,14 +698,14 @@ abstract class LogAPI
*
* @link https://www.itophub.io/wiki/page?id=3_0_0%3Aadmin%3Alog iTop log reference
*/
protected static function GetMinLogLevel($sChannel, $code = 'log_level_min')
protected static function GetMinLogLevel($sChannel, $sCode = 'log_level_min')
{
$oConfig = static::GetConfig();
if (!$oConfig instanceof Config) {
return static::GetLevelDefault();
}
$sLogLevelMin = $oConfig->Get($code);
$sLogLevelMin = $oConfig->Get($sCode);
if (empty($sLogLevelMin)) {
return static::GetLevelDefault();
@@ -1106,13 +1107,14 @@ class LogFileRotationProcess implements iScheduledProcess
/**
* Log exceptions using dedicated API and logic.
*
* Please use {@see ExceptionLog::ExceptionLog()} to log exceptions
* Please use {@see ExceptionLog::LogException()} to log exceptions
*
* @Since 3.0.0
* @since 3.0.0
*/
class ExceptionLog extends LogAPI
{
const CHANNEL_DEFAULT = 'Exception';
const CONTEXT_EXCEPTION = '__exception';
private static $oLastEventIssue = null;
@@ -1121,23 +1123,24 @@ class ExceptionLog extends LogAPI
/**
* This method should be used to write logs.
*
* As it encapsulate the operations performed using the Exception, you should prefer it to the standard API inherited from LogApi `ExceptionLog::Error($oException->getMessage(), get_class($oException), ['exception' => $oException]);`
* As it encapsulate the operations performed using the Exception, you should prefer it to the standard API inherited from LogApi `ExceptionLog::Error($oException->getMessage(), get_class($oException), ['__exception' => $oException]);`
* The parameter order is not standard, but in our use case, the resulting API is way more convenient this way.
*/
public static function FromException(Exception $oException, $aContext = array(), $sLevel = self::LEVEL_WARNING)
public static function LogException(Exception $oException, $aContext = array(), $sLevel = self::LEVEL_WARNING)
{
if (empty($aContext['exception'])) {
$aContext['exception'] = $oException;
if (empty($aContext[self::CONTEXT_EXCEPTION])) {
$aContext[self::CONTEXT_EXCEPTION] = $oException;
}
if (empty($aContext['exception class'])) {
$aContext['exception class'] = get_class($oException);
}
return self::Log($sLevel, $oException->getMessage(), get_class($oException), $aContext);
self::Log($sLevel, $oException->getMessage(), get_class($oException), $aContext);
}
/**
* @inheritDoc
* @throws \ConfigException if log wrongly configured
*/
public static function Log($sLevel, $sMessage, $sClass = null, $aContext = array())
@@ -1155,7 +1158,7 @@ class ExceptionLog extends LogAPI
$sChannel = self::FindClassChannel($sClass);
if (static::IsLogLevelEnabled($sLevel, $sChannel)) {
static::$m_oFileLog->$sLevel($sMessage, $sChannel, $aContext);
static::$m_oFileLog->$sLevel($sMessage, $sChannel, array_diff_key($aContext, [self::CONTEXT_EXCEPTION => null])); //The exception should not be included in the error.log because of its verbosity.
}
$sDbChannel = self::FindClassChannel($sClass, 'log_level_min.write_in_db');
@@ -1165,15 +1168,14 @@ class ExceptionLog extends LogAPI
}
protected static function FindClassChannel($sClass, $code = 'log_level_min')
protected static function FindClassChannel($sClass, $sCode = 'log_level_min')
{
$oConfig = static::GetConfig();
if (!$oConfig instanceof Config) {
return static::GetLevelDefault();
}
$sLogLevelMin = $oConfig->Get($code);
$sLogLevelMin = $oConfig->Get($sCode);
if (empty($sLogLevelMin)) {
return $sClass;
@@ -1200,6 +1202,9 @@ class ExceptionLog extends LogAPI
return $sClass;
}
/**
* @inheritDoc
*/
public static function Enable($sTargetFile = null)
{
if (empty($sTargetFile))
@@ -1211,7 +1216,9 @@ class ExceptionLog extends LogAPI
private static function WriteToDb(array $aContext): void
{
$exception = $aContext['exception'];
$exception = $aContext[self::CONTEXT_EXCEPTION];
unset($aContext[self::CONTEXT_EXCEPTION]);
if (MetaModel::IsLogEnabledIssue()) {
if (MetaModel::IsValidClass('EventIssue')) {
try {
@@ -1238,9 +1245,9 @@ class ExceptionLog extends LogAPI
}
/**
* used by the tests
* @internal Used by the tests
*/
private static function getLastEventIssue()
private static function GetLastEventIssue()
{
return self::$oLastEventIssue;
}

2
test/core/Log/ExceptionLogTest.php
View File

@@ -70,7 +70,7 @@ class ExceptionLogTest extends ItopDataTestCase
$oException = new $aExceptions[$i]("Iteration number $i");
$iExpectedWriteNumber = $aExpectedWriteNumber[$i];
$iExpectedDbWriteNumber = $aExpectedDbWriteNumber[$i];
$aExpectedFileContext = array_merge($aContext, ['__exception class' => get_class($oException)]); //The context is preserved, and, if the key '__exception' is not yet in the array, it is added
$aExpectedFileContext = array_merge($aContext, ['exception class' => get_class($oException)]); //The context is preserved, and, if the key 'exception class' is not yet in the array, it is added
$mockFileLog->expects($this->exactly($iExpectedWriteNumber))
->method($sLevel)
->with($oException->GetMessage(), $sChannel, $aExpectedFileContext)