From ca13096ec2644cc3b2fc76ac2bae9455ffa9b7f2 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 7 Aug 2025 19:04:07 +0300 Subject: [PATCH] Added PSR LoggerInterface --- composer.json | 1 + src/Api.php | 23 +++++++++++++++++++---- src/Client.php | 24 ++++++++++++++++++++++++ src/WebhookHandler.php | 8 ++++++++ tests/ApiTest.php | 18 +++++++++++------- tests/ClientTest.php | 33 +++++++++++++++++++++++++++++++++ tests/WebhookHandlerTest.php | 21 +++++++++++++++++++++ 7 files changed, 117 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index d723cca..50776a4 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ "ext-json": "*", "guzzlehttp/guzzle": "^6.5.8||^7.0", "guzzlehttp/psr7": "^1.8||^2.0", + "psr/log": "^3.0", "psr/http-client": "^1.0", "psr/http-factory": "^1.0", "psr/http-message": "^1.0||^2.0" diff --git a/src/Api.php b/src/Api.php index 3956199..2f76d8e 100644 --- a/src/Api.php +++ b/src/Api.php @@ -37,6 +37,8 @@ use BushlanovDev\MaxMessengerBot\Models\VideoAttachmentDetails; use InvalidArgumentException; use LogicException; use Psr\Http\Message\ServerRequestInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use ReflectionException; use RuntimeException; @@ -50,7 +52,7 @@ class Api { public const string API_VERSION = '0.0.6'; - private const string API_BASE_URL = 'https://botapi.max.ru'; + private const string API_BASE_URL = 'http://127.0.0.1:5001'; private const string METHOD_GET = 'GET'; private const string METHOD_POST = 'POST'; @@ -77,12 +79,15 @@ class Api private readonly ModelFactory $modelFactory; + private readonly LoggerInterface $logger; + /** * Api constructor. * * @param string $accessToken Your bot's access token from @MasterBot. * @param ClientApiInterface|null $client Http api client. * @param ModelFactory|null $modelFactory + * @param LoggerInterface|null $logger * * @throws InvalidArgumentException */ @@ -90,7 +95,10 @@ class Api string $accessToken, ?ClientApiInterface $client = null, ?ModelFactory $modelFactory = null, + ?LoggerInterface $logger = null, ) { + $this->logger = $logger ?? new NullLogger(); + if ($client === null) { if (!class_exists(\GuzzleHttp\Client::class) || !class_exists(\GuzzleHttp\Psr7\HttpFactory::class)) { throw new LogicException( @@ -108,6 +116,7 @@ class Api $httpFactory, self::API_BASE_URL, self::API_VERSION, + $this->logger, ); } @@ -144,7 +153,7 @@ class Api */ public function createWebhookHandler(?string $secret = null): WebhookHandler { - return new WebhookHandler($this, $this->modelFactory, $secret); + return new WebhookHandler($this, $this->modelFactory, $secret, $this->logger); } /** @@ -251,10 +260,16 @@ class Api try { $this->processUpdatesBatch($handlers, $timeout, $marker); } catch (NetworkException $e) { - error_log('Network error: ' . $e->getMessage()); + $this->logger->error( + 'Long-polling network error: {message}', + ['message' => $e->getMessage(), 'exception' => $e], + ); sleep(5); } catch (\Exception $e) { - error_log('An error occurred: ' . $e->getMessage()); + $this->logger->error( + 'An error occurred during long-polling: {message}', + ['message' => $e->getMessage(), 'exception' => $e], + ); sleep(1); } } diff --git a/src/Client.php b/src/Client.php index 8531093..5721bd5 100644 --- a/src/Client.php +++ b/src/Client.php @@ -19,6 +19,8 @@ use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamFactoryInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; /** * The low-level HTTP client responsible for communicating with the Max Bot API. @@ -34,6 +36,7 @@ final readonly class Client implements ClientApiInterface * @param StreamFactoryInterface $streamFactory A PSR-17 factory for creating request body streams. * @param string $baseUrl The base URL for API requests. * @param string|null $apiVersion The API version to use for requests. + * @param LoggerInterface $logger * * @throws InvalidArgumentException */ @@ -44,6 +47,7 @@ final readonly class Client implements ClientApiInterface private StreamFactoryInterface $streamFactory, private string $baseUrl, private ?string $apiVersion = null, + private LoggerInterface $logger = new NullLogger(), ) { if (empty($accessToken)) { throw new InvalidArgumentException('Access token cannot be empty.'); @@ -60,6 +64,12 @@ final readonly class Client implements ClientApiInterface $queryParams['v'] = $this->apiVersion; } + $this->logger->debug('Sending API request', [ + 'method' => $method, + 'url' => $this->baseUrl . $uri, + 'body' => $body, + ]); + $fullUrl = $this->baseUrl . $uri . '?' . http_build_query($queryParams); $request = $this->requestFactory->createRequest($method, $fullUrl); @@ -79,6 +89,10 @@ final readonly class Client implements ClientApiInterface $response = $this->httpClient->sendRequest($request); } catch (ClientExceptionInterface $e) { // This catches network errors, DNS failures, timeouts, etc. + $this->logger->error('Network exception during API request', [ + 'message' => $e->getMessage(), + 'exception' => $e, + ]); throw new NetworkException($e->getMessage(), $e->getCode(), $e); } @@ -86,6 +100,11 @@ final readonly class Client implements ClientApiInterface $responseBody = (string)$response->getBody(); + $this->logger->debug('Received API response', [ + 'status' => $response->getStatusCode(), + 'body' => $responseBody, + ]); + // Handle successful but empty responses (e.g., from DELETE endpoints) if (empty($responseBody)) { // The API spec often returns {"success": true}, so we can simulate that @@ -161,6 +180,11 @@ final readonly class Client implements ClientApiInterface $errorCode = $data['code'] ?? 'unknown'; $errorMessage = $data['message'] ?? 'An unknown error occurred.'; + $this->logger->error('API error response received', [ + 'status' => $statusCode, + 'body' => $responseBody, + ]); + throw match ($statusCode) { 401 => new UnauthorizedException($errorMessage, $errorCode, $response), 403 => new ForbiddenException($errorMessage, $errorCode, $response), diff --git a/src/WebhookHandler.php b/src/WebhookHandler.php index a1371f8..768d35a 100644 --- a/src/WebhookHandler.php +++ b/src/WebhookHandler.php @@ -9,6 +9,8 @@ use BushlanovDev\MaxMessengerBot\Exceptions\SecurityException; use BushlanovDev\MaxMessengerBot\Exceptions\SerializationException; use BushlanovDev\MaxMessengerBot\Models\Updates\AbstractUpdate; use Psr\Http\Message\ServerRequestInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; /** * A class designed to process incoming webhook requests from the Max API. @@ -26,11 +28,13 @@ final class WebhookHandler * @param Api $api An instance of the Api to be passed to handlers for immediate responses. * @param ModelFactory $modelFactory An instance of the model factory to create Update objects. * @param string|null $secret The secret key provided during webhook subscription to verify requests. + * @param LoggerInterface $logger A PSR-3 compatible logger. */ public function __construct( private readonly Api $api, private readonly ModelFactory $modelFactory, private readonly ?string $secret = null, + private readonly LoggerInterface $logger = new NullLogger(), ) { } @@ -254,6 +258,8 @@ final class WebhookHandler $payload = (string)$request->getBody(); $signature = $request->getHeaderLine('X-Max-Bot-Api-Secret'); + $this->logger->debug('Received webhook payload', ['body' => $payload]); + if (empty($payload)) { throw new SerializationException('Webhook body is empty.'); } @@ -263,6 +269,7 @@ final class WebhookHandler try { $data = json_decode($payload, true, 512, JSON_THROW_ON_ERROR); } catch (\JsonException $e) { + $this->logger->error('Failed to decode webhook JSON', ['payload' => $payload, 'exception' => $e]); throw new SerializationException('Failed to decode webhook body as JSON.', 0, $e); } @@ -297,6 +304,7 @@ final class WebhookHandler } if (!hash_equals($this->secret, $signature)) { + $this->logger->warning('Webhook signature verification failed', ['received_signature' => $signature]); throw new SecurityException('Signature verification failed.'); } } diff --git a/tests/ApiTest.php b/tests/ApiTest.php index 3ca97de..784ea15 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -70,6 +70,7 @@ use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; use ReflectionClass; use RuntimeException; @@ -122,6 +123,8 @@ final class ApiTest extends TestCase private MockObject&ClientApiInterface $clientMock; private MockObject&ModelFactory $modelFactoryMock; + private MockObject&LoggerInterface $loggerMock; + private Api $api; /** @@ -133,8 +136,9 @@ final class ApiTest extends TestCase $this->clientMock = $this->createMock(ClientApiInterface::class); $this->modelFactoryMock = $this->createMock(ModelFactory::class); + $this->loggerMock = $this->createMock(LoggerInterface::class); - $this->api = new Api('fake-token', $this->clientMock, $this->modelFactoryMock); + $this->api = new Api('fake-token', $this->clientMock, $this->modelFactoryMock, $this->loggerMock); } #[Test] @@ -773,10 +777,12 @@ final class ApiTest extends TestCase $handlers = [UpdateType::MessageCreated->value => fn() => null]; $apiMock = $this->getMockBuilder(Api::class) - ->setConstructorArgs(['fake-token', $this->clientMock, $this->modelFactoryMock]) + ->setConstructorArgs(['fake-token', $this->clientMock, $this->modelFactoryMock, $this->loggerMock]) ->onlyMethods(['processUpdatesBatch']) ->getMock(); + $this->loggerMock->expects($this->once())->method('error'); + $apiMock->expects($this->any()) ->method('processUpdatesBatch') ->willReturnCallback(function () { @@ -790,8 +796,6 @@ final class ApiTest extends TestCase } }); - $this->expectOutputRegex('/Network error: Simulated network error/'); - try { $apiMock->handleUpdates($handlers); } catch (\Error $e) { @@ -828,10 +832,12 @@ final class ApiTest extends TestCase $handlers = [UpdateType::MessageCreated->value => fn() => null]; $apiMock = $this->getMockBuilder(Api::class) - ->setConstructorArgs(['fake-token', $this->clientMock, $this->modelFactoryMock]) + ->setConstructorArgs(['fake-token', $this->clientMock, $this->modelFactoryMock, $this->loggerMock]) ->onlyMethods(['processUpdatesBatch']) ->getMock(); + $this->loggerMock->expects($this->once())->method('error'); + $apiMock->expects($this->any()) ->method('processUpdatesBatch') ->willReturnCallback(function () { @@ -847,8 +853,6 @@ final class ApiTest extends TestCase } }); - $this->expectOutputRegex('/An error occurred: Simulated JSON error/'); - try { $apiMock->handleUpdates($handlers); } catch (\Error $e) { diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 0506f6f..2dbd2bc 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -28,6 +28,7 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; +use Psr\Log\LoggerInterface; #[CoversClass(Client::class)] final class ClientTest extends TestCase @@ -42,6 +43,7 @@ final class ClientTest extends TestCase private MockObject&RequestInterface $requestMock; private MockObject&ResponseInterface $responseMock; private MockObject&StreamInterface $streamMock; + private MockObject&LoggerInterface $loggerMock; private Client $client; @@ -61,6 +63,7 @@ final class ClientTest extends TestCase $this->requestMock = $this->createMock(RequestInterface::class); $this->responseMock = $this->createMock(ResponseInterface::class); $this->streamMock = $this->createMock(StreamInterface::class); + $this->loggerMock = $this->createMock(LoggerInterface::class); // Common mock setups $this->requestFactoryMock->method('createRequest')->willReturn($this->requestMock); @@ -75,6 +78,7 @@ final class ClientTest extends TestCase $this->streamFactory, self::API_BASE_URL, self::API_VERSION, + $this->loggerMock, ); } @@ -376,4 +380,33 @@ final class ClientTest extends TestCase $this->streamMock->method('__toString')->willReturn('{not-a-valid-json'); $this->client->upload('http://some.url', 'content', 'file.txt'); } + + #[Test] + public function requestLogsRequestAndResponseOnDebugLevel(): void + { + $this->responseMock->method('getStatusCode')->willReturn(200); + $this->streamMock->method('__toString')->willReturn('{"success":true}'); + + $this->loggerMock + ->expects($this->exactly(2)) + ->method('debug'); + + $this->client->request('GET', '/me'); + } + + #[Test] + public function handleErrorResponseLogsWarning(): void + { + $this->responseMock->method('getStatusCode')->willReturn(404); + $this->streamMock->method('__toString')->willReturn('{"code":"not.found","message":"Not Found"}'); + + $this->loggerMock + ->expects($this->once()) + ->method('error') + ->with('API error response received', $this->anything()); + + $this->expectException(NotFoundException::class); + + $this->client->request('GET', '/not/found'); + } } diff --git a/tests/WebhookHandlerTest.php b/tests/WebhookHandlerTest.php index 01ec307..bd4e931 100644 --- a/tests/WebhookHandlerTest.php +++ b/tests/WebhookHandlerTest.php @@ -25,6 +25,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; #[CoversClass(WebhookHandler::class)] #[UsesClass(Message::class)] @@ -38,6 +39,8 @@ final class WebhookHandlerTest extends TestCase private MockObject&Api $apiMock; private MockObject&ModelFactory $modelFactoryMock; + private MockObject&LoggerInterface $loggerMock; + private const string SECRET = 'my-super-secret-key'; protected function setUp(): void @@ -45,6 +48,7 @@ final class WebhookHandlerTest extends TestCase parent::setUp(); $this->apiMock = $this->createMock(Api::class); $this->modelFactoryMock = $this->createMock(ModelFactory::class); + $this->loggerMock = $this->createMock(LoggerInterface::class); } private function createValidUpdatePayload(): string @@ -261,4 +265,21 @@ final class WebhookHandlerTest extends TestCase $webhookHandler = new WebhookHandler($this->apiMock, $this->modelFactoryMock); $webhookHandler->handle(null); } + + #[Test] + public function verifySignatureLogsWarningOnFailure(): void + { + $request = new ServerRequest( + 'POST', '/webhook', ['X-Max-Bot-Api-Secret' => 'wrong-signature'], $this->createValidUpdatePayload() + ); + $webhookHandler = new WebhookHandler($this->apiMock, $this->modelFactoryMock, self::SECRET, $this->loggerMock); + + $this->loggerMock + ->expects($this->once()) + ->method('warning') + ->with('Webhook signature verification failed', ['received_signature' => 'wrong-signature']); + + $this->expectException(SecurityException::class); + $webhookHandler->parseUpdate($request); + } }