From 8baa1a014393a857565986e54cd2eb0633ff60e7 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 16 Jun 2026 17:56:27 +0000 Subject: [PATCH] :recycle: Refactor media/process to take system as first argument - Change (defmulti process :cmd) -> (defmulti process (fn [_system params] (:cmd params))) - Change (run params) -> (run system params) - All process methods now receive [system params] - Update all callers: rpc/commands/media, profile, auth, fonts - Revert shell/exec! to require system with executor (no fallback) - Fix lint warnings and formatting Co-authored-by: mimo-v2.5-pro --- backend/src/app/media.clj | 54 +++++++------- backend/src/app/rpc/commands/auth.clj | 2 +- backend/src/app/rpc/commands/fonts.clj | 2 +- backend/src/app/rpc/commands/media.clj | 17 +++-- backend/src/app/rpc/commands/profile.clj | 16 ++--- backend/src/app/util/shell.clj | 69 +++++++++--------- backend/test/backend_tests/media_test.clj | 86 ++++++++++++----------- 7 files changed, 123 insertions(+), 123 deletions(-) diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index ae29d1a132..5219444952 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -89,11 +89,11 @@ max-size))) upload)) -(defmulti process :cmd) +(defmulti process (fn [_system params] (:cmd params))) (defmulti process-error class) (defmethod process :default - [{:keys [cmd] :as params}] + [_system {:keys [cmd] :as params}] (ex/raise :type :internal :code :not-implemented :hint (str/fmt "No impl found for process cmd: %s" cmd))) @@ -103,9 +103,9 @@ (throw error)) (defn run - [params] + [system params] (try - (process params) + (process system params) (catch Throwable e (process-error e)))) @@ -201,13 +201,13 @@ result)) (defn- generic-process - [{:keys [input format convert-args] :as params}] + [system {:keys [input format convert-args] :as params}] (let [{:keys [path mtype]} input format (or format (cm/mtype->format mtype)) ext (cm/format->extension format) tmp (tmp/tempfile :prefix "penpot.media." :suffix ext) args (into [(str path)] (conj (vec convert-args) (str tmp)))] - (exec-magick! nil args) + (exec-magick! system args) (assoc params :format format :mtype (cm/format->mtype format) @@ -215,26 +215,26 @@ :data tmp))) (defmethod process :generic-thumbnail - [params] + [system params] (let [{:keys [quality width height] :as params} (check-thumbnail-params params)] - (generic-process - (assoc params - :convert-args ["-auto-orient" "-strip" - "-thumbnail" (str width "x" height ">") - "-quality" (str quality)])))) + (generic-process system + (assoc params + :convert-args ["-auto-orient" "-strip" + "-thumbnail" (str width "x" height ">") + "-quality" (str quality)])))) (defmethod process :profile-thumbnail - [params] + [system params] (let [{:keys [quality width height] :as params} (check-thumbnail-params params)] - (generic-process - (assoc params - :convert-args ["-auto-orient" "-strip" - "-thumbnail" (str width "x" height "^") - "-gravity" "center" - "-extent" (str width "x" height) - "-quality" (str quality)])))) + (generic-process system + (assoc params + :convert-args ["-auto-orient" "-strip" + "-thumbnail" (str width "x" height "^") + "-gravity" "center" + "-extent" (str width "x" height) + "-quality" (str quality)])))) (defn get-basic-info-from-svg [{:keys [tag attrs] :as data}] @@ -264,11 +264,11 @@ {:width (int width) :height (int height)})))])) -(defn- get-dimensions-with-orientation [^String path] +(defn- get-dimensions-with-orientation [system ^String path] ;; Image magick doesn't give info about exif rotation so we use the identify command ;; If we are processing an animated gif we use the first frame with -scene 0 - (let [dim-result (exec-magick! nil ["identify" "-format" "%w %h\n" path]) - orient-result (exec-magick! nil ["identify" "-format" "%[EXIF:Orientation]\n" path])] + (let [dim-result (exec-magick! system ["identify" "-format" "%w %h\n" path]) + orient-result (exec-magick! system ["identify" "-format" "%[EXIF:Orientation]\n" path])] (when (= 0 (:exit dim-result)) (let [[w h] (-> (:out dim-result) str/trim @@ -283,7 +283,7 @@ {:width w :height h}))))) ; If orientation can't be read, use dimensions as-is (defmethod process :info - [{:keys [input] :as params}] + [system {:keys [input] :as params}] (let [{:keys [path mtype] :as input} (check-input input)] (if (= mtype "image/svg+xml") (let [info (some-> path slurp parse-svg get-basic-info-from-svg)] @@ -294,7 +294,7 @@ (merge input info {:ts (ct/now) :size (fs/size path)})) (let [path-str (str path) - identify-res (exec-magick! nil ["identify" "-format" "image/%[magick]\n" path-str]) + identify-res (exec-magick! system ["identify" "-format" "image/%[magick]\n" path-str]) ;; identify prints one line per frame (animated GIFs, etc.); we take the first one mtype' (if (zero? (:exit identify-res)) (-> identify-res @@ -307,7 +307,7 @@ :code :invalid-image :hint "invalid image")) {:keys [width height]} - (or (get-dimensions-with-orientation path-str) + (or (get-dimensions-with-orientation system path-str) (do (l/warn "Failed to read image dimensions with orientation" {:path path}) (ex/raise :type :validation @@ -398,7 +398,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defmethod process :generate-fonts - [{:keys [input] :as params}] + [_system {:keys [input] :as params}] (letfn [(ttf->otf [data] (let [finput (tmp/tempfile :prefix "penpot.font." :suffix "") foutput (fs/path (str finput ".otf")) diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index 959e96c3e1..5444273862 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -320,7 +320,7 @@ (try (let [storage (sto/resolve cfg) input (media/download-image cfg uri) - input (media/run {:cmd :info :input input}) + input (media/run cfg {:cmd :info :input input}) hash (sto/calculate-hash (:path input)) content (-> (sto/content (:path input) (:size input)) (sto/wrap-with-hash hash)) diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index 252ba48e96..5f031a4583 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -181,7 +181,7 @@ (defn create-font-variant [{:keys [::sto/storage] :as cfg} {:keys [data] :as params}] (letfn [(generate-missing [data] - (let [data (media/run {:cmd :generate-fonts :input data})] + (let [data (media/run cfg {:cmd :generate-fonts :input data})] (when (and (not (contains? data "font/otf")) (not (contains? data "font/ttf")) (not (contains? data "font/woff")) diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index 266abdcbc5..ff8add456a 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -123,11 +123,10 @@ :bucket "file-media-object"})) (defn- process-thumb-image - [info] - (let [thumb (-> thumbnail-options - (assoc :cmd :generic-thumbnail) - (assoc :input info) - (media/run)) + [cfg info] + (let [thumb (media/run cfg (assoc thumbnail-options + :cmd :generic-thumbnail + :input info)) hash (sto/calculate-hash (:data thumb)) data (-> (sto/content (:data thumb) (:size thumb)) (sto/wrap-with-hash hash))] @@ -138,12 +137,12 @@ :bucket "file-media-object"})) (defn- process-image - [content] - (let [info (media/run {:cmd :info :input content})] + [cfg content] + (let [info (media/run cfg {:cmd :info :input content})] (cond-> info (and (not (svg-image? info)) (big-enough-for-thumbnail? info)) - (assoc ::thumb (process-thumb-image info)) + (assoc ::thumb (process-thumb-image cfg info)) :always (assoc ::image (process-main-image info))))) @@ -170,7 +169,7 @@ :path (str (:path content)) :origin origin) - (let [result (process-image content) + (let [result (process-image cfg content) image (sto/put-object! storage (::image result)) thumb (when-let [params (::thumb result)] (sto/put-object! storage params)) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index ba1af4a362..7037c8fab4 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -298,14 +298,14 @@ :file-mtype (:mtype file)}})))) (defn- generate-thumbnail - [_ input] - (let [input (media/run {:cmd :info :input input}) - thumb (media/run {:cmd :profile-thumbnail - :format :jpeg - :quality 85 - :width 256 - :height 256 - :input input}) + [cfg input] + (let [input (media/run cfg {:cmd :info :input input}) + thumb (media/run cfg {:cmd :profile-thumbnail + :format :jpeg + :quality 85 + :width 256 + :height 256 + :input input}) hash (sto/calculate-hash (:data thumb)) content (-> (sto/content (:data thumb) (:size thumb)) (sto/wrap-with-hash hash))] diff --git a/backend/src/app/util/shell.clj b/backend/src/app/util/shell.clj index 197273b759..7ee3e5a697 100644 --- a/backend/src/app/util/shell.clj +++ b/backend/src/app/util/shell.clj @@ -15,8 +15,8 @@ (:import java.io.InputStream java.io.OutputStream - java.util.List java.util.concurrent.TimeUnit + java.util.List org.apache.commons.io.IOUtils)) (set! *warn-on-reflection* true) @@ -51,39 +51,38 @@ (assert (vector? cmd) "a command parameter should be a vector") (assert (every? string? cmd) "the command should be a vector of strings") - (let [executor (or (::wrk/executor system) - (px/cached-executor))] + (let [executor (::wrk/executor system) + _ (assert (some? executor) "executor is required, check ::wrk/executor") + builder (ProcessBuilder. ^List cmd) + env-map (.environment ^ProcessBuilder builder) + _ (reduce-kv set-env env-map env) + process (.start builder)] - (let [builder (ProcessBuilder. ^List cmd) - env-map (.environment ^ProcessBuilder builder) - _ (reduce-kv set-env env-map env) - process (.start builder)] + (if in + (px/run! executor + (fn [] + (with-open [^OutputStream stdin (.getOutputStream ^Process process)] + (io/write stdin in :encoding in-enc)))) + (io/close (.getOutputStream ^Process process))) - (if in - (px/run! executor - (fn [] - (with-open [^OutputStream stdin (.getOutputStream ^Process process)] - (io/write stdin in :encoding in-enc)))) - (io/close (.getOutputStream ^Process process))) - - (with-open [stdout (.getInputStream ^Process process) - stderr (.getErrorStream ^Process process)] - (let [out (px/submit! executor (fn [] (try (read-with-enc stdout out-enc) - (catch java.io.IOException _ "")))) - err (px/submit! executor (fn [] (try (read-as-string stderr) - (catch java.io.IOException _ "")))) - ext (if timeout - (let [completed (.waitFor ^Process process (long timeout) TimeUnit/SECONDS)] - (if completed - (.exitValue ^Process process) - (do - (.destroyForcibly ^Process process) - (ex/raise :type :internal - :code :process-timeout - :hint (str "process timed out after " timeout " seconds") - :cmd cmd - :timeout timeout)))) - (.waitFor ^Process process))] - {:exit ext - :out @out - :err @err}))))) + (with-open [stdout (.getInputStream ^Process process) + stderr (.getErrorStream ^Process process)] + (let [out (px/submit! executor (fn [] (try (read-with-enc stdout out-enc) + (catch java.io.IOException _ "")))) + err (px/submit! executor (fn [] (try (read-as-string stderr) + (catch java.io.IOException _ "")))) + ext (if timeout + (let [completed (.waitFor ^Process process (long timeout) TimeUnit/SECONDS)] + (if completed + (.exitValue ^Process process) + (do + (.destroyForcibly ^Process process) + (ex/raise :type :internal + :code :process-timeout + :hint (str "process timed out after " timeout " seconds") + :cmd cmd + :timeout timeout)))) + (.waitFor ^Process process))] + {:exit ext + :out @out + :err @err})))) diff --git a/backend/test/backend_tests/media_test.clj b/backend/test/backend_tests/media_test.clj index 741c1d671d..85b749274c 100644 --- a/backend/test/backend_tests/media_test.clj +++ b/backend/test/backend_tests/media_test.clj @@ -12,12 +12,14 @@ [clojure.test :as t] [datoteka.fs :as fs])) +(t/use-fixtures :once th/state-init) + (t/deftest info-jpeg (t/testing "info on valid JPEG returns dimensions and mime type" (let [path (th/tempfile "backend_tests/test_files/sample.jpg") - info (media/run {:cmd :info - :input {:path path - :mtype "image/jpeg"}})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/jpeg"}})] (t/is (pos? (:width info))) (t/is (pos? (:height info))) (t/is (= "image/jpeg" (:mtype info))) @@ -27,9 +29,9 @@ (t/deftest info-png (t/testing "info on valid PNG returns dimensions and mime type" (let [path (th/tempfile "backend_tests/test_files/sample.png") - info (media/run {:cmd :info - :input {:path path - :mtype "image/png"}})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/png"}})] (t/is (pos? (:width info))) (t/is (pos? (:height info))) (t/is (= "image/png" (:mtype info)))))) @@ -37,9 +39,9 @@ (t/deftest info-webp (t/testing "info on valid WebP returns dimensions and mime type" (let [path (th/tempfile "backend_tests/test_files/sample.webp") - info (media/run {:cmd :info - :input {:path path - :mtype "image/webp"}})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/webp"}})] (t/is (pos? (:width info))) (t/is (pos? (:height info))) (t/is (= "image/webp" (:mtype info)))))) @@ -47,9 +49,9 @@ (t/deftest info-svg (t/testing "info on valid SVG returns dimensions from viewBox" (let [path (th/tempfile "backend_tests/test_files/sample1.svg") - info (media/run {:cmd :info - :input {:path path - :mtype "image/svg+xml"}})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/svg+xml"}})] (t/is (pos? (:width info))) (t/is (pos? (:height info)))))) @@ -59,9 +61,9 @@ ;; Write garbage data (spit (str path) "not an image") (try - (media/run {:cmd :info - :input {:path path - :mtype "image/jpeg"}}) + (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/jpeg"}}) (t/is false "should have thrown") (catch Exception e (let [data (ex-data e)] @@ -73,15 +75,15 @@ (t/deftest generic-thumbnail (t/testing "generic-thumbnail produces a file of expected format" (let [path (th/tempfile "backend_tests/test_files/sample.jpg") - info (media/run {:cmd :info - :input {:path path - :mtype "image/jpeg"}}) - thumb (media/run {:cmd :generic-thumbnail - :input info - :format :jpeg - :quality 80 - :width 200 - :height 200})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/jpeg"}}) + thumb (media/run th/*system* {:cmd :generic-thumbnail + :input info + :format :jpeg + :quality 80 + :width 200 + :height 200})] (t/is (some? (:data thumb))) (t/is (pos? (:size thumb))) (t/is (= :jpeg (:format thumb))) @@ -92,15 +94,15 @@ (t/deftest profile-thumbnail (t/testing "profile-thumbnail produces a center-cropped file" (let [path (th/tempfile "backend_tests/test_files/sample.jpg") - info (media/run {:cmd :info - :input {:path path - :mtype "image/jpeg"}}) - thumb (media/run {:cmd :profile-thumbnail - :input info - :format :jpeg - :quality 85 - :width 128 - :height 128})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/jpeg"}}) + thumb (media/run th/*system* {:cmd :profile-thumbnail + :input info + :format :jpeg + :quality 85 + :width 128 + :height 128})] (t/is (some? (:data thumb))) (t/is (pos? (:size thumb))) (t/is (= :jpeg (:format thumb))) @@ -111,14 +113,14 @@ (t/deftest generic-thumbnail-webp (t/testing "generic-thumbnail can produce WebP format" (let [path (th/tempfile "backend_tests/test_files/sample.jpg") - info (media/run {:cmd :info - :input {:path path - :mtype "image/jpeg"}}) - thumb (media/run {:cmd :generic-thumbnail - :input info - :format :webp - :quality 80 - :width 200 - :height 200})] + info (media/run th/*system* {:cmd :info + :input {:path path + :mtype "image/jpeg"}}) + thumb (media/run th/*system* {:cmd :generic-thumbnail + :input info + :format :webp + :quality 80 + :width 200 + :height 200})] (t/is (= :webp (:format thumb))) (t/is (= "image/webp" (:mtype thumb))))))