Repository navigation
upload: add the Upload model, migration and real crud (swap the stub) - #198
Merged
Merged
Conversation
Replaces the no-op crud_upload with the real persistence layer: the Upload model (one row per accepted upload; sha256 indexed but not unique), a named-FK migration, and real create/exists_with_sha/get_multi_by_sha/all_shas. find_orphans is now typed against models.Upload. Restores the deferred tests -- re-upload of identical bytes is free, a second row is written, find_orphans/detection work.
…up front (#199) Four review findings on the image-upload work, none of which change the shape of the design. purpose was a free-form string. The karma gate tests for "figure" and the read-time misuse detection tests for "avatar", so any third value was an upload that was neither gated nor detected: karma 0 with purpose="banana" returned 200. It is now upload_purpose_T, a closed Literal beside editor_T, so anything else is a 422 at the boundary. find_usages scanned bodies only, but a purpose="avatar" upload lives in User.avatar_url and is embedded in no body at all, so every avatar ever uploaded read as an orphan -- and orphans are the input to any future garbage collection. Question.description had the same gap. Both avatar columns and both question description columns join the scan. is_configured() checked only the endpoint URL while the write path also needs the bucket (a NOT NULL column) and the public URL base, so a half-configured deployment 500'd *after* put_image had already stored the object. It now checks all three, which makes it the 503 it was meant to be, before anything is written. s15 shared one PNG fixture across checks, so the "new figure costs 2 coins" point was uploading bytes an earlier check had already sent -- correctly free, and reported as XFAIL against a backend that was right. Each check now builds its own bytes, and every point is a hard assertion rather than a note, so a regression fails the suite instead of printing a known-failure line. Claude-Session: https://claude.ai/code/session_01CqbhVfwzFUYGkgJWnWwSk1 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
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
The final piece: swap the no-op
crud.uploadstub for the real persistence layer — theUploadmodel, its migration, and the real crud — and restore every test that was deferred while the stub was in place.What's in here
models/upload.py— one row per accepted upload:sha256indexed but deliberately NOT unique (bucket = one object per sha; table = one row per upload, so a free re-upload still leaves an audit trail).storage_bucketrecords which bucket holds the bytes, so a future vendor switch is a verifiable list.purpose,content_type,size_bytes(post-sanitize),original_filename,uploader_id,created_at.3c2f1a8b9d4e— named FK (upload_uploader_id_fkey) sodowngradedrops cleanly.crud/crud_upload.py— realcreate,exists_with_sha,get_multi_by_sha,all_shas(no commits).models/__init__.py,models/user.py— registration +User.uploadsrelationship.services/uploads.py—find_orphansis now typedList[models.Upload].Tests restored (were deferred)
find_usages/find_orphansmisdeclared_avatarslogic + dedupe + swallowed-failure + article-read integrationThis is the PR that flips the e2e spec's last point — "identical bytes are free and deduplicate" — to OK.
Verified
Migration round-trips (
upgrade head+downgrade); full upload test suite passes (17); ratchets clean.