mirror of
https://github.com/penpot/penpot.git
synced 2026-10-30 14:30:37 +00:00
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Andrey Antukh
|
fffc3ee222 | Merge remote-tracking branch 'origin/staging' into develop | ||
|
Andrey Antukh
|
57fa280209
|
🐛 Validate and decode a job result with its declared schema (#12350)
* 🐛 Decode job results with the schema of the job-def A job result is stored as plain JSON, so a uuid comes back as the text it is on disk and a uuid used as a map key comes back as a keyword. The import resolution is a map keyed by file id, and the wizard lost that identity: choosing a library in the link step created no link at all, with no error and with the modal closing as if it had worked. A job-def can now declare a result schema and its precompiled decoder, the pair the params already have, and the substrate applies them when the row is read. A def that declares neither answers with the JSON the row holds, as before; a row whose job-def is gone is read untouched instead of raising. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Type the import result so its resolution keeps uuid keys The resolution of a version 3 import is a map keyed by the new file id, and the result column is plain JSON: without a schema of its own every key came back as a keyword while the :id of the entry came back as text, so the dashboard wizard could not match the file it was linking and created no library link at all. Declare the shape of the resolution (files, done and pending entries and their candidates) and give the job-def its result schema and decoder. The check before the result is stored keeps validating the same schema. Closes #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Take the imported file id from the entry, not the map key The wizard looked a file up in the selection with the key of the resolution map while it had indexed the selection by the :id of the entry. The key is keywordized on its way through the job result, so the lookup missed and the confirm button created no library link at all, with no error and with the modal closing as if it had worked. Take the id from the entry, the source the rest of the wizard already uses, and move the decision to a pure function covered by tests of both shapes of the resolution. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Test that an import result keeps its ids as uuids The regression this branch fixes is only visible through the read path of the substrate: the resolution of an import is a map keyed by the new file id, and a test that works with the value in memory never sees the keys coming back as keywords from the plain JSON column. Import a package whose library needs a replacement chosen by the user, read the result with `jobs/get-job` as the client does, and check the keys, the file ids and the ids of the candidates. Without the result decoder of the import job-def the test answers with text ids. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 📚 Document the result schema of a job-def The memories and the docstring of the substrate said a job result has no schema of its own and comes back as the JSON the row holds. A job-def can now declare its result schema and decoder, and the substrate applies them when the row is read. Keep the rule next to it: a result map keyed by uuid needs its schema declared, or every key comes back as a keyword, and the link-later step of the import wizard takes the file id from the entry and never from the key of the resolution map. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 📚 Fix the documented contract of a job result The docstring of get-job said a result has no schema of its own and comes back as the JSON the row holds, which is what this branch changes: the result is decoded with the schema its job-def declares, and a result that does not conform never reaches the column because the write validates it. Say the same in the management command that stores the result of an external worker, the other writer of the column. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Check the result of a job against its schema before storing it The result of a job crosses a plain JSON column, so a uuid comes back as text and a uuid used as a map key as a keyword. The job-def now declares the schema of its result and the substrate compiles the decoder from it, which is what the read path uses to get the types back. The same schema is the contract of the write path: complete refuses a result that does not conform, with the malli explanation, so nothing invalid reaches the column, and the answer is the same for the runner and for the external worker that completes a job through the management RPC. A job that is no longer completable is skipped without checking, and the read path decodes only: a row written by another version stays readable. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Check the result of an import inside its transaction The check ran after the transaction of the import had committed, so a result the schema does not accept failed the job with the files already created in the destination project: a half imported project and an error. The core decides whether the whole import is kept or rolled back, so the check belongs inside its transaction, which is what turns a contract violation into a rollback. The substrate checks the result again when the job is completed, which is the only net for a job that does not validate its own result. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 🐛 Cover the edges of an import result read by the client The shape of a result with no libraries to resolve, and the one of a version 1 import, is the most common in production and neither was covered by the tests that read a result the way the client does. Cover both through jobs/get-job, and the case of a registry that no longer knows the kind of the row: it answers with the JSON of the column, keywordized keys and uuid as text, instead of raising. Say in the two tests that depend on the decoder what implementation without it makes them fail. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 📚 Fix the prose and the fixtures of the result contract The result schema of a job-def is what the read decodes with and what the write checks: say that in the memories, with the rule that any uuid of a result needs it, whether a value or a map key. The motive of the wizard helper was a fact this branch makes false: what a key of the resolution map holds depends on the result schema of the job-def that wrote it, so the rule is to read the id from the entry. Same correction in its memory and in the test that pins the shape. The candidates of a pending entry match by slugified name, not by name, and every test row now states the kind of its job, because the name is what resolves its job-def when the result is read or checked. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * ♻️ Keep the wizard link helper private and cover its shape The helper that decides which links the wizard creates belongs to its namespace, as its neighbours do: reach it from the test with @#', the pattern the frontend tests already use for a private helper. Cover the shapes it had no test for: a file with two libraries pending resolution, a file whose libraries were auto-linked, and a selection where only one of two entries was resolved. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> * ♻️ Share one job-row helper across the job tests Three test namespaces had their own copy of the same insert, differing only in the kind of the job and in whether the row holds the resource it consumes. Move it to the test helpers as create-job*, with the kind of the job as its first argument: it is what resolves the job-def of the row when its result is read or checked, so every test states it. The bang of the name goes away with it: the design rules keep it for the mutating API of app.db, and this is a fixture of the tests. The three namespaces lose the requires that only their removed copy used. Relates to #12339 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: Andrey Antukh <niwi@niwi.nz> --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> |
||
|
Andrey Antukh
|
9abcfebf60
|
✨ Run binfile import and export as durable jobs (#12056)
* ✨ Run binfile import and export as durable jobs Import and export of `.penpot` files become durable jobs: two commands create them and answer at once with a job id the caller follows, while a worker of the `binfile` queue does the work. Each job keeps its own ledger (owner, params frozen at creation, result, progress history and an expiry), and the legacy commands and their SSE keep working. Why: both operations ran inside the request. A large import held it open for minutes, progress only existed while the caller kept the stream open, and a client that went away left work nobody recorded, retried or could cancel. The unified `job` substrate already dispatched, leased, retried and logged events, but it had no user-facing ledger. How: - The substrate gains the pieces a user-facing job needs: optional job-def metadata (family, resource-role), an expiry on submit, the owner in the runner context, and job resources in their own bucket, written with the profile of the job and never deduplicated. - One generic command per family: the caller names the job and sends its business params, the registry checks the family, the job-def decodes and validates, and permission and quota are checked first. - Progress taps of the core become milestones of the job: a shared stage vocabulary with self-contained counters, throttled heartbeats, and an interrupt that stops the run inside its own transaction on cancel. - Cancel reaches pending and running jobs from both modals, and closing a modal mid-run cancels the job it was following. - The export re-checks the read permission at run time and answers with the package descriptor; the import re-checks the edition permission, runs the core in a transaction so a cancellation rolls it back, and releases the consumed package and its temporary copy. AI-assisted-by: deepseek-v4.1-flash * ♻️ Handle externally-cancelled binfile jobs end to end The `cancelled` outcome of a job nobody followed no longer hangs its screens: the data layers emit an explicit terminal message for it. - The export emits `{:cancelled true}` per file and the modal paints a neutral cancelled row instead of success; a message without an artifact never reaches the download branch. - The import closes every entry with a dedicated cancelled message through the existing error rendering, so the wizard completes. - Both flows are pinned by data-layer tests, including the red state they replace. Also in this change: the progress-contract comment of the management API, early business-params validation in create-import-job (before assembling the upload), a `release-resource` helper without fabricated job ids, a note on the heartbeat write cost, and a memory cleanup. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Pre-translate the cancelled import message The cancelled entry of the import wizard carried its message as a key for the UI to translate, which needed a rehash marker comment. Like its neighboring errors, it now translates eagerly in the data layer: the literal call keeps rehash clean with no marker, and the tests assert through the same call instead of the raw key. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Move cancelled strings under generic labels keys The dashboard-specific cancelled key never shipped: before use it moves under `labels.*` next to the generic strings both flows already use, with Spanish translations following the established wording. AI-assisted-by: muse-spark-1.3-contributor * ✨ Add minor improvements * ♻️ Rename export job command to create-export-binfile-job Export now has one command per job type, the shape the :export-assets command needs: create-export-binfile-job freezes exactly the :export-binfile job, so :name no longer travels in the body and only :params does. Import keeps the generic envelope. submit-job reads the queue from the job-def metadata ::jobs/queue-name (falling back to :binfile), read by the command and never by the substrate, so a job-def can route its work to another queue without a change in the command. The rename reaches the frontend and the tests: the binfile export is asked for under its own name, and the exporter service keeps create-export-job. AI-assisted-by: deepseek-v4.1-flash * ✨ Polish the import and export status rows The status row of a file already carries what the file is doing, so the generic "Uploading file…"/"Downloading file…" lines are gone: the import wizard drops its own (the export one went with the jobs refactor), along with the now-unused style. In the export, every file is queued up front, even the ones whose job is created only after the previous one is over, and a file stays queued until its first milestone: a worker picking the job up no longer ends the queue. The queued text is a step smaller than the milestone. AI-assisted-by: deepseek-v4.1-flash * 🐛 Show the cancelled export and fix the import error The cancelled export was unreachable: mark-file-cancelled sets :loading false and the label lived inside the loading branch, so a cancelled file only painted a neutral notification. The terminal state now carries the label, and the export flags lose the `?` suffix (predicates keep it; a data prop is not a question). The import wizard rendered (:error entry) through tr, but every producer writes user-facing text (a hint or a translated label), never a key: it renders the text as-is now, and the dynamic-key ignores and the stale comment are gone. The total-text-limit comment no longer references the 200MiB it replaced; the value is 500MiB in develop already. AI-assisted-by: deepseek-v4.1-flash * ♻️ Reflect the real job params in the creation commands The creation commands declared their params as an open empty map, so the RPC contract and the generated docs said nothing about them. Each command now imports the schema from the job namespace (`export-binfile/schema:params`, `import-binfile/schema:create-params`), and the RPC validates the business params before the command runs. The job-def still owns the params and re-validates the assembled ones at run time. The commands no longer decode the params again: the RPC did it. Import joins export as one command per job type: `create-import-job` becomes `create-import-binfile-job`, its body no longer carries `:name` and the job-def is resolved by name like export's. Its creation schema keeps the version optional, because the command reads it from the package header when the caller does not say it, and fills the manifest metadata kept for audit. A version out of range is now refused by the schema (`:params-validation`) instead of the command's own `:unsupported-version` check, which is gone. AI-assisted-by: deepseek-v4.1-flash * 🐛 Stop the test system from running job runners The test system is main/system-config plus main/worker-config with a few components dissoc'ed. The runners of default, webhook and cron were listed by hand; the binfile runner added with the durable binfile jobs was not, so a real penpot/job-runner/binfile/0 thread ran during the whole suite. That runner races the tests that drive the loop by hand (run-batch plus run-worker-loop): it takes the job the test just dispatched and completes it, and the test then reads a state it did not produce. That is the reported flake of binfile-jobs-integration-test, whose middle assertion waits for a runner of another queue to not take the job. It only reproduced in CI: in the devenv PENPOT_TENANT makes the tenant baked into main/worker-config differ from the one the test builds, so the two used different queue keys locally. Drop every [<profile> :app.worker/runner] by key shape instead of listing the known queues, so a new queue cannot bring the race back. AI-assisted-by: deepseek-v4.1-flash * ✨ Upload the package one chunk at a time and as a percentage The upload milestone counts chunks, not units of work, so the row wrote "Uploading the package 3/9" over a file the user had uploaded once: it read as if nine packages were on their way. Write the stage as a percentage of the file instead ("Uploading file… (33%)"), and make the English text of the stage say file, not package. The chunks also go one at a time now (concat-all instead of merge-all with two in flight), so the percentage moves in a straight line instead of jumping. upload-blob-chunked is the generic uploader, so media and font uploads become sequential too. AI-assisted-by: deepseek-v4.1-flash * 🌐 Complete the Spanish catalogue es.po was 25 keys short of en.po: the 15 that the durable binfile jobs introduced (14 jobs.progress.stage.* and labels.queued) plus 10 gaps that predate them (errors.connection-error, errors.save-retrying, the find shortcuts, workspace.header.retrying, workspace.tokens.stroke-width and the mcp session labels). sync -l es then sorts the entries, syncs the #: comments from en and drops the 7 dashboard.import.progress.* keys en no longer has. The msgid sets of en and es are now identical (0 missing, 0 extra). AI-assisted-by: deepseek-v4.1-flash * ♻️ Move the job status keys into the jobs namespace The three status words the durable import and export rows render were filed under labels.*, next to the generic labels bucket, while their progress stages live under jobs.progress.stage.*: the strings of one feature sat in two places. Rename them so the whole job vocabulary is one namespace: labels.queued -> jobs.queued labels.export-cancelled -> jobs.export-cancelled labels.import-cancelled -> jobs.import-cancelled No other locale has these keys yet, so the rename costs no translation. The convention (a key names the feature that renders it; labels.* holds the generic reusable words) is written down in the frontend/translations memory. AI-assisted-by: deepseek-v4.1-flash * 🔥 Remove the translation keys nothing references Six keys of en.po (and of es.po, which now mirrors it) have no (tr ...) call site left in frontend/src or common/src, and no literal reference either: labels.uploading-file a duplicate of jobs.progress.stage.upload since the upload stage says "Uploading file…" labels.downloading-file dead since the durable jobs replaced the old import and export progress rows inspect.attributes.stroke.alignment.center inspect.attributes.stroke.alignment.inner inspect.attributes.stroke.alignment.outer workspace.toolbar.mcp-connect-here The first two are leftovers of the durable binfile jobs, the other four predate them. Both catalogues keep an identical msgid set afterwards. AI-assisted-by: deepseek-v4.1-flash |