diff --git a/app/Util/ActivityPub/Helpers.php b/app/Util/ActivityPub/Helpers.php index 7d9926744..c10452c17 100644 --- a/app/Util/ActivityPub/Helpers.php +++ b/app/Util/ActivityPub/Helpers.php @@ -25,6 +25,7 @@ use App\Services\SanitizeService; use App\Services\UserFilterService; use App\Util\Media\License; use Carbon\Carbon; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\Rule; @@ -1141,7 +1142,9 @@ class Helpers } $mediaModel = self::createMediaAttachment($media, $status, $key); - self::handleMediaStorage($mediaModel); + if ($mediaModel) { + self::handleMediaStorage($mediaModel); + } } $status->viewType(); @@ -1170,16 +1173,35 @@ class Helpers } /** - * Create media attachment record + * Create media attachment record. + * + * Idempotent on the (status_id, media_path) unique key: if a row already + * exists (e.g. a re-fetch, an Announce racing another inbox job, or a + * duplicate url within one activity's attachments) the existing row is + * returned instead of triggering a duplicate-key violation. + * + * @return Media|null the newly created model, or null when the attachment + * already existed (so the caller can skip re-storage) */ - public static function createMediaAttachment(array $media, Status $status, int $key): Media + public static function createMediaAttachment(array $media, Status $status, int $key): ?Media { + // Fast path: already imported for this status. + if (Media::whereStatusId($status->id)->whereMediaPath($media['url'])->exists()) { + return null; + } + $mediaModel = new Media; self::setBasicMediaAttributes($mediaModel, $media, $status, $key); self::setOptionalMediaAttributes($mediaModel, $media); - $mediaModel->save(); + try { + $mediaModel->save(); + } catch (UniqueConstraintViolationException $e) { + // Lost a race with a concurrent inbox job that inserted the same + // (status_id, media_path). Treat as already-imported. + return null; + } return $mediaModel; } diff --git a/tests/Feature/Federation/DuplicateMediaAttachmentTest.php b/tests/Feature/Federation/DuplicateMediaAttachmentTest.php new file mode 100644 index 000000000..a4d76f872 --- /dev/null +++ b/tests/Feature/Federation/DuplicateMediaAttachmentTest.php @@ -0,0 +1,90 @@ + 'Document', + 'mediaType' => 'image/jpeg', + 'url' => $url, + 'name' => 'alt text', + 'blurhash' => 'UREVf}R:E2WB~qNKWBs.XURkxZofD+n~oJR-', + 'width' => 768, + 'height' => 1024, + ]; +} + +it('does not create a duplicate media row for the same status and url', function () { + $user = User::factory()->create(); + $user->refresh(); + $status = Status::factory()->create(['profile_id' => $user->profile->id, 'type' => 'photo']); + + $url = 'https://files.mastodon.social/media_attachments/files/117/original/e9ab6f0314043b12.jpeg'; + $payload = attachmentPayload($url); + + $first = Helpers::createMediaAttachment($payload, $status, 0); + // Second call (simulating re-import / race) must not throw and must be a no-op. + $second = Helpers::createMediaAttachment($payload, $status, 0); + + expect($first)->not->toBeNull(); + expect($second)->toBeNull(); + expect(Media::whereStatusId($status->id)->whereMediaPath($url)->count())->toBe(1); +}); + +it('creates distinct rows for different urls on the same status', function () { + $user = User::factory()->create(); + $user->refresh(); + $status = Status::factory()->create(['profile_id' => $user->profile->id, 'type' => 'photo:album']); + + $a = Helpers::createMediaAttachment(attachmentPayload('https://files.mastodon.social/a.jpeg'), $status, 0); + $b = Helpers::createMediaAttachment(attachmentPayload('https://files.mastodon.social/b.jpeg'), $status, 1); + + expect($a)->not->toBeNull(); + expect($b)->not->toBeNull(); + expect(Media::whereStatusId($status->id)->count())->toBe(2); +}); + +it('returns null when the row was inserted concurrently after the existence check', function () { + $user = User::factory()->create(); + $user->refresh(); + $status = Status::factory()->create(['profile_id' => $user->profile->id, 'type' => 'photo']); + + $url = 'https://files.mastodon.social/race.jpeg'; + + // Pre-insert the row to simulate the concurrent winner. + Media::create([ + 'remote_media' => true, + 'status_id' => $status->id, + 'profile_id' => $status->profile_id, + 'media_path' => $url, + 'remote_url' => $url, + 'mime' => 'image/jpeg', + 'version' => 3, + 'order' => 1, + ]); + + // Should detect the existing row and return null without throwing. + $result = Helpers::createMediaAttachment(attachmentPayload($url), $status, 0); + + expect($result)->toBeNull(); + expect(Media::whereStatusId($status->id)->whereMediaPath($url)->count())->toBe(1); +});