vignesh/trypost
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: harden chunked cloud uploads after full PR review

Browse source 
Fix real edge cases: enforce 5MiB non-final parts, track offsets for ordered/idempotent chunks, size from bytes received, delete orphaned R2 objects if Media create fails, and split the controller into clearer paths.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Paulo Castellano 2026-07-23 11:16:39 -03:00
parent 3c59dce349
commit 08ce75d2ec
3 changed files with 213 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

102
app/Http/Controllers/App/AssetController.php
View file

@ -9,6 +9,7 @@
use App\Http\Requests\App\Asset\StoreChunkedAssetRequest;
use App\Http\Resources\App\MediaResource;
use App\Models\Media;
use App\Models\Workspace;
use App\Services\Brand\SafeHttpFetcher;
use App\Services\Media\ChunkedCloudUploader;
use App\Services\UnsplashService;
@ -22,6 +23,7 @@
use Inertia\Response;
use RuntimeException;
use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
use Throwable;
class AssetController extends Controller
{
@ -80,11 +82,12 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedCloudUplo
$totalSize = (int) $request->validated('total_size');
$fileName = (string) $request->validated('file_name');
$chunk = $request->getContent();
$identifier = md5($request->user()->id.$fileName.$totalSize);
if ($cloudUploader->shouldUseMultipart($fileName)) {
$result = $cloudUploader->receiveChunk(
return $this->storeChunkedViaMultipart(
$cloudUploader,
$workspace,
$identifier,
$fileName,
$chunk,
@ -92,58 +95,99 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedCloudUplo
$rangeEnd,
$totalSize,
);
}
if (! data_get($result, 'done')) {
return response()->json([
'done' => false,
'progress' => data_get($result, 'progress'),
]);
}
return $this->storeChunkedViaLocalAssemble(
$workspace,
$identifier,
$fileName,
$chunk,
$rangeStart,
$rangeEnd,
$totalSize,
);
}
private function storeChunkedViaMultipart(
ChunkedCloudUploader $cloudUploader,
Workspace $workspace,
string $identifier,
string $fileName,
string $chunk,
int $rangeStart,
int $rangeEnd,
int $totalSize,
): JsonResponse {
$result = $cloudUploader->receiveChunk(
$identifier,
$fileName,
$chunk,
$rangeStart,
$rangeEnd,
$totalSize,
);
if (! data_get($result, 'done')) {
return response()->json([
'done' => false,
'progress' => data_get($result, 'progress'),
]);
}
$path = (string) data_get($result, 'path');
try {
$media = $workspace->addMediaFromStoredPath(
(string) data_get($result, 'path'),
$path,
$fileName,
(string) data_get($result, 'mime_type'),
(int) data_get($result, 'size'),
'assets',
);
} catch (Throwable $exception) {
Storage::delete($path);
return response()->json([
'done' => true,
'id' => $media->id,
'path' => $media->path,
'url' => $media->url,
'type' => $media->type->value,
'mime_type' => $media->mime_type,
'original_filename' => $media->original_filename,
'size' => $media->size,
'meta' => $media->meta,
'created_at' => $media->created_at->toISOString(),
]);
throw $exception;
}
return $this->chunkedMediaResponse($media);
}
private function storeChunkedViaLocalAssemble(
Workspace $workspace,
string $identifier,
string $fileName,
string $chunk,
int $rangeStart,
int $rangeEnd,
int $totalSize,
): JsonResponse {
$tempFile = storage_path("app/private/chunks/{$identifier}");
$directory = dirname($tempFile);
if (! is_dir($directory)) {
mkdir($directory, 0755, true);
if (! is_dir(dirname($tempFile))) {
mkdir(dirname($tempFile), 0755, true);
}
file_put_contents($tempFile, $chunk, $rangeStart === 0 ? 0 : FILE_APPEND);
$isLastChunk = ($rangeEnd + 1) >= $totalSize;
if (! $isLastChunk) {
if (($rangeEnd + 1) < $totalSize) {
return response()->json([
'done' => false,
'progress' => (int) round(($rangeEnd + 1) / $totalSize * 100),
]);
}
$media = $workspace->addMediaFromPath($tempFile, $fileName, 'assets');
try {
$media = $workspace->addMediaFromPath($tempFile, $fileName, 'assets');
} finally {
@unlink($tempFile);
}
@unlink($tempFile);
return $this->chunkedMediaResponse($media);
}
private function chunkedMediaResponse(Media $media): JsonResponse
{
return response()->json([
'done' => true,
'id' => $media->id,

106
app/Services/Media/ChunkedCloudUploader.php
View file

@ -11,19 +11,19 @@
use Illuminate\Filesystem\AwsS3V3Adapter;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\Str;
use InvalidArgumentException;
use RuntimeException;
use Throwable;
/**
* 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
{
private const CACHE_PREFIX = 'chunked-cloud-upload:';
private const CACHE_TTL_HOURS = 6;
/** S3/R2 require every non-final multipart part to be at least 5 MiB. */
public const MIN_PART_BYTES = 5 * 1024 * 1024;
public function __construct(
private readonly CacheRepository $cache,
private readonly ?S3Client $client = null,
@ -32,9 +32,9 @@ public function __construct(
) {}
/**
* 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.
* Multipart is only needed when the disk is remote object storage and the
* file will not be rewritten locally (videos/PDFs). Local, public, and
* image uploads keep the assemble-then-store path.
*/
public function shouldUseMultipart(string $fileName, ?string $disk = null): bool
{
@ -66,24 +66,50 @@ public function receiveChunk(
int $totalSize,
): array {
$cacheKey = self::CACHE_PREFIX.$identifier;
$client = $this->s3();
$bucket = $this->bucket();
$chunkSize = strlen($chunk);
$isLastChunk = ($rangeEnd + 1) >= $totalSize;
if ($rangeEnd < $rangeStart || $chunkSize !== ($rangeEnd - $rangeStart + 1)) {
throw new InvalidArgumentException('Chunk bytes do not match Content-Range.');
}
if (! $isLastChunk && $chunkSize < self::MIN_PART_BYTES) {
throw new InvalidArgumentException(
'Non-final multipart parts must be at least '.self::MIN_PART_BYTES.' bytes.'
);
}
$state = $this->cache->get($cacheKey);
if ($rangeStart === 0) {
$this->abortIfPresent($cacheKey);
$state = $this->startUpload($client, $bucket, $fileName, $chunk);
} else {
$state = $this->cache->get($cacheKey);
$alreadyAcceptedFirstPart = is_array($state) && (int) data_get($state, 'next_offset', 0) > 0;
if (! is_array($state)) {
throw new RuntimeException('Chunked cloud upload session expired or missing.');
if (! $alreadyAcceptedFirstPart) {
$this->abortIfPresent($cacheKey);
$state = $this->startUpload($fileName, $chunk);
$this->cache->put($cacheKey, $state, now()->addHours(self::CACHE_TTL_HOURS));
}
} elseif (! is_array($state)) {
throw new RuntimeException('Chunked cloud upload session expired or missing.');
}
$nextOffset = (int) data_get($state, 'next_offset', 0);
// Idempotent replay: client retried a chunk the server already accepted.
if ($rangeStart < $nextOffset) {
return $this->status($state, $totalSize, completed: $nextOffset >= $totalSize);
}
if ($rangeStart !== $nextOffset) {
throw new InvalidArgumentException(
"Unexpected chunk offset {$rangeStart}, expected {$nextOffset}."
);
}
$partNumber = count(data_get($state, 'parts', [])) + 1;
$result = $client->uploadPart([
'Bucket' => $bucket,
$result = $this->s3()->uploadPart([
'Bucket' => $this->bucket(),
'Key' => data_get($state, 'key'),
'UploadId' => data_get($state, 'upload_id'),
'PartNumber' => $partNumber,
@ -94,20 +120,17 @@ public function receiveChunk(
'ETag' => data_get($result, 'ETag'),
'PartNumber' => $partNumber,
];
$isLastChunk = ($rangeEnd + 1) >= $totalSize;
$state['next_offset'] = $rangeEnd + 1;
$state['bytes_received'] = (int) data_get($state, 'bytes_received', 0) + $chunkSize;
if (! $isLastChunk) {
$this->cache->put($cacheKey, $state, now()->addHours(self::CACHE_TTL_HOURS));
return [
'done' => false,
'progress' => (int) round(($rangeEnd + 1) / $totalSize * 100),
];
return $this->status($state, $totalSize, completed: false);
}
$client->completeMultipartUpload([
'Bucket' => $bucket,
$this->s3()->completeMultipartUpload([
'Bucket' => $this->bucket(),
'Key' => data_get($state, 'key'),
'UploadId' => data_get($state, 'upload_id'),
'MultipartUpload' => [
@ -117,26 +140,47 @@ public function receiveChunk(
$this->cache->forget($cacheKey);
return $this->status($state, $totalSize, completed: true);
}
/**
* @param array<string, mixed> $state
* @return array{done: bool, progress: int, path?: string, size?: int, mime_type?: string}
*/
private function status(array $state, int $totalSize, bool $completed): array
{
$received = (int) data_get($state, 'bytes_received', 0);
$progress = $totalSize > 0
? (int) round(min($received, $totalSize) / $totalSize * 100)
: 0;
if (! $completed) {
return [
'done' => false,
'progress' => $progress,
];
}
return [
'done' => true,
'progress' => 100,
'path' => (string) data_get($state, 'key'),
'size' => $totalSize,
'size' => $received,
'mime_type' => (string) data_get($state, 'mime_type'),
];
}
/**
* @return array{upload_id: string, key: string, mime_type: string, parts: array<int, array{ETag: string, PartNumber: int}>}
* @return array{upload_id: string, key: string, mime_type: string, parts: array<int, mixed>, next_offset: int, bytes_received: int}
*/
private function startUpload(S3Client $client, string $bucket, string $fileName, string $firstChunk): array
private function startUpload(string $fileName, string $firstChunk): array
{
$extension = strtolower((string) pathinfo($fileName, PATHINFO_EXTENSION));
$key = 'medias/'.Str::uuid().'.'.$extension;
$mimeType = $this->detectMimeType($firstChunk, $extension);
$created = $client->createMultipartUpload([
'Bucket' => $bucket,
$created = $this->s3()->createMultipartUpload([
'Bucket' => $this->bucket(),
'Key' => $key,
'ContentType' => $mimeType,
]);
@ -146,6 +190,8 @@ private function startUpload(S3Client $client, string $bucket, string $fileName,
'key' => $key,
'mime_type' => $mimeType,
'parts' => [],
'next_offset' => 0,
'bytes_received' => 0,
];
}

65
tests/Feature/ChunkedCloudUploadTest.php
View file

@ -114,7 +114,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0
->andReturn(new Result([]));
$uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2');
$chunk1 = str_repeat('a', 100);
$chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES);
$chunk2 = str_repeat('b', 50);
$total = strlen($chunk1) + strlen($chunk2);
@ -137,6 +137,69 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0
expect(Cache::get('chunked-cloud-upload:id-1'))->toBeNull();
});
test('chunked cloud uploader rejects undersized non-final parts', function () {
$uploader = new ChunkedCloudUploader(
Cache::store(),
Mockery::mock(S3Client::class),
'test-bucket',
'r2',
);
expect(fn () => $uploader->receiveChunk(
'id-small',
'video.mp4',
str_repeat('a', 100),
0,
99,
ChunkedCloudUploader::MIN_PART_BYTES + 200,
))->toThrow(InvalidArgumentException::class);
});
test('chunked cloud uploader rejects unexpected offsets', function () {
$client = Mockery::mock(S3Client::class);
$client->shouldReceive('createMultipartUpload')
->once()
->andReturn(new Result(['UploadId' => 'upload-1']));
$client->shouldReceive('uploadPart')
->once()
->andReturn(new Result(['ETag' => '"etag-a"']));
$uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2');
$chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES);
$total = ChunkedCloudUploader::MIN_PART_BYTES * 2;
$uploader->receiveChunk('id-gap', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total);
expect(fn () => $uploader->receiveChunk(
'id-gap',
'video.mp4',
$chunk1,
ChunkedCloudUploader::MIN_PART_BYTES + 10,
ChunkedCloudUploader::MIN_PART_BYTES * 2 + 9,
$total,
))->toThrow(InvalidArgumentException::class);
});
test('chunked cloud uploader is idempotent when a chunk is retried', function () {
$client = Mockery::mock(S3Client::class);
$client->shouldReceive('createMultipartUpload')
->once()
->andReturn(new Result(['UploadId' => 'upload-1']));
$client->shouldReceive('uploadPart')
->once()
->andReturn(new Result(['ETag' => '"etag-a"']));
$uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2');
$chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES);
$total = ChunkedCloudUploader::MIN_PART_BYTES + 50;
$first = $uploader->receiveChunk('id-retry', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total);
$retry = $uploader->receiveChunk('id-retry', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total);
expect($first)->toMatchArray(['done' => false]);
expect($retry)->toMatchArray(['done' => false, 'progress' => $first['progress']]);
});
// ─── HTTP: local / public assemble path ──────────────────────────
test('chunked upload stores video on the local disk via assemble path', function () {