The weekly blob prune only sweeps the per-account trees (#228)

app:prune:blobs deleted week-old profile pictures, signature images and backgrounds under var/uploads because no row named them. The orphan sweep now only enters numeric account directories and only files in each writer's exact layout, so everything else on those volumes is left alone, including directories added later. Closes #214.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -8,6 +8,16 @@ The published image tags: `latest` follows the most recent release below,
 
 ## Unreleased
 
+**The weekly cleanup no longer deletes profile pictures, signature images and backgrounds.** Every
+Sunday at 05:00 `app:prune:blobs` removed week-old files it found no record for anywhere under
+`var/uploads`, `var/attachments` and `var/raw`. Profile pictures, signature images and appearance
+background images are kept under `var/uploads` without such a record, so they were deleted along
+with the real leftovers. The cleanup now only looks in the per-account directories that mail
+attachments, original messages and staged uploads are written to, and leaves everything else on
+those volumes alone, including directories added later. Files it already deleted cannot be brought
+back by plMail: restore them from a backup of the volume, or upload them again. No schema change.
+[#214](https://github.com/karatektus/pl_mail/issues/214).
+
 **Reconnecting a service in the setup wizard no longer fails.** Opening the Integrations step for
 a service that was already connected and saving it again answered with an error page, because the
 step built a second connection with the same name instead of editing the first. It now edits the

CONTRIBUTING.md

@@ -929,7 +929,7 @@ php bin/mirror-wiki.php --check
 | `app:gmail:quota-probe [account] [--mode=all] [--count=30]` | Without an account, list the Gmail accounts and ask which. Fetch a fixed number of stored Gmail messages one way per minute (batched as an import does, or singly in each format), holding the account's import meanwhile, so the cost Google charges for each can be read off the Cloud console's quota chart. Stores and changes nothing |
 | `app:gmail:skipped [account] [--list=N] [--forget]` | Show the Gmail messages an import looked at and did not store. Without an account, one line per Gmail account: stored, skipped, Gmail's own total. With one, the skipped messages grouped by kind (chat, draft, sent from another address, delivered to another address, unfetchable), by Gmail label and by address, and with `--list` the messages themselves. Reads only what was recorded when each message was looked at; asks Gmail nothing. `--forget` deletes the record for the account so the next listing asks for those messages again |
 | `app:attachments:reclassify` | Recompute inline/attachment classification for stored parts |
-| `app:prune:blobs [--days=N] [--dry-run]` | Expire staged JMAP uploads and delete files orphaned by deleted rows |
+| `app:prune:blobs [--days=N] [--dry-run]` | Expire staged JMAP uploads and delete files orphaned by deleted rows. Only looks in the per-account directories of `var/uploads`, `var/attachments` and `var/raw`; everything else on those volumes belongs to someone else |
 | `app:user:promote <email> [--revoke]` | Grant or revoke `ROLE_ADMIN` |
 | `app:user:2fa-disable <email> [--force]` | Turn off 2FA for someone locked out — see "Two-factor authentication" |
 | `app:user:password <email> [--password=…]` | Set a new password for a user who can no longer sign in. Prompted for, hidden and twice, unless `--password` is given. Console-only for the same reason as `app:user:2fa-disable`: an administrator who could do it from a browser would be a second way into that person's mailbox |

docs/de/internals/architecture.md

@@ -1,4 +1,4 @@
-<!-- translated-from: internals/architecture.md sha1:9a71407f7557ff10c7e390efae4605835733b595 -->
+<!-- translated-from: internals/architecture.md sha1:ec72660311a2cdee4cc506e88e54e70e89f571c7 -->
 # Architektur
 
 Die Schichten, was wo liegt, und die Regeln, die das so halten. Diese Seite beschreibt die
@@ -237,7 +237,7 @@ war, in dem das Projekt vorher steckte: Logs und verwaiste Blobs wuchsen ohne Gr
 | `50 3 * * *` | `app:calendar:materialise` | Rollt den Horizont der Termininstanzen vorwärts, damit einem Serientermin nicht klammheimlich die Termine ausgehen |
 | `30 4 * * *` | `app:monitoring:prune` | Log-Einträge, Push-Zustellungen, tote Heartbeats, fertige Hintergrundjobs (30 Tage) und abgelaufene vertraute Geräte |
 | `37 * * * *` | `app:updates:check` | Stündlich, zu einer krummen Minute: Die Registry und GitHubs anonyme API teilen sich alle Installationen. Jeder neue Build wird einmal angekündigt, wie oft er auch gefunden wird |
-| `0 5 * * 0` | `app:prune:blobs` | Wöchentlich; er läuft drei Verzeichnisbäume ab, und eine Woche Waisen ist ein Rundungsfehler |
+| `0 5 * * 0` | `app:prune:blobs` | Wöchentlich; er läuft die Verzeichnisse pro Konto in drei Ablagen ab, und eine Woche Waisen ist ein Rundungsfehler. Nur diese: Profilbilder, Signaturbilder, Hintergründe und alles andere auf denselben Volumes bleiben unangetastet |
 
 Der Zeitplan ist `stateful()` gegen den Cache und `processOnlyLastMissedRun(true)`. Ein Worker,
 der über einen geplanten Lauf hinweg unten war, holt also nach, statt den Tag stillschweigend

docs/internals/architecture.md

@@ -219,7 +219,7 @@ in before, with logs and orphaned blobs growing without bound.
 | `50 3 * * *` | `app:calendar:materialise` | Rolls the occurrence horizon forward, so a repeating event does not quietly run out of dates |
 | `30 4 * * *` | `app:monitoring:prune` | Log entries, push deliveries, dead heartbeats, finished background jobs (30 days) and expired trusted devices |
 | `37 * * * *` | `app:updates:check` | Hourly, at an odd minute: the registry and GitHub's anonymous API are shared with everybody else's installations. Each new build is announced once, however often it is found |
-| `0 5 * * 0` | `app:prune:blobs` | Weekly; it walks three directory trees and a week of orphans is a rounding error |
+| `0 5 * * 0` | `app:prune:blobs` | Weekly; it walks the per-account trees of three stores and a week of orphans is a rounding error. Only those trees: avatars, signature images, backgrounds and anything else kept under the same volumes are left alone |
 
 The schedule is `stateful()` against the cache and `processOnlyLastMissedRun(true)`, so a
 worker that was down over a scheduled run catches up rather than silently skipping the day —

src/Command/Maintenance/PruneBlobsCommand.php

@@ -39,13 +39,29 @@ use Symfony\Component\Console\Style\SymfonyStyle;
  *   1. Expired staging uploads — rows (and files) for blobs a client uploaded
  *      but never attached to anything. RFC 8620 §6.1 lets the server expire
  *      these. Row-driven.
- *   2. Orphaned files — files under the three roots with no row pointing at
+ *   2. Orphaned files — files in the three stores with no row pointing at
  *      them. Filesystem-driven.
  *
  * Both respect the same retention window, and the window is what makes the
  * second pass safe: every writer creates the file before committing the row,
  * so a file younger than the cutoff may simply be waiting for its INSERT.
  * A week of slack is many orders of magnitude more than that gap.
+ *
+ * The second pass only looks where the three writers write: the per-account
+ * trees, var/uploads/<account>/, var/attachments/<account>/ and
+ * var/raw/<account>/, and in them only files laid out the way the writer lays
+ * them out. The roots are shared volumes, and other owners keep files there
+ * that no column of ours names: profile pictures (uploads/avatars), signature
+ * images (uploads/signatures), appearance backgrounds (uploads/backgrounds),
+ * attachment thumbnails (attachments/thumbs). Until #214 the sweep walked the
+ * whole root, took those for orphans and deleted them every Sunday.
+ *
+ * The road not taken is a deny-list of those directories. It would have been
+ * a smaller diff, and it fails the same way the original did: the next owner
+ * who adds a directory under a root (sealed messages under raw/ are one) is
+ * swept until somebody remembers to extend the list. Here the default for a
+ * path nobody claimed is to leave it alone; a leaked file costs disk, a swept
+ * one costs somebody's data.
  */
 #[AsCommand(
     name: 'app:prune:blobs',
@@ -56,15 +72,28 @@ final class PruneBlobsCommand extends Command
     private const int DEFAULT_DAYS = 7;
 
     /**
-     * The roots this command sweeps. Which column references files in each is
+     * The roots this command sweeps, each with the shape of a path its writer
+     * produces, relative to the root. Which column references files in each is
      * the repositories' business — see referencedPaths().
      *
-     * @var list<string>
+     * The first segment is always the account id, so a directory under a root
+     * whose name is not a number is not ours and is never entered. The rest
+     * mirrors the writer exactly:
+     *
+     *   uploads      <account>/<32 hex>.blob              UploadStorage
+     *   attachments  <account>/<mailbox>/<uid>/<name>     AttachmentStorageHelper
+     *   raw          <account>/<id div 1000>/<id>.eml     RawMessageStorage
+     *
+     * A file inside an account tree that does not match is left alone too:
+     * if a writer ever changes its layout, the pattern here has to follow
+     * before anything in the new shape can be swept, not after.
+     *
+     * @var array<string, non-empty-string>
      */
     private const array STORES = [
-        'var/uploads',
-        'var/attachments',
-        'var/raw',
+        'var/uploads'     => '#^\d+/[0-9a-f]{32}\.blob$#',
+        'var/attachments' => '#^\d+/\d+/\d+/[^/]+$#',
+        'var/raw'         => '#^\d+/\d+/\d+\.eml$#',
     ];
 
     public function __construct(
@@ -116,8 +145,8 @@ final class PruneBlobsCommand extends Command
         }
 
         if (false === $input->getOption('skip-orphans')) {
-            foreach (self::STORES as $root) {
-                [$count, $size] = $this->sweepOrphans($io, $root, $cutoff, $dryRun);
+            foreach (self::STORES as $root => $layout) {
+                [$count, $size] = $this->sweepOrphans($io, $root, $layout, $cutoff, $dryRun);
                 $files += $count;
                 $bytes += $size;
             }
@@ -173,11 +202,13 @@ final class PruneBlobsCommand extends Command
     }
 
     /**
-     * Pass 2: files under $root that no row points at any more.
+     * Pass 2: files in $root's account trees that no row points at any more.
+     *
+     * @param non-empty-string $layout
      *
      * @return array{int, int} files, bytes
      */
-    private function sweepOrphans(SymfonyStyle $io, string $root, \DateTimeImmutable $cutoff, bool $dryRun): array
+    private function sweepOrphans(SymfonyStyle $io, string $root, string $layout, \DateTimeImmutable $cutoff, bool $dryRun): array
     {
         $absoluteRoot = $this->projectDir.'/'.$root;
 
@@ -192,29 +223,39 @@ final class PruneBlobsCommand extends Command
         $files = 0;
         $bytes = 0;
 
-        foreach ($this->walk($absoluteRoot) as $file) {
-            // A file younger than the cutoff may be one whose row has not been
-            // committed yet. Leave it for the next run.
-            if ($file->getMTime() >= $cutoff->getTimestamp()) {
-                continue;
-            }
+        $accounts = $this->accountDirectories($absoluteRoot);
 
-            $path = $root.'/'.$this->relativeTo($absoluteRoot, $file);
+        foreach ($accounts as $account) {
+            foreach ($this->walk($account) as $file) {
+                $relative = $this->relativeTo($absoluteRoot, $file);
 
-            if (true === isset($referenced[$path])) {
-                continue;
-            }
+                if (1 !== preg_match($layout, $relative)) {
+                    continue;
+                }
 
-            ++$files;
-            $bytes += $file->getSize();
+                // A file younger than the cutoff may be one whose row has not
+                // been committed yet. Leave it for the next run.
+                if ($file->getMTime() >= $cutoff->getTimestamp()) {
+                    continue;
+                }
 
-            if (false === $dryRun) {
-                @unlink($file->getPathname());
+                if (true === isset($referenced[$root.'/'.$relative])) {
+                    continue;
+                }
+
+                ++$files;
+                $bytes += $file->getSize();
+
+                if (false === $dryRun) {
+                    @unlink($file->getPathname());
+                }
             }
         }
 
         if (false === $dryRun && $files > 0) {
-            $this->removeEmptyDirectories($absoluteRoot);
+            foreach ($accounts as $account) {
+                $this->removeEmptyDirectories($account);
+            }
         }
 
         $io->text(sprintf('Orphans in %s: %d (%s).', $root, $files, $this->humanBytes($bytes)));
@@ -246,6 +287,30 @@ final class PruneBlobsCommand extends Command
         };
     }
 
+    /**
+     * The per-account directories directly under a root: the ones named by a
+     * number. Everything else at that level belongs to someone else.
+     *
+     * @return list<string> absolute paths
+     */
+    private function accountDirectories(string $root): array
+    {
+        $accounts = [];
+
+        foreach (new \FilesystemIterator($root) as $entry) {
+            if (
+                $entry instanceof \SplFileInfo
+                && true === $entry->isDir()
+                && false === $entry->isLink()
+                && 1 === preg_match('/^\d+$/', $entry->getFilename())
+            ) {
+                $accounts[] = $entry->getPathname();
+            }
+        }
+
+        return $accounts;
+    }
+
     /**
      * @return iterable<\SplFileInfo>
      */
@@ -272,6 +337,10 @@ final class PruneBlobsCommand extends Command
      * Attachments fan out one directory per message and raw messages one per
      * thousand ids, so a sweep leaves behind a tree of empty directories that
      * would otherwise never be collected.
+     *
+     * Called per account tree, never on a root, so another owner's directory
+     * that happens to be empty right now is not removed from under it. The
+     * account directory itself goes once it is empty; the writers recreate it.
      */
     private function removeEmptyDirectories(string $root): void
     {
@@ -285,6 +354,8 @@ final class PruneBlobsCommand extends Command
                 @rmdir($entry->getPathname());
             }
         }
+
+        @rmdir($root);
     }
 
     private function humanBytes(int $bytes): string

tests/Command/Maintenance/PruneBlobsCommandTest.php

@@ -0,0 +1,188 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Command\Maintenance;
+
+use App\Command\Maintenance\PruneBlobsCommand;
+use App\Domain\Helper\UploadStorage;
+use App\Repository\Mail\MessagePartRepository;
+use App\Repository\Mail\MessageRepository;
+use App\Repository\Mail\UploadedBlobRepository;
+use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Tester\CommandTester;
+use Symfony\Component\Filesystem\Filesystem;
+
+/**
+ * The orphan sweep against a project directory of its own, so the files it is
+ * allowed to delete and the ones it must not can both be laid out exactly.
+ *
+ * #214: the sweep used to walk all of var/uploads, var/attachments and var/raw
+ * and delete every week-old file no column named. Profile pictures, signature
+ * images and appearance backgrounds live under var/uploads with no column of
+ * the uploads store pointing at them, and went every Sunday. These tests hold
+ * those files, and a directory nobody has written yet, across a run, and check
+ * that the sweep still does its job in the account trees.
+ *
+ * The repositories are stubbed: what each column holds is their business and
+ * is covered where they are; what is under test here is which files the
+ * command even considers.
+ */
+final class PruneBlobsCommandTest extends TestCase
+{
+    private const string OLD = '-30 days';
+
+    private string $projectDir;
+
+    protected function setUp(): void
+    {
+        $this->projectDir = sys_get_temp_dir().'/prune-blobs-'.bin2hex(random_bytes(6));
+        mkdir($this->projectDir);
+    }
+
+    protected function tearDown(): void
+    {
+        new Filesystem()->remove($this->projectDir);
+    }
+
+    public function testFilesOtherOwnersKeepUnderTheRootsSurviveARun(): void
+    {
+        $others = [
+            'var/uploads/avatars/3/3f9c1e.png',
+            'var/uploads/signatures/3/logo-a1b2c3.png',
+            'var/uploads/backgrounds/7d2e.jpg',
+            'var/attachments/thumbs/42.webp',
+            // A store nobody has written yet, in the shape the sealed messages
+            // of v0.4.0 take: it is left alone without anyone listing it here.
+            'var/raw/sealed/3/0/17.bin',
+            'var/uploads/someday/3/'.str_repeat('a', 32).'.blob',
+            // Loose files at the root and inside a path that only looks like
+            // an account tree further down are not ours either.
+            'var/uploads/README',
+            'var/raw/backup/3/0/1.eml',
+        ];
+
+        foreach ($others as $path) {
+            $this->file($path, self::OLD);
+        }
+
+        $exit = $this->command()->execute([]);
+
+        self::assertSame(Command::SUCCESS, $exit);
+
+        foreach ($others as $path) {
+            self::assertFileExists($this->projectDir.'/'.$path, $path.' is not one of the sweep\'s files.');
+        }
+    }
+
+    public function testOrphansInTheAccountTreesAreStillDeleted(): void
+    {
+        $upload     = 'var/uploads/3/'.str_repeat('b', 32).'.blob';
+        $attachment = 'var/attachments/3/12/4711/invoice.pdf';
+        $raw        = 'var/raw/3/1/1042.eml';
+
+        foreach ([$upload, $attachment, $raw] as $path) {
+            $this->file($path, self::OLD);
+        }
+
+        $this->command()->execute([]);
+
+        self::assertFileDoesNotExist($this->projectDir.'/'.$upload);
+        self::assertFileDoesNotExist($this->projectDir.'/'.$attachment);
+        self::assertFileDoesNotExist($this->projectDir.'/'.$raw);
+
+        // The emptied fan-out directories go with them, account included.
+        self::assertDirectoryDoesNotExist($this->projectDir.'/var/attachments/3');
+        self::assertDirectoryDoesNotExist($this->projectDir.'/var/raw/3');
+    }
+
+    public function testReferencedAndRecentFilesInTheAccountTreesAreKept(): void
+    {
+        $referencedUpload = 'var/uploads/3/'.str_repeat('c', 32).'.blob';
+        $referencedPart   = 'var/attachments/3/12/4711/kept.pdf';
+        $referencedRaw    = 'var/raw/3/0/7.eml';
+        $recentOrphan     = 'var/raw/3/0/8.eml';
+
+        foreach ([$referencedUpload, $referencedPart, $referencedRaw] as $path) {
+            $this->file($path, self::OLD);
+        }
+
+        $this->file($recentOrphan, '-1 day');
+
+        $this->command(
+            uploads: [$referencedUpload => true],
+            parts: [$referencedPart => true],
+            raw: [$referencedRaw => true],
+        )->execute([]);
+
+        self::assertFileExists($this->projectDir.'/'.$referencedUpload);
+        self::assertFileExists($this->projectDir.'/'.$referencedPart);
+        self::assertFileExists($this->projectDir.'/'.$referencedRaw);
+        self::assertFileExists($this->projectDir.'/'.$recentOrphan, 'Inside the window, its row may not be committed yet.');
+    }
+
+    public function testADirectoryOfAnotherOwnerThatIsEmptyIsNotRemoved(): void
+    {
+        // An orphan elsewhere makes the run tidy up empty directories; the
+        // owner of this one may be about to write into it.
+        mkdir($this->projectDir.'/var/uploads/avatars/3', 0755, true);
+        $this->file('var/uploads/3/'.str_repeat('d', 32).'.blob', self::OLD);
+
+        $this->command()->execute([]);
+
+        self::assertDirectoryExists($this->projectDir.'/var/uploads/avatars/3');
+    }
+
+    public function testADryRunDeletesNothing(): void
+    {
+        $orphan = 'var/raw/3/1/1042.eml';
+        $this->file($orphan, self::OLD);
+
+        $tester = $this->command();
+        $tester->execute(['--dry-run' => true]);
+
+        self::assertFileExists($this->projectDir.'/'.$orphan);
+        self::assertStringContainsString('Orphans in var/raw: 1', $tester->getDisplay());
+    }
+
+    private function file(string $relativePath, string $age): void
+    {
+        $path = $this->projectDir.'/'.$relativePath;
+
+        if (false === is_dir(dirname($path))) {
+            mkdir(dirname($path), 0755, true);
+        }
+
+        file_put_contents($path, 'x');
+        touch($path, new \DateTimeImmutable($age)->getTimestamp());
+    }
+
+    /**
+     * @param array<string, true> $uploads
+     * @param array<string, true> $parts
+     * @param array<string, true> $raw
+     */
+    private function command(array $uploads = [], array $parts = [], array $raw = []): CommandTester
+    {
+        $blobs = $this->createStub(UploadedBlobRepository::class);
+        $blobs->method('findOlderThan')->willReturn([]);
+        $blobs->method('findReferencedPaths')->willReturn($uploads);
+
+        $messageParts = $this->createStub(MessagePartRepository::class);
+        $messageParts->method('findReferencedStoragePaths')->willReturn($parts);
+
+        $messages = $this->createStub(MessageRepository::class);
+        $messages->method('findReferencedRawPaths')->willReturn($raw);
+
+        return new CommandTester(new PruneBlobsCommand(
+            $blobs,
+            $messageParts,
+            $messages,
+            new UploadStorage($this->projectDir, 'var'),
+            $this->createStub(EntityManagerInterface::class),
+            $this->projectDir,
+        ));
+    }
+}