Fix logger classes and tests

This commit is contained in:
Philipp 2023-07-17 01:06:11 +02:00
parent 903ecc2a76
commit bca6abf4ab
No known key found for this signature in database
GPG Key ID: 24A7501396EB5432
5 changed files with 28 additions and 72 deletions

View File

@ -25,10 +25,10 @@ use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Core\Logger\Capabilities\LogChannel; use Friendica\Core\Logger\Capabilities\LogChannel;
use Friendica\Core\Logger\Exception\LoggerArgumentException; use Friendica\Core\Logger\Exception\LoggerArgumentException;
use Friendica\Core\Logger\Exception\LoggerException; use Friendica\Core\Logger\Exception\LoggerException;
use Friendica\Core\Logger\Exception\LogLevelException;
use Friendica\Core\Logger\Type\StreamLogger as StreamLoggerClass; use Friendica\Core\Logger\Type\StreamLogger as StreamLoggerClass;
use Friendica\Core\Logger\Util\FileSystem; use Friendica\Core\Logger\Util\FileSystem;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
use Psr\Log\NullLogger; use Psr\Log\NullLogger;
/** /**
@ -55,7 +55,7 @@ class StreamLogger extends AbstractLoggerTypeFactory
$fileSystem = new FileSystem(); $fileSystem = new FileSystem();
$logfile = $logfile ?? $config->get('system', 'logfile'); $logfile = $logfile ?? $config->get('system', 'logfile');
if ((@file_exists($logfile) && !@is_writable($logfile)) && !@is_writable(basename($logfile))) { if (!@file_exists($logfile) || !@is_writable($logfile)) {
throw new LoggerArgumentException(sprintf('%s is not a valid logfile', $logfile)); throw new LoggerArgumentException(sprintf('%s is not a valid logfile', $logfile));
} }
@ -64,7 +64,7 @@ class StreamLogger extends AbstractLoggerTypeFactory
if (array_key_exists($loglevel, StreamLoggerClass::levelToInt)) { if (array_key_exists($loglevel, StreamLoggerClass::levelToInt)) {
$loglevel = StreamLoggerClass::levelToInt[$loglevel]; $loglevel = StreamLoggerClass::levelToInt[$loglevel];
} else { } else {
$loglevel = StreamLoggerClass::levelToInt[LogLevel::NOTICE]; throw new LogLevelException(sprintf('The level "%s" is not valid.', $loglevel));
} }
$stream = $fileSystem->createStream($logfile); $stream = $fileSystem->createStream($logfile);

View File

@ -21,6 +21,8 @@
namespace Friendica\Core\Logger\Util; namespace Friendica\Core\Logger\Util;
use Friendica\Core\Logger\Exception\LoggerUnusableException;
/** /**
* Util class for filesystem manipulation for Logger classes * Util class for filesystem manipulation for Logger classes
*/ */
@ -37,6 +39,8 @@ class FileSystem
* @param string $file The file * @param string $file The file
* *
* @return string The directory name (empty if no directory is found, like urls) * @return string The directory name (empty if no directory is found, like urls)
*
* @throws LoggerUnusableException
*/ */
public function createDir(string $file): string public function createDir(string $file): string
{ {
@ -57,7 +61,7 @@ class FileSystem
restore_error_handler(); restore_error_handler();
if (!$status && !is_dir($dirname)) { if (!$status && !is_dir($dirname)) {
throw new \UnexpectedValueException(sprintf('Directory "%s" cannot get created: ' . $this->errorMessage, $dirname)); throw new LoggerUnusableException(sprintf('Directory "%s" cannot get created: ' . $this->errorMessage, $dirname));
} }
return $dirname; return $dirname;
@ -75,7 +79,7 @@ class FileSystem
* *
* @return resource the open stream resource * @return resource the open stream resource
* *
* @throws \UnexpectedValueException * @throws LoggerUnusableException
*/ */
public function createStream(string $url) public function createStream(string $url)
{ {
@ -89,7 +93,7 @@ class FileSystem
restore_error_handler(); restore_error_handler();
if (!is_resource($stream)) { if (!is_resource($stream)) {
throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened: ' . $this->errorMessage, $url)); throw new LoggerUnusableException(sprintf('The stream or file "%s" could not be opened: ' . $this->errorMessage, $url));
} }
return $stream; return $stream;

View File

@ -72,7 +72,7 @@ EOF;
public function dataHooks(): array public function dataHooks(): array
{ {
return [ return [
'normal' => [ 'normal' => [
'structure' => $this->structure, 'structure' => $this->structure,
'enabled' => $this->addons, 'enabled' => $this->addons,
'files' => [ 'files' => [
@ -86,7 +86,7 @@ EOF;
], ],
], ],
], ],
'double' => [ 'double' => [
'structure' => $this->structure, 'structure' => $this->structure,
'enabled' => $this->addons, 'enabled' => $this->addons,
'files' => [ 'files' => [
@ -101,7 +101,7 @@ EOF;
], ],
], ],
], ],
'wrongName' => [ 'wrongName' => [
'structure' => $this->structure, 'structure' => $this->structure,
'enabled' => $this->addons, 'enabled' => $this->addons,
'files' => [ 'files' => [

View File

@ -55,20 +55,12 @@ return [
\Psr\Log\NullLogger::class => [''], \Psr\Log\NullLogger::class => [''],
], ],
], ],
\Friendica\Core\Hooks\Capabilities\BehavioralHookType::DECORATOR => [
\Psr\Log\LoggerInterface::class => [
\Psr\Log\NullLogger::class,
],
],
]; ];
EOF, EOF,
'addonsArray' => [], 'addonsArray' => [],
'assertStrategies' => [ 'assertStrategies' => [
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
], ],
'assertDecorators' => [
[LoggerInterface::class, NullLogger::class],
],
], ],
'normalWithString' => [ 'normalWithString' => [
'content' => <<<EOF 'content' => <<<EOF
@ -80,18 +72,12 @@ return [
\Psr\Log\NullLogger::class => '', \Psr\Log\NullLogger::class => '',
], ],
], ],
\Friendica\Core\Hooks\Capabilities\BehavioralHookType::DECORATOR => [
\Psr\Log\LoggerInterface::class => \Psr\Log\NullLogger::class,
],
]; ];
EOF, EOF,
'addonsArray' => [], 'addonsArray' => [],
'assertStrategies' => [ 'assertStrategies' => [
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
], ],
'assertDecorators' => [
[LoggerInterface::class, NullLogger::class],
],
], ],
'withAddons' => [ 'withAddons' => [
'content' => <<<EOF 'content' => <<<EOF
@ -116,7 +102,6 @@ EOF,
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
[LoggerInterface::class, NullLogger::class, 'null'], [LoggerInterface::class, NullLogger::class, 'null'],
], ],
'assertDecorators' => [],
], ],
'withAddonsWithString' => [ 'withAddonsWithString' => [
'content' => <<<EOF 'content' => <<<EOF
@ -141,7 +126,6 @@ EOF,
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
[LoggerInterface::class, NullLogger::class, 'null'], [LoggerInterface::class, NullLogger::class, 'null'],
], ],
'assertDecorators' => [],
], ],
// This should work because unique name convention is part of the instance manager logic, not of the file-infrastructure layer // This should work because unique name convention is part of the instance manager logic, not of the file-infrastructure layer
'withAddonsDoubleNamed' => [ 'withAddonsDoubleNamed' => [
@ -167,7 +151,6 @@ EOF,
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
], ],
'assertDecorators' => [],
], ],
'withWrongContentButAddons' => [ 'withWrongContentButAddons' => [
'content' => <<<EOF 'content' => <<<EOF
@ -191,7 +174,6 @@ EOF,
'assertStrategies' => [ 'assertStrategies' => [
[LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''],
], ],
'assertDecorators' => [],
], ],
]; ];
} }
@ -199,7 +181,7 @@ EOF,
/** /**
* @dataProvider dataHooks * @dataProvider dataHooks
*/ */
public function testSetupHooks(string $content, array $addonsArray, array $assertStrategies, array $assertDecorators) public function testSetupHooks(string $content, array $addonsArray, array $assertStrategies)
{ {
vfsStream::newFile('static/hooks.config.php') vfsStream::newFile('static/hooks.config.php')
->withContent($content) ->withContent($content)
@ -215,10 +197,6 @@ EOF,
$instanceManager->shouldReceive('registerStrategy')->withArgs($assertStrategy)->once(); $instanceManager->shouldReceive('registerStrategy')->withArgs($assertStrategy)->once();
} }
foreach ($assertDecorators as $assertDecorator) {
$instanceManager->shouldReceive('registerDecorator')->withArgs($assertDecorator)->once();
}
$hookFileManager->setupHooks($instanceManager); $hookFileManager->setupHooks($instanceManager);
self::expectNotToPerformAssertions(); self::expectNotToPerformAssertions();

View File

@ -22,9 +22,7 @@
namespace Friendica\Test\src\Core\Logger; namespace Friendica\Test\src\Core\Logger;
use Friendica\Core\Logger\Exception\LoggerArgumentException; use Friendica\Core\Logger\Exception\LoggerArgumentException;
use Friendica\Core\Logger\Exception\LoggerException;
use Friendica\Core\Logger\Exception\LogLevelException; use Friendica\Core\Logger\Exception\LogLevelException;
use Friendica\Core\Logger\Util\FileSystem;
use Friendica\Test\Util\VFSTrait; use Friendica\Test\Util\VFSTrait;
use Friendica\Core\Logger\Type\StreamLogger; use Friendica\Core\Logger\Type\StreamLogger;
use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStream;
@ -40,33 +38,26 @@ class StreamLoggerTest extends AbstractLoggerTest
*/ */
private $logfile; private $logfile;
/**
* @var \Friendica\Core\Logger\Util\Filesystem
*/
private $fileSystem;
protected function setUp(): void protected function setUp(): void
{ {
parent::setUp(); parent::setUp();
$this->setUpVfsDir(); $this->setUpVfsDir();
$this->fileSystem = new FileSystem();
} }
/** /**
* {@@inheritdoc} * {@@inheritdoc}
*/ */
protected function getInstance($level = LogLevel::DEBUG) protected function getInstance($level = LogLevel::DEBUG, $logfile = 'friendica.log')
{ {
$this->logfile = vfsStream::newFile('friendica.log') $this->logfile = vfsStream::newFile($logfile)
->at($this->root); ->at($this->root);
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn($this->logfile->url())->once(); $this->config->shouldReceive('get')->with('system', 'logfile')->andReturn($this->logfile->url())->once();
$this->config->shouldReceive('get')->with('system', 'loglevel')->andReturn($level)->once();
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem, $level); $loggerFactory = new \Friendica\Core\Logger\Factory\StreamLogger($this->introspection, 'test');
return $loggerFactory->create($this->config);
return $logger;
} }
/** /**
@ -83,11 +74,12 @@ class StreamLoggerTest extends AbstractLoggerTest
public function testNoUrl() public function testNoUrl()
{ {
$this->expectException(LoggerArgumentException::class); $this->expectException(LoggerArgumentException::class);
$this->expectExceptionMessage("Missing stream URL."); $this->expectExceptionMessage(' is not a valid logfile');
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn('')->once(); $this->config->shouldReceive('get')->with('system', 'logfile')->andReturn('')->once();
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem); $loggerFactory = new \Friendica\Core\Logger\Factory\StreamLogger($this->introspection, 'test');
$logger = $loggerFactory->create($this->config);
$logger->emergency('not working'); $logger->emergency('not working');
} }
@ -104,7 +96,8 @@ class StreamLoggerTest extends AbstractLoggerTest
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn($logfile->url())->once(); $this->config->shouldReceive('get')->with('system', 'logfile')->andReturn($logfile->url())->once();
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem); $loggerFactory = new \Friendica\Core\Logger\Factory\StreamLogger($this->introspection, 'test');
$logger = $loggerFactory->create($this->config);
$logger->emergency('not working'); $logger->emergency('not working');
} }
@ -119,7 +112,8 @@ class StreamLoggerTest extends AbstractLoggerTest
static::markTestIncomplete('We need a platform independent way to set directory to readonly'); static::markTestIncomplete('We need a platform independent way to set directory to readonly');
$logger = new StreamLogger('test', '/$%/wrong/directory/file.txt', $this->introspection, $this->fileSystem); $loggerFactory = new \Friendica\Core\Logger\Factory\StreamLogger($this->introspection, 'test');
$logger = $loggerFactory->create($this->config);
$logger->emergency('not working'); $logger->emergency('not working');
} }
@ -132,9 +126,7 @@ class StreamLoggerTest extends AbstractLoggerTest
$this->expectException(LogLevelException::class); $this->expectException(LogLevelException::class);
$this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./");
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn('file.text')->once(); $logger = $this->getInstance('NOPE');
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem, 'NOPE');
} }
/** /**
@ -145,29 +137,11 @@ class StreamLoggerTest extends AbstractLoggerTest
$this->expectException(LogLevelException::class); $this->expectException(LogLevelException::class);
$this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./");
$logfile = vfsStream::newFile('friendica.log') $logger = $this->getInstance('NOPE');
->at($this->root);
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn($logfile->url())->once();
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem);
$logger->log('NOPE', 'a test'); $logger->log('NOPE', 'a test');
} }
/**
* Test when the file is null
*/
public function testWrongFile()
{
$this->expectException(LoggerArgumentException::class);
$this->expectExceptionMessage("A stream must either be a resource or a string.");
$this->config->shouldReceive('get')->with('system', 'logfile')->andReturn(null)->once();
$logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem);
}
/** /**
* Test a relative path * Test a relative path
* @doesNotPerformAssertions * @doesNotPerformAssertions