From e9346210d374aa7731b44224493b33de91d21b56 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 9 Oct 2026 07:09:41 +0000 Subject: [PATCH] :sparkles: Declare the export kind instead of sniffing item types Backend `export-assets` params gain optional `:kind` (`:shapes`/`:frames`, declared by the frames dialog); the worker routes on it instead of inferring frames from missing item keys, which `normalize-items` used to overwrite. Frontend freezes fully typed pages plus the kind. The shell moves to the new tree with an injected temp dir, and the dead legacy entry and transit utils go away. AI-assisted-by: muse-spark-1.3-contributor --- .serena/memories/backend/core.md | 2 +- .serena/memories/exporter/core.md | 8 +- backend/src/app/tasks/export_assets.clj | 6 +- .../backend_tests/rpc/commands/jobs_test.clj | 20 ++++ exporter/src/app/core.cljs | 96 ---------------- exporter/src/app/util/transit.cljs | 29 ----- exporter/src/app/wasm/pool.cljs | 2 +- exporter/src/exporter/consumer.cljs | 67 ++++++----- exporter/src/exporter/consumer/worker.cljs | 2 +- exporter/src/exporter/main.cljs | 9 +- exporter/src/exporter/renderer/browser.cljs | 63 +++++------ exporter/src/exporter/shell.cljs | 105 ++++++++++++++++++ exporter/src/exporter/tmpdir.cljs | 21 ++-- .../exporter_tests/browser_scope_test.cljs | 2 + .../test/exporter_tests/consumer_test.cljs | 66 +++++++++-- .../exporter_tests/consumer_worker_test.cljs | 6 +- .../exporter_tests/exporter_shell_test.cljs | 68 ++++++++++++ exporter/test/exporter_tests/main_test.cljs | 14 ++- .../exporter_tests/renderer_browser_test.cljs | 8 +- .../test/exporter_tests/renderer_test.cljs | 2 + exporter/test/exporter_tests/runner.cljs | 2 + .../src/app/main/data/exports/assets.cljs | 13 ++- .../data/exports_assets_test.cljs | 48 ++++++++ 23 files changed, 425 insertions(+), 234 deletions(-) delete mode 100644 exporter/src/app/core.cljs delete mode 100644 exporter/src/app/util/transit.cljs create mode 100644 exporter/src/exporter/shell.cljs create mode 100644 exporter/test/exporter_tests/exporter_shell_test.cljs diff --git a/.serena/memories/backend/core.md b/.serena/memories/backend/core.md index 133d9ca982..a2d67096a4 100644 --- a/.serena/memories/backend/core.md +++ b/.serena/memories/backend/core.md @@ -68,7 +68,7 @@ Every background job is a job-def: a plain `(defn execute-X [cfg params] ...)` i - Two retention knobs, easy to confuse: `:jobs-retention` sweeps terminal internal rows (the ones without a profile), while `:jobs-user-ttl` is the default expiry a user-facing job gets when it is created; the jobs GC deletes the row once `expires_at` passes and touches its resource. The heavy work of a user job runs on its own `:binfile` queue (`:worker-binfile-parallelism`, default 1) so it does not block the default runner. - The quote of a family of jobs (`::export-jobs-per-profile`, `::import-jobs-per-profile`) counts the jobs of the profile that are still on their way (`new`/`scheduled`/`running`/`retry`) whose name the registry gives for that family, so a new job of the family is covered without touching the quote. - The `:exporter` queue has no runner in the backend: its consumer is the exporter process (a separate Node worker; see `mem:exporter/core`). The backend serves it through `app.rpc.management.jobs`, shared-key authenticated (`X-Shared-Key: exporter `, the key the exporter derives from the same secret): `claim-job {job-id scheduled-at}` (exact `scheduled-at` match; a stale payload answers `{:action :skip :status ...}`), `report-job-progress {job-id progress {stage counters}}` (the worker owns the milestone words; the substrate validates the shape), `create-job-session {job-id}` (a real `http_session_v2` of the owner profile, so the assets fetch as the user; TTL `:job-session-ttl` default 1h), `complete-job` (JSON `result`/`resource-id` or multipart `content`/`filename`/`mtype` — allowlist `common.media/export-artifact-types`, cap `:exporter-max-result-size`; the checks run before any bytes are stored and a lost race stores nothing) and `fail-job {job-id error session-id}`. Every method answers `{:action :skip :status ...}` on a row it cannot write and reports the row status, and a settle named with `session-id` closes that render session whatever the write says. -- A user-facing job is created through `app.rpc.commands.jobs`. One command per job type: `create-export-binfile-job`, `create-export-assets-job` and `create-import-binfile-job` name the job they create and their body carries only `:params`, whose schema the command imports from the job namespace (`::sm/params`), so the RPC validates the business params up front, before the command runs; the job-def is still the place that owns them and validates them again at run time. The commands check the permission and the quote of the family before the expensive part — binfile checks the files it reads by id, and assets checks every item on its own: an item carrying `:share-id` validates through the share-link permissions the same way `get-file-fragment` does (a logged viewer of the share is enough; the command stays authenticated, so the anonymous visitor is refused, the same as the exporter cookie it replaces) —, submit with `max-retries 0`, an expiry from `:jobs-user-ttl` and the queue the job-def routes to through `::jobs/queue-name` (`:binfile` when the def says nothing; read by the command, never by the substrate), and answer with a summary of the row. The import command has its own creation schema (`import-binfile/schema:create-params`, version optional) because the request is not the stored params: the command fills the version (from the caller or the package header) and the manifest metadata kept for audit. It assembles the upload, reads the manifest of a version 3 package for the audit (never its content) and stores the package in `job-resource` as the resource of the job, releasing it when the job never comes to own it. +- A user-facing job is created through `app.rpc.commands.jobs`. One command per job type: `create-export-binfile-job`, `create-export-assets-job` and `create-import-binfile-job` name the job they create and their body carries only `:params`, whose schema the command imports from the job namespace (`::sm/params`), so the RPC validates the business params up front, before the command runs; the job-def is still the place that owns them and validates them again at run time. The commands check the permission and the quote of the family before the expensive part — binfile checks the files it reads by id, and assets checks every item on its own: an item carrying `:share-id` validates through the share-link permissions the same way `get-file-fragment` does (a logged viewer of the share is enough; the command stays authenticated, so the anonymous visitor is refused, the same as the exporter cookie it replaces) —, submit with `max-retries 0`, an expiry from `:jobs-user-ttl` and the queue the job-def routes to through `::jobs/queue-name` (`:binfile` when the def says nothing; read by the command, never by the substrate), and answer with a summary of the row. Assets params carry an optional `:kind` (`:shapes`/`:frames`, declared by the frames dialog; absent means shapes): the worker routes on it instead of inferring the export from the items. The import command has its own creation schema (`import-binfile/schema:create-params`, version optional) because the request is not the stored params: the command fills the version (from the caller or the package header) and the manifest metadata kept for audit. It assembles the upload, reads the manifest of a version 3 package for the audit (never its content) and stores the package in `job-resource` as the resource of the job, releasing it when the job never comes to own it. - `get-job` is the read surface a profile has over its own jobs: it answers with the same summary the creation commands return plus the `result` of a finished job or its public error (through `decode-job-error`, so the keywords come back), and a job of another profile is not distinguishable from one that does not exist: both answer `not-found`. `decode-row` decodes `params`, `result` and `error`, and the result is the plain JSON the row holds, so a uuid comes back as the text it is on disk (a result has no schema of its own like the params do). The event log still has no read surface: a client that follows a job takes its progress from the websocket. - `heartbeat` uses named options `[cfg & {:keys [job-id progress] :as options}]` and returns the number of durable writes, so 0 means there was nothing to write: no job context, or the throttle did not allow it. A write that finds the job no longer active (terminal or gone) raises an `:interrupt` (`code :job-interrupted`, carrying the job id and the status it found) through the shared `jobs/check-active` instead of counting zero, so a task in course stops at its next beat; the interrupt is internal and never reaches HTTP (the management progress report answers `:skip` for it). The job id falls back to `::jobs/job-id` on the cfg and then to the runner-bound `*job-id*`. The `modified_at` touch is throttled to ~1s and the progress event to ~200ms, so the check fires at most once per second with no extra query (the read of the row happens only when a write already found nothing). A progress report is a milestone (`schema:progress`): a required `:stage` keyword plus an optional `:counters` map of `scope -> {:current, :total?}`, and no other key. Counters are non-negative, a `total` is positive and never lower than its `current`, and stage and counter keys are names of at most 64 characters. A milestone without counters is valid (a point of the run with no units to count), the stage vocabulary belongs to the worker and not to the substrate, and a client must degrade for a stage it does not know. Read a stored payload back with `jobs/decode-progress`: the database holds the keywords as strings. The 1s beat (down from 60s, so a cancelled heavy job is seen as gone at its next beat) costs one `modified_at` touch per beating job per second: cheap while the only heavy beater runs alone on its queue, revisit if concurrent beating jobs ever grow. - `job.error` conforms to `app.jobs/schema:job-error` (`type` and `code` keywords, `hint` text, extra details allowed). Read it back with `jobs/decode-job-error`; a plain `db/decode-json-pgobject` leaves the keywords as strings. `complete` takes named options and an optional `resource-id`, which is only ever set, never replaced: it reads the row locked inside its own transaction and applies the rules in a fixed order, skipping a job that is no longer completable before refusing one for its resource, so the answer is a function of the row and not of the order two callers arrive in. diff --git a/.serena/memories/exporter/core.md b/.serena/memories/exporter/core.md index 992c796b84..1f7f70f659 100644 --- a/.serena/memories/exporter/core.md +++ b/.serena/memories/exporter/core.md @@ -46,15 +46,15 @@ on this side: the row of the job, its cancel and its result are the backend's. - `exporter.renderer.browser` merges the bitmap/pdf/svg flows into one `render [cfg params on-object check-cancelled]` (branching by type, `on-object` always awaited, unknown types raise); `cfg` carries what the job params do not freeze (`:exporter.browser/pool`, `:base-uri`, `:public-uri`, `:svgo?`). Dead xml predicates from the old tree did not survive the merge. - `exporter.browser.scope` owns the browser batch: `run [cfg exports]` fires every export through its own checkout, all at once, bounded by the pool. No lease — a browser batch shares nothing but the pool, so fan-out only. The facade validates once, partitions by backend and joins both scopes. - `exporter.renderer.wasm` is a thin single-shot over the scope lease. The scope fn survives one level down, as `exporter.wasm.scope/run`. -- `exporter.renderer` is the malli-validated facade with one entry point, the `render-fn` closure over `cfg`: it validates the task map (`:exports :on-object :check-cancelled`) and dispatches it through `render [cfg task]`, which assumes a valid task and only partitions and joins. The `:exporter/renderer` service builds the closure from pools (refs) plus static config. The consumer renders through the injected fn and never requires the facade takes a task map (`:exports :on-object :check-cancelled`) and fires the whole batch at once — the browser exports through `exporter.browser.scope`, the wasm ones through one leased worker — resolving nil once every export landed (objects travel through `on-object`). `cfg` is task-free infrastructure with process lifetime (pools, uris, flags); everything per-task rides in the validated task map. One closed schema validates data plus injections, so a bad task — or a misspelled key — never touches a pool; task validation raises `:data-validation`. Each driver takes the same `[cfg params on-object check-cancelled]` shape (`headless?` is now `wasm?`: the browser also renders headless, so the old name lied) and implements cancel internally: cooperative checkpoints on the browser, checkpoints plus a mid-render abort of its own leased worker on wasm. The lease lives one level down, in `exporter.wasm.scope/run`, which the facade calls with the task keys set once in `cfg`; the browser batch goes through `exporter.browser.scope/run [cfg exports]`. +- `exporter.renderer` is the malli-validated facade with one entry point, the `render-fn` closure over `cfg`: it validates the task map (`:exports :on-object :check-cancelled`) and dispatches it through `render [cfg task]`, which assumes a valid task and only partitions and joins — the browser exports through `exporter.browser.scope`, the wasm ones through one leased worker — resolving nil once every export landed (objects travel through `on-object`). The `:exporter/renderer` service builds the closure from pools plus tmpdir (refs) and static config. The consumer renders through the injected fn and never requires the facade. `cfg` is task-free infrastructure with process lifetime (pools, uris, flags, tmpdir); everything per-task rides in the validated task map. One closed schema validates data plus injections, so a bad task — or a misspelled key — never touches a pool; task validation raises `:data-validation`. Each driver takes the same `[cfg params on-object check-cancelled]` shape (`headless?` is now `wasm?`: the browser also renders headless, so the old name lied) and implements cancel internally: cooperative checkpoints on the browser, checkpoints plus a mid-render abort of its own leased worker on wasm. The lease lives one level down, in `exporter.wasm.scope/run`, which the facade calls with the task keys set once in `cfg`; the browser batch goes through `exporter.browser.scope/run [cfg exports]`. - `exporter.jobs` is the local cancel arm, smaller than the legacy `app.jobs`: one flag per claimed job (`register`/`mark-cancelled` returning true/nil/`cancelled?`/`release`) and one zero-arg check constructor per job (`check-cancelled`, raising `:job-cancelled`). No terminate callbacks and no shared-array signal travel through here — the abort belongs to whoever holds the render (the per-lease interval of `exporter.wasm.scope` terminates its own worker). The runner releases the registry entry on every settle; the legacy runner never did (one leaked entry per job). - `exporter.consumer.plan` is the render plan (`prepare-exports`, `count-objects`, `sanitize-file-regex`), dispatching on `exporter.renderer/wasm?` instead of the legacy `headless?`. -- `exporter.consumer` is the runner: `run-export [cfg claim params]` keeps the legacy beats/watchdog/settle (first breath, render session, multipart complete or `fail-job`, temp release) but renders through the facade (`render [cfg & task]`) with one check from `exporter.jobs` shared between units of work and drivers. `cfg` is the render view; the claim names the job, `params` carries the frozen job params. The settle never rejects; only bugs of this process do. +- `exporter.consumer` is the runner: `run-export [render tmpdir claim params]` keeps the legacy beats/watchdog/settle (first breath, render session, multipart complete or `fail-job`, temp release) but renders through the injected render fn with one check from `exporter.jobs` shared between units of work and drivers. `render` and `tmpdir` are the injected dependencies; the claim names the job, `params` carries the frozen job params. The job kind is declared, never sniffed: `make-plan` reads the top-level `:kind` (`:frames` joins pages into one pdf, anything else runs the shapes flow of single/zip), so there is no typeless-items convention and `normalize-items` only coerces wire strings. The settle never rejects; only bugs of this process do. - `exporter.tmpdir` owns the temp-file area (`:exporter/tmpdir`): the legacy shell creates its dir at namespace load, so referencing the var inside `init-key` pulls that moment into the lifecycle (fail fast on a bad `:tempdir`, ordered before the renderer that writes to it). Named for what it owns, not for the legacy namespace. - `exporter.consumer.api` is the backend management client (claim/progress/session/settle, transit bodies, multipart artifact), a faithful `^:async` port with no promesa chains and the same `fetch` def seam for hermetic tests. - `exporter.consumer.config` holds the consumer knobs (`concurrency` floored at 1, `queue-key`, `queue-name`): the runner, the worker and `exporter.main` read it, so no namespace under `exporter.*` depends on `app.consumer.*` anymore. -- `exporter.consumer.worker` is the queue poller (`read-payload`, `process`, `poll-once`, `start`/`stop` with a running flag, no promesa): K connections over the `:exporter` queue, claim → `run-export` through the injected `:renderer` → settle before the next pop. The worker config carries the queue knobs plus the `:renderer` by ref; no render statics nest inside it. `stop` quits every connection and ends every loop (the legacy loops kept failing turns after the stop); boot cleans orphaned temp files first. -- `exporter.main` wires both pools (`:exporter.browser/pool` and `:exporter.wasm.pool/pool`, each sized to the worker concurrency) plus `:exporter/renderer` (both pools as `ref`s, static uris/flags) plus `:exporter.consumer/worker` (`:renderer` by ref, `:concurrency`, `:queue-key`). Service keys follow the domain (`:exporter.wasm/pool`, `:exporter.consumer/worker`), written `::alias/name` where an alias covers them. No test boots the production wiring: the wasm pool warms one worker eagerly and the worker script in a test process would be the test bundle itself; the wiring is asserted as data and the lifecycle rides fake configs. +- `exporter.consumer.worker` is the queue poller (`read-payload`, `process`, `poll-once`, `start`/`stop` with a running flag, no promesa): K connections over the `:exporter` queue, claim → `run-export` through the injected `:renderer` and `:exporter/tmpdir` → settle before the next pop. The worker config carries the queue knobs plus both by ref; no render statics nest inside it. `stop` quits every connection and ends every loop (the legacy loops kept failing turns after the stop); boot cleans orphaned temp files first. +- `exporter.main` wires both pools (`:exporter.browser/pool` and `:exporter.wasm.pool/pool`, each sized to the worker concurrency) plus `:exporter/tmpdir` (path from env) plus `:exporter/renderer` (pools and tmpdir as `ref`s, static uris/flags) plus `:exporter.consumer/worker` (`:renderer` and `:exporter/tmpdir` by ref, `:concurrency`, `:queue-key`). Service keys follow the domain (`:exporter.wasm/pool`, `:exporter.consumer/worker`), written `::alias/name` where an alias covers them. No test boots the production wiring: the wasm pool warms one worker eagerly and the worker script in a test process would be the test bundle itself; the wiring is asserted as data and the lifecycle rides fake configs. - Async test vars prove their chain only when really awaited: a test passing with 0 assertions means the chain broke upstream. ## Render details diff --git a/backend/src/app/tasks/export_assets.clj b/backend/src/app/tasks/export_assets.clj index 8b46cc8729..11aee01ca8 100644 --- a/backend/src/app/tasks/export_assets.clj +++ b/backend/src/app/tasks/export_assets.clj @@ -50,9 +50,13 @@ (def schema:params "Business params of an asset export: the items to render, frozen when the job was created, plus the options that shape the output. There is - no `wait` here: a job is followed by its id, not waited on." + no `wait` here: a job is followed by its id, not waited on. The + optional `kind` declares the export the caller asked for (`:frames` + joins pages into one pdf); absent it is a shapes export. The worker + reads it as declared instead of inferring it from the items." [:map {:title "export-assets-params" :closed true} [:exports [:vector {:min 1} schema:export-item]] + [:kind {:optional true} [::sm/one-of #{:shapes :frames}]] [:name {:optional true} ::sm/text] [:skip-children {:optional true} ::sm/boolean] [:force-multiple {:optional true} ::sm/boolean] diff --git a/backend/test/backend_tests/rpc/commands/jobs_test.clj b/backend/test/backend_tests/rpc/commands/jobs_test.clj index 5c5e1ba2f9..506ba0e56b 100644 --- a/backend/test/backend_tests/rpc/commands/jobs_test.clj +++ b/backend/test/backend_tests/rpc/commands/jobs_test.clj @@ -274,6 +274,26 @@ params (jobs/decode-params job-def (:params job))] (t/is (= "" (:suffix (first (:exports params))))))))) +(t/deftest create-export-assets-job-accepts-a-frames-kind + ;; the frames dialog freezes fully typed pages plus an explicit kind, + ;; so the worker joins them into one pdf instead of inferring it + ;; from missing keys + (let [profile (th/create-profile* 1) + file-id (first (:file-ids (import-fixture! profile))) + item (assoc (export-item file-id) :type :pdf :scale 1 :suffix "") + out (th/command! {::th/type :create-export-assets-job + ::rpc/profile-id (:id profile) + :params {:exports [item] + :kind :frames}})] + + (t/is (th/success? out)) + + (t/testing "and the kind is frozen as-is for the worker" + (let [job (jobs/get-job th/*system* (:id (:result out))) + job-def (jobs/get-job-def (::jobs/defs th/*system*) :export-assets) + params (jobs/decode-params job-def (:params job))] + (t/is (= :frames (:kind params))))))) + (t/deftest create-export-assets-job-accepts-a-file-nameable-through-a-share ;; the viewer that renders a file through a public share exports the ;; same way every share fetch goes: the share id names the file, the diff --git a/exporter/src/app/core.cljs b/exporter/src/app/core.cljs deleted file mode 100644 index 776de3cafd..0000000000 --- a/exporter/src/app/core.cljs +++ /dev/null @@ -1,96 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS SUBSIDIARY SL - -(ns app.core - (:require - ["node:process" :as proc] - ["node:worker_threads" :as wt] - [app.browser :as bwr] - [app.common.logging :as l] - [app.config :as cf] - [app.consumer.config :as ccfg] - [app.consumer.worker :as worker] - [app.jobs.utils :as job.utils] - [app.wasm :as wasm] - [app.wasm.pool :as wasm.pool] - [app.wasm.worker :as wasm.worker] - [promesa.core :as p])) - -(enable-console-print!) -(l/setup! {:app :info}) - -(defn start - "Render workers run this same bundle, so the thread decides what gets booted: - one render worker, or the single role of the main thread: consuming the - backend's export queue." - [& _] - (if-not ^boolean wt/isMainThread - (wasm.worker/main) - (do - (l/info :msg "initializing" - :public-uri (str (cf/get :public-uri)) - :internal-uri (str (cf/get-internal-uri)) - :workers (ccfg/concurrency) - :version (:full cf/version)) - (l/info :msg "headless wasm export available" - :wasm-dir wasm/artifact-dir - :image-cache-size (cf/get :wasm-worker-image-cache-size)) - (p/do - (bwr/init) - (job.utils/clean-orphans) - (wasm.pool/init) - (worker/start))))) - -(def main start) - -;; Old hot reload path, superseded by exporter.main: the shadow entry -;; point moved to exporter.main/main and the reload hooks ride its -;; ^:dev metadata. Kept visible until app.* is fully ported. -;; Draining a pool waits for every checked-out resource to come back, which an -;; export in flight can hold for as long as its own timeout. On a hot reload -;; that would block `start` from ever running again, leaving a drained pool that -;; fails every later job. -#_(def ^:private shutdown-step-timeout 3000) - -#_(defn- shutdown-step - [label f] - (-> (p/race [(p/do (f)) - (p/fmap (constantly ::timeout) (p/delay shutdown-step-timeout))]) - (p/handle (fn [result cause] - (when (or (some? cause) (= ::timeout result)) - (l/warn :hint "shutdown step did not finish cleanly" - :step label - :cause cause)) - nil)))) - -#_(defn stop - [done] - ;; an empty line for visual feedback of restart - (js/console.log "") - - (if-not ^boolean wt/isMainThread - ;; A render worker owns no server, pools or connections; nothing to unwind. - (done) - (do - (l/info :msg "stopping") - (p/do - (shutdown-step "browser-pool" bwr/stop) - (shutdown-step "wasm-worker-pool" wasm.pool/stop) - ;; the pollers own their connections and may sit in a blocking - ;; pop for the whole poll timeout; draining the pools first lets - ;; the exports in flight unwind against their own resources - (shutdown-step "consumer-worker" worker/stop) - (done))))) - -(.on proc/default "uncaughtException" - (fn [cause] - (js/console.error cause))) - -;; Signals are only delivered to the main thread, and `exit` in a worker would -;; take down that worker rather than the process. -(when ^boolean wt/isMainThread - (.on proc/default "SIGTERM" (fn [] (proc/exit 0))) - (.on proc/default "SIGINT" (fn [] (proc/exit 0)))) diff --git a/exporter/src/app/util/transit.cljs b/exporter/src/app/util/transit.cljs deleted file mode 100644 index 859606e3ca..0000000000 --- a/exporter/src/app/util/transit.cljs +++ /dev/null @@ -1,29 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS SUBSIDIARY SL - -(ns app.util.transit - (:require - [cognitect.transit :as t])) - -;; --- Transit Handlers - -(def ^:private +read-handlers+ - {"u" uuid}) - -(def ^:private +write-handlers+ - {}) - -;; --- Public Api - -(defn decode - [data] - (let [r (t/reader :json {:handlers +read-handlers+})] - (t/read r data))) - -(defn encode - [data] - (let [w (t/writer :json-verbose {:handlers +write-handlers+})] - (t/write w data))) diff --git a/exporter/src/app/wasm/pool.cljs b/exporter/src/app/wasm/pool.cljs index 148132af21..88b7ed6777 100644 --- a/exporter/src/app/wasm/pool.cljs +++ b/exporter/src/app/wasm/pool.cljs @@ -15,7 +15,7 @@ Acquisition is not capped: the admission scheduler is the backpressure, and the idle watchdog guarantees a wedged worker gives its slot back. - Workers run the same bundle as the main thread; `app.core/start` branches on + Workers run the same bundle as the main thread; `exporter.main/start` branches on `isMainThread`. There is always at least one worker, since a headless render has nowhere else to go." (:require diff --git a/exporter/src/exporter/consumer.cljs b/exporter/src/exporter/consumer.cljs index a884b05647..c20fa3736f 100644 --- a/exporter/src/exporter/consumer.cljs +++ b/exporter/src/exporter/consumer.cljs @@ -37,11 +37,11 @@ [app.common.uuid :as uuid] [app.handlers.resources :as rsc] [app.jobs.utils :as job.utils] - [app.util.shell :as sh] [cuerdas.core :as str] [exporter.consumer.api :as api] [exporter.consumer.plan :as plan] - [exporter.jobs :as jobs])) + [exporter.jobs :as jobs] + [exporter.shell :as shell])) (def ^:private report-throttle-ms 250) (def ^:private watchdog-interval-ms 1000) @@ -115,12 +115,6 @@ ;; ---- THE PARAMS -(defn- frame-item? - "The frames export names pages of a file without a `type`: the pdf - of their pages. A shape item always names its type." - [item] - (nil? (:type item))) - (defn- ->uuid "One id of the claim into what the renderer wants: the claim carries the job row JSON, every id a string, while the render spec only @@ -131,10 +125,10 @@ (defn- normalize-items "The items the claim delivered are the plain JSON the job row holds: - the type arrives as text and the renderer dispatches on the keywords - the legacy surface received; every id arrives as text and the render - spec only takes uuid objects. The frames items are pages, not typed - shapes: they name the pdf of a page each." + the type arrives as text and the renderer dispatches on keywords; + every id arrives as text and the render spec only takes uuid objects. + Values that already are uuids or keywords (tests, callers in + process) pass through." [items] (->> items (mapv (fn [item] @@ -143,26 +137,27 @@ (string? (:file-id item)) (update :file-id ->uuid) (string? (:page-id item)) (update :page-id ->uuid) (string? (:object-id item)) (update :object-id ->uuid) - (string? (:share-id item)) (update :share-id ->uuid) - (nil? (:type item)) (assoc :type :pdf :scale 1 :suffix "")))))) + (string? (:share-id item)) (update :share-id ->uuid)))))) (defn- make-plan "The render plan the legacy handlers prepare: the same transducers of names and partition size they served the old surface with, the same `single?` rule (one prepared export with one object, not forced), and the artifact the whole run fills." - [job-id token {:keys [exports force-multiple name skip-children is-wasm]}] + [job-id token {:keys [exports kind force-multiple name skip-children is-wasm]}] (let [items (normalize-items exports) - frames? (boolean (every? frame-item? items)) + ;; declared by the caller, never sniffed: typeless items do not + ;; exist on the validated contract, so there is nothing to infer + frames? (= :frames (keyword kind)) prepared (plan/prepare-exports items token is-wasm) single? (and (not frames?) (not (true? force-multiple)) (= 1 (count prepared)) (= 1 (count (-> prepared first :objects)))) - kind (cond - single? (-> prepared first :type) - frames? :pdf - :else :zip)] + artifact-kind (cond + single? (-> prepared first :type) + frames? :pdf + :else :zip)] {:frames? frames? :single? single? :job-id (->uuid job-id) @@ -171,7 +166,7 @@ :skip-children skip-children :is-wasm (boolean is-wasm) :counter-kind (if frames? :pages :objects) - :resource (rsc/create kind + :resource (rsc/create artifact-kind (or name (-> prepared first :name)))})) ;; ---- THE RUNNERS @@ -204,7 +199,7 @@ (job.utils/track job-id (:path obj)) nil) :check-cancelled check-cancelled})) - (await (sh/move (:path @object) (:path resource))) + (await (shell/move (:path @object) (:path resource))) (check-beat (await (beat :packaging (counter counter-kind 1 1) :force? true))) resource)) @@ -245,17 +240,18 @@ (defn- ^:async join-pdf "The pages render one file per page; `pdfunite` stitches one pdf out of them all, the way the legacy export did." - [job-id file-id paths] + [job-id tmpdir file-id paths] (let [path (job.utils/track job-id - (sh/tempfile :prefix (str/concat "penpot.pdfunite." file-id ".") - :suffix ".pdf"))] - (await (sh/run-cmd "pdfunite" (into [] (concat (vec paths) [path])))) + (shell/tempfile tmpdir + :prefix (str/concat "penpot.pdfunite." file-id ".") + :suffix ".pdf"))] + (await (shell/run-cmd "pdfunite" (into [] (concat (vec paths) [path])))) path)) (defn- ^:async run-frames "The frames render: a file per page, joined into the pdf of the file once every page has landed." - [render beat plan check-cancelled] + [render tmpdir beat plan check-cancelled] (let [{:keys [job-id resource prepared total]} plan file-id (-> prepared first :file-id) paths (volatile! []) @@ -274,16 +270,16 @@ prepared) :on-object on-object :check-cancelled check-cancelled})) - (let [joined (await (join-pdf job-id file-id @paths))] - (await (sh/move joined (:path resource))) + (let [joined (await (join-pdf job-id tmpdir file-id @paths))] + (await (shell/move joined (:path resource))) (check-beat (await (beat :packaging (counter :pages total total) :force? true))) resource))) (defn- run-prepared - [render beat check-cancelled plan] + [render tmpdir beat check-cancelled plan] (cond (:single? plan) (run-single render beat plan check-cancelled) - (:frames? plan) (run-frames render beat plan check-cancelled) + (:frames? plan) (run-frames render tmpdir beat plan check-cancelled) :else (run-multiple render beat plan check-cancelled))) ;; ---- THE SETTLE @@ -324,9 +320,10 @@ "Runs one claimed export to its settle: first breath, render session, plan, render batch, multipart complete. `render` is the injected render fn (one task map per batch: `:exports`, `:on-object`, - `:check-cancelled`); the claim names the job and `params` carries - the frozen job params." - [render {:keys [job-id]} params] + `:check-cancelled`) plus `tmpdir`, the directory every temp path is + created under; the claim names the job and `params` carries the + frozen job params." + [render tmpdir {:keys [job-id]} params] (let [session (atom nil) stop-watchdog (atom nil) _ (jobs/register job-id) @@ -345,7 +342,7 @@ (l/info :hint "render session minted" :job-id (str job-id)) (let [plan (make-plan job-id (:session-token session') params) - resource (await (run-prepared render beat check-cancelled plan))] + resource (await (run-prepared render tmpdir beat check-cancelled plan))] ;; the watchdog outlives the render, so the run stops it on ;; the way out: a run that ends badly stops beating too, or ;; its interval would knock on a dead job forever, one HTTP diff --git a/exporter/src/exporter/consumer/worker.cljs b/exporter/src/exporter/consumer/worker.cljs index 1226e1e893..c439f67187 100644 --- a/exporter/src/exporter/consumer/worker.cljs +++ b/exporter/src/exporter/consumer/worker.cljs @@ -71,7 +71,7 @@ :name (:name answer) :status (:status answer)) (do (l/info :hint "running job" :job-id (str job-id) :name (:name answer)) - (await (consumer/run-export (:renderer cfg) {:job-id job-id} (:params answer))) + (await (consumer/run-export (:renderer cfg) (:exporter/tmpdir cfg) {:job-id job-id} (:params answer))) (l/info :hint "job settled" :job-id (str job-id)) nil)))) diff --git a/exporter/src/exporter/main.cljs b/exporter/src/exporter/main.cljs index 8f33367711..89c73465f4 100644 --- a/exporter/src/exporter/main.cljs +++ b/exporter/src/exporter/main.cljs @@ -43,7 +43,7 @@ {:max (ccfg/concurrency)} :exporter/tmpdir - {} + {:path (cf/get :tempdir)} :exporter/renderer {::browser/pool (system/ref :exporter.browser/pool) @@ -54,9 +54,10 @@ :svgo? (contains? cf/flags :exporter-svgo)} ::consumer/worker - {:renderer (system/ref :exporter/renderer) - :concurrency (ccfg/concurrency) - :queue-key (ccfg/queue-key)}}) + {:renderer (system/ref :exporter/renderer) + :exporter/tmpdir (system/ref :exporter/tmpdir) + :concurrency (ccfg/concurrency) + :queue-key (ccfg/queue-key)}}) ;; The running system map, or nil when nothing is started. ;; Counterpart of the backend's `app.system/system` var. diff --git a/exporter/src/exporter/renderer/browser.cljs b/exporter/src/exporter/renderer/browser.cljs index d31d60a074..f6a0c93067 100644 --- a/exporter/src/exporter/renderer/browser.cljs +++ b/exporter/src/exporter/renderer/browser.cljs @@ -13,7 +13,8 @@ plain value or a promise; it is always awaited. `cfg` is task-free infrastructure: the `:exporter.browser/pool`, the - render `:base-uri` and `:public-uri`, and the `:svgo?` flag. The + render `:base-uri` and `:public-uri`, the `:exporter/tmpdir` area and + the `:svgo?` flag. The per-task injections (`on-object`, `check-cancelled`) ride positional. There are no promesa chains in this namespace." (:require @@ -26,11 +27,11 @@ [app.common.types.color :as ctc] [app.common.uri :as u] [app.util.mime :as mime] - [app.util.shell :as sh] [clojure.walk :as walk] [cuerdas.core :as str] [exporter.browser :as browser] - [exporter.renderer.svg-gradient :as svg-gradient])) + [exporter.renderer.svg-gradient :as svg-gradient] + [exporter.shell :as shell])) (l/set-level! :trace) @@ -68,21 +69,21 @@ ;; --- BITMAP (png, jpeg, webp) (defn- ^:async render-bitmap-object - [page type object on-object] - (let [path (await (sh/tempfile :prefix "penpot.tmp.bitmap." :suffix (mime/get-extension type))) + [tmpdir page type object on-object] + (let [path (await (shell/tempfile tmpdir :prefix "penpot.tmp.bitmap." :suffix (mime/get-extension type))) node (browser/select page (str/concat "#screenshot-" (:id object)))] (await (browser/wait-for node)) (case type :png (await (browser/screenshot node {:omit-background? true :type type :path path})) :jpeg (await (browser/screenshot node {:omit-background? false :type type :path path})) - :webp (let [png-path (await (sh/tempfile :prefix "penpot.tmp.bitmap." :suffix ".png"))] + :webp (let [png-path (await (shell/tempfile tmpdir :prefix "penpot.tmp.bitmap." :suffix ".png"))] ;; playwright only supports jpg and png, we need to convert it afterwards (await (browser/screenshot node {:omit-background? true :type :png :path png-path})) - (await (sh/run-cmd "convert" png-path "-quality" "100" (str "WEBP:" path))))) + (await (shell/run-cmd "convert" png-path "-quality" "100" (str "WEBP:" path))))) (await (on-object (assoc object :path path))))) (defn- ^:async render-bitmap-page - [uri page type objects on-object] + [tmpdir uri page type objects on-object] (l/info :uri uri) ;; navigate to the page and perform basic setup (await (browser/nav page (str uri))) @@ -90,7 +91,7 @@ (await (browser/wait-for-fonts page)) (await (browser/eval page (js* "() => document.body.style.background = 'transparent'"))) ;; take the screnshot of requested objects, one by one - (await (js/Promise.all (mapv (fn [object] (render-bitmap-object page type object on-object)) + (await (js/Promise.all (mapv (fn [object] (render-bitmap-object tmpdir page type object on-object)) objects))) nil) @@ -105,7 +106,7 @@ uri (render-uri (:base-uri cfg) query)] (await (browser/exec (:exporter.browser/pool cfg) (prepare-options uri token scale) - (fn [page] (render-bitmap-page uri page type objects on-object)))))) + (fn [page] (render-bitmap-page (:exporter/tmpdir cfg) uri page type objects on-object)))))) ;; --- PDF @@ -140,9 +141,9 @@ "html, body, #app { margin: 0; padding: 0; width: " width "px; height: " height "px; overflow: visible; }")))))) (defn- ^:async render-pdf-object - [page base-uri params object on-object] + [tmpdir page base-uri params object on-object] (let [uri (prepare-pdf-uri base-uri params (:id object)) - path (await (sh/tempfile :prefix "penpot.tmp.pdf." :suffix (mime/get-extension (:type params))))] + path (await (shell/tempfile tmpdir :prefix "penpot.tmp.pdf." :suffix (mime/get-extension (:type params))))] (l/info :uri uri) (await (browser/nav page uri)) (let [dom (browser/select page (dm/str "#screenshot-" (:id object)))] @@ -155,10 +156,10 @@ (await (on-object (assoc object :path path)))))) (defn- ^:async render-pdf-page - [base-uri page params objects on-object check-cancelled] + [tmpdir base-uri page params objects on-object check-cancelled] (doseq [object objects] (raise-if-cancelled check-cancelled) - (await (render-pdf-object page base-uri params object on-object))) + (await (render-pdf-object tmpdir page base-uri params object on-object))) nil) (defn- ^:async render-pdf @@ -168,7 +169,7 @@ (await (browser/exec (:exporter.browser/pool cfg) (prepare-options base-uri token scale) (fn [page] - (render-pdf-page base-uri page params (:objects params) on-object check-cancelled)))))) + (render-pdf-page (:exporter/tmpdir cfg) base-uri page params (:objects params) on-object check-cancelled)))))) ;; --- SVG @@ -265,14 +266,14 @@ [pngpath] (let [ppmpath (str/concat pngpath "origin.ppm")] (l/trace :fn :convert-to-ppm :path ppmpath) - (await (sh/run-cmd "convert" pngpath ppmpath)) + (await (shell/run-cmd "convert" pngpath ppmpath)) ppmpath)) (defn- ^:async trace-color-mask [pbmpath] (l/trace :fn :trace-color-mask :pbmpath pbmpath) (let [svgpath (str/concat pbmpath ".svg")] - (await (sh/run-cmd "potrace" "--flat" "-b" "svg" pbmpath "-o" svgpath)) + (await (shell/run-cmd "potrace" "--flat" "-b" "svg" pbmpath "-o" svgpath)) svgpath)) (defn- ^:async generate-color-layer @@ -283,10 +284,10 @@ :hint (str "invalid hex color: " color))) (l/trace :fn :generate-color-layer :ppmpath ppmpath :color color) (let [pbmpath (str/concat ppmpath ".mask-" (subs color 1) ".pbm") - stdout (await (sh/run-cmd "ppmcolormask" color ppmpath))] - (await (sh/write-file pbmpath stdout)) + stdout (await (shell/run-cmd "ppmcolormask" color ppmpath))] + (await (shell/write-file pbmpath stdout)) (let [svgpath (await (trace-color-mask pbmpath)) - data (await (sh/read-file svgpath)) + data (await (shell/read-file svgpath)) data (xml->clj data) data (get-in data ["elements" 1])] {:color color @@ -373,11 +374,11 @@ (join-color-layers node layers))) (defn- ^:async trace-node - [node] + [tmpdir node] (l/trace :fn :trace-node) - (let [pngpath (await (sh/tempfile :prefix "penpot.tmp.render.svg.parse." - :suffix ".origin.png"))] - (await (sh/write-file pngpath (:data node))) + (let [pngpath (await (shell/tempfile tmpdir :prefix "penpot.tmp.render.svg.parse." + :suffix ".origin.png"))] + (await (shell/write-file pngpath (:data node))) (let [ppmpath (await (convert-to-ppm pngpath)) svgdata (await (convert-to-svg ppmpath node))] (-> node @@ -419,16 +420,16 @@ [shot node])) (defn- ^:async extract-txt-node - [page item] + [tmpdir page item] (let [[shot node] (await (resolve-text-node page item)) single (await (extract-single-node [shot node]))] - (await (trace-node single)))) + (await (trace-node tmpdir single)))) (defn- ^:async extract-txt-nodes - [page {:keys [id]}] + [tmpdir page {:keys [id]}] (l/trace :fn :process-text-nodes) (let [nodes (await (browser/select-all page (str/concat "#screenshot-" id " foreignObject"))) - nodes (await (js/Promise.all (mapv (partial extract-txt-node page) nodes)))] + nodes (await (js/Promise.all (mapv (partial extract-txt-node tmpdir page) nodes)))] (d/index-by :id nodes))) (defn- ^:async extract-svg @@ -445,11 +446,11 @@ (defn- ^:async render-svg-object [cfg page type object on-object] - (let [path (await (sh/tempfile :prefix "penpot.tmp.render.svg." :suffix (mime/get-extension type))) + (let [path (await (shell/tempfile (:exporter/tmpdir cfg) :prefix "penpot.tmp.render.svg." :suffix (mime/get-extension type))) node (browser/select page (str/concat "#screenshot-" (:id object)))] (await (browser/wait-for node)) (let [xmldata (await (extract-svg page object)) - txtdata (await (extract-txt-nodes page object)) + txtdata (await (extract-txt-nodes (:exporter/tmpdir cfg) page object)) result (replace-text-nodes xmldata txtdata) result (sanitize-svg-content result) @@ -458,7 +459,7 @@ result) result (replace-internal-uris result (:base-uri cfg) (:public-uri cfg))] - (await (sh/write-file path result)) + (await (shell/write-file path result)) (await (on-object (assoc object :path path)))))) (defn- ^:async render-svg-page diff --git a/exporter/src/exporter/shell.cljs b/exporter/src/exporter/shell.cljs new file mode 100644 index 0000000000..fb7ffcab50 --- /dev/null +++ b/exporter/src/exporter/shell.cljs @@ -0,0 +1,105 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns exporter.shell + "Shell and filesystem utilities of the new tree. + + Same operations as the legacy `app.util.shell`, adapted: nothing + runs at namespace load (no log level, no global tmpdir created the + first time anything requires this) and no promesa chains (`^:async` + + `await`). Paths are explicit everywhere: `tempfile` takes the + directory to create under, so the temp area is injected by whoever + owns it — the `:exporter/tmpdir` service at boot — instead of read + from a global. Legacy operations nobody calls did not survive the + move." + (:require + ["node:child_process" :as proc] + ["node:fs" :as fs] + ["node:path" :as path] + [app.common.exceptions :as ex] + [app.common.logging :as l] + [app.common.time :as ct] + [app.common.uuid :as uuid] + [cuerdas.core :as str])) + +(def ^:const default-deletion-delay + (* 60 60 1)) ;; 1h + +(defn ensure-dir + "Creates `dir` (and parents) when missing, returning it. The one + place a directory comes into existence in the new tree; a blank path + is a wiring error, not an empty directory." + [dir] + (when (str/blank? dir) + (throw (ex/error :type :assertion + :code :tmpdir-path-missing + :hint "shell needs a directory to ensure"))) + (when-not (fs/existsSync dir) + (fs/mkdirSync dir #js {:recursive true})) + dir) + +(defn schedule-deletion + ([path] (schedule-deletion path default-deletion-delay)) + ([path delay] + (let [remove-path + (fn [] + (try + (when (fs/existsSync path) + (fs/rmSync path #js {:recursive true}) + (l/trc :hint "tempfile permanently deleted" :path path)) + (catch :default cause + (l/err :hint "error on deleting temporal file" + :path path + :cause cause)))) + scheduled-at + (-> (ct/now) (ct/plus #js {:seconds delay}))] + + (l/trc :hint "schedule tempfile deletion" + :path path + :scheduled-at (ct/format-inst scheduled-at)) + + (js/setTimeout remove-path (* delay 1000)) + path))) + +(defn tempfile + "A fresh path under `dir` that nothing else owns yet, scheduled for + deletion like every other temp file." + [dir & {:keys [prefix suffix] + :or {prefix "penpot." + suffix ".tmp"}}] + (loop [i 0] + (if (< i 1000) + (let [path (path/join dir (str/concat prefix (uuid/next) "-" i suffix))] + (if (fs/existsSync path) + (recur (inc i)) + (schedule-deletion path))) + (throw (ex/error :type :internal + :code :unable-to-locate-temporal-file + :hint "unable to find a tempfile candidate"))))) + +(defn ^:async move + [origin-path dest-path] + (await (.rename fs/promises origin-path dest-path))) + +(defn ^:async write-file + [fpath content] + (await (.writeFile fs/promises fpath content))) + +(defn ^:async read-file + [fpath] + (await (.readFile fs/promises fpath))) + +(defn ^:async run-cmd + [cmd & args] + (await + (js/Promise. + (fn [resolve reject] + (l/trace :fn :run-cmd :cmd cmd :args args) + (proc/execFile cmd (clj->js args) #js {:encoding "buffer"} + (fn [error stdout _stderr] + (if error + (reject error) + (resolve stdout)))))))) diff --git a/exporter/src/exporter/tmpdir.cljs b/exporter/src/exporter/tmpdir.cljs index 6e20a1be40..478b0ae34c 100644 --- a/exporter/src/exporter/tmpdir.cljs +++ b/exporter/src/exporter/tmpdir.cljs @@ -7,19 +7,16 @@ (ns exporter.tmpdir "The temp-file area as a system component (`:exporter/tmpdir`). - The legacy `app.util.shell` creates its `tmpdir` at namespace load: - config read, log line and `mkdirSync` the first time anything - requires it, in every thread including render workers and tests. - This component pulls that moment into the lifecycle: referencing the - var forces the creation during boot (fail fast on a bad `:tempdir`), - orders it before anything that writes temp files, and carries the - path in the instance for the day the temp area becomes injected - instead of global. Named for what it owns, not for the legacy - namespace it shadows." + Its config value names the directory (`{:path ...}`, read from the + environment at the wiring layer); booting ensures it exists and the + running instance is the path itself, which renderers and runners + read straight from their config. Fail fast on a blank path instead + of failing the first render that writes. Named for what it owns, + not for the legacy namespace it shadows." (:require - [app.util.shell :as sh] + [exporter.shell :as shell] [exporter.utils.system :as system])) (defmethod system/init-key :exporter/tmpdir - [_ _] - {:tmpdir sh/tmpdir}) + [_ {:keys [path]}] + (shell/ensure-dir path)) diff --git a/exporter/test/exporter_tests/browser_scope_test.cljs b/exporter/test/exporter_tests/browser_scope_test.cljs index 53c100f28c..b014345ba6 100644 --- a/exporter/test/exporter_tests/browser_scope_test.cljs +++ b/exporter/test/exporter_tests/browser_scope_test.cljs @@ -9,6 +9,7 @@ once, against a stub pool with stub pages." (:require ["generic-pool" :as gp] + ["node:os" :as os] [app.common.uuid :as uuid] [app.config :as cf] [cljs.test :as t :include-macros true] @@ -54,6 +55,7 @@ (defn- test-cfg [calls] {:exporter.browser/pool (stub-pool calls) + :exporter/tmpdir (os/tmpdir) :base-uri (cf/get-internal-uri) :public-uri (cf/get :public-uri) :svgo? false}) diff --git a/exporter/test/exporter_tests/consumer_test.cljs b/exporter/test/exporter_tests/consumer_test.cljs index 3451caac9f..343feca968 100644 --- a/exporter/test/exporter_tests/consumer_test.cljs +++ b/exporter/test/exporter_tests/consumer_test.cljs @@ -11,15 +11,17 @@ failure or a cancel settles as fail-job and never leaks the beat." (:require ["node:fs/promises" :as fsp] + ["node:os" :as os] + ["node:path" :as path] ["undici" :as http] [app.common.transit :as transit] [app.common.uuid :as uuid] - [app.util.shell :as sh] [cljs.test :as t :include-macros true] [cuerdas.core :as cstr] [exporter.consumer :as consumer] [exporter.consumer.api :as api] - [exporter.jobs :as jobs])) + [exporter.jobs :as jobs] + [exporter.shell :as shell])) ;; ---- THE MANAGEMENT FAKE @@ -73,8 +75,8 @@ (js/Promise. (fn [resolve reject] (let [writes (mapv (fn [_] - (let [path (sh/tempfile :prefix "penpot.render." - :suffix ".png")] + (let [path (shell/tempfile (os/tmpdir) :prefix "penpot.render." + :suffix ".png")] (.then (fsp/writeFile path content) (fn [_] (on-object {:path path @@ -123,7 +125,7 @@ {:action :run}))) render (stub-render "rendered!")] (try - (await (consumer/run-export render {:job-id job-id} (export-params))) + (await (consumer/run-export render (os/tmpdir) {:job-id job-id} (export-params))) (t/testing "the run went by first breath, session, beats, settle" (t/is (= ["report-job-progress" "create-job-session" "report-job-progress" "report-job-progress" @@ -152,7 +154,7 @@ share-id (uuid/next)] (try (await (consumer/run-export - render + render (os/tmpdir) {:job-id job-id} {:exports [{:file-id (str (uuid/next)) :page-id (str (uuid/next)) @@ -186,7 +188,7 @@ job-id (uuid/next)] (try (await (consumer/run-export - render + render (os/tmpdir) {:job-id job-id} {:exports [{:file-id (str (uuid/next)) :page-id (str (uuid/next)) @@ -217,7 +219,7 @@ render (stub-render-fail) job-id (uuid/next)] (try - (await (consumer/run-export render {:job-id job-id} (export-params))) + (await (consumer/run-export render (os/tmpdir) {:job-id job-id} (export-params))) (await (js/Promise. (fn [resolve] (js/setTimeout resolve 1400)))) (t/testing "the settle was a fail, not a complete" (t/is (= "fail-job" (last (steps-of))))) @@ -242,7 +244,7 @@ render (stub-render "should never render") job-id (uuid/next)] (try - (await (consumer/run-export render {:job-id job-id} (export-params))) + (await (consumer/run-export render (os/tmpdir) {:job-id job-id} (export-params))) (t/testing "the settle of a cancelled job was a fail the backend answers skip" (t/is (= "fail-job" (last (steps-of)))) (let [fail (call-of "fail-job")] @@ -251,3 +253,49 @@ (t/is false (str "unexpected failure: " (ex-message cause)))) (finally (restore-fetch))))) + +(t/deftest ^:async run-joins-frame-pages-as-pdf + ;; the frames path end to end except pdfunite itself: the page + ;; renders, the join runs in the injected temp area, the artifact + ;; settles as pdf. Frames travel declared (`:kind`), not sniffed: + ;; the items are fully typed pages, as the frozen job holds them. + (let [tmpdir (shell/ensure-dir (path/join (os/tmpdir) "penpot-consumer-frames-test")) + restore-fetch (fake-fetch + (fn [method _body] + (case method + "create-job-session" {:session-id (uuid/next) + :session-token "session-token"} + {:action :run}))) + render (stub-render "page!") + joined (atom nil) + original-cmd shell/run-cmd + job-id (uuid/next)] + (set! shell/run-cmd + (^:async fn [cmd & args] + (let [dest (last (filter string? (flatten args)))] + (reset! joined [cmd dest]) + (await (shell/write-file dest "joined!")) + nil))) + (try + (await (consumer/run-export render tmpdir {:job-id job-id} + {:exports [{:file-id (str (uuid/next)) + :page-id (str (uuid/next)) + :object-id (str (uuid/next)) + :type "pdf" + :name "page" + :suffix "" + :scale 1}] + :kind "frames" + :name "the export"})) + (t/testing "pdfunite joined inside the injected temp area" + (t/is (= "pdfunite" (first @joined))) + (t/is (cstr/starts-with? (second @joined) tmpdir))) + (t/testing "the artifact settled as pdf" + (let [fd (call-of "complete-job")] + (t/is (instance? http/FormData fd)) + (t/is (= "application/pdf" (.get fd "mtype"))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))) + (finally + (restore-fetch) + (set! shell/run-cmd original-cmd))))) diff --git a/exporter/test/exporter_tests/consumer_worker_test.cljs b/exporter/test/exporter_tests/consumer_worker_test.cljs index 0ef000dc5d..6731730bb4 100644 --- a/exporter/test/exporter_tests/consumer_worker_test.cljs +++ b/exporter/test/exporter_tests/consumer_worker_test.cljs @@ -10,6 +10,7 @@ and a fake management http." (:require ["node:fs/promises" :as fsp] + ["node:os" :as os] ["undici" :as http] [app.common.transit :as transit] [app.common.uuid :as uuid] @@ -60,8 +61,9 @@ (defn- test-cfg [extra] - (merge {:concurrency 1 - :queue-key "the.queue.key"} + (merge {:concurrency 1 + :queue-key "the.queue.key" + :exporter/tmpdir (os/tmpdir)} extra)) (t/deftest read-decodes-the-dispatcher-payload diff --git a/exporter/test/exporter_tests/exporter_shell_test.cljs b/exporter/test/exporter_tests/exporter_shell_test.cljs new file mode 100644 index 0000000000..549b56b502 --- /dev/null +++ b/exporter/test/exporter_tests/exporter_shell_test.cljs @@ -0,0 +1,68 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns exporter-tests.exporter-shell-test + "The shell utilities of the new tree: every path explicit, no + global tmpdir, over a scratch area of the test itself." + (:require + ["node:fs" :as fs] + ["node:os" :as os] + ["node:path" :as path] + [cljs.test :as t :include-macros true] + [cuerdas.core :as str] + [exporter.shell :as shell])) + +(defn- scratch-dir + [] + (path/join (os/tmpdir) "penpot-exporter-shell-test")) + +(t/deftest ensure-dir-creates-and-returns + (let [nested (path/join (scratch-dir) "nested" "deeper")] + (t/is (= nested (shell/ensure-dir nested))) + (t/is (true? (fs/existsSync nested))) + (t/testing "a blank path is a wiring error, not an empty directory" + (try + (shell/ensure-dir " ") + (t/is false "ensure-dir should have rejected a blank path") + (catch :default cause + (t/is (= :tmpdir-path-missing (-> cause ex-data :code)))))))) + +(t/deftest tempfile-names-live-under-the-given-dir + (let [dir (shell/ensure-dir (scratch-dir)) + path (shell/tempfile dir :prefix "penpot.test." :suffix ".png")] + (t/is (str/starts-with? path dir)) + (t/is (str/ends-with? path ".png")) + (t/is (str/includes? path "penpot.test.")))) + +(t/deftest ^:async schedule-deletion-removes + (try + (let [dir (shell/ensure-dir (scratch-dir)) + fpath (path/join dir "doomed.tmp")] + (await (shell/write-file fpath "doomed")) + (shell/schedule-deletion fpath 0.05) + (await (js/Promise. (fn [resolve] (js/setTimeout resolve 300)))) + (t/is (false? (fs/existsSync fpath)))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async move-write-and-read-roundtrip + (try + (let [dir (shell/ensure-dir (scratch-dir)) + src (path/join dir "src.tmp") + dest (path/join dir "dest.tmp")] + (await (shell/write-file src "bytes")) + (await (shell/move src dest)) + (t/is (false? (fs/existsSync src))) + (t/is (= "bytes" (str (await (shell/read-file dest)))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async run-cmd-resolves-stdout + (try + (let [out (await (shell/run-cmd "echo" "hello"))] + (t/is (str/includes? (str out) "hello"))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) diff --git a/exporter/test/exporter_tests/main_test.cljs b/exporter/test/exporter_tests/main_test.cljs index 63220513fd..adb13d23b6 100644 --- a/exporter/test/exporter_tests/main_test.cljs +++ b/exporter/test/exporter_tests/main_test.cljs @@ -8,8 +8,11 @@ "The entry point of the new tree: the namespace loads, and start/stop/restart manage the running system." (:require + ["node:os" :as os] + ["node:path" :as path] ["node:process" :as proc] [app.common.logging :as l] + [app.config :as cf] [cljs.test :as t :include-macros true] [exporter.browser :as browser] [exporter.consumer.config :as ccfg] @@ -56,7 +59,7 @@ :base-uri "http://internal/" :public-uri "http://public/" :svgo? false} - :exporter/tmpdir {}}) + :exporter/tmpdir {:path (path/join (os/tmpdir) "penpot-exporter-test")}}) (defn- pool-of [] @@ -108,7 +111,11 @@ (let [worker (:exporter.consumer/worker main/system-config)] (t/is (= (ccfg/concurrency) (:concurrency worker))) (t/is (= (ccfg/queue-key) (:queue-key worker))) - (t/is (= (system/ref :exporter/renderer) (:renderer worker)))))) + (t/is (= (system/ref :exporter/renderer) (:renderer worker))) + (t/is (= (system/ref :exporter/tmpdir) (:exporter/tmpdir worker))))) + (t/testing "the temp area is wired from the environment" + (t/is (= {:path (cf/get :tempdir)} + (:exporter/tmpdir main/system-config))))) ;; NOTE: no test boots the production wiring: the wasm pool warms one ;; worker eagerly (min 1), and in this process the worker script would @@ -124,7 +131,8 @@ (t/testing "the renderer boots as a render fn over the resolved pools" (t/is (fn? (:exporter/renderer @main/system)))) (t/testing "the temp area boots before anything that writes to it" - (t/is (string? (:tmpdir (:exporter/tmpdir @main/system))))) + (t/is (= (path/join (os/tmpdir) "penpot-exporter-test") + (:exporter/tmpdir @main/system)))) (t/testing "the wasm pool rides the same lifecycle" (t/is (some? (:pool (wasm-pool-of)))) (t/is (= 300000 (:timeout-ms (wasm-pool-of))))) diff --git a/exporter/test/exporter_tests/renderer_browser_test.cljs b/exporter/test/exporter_tests/renderer_browser_test.cljs index 14dc93c310..791a0e827d 100644 --- a/exporter/test/exporter_tests/renderer_browser_test.cljs +++ b/exporter/test/exporter_tests/renderer_browser_test.cljs @@ -10,13 +10,14 @@ shell-outs; the exported file itself proves the reassembly." (:require ["generic-pool" :as gp] + ["node:os" :as os] [app.common.uuid :as uuid] [app.config :as cf] - [app.util.shell :as sh] [cljs.test :as t :include-macros true] [clojure.string :as str] [exporter.renderer.browser :as render] - [exporter.renderer.svg-gradient :as grad])) + [exporter.renderer.svg-gradient :as grad] + [exporter.shell :as shell])) (defn- stub-locator [calls evaluate-fn] @@ -65,6 +66,7 @@ (defn- test-cfg [pool] {:exporter.browser/pool pool + :exporter/tmpdir (os/tmpdir) :base-uri (cf/get-internal-uri) :public-uri (cf/get :public-uri) :svgo? false}) @@ -184,7 +186,7 @@ {:keys [seen fn]} (collect-on-object) _ (await (render/render (test-cfg pool) (test-params :svg ["a"]) fn (never-cancelled))) path (:path (first @seen)) - content (await (sh/read-file path))] + content (await (shell/read-file path))] (t/is (= 1 (count @seen))) (t/is (str/ends-with? path ".svg")) (t/is (str/includes? content "")) diff --git a/exporter/test/exporter_tests/renderer_test.cljs b/exporter/test/exporter_tests/renderer_test.cljs index df07ffaf03..058b5ce8bd 100644 --- a/exporter/test/exporter_tests/renderer_test.cljs +++ b/exporter/test/exporter_tests/renderer_test.cljs @@ -11,6 +11,7 @@ (:require ["generic-pool" :as gp] ["node:events" :as events] + ["node:os" :as os] [app.common.transit :as transit] [app.common.uuid :as uuid] [app.config :as cf] @@ -93,6 +94,7 @@ [calls] (let [worker (stub-worker calls)] {:exporter.browser/pool (stub-browser-pool calls) + :exporter/tmpdir (os/tmpdir) :base-uri (cf/get-internal-uri) :public-uri (cf/get :public-uri) :svgo? false diff --git a/exporter/test/exporter_tests/runner.cljs b/exporter/test/exporter_tests/runner.cljs index a2d29a8773..15fdddf303 100644 --- a/exporter/test/exporter_tests/runner.cljs +++ b/exporter/test/exporter_tests/runner.cljs @@ -21,6 +21,7 @@ [exporter-tests.exporter-consumer-config-test] [exporter-tests.exporter-consumer-plan-test] [exporter-tests.exporter-jobs-test] + [exporter-tests.exporter-shell-test] [exporter-tests.jobs-test] [exporter-tests.main-test] [exporter-tests.renderer-browser-test] @@ -54,6 +55,7 @@ 'exporter-tests.exporter-consumer-config-test 'exporter-tests.exporter-consumer-plan-test 'exporter-tests.exporter-jobs-test + 'exporter-tests.exporter-shell-test 'exporter-tests.jobs-test 'exporter-tests.main-test 'exporter-tests.api-test diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index 404179e6d7..0ee9ef3e8b 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -139,7 +139,12 @@ :file-id file-id :object-id (:id frame) :shape frame - :name (:name frame)}) + :name (:name frame) + ;; pages render as one pdf: the kind + ;; travels declared, the items fully typed + :type :pdf + :scale 1 + :suffix ""}) frames)] (rx/of (modal/show :export-frames @@ -322,12 +327,15 @@ A creation that could not even happen (the export queue saturated, mostly) reports on the widget and, unless it is saturation, rethrows for the global error handling of the store." - [ws-conn exports cmd {:keys [force-multiple name is-wasm]}] + [ws-conn exports cmd {:keys [force-multiple name is-wasm kind]}] (let [params (cond-> {:exports exports :is-wasm is-wasm} (some? name) (assoc :name name) + (some? kind) + (assoc :kind kind) + (some? force-multiple) (assoc :force-multiple force-multiple))] @@ -403,6 +411,7 @@ (->> (export-stream! ws-conn exports cmd {:force-multiple true :name name + :kind (if (= :export-frames cmd) :frames :shapes) :is-wasm (wasm-export-enabled? state)})))))))) (defn request-export diff --git a/frontend/test/frontend_tests/data/exports_assets_test.cljs b/frontend/test/frontend_tests/data/exports_assets_test.cljs index fd308cf9b0..f9ac2f1f30 100644 --- a/frontend/test/frontend_tests/data/exports_assets_test.cljs +++ b/frontend/test/frontend_tests/data/exports_assets_test.cljs @@ -200,6 +200,54 @@ :the/end))) done)))) +(t/deftest request-multiple-export-declares-the-frames-kind + ;; A frames run names its kind up front: the dialog freezes fully + ;; typed pages plus `:kind :frames`, so the worker joins them into + ;; one pdf instead of inferring it from missing keys. + (t/async done + (let [items [{:page-id (uuid/next) + :file-id (uuid/next) + :object-id (uuid/next) + :name "page" + :type :pdf + :scale 1 + :suffix ""}] + observed (atom nil)] + (mock/with-mocks + {repo/cmd! (mock/stub (fn [cmd params] + (reset! observed {:cmd cmd :params (:params params)}) + (rx/of fake-job))) + dj/watch-job (mock/stub (fn [_ _] + (fake-watch + [(milestone :rendering 1 1 :pages) + (completed-emission {:resource-uri "https://resources/uri" + :filename "file.pdf" + :mtype "application/pdf"})]))) + dom/trigger-download-uri (mock/stub (fn [& _] nil)) + st/ongoing-tasks (atom #{})} + (fn [done'] + (let [completed (fn [state] + (t/testing "the creation declared the frames run" + (t/is (= :create-export-assets-job (:cmd @observed))) + (t/is (= :frames (get-in @observed [:params :kind]))) + (t/is (= [{:page-id (:page-id (first items)) + :file-id (:file-id (first items)) + :object-id (:object-id (first items)) + :name "page" + :type :pdf + :scale 1 + :suffix ""}] + (get-in @observed [:params :exports])))) + (t/testing "the widget followed the job" + (t/is (false? (get-in state [:export :in-progress]))) + (t/is (= "ended" (get-in state [:export :status])))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-multiple-export {:exports items + :cmd :export-frames + :name "the export"}) + :the/end))) + done)))) + (t/deftest request-multiple-export-reports-the-saturation ;; The creation refused because the queue is full: an answer, not a ;; fault — the widget names it and offers the retry.