fix(preview): regenerate previews whose stored file is gone - #63486
fix(preview): regenerate previews whose stored file is gone#63486rayvincent2 wants to merge 2 commits into
Conversation
Sorry, something went wrong.
|
Follow-up to the live-test note in the description: I have now run a path-scoped preview:generate-all on production (34.0.3 + this patch, local preview storage). Context: I had already completed successful preview:generate-all runs over all images earlier (including repairs of missing preview files via this change). The runs below are not a cold full regenerate of missing previews — they are re-runs over a tree where previews already exist. “Cold” vs “warm” here only means first pass vs second/third pass with OS page/dentry cache for stat (and similar) warmed. docker exec -u www-data -e PHP_MEMORY_LIMIT=2G nextcloud-manual_install-nextcloud \
php -d memory_limit=2G /var/www/html/occ preview:generate-all -vv \
--path "/ray/files/Photos/My Photos/2026"Results for /ray/files/Photos/My Photos/2026:
So this is a smoke / regression check that preview:generate-all still walks the tree cleanly with the patch applied when previews are already present. The original bug (DB row present, preview file missing → hard fail on readPreview) was verified earlier with single-file preview:generate on known-broken rows. I still have not live-tested object-store primary storage. |
Sorry, something went wrong.
| 'store' => $store, | ||
| ] = $this->getObjectStoreInfoForExistingPreview($preview); | ||
|
|
||
| return $store->objectExists($urn); |
There was a problem hiding this comment.
this generates network requests, which is not great for performance. I wonder if for object store we could just always return true (at least for now until a better solution is found)
Sorry, something went wrong.
There was a problem hiding this comment.
Agreed. For object store I just made previewExists always return true for now so we don’t add a network hop per preview. Local storage keeps the real existence check (cheap stat). A proper bulk existence API for object store can be a follow-up if we want to repair missing objects the same way.
Sorry, something went wrong.
When a preview row exists in the database but the backing file is missing, preview:generate failed with NotFoundException from LocalPreviewStorage::readPreview. Drop the stale row and generate a new preview instead so occ preview:generate and preview:generate-all can repair as they run. Fixes: nextcloud#63349 Assisted-by: Grok:grok-4.6 Signed-off-by: Ray Vincent <rayhvincent@gmail.com>
ObjectExists() is a network round-trip per preview. Treat the DB row as authoritative for now so bulk listing stays cheap. Missing objects still fail later on read. Local storage keeps a real filesystem check. Assisted-by: Grok:grok-4.6 Signed-off-by: Ray Vincent <rayhvincent@gmail.com>
Reviewers
At least 2 approving reviews are required to merge this pull request.
Summary
If a preview database row exists but the stored file is gone, generation used that row and then failed when it tried to read the file. Generator now checks that the file is still present, deletes the stale row if it is not, and regenerates the preview. That covers a missing max preview and missing extra sizes when several sizes are configured for one file.
Unit tests cover local storage, object storage, and multiple sizes per file.
Live test: I applied this change on a Nextcloud 34.0.3 instance with local preview storage. occ preview:generate had been failing with NotFoundException from LocalPreviewStorage::readPreview (DB row for 1535-2048-max.jpg, file missing). After the change the same command succeeded. Several other images were in the same state. Opening them in Memories requested previews again and those thumbnails regenerated instead of staying broken.
I have not live-tested object store or a full preview:generate-all re-run.
Checklist
AI (if applicable)
I used Grok to implement the fix and tests; I reviewed the diff, ran the unit tests, and verified it on 34.0.3.