Add TODO.md; enhance media:maintenance with --server filter and state annotations

- TODO.md: capture follow-ups (centralized status media teardown, DM leak fix,
  no-DB-cascade rationale, remote-edit orphaning, media:gc re-check).
- media:maintenance: add --server remote|local|both (default both) to filter
  orphaned media by origin.
- Annotate each row with status state (live/soft-deleted/hard-deleted) next to
  status_id and profile state (live/soft-deleted/hard-deleted) next to
  profile_id, in both dry-run table and verbose run output. States are resolved
  in batched, trashed-aware queries.
pull/6991/head
Your Name 4 weeks ago
parent 48a1fe5e5e
commit 838a6b999c

@ -0,0 +1,53 @@
# TODO / Future Improvements
Running list of follow-up work identified while debugging the media lifecycle.
## Media / Status lifecycle
- [ ] **Centralize status media teardown.** The detach-then-dispatch media
cleanup is currently copy-pasted in `StatusDelete`, `RemoteStatusDelete`, and
`DeleteRemoteStatusPipeline`. Extract a single helper (e.g.
`MediaStorageService::deleteStatusMedia(Status $status)`) and call it from
every status-delete path so the logic can't drift out of sync.
- [ ] **Fix DirectMessageController media leak.** `DirectMessageController`
deletes DM statuses via `forceDeleteQuietly()`, which bypasses model events
AND never dispatches `MediaDeletePipeline`. DM media rows and their files
(local/S3/HLS) leak. Route it through the centralized helper above.
- [ ] **Do NOT use a DB FK `ON DELETE CASCADE` for media→status.** Reasons:
both models use `SoftDeletes` (soft delete is an UPDATE, cascade only fires on
hard DELETE), and a row-level cascade would orphan the actual stored files
(local disk, S3 objects, HLS `.m3u8`/`.ts` segments) since all file cleanup
lives in `MediaDeletePipeline`, not the DB. Keep teardown in application code.
- [ ] **Optional defense-in-depth:** a `Status::deleting` observer hook that
calls the centralized teardown, so future delete paths are covered
automatically. Note `forceDeleteQuietly()` callers skip events and must still
call the helper explicitly.
## StatusRemoteUpdatePipeline (remote edits)
- [ ] **Don't orphan media on remote edit when re-fetch fails.**
`StatusRemoteUpdatePipeline::updateMedia()` detaches all media
(`status_id = null`) then re-imports; if the new attachment HEAD requests 404
(origin deleted the media) nothing is re-attached and the rows are left
orphaned. Only detach media that is actually being replaced, or keep existing
media when the incoming set resolves to nothing.
## media:gc (GarbageCollectorMedia)
- [ ] **Re-check `status_id` immediately before dispatch.** `media:gc` selects a
batch with `->get()` then dispatches per row; a row can be re-attached between
select and dispatch. Re-read `status_id` right before dispatching.
- [ ] **Consider a `remote_media` guard / policy.** Federated media lifecycle is
owned by the remote instance; decide whether `media:gc` should treat it
differently from local orphan uploads.
## Diagnostics / tooling
- [ ] Roll back the temporary `MEDIA-TRACE:` debug logging
(branch `debug/media-lifecycle-trace-logs`) once diagnosis is complete.
- [ ] Consider additional `media:maintenance --scope` routines (e.g. missing
local files, stale cloud URLs, mime backfill).

@ -3,8 +3,12 @@
namespace App\Console\Commands\Media;
use App\Models\Media;
use App\Models\Profile;
use App\Models\Status;
use App\Services\MediaStorageService;
use Illuminate\Console\Command;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
class MediaMaintenance extends Command
@ -16,6 +20,7 @@ class MediaMaintenance extends Command
*/
protected $signature = 'media:maintenance
{--scope= : The maintenance routine to run. Supported: orphanedMedia}
{--server=both : Which media to target by origin: remote, local, or both}
{--limit=1000 : Max media rows to process this run}
{--dry-run : Report what would happen without detaching or deleting}
{--force : Skip confirmation prompts}';
@ -36,6 +41,13 @@ class MediaMaintenance extends Command
'orphanedMedia' => 'handleOrphanedMedia',
];
/**
* Supported --server values.
*
* @var array<int, string>
*/
protected array $servers = ['remote', 'local', 'both'];
public function handle(): int
{
$scope = $this->option('scope');
@ -52,6 +64,13 @@ class MediaMaintenance extends Command
return self::FAILURE;
}
$server = (string) $this->option('server');
if (! in_array($server, $this->servers, true)) {
$this->error('Invalid --server "'.$server.'". Supported: '.implode(', ', $this->servers).'.');
return self::FAILURE;
}
return $this->{$this->scopes[$scope]}();
}
@ -67,10 +86,13 @@ class MediaMaintenance extends Command
{
$limit = max(1, (int) $this->option('limit'));
$dryRun = (bool) $this->option('dry-run');
$server = (string) $this->option('server');
// Media whose status_id is set but has no matching live (non-trashed)
// status row.
// status row, filtered by origin (remote/local/both).
$query = Media::whereNotNull('status_id')
->when($server === 'remote', fn ($q) => $q->where('remote_media', true))
->when($server === 'local', fn ($q) => $q->where('remote_media', false))
->whereNotExists(function ($q) {
$q->select(DB::raw(1))
->from('statuses')
@ -81,37 +103,46 @@ class MediaMaintenance extends Command
$total = (clone $query)->count();
if ($total === 0) {
$this->info('No orphaned media found. Nothing to do.');
$this->info('No orphaned media found for --server='.$server.'. Nothing to do.');
return self::SUCCESS;
}
$media = $query->limit($limit)->get();
$this->info('Found '.$total.' orphaned media row(s); processing '.$media->count().' this run (limit '.$limit.').');
// Resolve status + profile state for the rows in this batch (batched,
// trashed-aware) so we can annotate live vs soft/hard-deleted.
$stateMap = $this->resolveStates($media);
$this->info('Found '.$total.' orphaned media row(s) [--server='.$server.']; processing '.$media->count().' this run (limit '.$limit.').');
if ($dryRun) {
$columns = $this->output->isVerbose()
? ['media_id', 'status_id', 'remote_media', 'profile_id', 'mime', 'size', 'created_at', 'media_path']
: ['media_id', 'status_id', 'remote_media', 'media_path'];
$verbose = $this->output->isVerbose();
$columns = $verbose
? ['media_id', 'status_id', 'status_state', 'profile_id', 'profile_state', 'remote_media', 'mime', 'size', 'created_at', 'media_path']
: ['media_id', 'status_id', 'status_state', 'profile_id', 'profile_state', 'remote_media', 'media_path'];
$this->table(
$columns,
$media->map(function ($m) {
$base = [
$media->map(function ($m) use ($stateMap, $verbose) {
$row = [
'media_id' => $m->id,
'status_id' => $m->status_id,
'status_state' => $stateMap['status'][$m->status_id] ?? 'unknown',
'profile_id' => $m->profile_id ?? 'null',
'profile_state' => $m->profile_id ? ($stateMap['profile'][$m->profile_id] ?? 'unknown') : 'n/a',
'remote_media' => (bool) $m->remote_media ? 'true' : 'false',
'profile_id' => $m->profile_id,
'mime' => $m->mime,
'size' => $m->size,
'created_at' => optional($m->created_at)->toDateTimeString(),
'media_path' => $m->media_path,
];
return $this->output->isVerbose()
? array_values($base)
: [$base['media_id'], $base['status_id'], $base['remote_media'], $base['media_path']];
if (! $verbose) {
unset($row['mime'], $row['size'], $row['created_at']);
}
return array_values($row);
})->all()
);
$this->comment('[dry-run] Would detach and delete the '.$media->count().' row(s) above.');
@ -142,13 +173,15 @@ class MediaMaintenance extends Command
if ($verbose) {
$this->line(sprintf(
' [%d/%d] detached + dispatched delete: media_id=%d status_id=%s remote_media=%s profile_id=%s mime=%s size=%s path=%s',
' [%d/%d] detached + dispatched delete: media_id=%d status_id=%s (%s) profile_id=%s (%s) remote_media=%s mime=%s size=%s path=%s',
$processed,
$media->count(),
$m->id,
$originalStatusId ?? 'null',
(bool) $m->remote_media ? 'true' : 'false',
$stateMap['status'][$originalStatusId] ?? 'unknown',
$m->profile_id ?? 'null',
$m->profile_id ? ($stateMap['profile'][$m->profile_id] ?? 'unknown') : 'n/a',
(bool) $m->remote_media ? 'true' : 'false',
$m->mime ?? 'null',
$m->size ?? 'null',
$m->media_path ?? 'null'
@ -170,4 +203,58 @@ class MediaMaintenance extends Command
return self::SUCCESS;
}
/**
* Resolve the lifecycle state of the statuses and profiles referenced by a
* batch of media rows.
*
* A referenced id can be:
* - live row exists and is not soft-deleted
* - soft-deleted row exists with deleted_at set
* - hard-deleted row does not exist at all (only meaningful for statuses)
*
* @param Collection<int, Media> $media
* @return array{status: array<int, string>, profile: array<int, string>}
*/
protected function resolveStates($media): array
{
$statusIds = $media->pluck('status_id')->filter()->unique()->values();
$profileIds = $media->pluck('profile_id')->filter()->unique()->values();
return [
'status' => $this->stateFor(Status::class, $statusIds),
'profile' => $this->stateFor(Profile::class, $profileIds),
];
}
/**
* Build an id => state map for a soft-deletable model over the given ids.
*
* @param class-string<Model> $model
* @param Collection<int, int|string> $ids
* @return array<int, string>
*/
protected function stateFor(string $model, $ids): array
{
if ($ids->isEmpty()) {
return [];
}
$rows = $model::withTrashed()
->whereIn('id', $ids->all())
->pluck('deleted_at', 'id');
$map = [];
foreach ($ids as $id) {
if (! $rows->has($id)) {
$map[$id] = 'hard-deleted';
} elseif ($rows->get($id) !== null) {
$map[$id] = 'soft-deleted';
} else {
$map[$id] = 'live';
}
}
return $map;
}
}

Loading…
Cancel
Save