diff --git a/app/Services/Post/MediaAttacher.php b/app/Services/Post/MediaAttacher.php index a5320334..d84ec4ca 100644 --- a/app/Services/Post/MediaAttacher.php +++ b/app/Services/Post/MediaAttacher.php @@ -5,6 +5,7 @@ namespace App\Services\Post; use App\Enums\Media\Type as MediaType; +use App\Models\Media; use App\Models\Post; use App\Models\Workspace; use Illuminate\Support\Facades\Http; @@ -46,7 +47,8 @@ public function attachFromUrls(Post $post, array $urls): array /** * Resolve an inline media array into hosted items: items with a `path` pass - * through, external URLs are downloaded and hosted. + * through, external URLs are downloaded and hosted. Atomic — when any item + * fails, media hosted in this call is rolled back so nothing is orphaned. * * @param array $allowedTypes * @param array> $items @@ -56,6 +58,7 @@ public function resolveInlineMedia(Workspace $workspace, array $allowedTypes, ar { $media = []; $failed = []; + $hostedIds = []; foreach ($items as $item) { if (filled(data_get($item, 'path'))) { @@ -65,10 +68,20 @@ public function resolveInlineMedia(Workspace $workspace, array $allowedTypes, ar } $url = (string) data_get($item, 'url', ''); + $hosted = $this->fetchToWorkspace($workspace, $allowedTypes, $url); - ($hosted = $this->fetchToWorkspace($workspace, $allowedTypes, $url)) === null - ? $failed[] = $url - : $media[] = $hosted; + if ($hosted === null) { + $failed[] = $url; + + continue; + } + + $media[] = $hosted; + $hostedIds[] = data_get($hosted, 'id'); + } + + if ($failed !== [] && $hostedIds !== []) { + Media::query()->whereKey($hostedIds)->get()->each->delete(); } return ['media' => $media, 'failed' => $failed]; @@ -80,7 +93,7 @@ public function resolveInlineMedia(Workspace $workspace, array $allowedTypes, ar * @param array $allowedTypes * @return array|null */ - public function fetchToWorkspace(Workspace $workspace, array $allowedTypes, string $url): ?array + private function fetchToWorkspace(Workspace $workspace, array $allowedTypes, string $url): ?array { $download = $this->download($url); diff --git a/app/Support/PostMediaRules.php b/app/Support/PostMediaRules.php index ca9ca763..e57c72dd 100644 --- a/app/Support/PostMediaRules.php +++ b/app/Support/PostMediaRules.php @@ -17,13 +17,13 @@ class PostMediaRules { /** * @param bool $hosted true (web): items must already be hosted (id + path - * required) and may carry source metadata; false (API): - * a bare external `url` is accepted and downloaded. + * required); false (API): a bare external `url` is + * accepted (and downloaded). * @return array */ public static function rules(bool $hosted): array { - $rules = [ + return [ 'media' => ['sometimes', 'array'], 'media.*.id' => $hosted ? ['required', 'string'] : ['sometimes', 'nullable', 'string'], 'media.*.path' => $hosted ? ['required', 'string', 'max:500'] : ['sometimes', 'nullable', 'string', 'max:500'], @@ -35,13 +35,8 @@ public static function rules(bool $hosted): array 'media.*.original_filename' => ['sometimes', 'nullable', 'string', 'max:500'], 'media.*.size' => ['sometimes', 'nullable', 'integer'], 'media.*.meta' => ['sometimes', 'nullable', 'array'], + 'media.*.source' => ['sometimes', 'nullable', 'string', Rule::in(array_column(Source::cases(), 'value'))], + 'media.*.source_meta' => ['sometimes', 'nullable', 'array'], ]; - - if ($hosted) { - $rules['media.*.source'] = ['sometimes', 'nullable', 'string', Rule::in(array_column(Source::cases(), 'value'))]; - $rules['media.*.source_meta'] = ['sometimes', 'nullable', 'array']; - } - - return $rules; } } diff --git a/tests/Feature/Api/PostMediaApiTest.php b/tests/Feature/Api/PostMediaApiTest.php index b9179946..067f2b71 100644 --- a/tests/Feature/Api/PostMediaApiTest.php +++ b/tests/Feature/Api/PostMediaApiTest.php @@ -269,6 +269,36 @@ expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(0); }); +it('rolls back already-hosted media when another url in the batch fails', function () { + $this->socialAccount->update(['is_active' => true]); + + Http::fake([ + 'cdn.example.com/good.jpg' => Http::response( + file_get_contents(__DIR__.'/../../fixtures/1x1.png'), + 200, + ['Content-Type' => 'image/png'], + ), + 'cdn.example.com/missing.jpg' => Http::response(null, 404), + ]); + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), [ + 'content' => 'Partial media post', + 'media' => [ + ['url' => 'https://cdn.example.com/good.jpg'], + ['url' => 'https://cdn.example.com/missing.jpg'], + ], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]) + ->assertUnprocessable() + ->assertJsonValidationErrors(['media']); + + expect(Post::where('content', 'Partial media post')->exists())->toBeFalse(); + expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(0); +}); + it('passes already-hosted media through on create without downloading', function () { $this->socialAccount->update(['is_active' => true]); Http::preventStrayRequests(); diff --git a/tests/Unit/Support/PostMediaRulesTest.php b/tests/Unit/Support/PostMediaRulesTest.php index beb02baa..0b4c0d20 100644 --- a/tests/Unit/Support/PostMediaRulesTest.php +++ b/tests/Unit/Support/PostMediaRulesTest.php @@ -4,22 +4,20 @@ use App\Support\PostMediaRules; -test('hosted media rules require id and path and allow source', function () { +test('hosted media rules require id and path', function () { $rules = PostMediaRules::rules(hosted: true); expect($rules['media.*.id'])->toContain('required') ->and($rules['media.*.path'])->toContain('required') - ->and($rules)->toHaveKey('media.*.source') - ->and($rules)->toHaveKey('media.*.source_meta'); + ->and($rules['media.*.url'])->not->toContain('url:http,https'); }); -test('api media rules accept a bare external url and omit source', function () { +test('api media rules accept a bare external url with nullable id/path', function () { $rules = PostMediaRules::rules(hosted: false); expect($rules['media.*.id'])->toContain('nullable') - ->and($rules['media.*.url'])->toContain('url:http,https') - ->and($rules)->not->toHaveKey('media.*.source') - ->and($rules)->not->toHaveKey('media.*.source_meta'); + ->and($rules['media.*.path'])->toContain('nullable') + ->and($rules['media.*.url'])->toContain('url:http,https'); }); test('both variants keep the shared item keys so validated() preserves them', function () {