Sopamo / laravel-filepond

Laravel backend module for filepond uploads
MIT License
202 stars 56 forks source link

What do you mean by "append using the driver's capabilities"? #74

Open stratboy opened 5 months ago

stratboy commented 5 months ago

Hi, in your source code I find this comment:

      Laravel's local disk implementation is quite inefficient for appending data to existing files
      To be at least a bit more efficient, we build the final content ourselves, but the most efficient
      Way to do this would be to append using the driver's capabilities

But what exactly do you mean? Could you show an example? I'm using AS3, thus the League\Flysystem\AwsS3V3\AwsS3V3Adapter, but it doesn't feature any 'append' method or the like.

Thank you

Sopamo commented 5 months ago

I think that's exactly the reason why Laravel doesn't generally use a "native" append method, because not all drivers (storage providers) allow you to append data. I don't know if AWS doesnt allow it or if it's just not implemented in the driver, but I do know that some providers have a way to append data. The idea was to at some point to add specific code here to check which driver is being used, if it has an append method and use it if available. I would be open to PRs for that, but I think it needs a bit of refactoring so if someone wants to take that on, it might be a good idea to open an issue first and discuss a possible way on how to implement it exactly.

stratboy commented 5 months ago

I think that's exactly the reason why Laravel doesn't generally use a "native" append method, because not all drivers (storage providers) allow you to append data. I don't know if AWS doesnt allow it or if it's just not implemented in the driver, but I do know that some providers have a way to append data. The idea was to at some point to add specific code here to check which driver is being used, if it has an append method and use it if available. I would be open to PRs for that, but I think it needs a bit of refactoring so if someone wants to take that on, it might be a good idea to open an issue first and discuss a possible way on how to implement it exactly.

Mmm ok, thank you. If I find some time maybe I'll look into it. In the meantime, I wanted to suggest: why not use original filepond's headers instead of the equivalents server fields in chunk()? I mean:

$offset = $request->server('HTTP_UPLOAD_OFFSET');
$length = $request->server('HTTP_UPLOAD_LENGTH');

could be written as

$length = $request->header('Upload-Length'); // total size
$offset = $request->header('Upload-Offset');

The above headers are documented in filepond's docs, so they maybe generate less confusion if a coder take a look at this source code. Also, maybe we could name $length as $total_size instead, but this I admit is more a personal preference. :)

What do you think?