[Feature]: Upload video from external folder like FTP

[ASECU-Cloud]

Description

I notice that when we want to upload a video the parameter require is path to file(local file). I try to duplicate uploadVideo function and remove the function to check the file locally and just return the streamData (the video) and it work.

here is the code streamAPI.php

public function uploadVideoFTP(
        int $libraryId,
        string $videoId,
        string $ftpStream,
        array $query = [],
    ): BunnyClientResponseInterface {
        $endpoint = new UploadVideoFTP();

        ParameterValidator::validate($query, $endpoint->getQuery());

        return $this->client->request(
            endpoint: $endpoint,
            parameters: [$libraryId, $videoId],
            query: $query,
            body: $ftpStream,
        );
    }

and this code is duplicating uploadVideo.php

public function uploadVideoFTP(
        int $libraryId,
        string $videoId,
        string $ftpStream,
        array $query = [],
    ): BunnyClientResponseInterface {
        $endpoint = new UploadVideoFTP();

        ParameterValidator::validate($query, $endpoint->getQuery());

        return $this->client->request(
            endpoint: $endpoint,
            parameters: [$libraryId, $videoId],
            query: $query,
            body: $ftpStream,
        );
    }

but i don't know what is the best way to make this code better, so i create this issues so you can take a look. thanks

Example

No response

[ASECU-Cloud]

<?php

declare(strict_types=1);

namespace ToshY\BunnyNet\Model\API\Stream\ManageVideos;

use ToshY\BunnyNet\Enum\Header;
use ToshY\BunnyNet\Enum\Method;
use ToshY\BunnyNet\Enum\Type;
use ToshY\BunnyNet\Model\AbstractParameter;
use ToshY\BunnyNet\Model\EndpointInterface;
use ToshY\BunnyNet\Model\EndpointQueryInterface;

class UploadVideoFTP implements EndpointInterface, EndpointQueryInterface
{
    public function getMethod(): Method
    {
        return Method::PUT;
    }

    public function getPath(): string
    {
        return 'library/%d/videos/%s';
    }

    public function getHeaders(): array
    {
        return [
            Header::CONTENT_TYPE_OCTET_STREAM,
        ];
    }

    public function getQuery(): array
    {
        return [
            new AbstractParameter(name: 'enabledResolutions', type: Type::STRING_TYPE),
        ];
    }
}

i forget to attach this aswell

[ToshY]

Hey @ASECU-Cloud 👋

Okay, so if I understand your use case correctly, you've already read in the file contents and want to upload that instead of using a filepath? So the following snippet is what you're currently trying to achieve:

$myFile = 'test.mp4';
// Read in file contents (either from FTP or somewhere else).
$fileContents = file_get_contents($myFile);

/*
 * As the `uploadVideo` method  requires a local filepath, you cannot upload the `$fileContents` right now.
 * That's why you suggested a new method, `uploadVideoFTP`, that allows uploading the contents.
 */
$streamAPI->uploadVideoFTP(
    libraryId: 1,
    videoId: 'e7e9b99a-ea2a-434a-b200-f6615e7b6abd',
    ftpStream: $fileContents,
    query: [
        'enabledResolutions' => '240p,360p,480p,720p,1080p,1440p,2160p',
    ],
);

It the above example similar to your use case? If so, you've pointed out a valid issue with the current implementation, and I haven't thought about it before. It would however not just be a problem specific to FTP but basically any other source if you've already read in the file's contents. That's why I propose a slighlty different solution.

Instead of creating a new method, we can "simplify" the current uploadVideo method by doing the following:

1. Change the string $localFilePath to mixed $body
2. Remove call to helper BodyContentHelper::openFileStream($localFilePath)

/**
 * @throws ClientExceptionInterface
 * @throws Exception\BunnyClientResponseException
 * @throws Exception\JSONException
 * @throws Exception\InvalidTypeForKeyValueException
 * @throws Exception\InvalidTypeForListValueException
 * @throws Exception\ParameterIsRequiredException
 * @param int $libraryId
 * @param string $videoId
 * @param mixed $body
 * @param array<string,mixed> $query
 * @return BunnyClientResponseInterface
 */
public function uploadVideo(
    int $libraryId,
    string $videoId,
    mixed $body,
    array $query = [],
): BunnyClientResponseInterface {
    $endpoint = new UploadVideo();

    ParameterValidator::validate($query, $endpoint->getQuery());

    return $this->client->request(
        endpoint: $endpoint,
        parameters: [$libraryId, $videoId],
        query: $query,
        body: $body,
    );
}

This comes with some pros and cons:

• Pros:
  • Typed body parameter as mixed now allows for underlying types string|resource|StreamInterface|null (see Nyholm\Psr7\Request)
  • Less responsibility 👍
• Cons:
  • For consistency, the BaseAPI::importDnsRecords and EdgeStorageAPI::uploadFile would also need to be refactored.
  • Users would be required to read in the file/stream before calling uploadVideo, making it a breaking change 😐

---

Let me know what you think.

[ASECU-Cloud]

yes, i knew about the pros and cons, and because of that i do not really know what is the best solution is,

what about the normal way of uploading but adding override parameter (boolean)?? like the default is reading the file from local but we can add overide which body to use

public function uploadVideo(
    int $libraryId,
    string $videoId,
    mixed $body,
    boolean $external = false
    array $query = [],
): BunnyClientResponseInterface {
    $endpoint = new UploadVideo();
//adding check the $external value here? 
    ParameterValidator::validate($query, $endpoint->getQuery());

    return $this->client->request(
        endpoint: $endpoint,
        parameters: [$libraryId, $videoId],
        query: $query,
        body: $body,
    );
}

i have really bad english, sorry XD

[ToshY]

i have really bad english, sorry XD

That's not a problem, as long s we understand eachother 🙂

what about the normal way of uploading but adding override parameter (boolean)??

While adding a boolean argument and adding an if statement inside the method could definitely work (making it not a breaking change), my main issue is that the method now would gain complexity and the boolean flags is actually a violation of the Single Responsiblity Principle.

So the more I think about it, the more I'm convinced it should not be the responsibility of the library to read in body/file contents, and instead should be handled before actually calling the upload method.

StreamAPI::uploadVideo

```php public function uploadVideo( int $libraryId, string $videoId, mixed $body, array $query = [], ): BunnyClientResponseInterface { $endpoint = new UploadVideo(); ParameterValidator::validate($query, $endpoint->getQuery()); return $this->client->request( endpoint: $endpoint, parameters: [$libraryId, $videoId], query: $query, body: $body, ); } ```

Use case:

<?php

require 'vendor/autoload.php';

use ToshY\BunnyNet\StreamAPI;

$streamApi = new StreamAPI(
    apiKey: '710d5fb6-d923-43d6-87f8-ea65c09e76dc',
    client: new BunnyClient(
        client: new \Symfony\Component\HttpClient\Psr18Client()
    )
);

// Local file (1): Read in the file contents on whatever way you like.
$body = file_get_contents('./bunny-hop.mp4');

// Local file (2): Use the existing BodyContentHelper.
$body = BodyContentHelper::openFileStream('./bunny-hop.mp4');

/*
 * Remote file: 
 * Get resource from external source. 
 * Example uses `$filesystem`, representing Flysystem FtpAdapter, to get the resource `./bunny-hop.mp4' on FTP server.
 */
$body = $filesystem->readStream('./bunny-hop.mp4');

// Pass `$body`, which is of type `string|resource|StreamInterface|null` to `uploadVideo`.
$streamApi->uploadVideo(
    libraryId: 1,
    videoId: 'e7e9b99a-ea2a-434a-b200-f6615e7b6abd',
    body: $body,
    query: [
        'enabledResolutions' => '240p,360p,480p,720p,1080p,1440p,2160p',
    ],
);

[ASECU-Cloud]

So the more I think about it, the more I'm convinced it should not be the responsibility of the library to read in body/file contents, and instead should be handled before actually calling the upload method.

i agree, so you gonna make breaking change now? or maybe put in on hold?

[ToshY]

i agree, so you gonna make breaking change now? or maybe put in on hold?

I will implement this as this would definitely be an improvement.

[ToshY]

Hey @ASECU-Cloud 👋

This has been released in 4.0.0. You can check the documentation section Upload Video on how to use it.

If you have any questions or remarks, let me know 👌