From f4ea74447e8a487bdd7236bdc6789a5d53fa2226 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 29 Oct 2021 08:03:59 +0200 Subject: [PATCH] Add Feedback :-) --- .../Logger/Exception/LogLevelException.php | 13 +++++++ src/Core/Logger/Factory/Logger.php | 39 ++++++++++++++----- src/Core/Logger/Type/StreamLogger.php | 8 ++-- src/Core/Logger/Type/SyslogLogger.php | 12 +++--- src/DI.php | 4 +- ...stPerHttp.php => ICanSendHttpRequests.php} | 2 +- .../{HttpClientCan.php => HttpClient.php} | 4 +- src/Network/HTTPClient/Factory/HttpClient.php | 8 ++-- static/dependencies.config.php | 4 +- tests/DiceHttpMockHandlerTrait.php | 4 +- tests/src/Core/InstallerTest.php | 10 ++--- tests/src/Core/Logger/StreamLoggerTest.php | 5 ++- tests/src/Core/Logger/SyslogLoggerTest.php | 5 ++- .../Storage/Repository/StorageManagerTest.php | 6 +-- 14 files changed, 80 insertions(+), 44 deletions(-) create mode 100644 src/Core/Logger/Exception/LogLevelException.php rename src/Network/HTTPClient/Capability/{ICanRequestPerHttp.php => ICanSendHttpRequests.php} (99%) rename src/Network/HTTPClient/Client/{HttpClientCan.php => HttpClient.php} (98%) diff --git a/src/Core/Logger/Exception/LogLevelException.php b/src/Core/Logger/Exception/LogLevelException.php new file mode 100644 index 000000000..96abd2a9a --- /dev/null +++ b/src/Core/Logger/Exception/LogLevelException.php @@ -0,0 +1,13 @@ +get('system', 'debugging', false))) { $logger = new NullLogger(); @@ -84,8 +85,8 @@ class Logger } $introspection = new Introspection(self::$ignoreClassList); - $level = $config->get('system', 'loglevel'); - $loglevel = self::mapLegacyConfigDebugLevel((string)$level); + $minLevel = $minLevel ?? $config->get('system', 'loglevel'); + $loglevel = self::mapLegacyConfigDebugLevel((string)$minLevel); switch ($config->get('system', 'logger_config', 'stream')) { case 'monolog': @@ -106,8 +107,12 @@ class Logger static::addStreamHandler($logger, $stream, $loglevel); } catch (\Throwable $e) { // No Logger .. - /// @todo isn't it possible to give the admin any hint about this wrong configuration? - $logger = new NullLogger(); + try { + $logger = new SyslogLogger($this->channel, $introspection, $loglevel); + } catch (\Throwable $e) { + // No logger ... + $logger = new NullLogger(); + } } } break; @@ -115,9 +120,12 @@ class Logger case 'syslog': try { $logger = new SyslogLogger($this->channel, $introspection, $loglevel); + } catch (LogLevelException $exception) { + // If there's a wrong config value for loglevel, try again with standard + $logger = $this->create($database, $config, $profiler, $fileSystem, LogLevel::NOTICE); + $logger->warning('Invalid loglevel set in config.', ['loglevel' => $loglevel]); } catch (\Throwable $e) { // No logger ... - /// @todo isn't it possible to give the admin any hint about this wrong configuration? $logger = new NullLogger(); } break; @@ -129,14 +137,25 @@ class Logger if (!is_file($stream) || is_writable($stream)) { try { $logger = new StreamLogger($this->channel, $stream, $introspection, $fileSystem, $loglevel); + } catch (LogLevelException $exception) { + // If there's a wrong config value for loglevel, try again with standard + $logger = $this->create($database, $config, $profiler, $fileSystem, LogLevel::NOTICE); + $logger->warning('Invalid loglevel set in config.', ['loglevel' => $loglevel]); } catch (\Throwable $t) { // No logger ... - /// @todo isn't it possible to give the admin any hint about this wrong configuration? $logger = new NullLogger(); } } else { - /// @todo isn't it possible to give the admin any hint about this wrong configuration? - $logger = new NullLogger(); + try { + $logger = new SyslogLogger($this->channel, $introspection, $loglevel); + } catch (LogLevelException $exception) { + // If there's a wrong config value for loglevel, try again with standard + $logger = $this->create($database, $config, $profiler, $fileSystem, LogLevel::NOTICE); + $logger->warning('Invalid loglevel set in config.', ['loglevel' => $loglevel]); + } catch (\Throwable $e) { + // No logger ... + $logger = new NullLogger(); + } } break; } diff --git a/src/Core/Logger/Type/StreamLogger.php b/src/Core/Logger/Type/StreamLogger.php index be0283d0e..f67aef9a4 100644 --- a/src/Core/Logger/Type/StreamLogger.php +++ b/src/Core/Logger/Type/StreamLogger.php @@ -23,6 +23,7 @@ namespace Friendica\Core\Logger\Type; use Friendica\Core\Logger\Exception\LoggerArgumentException; use Friendica\Core\Logger\Exception\LoggerException; +use Friendica\Core\Logger\Exception\LogLevelException; use Friendica\Util\DateTimeFormat; use Friendica\Util\FileSystem; use Friendica\Util\Introspection; @@ -83,6 +84,7 @@ class StreamLogger extends AbstractLogger * @param string $level The minimum loglevel at which this logger will be triggered * * @throws LoggerArgumentException + * @throws LogLevelException */ public function __construct($channel, $stream, Introspection $introspection, FileSystem $fileSystem, string $level = LogLevel::DEBUG) { @@ -102,7 +104,7 @@ class StreamLogger extends AbstractLogger if (array_key_exists($level, $this->levelToInt)) { $this->logLevel = $this->levelToInt[$level]; } else { - throw new LoggerArgumentException(sprintf('The level "%s" is not valid.', $level)); + throw new LogLevelException(sprintf('The level "%s" is not valid.', $level)); } $this->checkStream(); @@ -127,12 +129,12 @@ class StreamLogger extends AbstractLogger * @return void * * @throws LoggerException - * @throws LoggerArgumentException + * @throws LogLevelException */ protected function addEntry($level, string $message, array $context = []) { if (!array_key_exists($level, $this->levelToInt)) { - throw new LoggerArgumentException(sprintf('The level "%s" is not valid.', $level)); + throw new LogLevelException(sprintf('The level "%s" is not valid.', $level)); } $logLevel = $this->levelToInt[$level]; diff --git a/src/Core/Logger/Type/SyslogLogger.php b/src/Core/Logger/Type/SyslogLogger.php index 667b44ccc..024d47b31 100644 --- a/src/Core/Logger/Type/SyslogLogger.php +++ b/src/Core/Logger/Type/SyslogLogger.php @@ -21,10 +21,9 @@ namespace Friendica\Core\Logger\Type; -use Friendica\Core\Logger\Exception\LoggerArgumentException; use Friendica\Core\Logger\Exception\LoggerException; +use Friendica\Core\Logger\Exception\LogLevelException; use Friendica\Util\Introspection; -use Psr\Log\InvalidArgumentException; use Psr\Log\LogLevel; /** @@ -99,7 +98,8 @@ class SyslogLogger extends AbstractLogger * @param int $logOpts Indicates what logging options will be used when generating a log message * @param int $logFacility Used to specify what type of program is logging the message * - * @throws LoggerArgumentException + * @throws LogLevelException + * @throws LoggerException */ public function __construct($channel, Introspection $introspection, string $level = LogLevel::NOTICE, int $logOpts = LOG_PID, int $logFacility = LOG_USER) { @@ -117,7 +117,7 @@ class SyslogLogger extends AbstractLogger * @param string $message * @param array $context * - * @throws LoggerArgumentException in case the level isn't valid + * @throws LogLevelException in case the level isn't valid * @throws LoggerException In case the syslog cannot be opened for writing */ protected function addEntry($level, string $message, array $context = []) @@ -139,12 +139,12 @@ class SyslogLogger extends AbstractLogger * * @return int The SysLog priority * - * @throws LoggerArgumentException If the loglevel isn't valid + * @throws LogLevelException If the loglevel isn't valid */ public function mapLevelToPriority(string $level): int { if (!array_key_exists($level, $this->logLevels)) { - throw new LoggerArgumentException(sprintf('The level "%s" is not valid.', $level)); + throw new LogLevelException(sprintf('The level "%s" is not valid.', $level)); } return $this->logLevels[$level]; diff --git a/src/DI.php b/src/DI.php index ecc65bbc9..823644784 100644 --- a/src/DI.php +++ b/src/DI.php @@ -415,11 +415,11 @@ abstract class DI // /** - * @return Network\HTTPClient\Capability\ICanRequestPerHttp + * @return Network\HTTPClient\Capability\ICanSendHttpRequests */ public static function httpClient() { - return self::$dice->create(Network\HTTPClient\Capability\ICanRequestPerHttp::class); + return self::$dice->create(Network\HTTPClient\Capability\ICanSendHttpRequests::class); } // diff --git a/src/Network/HTTPClient/Capability/ICanRequestPerHttp.php b/src/Network/HTTPClient/Capability/ICanSendHttpRequests.php similarity index 99% rename from src/Network/HTTPClient/Capability/ICanRequestPerHttp.php rename to src/Network/HTTPClient/Capability/ICanSendHttpRequests.php index b7ddcb254..24eb6dc50 100644 --- a/src/Network/HTTPClient/Capability/ICanRequestPerHttp.php +++ b/src/Network/HTTPClient/Capability/ICanSendHttpRequests.php @@ -26,7 +26,7 @@ use GuzzleHttp\Exception\TransferException; /** * Interface for calling HTTP requests and returning their responses */ -interface ICanRequestPerHttp +interface ICanSendHttpRequests { /** * Fetches the content of an URL diff --git a/src/Network/HTTPClient/Client/HttpClientCan.php b/src/Network/HTTPClient/Client/HttpClient.php similarity index 98% rename from src/Network/HTTPClient/Client/HttpClientCan.php rename to src/Network/HTTPClient/Client/HttpClient.php index ea07a5c83..33bb8357e 100644 --- a/src/Network/HTTPClient/Client/HttpClientCan.php +++ b/src/Network/HTTPClient/Client/HttpClient.php @@ -24,7 +24,7 @@ namespace Friendica\Network\HTTPClient\Client; use Friendica\Core\System; use Friendica\Network\HTTPClient\Response\CurlResult; use Friendica\Network\HTTPClient\Response\GuzzleResponse; -use Friendica\Network\HTTPClient\Capability\ICanRequestPerHttp; +use Friendica\Network\HTTPClient\Capability\ICanSendHttpRequests; use Friendica\Network\HTTPClient\Capability\ICanHandleHttpResponses; use Friendica\Network\HTTPException\InternalServerErrorException; use Friendica\Util\Network; @@ -42,7 +42,7 @@ use Psr\Log\LoggerInterface; /** * Performs HTTP requests to a given URL */ -class HttpClientCan implements ICanRequestPerHttp +class HttpClient implements ICanSendHttpRequests { /** @var LoggerInterface */ private $logger; diff --git a/src/Network/HTTPClient/Factory/HttpClient.php b/src/Network/HTTPClient/Factory/HttpClient.php index 732f01b34..c7ee68a92 100644 --- a/src/Network/HTTPClient/Factory/HttpClient.php +++ b/src/Network/HTTPClient/Factory/HttpClient.php @@ -6,7 +6,7 @@ use Friendica\App; use Friendica\BaseFactory; use Friendica\Core\Config\Capability\IManageConfigValues; use Friendica\Network\HTTPClient\Client; -use Friendica\Network\HTTPClient\Capability\ICanRequestPerHttp; +use Friendica\Network\HTTPClient\Capability\ICanSendHttpRequests; use Friendica\Util\Profiler; use Friendica\Util\Strings; use GuzzleHttp; @@ -42,9 +42,9 @@ class HttpClient extends BaseFactory * * @param HandlerStack|null $handlerStack (optional) A handler replacement (just usefull at test environments) * - * @return ICanRequestPerHttp + * @return ICanSendHttpRequests */ - public function createClient(HandlerStack $handlerStack = null): ICanRequestPerHttp + public function createClient(HandlerStack $handlerStack = null): ICanSendHttpRequests { $proxy = $this->config->get('system', 'proxy'); @@ -108,6 +108,6 @@ class HttpClient extends BaseFactory // Some websites test the browser for cookie support, so this enhances results. $resolver->setCookieJar(get_temppath() .'/resolver-cookie-' . Strings::getRandomName(10)); - return new Client\HttpClientCan($logger, $this->profiler, $guzzle, $resolver); + return new Client\HttpClient($logger, $this->profiler, $guzzle, $resolver); } } diff --git a/static/dependencies.config.php b/static/dependencies.config.php index dad374d8d..a43fb368a 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -146,7 +146,7 @@ return [ 'index', ], 'call' => [ - ['create', ['index'], Dice::CHAIN_CALL], + ['create', [], Dice::CHAIN_CALL], ], ], '$devLogger' => [ @@ -224,7 +224,7 @@ return [ ['getBackend', [], Dice::CHAIN_CALL], ], ], - Network\HTTPClient\Capability\ICanRequestPerHttp::class => [ + Network\HTTPClient\Capability\ICanSendHttpRequests::class => [ 'instanceOf' => Network\HTTPClient\Factory\HttpClient::class, 'call' => [ ['createClient', [], Dice::CHAIN_CALL], diff --git a/tests/DiceHttpMockHandlerTrait.php b/tests/DiceHttpMockHandlerTrait.php index 7f77d7e4f..3381a3219 100644 --- a/tests/DiceHttpMockHandlerTrait.php +++ b/tests/DiceHttpMockHandlerTrait.php @@ -24,7 +24,7 @@ namespace Friendica\Test; use Dice\Dice; use Friendica\DI; use Friendica\Network\HTTPClient\Factory\HttpClient; -use Friendica\Network\HTTPClient\Capability\ICanRequestPerHttp; +use Friendica\Network\HTTPClient\Capability\ICanSendHttpRequests; use GuzzleHttp\HandlerStack; /** @@ -49,7 +49,7 @@ trait DiceHttpMockHandlerTrait $dice = DI::getDice(); // addRule() clones the current instance and returns a new one, so no concurrency problems :-) - $newDice = $dice->addRule(ICanRequestPerHttp::class, [ + $newDice = $dice->addRule(ICanSendHttpRequests::class, [ 'instanceOf' => HttpClient::class, 'call' => [ ['createClient', [$this->httpRequestHandler], Dice::CHAIN_CALL], diff --git a/tests/src/Core/InstallerTest.php b/tests/src/Core/InstallerTest.php index 8db5a7d5c..0e44f19a4 100644 --- a/tests/src/Core/InstallerTest.php +++ b/tests/src/Core/InstallerTest.php @@ -26,7 +26,7 @@ use Dice\Dice; use Friendica\Core\Config\ValueObject\Cache; use Friendica\DI; use Friendica\Network\HTTPClient\Capability\ICanHandleHttpResponses; -use Friendica\Network\HTTPClient\Capability\ICanRequestPerHttp; +use Friendica\Network\HTTPClient\Capability\ICanSendHttpRequests; use Friendica\Test\MockedTest; use Friendica\Test\Util\VFSTrait; use Mockery; @@ -331,7 +331,7 @@ class InstallerTest extends MockedTest ->andReturn('test Error'); // Mocking the CURL Request - $networkMock = Mockery::mock(ICanRequestPerHttp::class); + $networkMock = Mockery::mock(ICanSendHttpRequests::class); $networkMock ->shouldReceive('fetchFull') ->with('https://test/install/testrewrite') @@ -342,7 +342,7 @@ class InstallerTest extends MockedTest ->andReturn($IHTTPResult); $this->dice->shouldReceive('create') - ->with(ICanRequestPerHttp::class) + ->with(ICanSendHttpRequests::class) ->andReturn($networkMock); DI::init($this->dice); @@ -378,7 +378,7 @@ class InstallerTest extends MockedTest ->andReturn('204'); // Mocking the CURL Request - $networkMock = Mockery::mock(ICanRequestPerHttp::class); + $networkMock = Mockery::mock(ICanSendHttpRequests::class); $networkMock ->shouldReceive('fetchFull') ->with('https://test/install/testrewrite') @@ -389,7 +389,7 @@ class InstallerTest extends MockedTest ->andReturn($IHTTPResultW); $this->dice->shouldReceive('create') - ->with(ICanRequestPerHttp::class) + ->with(ICanSendHttpRequests::class) ->andReturn($networkMock); DI::init($this->dice); diff --git a/tests/src/Core/Logger/StreamLoggerTest.php b/tests/src/Core/Logger/StreamLoggerTest.php index 65ef76ea3..16c8786be 100644 --- a/tests/src/Core/Logger/StreamLoggerTest.php +++ b/tests/src/Core/Logger/StreamLoggerTest.php @@ -23,6 +23,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\Util\FileSystem; use Friendica\Test\Util\VFSTrait; use Friendica\Core\Logger\Type\StreamLogger; @@ -160,7 +161,7 @@ class StreamLoggerTest extends AbstractLoggerTest */ public function testWrongMinimumLevel() { - $this->expectException(LoggerArgumentException::class); + $this->expectException(LogLevelException::class); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $logger = new StreamLogger('test', 'file.text', $this->introspection, $this->fileSystem, 'NOPE'); @@ -171,7 +172,7 @@ class StreamLoggerTest extends AbstractLoggerTest */ public function testWrongLogLevel() { - $this->expectException(LoggerArgumentException::class); + $this->expectException(LogLevelException::class); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $logfile = vfsStream::newFile('friendica.log') diff --git a/tests/src/Core/Logger/SyslogLoggerTest.php b/tests/src/Core/Logger/SyslogLoggerTest.php index 8ba2ebc08..fc3525e0b 100644 --- a/tests/src/Core/Logger/SyslogLoggerTest.php +++ b/tests/src/Core/Logger/SyslogLoggerTest.php @@ -23,6 +23,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\Type\SyslogLogger; use Psr\Log\LogLevel; @@ -64,7 +65,7 @@ class SyslogLoggerTest extends AbstractLoggerTest */ public function testWrongMinimumLevel() { - $this->expectException(LoggerArgumentException::class); + $this->expectException(LogLevelException::class); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $logger = new SyslogLoggerWrapper('test', $this->introspection, 'NOPE'); @@ -75,7 +76,7 @@ class SyslogLoggerTest extends AbstractLoggerTest */ public function testWrongLogLevel() { - $this->expectException(LoggerArgumentException::class); + $this->expectException(LogLevelException::class); $this->expectExceptionMessageMatches("/The level \".*\" is not valid./"); $logger = new SyslogLoggerWrapper('test', $this->introspection); diff --git a/tests/src/Core/Storage/Repository/StorageManagerTest.php b/tests/src/Core/Storage/Repository/StorageManagerTest.php index c090c2696..608e48f07 100644 --- a/tests/src/Core/Storage/Repository/StorageManagerTest.php +++ b/tests/src/Core/Storage/Repository/StorageManagerTest.php @@ -40,7 +40,7 @@ use Friendica\DI; use Friendica\Core\Config\Factory\Config; use Friendica\Core\Config\Repository; use Friendica\Core\Storage\Type; -use Friendica\Network\HTTPClient\Client\HttpClientCan; +use Friendica\Network\HTTPClient\Client\HttpClient; use Friendica\Test\DatabaseTest; use Friendica\Test\Util\Database\StaticDatabase; use Friendica\Test\Util\VFSTrait; @@ -61,7 +61,7 @@ class StorageManagerTest extends DatabaseTest private $logger; /** @var L10n */ private $l10n; - /** @var HttpClientCan */ + /** @var HttpClient */ private $httpRequest; protected function setUp(): void @@ -93,7 +93,7 @@ class StorageManagerTest extends DatabaseTest $this->l10n = \Mockery::mock(L10n::class); - $this->httpRequest = \Mockery::mock(HttpClientCan::class); + $this->httpRequest = \Mockery::mock(HttpClient::class); } protected function tearDown(): void