From 51f15b514f20c9c89c144ec15e112e2659c5143e Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Thu, 12 Nov 2020 08:41:58 +0100 Subject: [PATCH] Introduce Logger plugin --- README.md | 1 - composer.json | 1 + src/Client/Http/LoggerPlugin.php | 43 +++++++++++++++++++++++++ src/Client/PsrClickHouseAsyncClient.php | 7 ---- src/Client/PsrClickHouseClient.php | 9 ------ src/Logger/LoggerChain.php | 16 +++++---- src/Logger/PsrLogger.php | 7 ++-- src/Logger/SqlLogger.php | 5 ++- tests/Logger/LoggerChainTest.php | 17 ++++------ tests/WithClient.php | 5 --- 10 files changed, 66 insertions(+), 45 deletions(-) create mode 100644 src/Client/Http/LoggerPlugin.php diff --git a/README.md b/README.md index 5929123..2807f95 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,6 @@ Naming used here is the same as in ClickHouse docs. - All [ClickHouse Formats](https://clickhouse.yandex/docs/en/interfaces/formats/) support - Logging ([PSR-3 compliant](https://www.php-fig.org/psr/psr-3/)) - SQL Factory for [parameters "binding"](#parameters-binding) -- Dependency only on PSR interfaces, Guzzle A+ Promises for async requests and Safe ## Contents diff --git a/composer.json b/composer.json index 5bc2075..b8362ea 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "require": { "php-64bit": "^7.4", "guzzlehttp/promises": "^1.4", + "php-http/client-common": "^2.0", "psr/http-client": "^1.0", "psr/http-factory": "^1.0", "psr/http-message": "^1.0", diff --git a/src/Client/Http/LoggerPlugin.php b/src/Client/Http/LoggerPlugin.php new file mode 100644 index 0000000..ebd1a16 --- /dev/null +++ b/src/Client/Http/LoggerPlugin.php @@ -0,0 +1,43 @@ +logger = $logger; + } + + public function handleRequest(RequestInterface $request, callable $next, callable $first) : Promise + { + $id = uniqid('', true); + $this->logger->startQuery($id, (string) $request->getBody()); + + return $next($request)->then( + function (ResponseInterface $response) use ($id) : ResponseInterface { + $this->logger->stopQuery($id); + + return $response; + }, + function (Throwable $throwable) use ($id) : void { + $this->logger->stopQuery($id); + + throw $throwable; + } + ); + } +} diff --git a/src/Client/PsrClickHouseAsyncClient.php b/src/Client/PsrClickHouseAsyncClient.php index 6b8b5c7..9242062 100644 --- a/src/Client/PsrClickHouseAsyncClient.php +++ b/src/Client/PsrClickHouseAsyncClient.php @@ -9,7 +9,6 @@ use GuzzleHttp\Promise\PromiseInterface; use Http\Client\HttpAsyncClient; use Psr\Http\Message\ResponseInterface; -use Psr\Log\LoggerInterface; use SimPod\ClickHouseClient\Client\Http\RequestFactory; use SimPod\ClickHouseClient\Client\Http\RequestOptions; use SimPod\ClickHouseClient\Exception\ServerError; @@ -24,8 +23,6 @@ class PsrClickHouseAsyncClient implements ClickHouseAsyncClient private RequestFactory $requestFactory; - private LoggerInterface $logger; - private string $endpoint; /** @var array */ @@ -37,14 +34,12 @@ class PsrClickHouseAsyncClient implements ClickHouseAsyncClient public function __construct( HttpAsyncClient $asyncClient, RequestFactory $requestFactory, - LoggerInterface $logger, string $endpoint, array $defaultParameters = [], ?DateTimeZone $clickHouseTimeZone = null ) { $this->asyncClient = $asyncClient; $this->requestFactory = $requestFactory; - $this->logger = $logger; $this->endpoint = $endpoint; $this->defaultParameters = $defaultParameters; $this->sqlFactory = new SqlFactory(new ValueFormatter($clickHouseTimeZone)); @@ -90,8 +85,6 @@ private function executeRequest( array $requestParameters = [], ?callable $processResponse = null ) : PromiseInterface { - $this->logger->debug($sql, $requestParameters); - $request = $this->requestFactory->prepareRequest( $this->endpoint, new RequestOptions( diff --git a/src/Client/PsrClickHouseClient.php b/src/Client/PsrClickHouseClient.php index 56a787d..90dbd85 100644 --- a/src/Client/PsrClickHouseClient.php +++ b/src/Client/PsrClickHouseClient.php @@ -12,7 +12,6 @@ use SimPod\ClickHouseClient\Exception\CannotInsert; use SimPod\ClickHouseClient\Exception\ServerError; use SimPod\ClickHouseClient\Format\Format; -use SimPod\ClickHouseClient\Logger\SqlLogger; use SimPod\ClickHouseClient\Output\Output; use SimPod\ClickHouseClient\Sql\Escaper; use SimPod\ClickHouseClient\Sql\SqlFactory; @@ -31,8 +30,6 @@ class PsrClickHouseClient implements ClickHouseClient private RequestFactory $requestFactory; - private SqlLogger $logger; - private string $endpoint; /** @var array */ @@ -46,14 +43,12 @@ class PsrClickHouseClient implements ClickHouseClient public function __construct( ClientInterface $client, RequestFactory $requestFactory, - SqlLogger $logger, string $endpoint, array $defaultParameters = [], ?DateTimeZone $clickHouseTimeZone = null ) { $this->client = $client; $this->requestFactory = $requestFactory; - $this->logger = $logger; $this->endpoint = $endpoint; $this->defaultParameters = $defaultParameters; $this->valueFormatter = new ValueFormatter($clickHouseTimeZone); @@ -171,12 +166,8 @@ private function executeRequest(string $sql, array $requestParameters = []) : Re ) ); - $this->logger->startQuery($sql); - $response = $this->client->sendRequest($request); - $this->logger->stopQuery(); - if ($response->getStatusCode() !== 200) { throw ServerError::fromResponse($response); } diff --git a/src/Logger/LoggerChain.php b/src/Logger/LoggerChain.php index 111fef1..3d6617f 100644 --- a/src/Logger/LoggerChain.php +++ b/src/Logger/LoggerChain.php @@ -4,6 +4,8 @@ namespace SimPod\ClickHouseClient\Logger; +use function array_filter; + final class LoggerChain implements SqlLogger { /** @var SqlLogger[] */ @@ -12,21 +14,23 @@ final class LoggerChain implements SqlLogger /** @param SqlLogger[] $loggers */ public function __construct(array $loggers = []) { - $this->loggers = $loggers; + $this->loggers = array_filter( + $loggers, + static fn (SqlLogger $logger) : bool => ! $logger instanceof self + ); } - /** @inheritdoc */ - public function startQuery(string $sql, array $params = []) : void + public function startQuery(string $id, string $sql) : void { foreach ($this->loggers as $logger) { - $logger->startQuery($sql, $params); + $logger->startQuery($id, $sql); } } - public function stopQuery() : void + public function stopQuery(string $id) : void { foreach ($this->loggers as $logger) { - $logger->stopQuery(); + $logger->stopQuery($id); } } } diff --git a/src/Logger/PsrLogger.php b/src/Logger/PsrLogger.php index 4cfc545..a5bf8c2 100644 --- a/src/Logger/PsrLogger.php +++ b/src/Logger/PsrLogger.php @@ -15,13 +15,12 @@ public function __construct(LoggerInterface $logger) $this->logger = $logger; } - /** @inheritdoc */ - public function startQuery(string $sql, array $params = []) : void + public function startQuery(string $id, string $sql) : void { - $this->logger->debug($sql, $params); + $this->logger->debug($sql); } - public function stopQuery() : void + public function stopQuery(string $id) : void { } } diff --git a/src/Logger/SqlLogger.php b/src/Logger/SqlLogger.php index a670beb..fd42ced 100644 --- a/src/Logger/SqlLogger.php +++ b/src/Logger/SqlLogger.php @@ -6,8 +6,7 @@ interface SqlLogger { - /** @param array $params */ - public function startQuery(string $sql, array $params = []) : void; + public function startQuery(string $id, string $sql) : void; - public function stopQuery() : void; + public function stopQuery(string $id) : void; } diff --git a/tests/Logger/LoggerChainTest.php b/tests/Logger/LoggerChainTest.php index 156b7f1..545f0b2 100644 --- a/tests/Logger/LoggerChainTest.php +++ b/tests/Logger/LoggerChainTest.php @@ -13,24 +13,22 @@ final class LoggerChainTest extends TestCase public function testLog() : void { $logger = new class implements SqlLogger { - public ?string $sql = null; + public string $id; - /** @var array|null $params */ - public ?array $params = null; + public ?string $sql = null; public bool $started = false; public bool $stopped = false; - /** @inheritDoc */ - public function startQuery(string $sql, array $params = []) : void + public function startQuery(string $id, string $sql) : void { + $this->id = $id; $this->sql = $sql; - $this->params = $params; $this->started = true; } - public function stopQuery() : void + public function stopQuery(string $id) : void { $this->stopped = true; } @@ -38,11 +36,10 @@ public function stopQuery() : void $chain = new LoggerChain([$logger]); - $chain->startQuery('sql', []); - $chain->stopQuery(); + $chain->startQuery('a', 'sql'); + $chain->stopQuery('a'); self::assertSame('sql', $logger->sql); - self::assertSame([], $logger->params); self::assertTrue($logger->started); self::assertTrue($logger->stopped); } diff --git a/tests/WithClient.php b/tests/WithClient.php index 152019b..bf5b7ca 100644 --- a/tests/WithClient.php +++ b/tests/WithClient.php @@ -6,13 +6,11 @@ use Http\Client\Curl\Client; use Nyholm\Psr7\Factory\Psr17Factory; -use Psr\Log\Test\TestLogger; use SimPod\ClickHouseClient\Client\ClickHouseAsyncClient; use SimPod\ClickHouseClient\Client\ClickHouseClient; use SimPod\ClickHouseClient\Client\Http\RequestFactory; use SimPod\ClickHouseClient\Client\PsrClickHouseAsyncClient; use SimPod\ClickHouseClient\Client\PsrClickHouseClient; -use SimPod\ClickHouseClient\Logger\PsrLogger; use function assert; use function getenv; @@ -64,7 +62,6 @@ public function restartClickHouseClient() : void new Psr17Factory(), new Psr17Factory() ), - new PsrLogger(new TestLogger()), $endpoint, $defaultParameters ); @@ -78,7 +75,6 @@ public function restartClickHouseClient() : void new Psr17Factory(), new Psr17Factory() ), - new PsrLogger(new TestLogger()), $endpoint, $defaultParameters ); @@ -90,7 +86,6 @@ public function restartClickHouseClient() : void new Psr17Factory(), new Psr17Factory() ), - new TestLogger(), $endpoint, $defaultParameters );