mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 22:10:34 +00:00
11 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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 |
||
|
|
b5fbc4fd8c
|
✨ Add size limits to profile props and plugin registry (#11596)
* ✨ Add size limits to profile props and plugin registry Bound the total serialized size of profile settings to 2 MiB (:profile-props-max-size), checked on the merged result before persisting, with a controlled :props-too-large error. Profiles that already exceed the limit can still shrink but cannot grow. Cap plugin registry entries in the shared schema (code 1 MiB, 50 plugins max, bounded name/host/description/icon) and restore rate limiting on the plugin RPCs (profile-mutations bucket, one write at a time per profile). The plugin manager now asks for confirmation before removal and ignores repeated clicks while a persist request is in flight. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 🐛 Enforce plugin count cap, byte sizes and removal guard Enforce the declared 50-plugin cap in add-profile-plugin with a specific :too-many-plugins error (updates of existing entries still pass); the cap lives in a shared max-plugins constant. Measure profile props size in UTF-8 bytes instead of chars so multibyte content cannot slip past the limit. Cover install/remove persist logic with mocked-RPC frontend tests (release semantics, in-flight dedupe, validation vs rollback split) and add the missing boundary tests in common. Expose the in-flight persist set from the plugin registry and disable the remove button of entries being saved. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 🐛 Fix rollback loops and restore paths in plugin registry Restore the previous plugin version instead of dropping the entry when a validation error rejects an update of an installed plugin. Make compensating writes one-shot with terminal callbacks so a persistent failure cannot ping-pong between install and remove. Restores keep the original list position; the unused public plugin-persisting? predicate is removed. Pin count-before-size precedence with a dedicated test and fix translation source refs to their canonical lines. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 🐛 Guard notifications write and fix restore ordering Route update-profile-notifications through check-props-size! so oversized profiles cannot grow through that path; document the exempt system writers. Remove the duplicated stale entries in en.po, keeping the canonical translation refs. Restore rejected plugin updates at their original list position instead of leaving the optimistic move in place. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 🐛 Skip no-op plugin removal and clarify size comments Return early from remove-profile-plugin when the id is absent: no wasted write, no size check, and no manufactured :plugins key that could spuriously fail on oversized profiles. Clarify that per-field string caps count chars while the byte budget is enforced by profile-props-max-size. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 📎 Fix formatting in rlimit.edn for profile operations Signed-off-by: Andrey Antukh <niwi@niwi.nz> * 📎 Fix formatting of import-binfile/global entry Signed-off-by: Andrey Antukh <niwi@niwi.nz> * ♻️ Simplify props size check and tighten plugin entry caps Measure props with transit bytes directly instead of the PGobject string roundtrip. Rename check-props-size! to check-props-size: single hard limit on the merged props, no growth comparison, and return props so writers thread the check into the update. Move the 2 MiB default into default-props-max-size on the profile namespace, still overridable with the optional :profile-props-max-size config entry. Tighten registry-entry :code and :icon to 500 chars: they hold manifest paths, not content. Closes #11592 AI-assisted-by: muse-spark-1.3-contributor * 🐛 Fix compatibility problems --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: alonso.torres <alonso.torres@kaleidos.net> |
||
|
|
3033da4409
|
🐛 Add concurrency limit to import-binfile RPC handler (#11024)
Apply climit with 4 global permits and 1 per-profile permit (queue 2) to prevent connection pool exhaustion from concurrent imports. Each import holds a DB connection for its entire duration with idle transaction timeout disabled, so unbounded concurrency could exhaust the pool (default 60 connections). AI-assisted-by: mimo-v2.5-pro |
||
|
|
4f7bb94bb1
|
🐛 Add size limit and rate limiting to send-user-feedback (#10979) (#10990)
Prevent email bombing attacks on the send-user-feedback endpoint by limiting the error-report field to 1MiB and adding climit rate limits: by-profile (1 permit, queue 3) and global (4 permits), configured in climit.edn. Make the schema public so it can be exercised by tests, and add schema validation tests covering the new size limit. AI-assisted-by: qwen3.7-plus |
||
|
|
405a73e8ba
|
✨ Add climit impl and config for file snapshot methods (#9722)
* ✨ Add dedicated concurrency limit for restore-file-snapshot This adds a dedicated climit configuration for the restore-file-snapshot RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout) and a global limit of 3. Previously the method only used the generic root/by-profile and root/global limits, allowing up to 7 concurrent restore operations per profile which caused database row lock contention on FOR UPDATE and connection pool exhaustion. * ✨ Skip locking on restore! to avoid blocking other operations Changes the row lock acquisition in restore! from a blocking FOR UPDATE to FOR UPDATE SKIP LOCKED. If the file row is already locked by another concurrent operation (e.g., another restore or an update-file), the query returns no rows and the caller fails fast with a clear conflict error instead of blocking indefinitely holding a database connection. * ✨ Add queue and timeout limits to root/by-profile concurrency limit Previously root/by-profile had no queue limit (unbounded Integer/MAX_VALUE) and no timeout, allowing requests to pile up indefinitely behind a profile whose permits were exhausted by long-running operations. This could lead to memory pressure and cascading failures. Now limited to 30 queued requests with a 30-second timeout so excess requests fail fast. * ✨ Move backup snapshot creation outside restore transaction The backup snapshot (fsnap/create!) is now created in its own short-lived connection before the actual restore transaction begins. This ensures the backup is persisted independently of the restore outcome and reduces the restore transaction window. The restore itself runs inside a db/tx-run! block with an optimistic locking check: it reads the file with FOR UPDATE and compares its revn against the value captured at backup time. If the file was edited concurrently, the restore aborts with a conflict error to prevent data loss. Co-dependent with the SKIP LOCKED change in restore! — the FOR UPDATE acquired here is in the same transaction as restore!, so the SKIP LOCKED inside restore! correctly sees the row as unlocked (same transaction). * ♻️ Remove unused private function get-minimal-file The local get-minimal-file function in file_snapshots.clj is no longer used since restore! switched to direct exec-one! with FOR UPDATE SKIP LOCKED. The sql:get-minimal-file SQL constant is still used directly. * ✨ Add minor improvements on db connection management * ♻️ Refactor create-file-snapshot to use explicit transaction management Remove automatic transaction wrapping (`::db/transaction true`) and pass `cfg` through the call chain instead of destructured `conn`. Wrap `fsnap/create!` in an explicit `db/tx-run!` for clearer transaction boundaries. Signed-off-by: Andrey Antukh <niwi@niwi.nz> * ✨ Add dedicated concurrency limit for create-file-snapshot This adds a dedicated climit configuration for the create-file-snapshot RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout) and a global limit of 3. Previously the method only used the generic root/by-profile and root/global limits, allowing up to 10 concurrent snapshot creation operations per profile which could cause database contention and connection pool exhaustion. Signed-off-by: Andrey Antukh <niwi@niwi.nz> --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> |
||
|
|
a5c6d78ee5 |
♻️ Fix some fundamental bugs on climit module
The climit previously of this commit is heavily used inside a transactions, so in heavy contention operation such that file thumbnail creation can cause a db pool exhaust. This commit fixes this issue setting up a better resource limiting mechanism that works outside the transactions so, contention will no longer hold an open connection/transaction. It also adds general improvement to the traceability to the climit mechanism: it now properly logs the profile-id that is currently cause some contention on specific resources. It also add a general/root climit that is applied to all requests so if someone start making abussive requests, we can clearly detect it. |
||
|
|
54341d5b22 | ✨ Make the RPC climit subsystem more robust | ||
|
|
da68eae7c6 | ✨ Add proper climit configuration for file-thumbnails | ||
|
|
aafbf6bc15 |
♻️ Refactor cocurrency model on backend
Mainly the followin changes: - Pass majority of code to the old and plain synchronous style and start using virtual threads for the RPC (and partially some HTTP server middlewares). - Make some improvements on how CLIMIT is handled, simplifying code - Improve considerably performance reducing the reflection and unnecesary funcion calls on the whole stack-trace of an RPC call. - Improve efficiency reducing considerably the total threads number. |
||
|
|
7f589b09ca | ♻️ Move audit http handler to RPC | ||
|
|
37ad04d2a6 | 🎉 Add robust concurrency limiter for RPC |