fix(api): roll back hosted media when an inline url batch partially fails
Final-review follow-ups: - MediaAttacher::resolveInlineMedia now deletes the media it hosted in this call when any item fails, so a mixed [good, bad] batch no longer orphans the good item's Media row + file while the request is correctly rejected with 422. Makes the create/update media resolution truly all-or-nothing. - PostMediaRules: keep source/source_meta on both contracts (the API previously passed them through with no item rules — don't silently drop them) so the media item shape is uniform; only id/path/url differ by contract. - Make MediaAttacher::fetchToWorkspace private (no external callers). - Test the partial-batch rollback (no Media, no post persisted).
This commit is contained in:
Paulo Castellano
parent
1242cc7384
commit
df891574f7
4 changed files with 58 additions and 22 deletions
|
|
@ -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<MediaType> $allowedTypes
|
||||
* @param array<int, array<string, mixed>> $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<MediaType> $allowedTypes
|
||||
* @return array<string, mixed>|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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string, mixed>
|
||||
*/
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
Loading…
Reference in a new issue