diff --git a/app/Http/Controllers/DirectMessageController.php b/app/Http/Controllers/DirectMessageController.php index 22b0a2cfb..3d761c553 100644 --- a/app/Http/Controllers/DirectMessageController.php +++ b/app/Http/Controllers/DirectMessageController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers; use App\Jobs\DirectPipeline\DirectDeletePipeline; use App\Jobs\DirectPipeline\DirectDeliverPipeline; +use App\Jobs\MediaPipeline\MediaDeletePipeline; use App\Jobs\StatusPipeline\StatusDelete; use App\Models\Conversation; use App\Models\DirectMessage; @@ -373,6 +374,21 @@ class DirectMessageController extends Controller StatusDelete::dispatch($status)->onQueue('high'); } + // Clean up the DM's media regardless of recipient locality. The remote + // branch only federates a Delete activity, and the local branch's async + // StatusDelete races the forceDeleteQuietly() below, so neither reliably + // reaches MediaDeletePipeline. Detach first (status_id has no FK/cascade) + // so the delete job's orphan guard passes, then dispatch the purge which + // removes the file and refunds the owner's storage quota. + $dmMedia = Media::whereStatusId($status->id)->get(); + if ($dmMedia->isNotEmpty()) { + Media::whereStatusId($status->id)->update(['status_id' => null]); + $dmMedia->each(function ($m) { + $m->status_id = null; + MediaDeletePipeline::dispatch($m)->onQueue('mmo'); + }); + } + if (Conversation::whereStatusId($sid)->count()) { $latest = DirectMessage::where(['from_id' => $dm->from_id, 'to_id' => $dm->to_id]) ->orWhere(['to_id' => $dm->from_id, 'from_id' => $dm->to_id]) diff --git a/tests/Feature/Api/DirectMessageMediaDeleteTest.php b/tests/Feature/Api/DirectMessageMediaDeleteTest.php new file mode 100644 index 000000000..789a9068e --- /dev/null +++ b/tests/Feature/Api/DirectMessageMediaDeleteTest.php @@ -0,0 +1,112 @@ +create([ + 'profile_id' => $sender->profile_id, + 'type' => 'photo', + 'scope' => 'direct', + 'visibility' => 'direct', + ]); + + $media = Media::create([ + 'status_id' => $status->id, + 'profile_id' => $sender->profile_id, + 'user_id' => $sender->id, + 'media_path' => 'public/m/_v2/1/dm-'.$status->id.'.jpeg', + 'mime' => 'image/jpeg', + 'size' => 250000, + 'order' => 1, + ]); + + $dm = new DirectMessage; + $dm->to_id = $recipientProfileId; + $dm->from_id = $sender->profile_id; + $dm->status_id = $status->id; + $dm->type = 'photo'; + $dm->save(); + + return $media; +} + +it('cleans up DM media when the recipient is local', function () { + Bus::fake(); + + $sender = User::factory()->create(); + $sender->refresh(); + $recipient = User::factory()->create(); + $recipient->refresh(); + + $media = makeDmPhoto($sender, $recipient->profile_id); + $statusId = $media->status_id; + + Passport::actingAs($sender, ['write']); + + $this->deleteJson('/api/v1.1/direct/thread/message', ['id' => $statusId]) + ->assertOk(); + + // The media is detached from the (hard-deleted) status and queued for purge. + expect(Media::whereId($media->id)->first()->status_id)->toBeNull(); + Bus::assertDispatched(MediaDeletePipeline::class, function ($job) use ($media) { + return $job->uniqueId() === 'media:purge-job:id-'.$media->id; + }); + + expect(Status::whereId($statusId)->exists())->toBeFalse(); +}); + +it('cleans up DM media when the recipient is remote', function () { + Bus::fake(); + + $sender = User::factory()->create(); + $sender->refresh(); + + // Remote recipient: user_id null + domain set -> AccountService local=false. + $remote = Profile::factory()->remote()->create(); + + $media = makeDmPhoto($sender, $remote->id); + $statusId = $media->status_id; + + Passport::actingAs($sender, ['write']); + + $this->deleteJson('/api/v1.1/direct/thread/message', ['id' => $statusId]) + ->assertOk(); + + // Regression: the remote branch used to only federate a Delete and never + // reach MediaDeletePipeline, orphaning the media row and leaking quota. + expect(Media::whereId($media->id)->first()->status_id)->toBeNull(); + Bus::assertDispatched(MediaDeletePipeline::class, function ($job) use ($media) { + return $job->uniqueId() === 'media:purge-job:id-'.$media->id; + }); + + expect(Status::whereId($statusId)->exists())->toBeFalse(); +});