refactor: clarify multipart is an object-storage fast path, not a disk limit
Local/public already used the assemble-then-store flow. Rename supports() so it is obvious every filesystem disk works; multipart only kicks in for S3-compatible remote disks. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
cd79554cfb
commit
d4be085642
3 changed files with 28 additions and 28 deletions
|
|
@ -83,7 +83,7 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedCloudUplo
|
|||
|
||||
$identifier = md5($request->user()->id.$fileName.$totalSize);
|
||||
|
||||
if ($cloudUploader->supports() && $cloudUploader->shouldUseMultipart($fileName)) {
|
||||
if ($cloudUploader->shouldUseMultipart($fileName)) {
|
||||
$result = $cloudUploader->receiveChunk(
|
||||
$identifier,
|
||||
$fileName,
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@
|
|||
use Throwable;
|
||||
|
||||
/**
|
||||
* Multipart chunked uploads to S3-compatible disks (R2), so each request only
|
||||
* sends one part instead of re-uploading the full file on finalize.
|
||||
* Fast path for large uploads on object-storage disks (S3 / R2 / Spaces).
|
||||
* Local and public disks keep the normal assemble-then-store flow — both work.
|
||||
*/
|
||||
class ChunkedCloudUploader
|
||||
{
|
||||
|
|
@ -31,24 +31,29 @@ public function __construct(
|
|||
private readonly ?string $disk = null,
|
||||
) {}
|
||||
|
||||
public function supports(?string $disk = null): bool
|
||||
/**
|
||||
* Use S3 multipart when the default disk is object storage and the file is
|
||||
* a video/PDF. Images still assemble locally (format normalization). Local
|
||||
* and public disks always return false and use the regular chunk path.
|
||||
*/
|
||||
public function shouldUseMultipart(string $fileName, ?string $disk = null): bool
|
||||
{
|
||||
if (! $this->isObjectStorageDisk($disk)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$type = MediaType::fromExtension(pathinfo($fileName, PATHINFO_EXTENSION));
|
||||
|
||||
return in_array($type, [MediaType::Video, MediaType::Document], true);
|
||||
}
|
||||
|
||||
public function isObjectStorageDisk(?string $disk = null): bool
|
||||
{
|
||||
$disk ??= $this->diskName();
|
||||
|
||||
return config("filesystems.disks.{$disk}.driver") === 's3';
|
||||
}
|
||||
|
||||
/**
|
||||
* Images still need local assembly for format normalization. Videos and
|
||||
* PDFs are safe to multipart directly to object storage.
|
||||
*/
|
||||
public function shouldUseMultipart(string $fileName): bool
|
||||
{
|
||||
$type = MediaType::fromExtension(pathinfo($fileName, PATHINFO_EXTENSION));
|
||||
|
||||
return in_array($type, [MediaType::Video, MediaType::Document], true);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{done: bool, progress: int, path?: string, size?: int, mime_type?: string}
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -17,20 +17,16 @@
|
|||
Cache::flush();
|
||||
});
|
||||
|
||||
test('chunked cloud uploader supports only s3 disks', function () {
|
||||
$uploader = new ChunkedCloudUploader(Cache::store(), disk: 'local');
|
||||
expect($uploader->supports('local'))->toBeFalse();
|
||||
test('chunked cloud uploader only uses multipart on object storage for video and pdf', function () {
|
||||
$local = new ChunkedCloudUploader(Cache::store(), disk: 'local');
|
||||
expect($local->shouldUseMultipart('clip.mp4'))->toBeFalse();
|
||||
expect($local->shouldUseMultipart('photo.png'))->toBeFalse();
|
||||
|
||||
config(['filesystems.disks.r2.driver' => 's3']);
|
||||
expect($uploader->supports('r2'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('chunked cloud uploader uses multipart for video and pdf only', function () {
|
||||
$uploader = new ChunkedCloudUploader(Cache::store());
|
||||
|
||||
expect($uploader->shouldUseMultipart('clip.mp4'))->toBeTrue();
|
||||
expect($uploader->shouldUseMultipart('deck.pdf'))->toBeTrue();
|
||||
expect($uploader->shouldUseMultipart('photo.png'))->toBeFalse();
|
||||
$r2 = new ChunkedCloudUploader(Cache::store(), disk: 'r2');
|
||||
expect($r2->shouldUseMultipart('clip.mp4'))->toBeTrue();
|
||||
expect($r2->shouldUseMultipart('deck.pdf'))->toBeTrue();
|
||||
expect($r2->shouldUseMultipart('photo.png'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('chunked cloud uploader uploads parts and completes multipart', function () {
|
||||
|
|
@ -96,7 +92,6 @@
|
|||
]);
|
||||
|
||||
$fake = Mockery::mock(ChunkedCloudUploader::class);
|
||||
$fake->shouldReceive('supports')->andReturn(true);
|
||||
$fake->shouldReceive('shouldUseMultipart')->with('clip.mp4')->andReturn(true);
|
||||
$fake->shouldReceive('receiveChunk')
|
||||
->once()
|
||||
|
|
|
|||
Loading…
Reference in a new issue