mirror of
https://github.com/penpot/penpot.git
synced 2026-11-01 23:50:38 +00:00
14 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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 |
||
|
Andrey Antukh
|
d67a00c1d5
|
✨ Normalize storage metadata with a closed schema (#11987)
* ✨ Add Malli schema for storage metadata with dual decode Phase 1 of the storage_object.metadata migration: reads accept both Transit and plain JSON (sniffed by the marker) and always return the normalized shape; writes validate against a closed per-bucket Malli schema and still serialize as Transit unless the new :storage-metadata-as-json config flag is set. The 0155 migration normalizes existing rows inside Transit (reference to bucket, default bucket, drop of chunk leftovers) and is idempotent; large instances should fake it and run the batched script instead. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Address review findings on storage metadata Phase 1 Collapse the dead :reference leg of the gc-touched bucket fallback (the decode always sets :bucket on non-nil metadata, so it is only reachable with a NULL column) and fix its comment. Pin the write flag off in the transit-assuming metadata tests so the suite proves the same with the flag set, and add coverage for the flag rollback contract, JSON hash survival, NULL metadata in gc-touched, and the 0155 normalization statements. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Backfill NULLs, canonical buckets, comment fix Backfill NULL metadata columns in 0155 via coalesce (the key-missing rule already matches them), derive valid-buckets from the Malli schema dispatch entries so the list lives in one place, and correct the lookup-bucket fallback comment to NULL columns. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Defer corrupt metadata rows in storage gc-touched Decode touched rows individually so one non-map metadata value no longer aborts the whole chunk: corrupt rows are logged and deferred exactly one day in the same transaction, keeping their metadata intact for a later repair, while healthy rows process normally. AI-assisted-by: muse-spark-1.3-contributor * ♻️ Address storage metadata phase 1 review findings Address the review findings on the storage metadata phase 1 branch: - Fix put-and-delete-object: it stored the object with ::sto/expired-at, so the row was already deleted and del-object! returned false. Add delete-expired-object-returns-false to keep the expired-delete case covered. - Cache the Malli decoder and encoder per process. Building them compiles the closed multi-dispatch schema, and decode-metadata runs on every read path (get-object, dedup probes, GC batches). - Catch Exception instead of Throwable in try-decode-row so JVM Errors are not deferred as corrupt metadata. - Add penpot_storage_gc_poison_total, emitted from storage-gc-touched; wire ::mtx/metrics into its handler. - Anchor the encoding sniff to the start of the document so a plain JSON value that begins with a Transit-looking prefix is not read as Transit. - Cover every bucket on both encodings, a JSON roundtrip through the jsonb column, nil metadata, the canonical bucket set and a poison-only GC chunk. - Rename private check-metadata! to check-metadata. AI-assisted-by: deepseek-v4.1-flash * ♻️ Simplify the storage metadata schema to a single map Replace the per-bucket :multi dispatch with a single closed map: the bucket is validated with ::sm/one-of over metadata-buckets (now a plain set) and the remaining keys are typed optional fields. Per-bucket enforcement shrinks to a one-line :fn guard requiring :file-id and :id for file-data, whose ids the GC reads to resolve references. - Drop the dead (sm/register! ::metadata ...): nothing references the schema by keyword. - Define tempfile-bucket and upload-session-bucket in the schema and alias them from app.storage, removing duplicated literals. - Keep content-type required and the map closed, so an unknown bucket or key still fails fast on write. AI-assisted-by: deepseek-v4.1-flash * ♻️ Drop input coercion from encode-metadata encode-metadata no longer runs the json-transformer decoder before validation. On the write path its only effect was coercing string UUIDs to UUID, and every producer already passes native UUIDs (the RPC profile-id, uuid/random, or binfile ids decoded as ::sm/uuid). Reads keep decoding, so stored Transit or JSON values still come back as native types. - Replace encode-accepts-string-uuids with encode-rejects-string-uuids, pinning the stricter contract. - Pass native UUIDs in encode-writes-plain-json-with-flag. AI-assisted-by: deepseek-v4.1-flash * 📚 Document each statement in the storage metadata migration Move the per-statement rules out of the header and add a comment to each UPDATE explaining what it does: drop chunk leftovers, promote the legacy "~:reference" to "~:bucket", drop residual "~:reference", and backfill the default bucket. The header keeps the scope, the encoding note, the `->` vs `?` note and the large-instance warning. AI-assisted-by: deepseek-v4.1-flash * 📚 Unwrap wrapped lines in the backend storage memory One line per bullet or paragraph, as mem:memory-maintenance requires. Only formatting; no content change. AI-assisted-by: deepseek-v4.1-flash * ♻️ Defer storage GC poison rows in their own transaction process-chunk! no longer takes poison-ids; it only processes the healthy chunk. The deferral moves to defer-poison! and process-touched! runs it in its own transaction, separate from the freeze/delete work. The loop still drains while there is chunk or poison, so a batch made only of poison rows does not leave healthy rows behind the LIMIT 10 waiting for the next run. Add a regression test: ten poison rows plus one healthy row with a later touched_at are all handled in the same run. AI-assisted-by: deepseek-v4.1-flash * ♻️ Declare per-bucket metadata requirements in one map Replace the file-data-specific predicate with bucket-requirements, a map from bucket to the extra keys it must carry. metadata-buckets is derived from its keys and a single generic :fn enforces presence, so a new bucket and its contract are one entry. organization now requires :organization-id; file-data keeps requiring :file-id and :id. Update the http-assets test helper to set organization-id for its organization objects. AI-assisted-by: deepseek-v4.1-flash * ♻️ Drop the ! suffix from storage GC helpers Rename the internal helpers in app.storage.gc-touched (process-chunk, defer-poison, mark-freeze-in-bulk, ...) to drop the trailing !. AI-assisted-by: deepseek-v4.1-flash * ♻️ Drop the ! suffix from storage GC deleted helpers Rename the internal helpers in app.storage.gc-deleted (clean-deleted, delete-sobjects, delete-give-up, ...) to drop the trailing !. AI-assisted-by: deepseek-v4.1-flash * 🐛 Fix dedup lookup for JSON-encoded storage metadata get-database-object-by-hash only matched the Transit keys, so once the :storage-metadata-as-json flag wrote plain JSON rows the dedup stopped finding them and duplicated blobs. Match both encodings with a UNION ALL of two indexable branches. - Add migration 0156 with the plain-key dedup index; the legacy 0068 index stays until Transit support is removed. - Cover it with a JSON dedup test and a Transit -> JSON cross test. AI-assisted-by: deepseek-v4.1-flash |
||
|
Andrey Antukh
|
452f38cf5d
|
✨ Add Prometheus metrics for storage operations (#11700)
* ✨ Add storage operation metrics for S3 and buckets Expose Prometheus metrics for the object storage subsystem. The S3 backend now attaches an AWS SDK MetricPublisher that counts API calls, retries and latency per operation and target. The storage layer counts logical operations and deduplication outcomes per Penpot bucket, and the assets handlers count served requests per route. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Fix storage metrics labels, errors and test gaps Address the review findings on the storage metrics commit. Label reads with the object's own backend, count failed asset serving as errors without swallowing them, and cover the failed S3 call, S3 asset path and permission-denied branches with tests. Also share the label helper and reuse the metrics test helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics and fill test gaps Address the second-round review findings on storage metrics. Unknown backends now fail explicitly and count as errors, exists stays paired with its dedup outcome, and the thumbnail, missing storage, expired reads, unknown buckets and write failure paths are covered by tests. Label coercion goes through the shared metrics helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics accuracy and coverage Address the full-branch review findings on storage metrics. Touch and delete emit only on changed rows, reads emit after the backend fetch, unknown backends fail explicitly, and tempfile mismatches count as unauthorized. Publisher nil policy, pairing rules and attempt semantics are documented and covered by tests. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Address full-branch review findings on storage metrics Touch and delete resolve labels from the row, reads stay paired, failures are covered by tests, and logging, ranges and docs are tightened. Includes the label helper unit tests and the retries wording clarification. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ⚡ Label touch and del metrics from UPDATE RETURNING The storage metrics change resolved metric labels for touch-object! and del-object! with an extra SELECT per id-based call. Since app.main instruments storage unconditionally, every GC collector and binfile import paid that extra round trip: deleting a team with 10k media objects doubled the storage_object statements exactly on the paths that already process the most rows. touch-object! and del-object! now take only the object id (UUID) and read the labels from the updated row itself via RETURNING id, backend, metadata: one statement, no pre-read, and labels that always match the row actually mutated. del-object! additionally guards on deleted_at IS NULL, so a repeated delete returns false and emits no metric. Also from the review of the full branch: extract the duplicated serve/emit/rethrow block in app.http.assets into one helper; give penpot_storage_s3_timing explicit histogram buckets up to 60s (the default cap at 7.5s hid the slow S3 calls the metric exists for); drop the unused ::target-id config key from the S3 backend and hardcode the :default target label until per-bucket routing lands. AI-assisted-by: glm-5.3-flash * ✨ Harden storage metric recording and definitions The metric definition schema is now closed and declares every key the collectors read: buckets, quantiles, max-age and reg. A typo such as a misspelled ::mdef/buckets used to compile and silently fall back to the default histogram buckets; it now fails at startup. The asset result-label fallback coerced an absent status to 500, so a future serve path without a status would have counted successes as errors. The mapping is now explicit and documented: served below 400, unauthorized for 401/403, not-found for 404, and error for everything else, including an absent status. The never-fail try/catch around metric recording existed four times with drift. One app.metrics/run-safe! helper replaces them: it no-ops on a nil metrics instance and logs the first failure per hint at warn level, then at debug, so a broken setup surfaces once without flooding the log. The S3 publisher keeps its outer try/catch: it is the SDK MetricPublisher contract boundary. AI-assisted-by: glm-5.3-flash * ✨ Make metrics mandatory and run! safe by default Recording a metric must never change the behavior of the operation being measured, so `run!` now catches recording failures itself: the first failure per metric id logs at warn, later ones at debug. This replaces the `run-safe!` helper, whose four copies had drifted, and applies the guarantee to every emit site instead of only storage. The metrics instance precondition is a plain assert, and the collector lookup stays outside the recording guard, so a missing instance fails hard even when asserts are disabled. Metrics is therefore no longer optional: the storage, s3-backend and db-pool schemas require `::mtx/metrics`, and the assets handler cfg always carries it. `wrap-publisher` no longer returns nil for a nil instance, and the db pool wires the prometheus tracker unconditionally. AI-assisted-by: deepseek-v4.1-flash |
||
|
elhombretecla
|
a91d81c695 | 🎉 Add link preview metadata for shared links | ||
|
Andrey Antukh
|
87c51090b1 | Merge remote-tracking branch 'origin/staging' into develop | ||
|
Andrey Antukh
|
7079d33ae1
|
🐛 Enforce ownership check on tempfile bucket access (#11270)
* 🐛 Enforce ownership check on tempfile bucket access The upload-tempfile RPC stores profile-id with tempfile objects, but objects-handler never verified the requester was the owner. Any authenticated user who knew the UUID could access the tempfile. Add ownership check: tempfile bucket now requires the request's profile-id to match the stored profile-id. Returns 404 on mismatch (not 403) to avoid leaking object existence. Legacy tempfiles without stored profile-id remain accessible to any authenticated user for backward compatibility. Closes #11269 AI-assisted-by: qwen3.7-plus * ♻️ Extract tempfile-bucket constant and fix docstring indentation Extract the 'tempfile' bucket string literal into a named constant (sto/tempfile-bucket) to prevent typos and make future bucket renames trivial. Updated 9 occurrences across 7 files. Also fixed minor docstring indentation inconsistency in authenticated? function. AI-assisted-by: qwen3.7-plus * ♻️ Refactor process-bucket! and authenticated? helpers Replace case with cond in process-bucket! to properly resolve sto/tempfile-bucket var from another namespace (case does not evaluate qualified vars at compile time). Redefine authenticated? in terms of request-profile-id to remove duplicated lookup logic. Closes #11269 AI-assisted-by: mimo-v2.5-pro |
||
|
Andrey Antukh
|
d655aa9c63 | Merge remote-tracking branch 'origin/staging' into develop | ||
|
Andrey Antukh
|
b79680eeb7
|
🐛 Fix asset 404 for unauthenticated share-link viewers (#11342)
PR #11036 added a per-request permission check to the file-media asset endpoints (/assets/by-file-media-id/:id and the /thumbnail variant) using bfc/get-file-permissions. Anonymous share-link viewers were then rejected because they have neither a session nor an access token, the asset URL carries no share context, and the 2-arg get-file-permissions short-circuits to nil when profile-id is nil. Make the asset endpoints share-link aware, mirroring how get-view-only-bundle already authorizes the same scenario: read the share-id from the query string, validate it as a UUID, and call the 3-arg perms/get-file-read-permissions (which chains the existing 2-arg bfc lookup, the bfc share-link fallback, and the Nitrate org-owner fallback). On the frontend, extend cf/resolve-file-media with an optional share-id arg and pass it from the WASM viewer render path using the share-id already present in [:viewer-local :share-id]. Non-viewer call sites (workspace, clipboard, code-gen) keep the original URL shape because the new arg defaults to nil. Closes #11338 AI-assisted-by: minimax-m3 |
||
|
Andrey Antukh
|
81c3b3cd56
|
📎 Update copyright name on file header (#11346) | ||
|
Jan Kahmen
|
162a381aed
|
🐛 Apply the asset attachment disposition on the s3 backend too (#10989)
29dbf9ab1 marks non public buckets as attachments, which works on the fs backend because nginx applies those headers to the internally redirected response. On the s3 backend the handler answers 307 and the client then fetches the bytes from the object store, so the header set on the redirect does not reach the response that carries the object. Sign the disposition into the presigned url as well, so the object store returns it. It is only signed when the bucket is not public, so urls for inline served objects are unchanged. Also cover the disposition in the handler tests, for the non public buckets and for the public ones that stay inline. Co-authored-by: Andrey Antukh <niwi@niwi.nz> |
||
|
Andrey Antukh
|
25066c2f46
|
🐛 Require file read permissions for asset endpoints (#11036)
Add authorization check to generic-handler in assets.clj so that /assets/by-file-media-id/:id and its /thumbnail variant verify the requesting profile has read access to the parent file. Return 404 (not 403) when access is denied to avoid confirming existence. Also switch get-file-media-object from db/get to db/get* so that non-existent media objects return nil instead of raising. AI-assisted-by: mimo-v2.5-pro |
||
|
María Valderrama
|
66bb6a11df
|
🐛 Make organization logos publicly accessible for invitation emails (#10831) | ||
|
Yamila Moreno
|
ddba2ffa75
|
📎 Update Kaleidos Copyright (#9929) | ||
|
Andrey Antukh
|
ff23f786b4 |
🐛 Fix broken authentication on /assets handlers
- Add ::setup/props and ::db/pool to :app.http.assets/routes config so session renewal works correctly for asset requests. - Add actoken/authz middleware to the assets middleware chain so access tokens are properly recognized. - Add authenticated? helper that checks both ::session/profile-id and ::actoken/profile-id, fixing 401 errors when accessing protected assets with a valid access token. - Add comprehensive test suite for assets auth scenarios. Closes #9677 Signed-off-by: Andrey Antukh <niwi@niwi.nz> |