fix(api): restore the stale pending upload sweeper lost from main - #47
Merged
Conversation
Add a scheduled cleanup job that deletes PENDING photos older than 24h from S3 and Postgres, with batching, env-based config, and a status index for efficient sweeps. Co-authored-by: Cursor <cursoragent@cursor.com>
API unit-test coverage
Unit suite only; controllers are exercised by the e2e suite. |
…-cleanup # Conflicts: # api/docs/photos-architecture.md # api/src/photos/photos.constants.ts
MehrshadFb
added a commit
that referenced
this pull request
Sep 4, 2026
Usage was already per uploader, but the cap was one env var for everyone, so a paid or beta account could not be given a bigger ceiling without a redeploy. The limit belongs on the account, not in config. - Add User.storageLimitBytes (BigInt, default 5 GiB) and its migration; NOT NULL DEFAULT means existing rows and JIT-provisioned users get the free tier with no backfill and no code setting it - Read the limit per userId in PhotoStorageService, inside the same Serializable transaction as the usage SUM so the reservation compares against a consistent snapshot; a missing user row surfaces as 404 - Remove the global override: PHOTO_STORAGE_LIMIT_BYTES, the storageLimitBytes config key, and the ConfigService dependency of PhotoStorageService. Raising a limit is now a row update - Keep src/config/photos.config.ts, which still holds the pending cleanup keys added by #47, and keep photosConfig registered in AppModule. Deleting the file was correct when it held only the storage limit, and is not correct after #47 - Add an e2e assertion that the photos config namespace resolves from a booted AppModule; the cleanup scheduler reads those keys only when its cron fires, so an unregistered namespace would otherwise surface in production rather than in any test - Add storageLimitBytes to the user fixtures and inline UserWithDetails literals; cover the per-user limit, the free-tier default and the 404 in unit and e2e tests - GET /users/me/storage and POST .../photos/upload-urls keep their contracts, so no OpenAPI or mobile change
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.
Summary
PENDINGupload sweeper from feat(api): sweep stale pending photo uploads hourly #39, which was merged on 1 September and then lost whenmainwas force-updatedmainand reconciled with the storage quota work that landed after themPhotoPendingCleanupServiceand its hourly scheduler, thePHOTO_PENDING_CLEANUP_*config, and the(status, createdAt)index that makes the sweep query cheapWhy
#39's merge commit is not an ancestor of
main. The history went from the type-check commit on 25 August, to #38 on the 28th, to #39 on 1 September, and was then force-updated to the storage quota commit, whose parent is that 25 August commit. That rewind discarded two merge commits. #38 survived by accident, because the quota branch already contained its changes and carried them into the squash. #39 did not, so the sweeper is absent frommainwhile GitHub still reports the pull request as merged.The gap matters beyond the missing job.
PENDINGrows count toward the storage quota, so an upload slot that is never confirmed holds its bytes against the uploader's cap. Reads filter toREADY, so the row is invisible to both the list endpoint and single-photo reads, and the owner has nothing to delete. Without this sweeper a user can reach their limit with no visible photos and no way to recover.Conflicts resolved
Five files needed both sides kept, since the quota work and the sweeper touch the same places:
src/config/photos.config.tssrc/photos/photos.constants.tssrc/photos/photos.module.tsPhotoStorageServiceplus the two cleanup providers.env.exampledocs/photos-architecture.mdThe
buildPhotoS3Keypart of #39's diff applied as a no-op, because #38's uploader-namespaced keys are already onmain.Notes
20260829031200_add_photo_status_created_at_indexis additive and index-only, and it sorts after the migrations already onmain@nestjs/schedulev6 andScheduleModule.forRoot()come back with it. feat(api): reclaim orphaned photo objects from S3 daily #43 also adds both, so whichever merges second will need that line droppedTest plan
npx jest --watchman=false(254 tests, 17 suites)npx jest --config ./test/jest-e2e.json --watchman=false(94 tests)npm run typechecknpm run openapi:checknpx eslint,npm run format:check