From 335180d7a158f9dccd497242150c12600c44378b Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Mon, 4 May 2026 15:17:48 -0300 Subject: [PATCH] refactor: move post-state concerns out of MediaAttacher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MediaAttacher had grown into a 180-line class with four private methods doing different things — download, validation, storage, and post-state mutation. Two of those (lock-and-merge into the JSON media column, intersection of allowed media types per platform) are post concerns, not network concerns. Move them onto the Post model: Post::allowedMediaTypes(): array The intersection of media types accepted by every enabled platform. Used to be inlined as MediaAttacher::allowedMediaTypesFor. Post::appendMedia(array $items): void Lock-then-merge into the post's media[] JSON column so concurrent writers don't overwrite each other. Used to be MediaAttacher::mergeIntoPostMedia. MediaAttacher now reads cleanly: attachFromUrls — orchestrate (loop URLs, batch the appendMedia) attachOne — download → validate type/size → handoff download — Http::sink + progress abort, returns DTO Drops to ~120 lines with no nested concerns. The streaming progress abort uses the same throw-RuntimeException pattern, but now contained in download() instead of leaking into the orchestrator. --- app/Models/Post.php | 48 ++++++++ app/Services/Post/MediaAttacher.php | 177 ++++++++++------------------ 2 files changed, 111 insertions(+), 114 deletions(-) diff --git a/app/Models/Post.php b/app/Models/Post.php index 77328c36..04f0c9fe 100644 --- a/app/Models/Post.php +++ b/app/Models/Post.php @@ -5,6 +5,7 @@ namespace App\Models; use App\DataTransferObjects\MediaItem; +use App\Enums\Media\Type; use App\Enums\Post\Status as PostStatus; use Database\Factories\PostFactory; use Illuminate\Database\Eloquent\Builder; @@ -16,6 +17,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\DB; class Post extends Model { @@ -129,4 +131,50 @@ public function markAsFailed(): void { $this->update(['status' => PostStatus::Failed]); } + + /** + * MediaTypes accepted by this post — the intersection of what every + * enabled platform allows. With no platform enabled, accept anything. + * + * @return array + */ + public function allowedMediaTypes(): array + { + $platforms = $this->postPlatforms() + ->where('enabled', true) + ->with('socialAccount') + ->get() + ->pluck('socialAccount.platform') + ->filter(); + + if ($platforms->isEmpty()) { + return Type::cases(); + } + + $sets = $platforms + ->map(fn ($platform) => array_map(fn ($type) => $type->value, $platform->allowedMediaTypes())) + ->all(); + + return array_map( + Type::from(...), + array_values(array_intersect(...$sets)), + ); + } + + /** + * Append items to the JSON `media` column under a row lock so + * concurrent writers don't overwrite each other's appends. + * + * @param array> $items + */ + public function appendMedia(array $items): void + { + DB::transaction(function () use ($items): void { + $fresh = static::whereKey($this->id)->lockForUpdate()->first(); + $fresh->update([ + 'media' => collect($fresh->media ?? [])->concat($items)->all(), + ]); + $this->setRawAttributes($fresh->getAttributes(), true); + }); + } } diff --git a/app/Services/Post/MediaAttacher.php b/app/Services/Post/MediaAttacher.php index 13a6160d..4698961e 100644 --- a/app/Services/Post/MediaAttacher.php +++ b/app/Services/Post/MediaAttacher.php @@ -6,36 +6,17 @@ use App\Enums\Media\Type as MediaType; use App\Models\Post; -use App\Models\Workspace; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; use RuntimeException; /** - * Downloads public URLs and attaches them as media to a post. Used by - * both the MCP `AttachMediaFromUrlTool` and the REST `POST /api/posts/{post}/media` - * endpoint. + * Downloads public URLs and attaches them as media to a post — used by + * the MCP `AttachMediaFromUrlTool` and the REST attach-media endpoint. * - * URL syntax (`url:http,https`) and DNS resolvability (`active_url`) are - * enforced at the request validation layer. SSRF defense beyond that is - * the responsibility of network-level egress controls in production. - * - * Flow per URL: - * 1. Stream the body to a temp file via Http::sink + a progress - * callback that aborts once we exceed the largest configured - * per-type cap (video = 1 GB by default, see config/trypost.php). - * 2. Resolve the MediaType from the response's Content-Type via - * `MediaType::fromMime()`. Reject if the type isn't accepted by - * the intersection of platforms enabled on the post. - * 3. Enforce the per-type cap (`MediaType::Image->maxSizeInBytes()` - * vs `Video`) — a 100 MB jpeg is rejected even though we - * streamed up to the video cap. - * 4. Hand off to `Workspace::addMediaFromPath()` so storage path, - * MIME re-detection, image normalization, and the Media row stay - * in one place (same path as the web upload flow). - * 5. Append the resulting media item to the post's `media[]` JSON - * column under a row lock so concurrent attach calls don't clobber - * each other. + * URL syntax + DNS resolvability are validated at the request layer + * (`url:http,https`, `active_url`). Locking, intersection of accepted + * media types, and the JSON-column merge live on the Post model so this + * class can stay focused on the network → MIME-validate → handoff flow. */ class MediaAttacher { @@ -45,78 +26,46 @@ class MediaAttacher */ public function attachFromUrls(Post $post, array $urls): array { - $allowedTypes = $this->allowedMediaTypesFor($post); - $attached = []; $failed = []; foreach ($urls as $url) { - $item = $this->processOne($post->workspace, $url, $allowedTypes); - - if ($item === null) { - $failed[] = $url; - - continue; - } - - $attached[] = $item; + ($item = $this->attachOne($post, $url)) === null + ? $failed[] = $url + : $attached[] = $item; } if ($attached !== []) { - $this->mergeIntoPostMedia($post, $attached); + $post->appendMedia($attached); } return ['attached' => $attached, 'failed' => $failed]; } /** - * @param array $allowedTypes * @return array|null */ - private function processOne(Workspace $workspace, string $url, array $allowedTypes): ?array + private function attachOne(Post $post, string $url): ?array { - // Use the largest configured per-type cap as the streaming-abort - // threshold; the actual per-type limit is checked below once we - // know the MIME. - $streamCap = MediaType::Video->maxSizeInBytes(); + $download = $this->download($url); - $temp = tempnam(sys_get_temp_dir(), 'media_'); + if ($download === null) { + return null; + } try { - $response = Http::timeout(20) - ->sink($temp) - ->withOptions([ - 'allow_redirects' => false, - 'progress' => static function ($total, $downloaded) use ($streamCap): void { - if ($downloaded > $streamCap) { - throw new RuntimeException('exceeded max bytes'); - } - }, - ]) - ->get($url); + $type = MediaType::fromMime($download['mime'] ?? ''); - $bytes = filesize($temp) ?: 0; - - if (! $response->successful() || $bytes === 0) { + if ($type === null || ! in_array($type, $post->allowedMediaTypes(), true)) { return null; } - $mime = trim(explode(';', (string) $response->header('Content-Type'))[0]); - $type = MediaType::fromMime($mime); - - if ($type === null || ! in_array($type, $allowedTypes, true)) { + if ($download['bytes'] > $type->maxSizeInBytes()) { return null; } - // Per-type size enforcement. Image is 10 MB even though we - // streamed up to the video cap, so a 100 MB jpeg is rejected - // here before we persist it. - if ($bytes > $type->maxSizeInBytes()) { - return null; - } - - $originalFilename = basename(parse_url($url, PHP_URL_PATH) ?? '') ?: 'download.bin'; - $media = $workspace->addMediaFromPath($temp, $originalFilename, 'assets'); + $name = basename(parse_url($url, PHP_URL_PATH) ?? '') ?: 'download.bin'; + $media = $post->workspace->addMediaFromPath($download['path'], $name, 'assets'); return [ 'id' => $media->id, @@ -126,55 +75,55 @@ private function processOne(Workspace $workspace, string $url, array $allowedTyp 'mime_type' => $media->mime_type, 'original_filename' => $media->original_filename, ]; - } catch (RuntimeException) { - return null; } finally { + @unlink($download['path']); + } + } + + /** + * Stream the URL to a temp file, aborting once we exceed the largest + * configured per-type cap (video). The actual per-type limit is + * enforced by the caller after we know the MIME. + * + * @return array{path: string, mime: ?string, bytes: int}|null + */ + private function download(string $url): ?array + { + $cap = MediaType::Video->maxSizeInBytes(); + $temp = tempnam(sys_get_temp_dir(), 'media_'); + + try { + $response = Http::timeout(20) + ->sink($temp) + ->withOptions([ + 'allow_redirects' => false, + 'progress' => static function ($total, $downloaded) use ($cap): void { + if ($downloaded > $cap) { + throw new RuntimeException('exceeded max bytes'); + } + }, + ]) + ->get($url); + } catch (RuntimeException) { @unlink($temp); - } - } - /** - * Lock-then-merge so concurrent attach calls don't overwrite each - * other's appended items in the JSON `media` column. - * - * @param array> $attached - */ - private function mergeIntoPostMedia(Post $post, array $attached): void - { - DB::transaction(function () use ($post, $attached): void { - $fresh = Post::whereKey($post->id)->lockForUpdate()->first(); - $fresh->update([ - 'media' => collect($fresh->media ?? [])->concat($attached)->all(), - ]); - $post->setRawAttributes($fresh->getAttributes(), true); - }); - } - - /** - * Intersection of allowed media types across platforms enabled on - * the post. With no enabled platform, accept anything we support. - * - * @return array - */ - private function allowedMediaTypesFor(Post $post): array - { - $enabledPlatforms = $post->postPlatforms() - ->where('enabled', true) - ->with('socialAccount') - ->get() - ->pluck('socialAccount.platform') - ->filter(); - - if ($enabledPlatforms->isEmpty()) { - return [MediaType::Image, MediaType::Video]; + return null; } - $sets = $enabledPlatforms - ->map(fn ($platform) => array_map(fn ($type) => $type->value, $platform->allowedMediaTypes())) - ->all(); + $bytes = filesize($temp) ?: 0; - $intersection = array_values(array_intersect(...$sets)); + if (! $response->successful() || $bytes === 0) { + @unlink($temp); - return array_map(fn ($value) => MediaType::from($value), $intersection); + return null; + } + + $mime = trim(explode(';', (string) $response->header('Content-Type'))[0]); + + return [ + 'path' => $temp, + 'mime' => $mime !== '' ? $mime : null, + 'bytes' => $bytes, + ]; } }