fix(server): make cached-file storage accounting opt-in, defaulting to upstream semantics - #979
Merged
Merged
Conversation
github-actions
Bot
added
documentation
… add android motion test
…ream semantics Adds user.physicalUsageInBytes (originals + thumbnails + transcodes) and a migration that preserves the current quotaUsageInBytes value into it before resetting quotaUsageInBytes to upstream semantics (asset_exif.fileSizeInByte sum, external libraries excluded). Also updates the test factory for the new column and keeps revert-to-immich.sql / its coverage test in sync with the new fork migration and column.
Adds physicalUsageInBytes to UserAdminResponseDto and storageUsageIncludesDerivatives to ServerConfigDto so clients can pick which storage-usage number to show, without exposing the separate quota-enforcement toggle.
…ll physical usage
Polish used three different storage nouns (magazynu / no noun / przestrzeni dyskowej) across the panel's four switch strings; normalized to magazyn throughout, keeping kontyngent for the quota concept itself. Italian's display-switch description said spazio di archiviazione while its own title said utilizzo visualizzato; changed the description to echo utilizzo di archiviazione, matching the title and the panel's other keys.
The @GenerateSql decorator on AssetRepository.getExternalAssetIds emits a block that was never written to src/queries/asset.repository.sql, so the sql-schema-up-to-date job would fail on the diff.
Picks up ServerConfigDto.storageUsageIncludesDerivatives, SystemConfigDto.storageUsage, UserAdminResponseDto.physicalUsageInBytes and the new SystemConfigStorageUsageDto.
GET /server/config now returns the required storageUsageIncludesDerivatives, so the toEqual literal has to carry it. With both storage usage toggles off the exposed value is false.
…forcement Two independent toggles meant a supported configuration (includeDerivativesInDisplay=false, includeDerivativesInQuota=true) showed a user "10 GB of 20 GB" while rejecting their upload as over quota — the exact misleading number this work set out to remove. - server: expose storageUsageIncludesDerivatives as the OR of the two toggles, so opting quota in implies showing the enforced figure. - web: make the admin user page's storage meter follow the same server config flag instead of always reading quotaUsageInBytes, so an admin investigating a rejected upload sees the number enforcement used.
SystemConfigService.updateSystemConfig rejects outright while IMMICH_CONFIG_FILE is set, so those installs never emit ConfigUpdate and the only resync trigger never fired. An admin who set includeDerivativesInDisplay in a config file saw 0 B on a fresh install, or a drifted migration-time value on an existing one, until the nightly job ran. Resync on ConfigInit instead, gated on the install actually using a config file and on at least one toggle being on. Pinned to the microservices worker like onConfigUpdate, since UserSyncUsage carries no jobId. Also documents how the figure populates and how it stays fresh, including that nightlyTasks.syncQuotaUsage is what keeps it current.
The figure is only recalculated when a toggle is first turned on and then by the nightly Sync quota usage task, so an admin who disabled that task would see a permanently frozen number with nothing on screen saying why. Adds the hint in en plus the nine maintained locales.
… comments - Mark every storage-usage hunk sitting in an otherwise upstream file with a Gallery-fork comment, starting with the greatest(0, ...) line inside updateUsage — the one a rebase resolver would most easily drop. - Restore the circular-dependency note on the lazy StorageService import in base.service, matching the other three lazy-import sites in that file. - Name the actual syncUsage call sites instead of "below this method". - Drop the claim that physicalUsageInBytes is not in the userAdmin projection; it has been since database.ts added it.
The two toggles are no longer independent — enabling quota enforcement now switches the displayed figure too — so the admin panel copy for the quota switch and the two code comments that described the flag as display-only were incomplete. Updated in en plus the nine maintained locales.
…p collision #834 landed 1785000000000-AddFaceRepairLock, which shares a timestamp prefix with this branch's 1785000000000-AddPhysicalUsageInBytes. Two migrations on one timestamp is ambiguous and breaks the repo's unique-round-timestamp convention, so move ours to 1791000000000 — above main's highest (1790000000000). The migration has never merged, so no database has recorded the old name and no pre-rename alias entry is needed in the revert script.
The two independent opt-ins (display and quota) could disagree, which meant holding two numbers at once: a physicalUsageInBytes column, a migration, a field on AuthUser/UserAdmin and both user DTOs, a ServerConfigDto flag telling clients which number to render, and a matching read-site change everywhere storage is shown. One key, storageUsage.includeDerivatives, removes all of it. quotaUsageInBytes again holds whatever the admin chose: upstream's originals-only statement when off, the disk/S3 walk when on. Nothing downstream has to know which produced it, so the column, the migration, the DTO fields and the enforcement branch in requireQuota all revert to upstream. UserRepository keeps upstream syncUsage verbatim and updateUsage exactly as upstream has it; setUsage returns as the fork setter writing the same column. BaseService.syncUsage is now either/or - running upstream's statement alongside the walk would overwrite the walked figure with the originals-only one. The resync trigger fires on any change of the flag, not just off->on: switching off leaves a derivative-inclusive figure that also needs recomputing.
Adapts the handleUserSyncUsage block to setUsage and one flag, keeping the S3 prefix walk and the external-library exclusion, and pins the either/or: with the toggle on, upstream's originals-only statement must not also run. The storage-usage service spec now covers on->off queuing alongside off->on. Tests that only asserted removed behaviour - quota enforcement picking a column, the ServerConfigDto flag, the physicalUsageInBytes column shape - are deleted rather than left asserting nothing. The medium spec is renamed to match what it still covers.
…itch With quotaUsageInBytes always holding the figure the admin asked for, no client needs to know the setting exists. StorageSpace, rail-storage and the admin user page go back to reading that column directly, which retires the shared derivation helper, the serverConfigManager.valueOrUndefined accessor and the rail-storage tests covering the removed branch. rail-storage keeps its original "Duplicated from StorageSpace.svelte" comment and parity block. The settings panel keeps its refresh hint and drops to a single switch.
Replaces the separate display and quota switch strings with a single pair describing what the one switch now does - it moves the shown figure and the enforced figure together - and rewords the refresh hint, which said "when you first turn one of these on" and now has to cover turning it off as well. en plus the nine maintained locales.
Only SystemConfigStorageUsageDto survives, with one field. ServerConfigDto and UserAdminResponseDto return to upstream: no client reads a second usage number or a flag selecting between them. Regenerated from a built server (sync-open-api, oazapfts, generate-dart-sdk), not hand-edited.
config-file.md documents storageUsage.includeDerivatives and drops the two-flag interaction that no longer exists. The ownership manifest keeps the paths this change still touches and drops the ones that reverted to upstream, along with the migration glob for the deleted column migration. The plan doc described a two-column design the code no longer implements, so it is rewritten as a design record of what shipped, including why the two-toggle draft was abandoned.
Deeds67
force-pushed
the
fix/storage-usage-opt-in-cached
branch
from
August 12, 2026 18:35
a9ad580 to
3502c1d
Compare
CI's Test & Lint Server job runs `prettier --check .` first, and a misformatted storageUsage config-file test was failing it before any test ran.
…he refresh-hint clause The derivatives-setting description named upstream Immich by product name, which the branding pipeline has no override for — substituting "Noodle Gallery" there would turn "matches upstream Immich" into the false claim "matches upstream Noodle Gallery". Reworded to reference the upstream default without naming the product, in en and all nine translated locales. The refresh-hint string also overstated what stops when the nightly sync task is disabled: UserService.onAssetCreate keeps applying live original-size deltas regardless, so only the reconciliation of derivative bytes actually stops. Reworded the same nine locales.
…sign
Both comments still described the abandoned two-toggle draft ("when
the toggles are enabled", "opt-ins for counting..."). There is only
one admin toggle now.
src/gallery/** now holds this PR's core storage-usage logic but was missing from the coverage include list alongside cores/services/utils/ sql-tools, so it never showed up in coverage output.
getExternalAssetIds returns Postgres-lowercased uuids, but getDerivativeAssetId preserved the filename's own casing, so an uppercase-cased filename would fail the set-membership check and get counted as non-external. Unreachable today since filenames are built from asset.id (already lowercase), but the test was locking in the mismatch by asserting the uppercase value came back. Normalise the extraction instead and flip the test to match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts user storage usage to upstream Immich semantics by default, and puts the fork's derivative-inclusive accounting behind one admin opt-in.
Since #492 the fork counted thumbnails and transcoded videos in a user's storage usage. That measures real disk consumption, but it makes the number shown to a user larger than the media they actually uploaded, which several people found misleading. This keeps both behaviours available and makes upstream's the default.
Behaviour
Admin → System Settings → Storage usage → "Include cached files in storage usage", default off. Also settable from a config file as
storageUsage.includeDerivatives.The setting governs both what users are shown and what their quota is enforced against, so the displayed figure can never contradict the one that rejects an upload.
How it works
quotaUsageInByteskeeps holding whichever number the setting implies — the same approach #492 took, so there is no new column and no database migration.syncUsageSQL runs, one statement, originals only, external-library assets excluded. The nightly job costs exactly what upstream's does.library/,upload/,profile/,thumbs/andencoded-video/and writes the total into the same column.Nothing downstream needs to know about the setting — enforcement, the sidebar meter and the admin user page all just read the column, exactly as upstream does.
asset-media.service.ts,StorageSpace.svelte,database.ts, the user and server DTOs and the generated clients are all untouched by this PR.Toggling the setting either direction queues an immediate resync, so the figure is correct without waiting for the nightly job. Config-file installs never emit a config-update event, so they get the same resync at boot.
External libraries
Derivatives belonging to external-library assets are no longer counted. Upstream excludes external assets from quota entirely, but thumbnail and transcode files are laid out by owner id with no library dimension, so the previous folder walk swept them in — inflating usage for anyone with a large external library. The asset id is recovered from the derivative filename to filter them out.
This is separate from the sidebar showing whole-disk figures for users without a quota, which is stock upstream behaviour and is unchanged here.
Upgrade
No migration. On upgrade the column still holds whatever #492 last wrote, so an existing install keeps showing today's derivative-inclusive figure until the next nightly sync recomputes it as originals-only. The improvement arrives within a day rather than at restart; nothing regresses in the meantime.
Validation on a real library
Deployed as an RC to a live instance (69,146 assets, S3-backed) and checked both toggle states against measurements taken independently of this code — a separate S3
ListObjectsV2census and a separatestat-based disk walk.The external-library exclusion could not be exercised there (that instance has no external assets); it is covered by the medium tests instead.
Notes
nightlyTasks.syncQuotaUsage; this is called out in the admin panel and the config-file docs.Testing
Server 5765 · web 5629 ·
tscboth packages ·svelte-check607 files 0 errors · eslint both · prettier (i18n + docs) · branding check — all green locally, and CI is green including Medium Tests (Server), SQL Schema Checks, Test Branding and the full E2E matrix.make fork-ownership-coverage-checkfails onmainitself: the manifest'slast_verified_fork_headis stale and unrelated files (web/vite.config.ts,web/src/test-data/mocks/*) have moved since. This PR touches none of them and adds no uncovered paths — worth a separate fix on main.Design notes:
docs/superpowers/plans/2026-08-12-storage-usage-opt-in-cached.md