Merge pull request #5 from BushlanovDev/fix-upload-files

Fix upload files

docs/README.md

@@ -31,6 +31,7 @@
     - `GET /updates` (`getUpdates`) - [*Получение обновлений через Long-Polling.*](#Получение-обновлений-через-Long-Polling)
 - [Загрузка файлов](#Загрузка-файлов)
     - `POST /uploads` (`getUploadUrl`) - [*Получение URL для загрузки файла.*](#Получение-URL-для-загрузки-файла)
+    - `uploadAttachment` - [*Загрузка файла.*](#Загрузка-файла)
 - [Сообщения](#Сообщения)
     - `GET /messages` (`getMessages`) - [*Получение списка сообщений из чата.*](#Получение-списка-сообщений-из-чата)
     - `POST /messages` (`sendMessage`) - [*Отправка сообщения.*](#Отправка-сообщения)
@@ -308,6 +309,18 @@ $updateList = $api->getUpdates(
 
 ```php
 $uploadEndpoint = $api->getUploadUrl(UploadType::Video);
+// Далее вы можете загрузить файл по полученному URL самостоятельно или воспользоваться методом Client::upload()
+```
+
+### Загрузка файла
+
+Данный метод получит URL для загрузки, отправит файл и вернет готовый аттачмент
+
+```php
+$photoAttachmentRequest = $api->uploadAttachment(
+    type: UploadType::Image,
+    filePath: __DIR__ . '/test.jpg',
+);
 ```
 
 ## Сообщения
@@ -327,11 +340,17 @@ $messages = $api->getMessages(
 ### Отправка сообщения
 
 ```php
+$fileAttachmentRequest = $api->uploadAttachment(
+    type: UploadType::File,
+    filePath: __DIR__ . '/test.pdf',
+);
+
 $message = $api->sendMessage(
     userId: 12345,                      // Если вы отправляете сообщение пользователю, укажите его ID (необязательно)
     chatId: 54321,                      // Если сообщение отправляется в чат, укажите его ID (необязательно)
     text: 'Привет мир!',                // Текст сообщения (необязательно)
     attachments: [                      // Прикрепленные элементы (необязательно)
+        $fileAttachmentRequest,
         PhotoAttachmentRequest::fromUrl('https://example.com/image.jpg'),
         new LocationAttachmentRequest(
             latitude: 55.7520233,

src/Api.php

@@ -33,6 +33,7 @@ use BushlanovDev\MaxMessengerBot\Models\UpdateList;
 use BushlanovDev\MaxMessengerBot\Models\UploadEndpoint;
 use BushlanovDev\MaxMessengerBot\Models\VideoAttachmentDetails;
 use InvalidArgumentException;
+use JsonException;
 use LogicException;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
@@ -470,27 +471,51 @@ class Api
 
         $uploadEndpoint = $this->getUploadUrl($type);
 
-        $uploadResult = $this->client->upload(
-            $uploadEndpoint->url,
-            $fileHandle,
-            basename($filePath),
-        );
+        // For audio and video, the token is received *before* the upload
+        // The actual upload response is not JSON and can be ignored on success
+        if ($type === UploadType::Audio || $type === UploadType::Video) {
+            if (empty($uploadEndpoint->token)) {
+                throw new SerializationException(
+                    "API did not return a pre-upload token for type '$type->value'."
+                );
+            }
+            $this->client->upload($uploadEndpoint->url, $fileHandle, basename($filePath));
+            fclose($fileHandle);
 
-        fclose($fileHandle);
-
-        if (!isset($uploadResult['token'])) {
-            throw new SerializationException('Could not find "token" in upload server response.');
+            return match ($type) {
+                UploadType::Audio => new AudioAttachmentRequest($uploadEndpoint->token),
+                UploadType::Video => new VideoAttachmentRequest($uploadEndpoint->token),
+            };
         }
 
-        return match ($type) {
-            UploadType::Image => PhotoAttachmentRequest::fromToken($uploadResult['token']),
-            UploadType::Video => new VideoAttachmentRequest($uploadResult['token']),
-            UploadType::Audio => new AudioAttachmentRequest($uploadResult['token']),
-            UploadType::File => new FileAttachmentRequest($uploadResult['token']), // @phpstan-ignore-line
-            default => throw new LogicException(
-                "Attachment creation for type '$type->value' is not yet implemented."
-            ),
-        };
+        // For images and files, the token is in the response *after* the upload.
+        $responseBody = $this->client->upload($uploadEndpoint->url, $fileHandle, basename($filePath));
+        fclose($fileHandle);
+
+        try {
+            $uploadResult = json_decode($responseBody, true, 512, JSON_THROW_ON_ERROR);
+        } catch (JsonException $e) {
+            throw new SerializationException('Failed to decode upload server response JSON.', 0, $e);
+        }
+
+        // Using switch because match expression arms cannot be code blocks.
+        switch ($type) {
+            case UploadType::Image:
+                $photoData = current($uploadResult['photos'] ?? []); // Get first photo from response
+                if (!isset($photoData['token'])) {
+                    throw new SerializationException('Could not find "token" in photo upload response.');
+                }
+                return PhotoAttachmentRequest::fromToken($photoData['token']);
+            case UploadType::File:
+                if (!isset($uploadResult['token'])) {
+                    throw new SerializationException('Could not find "token" in file upload response.');
+                }
+                return new FileAttachmentRequest($uploadResult['token']);
+        }
+
+        // @codeCoverageIgnoreStart
+        throw new LogicException("Attachment creation for type '$type->value' is not yet implemented."); // @phpstan-ignore-line
+        // @codeCoverageIgnoreEnd
     }
 
     /**

src/Client.php

@@ -122,7 +122,7 @@ final readonly class Client implements ClientApiInterface
     /**
      * @inheritDoc
      */
-    public function upload(string $uri, mixed $fileContents, string $fileName): array
+    public function upload(string $uri, mixed $fileContents, string $fileName): string
     {
         $boundary = '--------------------------' . microtime(true);
         $bodyStream = $this->streamFactory->createStream();
@@ -152,13 +152,7 @@ final readonly class Client implements ClientApiInterface
 
         $this->handleErrorResponse($response);
 
-        $responseBody = (string)$response->getBody();
-
-        try {
-            return json_decode($responseBody, true, 512, JSON_THROW_ON_ERROR);
-        } catch (JsonException $e) {
-            throw new SerializationException('Failed to decode upload server response JSON.', 0, $e);
-        }
+        return (string)$response->getBody();
     }
 
     /**

src/ClientApiInterface.php

@@ -32,10 +32,10 @@ interface ClientApiInterface
      * @param resource|string $fileContents File content (stream resource or string).
      * @param string $fileName The name of the file that will be sent to the server.
      *
-     * @return array<string, mixed>
+     * @return string The raw response body from the upload server.
      * @throws ClientApiException
      * @throws NetworkException
      * @throws SerializationException
      */
-    public function upload(string $uri, mixed $fileContents, string $fileName): array;
+    public function upload(string $uri, mixed $fileContents, string $fileName): string;
 }

tests/ApiTest.php

@@ -451,24 +451,59 @@ final class ApiTest extends TestCase
     }
 
     #[Test]
-    public function uploadAttachmentSuccessfullyUploadsImageAndReturnsAttachment(): void
+    public function uploadAttachmentForImage(): void
     {
-        $filePath = tempnam(sys_get_temp_dir(), 'test_upload_');
-        file_put_contents($filePath, 'fake-image-content');
+        $filePath = $this->createTempFile('image-content');
+        $uploadUrl = 'https://upload.server/image';
+        $uploadResponseJson = '{"photos":{"random_key_123":{"token":"final_image_token"}}}';
+        $expectedAttachment = PhotoAttachmentRequest::fromToken('final_image_token');
 
-        $uploadType = UploadType::Image;
-        $uploadUrl = 'https://upload.server/gohere';
-        $uploadToken = 'FINAL_TOKEN_123';
+        $this->clientMock->method('request')->willReturn(['url' => $uploadUrl]);
+        $this->modelFactoryMock->method('createUploadEndpoint')->willReturn(new UploadEndpoint($uploadUrl));
 
-        $getUploadUrlResponse = ['url' => $uploadUrl];
-        $uploadResponse = ['token' => $uploadToken];
-        $expectedEndpoint = new UploadEndpoint($uploadUrl);
-        $expectedAttachment = PhotoAttachmentRequest::fromToken($uploadToken);
+        $this->clientMock->method('upload')->willReturn($uploadResponseJson);
+
+        $result = $this->api->uploadAttachment(UploadType::Image, $filePath);
+
+        $this->assertEquals($expectedAttachment, $result);
+        unlink($filePath);
+    }
+
+    #[Test]
+    public function uploadAttachmentForFile(): void
+    {
+        $filePath = $this->createTempFile('file-content');
+        $uploadUrl = 'https://upload.server/file';
+        $uploadResponseJson = '{"token":"final_file_token"}';
+        $expectedAttachment = new FileAttachmentRequest('final_file_token');
+
+        $this->clientMock->method('request')->willReturn(['url' => $uploadUrl]);
+        $this->modelFactoryMock->method('createUploadEndpoint')->willReturn(new UploadEndpoint($uploadUrl));
+
+        $this->clientMock->method('upload')->willReturn($uploadResponseJson);
+
+        $result = $this->api->uploadAttachment(UploadType::File, $filePath);
+
+        $this->assertEquals($expectedAttachment, $result);
+        unlink($filePath);
+    }
+
+    #[Test]
+    public function uploadAttachmentForAudio(): void
+    {
+        $filePath = $this->createTempFile('audio-content');
+        $uploadUrl = 'https://upload.server/audio';
+        $preUploadToken = 'pre_upload_audio_token';
+        $uploadResponse = '<retval>1</retval>';
+        $expectedAttachment = new AudioAttachmentRequest($preUploadToken);
+
+        $getUploadUrlResponse = ['url' => $uploadUrl, 'token' => $preUploadToken];
+        $expectedEndpoint = new UploadEndpoint($uploadUrl, $preUploadToken);
 
         $this->clientMock
             ->expects($this->once())
             ->method('request')
-            ->with('POST', '/uploads', ['type' => $uploadType->value])
+            ->with('POST', '/uploads', ['type' => UploadType::Audio->value])
             ->willReturn($getUploadUrlResponse);
 
         $this->modelFactoryMock
@@ -483,7 +518,7 @@ final class ApiTest extends TestCase
             ->with($uploadUrl, $this->isResource(), basename($filePath))
             ->willReturn($uploadResponse);
 
-        $result = $this->api->uploadAttachment($uploadType, $filePath);
+        $result = $this->api->uploadAttachment(UploadType::Audio, $filePath);
 
         $this->assertEquals($expectedAttachment, $result);
 
@@ -491,29 +526,49 @@ final class ApiTest extends TestCase
     }
 
     #[Test]
-    public function uploadAttachmentForMultiplePhotosReturnsCorrectAttachment(): void
+    public function uploadAttachmentForVideo(): void
     {
-        $filePath = tempnam(sys_get_temp_dir(), 'test_');
-        file_put_contents($filePath, 'content');
+        $filePath = $this->createTempFile('video-content');
+        $uploadUrl = 'https://upload.server/video';
+        $preUploadToken = 'pre_upload_video_token';
+        $uploadResponse = '<retval>1</retval>';
+        $expectedAttachment = new VideoAttachmentRequest($preUploadToken);
 
-        $getUploadUrlResponse = ['url' => 'http://upload.server'];
-        $expectedEndpoint = new UploadEndpoint('http://upload.server');
+        $getUploadUrlResponse = ['url' => $uploadUrl, 'token' => $preUploadToken];
+        $expectedEndpoint = new UploadEndpoint($uploadUrl, $preUploadToken);
 
-        $uploadResponse = ['token' => 'token'];
+        $this->clientMock
+            ->expects($this->once())
+            ->method('request')
+            ->with('POST', '/uploads', ['type' => UploadType::Video->value])
+            ->willReturn($getUploadUrlResponse);
 
-        $expectedAttachment = PhotoAttachmentRequest::fromToken('token');
+        $this->modelFactoryMock
+            ->expects($this->once())
+            ->method('createUploadEndpoint')
+            ->with($getUploadUrlResponse)
+            ->willReturn($expectedEndpoint);
 
-        $this->clientMock->method('request')->willReturn($getUploadUrlResponse);
-        $this->modelFactoryMock->method('createUploadEndpoint')->willReturn($expectedEndpoint);
-        $this->clientMock->method('upload')->willReturn($uploadResponse);
+        $this->clientMock
+            ->expects($this->once())
+            ->method('upload')
+            ->with($uploadUrl, $this->isResource(), basename($filePath))
+            ->willReturn($uploadResponse);
 
-        $result = $this->api->uploadAttachment(UploadType::Image, $filePath);
+        $result = $this->api->uploadAttachment(UploadType::Video, $filePath);
 
         $this->assertEquals($expectedAttachment, $result);
 
         unlink($filePath);
     }
 
+    private function createTempFile(string $content): string
+    {
+        $filePath = tempnam(sys_get_temp_dir(), 'test_upload_');
+        file_put_contents($filePath, $content);
+        return $filePath;
+    }
+
     #[Test]
     public function uploadAttachmentThrowsExceptionForNonExistentFile(): void
     {
@@ -673,10 +728,10 @@ final class ApiTest extends TestCase
             ->expects($this->once())
             ->method('upload')
             ->with($uploadUrl, $this->isResource(), basename($filePath))
-            ->willReturn($invalidUploadResponse);
+            ->willReturn(json_encode($invalidUploadResponse));
 
         $this->expectException(SerializationException::class);
-        $this->expectExceptionMessage('Could not find "token" in upload server response.');
+        $this->expectExceptionMessage('Could not find "token" in photo upload response.');
 
         try {
             $this->api->uploadAttachment($uploadType, $filePath);
@@ -685,86 +740,6 @@ final class ApiTest extends TestCase
         }
     }
 
-    #[Test]
-    public function uploadAttachmentSuccessfullyUploadsVideoAndReturnsAttachment(): void
-    {
-        $filePath = tempnam(sys_get_temp_dir(), 'test_video_');
-        file_put_contents($filePath, 'fake-video-content');
-
-        $uploadType = UploadType::Video;
-        $uploadUrl = 'https://upload.server/video_path';
-        $uploadToken = 'VIDEO_TOKEN_XYZ';
-
-        $getUploadUrlResponse = ['url' => $uploadUrl];
-        $uploadResponse = ['token' => $uploadToken];
-        $expectedEndpoint = new UploadEndpoint($uploadUrl);
-        $expectedAttachment = new VideoAttachmentRequest($uploadToken);
-
-        $this->clientMock
-            ->expects($this->once())
-            ->method('request')
-            ->with('POST', '/uploads', ['type' => $uploadType->value])
-            ->willReturn($getUploadUrlResponse);
-
-        $this->modelFactoryMock
-            ->expects($this->once())
-            ->method('createUploadEndpoint')
-            ->with($getUploadUrlResponse)
-            ->willReturn($expectedEndpoint);
-
-        $this->clientMock
-            ->expects($this->once())
-            ->method('upload')
-            ->with($uploadUrl, $this->isResource(), basename($filePath))
-            ->willReturn($uploadResponse);
-
-        $result = $this->api->uploadAttachment($uploadType, $filePath);
-
-        $this->assertEquals($expectedAttachment, $result);
-
-        unlink($filePath);
-    }
-
-    #[Test]
-    public function uploadAttachmentSuccessfullyUploadsAudioAndReturnsAttachment(): void
-    {
-        $filePath = tempnam(sys_get_temp_dir(), 'test_audio_');
-        file_put_contents($filePath, 'fake-audio-content');
-
-        $uploadType = UploadType::Audio;
-        $uploadUrl = 'https://upload.server/audio_path';
-        $uploadToken = 'AUDIO_TOKEN_ABC';
-
-        $getUploadUrlResponse = ['url' => $uploadUrl];
-        $uploadResponse = ['token' => $uploadToken];
-        $expectedEndpoint = new UploadEndpoint($uploadUrl);
-        $expectedAttachment = new AudioAttachmentRequest($uploadToken);
-
-        $this->clientMock
-            ->expects($this->once())
-            ->method('request')
-            ->with('POST', '/uploads', ['type' => $uploadType->value])
-            ->willReturn($getUploadUrlResponse);
-
-        $this->modelFactoryMock
-            ->expects($this->once())
-            ->method('createUploadEndpoint')
-            ->with($getUploadUrlResponse)
-            ->willReturn($expectedEndpoint);
-
-        $this->clientMock
-            ->expects($this->once())
-            ->method('upload')
-            ->with($uploadUrl, $this->isResource(), basename($filePath))
-            ->willReturn($uploadResponse);
-
-        $result = $this->api->uploadAttachment($uploadType, $filePath);
-
-        $this->assertEquals($expectedAttachment, $result);
-
-        unlink($filePath);
-    }
-
     #[Test]
     public function uploadAttachmentSuccessfullyUploadsFileAndReturnsAttachment(): void
     {
@@ -796,7 +771,7 @@ final class ApiTest extends TestCase
             ->expects($this->once())
             ->method('upload')
             ->with($uploadUrl, $this->isResource(), basename($filePath))
-            ->willReturn($uploadResponse);
+            ->willReturn(json_encode($uploadResponse));
 
         $result = $this->api->uploadAttachment($uploadType, $filePath);
 
@@ -1992,4 +1967,88 @@ final class ApiTest extends TestCase
             client: null
         );
     }
+
+    #[Test]
+    public function uploadAttachmentThrowsSerializationExceptionOnInvalidUploadResponse(): void
+    {
+        $this->expectException(SerializationException::class);
+        $this->expectExceptionMessage('Failed to decode upload server response JSON.');
+
+        $filePath = $this->createTempFile('image-content');
+        $uploadUrl = 'https://upload.server/image';
+        $invalidJsonResponse = '{not-valid-json';
+
+        $this->clientMock->method('request')->willReturn(['url' => $uploadUrl]);
+        $this->modelFactoryMock->method('createUploadEndpoint')->willReturn(new UploadEndpoint($uploadUrl));
+
+        $this->clientMock
+            ->expects($this->once())
+            ->method('upload')
+            ->willReturn($invalidJsonResponse);
+
+        try {
+            $this->api->uploadAttachment(UploadType::Image, $filePath);
+        } finally {
+            unlink($filePath);
+        }
+    }
+
+    #[Test]
+    public function uploadAttachmentForVideoThrowsExceptionOnMissingPreUploadToken(): void
+    {
+        $this->expectException(SerializationException::class);
+        $this->expectExceptionMessage("API did not return a pre-upload token for type 'video'.");
+
+        $filePath = $this->createTempFile('video-content');
+        $uploadUrl = 'https://upload.server/video';
+
+        $getUploadUrlResponse = ['url' => $uploadUrl];
+        $expectedEndpoint = new UploadEndpoint($uploadUrl, null);
+
+        $this->clientMock
+            ->expects($this->once())
+            ->method('request')
+            ->with('POST', '/uploads', ['type' => UploadType::Video->value])
+            ->willReturn($getUploadUrlResponse);
+
+        $this->modelFactoryMock
+            ->expects($this->once())
+            ->method('createUploadEndpoint')
+            ->with($getUploadUrlResponse)
+            ->willReturn($expectedEndpoint);
+
+        $this->clientMock->expects($this->never())->method('upload');
+
+        try {
+            $this->api->uploadAttachment(UploadType::Video, $filePath);
+        } finally {
+            unlink($filePath);
+        }
+    }
+
+    #[Test]
+    public function uploadAttachmentForFileThrowsExceptionOnMissingPostUploadToken(): void
+    {
+        $this->expectException(SerializationException::class);
+        $this->expectExceptionMessage('Could not find "token" in file upload response.');
+
+        $filePath = $this->createTempFile('file-content');
+        $uploadUrl = 'https://upload.server/file';
+
+        $invalidUploadResponse = json_encode(['status' => 'success', 'file_id' => 123]);
+
+        $this->clientMock->method('request')->willReturn(['url' => $uploadUrl]);
+        $this->modelFactoryMock->method('createUploadEndpoint')->willReturn(new UploadEndpoint($uploadUrl));
+
+        $this->clientMock
+            ->expects($this->once())
+            ->method('upload')
+            ->willReturn($invalidUploadResponse);
+
+        try {
+            $this->api->uploadAttachment(UploadType::File, $filePath);
+        } finally {
+            unlink($filePath);
+        }
+    }
 }

tests/ClientTest.php

@@ -314,7 +314,7 @@ final class ClientTest extends TestCase
         $this->streamMock->method('__toString')->willReturn(json_encode($responsePayload));
 
         $result = $this->client->upload($uploadUrl, $fileContents, $fileName);
-        $this->assertSame($responsePayload, $result);
+        $this->assertSame(json_encode($responsePayload), $result);
     }
 
     #[Test]
@@ -338,7 +338,7 @@ final class ClientTest extends TestCase
 
         $result = $this->client->upload($uploadUrl, $tmpFileHandle, $fileName);
 
-        $this->assertSame($responsePayload, $result);
+        $this->assertSame(json_encode($responsePayload), $result);
         fclose($tmpFileHandle);
     }
 
@@ -361,26 +361,6 @@ final class ClientTest extends TestCase
         $this->client->upload('http://some.url', 'content', 'file.txt');
     }
 
-    #[Test]
-    public function uploadThrowsSerializationExceptionOnInvalidJsonResponse(): void
-    {
-        $this->expectException(SerializationException::class);
-        $this->expectExceptionMessage('Failed to decode upload server response JSON.');
-
-        $this->requestFactoryMock->method('createRequest')->willReturn($this->requestMock);
-        $this->requestMock->method('withHeader')->willReturn($this->requestMock);
-        $this->requestMock->method('withBody')->willReturn($this->requestMock);
-
-        $this->httpClientMock
-            ->method('sendRequest')
-            ->with($this->requestMock)
-            ->willReturn($this->responseMock);
-
-        $this->responseMock->method('getStatusCode')->willReturn(200);
-        $this->streamMock->method('__toString')->willReturn('{not-a-valid-json');
-        $this->client->upload('http://some.url', 'content', 'file.txt');
-    }
-
     #[Test]
     public function requestLogsRequestAndResponseOnDebugLevel(): void
     {
@@ -409,4 +389,24 @@ final class ClientTest extends TestCase
 
         $this->client->request('GET', '/not/found');
     }
+
+    #[Test]
+    public function uploadMethodReturnsRawStringResponse(): void
+    {
+        $uploadUrl = 'https://upload.server/path';
+        $fileContents = 'data';
+        $fileName = 'file.txt';
+        $rawResponse = '<retval>1</retval>';
+
+        $this->requestFactoryMock->method('createRequest')->willReturn($this->requestMock);
+        $this->requestMock->method('withHeader')->willReturn($this->requestMock);
+        $this->requestMock->method('withBody')->willReturn($this->requestMock);
+        $this->httpClientMock->method('sendRequest')->willReturn($this->responseMock);
+
+        $this->responseMock->method('getStatusCode')->willReturn(200);
+        $this->streamMock->method('__toString')->willReturn($rawResponse);
+
+        $result = $this->client->upload($uploadUrl, $fileContents, $fileName);
+        $this->assertSame($rawResponse, $result);
+    }
 }