Fix logger classes and tests
authorPhilipp <admin@philipp.info>
Sun, 16 Jul 2023 23:06:11 +0000 (01:06 +0200)
committerPhilipp <admin@philipp.info>
Sun, 16 Jul 2023 23:06:11 +0000 (01:06 +0200)
src/Core/Logger/Factory/StreamLogger.php
src/Core/Logger/Util/FileSystem.php
tests/src/Core/Addon/Model/AddonLoaderTest.php
tests/src/Core/Hooks/Util/HookFileManagerTest.php
tests/src/Core/Logger/StreamLoggerTest.php

index 6da4b8c..caad78e 100644 (file)
@@ -25,10 +25,10 @@ use Friendica\Core\Config\Capability\IManageConfigValues;
 use Friendica\Core\Logger\Capabilities\LogChannel;
 use Friendica\Core\Logger\Exception\LoggerArgumentException;
 use Friendica\Core\Logger\Exception\LoggerException;
+use Friendica\Core\Logger\Exception\LogLevelException;
 use Friendica\Core\Logger\Type\StreamLogger as StreamLoggerClass;
 use Friendica\Core\Logger\Util\FileSystem;
 use Psr\Log\LoggerInterface;
-use Psr\Log\LogLevel;
 use Psr\Log\NullLogger;
 
 /**
@@ -55,7 +55,7 @@ class StreamLogger extends AbstractLoggerTypeFactory
                $fileSystem = new FileSystem();
 
                $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));
                }
 
@@ -64,7 +64,7 @@ class StreamLogger extends AbstractLoggerTypeFactory
                if (array_key_exists($loglevel, StreamLoggerClass::levelToInt)) {
                        $loglevel = StreamLoggerClass::levelToInt[$loglevel];
                } else {
-                       $loglevel = StreamLoggerClass::levelToInt[LogLevel::NOTICE];
+                       throw new LogLevelException(sprintf('The level "%s" is not valid.', $loglevel));
                }
 
                $stream = $fileSystem->createStream($logfile);
index 3793ac4..41bb423 100644 (file)
@@ -21,6 +21,8 @@
 
 namespace Friendica\Core\Logger\Util;
 
+use Friendica\Core\Logger\Exception\LoggerUnusableException;
+
 /**
  * Util class for filesystem manipulation for Logger classes
  */
@@ -37,6 +39,8 @@ class FileSystem
         * @param string $file The file
         *
         * @return string The directory name (empty if no directory is found, like urls)
+        *
+        * @throws LoggerUnusableException
         */
        public function createDir(string $file): string
        {
@@ -57,7 +61,7 @@ class FileSystem
                        restore_error_handler();
 
                        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;
@@ -75,7 +79,7 @@ class FileSystem
         *
         * @return resource the open stream resource
         *
-        * @throws \UnexpectedValueException
+        * @throws LoggerUnusableException
         */
        public function createStream(string $url)
        {
@@ -89,7 +93,7 @@ class FileSystem
                restore_error_handler();
 
                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;
index 3e9a325..189110f 100644 (file)
@@ -72,7 +72,7 @@ EOF;
        public function dataHooks(): array
        {
                return [
-                       'normal'                  => [
+                       'normal' => [
                                'structure' => $this->structure,
                                'enabled'   => $this->addons,
                                'files'     => [
@@ -86,7 +86,7 @@ EOF;
                                        ],
                                ],
                        ],
-                       'double'                  => [
+                       'double' => [
                                'structure' => $this->structure,
                                'enabled'   => $this->addons,
                                'files'     => [
@@ -101,7 +101,7 @@ EOF;
                                        ],
                                ],
                        ],
-                       'wrongName'               => [
+                       'wrongName' => [
                                'structure' => $this->structure,
                                'enabled'   => $this->addons,
                                'files'     => [
index 216a983..f718e8a 100644 (file)
@@ -55,20 +55,12 @@ return [
                        \Psr\Log\NullLogger::class => [''],
                ],
        ],
-       \Friendica\Core\Hooks\Capabilities\BehavioralHookType::DECORATOR => [
-               \Psr\Log\LoggerInterface::class => [
-                       \Psr\Log\NullLogger::class,
-               ],
-       ],
 ];
 EOF,
                                'addonsArray'      => [],
                                'assertStrategies' => [
                                        [LoggerInterface::class, NullLogger::class, ''],
                                ],
-                               'assertDecorators' => [
-                                       [LoggerInterface::class, NullLogger::class],
-                               ],
                        ],
                        'normalWithString' => [
                                'content' => <<<EOF
@@ -80,18 +72,12 @@ return [
                        \Psr\Log\NullLogger::class => '',
                ],
        ],
-       \Friendica\Core\Hooks\Capabilities\BehavioralHookType::DECORATOR => [
-               \Psr\Log\LoggerInterface::class => \Psr\Log\NullLogger::class,
-       ],
 ];
 EOF,
                                'addonsArray'      => [],
                                'assertStrategies' => [
                                        [LoggerInterface::class, NullLogger::class, ''],
                                ],
-                               'assertDecorators' => [
-                                       [LoggerInterface::class, NullLogger::class],
-                               ],
                        ],
                        'withAddons' => [
                                'content' => <<<EOF
@@ -116,7 +102,6 @@ EOF,
                                        [LoggerInterface::class, NullLogger::class, ''],
                                        [LoggerInterface::class, NullLogger::class, 'null'],
                                ],
-                               'assertDecorators' => [],
                        ],
                        'withAddonsWithString' => [
                                'content' => <<<EOF
@@ -141,7 +126,6 @@ EOF,
                                        [LoggerInterface::class, NullLogger::class, ''],
                                        [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
                        'withAddonsDoubleNamed' => [
@@ -167,7 +151,6 @@ EOF,
                                        [LoggerInterface::class, NullLogger::class, ''],
                                        [LoggerInterface::class, NullLogger::class, ''],
                                ],
-                               'assertDecorators' => [],
                        ],
                        'withWrongContentButAddons' => [
                                'content' => <<<EOF
@@ -191,7 +174,6 @@ EOF,
                                'assertStrategies' => [
                                        [LoggerInterface::class, NullLogger::class, ''],
                                ],
-                               'assertDecorators' => [],
                        ],
                ];
        }
@@ -199,7 +181,7 @@ EOF,
        /**
         * @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')
                        ->withContent($content)
@@ -215,10 +197,6 @@ EOF,
                        $instanceManager->shouldReceive('registerStrategy')->withArgs($assertStrategy)->once();
                }
 
-               foreach ($assertDecorators as $assertDecorator) {
-                       $instanceManager->shouldReceive('registerDecorator')->withArgs($assertDecorator)->once();
-               }
-
                $hookFileManager->setupHooks($instanceManager);
 
                self::expectNotToPerformAssertions();
index c8fac49..a1e0af5 100644 (file)
@@ -22,9 +22,7 @@
 namespace Friendica\Test\src\Core\Logger;
 
 use Friendica\Core\Logger\Exception\LoggerArgumentException;
-use Friendica\Core\Logger\Exception\LoggerException;
 use Friendica\Core\Logger\Exception\LogLevelException;
-use Friendica\Core\Logger\Util\FileSystem;
 use Friendica\Test\Util\VFSTrait;
 use Friendica\Core\Logger\Type\StreamLogger;
 use org\bovigo\vfs\vfsStream;
@@ -40,33 +38,26 @@ class StreamLoggerTest extends AbstractLoggerTest
         */
        private $logfile;
 
-       /**
-        * @var \Friendica\Core\Logger\Util\Filesystem
-        */
-       private $fileSystem;
-
        protected function setUp(): void
        {
                parent::setUp();
 
                $this->setUpVfsDir();
-
-               $this->fileSystem = new FileSystem();
        }
 
        /**
         * {@@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);
 
                $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);
-
-               return $logger;
+               $loggerFactory = new \Friendica\Core\Logger\Factory\StreamLogger($this->introspection, 'test');
+               return $loggerFactory->create($this->config);
        }
 
        /**
@@ -83,11 +74,12 @@ class StreamLoggerTest extends AbstractLoggerTest
        public function testNoUrl()
        {
                $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();
 
-               $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');
        }
@@ -104,7 +96,8 @@ class StreamLoggerTest extends AbstractLoggerTest
 
                $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');
        }
@@ -119,7 +112,8 @@ class StreamLoggerTest extends AbstractLoggerTest
 
                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');
        }
@@ -132,9 +126,7 @@ class StreamLoggerTest extends AbstractLoggerTest
                $this->expectException(LogLevelException::class);
                $this->expectExceptionMessageMatches("/The level \".*\" is not valid./");
 
-               $this->config->shouldReceive('get')->with('system', 'logfile')->andReturn('file.text')->once();
-
-               $logger = new StreamLogger('test', $this->config, $this->introspection, $this->fileSystem, 'NOPE');
+               $logger = $this->getInstance('NOPE');
        }
 
        /**
@@ -145,29 +137,11 @@ class StreamLoggerTest extends AbstractLoggerTest
                $this->expectException(LogLevelException::class);
                $this->expectExceptionMessageMatches("/The level \".*\" is not valid./");
 
-               $logfile = vfsStream::newFile('friendica.log')
-                       ->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 = $this->getInstance('NOPE');
 
                $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
         * @doesNotPerformAssertions