Add input validation for file params, resolvePending, and postMultipart (#5)

Author: antonkomarev

README.md

@@ -34,7 +34,13 @@ $bot = new \BelkaTech\VkTeamsBot\Bot(
     new \BelkaTech\VkTeamsBot\Http\HttpClient(
         baseUri: 'https://api.icq.net/bot',
         token: 'YOUR_BOT_TOKEN',
-        client: new \GuzzleHttp\Client(),
+        client: new \GuzzleHttp\Client(
+            [
+                'connect_timeout' => 4,
+                'timeout' => 15,
+                'http_errors' => false,
+            ],
+        ),
         requestFactory: new \GuzzleHttp\Psr7\HttpFactory(),
         streamFactory: new \GuzzleHttp\Psr7\HttpFactory(),
     ),

src/Api/ChatsApi.php

@@ -5,7 +5,6 @@
 namespace BelkaTech\VkTeamsBot\Api;
 
 use BelkaTech\VkTeamsBot\Http\HttpClient;
-use Exception;
 use Psr\Http\Client\ClientExceptionInterface;
 
 final readonly class ChatsApi
@@ -233,33 +232,32 @@ public function unblockUser(
      * @return array{ok: bool}
      *
      * @throws ClientExceptionInterface
-     * @throws Exception
      */
     public function resolvePending(
         string $chatId,
         bool $approve,
         ?string $userId = null,
         ?bool $everyone = null,
     ): array {
-        if ($userId !== null) {
-            /** @phpstan-ignore return.type */
-            return $this->httpClient->get('/v1/chats/resolvePending', [
-                'chatId' => $chatId,
-                'approve' => $approve,
-                'userId' => $userId,
-            ]);
+        if (!($userId === null xor $everyone === null)) {
+            throw new \InvalidArgumentException('Exactly one of userId or everyone must be provided');
         }
 
-        if ($everyone !== null) {
+        if ($userId !== null) {
             /** @phpstan-ignore return.type */
             return $this->httpClient->get('/v1/chats/resolvePending', [
                 'chatId' => $chatId,
                 'approve' => $approve,
-                'everyone' => $everyone,
+                'userId' => $userId,
             ]);
         }
 
-        throw new Exception('userId or everyone must be provided');
+        /** @phpstan-ignore return.type */
+        return $this->httpClient->get('/v1/chats/resolvePending', [
+            'chatId' => $chatId,
+            'approve' => $approve,
+            'everyone' => $everyone,
+        ]);
     }
 
     /**

src/Api/MessagesApi.php

@@ -104,6 +104,10 @@ public function sendVoice(
         ?array $forwardMsgId = null,
         ?Keyboard $inlineKeyboardMarkup = null,
     ): array {
+        if (!($fileId === null xor $filePath === null)) {
+            throw new \InvalidArgumentException('Exactly one of fileId or filePath must be provided');
+        }
+
         $params = [
             'chatId' => $chatId,
             'fileId' => $fileId,
@@ -264,6 +268,10 @@ private function sendMedia(
         ?object $format,
         ?ParseModeEnum $parseMode,
     ): array {
+        if (!($fileId === null xor $filePath === null)) {
+            throw new \InvalidArgumentException('Exactly one of fileId or filePath must be provided');
+        }
+
         $params = [
             'chatId' => $chatId,
             'caption' => $caption,

src/BotEventListener.php

@@ -6,7 +6,6 @@
 
 use BelkaTech\VkTeamsBot\Enum\EventTypeEnum;
 use BelkaTech\VkTeamsBot\Event\EventDto;
-use InvalidArgumentException;
 use Psr\Http\Client\NetworkExceptionInterface;
 
 final class BotEventListener
@@ -32,7 +31,7 @@ public function onCommand(
         \Closure $handler,
     ): void {
         if (isset($this->commandHandlers[$command])) {
-            throw new InvalidArgumentException(
+            throw new \InvalidArgumentException(
                 "Command handler for '{$command}' is already registered",
             );
         }
@@ -126,7 +125,7 @@ public function listen(
         ?\Closure $onException = null,
     ): void {
         if ($pollTime < 1 || $pollTime > 60) {
-            throw new InvalidArgumentException(
+            throw new \InvalidArgumentException(
                 "pollTime must be between 1 and 60 seconds, got {$pollTime}",
             );
         }

src/Http/HttpClient.php

@@ -62,6 +62,12 @@ public function postMultipart(
         $query = http_build_query($this->filterParams($params));
         $url = $this->baseUri . $path . '?' . $query;
 
+        if (!is_file($filePath)) {
+            throw new \InvalidArgumentException(
+                "File not found: {$filePath}",
+            );
+        }
+
         $boundary = bin2hex(random_bytes(16));
 
         $body = "--{$boundary}\r\n"

test/Unit/Api/ChatsApiTest.php

@@ -6,6 +6,7 @@
 
 use BelkaTech\VkTeamsBot\Api\ChatsApi;
 use BelkaTech\VkTeamsBot\Test\Spy\HttpClientSpy;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 final class ChatsApiTest extends TestCase
@@ -166,12 +167,22 @@ public function testResolvePendingWithEveryone(): void
         $this->assertArrayNotHasKey('userId', $params);
     }
 
-    public function testResolvePendingThrowsWithoutUserIdOrEveryone(): void
+    /**
+     * @return iterable<string, array{?string, ?bool}>
+     */
+    public static function invalidResolvePendingParamsProvider(): iterable
     {
-        $this->expectException(\Exception::class);
-        $this->expectExceptionMessage('userId or everyone must be provided');
+        yield 'both null' => [null, null];
+        yield 'both provided' => ['user1', true];
+    }
+
+    #[DataProvider('invalidResolvePendingParamsProvider')]
+    public function testResolvePendingThrowsOnInvalidParams(?string $userId, ?bool $everyone): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Exactly one of userId or everyone must be provided');
 
-        $this->api->resolvePending('chat1', approve: true);
+        $this->api->resolvePending('chat1', approve: true, userId: $userId, everyone: $everyone);
     }
 
     public function testSetTitle(): void

test/Unit/Api/MessagesApiTest.php

@@ -85,6 +85,24 @@ public function testNullInlineKeyboardIsNotEncodedAsJsonNullString(): void
         $this->assertNotSame('null', $params['inlineKeyboardMarkup']);
     }
 
+    /**
+     * @return iterable<string, array{?string, ?string}>
+     */
+    public static function invalidFileParamsProvider(): iterable
+    {
+        yield 'both null' => [null, null];
+        yield 'both provided' => ['abc', '/tmp/file.txt'];
+    }
+
+    #[DataProvider('invalidFileParamsProvider')]
+    public function testSendFileThrowsOnInvalidFileParams(?string $fileId, ?string $filePath): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Exactly one of fileId or filePath must be provided');
+
+        $this->api->sendFile(chatId: 'chat1', fileId: $fileId, filePath: $filePath);
+    }
+
     public function testSendFileWithFileIdUsesGet(): void
     {
         $this->api->sendFile(chatId: 'chat1', fileId: 'abc');
@@ -114,6 +132,15 @@ public function testSendFileWithCaption(): void
         $this->assertSame('my file', $params['caption']);
     }
 
+    #[DataProvider('invalidFileParamsProvider')]
+    public function testSendVoiceThrowsOnInvalidFileParams(?string $fileId, ?string $filePath): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Exactly one of fileId or filePath must be provided');
+
+        $this->api->sendVoice(chatId: 'chat1', fileId: $fileId, filePath: $filePath);
+    }
+
     public function testSendVoiceWithFileIdUsesGet(): void
     {
         $this->api->sendVoice(chatId: 'chat1', fileId: 'voice1');

test/Unit/BotEventListenerTest.php

@@ -8,7 +8,6 @@
 use BelkaTech\VkTeamsBot\BotEventListener;
 use BelkaTech\VkTeamsBot\Event\EventDto;
 use BelkaTech\VkTeamsBot\Http\HttpClient;
-use InvalidArgumentException;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -19,7 +18,7 @@ public function testListenThrowsOnPollTimeTooLow(): void
         $bot = new Bot($this->createMock(HttpClient::class));
         $listener = new BotEventListener($bot);
 
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionMessage('pollTime must be between 1 and 60');
 
         $listener->listen(0);
@@ -30,7 +29,7 @@ public function testListenThrowsOnPollTimeTooHigh(): void
         $bot = new Bot($this->createMock(HttpClient::class));
         $listener = new BotEventListener($bot);
 
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionMessage('pollTime must be between 1 and 60');
 
         $listener->listen(61);
@@ -69,7 +68,7 @@ public function testDuplicateCommandRegistrationThrows(): void
 
         $listener->onCommand('/start', function () {});
 
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionMessage("Command handler for '/start' is already registered");
 
         $listener->onCommand('/start', function () {});

test/Unit/Http/HttpClientTest.php

@@ -232,6 +232,26 @@ public function testFilterParamsRemovesNullButKeepsOtherFalsyValues(): void
         ]);
     }
 
+    public function testPostMultipartThrowsWhenFileNotFound(): void
+    {
+        $streamFactory = $this->createMock(StreamFactoryInterface::class);
+        $requestFactory = $this->createMock(RequestFactoryInterface::class);
+        $psrClient = $this->createMock(ClientInterface::class);
+
+        $httpClient = new HttpClient(
+            baseUri: 'https://api.example.com',
+            token: 'test-token',
+            client: $psrClient,
+            requestFactory: $requestFactory,
+            streamFactory: $streamFactory,
+        );
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('File not found: /nonexistent/file.txt');
+
+        $httpClient->postMultipart('/upload', [], '/nonexistent/file.txt');
+    }
+
     public function testGetThrowsOnClientError(): void
     {
         $stream = $this->createMock(StreamInterface::class);