Repository navigation
Improve image processing reliability, caching and admin workflows - #142
fabiodalez-dev merged 12 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 129 files, which is 29 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
A digest-pinned php:8.5.12 base and a git-commit Imagick build break the security-rebuild path used for 1.4.27 (re-run the publish workflow with pull: true on php:8.5-apache-trixie): every PHP patch would need a PR. - Dockerfile: php:8.5-apache-trixie and the released imagick extension. - Version checks accept PHP 8.5 >= 8.5.12 (FPM test host: 8.2 >= 8.2.35) instead of an exact version that fails on the next patch release. - image-jobs.yml: same SHA-pinned Node 24 actions as the other workflows, push runs only for main, and it also runs the visitor watcher test.
MAX_ATTEMPTS only capped the backoff exponent, so a job that can never
succeed was retried hourly forever, the Docker worker booted bin/console
every 15 s for backed-off jobs, and every admin request (including the
progress poll) scheduled a drain.
- After MAX_ATTEMPTS failures a job is parked as {id}.failed and logged
as an error; enqueueing the image again starts it afresh.
- bin/image-worker.sh starts PHP only for jobs whose next_at has passed.
- AuthMiddleware drains only when a job is due, never from the
/admin/api/image-jobs poll.
- runSynchronously() waits at most 20 s for a running drain, then leaves
the job queued instead of blocking the upload request.
- Lock files of deleted images are pruned under the worker lock.
- images:generate regenerates everything again unless --missing is given (--missing had become a no-op); --force still wins. - GD fallbacks decode WebP and GIF originals, as the old command did. - JPEG-XL variant dimensions are capped at the original width. - The cjxl route (full-size Imagick decode + external process) honours CIMAISE_DISABLE_IMAGICK and runs only from the CLI; libvips+libjxl still works in any SAPI. - A blur batch skips an image held by the worker instead of aborting the whole album.
…nding Every open photo page polled /api/image-generation-revision every 30 s forever, each poll booting the full app. The layout now says whether any image job is queued: without one the page does not poll, otherwise it watches for at most 10 minutes and stops once the pill is shown. The early ETag is derived from the page-cache entry, not from the HTML, so it now includes the generation state; otherwise visitors kept a stale "pending" flag through 304 responses.
- Albums: drop the album.N.show_equipment settings write. The column is the source of truth; the row was written inside the album transaction (refreshing the shared settings cache before commit) and added one row per album to the settings hashed into every HTML validator. - TwigGlobalsCache: bump the prefix to v6 for site_logo_dark and add its default. - SQLite backup: keep text readable through quote(); only text holding a NUL byte is hex-encoded. - French: translate the 49 new admin keys and 6 frontend keys; a test now checks fr/it cover every English key. - .gitignore: whitelist the new test files. - CHANGELOG: document upload compression removal, job parking and the bounded visitor watcher.
|
Thanks a lot for this, Mirano. It is a big and careful piece of work, and the test coverage (Apache, PHP-FPM, restricted exec, SQLite contention, Chromium) made it much easier to review. I went through the whole diff, ran the suites and the Docker build, and pushed the fixes for everything I found directly to this branch so you don't have to do another round. Here is what I changed and why. Rebase on 1.4.27. I merged Docker base image no longer pinned. The Dockerfile used Failing image jobs were retried forever.
Synchronous uploads could hang behind a drain. With Per-image locks.
Visitors' version watcher. Every visitor with a photo page open polled
JPEG-XL in web requests. The cjxl route decodes the full original through Imagick and spawns Smaller things.
One behaviour change I kept, documented. Removing Tests. I added a regression test for each fix: queue parking, due-only wakeups, bounded synchronous wait, lock pruning, worker back-off, poll path, ETag, globals shape, GD WebP/GIF, the cjxl gate, JXL dimensions, the blur batch,
Splitting future changes of this size into a few PRs would help a lot, mostly so that CodeRabbit (capped at 100 files) can review them too. Thanks again! |
|
I will be pushing them in smaller chunks for now on! I'm just editing the Docker to get away from Adobe Portfolio haha, thank you for all your work! |
- Uploads: a file the server would reject (over the 40 MP pixel cap or
PHP's upload size) is downscaled in the browser instead of failing;
every other file is still sent byte for byte. 45-61 MP camera
originals were accepted before only because of the old compressor.
- Per-image lock: the CLI waits up to 120 s for the holder; web requests
get ImageProcessingBusy at once. An on-demand variant request no longer
records a failure marker while the worker holds the image, and a busy
image is postponed in the queue without counting an attempt.
- Lock files live in storage/image-jobs/locks, so the per-request job
listing no longer scans one file per image; orphan locks are pruned at
most once a day.
- A synchronous generation failure gets the same backoff and progress
state as a drain failure. A missing placeholder is logged, not retried:
the variants exist and the image is usable.
- SQLite busy_timeout is back to 30 s: transactions and direct PDO
statements are not covered by SqliteRetry.
- Upload widget and progress panel treat parked ("failed") and retrying
jobs as settled instead of spinning forever; new en/it/fr strings.
- iOS is matched on whole tokens ("Studios" is not iOS).
- Media library previews of public images are static files again;
protected previews are cached privately for 5 minutes.
- File page cache ETag uses the file's stat instead of hashing it, and
typography_version hashes the typography settings instead of
generating the stylesheet on every request.
|
Thanks a lot, Mirano! I'm really glad you're choosing Cimaise to move away from Adobe Portfolio, that genuinely made my day. If anything in the Docker setup gets in your way during the migration, just open an issue and I'll take a look. I merged this PR and it's out in 1.4.28, together with the new Docker image (
Smaller PRs from now on sound perfect. Thanks again for all the work! |
Uploads and administrative edits need reliable background processing and cache invalidation, while large libraries make the current navigation, polling and repeated filesystem checks costly. This consolidates the remaining tested fork improvements on top of current main, including the first performance batch. Existing upstream lifecycle tests are retained.
Changes
Compatibility
Existing settings and single-setting callers remain supported. No external cron job or changed volume mapping is required. This keeps upstream's version and release history; fork-only release metadata and upgrade-image dependencies are excluded.
Validation
Verified runs: integrated fork suite (280 tests, 1,291 assertions; 13 environment-dependent skips) and this contribution branch, including the published upstream Docker upgrade. Both full integration workflows passed. Upstream CI also runs on this PR. Live-site deployment is left to the maintainer.