Repository navigation
feat(uploads): derived files (variants) for thumbnails, previews and poster frames (experimental) - #3020
feat(uploads): derived files (variants) for thumbnails, previews and poster frames (experimental)#3020Tobbe wants to merge 3 commits into
Conversation
…poster frames (experimental)
✅ Deploy Preview for cedarjs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 48 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (row.storageKey) { | ||
| await resolveTarget(targets, row.target).delete(row.storageKey) | ||
| } | ||
|
|
||
| await db.upload.deleteMany({ where: { id: row.id } }) |
There was a problem hiding this comment.
If deleteVariants() reads a pending row with no storageKey, the generator can complete before deleteMany() runs. deleteVariantRow() then skips the new file but deletes its row. The generator saved successfully, so its lost-claim cleanup never runs. Claim the row before deleting it, or condition deletion on the saved state and retry if it changed.
| const { count } = await db.upload.updateMany({ | ||
| where: ours, | ||
| data: { | ||
| status: 'completed', | ||
| storageKey, |
There was a problem hiding this comment.
If the completion updateMany() rejects after target.write() succeeds, the new file is left behind. The save is outside the write's try block, and the pending row still has no storageKey, so cleanupStaleUploads() cannot find the file. Handle completion errors and safely remove or retain a record of the written file, as storeFile() does when saving its row fails.
| data: { | ||
| status: 'completed', | ||
| storageKey, |
There was a problem hiding this comment.
If a deployment changes a variant's target, retrying a failed row or taking over an old claim writes to the current definition.target. Completion never updates the row's target, so the completed row names the old provider while its file lives on the new one. Reads and deletion then use the wrong provider. Save the target used by the winning attempt together with its result.
| const { count } = await db.upload.updateMany({ | ||
| where: { id: row.id, status: 'failed', updatedAt: { lt: cutoff } }, | ||
| data: { status: 'pending', tokenId: claim }, |
There was a problem hiding this comment.
A failed variant retry reuses its old createdAt. cleanupStaleUploads() checks that time, not the fresh claim's updatedAt. With the defaults, a retry after one day immediately qualifies for the one-hour cleanup cutoff. If cleanup runs during the attempt, it marks the row failed; the successful generator discards its result, and another retry is delayed a day. Make cleanup honor the age of the current variant claim.
| const key = `${sourceId}\0${name}` | ||
| const running = inFlight.get(key) | ||
|
|
||
| if (running) { | ||
| return running |
There was a problem hiding this comment.
Start getVariant(source, 'a') and getVariant(source, 'b') together, with each generator calling readVariant() for the other. Their separate chains contain only their own names, so both cycle checks pass and each returns the other's unresolved promise. Both requests hang indefinitely; claimTimeout does not apply to shared promises. Track active dependencies before returning an existing attempt so this cycle is refused too.
| if (remake) { | ||
| const made = await getVariant(sourceId, name).catch((e: unknown) => { | ||
| onError(e, { sourceId, variant: name }) | ||
| return null | ||
| }) |
There was a problem hiding this comment.
If regenerateVariants() deletes a row whose generator is still running in this instance, getVariant() reuses that same attempt. The old attempt finds its row gone, removes its output, and returns null. Regeneration counts a failure and never starts a replacement, even with remake: true and a working generator. Wait for or safely retire the deleted row's attempt before requesting a fresh one.
| if (!options.targets) { | ||
| throw new UploadError( | ||
| 'HAS_VARIANTS', | ||
| `Upload '${upload.id}' has derived files (variants), so its row ` + | ||
| 'cannot be deleted on its own. Pass `targets` to deleteFile() to ' + | ||
| 'delete the variants with it, or call deleteVariants() first.', |
There was a problem hiding this comment.
P2003 does not identify a variant relation. If an app model still references an upload with restricted deletion, it raises the same error—even without variant columns. This branch labels it HAS_VARIANTS and recommends helpers that cannot fix the reference. Only use this message when variants are established as the cause; preserve other foreign-key errors.
|
View your CI Pipeline Execution ↗ for commit 2b2fae4
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
This branch has not been deployed
1 participant
Summary
Adds derived files (variants) to
@cedarjs/uploads, marked experimental. An upload can have variants (thumbnails, previews, a video's poster frame), each anUploadrow of its own, linked to its source and named (thumb-164,preview). Cedar keeps the bookkeeping; the app supplies only the function that makes one.Cedar owns: the link to the source, deletion with the source, making a missing variant once even under concurrency (in one process and across processes), remembering a failure with a retry-after, remaking every variant of one name, and reading/serving. The package does not depend on sharp, pdf.js or ffmpeg: a generator is any function from source bytes to new bytes.
Implementation plan:
docs/implementation-plans/2026-10-12-upload-derived-files.mdThe problem
In a production Cedar app (Effibo) that shows thumbnails of photos and PDFs and a drawn first page of each PDF, most of the effort went into plumbing, not image making:
Uploadgot a back-relation for each: eight of its eleven back-relations are derived filesHow it fits what uploads already has
Uploads has no processors, hooks or Prisma extension: services call
storeFile()/deleteFile()explicitly, and every file is one row in theUploadtable with bytes on a namedStorageProvider. Variants build on exactly that:Uploadrow, on any configured target (S3, FS, or inline for DB targets, with the same 1 MB cap), with its source'suserIdandorganizationId, so ownership checks and tenancy apply unchanged.@withSignedUrl/@withDataUri;getVariantUrl()is the same logic for services.deleteFile()takes an optionaltargetsand then deletes the upload's variants (bytes, then rows) before the upload. Nothing happens inside a Prisma query.statusdoes the bookkeeping:pendingis the claim while a variant is being made,completedis made,failed(with no file) is the failure memo, timed byupdatedAt.cleanupStaleUploads()already ignores suchfailedrows and turns a long-abandonedpendingclaim into a remembered failure.tokenIdholds the attempt's claim id while pending.UploadDatabasetype is unchanged; a separate structuralUploadVariantDatabasetype covers the new columns, so apps that don't opt in are unaffected.API
Why this schema: variants in the
Uploadtable reuse everything uploads already does; the app's models need no thumbnail columns; the unique index is the cross-process lock; andonDelete: Restrict(notCascade, which would orphan bytes silently) makes deleting a source without its variants a loudHAS_VARIANTSerror that names both ways out.What the tests guard
packages/uploads/src/__tests__/variants.test.ts, against the package's real SQLite test database, with trivial generators:null) is remembered and not retried untilretryFailedAfterhas passed, then retried and maderegenerateVariants()remakes one variant name and leaves the others untouched (same row, same file); it also retries remembered failures;remake: falseonly deletestargetsdeletes its variants and their stored files on every target, and leaves other sources' variants; withouttargetsit's refused withHAS_VARIANTS; a variant finished after its source was deleted leaves no file behindreadVariant()chaining, self-dependency refused, invalid/unknown names, variants of variants, DB-target data URIs, signed URLs,findVariants()The test helper also assigns the generated Prisma client to
UploadDatabase & UploadVariantDatabase, so the structural types can't drift from Prisma's.Ran:
vitest runfor the package (141 tests),tsc --noEmit, ESLint onpackages/uploads/src, Prettier,yarn build,yarn check:package.Open questions
yarn cedar setup uploadsadd the variant columns for everyone, or stay opt-in (as now, documented) while experimental?sourceId/varianton existing rows in a data migration. Worth a recipe once a second app does it.getVariant()makes on demand; apps that can't afford that call it from a job and usefindVariants()in requests. Is a "queue it" mode tied to@cedarjs/jobsworth adding?readSource()buffers; a video generator may want a stream.Related proposals from the same work: declarative recurring jobs and unique jobs in
@cedarjs/jobs, and a "locked child" runner for parsing untrusted files in a sandboxed Node process (generators that parse untrusted files would use it). Sniffing a file's type by its bytes belongs in uploads and is noted as a follow-up in the plan, not built here.🤖 Generated with Claude Code