From b22cce46f11cebef75782f72f8b0edcae2e13fb5 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 7 Oct 2026 13:42:20 +0000 Subject: [PATCH] :bug: Parse the claim strings into renderer uuids The claim carries the job row JSON: every id a string. The run handed them to the renderer as-is, and the render spec only takes uuid objects, so every export died on `render-params`. The run now parses the item ids at the normalize boundary, threads the job-id through the plan (it never arrived there: the render saw nil), and omits an absent share-id instead of naming nil. AI-assisted-by: glm-5.3-flash --- exporter/src/app/consumer/exports.cljs | 25 ++++++-- exporter/src/app/consumer/plan.cljs | 18 +++--- exporter/test/exporter_tests/worker_test.cljs | 64 +++++++++++++++---- 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/exporter/src/app/consumer/exports.cljs b/exporter/src/app/consumer/exports.cljs index f8e4e63aec..447213723e 100644 --- a/exporter/src/app/consumer/exports.cljs +++ b/exporter/src/app/consumer/exports.cljs @@ -31,6 +31,7 @@ (:require [app.common.exceptions :as ex] [app.common.logging :as l] + [app.common.uuid :as uuid] [app.consumer.api :as api] [app.consumer.plan :as plan] [app.handlers.resources :as rsc] @@ -121,24 +122,37 @@ [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 + takes uuid objects. Values that already are uuids (tests, callers in + process) pass through." + [v] + (if (string? v) (uuid/uuid v) v)) + (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. The frames items are pages, not typed + 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." [items] (->> items (mapv (fn [item] (cond-> item - (string? (:type item)) (update :type keyword) - (nil? (:type item)) (assoc :type :pdf :scale 1 :suffix "")))))) + (string? (:type item)) (update :type keyword) + (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 "")))))) (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." - [token {:keys [exports force-multiple name skip-children is-wasm]}] + [job-id token {:keys [exports force-multiple name skip-children is-wasm]}] (let [items (normalize-items exports) frames? (boolean (every? frame-item? items)) prepared (plan/prepare-exports items token is-wasm) @@ -152,6 +166,7 @@ :else :zip)] {:frames? frames? :single? single? + :job-id (->uuid job-id) :prepared prepared :total (plan/count-objects prepared) :skip-children skip-children @@ -342,7 +357,7 @@ beats! (:beat! beats) stop! (with-watchdog! beats! (:last beats)) _ (check-beat! (beats! :preparing {} :force? true)) - plan (make-plan token params) + plan (make-plan job-id token params) resource (run-prepared! beats! cancelled plan) _ (stop!) artifact {:path (str (:path resource)) diff --git a/exporter/src/app/consumer/plan.cljs b/exporter/src/app/consumer/plan.cljs index dec8b7e714..7c2c1b5c61 100644 --- a/exporter/src/app/consumer/plan.cljs +++ b/exporter/src/app/consumer/plan.cljs @@ -65,14 +65,16 @@ group))) (build-render [[part1 :as part]] - {:file-id (:file-id part1) - :page-id (:page-id part1) - :share-id (:share-id part1) - :name (:name part1) - :token token - :type (:type part1) - :scale (:scale part1) - :objects (mapv part-entry->object part)}) + (cond-> {:file-id (:file-id part1) + :page-id (:page-id part1) + :name (:name part1) + :token token + :type (:type part1) + :scale (:scale part1) + :objects (mapv part-entry->object part)} + ;; a nil share-id is not an absent one for the render + ;; spec: only name it when the item came through one + (some? (:share-id part1)) (assoc :share-id (:share-id part1)))) (part-entry->object [entry] {:id (:object-id entry) diff --git a/exporter/test/exporter_tests/worker_test.cljs b/exporter/test/exporter_tests/worker_test.cljs index 5d677fb460..0b6bae2633 100644 --- a/exporter/test/exporter_tests/worker_test.cljs +++ b/exporter/test/exporter_tests/worker_test.cljs @@ -76,18 +76,22 @@ (defn- fake-render! "Installs a fake renderer that writes one file of `content` per - object and calls back with it. Returns the restore thunk." - [content] - (let [original rd/render] - (set! rd/render - (fn [_params on-object] - (p/let [rendered-path (sh/tempfile :prefix "penpot.render." - :suffix ".png") - _ (fsp/writeFile rendered-path content)] - (on-object {:path rendered-path - :filename "rendered.png"}) - (p/resolved nil)))) - (fn [] (set! rd/render original)))) + object and calls back with it. When `captured` is an atom, it also + keeps the params of the last render. Returns the restore thunk." + ([content] (fake-render! content nil)) + ([content captured] + (let [original rd/render] + (set! rd/render + (fn [params on-object] + (when (some? captured) + (reset! captured params)) + (p/let [rendered-path (sh/tempfile :prefix "penpot.render." + :suffix ".png") + _ (fsp/writeFile rendered-path content)] + (on-object {:path rendered-path + :filename "rendered.png"}) + (p/resolved nil)))) + (fn [] (set! rd/render original))))) (defn- fake-render-fail! "A renderer that always fails: the failure path of the run." @@ -162,6 +166,42 @@ (t/testing "a corrupt payload is dropped, not thrown at the loop" (t/is (nil? (worker/read-payload! "{not a payload"))))) +(t/deftest process-hands-the-renderer-uuids-not-claim-strings + ;; the claim carries the job row JSON: every id a string. The render + ;; spec wants uuid objects (and a nil job-id never renders), so the + ;; run parses them before the first render. + (t/async done + (let [job-id (uuid/next) + sid (uuid/next) + share-id (uuid/next) + params {:exports [{:file-id (str (uuid/next)) + :page-id (str (uuid/next)) + :object-id (str (uuid/next)) + :share-id (str share-id) + :type "png" + :name "test shape" + :suffix "" + :scale 1}] + :name "the export"} + stages (volatile! []) + captured (atom nil) + respond! (fake-fetch! (answer-with params sid stages)) + restore! (fake-render! "rendered!" captured)] + (p/let [_ (worker/process! {:job-id (str job-id) + :scheduled-at "2026-10-07T09:00:00Z"})] + (let [render-params @captured] + (t/testing "every id arrived as a uuid object" + (t/is (uuid? (:file-id render-params))) + (t/is (uuid? (:page-id render-params))) + (t/is (uuid? (:share-id render-params))) + (t/is (= share-id (:share-id render-params))) + (t/is (uuid? (-> render-params :objects first :id)))) + (t/testing "and the job-id traveled with the render" + (t/is (= job-id (:job-id render-params))))) + (respond!) + (restore!) + (done))))) + (t/deftest poll-once-awaits-what-it-claims (t/async done (let [queue (atom [(encoded-claim (uuid/next) "2026-10-07T09:00:00Z")])