intoduce new async_slow queue for image deletion

[blued-gear]

On some platforms the deletion of many images when a user gets deleted takes quite a while. To decouple the deletion from the transaction which removes the user, this PR adds a new queue and new message for this job.

[blued-gear]

I reworked the new code to lower the chance of race conditions between entity and file deletions.

Also, the tests now cover the MessageHandler.

[melroy89]

Found three correctness issues in the image-deletion flow.

[melroy89]

Image::filePath is nullable, and both message factories can therefore put a null value in this batch. When that value reaches ImageManagerInterface::remove(string), PHP throws a TypeError. The current catch (\Exception) does not catch it because TypeError implements Throwable, not Exception. The handler then fails and retries instead of continuing with the remaining images.

A simple guard keeps null paths away from the string-only API:

foreach ($filesToDelete as $path) {
    if (null === $path) {
        continue;
    }

    try {
        $this->imageManager->remove($path);
    } catch (\Exception $e) {
        // existing logging
    }
}

It would also help to stop adding null paths to $filesToDelete in the first place, but the removal boundary should still be safe because the incoming message may already contain null.

[melroy89]

is this done..?

[BentiGorlich]

It is because if the path is null, then the image is skipped now. If the image was not referenced the entity in the db got deleted earlier as well

I don't want to block PRs

[BentiGorlich]

It looks good to me. What I don't understand is why you're keeping the old message and the old handler. Either it is a v2 or it is a special handler for big batches, which the name should reflect

[BentiGorlich]

That is not correct... The handler has this at the bottom:

    if ($image?->filePath) {
        $this->imageManager->remove($image->filePath);
    }

[blued-gear]

What I don't understand is why you're keeping the old message and the old handler. Either it is a v2 or it is a special handler for big batches, which the name should reflect

It is a V2. I kept the old Message version because it is to be expected that queues are not empty when admins update their Mbin version. Then Symfony would not be able to deserialize the old messages, and we would lose data (data here means desired actions).
There are many more usages of the old version, which I would like to keep until the V2 has shown to be effective in prod.

[BentiGorlich]

CI is failing with this error:

Fatal error: Cannot apply #[\Deprecated] to class App\Message\DeleteImageMessage in /__w/mbin/mbin/src/Message/DeleteImageMessage.php on line 13