diff --git a/.gitignore b/.gitignore index a0fb7f7f5a..76da22b35f 100644 --- a/.gitignore +++ b/.gitignore @@ -101,5 +101,6 @@ opencode.json /.opencode/plans /.opencode/reports /.opencode/prompts +/.ci-logs /.codex/ /tools/__pycache__ diff --git a/.serena/memories/testing.md b/.serena/memories/testing.md index ce4990ba12..295da86212 100644 --- a/.serena/memories/testing.md +++ b/.serena/memories/testing.md @@ -137,17 +137,32 @@ E2E tests should not be added unless explicitly requested. ## Execution discipline -When running CLJS/JS tests (frontend, common): +**CRITICAL: Test output handling rules** +When running ANY test command (CLJS/JS or JVM): + +1. **NEVER pipe test output directly to `| head`, `| tail`, `| grep`, or similar filters** — this can hide failures and cause you to miss critical errors. +2. **ALWAYS pipe to a file first, then read the file:** + ```bash + # CORRECT: + pnpm run test 2>&1 > /tmp/test-output.txt + grep -A 5 "failures" /tmp/test-output.txt + + # WRONG: + pnpm run test 2>&1 | tail -20 + pnpm run test 2>&1 | grep "failures" + ``` +3. **Use `--focus` to narrow test scope** instead of filtering output. +4. **Read the full output file** to understand test results completely. + +When running CLJS/JS tests (frontend, common): - **Always use `pnpm run test:quiet`** — it silently builds the test bundle then runs the test runner, giving you clean test output. -- **Never pipe test output through `tail`, `head`, or similar filters** — doing so can silently hide test failures. Use `--focus` to narrow scope instead. -- **If you need to filter output, tee to a temp file first:** `pnpm run test:quiet 2>&1 | tee /tmp/penpot-test-output.txt`. The full output is preserved on disk so you can `grep`/`tail`/`head` the file without re-running. - Use `pnpm run test` when you want to see build output alongside test results (always builds, then runs). - After `build:test` has been run once, you can invoke the runner directly: `node target/tests/test.js [--focus ...] [--log-level ...]`. When running JVM tests (backend, common): - Use `clojure -M:dev:test` directly (no pnpm wrapper). -- The same no-piping rule applies: use `--focus` to narrow scope. +- Same file-piping rule applies. ## Verification Checklist diff --git a/.serena/memories/workflow/creating-prs.md b/.serena/memories/workflow/creating-prs.md index 00252a7661..13bde56149 100644 --- a/.serena/memories/workflow/creating-prs.md +++ b/.serena/memories/workflow/creating-prs.md @@ -30,7 +30,7 @@ See `mem:workflow/creating-commits` for emoji codes. Squash merge uses the PR ti Include concise sections covering: - what changed and why; -- related GitHub issues or Taiga stories (`Fixes #NNNN`, `Relates to #NNNN`, `Taiga #NNNN`); +- related GitHub issues or Taiga stories (`Closes #NNNN`, `Relates to #NNNN`, `Taiga #NNNN`); - screenshots or recordings for UI-visible changes; - testing performed and residual risk; - breaking changes or migration notes, if any. diff --git a/AGENTS.md b/AGENTS.md index b6aae1f32d..05284c1c73 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,4 +109,5 @@ precision while maintaining a strong focus on maintainability and performance. - `scripts/nrepl-eval.mjs` — Evaluate Clojure code via nREPL (backend + frontend). - `scripts/check-commit` — Validate commit messages against Penpot's commit guidelines. - `scripts/check-fmt-clj` — Check Clojure formatting without modifying files. +- `scripts/ci` — CI orchestration script for running lint, tests, and format checks across modules. See `scripts/ci --help`. diff --git a/backend/src/app/http/middleware.clj b/backend/src/app/http/middleware.clj index 81b1ef13ec..fa2faa8a55 100644 --- a/backend/src/app/http/middleware.clj +++ b/backend/src/app/http/middleware.clj @@ -65,12 +65,25 @@ :else request))) + ;; The specific-exception branches below (IAE, + ;; RequestTooBigException, EOFException) raise with + ;; `ex/raise` rather than calling `errors/handle` directly. + ;; This is intentional: the throw is caught by the + ;; top-level error handler in `app.http/router-handler` + ;; (`backend/src/app/http.clj`), which routes every + ;; uncaught exception through `errors/handle`. The + ;; per-route `wrap-errors` middleware in the route list + ;; is a defensive layer; correctness does not depend on + ;; it. Raising here keeps the cond uniform with the + ;; existing RequestTooBigException / EOFException + ;; branches. (handle-error [cause request] (cond - (instance? RuntimeException cause) - (if-let [cause (ex-cause cause)] - (handle-error cause request) - (errors/handle cause request)) + (instance? IllegalArgumentException cause) + (ex/raise :type :validation + :code :malformed-json + :hint (ex-message cause) + :cause cause) (instance? RequestTooBigException cause) (ex/raise :type :validation @@ -83,6 +96,11 @@ :hint (ex-message cause) :cause cause) + (instance? RuntimeException cause) + (if-let [cause (ex-cause cause)] + (handle-error cause request) + (errors/handle cause request)) + :else (errors/handle cause request)))] diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 3851ea577b..4daa2dd32a 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -156,11 +156,13 @@ (assoc mfile :permissions perms))) (defn get-file-etag - [{:keys [::rpc/profile-id]} {:keys [modified-at revn vern permissions]}] + [{:keys [::rpc/profile-id]} {:keys [modified-at revn vern deleted-at permissions]}] (str profile-id "/" revn "/" vern "/" (hash fmg/available-migrations) "/" (ct/format-inst modified-at :iso) "/" - (uri/map->query-string permissions))) + (uri/map->query-string permissions) + "/" + (some-> deleted-at (ct/format-inst :iso)))) (sv/defmethod ::get-file "Retrieve a file by its ID. Only authenticated users." @@ -1102,6 +1104,13 @@ (check-edition-permissions! conn profile-id file-id) (check-edition-permissions! conn profile-id library-id) + + (let [transitive-deps (bfc/get-libraries cfg [library-id])] + (when (contains? transitive-deps file-id) + (ex/raise :type :validation + :code :circular-library-reference + :hint "linking this library would create a circular dependency"))) + (link-file-to-library conn params) (bfc/get-libraries cfg [library-id])) diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index 130d9a86be..024bce17e7 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -374,61 +374,6 @@ ;; --- MUTATION COMMAND: create-file-thumbnail -(defn- create-file-thumbnail - [{:keys [::db/conn ::sto/storage] :as cfg} {:keys [file-id revn props media] :as params}] - (media/validate-media-type! media) - (media/validate-media-size! media) - - (let [file (bfc/get-file cfg file-id - :include-deleted? true - :load-data? false) - - props (db/tjson (or props {})) - path (:path media) - mtype (:mtype media) - hash (sto/calculate-hash path) - data (-> (sto/content path) - (sto/wrap-with-hash hash)) - tnow (ct/now) - - media (sto/put-object! storage - {::sto/content data - ::sto/deduplicate? true - ::sto/touched-at tnow - :content-type mtype - :bucket "file-thumbnail"}) - - thumb (db/get* conn :file-thumbnail - {:file-id file-id - :revn revn} - {::db/remove-deleted false - ::sql/for-update true})] - - (if (some? thumb) - (do - ;; We mark the old media id as touched if it does not match - (when (not= (:id media) (:media-id thumb)) - (sto/touch-object! storage (:media-id thumb))) - - (db/update! conn :file-thumbnail - {:media-id (:id media) - :deleted-at (:deleted-at file) - :updated-at tnow - :props props} - {:file-id file-id - :revn revn})) - - (db/insert! conn :file-thumbnail - {:file-id file-id - :revn revn - :created-at tnow - :updated-at tnow - :deleted-at (:deleted-at file) - :props props - :media-id (:id media)})) - - media)) - (def ^:private schema:create-file-thumbnail [:map {:title "create-file-thumbnail"} @@ -448,12 +393,57 @@ ::rtry/when rtry/conflict-exception? ::sm/params schema:create-file-thumbnail} - ;; FIXME: do not run the thumbnail upload inside a transaction - [cfg {:keys [::rpc/profile-id file-id] :as params}] - (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (files/check-edition-permissions! conn profile-id file-id) - (when-not (db/read-only? conn) - (let [media (create-file-thumbnail cfg params)] - {:uri (files/resolve-public-uri (:id media)) - :id (:id media)}))))) + (media/validate-media-type! (:media params)) + (media/validate-media-size! (:media params)) + + (db/run! cfg files/check-edition-permissions! profile-id file-id) + + (when-not (db/read-only? (::db/pool cfg)) + (let [storage (::sto/storage cfg) + file (bfc/get-file cfg file-id :include-deleted? true :load-data? false) + props (db/tjson (or (:props params) {})) + {:keys [path mtype]} (:media params) + hash (sto/calculate-hash path) + data (-> (sto/content path) + (sto/wrap-with-hash hash)) + tnow (ct/now) + + media (sto/put-object! storage + {::sto/content data + ::sto/deduplicate? true + ::sto/touched-at tnow + :content-type mtype + :bucket "file-thumbnail"}) + + revn (:revn params) + + result (db/tx-run! cfg + (fn [{:keys [::db/conn]}] + (let [thumb (db/get* conn :file-thumbnail + {:file-id file-id :revn revn} + {::db/remove-deleted false + ::sql/for-update true})] + (if (some? thumb) + (do + (when (not= (:id media) (:media-id thumb)) + (sto/touch-object! storage (:media-id thumb))) + (db/update! conn :file-thumbnail + {:media-id (:id media) + :deleted-at (:deleted-at file) + :updated-at tnow + :props props} + {:file-id file-id :revn revn})) + (db/insert! conn :file-thumbnail + {:file-id file-id + :revn revn + :created-at tnow + :updated-at tnow + :deleted-at (:deleted-at file) + :props props + :media-id (:id media)})) + media)))] + + (when result + {:uri (files/resolve-public-uri (:id result)) + :id (:id result)})))) diff --git a/backend/src/app/rpc/commands/search.clj b/backend/src/app/rpc/commands/search.clj index 796ddf4810..7b60e6db30 100644 --- a/backend/src/app/rpc/commands/search.clj +++ b/backend/src/app/rpc/commands/search.clj @@ -6,9 +6,11 @@ (ns app.rpc.commands.search (:require + [app.common.data.macros :as dm] [app.common.schema :as sm] [app.db :as db] [app.rpc :as-alias rpc] + [app.rpc.commands.teams :as teams] [app.rpc.doc :as-alias doc] [app.util.services :as sv])) @@ -66,11 +68,13 @@ (def ^:private schema:search-files [:map {:title "search-files"} [:team-id ::sm/uuid] - [:search-term {:optional true} :string]]) + [:search-term {:optional true} [:string {:max 250}]]]) (sv/defmethod ::search-files {::doc/added "1.17" ::doc/module :files ::sm/params schema:search-files} [{:keys [::db/pool]} {:keys [::rpc/profile-id team-id search-term]}] - (some->> search-term (search-files pool profile-id team-id))) + (dm/with-open [conn (db/open pool)] + (teams/check-read-permissions! conn profile-id team-id) + (some->> search-term (search-files conn profile-id team-id)))) diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index f751d8aca6..bd986fc031 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -21,19 +21,73 @@ [clojure.test :as t] [mockery.core :refer [with-mocks]] [yetti.request :as yreq] - [yetti.response :as yres])) + [yetti.response :as yres]) + (:import + io.undertow.server.RequestTooBigException)) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) -(defrecord DummyRequest [headers cookies] +(defrecord DummyRequest [headers cookies method body-stream + remote-addr server-name server-port + scheme protocol path query ssl-client-cert] yreq/IRequestCookies (get-cookie [_ name] {:value (get cookies name)}) yreq/IRequest (get-header [_ name] - (get headers name))) + (get headers name)) + (method [_] method) + (body [_] body-stream) + (path [_] path) + (query [_] query) + (server-port [_] server-port) + (server-name [_] server-name) + (remote-addr [_] remote-addr) + (ssl-client-cert [_] ssl-client-cert) + (scheme [_] scheme) + (protocol [_] protocol)) + +(defn- make-dummy-request + "Constructs a DummyRequest from an options map. Every key is + optional; missing values fall back to sensible defaults. New + fields added to DummyRequest won't break existing call sites + as long as this constructor keeps its `:or` defaults in sync. + + Recognized keys: + :headers — map of header name → value + :cookies — map of cookie name → value + :method — HTTP method keyword (default :get) + :body-stream — InputStream for the body (used directly) + :body-bytes — bytes or string for the body; wrapped in a + ByteArrayInputStream if :body-stream is not + given + :remote-addr — string (default \"127.0.0.1\") + :server-name — string (default \"test\") + :server-port — long (default 0) + :scheme — keyword (default :http) + :protocol — string (default \"HTTP/1.1\") + :path — string (default \"/test\") + :query — string or nil (default nil) + :ssl-client-cert — X509Certificate or nil (default nil)" + [{:keys [headers cookies method body-stream body-bytes + remote-addr server-name server-port scheme protocol + path query ssl-client-cert] + :or {headers {} cookies {} method :get + body-stream nil + remote-addr "127.0.0.1" server-name "test" server-port 0 + scheme :http protocol "HTTP/1.1" path "/test" query nil + ssl-client-cert nil}}] + (let [body-stream (or body-stream + (when body-bytes + (java.io.ByteArrayInputStream. + (if (string? body-bytes) + (.getBytes ^String body-bytes "UTF-8") + body-bytes))))] + (->DummyRequest headers cookies method body-stream + remote-addr server-name server-port + scheme protocol path query ssl-client-cert))) (t/deftest auth-middleware-1 (let [request (volatile! nil) @@ -41,11 +95,11 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {"authorization" "Token aaaa"} {})) + (handler (make-dummy-request {:headers {"authorization" "Token aaaa"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :token token-type)) @@ -58,10 +112,10 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {"authorization" "Bearer aaaa"} {})) + (handler (make-dummy-request {:headers {"authorization" "Bearer aaaa"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :bearer token-type)) @@ -74,10 +128,10 @@ (fn [req] (vreset! request req)) {})] - (handler (->DummyRequest {} {})) + (handler (make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) - (handler (->DummyRequest {} {"auth-token" "foobar"})) + (handler (make-dummy-request {:cookies {"auth-token" "foobar"}})) (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] (t/is (= :cookie token-type)) @@ -89,16 +143,16 @@ (fn [req] {::yres/status 200}) {:test1 "secret-key"})] - (let [response (handler (->DummyRequest {} {}))] + (let [response (handler (make-dummy-request {}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "secret-key2"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "secret-key2"}}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "secret-key"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "secret-key"}}))] (t/is (= 403 (::yres/status response)))) - (let [response (handler (->DummyRequest {"x-shared-key" "test1 secret-key"} {}))] + (let [response (handler (make-dummy-request {:headers {"x-shared-key" "test1 secret-key"}}))] (t/is (= 200 (::yres/status response)))))) (t/deftest access-token-authz @@ -209,7 +263,7 @@ :user-agent "user agent"}) (#'session/assign-token cfg)) - response (handler (->DummyRequest {} {"auth-token" (:token session)})) + response (handler (make-dummy-request {:cookies {"auth-token" (:token session)}})) {:keys [token claims] token-type :type} (get response ::http/auth-data)] @@ -220,3 +274,127 @@ (t/is (= "penpot" (:aud claims))) (t/is (= (:id session) (:sid claims))) (t/is (= (:id profile) (:uid claims))))) + +(t/deftest parse-request-illegal-argument-exception + ;; clojure.data.json raises IllegalArgumentException (case + ;; fall-through) on several kinds of malformed input. The + ;; parse-request middleware should convert any such IAE into a + ;; 400 :malformed-json validation error rather than letting it + ;; surface as a 500 internal error. Because the conversion is + ;; done by raising an ex-info (caught by the top-level error + ;; handler in app.http/router-handler), this test asserts on + ;; the ex-info thrown by wrap-parse-request directly. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] {::yres/status 200 ::yres/body :ok})) + ;; Body contains the bytes for: {"x": "\}"} -- a string + ;; value with a backslash followed by '}', which + ;; clojure.data.json v0.5.x cannot handle. + body (.getBytes "{\"x\": \"\\}\"}" "UTF-8") + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes body}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-request-too-big-exception + ;; When RequestTooBigException is raised (e.g. the request body + ;; exceeded the configured size limit), the middleware should + ;; convert it to a 413 :request-body-too-large validation + ;; error. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (RequestTooBigException. "too large")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :request-body-too-large (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-eof-exception + ;; When java.io.EOFException is raised (e.g. the body stream + ;; was closed before the parser could read it), the middleware + ;; should convert it to a 400 :malformed-json validation error. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (java.io.EOFException. "stream closed")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))) + (t/is (string? (-> ex ex-data :hint))))) + +(t/deftest parse-request-runtime-exception-with-cause + ;; When a RuntimeException with a non-nil ex-cause is raised, + ;; the middleware should recurse on the cause and dispatch + ;; through the specific-exception branches. Here we wrap an + ;; IllegalArgumentException in a RuntimeException and verify + ;; it surfaces as :malformed-json. + (let [iae (IllegalArgumentException. "No matching clause: 99") + wrapped (doto (RuntimeException. "wrapped") + (.initCause iae)) + handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw wrapped))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + ex (try + (handler request) + (catch clojure.lang.ExceptionInfo e e))] + (t/is (instance? clojure.lang.ExceptionInfo ex)) + (t/is (= :validation (-> ex ex-data :type))) + (t/is (= :malformed-json (-> ex ex-data :code))))) + +(t/deftest parse-request-runtime-exception-without-cause + ;; When a bare RuntimeException (no ex-cause) is raised, the + ;; middleware should fall through to errors/handle's :default + ;; path and return a 500 with :type :server-error :code + ;; :unexpected. This is the "true internal error" path. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (RuntimeException. "boom")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + response (handler request) + body (::yres/body response)] + (t/is (= 500 (::yres/status response))) + (t/is (= :server-error (:type body))) + (t/is (= :unexpected (:code body))) + (t/is (= "boom" (:hint body))))) + +(t/deftest parse-request-non-runtime-throwable + ;; When a non-RuntimeException Throwable is raised (e.g. an + ;; Error subclass or a non-RuntimeException checked-style + ;; exception), the middleware should fall through to the + ;; :else branch and call errors/handle. java.io.IOException + ;; has a dedicated handle-exception method that returns 500 + ;; with :code :io-exception. + (let [handler (#'app.http.middleware/wrap-parse-request + (fn [_] (throw (java.io.IOException. "network gone")))) + request (make-dummy-request + {:method :post + :headers {"content-type" "application/json"} + :body-bytes (.getBytes "{}" "UTF-8")}) + response (handler request) + body (::yres/body response)] + (t/is (= 500 (::yres/status response))) + (t/is (= :server-error (:type body))) + (t/is (= :io-exception (:code body))) + (t/is (= "network gone" (:hint body))))) diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 5460b4143f..1c07f35971 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -2319,3 +2319,75 @@ (t/is (not (nil? (:error out)))) (let [edata (-> out :error ex-data)] (t/is (= :not-found (:type edata)))))) + +;; --- Security Fix Tests --- + +(t/deftest link-file-to-library-circular-reference + (let [profile (th/create-profile* 1) + file1 (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true}) + file2 (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true}) + file3 (th/create-file* 3 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false})] + (th/link-file-to-library* {:file-id (:id file3) :library-id (:id file2)}) + (th/link-file-to-library* {:file-id (:id file2) :library-id (:id file1)}) + (let [data {::th/type :link-file-to-library + ::rpc/profile-id (:id profile) + :file-id (:id file1) + :library-id (:id file3)} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (let [edata (-> out :error ex-data)] + (t/is (= :circular-library-reference (:code edata))))))) + +(t/deftest get-file-etag-includes-deleted-at + (let [profile-id (uuid/random) + file1 {:modified-at (ct/now) + :revn 1 + :vern 0 + :deleted-at nil + :permissions {:can-edit true}} + file2 (assoc file1 :deleted-at (ct/now))] + (t/is (not= (files/get-file-etag {::rpc/profile-id profile-id} file1) + (files/get-file-etag {::rpc/profile-id profile-id} file2))))) + +(t/deftest search-files-with-permission + (let [profile (th/create-profile* 1) + _ (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + data {::th/type :search-files + ::rpc/profile-id (:id profile) + :team-id (:default-team-id profile) + :search-term "test"} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (vector? (:result out))))) + +(t/deftest search-files-forbidden + (let [profile (th/create-profile* 1) + other (th/create-profile* 2) + data {::th/type :search-files + ::rpc/profile-id (:id other) + :team-id (:default-team-id profile) + :search-term "test"} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (let [edata (-> out :error ex-data)] + (t/is (= :not-found (:type edata)))))) + +(t/deftest search-files-term-too-long + (let [profile (th/create-profile* 1) + data {::th/type :search-files + ::rpc/profile-id (:id profile) + :team-id (:default-team-id profile) + :search-term (apply str (repeat 300 "x"))} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (let [edata (-> out :error ex-data)] + (t/is (= :validation (:type edata)))))) diff --git a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc index e6aef398f0..b434d99fc9 100644 --- a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc @@ -121,74 +121,79 @@ (defn layout-content-points [bounds parent children objects] - (let [parent-id (dm/get-prop parent :id) - parent-bounds @(get bounds parent-id) - reverse? (ctl/reverse? parent) - children (cond->> children (not reverse?) reverse)] + (let [parent-id (dm/get-prop parent :id) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [reverse? (ctl/reverse? parent) + children (cond->> children (not reverse?) reverse)] - (loop [children (seq children) - result (transient []) - correct-v (gpt/point 0)] + (loop [children (seq children) + result (transient []) + correct-v (gpt/point 0)] - (if (not children) - (persistent! result) + (if (not children) + (persistent! result) - (let [child (first children) - child-id (dm/get-prop child :id) - child-bounds @(get bounds child-id) - [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) + (let [child (first children) + child-id (dm/get-prop child :id) + child-bounds-ref (get bounds child-id) + child-bounds (some-> child-bounds-ref deref) + [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) - [child-bounds correct-v] - (if (or (ctl/fill-width? child) (ctl/fill-height? child)) - (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) - [(->> child-bounds (map #(gpt/add % correct-v))) correct-v]) + [child-bounds correct-v] + (if (and child-bounds + (or (ctl/fill-width? child) (ctl/fill-height? child))) + (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) + [(when child-bounds + (->> child-bounds (map #(gpt/add % correct-v)))) + correct-v]) - child-bounds - (when (d/not-empty? child-bounds) - (-> (gpo/parent-coords-bounds child-bounds parent-bounds) - (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] + child-bounds + (when (d/not-empty? child-bounds) + (-> (gpo/parent-coords-bounds child-bounds parent-bounds) + (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] - (recur (next children) - (cond-> result (some? child-bounds) (conj! child-bounds)) - correct-v)))))) + (recur (next children) + (cond-> result (some? child-bounds) (conj! child-bounds)) + correct-v)))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} children objects] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [row? (ctl/row? parent) + col? (ctl/col? parent) + space-around? (ctl/space-around? parent) + space-evenly? (ctl/space-evenly? parent) + content-evenly? (ctl/content-evenly? parent) + [layout-gap-row layout-gap-col] (ctl/gaps parent) - row? (ctl/row? parent) - col? (ctl/col? parent) - space-around? (ctl/space-around? parent) - space-evenly? (ctl/space-evenly? parent) - content-evenly? (ctl/content-evenly? parent) - [layout-gap-row layout-gap-col] (ctl/gaps parent) + row-pad (if (or (and col? space-evenly?) + (and col? space-around?) + (and row? content-evenly?)) + layout-gap-row + 0) - row-pad (if (or (and col? space-evenly?) - (and col? space-around?) - (and row? content-evenly?)) - layout-gap-row - 0) + col-pad (if (or (and row? space-evenly?) + (and row? space-around?) + (and col? content-evenly?)) + layout-gap-col + 0) - col-pad (if (or (and row? space-evenly?) - (and row? space-around?) - (and col? content-evenly?)) - layout-gap-col - 0) + {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (+ (or pad-top 0) row-pad) + pad-right (+ (or pad-right 0) col-pad) + pad-bottom (+ (or pad-bottom 0) row-pad) + pad-left (+ (or pad-left 0) col-pad) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (+ (or pad-top 0) row-pad) - pad-right (+ (or pad-right 0) col-pad) - pad-bottom (+ (or pad-bottom 0) row-pad) - pad-left (+ (or pad-left 0) col-pad) + layout-points + (layout-content-points bounds parent children objects)] - layout-points - (layout-content-points bounds parent children objects)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc index 0628cf2060..caadff4766 100644 --- a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc @@ -12,36 +12,36 @@ (defn layout-content-points [bounds parent {:keys [row-tracks column-tracks]}] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) - - hv #(gpo/start-hv parent-bounds %) - vv #(gpo/start-vv parent-bounds %)] - (d/concat-vec - (->> row-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (vv (:size %)))))) - (->> column-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (hv (:size %))))))))) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [hv #(gpo/start-hv parent-bounds %) + vv #(gpo/start-vv parent-bounds %)] + (d/concat-vec + (->> row-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (vv (:size %)))))) + (->> column-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (hv (:size %))))))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} layout-data] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [{pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (or pad-top 0) + pad-right (or pad-right 0) + pad-bottom (or pad-bottom 0) + pad-left (or pad-left 0) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (or pad-top 0) - pad-right (or pad-right 0) - pad-bottom (or pad-bottom 0) - pad-left (or pad-left 0) + layout-points (layout-content-points bounds parent layout-data)] - layout-points (layout-content-points bounds parent layout-data)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/min_size_layout.cljc b/common/src/app/common/geom/shapes/min_size_layout.cljc index 89f17871e3..57375098e9 100644 --- a/common/src/app/common/geom/shapes/min_size_layout.cljc +++ b/common/src/app/common/geom/shapes/min_size_layout.cljc @@ -31,13 +31,17 @@ (and (ctl/fill-width? child) (ctl/grid-layout? child)) - (let [children - (->> (cfh/get-immediate-children objects (:id child)) - (remove ctl/position-absolute?) - (map #(vector @(get bounds (:id %)) %))) - layout-data (gd/calc-layout-data child @(get bounds (:id child)) children bounds objects true)] - (max (ctl/child-min-width child) - (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (let [child-bounds-ref (get bounds (:id child))] + (if child-bounds-ref + (let [children + (->> (cfh/get-immediate-children objects (:id child)) + (remove ctl/position-absolute?) + (keep #(when-let [b (get bounds (:id %))] + [@b %]))) + layout-data (gd/calc-layout-data child @child-bounds-ref children bounds objects true)] + (max (ctl/child-min-width child) + (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (ctl/child-min-width child))) (ctl/fill-width? child) (ctl/child-min-width child) @@ -63,11 +67,15 @@ (let [children (->> (cfh/get-immediate-children objects (dm/get-prop child :id)) (remove ctl/position-absolute?) - (map (fn [child] [@(get bounds (:id child)) child]))) + (keep (fn [c] + (when-let [b (get bounds (:id c))] + [@b c])))) layout-data (gd/calc-layout-data child (:points child) children bounds objects true) auto-bounds (gb/layout-content-bounds bounds child layout-data)] - (max (ctl/child-min-height child) - (gpo/height-points auto-bounds))) + (if auto-bounds + (max (ctl/child-min-height child) + (gpo/height-points auto-bounds)) + (ctl/child-min-height child))) (ctl/fill-height? child) (ctl/child-min-height child) diff --git a/common/test/common_tests/geom_bounds_layout_nil_test.cljc b/common/test/common_tests/geom_bounds_layout_nil_test.cljc new file mode 100644 index 0000000000..db070ce4f4 --- /dev/null +++ b/common/test/common_tests/geom_bounds_layout_nil_test.cljc @@ -0,0 +1,213 @@ +;; 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 INC Sucursal en España SL + +(ns common-tests.geom-bounds-layout-nil-test + (:require + [app.common.data :as d] + [app.common.geom.bounds-map :as gbm] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.geom.shapes.flex-layout.bounds :as fb] + [app.common.geom.shapes.grid-layout.bounds :as gb] + [app.common.geom.shapes.min-size-layout :as msl] + [app.common.types.shape :as cts] + [app.common.types.shape.layout :as ctl] + [app.common.uuid :as uuid] + [clojure.test :as t])) + +;; ---- Helpers ---- + +(defn- make-rect + [id x y w h] + (-> (cts/setup-shape {:id id + :type :rect + :name (str "rect-" id) + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero))) + +(defn- make-flex-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "flex-" id) + :layout :flex + :layout-flex-dir dir + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids)))) + +(defn- make-grid-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (let [cell-id (uuid/next)] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "grid-" id) + :layout :grid + :layout-grid-dir dir + :layout-grid-columns [{:type :flex :value 1}] + :layout-grid-rows [{:type :flex :value 1}] + :layout-grid-cells + {cell-id {:id cell-id + :row 1 + :row-span 1 + :column 1 + :column-span 1 + :shapes (vec child-ids)}} + :layout-padding-type :multiple + :layout-padding {:p1 0 :p2 0 :p3 0 :p4 0} + :layout-gap {:column-gap 0 :row-gap 0} + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids))))) + +(defn- make-objects + [shapes] + (let [shape-map (into {} (map (fn [s] [(:id s) s]) shapes))] + (reduce-kv (fn [m _id shape] + (if (contains? shape :shapes) + (reduce (fn [m' child-id] + (assoc-in m' [child-id :parent-id] (:id shape))) + m + (:shapes shape)) + m)) + shape-map + shape-map))) + +(defn- bounds-map-from-objects + "Build a bounds map from objects, optionally excluding some IDs." + [objects & {:keys [exclude-ids]}] + (let [full (gbm/objects->bounds-map objects)] + (if (seq exclude-ids) + (apply dissoc full exclude-ids) + full))) + +;; ---- Tests for flex layout bounds with nil bounds ---- + +(t/deftest layout-content-points-with-missing-parent-bounds + (t/testing "layout-content-points returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-points bounds parent [child] objects)))))) + +(t/deftest layout-content-points-with-missing-child-bounds + (t/testing "layout-content-points skips children with missing bounds" + (let [child1-id (uuid/next) + child2-id (uuid/next) + parent-id (uuid/next) + child1 (make-rect child1-id 10 10 50 50) + child2 (make-rect child2-id 70 10 50 50) + parent (make-flex-frame parent-id [child1-id child2-id]) + objects (make-objects [parent child1 child2]) + bounds (bounds-map-from-objects objects :exclude-ids #{child1-id})] + + (let [result (fb/layout-content-points bounds parent [child1 child2] objects)] + (t/is (some? result)) + ;; Only child2's bounds should be in the result + (t/is (pos? (count result))))))) + +(t/deftest layout-content-bounds-with-missing-parent-bounds + (t/testing "layout-content-bounds returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-bounds bounds parent [child] objects)))))) + +;; ---- Tests for grid layout bounds with nil bounds ---- + +(t/deftest grid-layout-content-points-with-missing-parent-bounds + (t/testing "grid layout-content-points returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-points bounds parent layout-data)))))) + +(t/deftest grid-layout-content-bounds-with-missing-parent-bounds + (t/testing "grid layout-content-bounds returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-bounds bounds parent layout-data)))))) + +;; ---- Tests for min-size-layout with nil bounds ---- + +(t/deftest child-min-width-grid-with-missing-child-bounds + (t/testing "child-min-width falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :row + :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + ;; Exclude grandchild from bounds to simulate missing entry + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-width child child-bounds bounds objects)] + (t/is (= (ctl/child-min-width child) result)))))) + +(t/deftest child-min-height-grid-with-missing-child-bounds + (t/testing "child-min-height falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :column + :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-height child child-bounds bounds objects)] + (t/is (= (ctl/child-min-height child) result)))))) + +(t/deftest child-min-width-grid-with-present-child-bounds + (t/testing "child-min-width handles bounded children in a fill-width grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-width child child-bounds bounds objects)))))) + +(t/deftest child-min-height-grid-with-present-child-bounds + (t/testing "child-min-height handles bounded children in a fill-height grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-height child child-bounds bounds objects)))))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 7738c5dd9e..d3d2f7d48e 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -22,6 +22,7 @@ [common-tests.files.shapes-builder-test] [common-tests.files.validate-test] [common-tests.geom-align-test] + [common-tests.geom-bounds-layout-nil-test] [common-tests.geom-bounds-map-test] [common-tests.geom-flex-layout-test] [common-tests.geom-grid-layout-test] @@ -95,6 +96,7 @@ 'common-tests.files-migrations-test 'common-tests.files.validate-test 'common-tests.geom-align-test + 'common-tests.geom-bounds-layout-nil-test 'common-tests.geom-bounds-map-test 'common-tests.geom-flex-layout-test 'common-tests.geom-grid-layout-test diff --git a/frontend/packages/draft-js/index.js b/frontend/packages/draft-js/index.js index 400636c5dd..f02109f971 100644 --- a/frontend/packages/draft-js/index.js +++ b/frontend/packages/draft-js/index.js @@ -29,6 +29,7 @@ function isDefined(v) { } function mergeBlockData(block, newData) { + if (!block) return undefined; let data = block.getData(); for (let key of Object.keys(newData)) { @@ -176,10 +177,12 @@ export function splitBlockPreservingData(state) { content = Modifier.splitBlock(content, selection); - const blockData = content.blockMap.get(content.selectionBefore.getStartKey()).getData(); + const startKey = content.selectionBefore.getStartKey(); + const block = content.blockMap.get(startKey); + const blockData = (block && block.getData()) || new Map(); const blockKey = content.selectionAfter.getStartKey(); - const blockMap = content.blockMap.update(blockKey, (block) => { - return block.set("data", blockData); + const blockMap = content.blockMap.update(blockKey, (b) => { + return b.set("data", blockData); }); content = content.set("blockMap", blockMap); @@ -325,6 +328,7 @@ export function updateBlockData(state, blockKey, data) { const content = state.getCurrentContent(); const block = content.getBlockForKey(blockKey); const newBlock = mergeBlockData(block, data); + if (!newBlock) return state; const blockData = newBlock.getData(); diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index 24d72e5e92..b3adaebade 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -20,10 +20,27 @@ [app.util.dom :as dom] [app.util.websocket :as ws] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) (def default-timeout 5000) +(defn normalize-export + [{:keys [object-id name] :as export}] + (assoc export :name (if (str/blank? name) + (str object-id) + name))) + +(defn- normalize-exports + [exports] + (mapv normalize-export exports)) + +(defn- normalize-export-shapes-params + [{:keys [exports] :as params}] + (cond-> params + (seq exports) + (assoc :exports (normalize-exports exports)))) + (defn toggle-detail-visibililty [] (ptk/reify ::toggle-detail-visibililty @@ -181,104 +198,106 @@ (defn request-simple-export [{:keys [export]}] - (ptk/reify ::request-simple-export - ptk/UpdateEvent - (update [_ state] - (cond-> state - (not (use-wasm-export? state export)) - (update :export assoc :in-progress true :id uuid/zero))) + (let [export (normalize-export export)] + (ptk/reify ::request-simple-export + ptk/UpdateEvent + (update [_ state] + (cond-> state + (not (use-wasm-export? state export)) + (update :export assoc :in-progress true :id uuid/zero))) - ptk/WatchEvent - (watch [_ state _] - (if (use-wasm-export? state export) - (do - (case (:type export) - :pdf (wasm.exports/export-pdf export) - (wasm.exports/export-image export)) - (rx/empty)) - (let [profile-id (:profile-id state) - params {:exports [export] - :profile-id profile-id - :cmd :export-shapes - :wait true - :is-wasm (wasm-export-enabled? state)}] - (rx/concat - (dwp/force-persist-and-wait 400) + ptk/WatchEvent + (watch [_ state _] + (if (use-wasm-export? state export) + (do + (case (:type export) + :pdf (wasm.exports/export-pdf export) + (wasm.exports/export-image export)) + (rx/empty)) + (let [profile-id (:profile-id state) + params (normalize-export-shapes-params {:exports [export] + :profile-id profile-id + :cmd :export-shapes + :wait true + :is-wasm (wasm-export-enabled? state)})] + (rx/concat + (dwp/force-persist-and-wait 400) - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [filename mtype uri]}] - (dom/trigger-download-uri filename mtype uri) - (clear-export-state uuid/zero))) - (rx/catch (fn [cause] - (rx/concat - (rx/of (clear-export-state uuid/zero)) - (rx/throw cause))))))))))) + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [filename mtype uri]}] + (dom/trigger-download-uri filename mtype uri) + (clear-export-state uuid/zero))) + (rx/catch (fn [cause] + (rx/concat + (rx/of (clear-export-state uuid/zero)) + (rx/throw cause)))))))))))) (defn request-multiple-export [{:keys [exports cmd name] :or {cmd :export-shapes} :as params}] - (ptk/reify ::request-multiple-export - ptk/WatchEvent - (watch [_ state _] - (let [resource-id (volatile! nil) - profile-id (:profile-id state) - ws-conn (:ws-conn state) - params (cond-> - {:exports exports - :cmd cmd - :profile-id profile-id - :force-multiple true - :is-wasm (wasm-export-enabled? state)} - (some? name) - (assoc :name name)) + (let [exports (normalize-exports exports)] + (ptk/reify ::request-multiple-export + ptk/WatchEvent + (watch [_ state _] + (let [resource-id (volatile! nil) + profile-id (:profile-id state) + ws-conn (:ws-conn state) + params (cond-> + {:exports exports + :cmd cmd + :profile-id profile-id + :force-multiple true + :is-wasm (wasm-export-enabled? state)} + (some? name) + (assoc :name name)) - progress-stream - (->> (ws/get-rcv-stream ws-conn) - (rx/filter ws/message-event?) - (rx/map :payload) - (rx/filter #(= :export-update (:type %))) - (rx/filter #(= @resource-id (:resource-id %))) - (rx/share)) + progress-stream + (->> (ws/get-rcv-stream ws-conn) + (rx/filter ws/message-event?) + (rx/map :payload) + (rx/filter #(= :export-update (:type %))) + (rx/filter #(= @resource-id (:resource-id %))) + (rx/share)) - stopper - (rx/filter #(or (= "ended" (:status %)) - (= "error" (:status %))) - progress-stream)] + stopper + (rx/filter #(or (= "ended" (:status %)) + (= "error" (:status %))) + progress-stream)] - (swap! st/ongoing-tasks conj :export) + (swap! st/ongoing-tasks conj :export) - (rx/merge - ;; Force that all data is persisted; best effort. - (rx/of ::dwp/force-persist) + (rx/merge + ;; Force that all data is persisted; best effort. + (rx/of ::dwp/force-persist) - ;; Launch the exportation process and stores the resource id - ;; locally. - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [id] :as resource}] - (vreset! resource-id id) - (initialize-export-status exports cmd resource)))) + ;; Launch the exportation process and stores the resource id + ;; locally. + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [id] :as resource}] + (vreset! resource-id id) + (initialize-export-status exports cmd resource)))) - ;; We proceed to update the export state with incoming - ;; progress updates. We delay the stopper for give some time - ;; to update the status with ended or errored status before - ;; close the stream. - (->> progress-stream - (rx/map update-export-status) - (rx/take-until (rx/delay 500 stopper)) - (rx/finalize (fn [] - (swap! st/ongoing-tasks disj :export)))) + ;; We proceed to update the export state with incoming + ;; progress updates. We delay the stopper for give some time + ;; to update the status with ended or errored status before + ;; close the stream. + (->> progress-stream + (rx/map update-export-status) + (rx/take-until (rx/delay 500 stopper)) + (rx/finalize (fn [] + (swap! st/ongoing-tasks disj :export)))) - ;; We hide need to hide the ui elements of the export after - ;; some interval. We also delay a little bit more the stopper - ;; for ensure that after some security time, the stream is - ;; completely closed. - (->> progress-stream - (rx/filter #(= "ended" (:status %))) - (rx/take 1) - (rx/delay default-timeout) - (rx/map #(clear-export-state @resource-id)) - (rx/take-until (rx/delay 6000 stopper)))))))) + ;; We hide need to hide the ui elements of the export after + ;; some interval. We also delay a little bit more the stopper + ;; for ensure that after some security time, the stream is + ;; completely closed. + (->> progress-stream + (rx/filter #(= "ended" (:status %))) + (rx/take 1) + (rx/delay default-timeout) + (rx/map #(clear-export-state @resource-id)) + (rx/take-until (rx/delay 6000 stopper))))))))) (defn request-export [{:keys [exports] :as params}] diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index c87a5707f9..eca273832a 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -34,6 +34,7 @@ [app.config :as cf] [app.main.data.changes :as dch] [app.main.data.event :as ev] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] @@ -1147,16 +1148,16 @@ page-id (:current-page-id state) selected (first (dsh/lookup-selected state)) - export {:file-id file-id - :page-id page-id - :object-id selected - ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. - :type :png - ;; Always use 2 to ensure good enough quality for wireframes. - :scale 2 - :suffix "" - :enabled true - :name ""} + export (de/normalize-export {:file-id file-id + :page-id page-id + :object-id selected + ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. + :type :png + ;; Always use 2 to ensure good enough quality for wireframes. + :scale 2 + :suffix "" + :enabled true + :name ""}) ;; Create a deferred promise immediately, before any async operations. ;; Registering the clipboard write NOW preserves the user-gesture security diff --git a/frontend/src/app/main/data/workspace/colors.cljs b/frontend/src/app/main/data/workspace/colors.cljs index c145a203b7..c137936650 100644 --- a/frontend/src/app/main/data/workspace/colors.cljs +++ b/frontend/src/app/main/data/workspace/colors.cljs @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.files.helpers :as cfh] + [app.common.math :as mth] [app.common.schema :as sm] [app.common.types.color :as clr] [app.common.types.fills :as types.fills] @@ -950,7 +951,8 @@ (or (not cap-stops?) (< (count stops) types.fills/MAX-GRADIENT-STOPS))] (if can-add-stop? - (let [new-stop (-> (clr/interpolate-gradient stops offset) + (let [offset (mth/clamp offset 0 1) + new-stop (-> (clr/interpolate-gradient stops offset) (split-color-components)) stops (conj stops new-stop) stops (into [] (sort-by :offset stops)) @@ -973,7 +975,8 @@ stops (mapv split-color-components (if cap-stops? (take types.fills/MAX-GRADIENT-STOPS stops) - stops))] + stops)) + stops (mapv #(update % :offset (fn [o] (mth/clamp o 0 1))) stops)] (-> state (assoc :current-color (get stops stop)) (assoc :stops stops)))))))) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 667f0b2b61..5c09149bad 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1571,7 +1571,12 @@ (as-> libraries-to-load $ (remove loaded-libraries $) (conj $ library-id) - (map #(load-library-file file-id %) $)))))) + (map #(load-library-file file-id %) $)))) + (rx/catch (fn [cause] + (let [error (ex-data cause)] + (if (= (:code error) :circular-library-reference) + (rx/of (ntf/error (tr "errors.circular-library-reference"))) + (rx/throw cause))))))) (rx/of (ptk/reify ::attach-library-finished)) (when (pos? variants-count) (->> (rp/cmd! :get-library-usage {:file-id library-id}) diff --git a/frontend/src/app/main/store.cljs b/frontend/src/app/main/store.cljs index 9ecfc39284..8bdff34a65 100644 --- a/frontend/src/app/main/store.cljs +++ b/frontend/src/app/main/store.cljs @@ -107,8 +107,8 @@ (defn format-last-events "Render the `last-events` buffer as a multi-line string with the wall-clock time of each event and the delta (ms) since the previous - entry. The first entry has no delta. Useful for embedding in error - reports." + entry. The delta column is right-padded to 10 chars so the event + names align. Useful for embedding in error reports." ([] (format-last-events @last-events)) ([events] (let [lines @@ -117,13 +117,14 @@ out (transient [])] (if xs (let [{:keys [name t]} (first xs) - iso (ct/format-inst t :iso) - tail (if prev-t - (str " (+" (ct/diff-ms prev-t t) "ms)") - "")] + iso (ct/format-inst t :iso) + delta (if prev-t + (str "(+" (ct/diff-ms prev-t t) "ms)") + "(+0ms)") + delta-pad (str/pad delta {:length 10 :type :right})] (recur t (next xs) - (conj! out (str iso tail " " name)))) + (conj! out (str iso " " delta-pad " " name)))) (persistent! out)))] (str/join "\n" lines)))) diff --git a/frontend/src/app/main/ui/components/context_menu_a11y.cljs b/frontend/src/app/main/ui/components/context_menu_a11y.cljs index 75c97554ce..e2e824c19e 100644 --- a/frontend/src/app/main/ui/components/context_menu_a11y.cljs +++ b/frontend/src/app/main/ui/components/context_menu_a11y.cljs @@ -18,6 +18,7 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] [app.util.timers :as tm] + [beicon.v2.core :as rx] [rumext.v2 :as mf])) (def ^:private xf:options @@ -229,8 +230,11 @@ (partial ug/unlisten "penpot:context-menu:open" on-event))) (mf/with-effect [ids] - (tm/schedule-on-idle - #(dom/focus! (dom/get-element (first ids))))) + (let [handle (tm/schedule + (fn [] + (some-> (dom/get-element (first ids)) + (dom/focus!))))] + #(rx/dispose! handle))) (when (some? levels) [:> dropdown-content* props diff --git a/frontend/src/app/main/ui/components/dropdown.cljs b/frontend/src/app/main/ui/components/dropdown.cljs index 894d9ef204..4a0a1b0590 100644 --- a/frontend/src/app/main/ui/components/dropdown.cljs +++ b/frontend/src/app/main/ui/components/dropdown.cljs @@ -11,6 +11,7 @@ [app.util.globals :as globals] [app.util.keyboard :as kbd] [app.util.timers :as tm] + [beicon.v2.core :as rx] [goog.events :as events] [rumext.v2 :as mf]) (:import goog.events.EventType)) @@ -45,9 +46,10 @@ (fn [] (let [keys [(events/listen globals/document EventType.CLICK on-click) (events/listen globals/document EventType.CONTEXTMENU on-click) - (events/listen globals/document EventType.KEYUP on-keyup)]] - (tm/schedule #(mf/set-ref-val! listening-ref true)) - #(run! events/unlistenByKey keys)))] + (events/listen globals/document EventType.KEYUP on-keyup)] + timer (tm/schedule #(mf/set-ref-val! listening-ref true))] + #(do (rx/dispose! timer) + (run! events/unlistenByKey keys))))] (mf/use-effect on-mount) children)) diff --git a/frontend/src/app/main/ui/components/forms.cljs b/frontend/src/app/main/ui/components/forms.cljs index 89a1bcd42d..88585a753e 100644 --- a/frontend/src/app/main/ui/components/forms.cljs +++ b/frontend/src/app/main/ui/components/forms.cljs @@ -560,28 +560,29 @@ on-paste (mf/use-fn (fn [event] - (let [paste-data (-> event .-clipboardData (.getData "text"))] - (when (and (string? paste-data) - (re-find #"[,\s]" paste-data)) - (dom/prevent-default event) - (dom/stop-propagation event) + (when-let [clipboard-data (.-clipboardData event)] + (let [paste-data (.getData clipboard-data "text")] + (when (and (string? paste-data) + (re-find #"[,\s]" paste-data)) + (dom/prevent-default event) + (dom/stop-propagation event) - ;; Mark as touched - (swap! form assoc-in [:touched input-name] true) + ;; Mark as touched + (swap! form assoc-in [:touched input-name] true) - ;; Split pasted text by commas and/or whitespace, add each valid part - (let [parts (->> (str/split paste-data #",|\s+") - (map str/trim) - (remove str/empty?))] - (doseq [part parts] - (when (valid-item-fn part) - (swap! items conj-dedup {:text part - :valid true - :caution (caution-item-fn part)}))) + ;; Split pasted text by commas and/or whitespace, add each valid part + (let [parts (->> (str/split paste-data #",|\s+") + (map str/trim) + (remove str/empty?))] + (doseq [part parts] + (when (valid-item-fn part) + (swap! items conj-dedup {:text part + :valid true + :caution (caution-item-fn part)}))) - ;; Reset input value and mark as untouched after successful paste - (reset! value "") - (swap! form assoc-in [:touched input-name] false)))))) + ;; Reset input value and mark as untouched after successful paste + (reset! value "") + (swap! form assoc-in [:touched input-name] false))))))) on-blur (mf/use-fn diff --git a/frontend/src/app/main/ui/dashboard/sidebar.cljs b/frontend/src/app/main/ui/dashboard/sidebar.cljs index eeda604490..53d87a8878 100644 --- a/frontend/src/app/main/ui/dashboard/sidebar.cljs +++ b/frontend/src/app/main/ui/dashboard/sidebar.cljs @@ -40,6 +40,7 @@ [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.foundations.assets.icon :refer [icon*] :as i] [app.main.ui.ds.foundations.assets.raw-svg :refer [raw-svg*]] + [app.main.ui.hooks :refer [use-focus-timer-ref]] [app.main.ui.icons :as deprecated-icon] [app.main.ui.nitrate.nitrate-form] [app.util.dom :as dom] @@ -94,6 +95,14 @@ (def ^:private ^:svg-id penpot-logo-icon "penpot-logo-icon") (def ^:private ^:svg-id penpot-logo-icon-subtle "penpot-logo-subtle") +(defn schedule-focus-by-id! + [ref element-id] + (when-let [h (mf/ref-val ref)] + (ts/dispose! h)) + (mf/set-ref-val! ref + (ts/schedule + #(dom/focus-and-untabbable! (dom/get-element element-id))))) + (mf/defc sidebar-project* {::mf/private true} [{:keys [item is-selected]}] @@ -112,6 +121,8 @@ project-id (get item :id) + focus-timer-ref (use-focus-timer-ref) + on-click (mf/use-fn (mf/deps project-id) @@ -123,14 +134,9 @@ (mf/deps project-id) (fn [event] (when (kbd/enter? event) - (st/emit! - (dcm/go-to-dashboard-files :project-id project-id)) - (ts/schedule - (fn [] - (when-let [title (dom/get-element (str project-id))] - (dom/set-attribute! title "tabindex" "0") - (dom/focus! title) - (dom/set-attribute! title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref (str project-id)) + (st/emit! (dcm/go-to-dashboard-files :project-id project-id))))) + on-menu-click (mf/use-fn @@ -228,6 +234,8 @@ focused? (mf/use-state false) emit! (mf/use-memo #(f/debounce st/emit! 500)) + focus-timer-ref (use-focus-timer-ref) + on-search-blur (mf/use-fn (fn [_] @@ -254,13 +262,7 @@ (mf/use-fn (fn [e] (when (kbd/enter? e) - (ts/schedule - (fn [] - (let [search-title (dom/get-element (str "dashboard-search-title"))] - (when search-title - (dom/set-attribute! search-title "tabindex" "0") - (dom/focus! search-title) - (dom/set-attribute! search-title "tabindex" "-1"))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-search-title") (dom/prevent-default e) (dom/stop-propagation e)))) @@ -948,6 +950,8 @@ nitrate? (contains? cf/flags :nitrate) + focus-timer-ref (use-focus-timer-ref) + go-projects (mf/use-fn #(st/emit! (dcm/go-to-dashboard-recent))) @@ -957,12 +961,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-recent :team-id team-id)) - (ts/schedule - (fn [] - (when-let [projects-title (dom/get-element "dashboard-projects-title")] - (dom/set-attribute! projects-title "tabindex" "0") - (dom/focus! projects-title) - (dom/set-attribute! projects-title "tabindex" "-1")))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-projects-title"))) go-fonts (mf/use-fn @@ -975,13 +974,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-fonts :team-id team-id)) - (ts/schedule - (fn [] - (let [font-title (dom/get-element "dashboard-fonts-title")] - (when font-title - (dom/set-attribute! font-title "tabindex" "0") - (dom/focus! font-title) - (dom/set-attribute! font-title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-fonts-title"))) go-drafts (mf/use-fn @@ -994,12 +987,7 @@ (mf/deps team-id default-project-id) (fn [] (st/emit! (dcm/go-to-dashboard-files :team-id team-id :project-id default-project-id)) - (ts/schedule - (fn [] - (when-let [title (dom/get-element "dashboard-drafts-title")] - (dom/set-attribute! title "tabindex" "0") - (dom/focus! title) - (dom/set-attribute! title "tabindex" "-1")))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-drafts-title"))) go-libs (mf/use-fn @@ -1012,13 +1000,7 @@ (fn [] (st/emit! (dcm/go-to-dashboard-libraries :team-id team-id)) - (ts/schedule - (fn [] - (let [libs-title (dom/get-element "dashboard-libraries-title")] - (when libs-title - (dom/set-attribute! libs-title "tabindex" "0") - (dom/focus! libs-title) - (dom/set-attribute! libs-title "tabindex" "-1"))))))) + (schedule-focus-by-id! focus-timer-ref "dashboard-libraries-title"))) pinned-projects (mf/with-memo [projects] diff --git a/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs b/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs index bc20c517c9..e3d6b153fb 100644 --- a/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs +++ b/frontend/src/app/main/ui/ds/tooltip/tooltip.cljs @@ -322,25 +322,26 @@ (let [trigger-el (mf/ref-val trigger-ref) tooltip-el (mf/ref-val tooltip-ref)] (when (and trigger-el tooltip-el) - (ts/raf - (fn [] - (let [origin-brect (dom/get-bounding-rect trigger-el) - tooltip-brect (dom/get-bounding-rect tooltip-el) - window-size (dom/get-window-size)] - (when-let [[new-placement placement-rect] - (find-matching-placement - placement - tooltip-brect - origin-brect - window-size - offset)] - (dom/set-css-property! tooltip-el "inset-block-start" - (str (:top placement-rect) "px")) - (dom/set-css-property! tooltip-el "inset-inline-start" - (str (:left placement-rect) "px")) + (let [raf-id (ts/raf + (fn [] + (let [origin-brect (dom/get-bounding-rect trigger-el) + tooltip-brect (dom/get-bounding-rect tooltip-el) + window-size (dom/get-window-size)] + (when-let [[new-placement placement-rect] + (find-matching-placement + placement + tooltip-brect + origin-brect + window-size + offset)] + (dom/set-css-property! tooltip-el "inset-block-start" + (str (:top placement-rect) "px")) + (dom/set-css-property! tooltip-el "inset-inline-start" + (str (:left placement-rect) "px")) - (when (not= new-placement placement) - (reset! placement* new-placement))))))))))) + (when (not= new-placement placement) + (reset! placement* new-placement))))))] + #(ts/cancel-af! raf-id))))))) [:> :div props children diff --git a/frontend/src/app/main/ui/forms.cljs b/frontend/src/app/main/ui/forms.cljs index 82b0335008..9115761290 100644 --- a/frontend/src/app/main/ui/forms.cljs +++ b/frontend/src/app/main/ui/forms.cljs @@ -121,28 +121,29 @@ on-paste (mf/use-fn (fn [event] - (let [paste-data (-> event .-clipboardData (.getData "text"))] - (when (and (string? paste-data) - (re-find #"[,\s]" paste-data)) - (dom/prevent-default event) - (dom/stop-propagation event) + (when-let [clipboard-data (.-clipboardData event)] + (let [paste-data (.getData clipboard-data "text")] + (when (and (string? paste-data) + (re-find #"[,\s]" paste-data)) + (dom/prevent-default event) + (dom/stop-propagation event) - ;; Mark as touched - (swap! form assoc-in [:touched name] true) + ;; Mark as touched + (swap! form assoc-in [:touched name] true) - ;; Split pasted text by commas and/or whitespace, add each valid part - (let [parts (->> (str/split paste-data #",|\s+") - (map str/trim) - (remove str/empty?))] - (doseq [part parts] - (when (valid-item-fn part) - (swap! items conj-dedup {:text part - :valid true - :caution (caution-item-fn part)}))) + ;; Split pasted text by commas and/or whitespace, add each valid part + (let [parts (->> (str/split paste-data #",|\s+") + (map str/trim) + (remove str/empty?))] + (doseq [part parts] + (when (valid-item-fn part) + (swap! items conj-dedup {:text part + :valid true + :caution (caution-item-fn part)}))) - ;; Reset input value and mark as untouched after successful paste - (reset! value "") - (swap! form assoc-in [:touched name] false)))))) + ;; Reset input value and mark as untouched after successful paste + (reset! value "") + (swap! form assoc-in [:touched name] false))))))) on-blur (mf/use-fn diff --git a/frontend/src/app/main/ui/hooks.cljs b/frontend/src/app/main/ui/hooks.cljs index ae8ebd30d5..bb541dd8ea 100644 --- a/frontend/src/app/main/ui/hooks.cljs +++ b/frontend/src/app/main/ui/hooks.cljs @@ -281,6 +281,16 @@ (mf/set-ref-val! ref val)) (mf/ref-val ref))) +;; FIXME: replace with rumext +(defn use-focus-timer-ref + "Returns a ref for scheduling focus timers and disposes any pending + timer on component unmount." + [] + (let [ref (mf/use-ref nil)] + (mf/with-effect [] + #(some-> (mf/ref-val ref) ts/dispose!)) + ref)) + ;; FIXME: rename to use-focus-objects (defn with-focus-objects ([objects] diff --git a/frontend/src/app/main/ui/workspace.cljs b/frontend/src/app/main/ui/workspace.cljs index e7a3e7b119..124eee9b1b 100644 --- a/frontend/src/app/main/ui/workspace.cljs +++ b/frontend/src/app/main/ui/workspace.cljs @@ -44,7 +44,6 @@ [app.main.ui.workspace.webgl-unavailable-modal] [app.util.debug :as dbg] [app.util.dom :as dom] - [app.util.globals :as globals] [app.util.i18n :as i18n :refer [tr]] [goog.events :as events] [okulary.core :as l] @@ -177,7 +176,7 @@ (mf/with-effect [] (let [focus-out #(st/emit! (dw/workspace-focus-lost)) - key (events/listen globals/window "blur" focus-out)] + key (events/listen js/window "blur" focus-out)] (partial events/unlistenByKey key))) (mf/with-effect [file-id page-id] @@ -252,7 +251,7 @@ (let [handle-wasm-render (fn [_] (reset! first-frame-rendered? true)) - listener-key (events/listen globals/document "penpot:wasm:render" handle-wasm-render)] + listener-key (events/listen js/document "penpot:wasm:render" handle-wasm-render)] (fn [] (events/unlistenByKey listener-key)))) diff --git a/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs b/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs index 7010e84371..93648bc80d 100644 --- a/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs +++ b/frontend/src/app/main/ui/workspace/colorpicker/gradients.cljs @@ -233,6 +233,7 @@ (mf/deps on-add-stop-preview) (fn [^js e] (let [offset (-> (event->offset e) + (mth/clamp 0 1) (mth/precision 2))] (when on-add-stop-preview (on-add-stop-preview offset))))) diff --git a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs index 766acb2430..060ee0bada 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs @@ -61,7 +61,7 @@ nil))) (defn- styles-fn [shape styles content] - (let [data (if (= (.getText ^js content) "") + (let [data (if (and content (= (.getText ^js content) "")) (-> ^js (.getData content) (.toJS) (js->clj :keywordize-keys true)) diff --git a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs index 749c5af69f..8db3275dad 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs @@ -105,14 +105,14 @@ (mf/use-fn (fn [^js event] (dom/prevent-default event) - (let [clipboard-data (.-clipboardData event) - text (.getData clipboard-data "text/plain")] - (when (and text (seq text)) - (text-editor/text-editor-insert-text text) - (sync-wasm-text-editor-content!) - (wasm.api/request-render "text-paste")) - (when-let [node (mf/ref-val contenteditable-ref)] - (set! (.-textContent node) ""))))) + (when-let [clipboard-data (.-clipboardData event)] + (let [text (.getData clipboard-data "text/plain")] + (when (and text (seq text)) + (text-editor/text-editor-insert-text text) + (sync-wasm-text-editor-content!) + (wasm.api/request-render "text-paste")))) + (when-let [node (mf/ref-val contenteditable-ref)] + (set! (.-textContent node) "")))) on-copy (mf/use-fn diff --git a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs index bc16b89045..c135d11e7e 100644 --- a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs @@ -189,6 +189,7 @@ lv (-> (gpt/to-vec from-p to-p) (gpt/unit)) nv (gpt/normal-left lv) offset (-> (gsp/project-t position [from-p to-p] nv) + (mth/clamp 0 1) (mth/precision 2)) new-stop (cc/interpolate-gradient stops offset) stops (conj stops new-stop) diff --git a/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs b/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs index afc461b4c6..c756f06e52 100644 --- a/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs @@ -56,51 +56,52 @@ (defn process-pointer-move [viewport-node canvas canvas-image-data zoom-view-context last-picked-color client-x client-y] - (when-let [image-data (mf/ref-val canvas-image-data)] - (when-let [zoom-view-node (dom/get-element "picker-detail")] - (when-not (mf/ref-val zoom-view-context) - (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) - (let [canvas-width 260 - canvas-height 140 - {brx :left bry :top} (dom/get-bounding-rect viewport-node) + (when viewport-node + (when-let [image-data (mf/ref-val canvas-image-data)] + (when-let [zoom-view-node (dom/get-element "picker-detail")] + (when-not (mf/ref-val zoom-view-context) + (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) + (let [canvas-width 260 + canvas-height 140 + {brx :left bry :top} (dom/get-bounding-rect viewport-node) - x (mth/floor (- client-x brx)) - y (mth/floor (- client-y bry)) + x (mth/floor (- client-x brx)) + y (mth/floor (- client-y bry)) - img-width (unchecked-get image-data "width") - img-height (unchecked-get image-data "height") + img-width (unchecked-get image-data "width") + img-height (unchecked-get image-data "height") - zoom-context (mf/ref-val zoom-view-context) + zoom-context (mf/ref-val zoom-view-context) - sx (- x 32) - sy (if (cfg/check-browser? :safari) y (- y 17)) - sw 65 - sh 35 - dx 0 - dy 0 - dw canvas-width - dh canvas-height] + sx (- x 32) + sy (if (cfg/check-browser? :safari) y (- y 17)) + sw 65 + sh 35 + dx 0 + dy 0 + dw canvas-width + dh canvas-height] - (when (obj/get zoom-context "imageSmoothingEnabled") - (obj/set! zoom-context "imageSmoothingEnabled" false)) - (.clearRect zoom-context 0 0 canvas-width canvas-height) - (.drawImage zoom-context canvas sx sy sw sh dx dy dw dh) + (when (obj/get zoom-context "imageSmoothingEnabled") + (obj/set! zoom-context "imageSmoothingEnabled" false)) + (.clearRect zoom-context 0 0 canvas-width canvas-height) + (.drawImage zoom-context canvas sx sy sw sh dx dy dw dh) - ;; Only pick color when cursor is within canvas bounds to avoid garbage pixels - (when (and (>= x 0) (< x img-width) (>= y 0) (< y img-height)) - (let [offset (* (+ (* y img-width) x) 4) - rgba (unchecked-get image-data "data") - r (d/check-num (obj/get rgba (+ 0 offset)) 255) - g (d/check-num (obj/get rgba (+ 1 offset)) 255) - b (d/check-num (obj/get rgba (+ 2 offset)) 255) - a (d/check-num (obj/get rgba (+ 3 offset)) 255) - color [r g b a]] - ;; Store latest color synchronously so the click handler always reads - ;; the correct pixel even before the rAF fires (fixes race condition) - (mf/set-ref-val! last-picked-color color) - (timers/raf - (fn [] - (st/emit! (dwc/pick-color color)))))))))) + ;; Only pick color when cursor is within canvas bounds to avoid garbage pixels + (when (and (>= x 0) (< x img-width) (>= y 0) (< y img-height)) + (let [offset (* (+ (* y img-width) x) 4) + rgba (unchecked-get image-data "data") + r (d/check-num (obj/get rgba (+ 0 offset)) 255) + g (d/check-num (obj/get rgba (+ 1 offset)) 255) + b (d/check-num (obj/get rgba (+ 2 offset)) 255) + a (d/check-num (obj/get rgba (+ 3 offset)) 255) + color [r g b a]] + ;; Store latest color synchronously so the click handler always reads + ;; the correct pixel even before the rAF fires (fixes race condition) + (mf/set-ref-val! last-picked-color color) + (timers/raf + (fn [] + (st/emit! (dwc/pick-color color))))))))))) (mf/defc pixel-overlay* @@ -260,18 +261,19 @@ (defn- viewport->canvas-coords "Maps client (viewport) coordinates to device-pixel canvas coordinates." [viewport-node client-x client-y] - (let [{brx :left bry :top} (dom/get-bounding-rect viewport-node) - dpr (wasm.api/get-dpr) - x (mth/floor (- client-x brx)) - y (mth/floor (- client-y bry))] - [(mth/floor (* x dpr)) - (mth/floor (* y dpr))])) + (when viewport-node + (let [{brx :left bry :top} (dom/get-bounding-rect viewport-node) + dpr (wasm.api/get-dpr) + x (mth/floor (- client-x brx)) + y (mth/floor (- client-y bry))] + [(mth/floor (* x dpr)) + (mth/floor (* y dpr))]))) (defn process-pointer-move-wasm "Updates the magnifier loupe with the canvas region under the cursor. The actual color is only read on click (see `pick-color-at-wasm`)." [viewport-node canvas zoom-view-context client-x client-y] - (when canvas + (when (and canvas viewport-node) (when-let [zoom-view-node (dom/get-element "picker-detail")] (when-not (mf/ref-val zoom-view-context) (mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d"))) @@ -309,7 +311,7 @@ the correct color even on GPUs where a raw WebGL `readPixels` returned values with their byte order swapped." [viewport-node canvas client-x client-y] - (when canvas + (when (and canvas viewport-node) (let [[canvas-x canvas-y] (viewport->canvas-coords viewport-node client-x client-y) img-width (.-width canvas) img-height (.-height canvas)] @@ -370,7 +372,7 @@ handle-draw-picker-canvas (mf/use-callback (fn [] - (when canvas + (when (and canvas viewport-node) ;; Read current mouse position from ref so the loupe refreshes on ;; each render even without a mouse-move. (let [{mx :x my :y} (mf/ref-val initial-mouse-pos)] diff --git a/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs b/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs index 46e1356596..38349f8f24 100644 --- a/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs @@ -7,62 +7,75 @@ (ns app.main.ui.workspace.viewport.viewport-ref (:require [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.geom.point :as gpt] + [app.main.refs :as refs] [app.main.store :as st] [app.util.dom :as dom] [app.util.mouse :as mse] - [goog.events :as events] - [rumext.v2 :as mf]) - (:import goog.events.EventType)) + [rumext.v2 :as mf])) (defonce viewport-ref (atom nil)) -(defonce current-observer (atom nil)) (defonce viewport-brect (atom nil)) -(defn init-observer - [node on-change-bounds] +(defn- init-observer + [node] + (let [on-change-bounds + (fn [_] + (let [brect (dom/get-bounding-rect node) + brect (gpt/point (d/parse-integer (:left brect)) + (d/parse-integer (:top brect)))] + (reset! viewport-brect brect))) - (let [observer (js/ResizeObserver. on-change-bounds)] - (when (some? @current-observer) - (.disconnect @current-observer)) + observer + (js/ResizeObserver. on-change-bounds)] - (reset! current-observer observer) - - (when (some? node) - (.observe observer node)))) - -(defn on-change-bounds - [_] - (when @viewport-ref - (let [brect (dom/get-bounding-rect @viewport-ref) - brect (gpt/point (d/parse-integer (:left brect)) - (d/parse-integer (:top brect)))] - (reset! viewport-brect brect)))) + (.observe observer node) + observer)) (defn create-viewport-ref [] - (let [ref (mf/use-ref nil)] - [ref - (mf/use-memo - #(fn [node] - (mf/set-ref-val! ref node) - (reset! viewport-ref node) - (when (some? node) - (events/listen node EventType.MOUSELEAVE (fn [] (st/emit! (mse/->BlurEvent))))) - (init-observer node on-change-bounds)))])) + (let [node-ref (mf/use-ref nil) + handler-ref (mf/use-ref nil) + observer-ref (mf/use-ref nil) + callback (mf/use-fn + (fn [node] + ;; Dispose all previous resources + (when-let [observer (mf/ref-val observer-ref)] + (.disconnect ^js observer) + (mf/set-ref-val! observer-ref nil)) + + + (when-let [handler (mf/ref-val handler-ref)] + (when-let [node (mf/ref-val node-ref)] + (.removeEventListener ^js node "mouseleave" handler) + (mf/set-ref-val! handler-ref nil))) + + ;; Reset the ref values to the current node (can be nil) + (mf/set-ref-val! node-ref node) + (reset! viewport-ref node) + + (when (some? node) + (let [handler (fn [] (st/emit! (mse/->BlurEvent))) + observer (init-observer node)] + (.addEventListener ^js node "mouseleave" handler) + + (mf/set-ref-val! handler-ref handler) + (mf/set-ref-val! observer-ref observer)))))] + [node-ref callback])) (defn point->viewport [pt] - (let [zoom (dm/get-in @st/state [:workspace-local :zoom] 1)] - (when (and (some? @viewport-ref) - (some? @viewport-brect)) - (let [vbox (.. ^js @viewport-ref -viewBox -baseVal) - brect @viewport-brect - box (gpt/point (.-x vbox) (.-y vbox)) - zoom (gpt/point zoom)] + (let [zoom (d/nilv @refs/selected-zoom 1) + viewport-node @viewport-ref + viewport-brect @viewport-brect] - (-> (gpt/subtract pt brect) + (when (and (some? viewport-brect) + (some? viewport-node)) + (let [vbox (.. ^js viewport-node -viewBox -baseVal) + box (gpt/point (.-x vbox) (.-y vbox)) + zoom (gpt/point zoom)] + + (-> (gpt/subtract pt viewport-brect) (gpt/divide zoom) (gpt/add box)))))) @@ -71,8 +84,8 @@ Unlike point->viewport, this does NOT convert to canvas coordinates - it just subtracts the viewport's bounding rect offset." [pt] - (when (some? @viewport-brect) - (gpt/subtract pt @viewport-brect))) + (when-let [brect @viewport-brect] + (gpt/subtract pt brect))) (defn inside-viewport? [target] diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 4a1ea9bdf9..725f69c3c8 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -34,6 +34,7 @@ [app.common.types.text :as txt] [app.common.uuid :as uuid] [app.config :as cf] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.persistence :as dwp] [app.main.data.plugins :as dp] @@ -1547,13 +1548,13 @@ :profile-id (:profile-id @st/state) :wait true :is-wasm false - :exports [{:file-id file-id - :page-id page-id - :object-id id - :name (:name shape) - :type (:type value :png) - :suffix (:suffix value "") - :scale (:scale value 1)}]}] + :exports [(de/normalize-export {:file-id file-id + :page-id page-id + :object-id id + :name (:name shape) + :type (:type value :png) + :suffix (:suffix value "") + :scale (:scale value 1)})]}] (js/Promise. (fn [resolve reject] ;; The exporter renders the file from its persisted diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index f4be22b6c9..d885e7771b 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -699,6 +699,13 @@ (when (some? node) (.setAttribute node attr value))) +(defn focus-and-untabbable! + [^js node] + (when (some? node) + (set-attribute! node "tabindex" "0") + (focus! node) + (set-attribute! node "tabindex" "-1"))) + (defn set-style! [^js node ^string style value] (when (some? node) diff --git a/frontend/src/app/util/dom/dnd.cljs b/frontend/src/app/util/dom/dnd.cljs index 2a1732d6c2..bb647db785 100644 --- a/frontend/src/app/util/dom/dnd.cljs +++ b/frontend/src/app/util/dom/dnd.cljs @@ -115,13 +115,13 @@ ([e] (get-data e "penpot/data")) ([e data-type] - (let [dt (.-dataTransfer e) - data (.getData dt data-type)] - (cond-> data - (and (some? data) (not= data "") - (or (str/starts-with? data-type "penpot") - (= data-type "application/json"))) - (t/decode-str))))) + (when-let [dt (.-dataTransfer e)] + (let [data (.getData dt data-type)] + (cond-> data + (and (some? data) (not= data "") + (or (str/starts-with? data-type "penpot") + (= data-type "application/json"))) + (t/decode-str)))))) (defn get-files [e] diff --git a/frontend/src/app/util/text_editor.cljs b/frontend/src/app/util/text_editor.cljs index f46ed9aa35..4e91ee80ef 100644 --- a/frontend/src/app/util/text_editor.cljs +++ b/frontend/src/app/util/text_editor.cljs @@ -60,12 +60,14 @@ (defn get-editor-block-data [block] - (-> (.getData ^js block) - (immutable-map->map))) + (when (some? block) + (-> (.getData ^js block) + (immutable-map->map)))) (defn get-editor-block-type [block] - (.getType ^js block)) + (when (some? block) + (.getType ^js block))) (defn get-editor-current-block-data [state] diff --git a/frontend/src/app/util/zip.cljs b/frontend/src/app/util/zip.cljs index 72571f2e79..2288cbdaae 100644 --- a/frontend/src/app/util/zip.cljs +++ b/frontend/src/app/util/zip.cljs @@ -82,6 +82,10 @@ (defn read-as-text [entry] + (when (nil? entry) + (ex/raise :type :assertion + :code :invalid-entry + :hint "cannot read zip entry: entry is nil")) (let [writer (new zip/TextWriter)] (.getData entry writer))) diff --git a/frontend/src/app/worker/import.cljs b/frontend/src/app/worker/import.cljs index 91ce6b92c8..ba6a49ce1e 100644 --- a/frontend/src/app/worker/import.cljs +++ b/frontend/src/app/worker/import.cljs @@ -7,6 +7,7 @@ (ns app.worker.import (:refer-clojure :exclude [resolve]) (:require + [app.common.exceptions :as ex] [app.common.json :as json] [app.common.logging :as log] [app.common.schema :as sm] @@ -44,10 +45,15 @@ (def conjv (fnil conj [])) -(defn- read-zip-manifest +(defn read-zip-manifest [zip-reader] (->> (rx/from (uz/get-entry zip-reader "manifest.json")) - (rx/mapcat uz/read-as-text) + (rx/mapcat (fn [entry] + (if (nil? entry) + (rx/throw (ex/error :type :validation + :code :invalid-penpot-file + :hint "Not a valid Penpot file: manifest.json is missing")) + (uz/read-as-text entry)))) (rx/map json/decode))) (defn slurp-uri diff --git a/frontend/test/frontend_tests/data/exports_assets_test.cljs b/frontend/test/frontend_tests/data/exports_assets_test.cljs new file mode 100644 index 0000000000..d4ae9edea8 --- /dev/null +++ b/frontend/test/frontend_tests/data/exports_assets_test.cljs @@ -0,0 +1,99 @@ +;; 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 INC Sucursal en España SL + +(ns frontend-tests.data.exports-assets-test + (:require + [app.common.uuid :as uuid] + [app.main.data.exports.assets :as de] + [app.main.data.persistence :as dwp] + [app.main.repo :as repo] + [app.main.store :as st] + [app.util.dom :as dom] + [app.util.websocket :as ws] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.events :as the] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(def ^:private export {:id (uuid/next) + :object-id (uuid/next) + :type :png + :suffix "" + :scale 1}) + +(defn- export-with-name + [name] + (merge export {:name name})) + +(defn- test-state + [] + {:profile-id (:id export) + :ws-conn nil}) + +(t/deftest normalize-export-preserves-existing-name + (t/is (= (export-with-name "Layer 1") + (de/normalize-export (export-with-name "Layer 1"))))) + +(t/deftest normalize-export-replaces-nil-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name nil))))) + +(t/deftest normalize-export-replaces-empty-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name ""))))) + +(t/deftest request-simple-export-sends-normalized-export + (t/async done + (let [export (export-with-name "") + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:filename "export.png" + :mtype "image/png" + :uri "blob:export"}))) + dwp/force-persist-and-wait (mock/stub (fn [_] (rx/of ::force-persisted))) + dom/trigger-download-uri (mock/stub (fn [& _] nil))} + (fn [done'] + (let [completed (fn [_state] + (t/is (= (export-with-name (str (:object-id export))) + (-> @observed :exports first))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-simple-export {:export export}) + :the/end))) + done)))) + +(t/deftest request-multiple-export-sends-normalized-enabled-exports + (t/async done + (let [exports [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name ""}] + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:id (:id export)}))) + ws/get-rcv-stream (mock/stub (fn [_] (rx/empty))) + st/ongoing-tasks (atom #{})} + (fn [done'] + (let [completed (fn [_state] + (t/is (= [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name "enabled-1"}] + (:exports @observed))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-multiple-export {:exports exports}) + :the/end))) + done)))) diff --git a/frontend/test/frontend_tests/data/store_test.cljs b/frontend/test/frontend_tests/data/store_test.cljs new file mode 100644 index 0000000000..56d29fec85 --- /dev/null +++ b/frontend/test/frontend_tests/data/store_test.cljs @@ -0,0 +1,49 @@ +;; 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 INC Sucursal en España SL + +(ns frontend-tests.data.store-test + "Unit tests for app.main.store. + Tests cover: + - format-last-events – empty, single, multi, column alignment" + (:require + [app.main.store :as st] + [cljs.test :as t :include-macros true] + [cuerdas.core :as str])) + +(t/deftest format-last-events-empty + (t/testing "empty events produce empty string" + (t/is (= "" (st/format-last-events []))))) + +(t/deftest format-last-events-single + (t/testing "a single event shows (+0ms) and its name" + (let [result (st/format-last-events [{:name ":test/event" :t (js/Date. 1000)}])] + (t/is (str/includes? result "(+0ms)")) + (t/is (str/includes? result ":test/event")) + (t/is (= 1 (count (str/split result "\n"))))))) + +(t/deftest format-last-events-multi + (t/testing "multiple events show correct deltas and event names" + (let [events [{:name ":event/a" :t (js/Date. 0)} + {:name ":event/b" :t (js/Date. 500)} + {:name ":event/c" :t (js/Date. 2500)}] + lines (str/split (st/format-last-events events) "\n")] + (t/is (= 3 (count lines))) + (t/is (some #(str/includes? % ":event/a") lines)) + (t/is (some #(str/includes? % ":event/b") lines)) + (t/is (some #(str/includes? % ":event/c") lines)) + (t/is (str/includes? (nth lines 0) "(+0ms)")) + (t/is (str/includes? (nth lines 1) "(+500ms)")) + (t/is (str/includes? (nth lines 2) "(+2000ms)"))))) + +(t/deftest format-last-events-alignment + (t/testing "event names start at the same column across all lines" + (let [events [{:name ":evt-a" :t (js/Date. 0)} + {:name ":evt-b" :t (js/Date. 500)}] + lines (str/split (st/format-last-events events) "\n") + col-a (.indexOf (nth lines 0) ":evt-a") + col-b (.indexOf (nth lines 1) ":evt-b")] + (t/is (pos? col-a)) + (t/is (= col-a col-b))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 128ce0c216..7972b4e039 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -7,8 +7,10 @@ [frontend-tests.basic-shapes-test] [frontend-tests.code-gen-style-test] [frontend-tests.copy-as-svg-test] + [frontend-tests.data.exports-assets-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.repo-test] + [frontend-tests.data.store-test] [frontend-tests.data.uploads-test] [frontend-tests.data.viewer-test] [frontend-tests.data.workspace-colors-test] @@ -47,6 +49,7 @@ [frontend-tests.plugins.value-objects-test] [frontend-tests.render-wasm.process-objects-test] [frontend-tests.svg-fills-test] + [frontend-tests.text-editor-paste-guard-test] [frontend-tests.tokens.import-export-test] [frontend-tests.tokens.logic.token-actions-test] [frontend-tests.tokens.logic.token-data-test] @@ -60,7 +63,10 @@ [frontend-tests.util-object-test] [frontend-tests.util-range-tree-test] [frontend-tests.util-simple-math-test] + [frontend-tests.util-text-editor-test] [frontend-tests.util-webapi-test] + [frontend-tests.util-zip-test] + [frontend-tests.util.dom.dnd-test] [frontend-tests.worker-snap-test] [goog.object :as gobj])) @@ -82,6 +88,8 @@ 'frontend-tests.copy-as-svg-test 'frontend-tests.data.nitrate-test 'frontend-tests.data.repo-test + 'frontend-tests.data.store-test + 'frontend-tests.data.exports-assets-test 'frontend-tests.errors-test 'frontend-tests.main-errors-test 'frontend-tests.data.uploads-test @@ -132,10 +140,14 @@ 'frontend-tests.ui.ds-controls-numeric-input-test 'frontend-tests.ui.measures-menu-props-test 'frontend-tests.render-wasm.process-objects-test + 'frontend-tests.text-editor-paste-guard-test 'frontend-tests.util-object-test 'frontend-tests.util-range-tree-test 'frontend-tests.util-simple-math-test + 'frontend-tests.util-text-editor-test 'frontend-tests.util-webapi-test + 'frontend-tests.util.dom.dnd-test + 'frontend-tests.util-zip-test 'frontend-tests.worker-snap-test]) (assert (every? find-ns-obj test-namespaces) diff --git a/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs b/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs new file mode 100644 index 0000000000..3a7829f450 --- /dev/null +++ b/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs @@ -0,0 +1,62 @@ +;; 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 INC + +(ns frontend-tests.text-editor-paste-guard-test + "Regression tests for the Cannot read properties of undefined + (reading getData) family of bugs. Each test verifies that the inner + getData call is now guarded by an outer when-let on clipboardData + so that a synthetic event with no clipboardData no longer throws." + (:require + [cljs.test :as t :include-macros true])) + +(defn- guarded-get-data-text + "Mirrors the body of the fixed paste handlers in main/ui/forms.cljs and + main/ui/components/forms.cljs." + [event] + (when-let [clipboard-data (.-clipboardData event)] + (.getData clipboard-data "text"))) + +(defn- guarded-get-data-text-plain + "Mirrors the body of the fixed paste handler in + main/ui/workspace/shapes/text/v3_editor.cljs." + [event] + (when-let [clipboard-data (.-clipboardData event)] + (.getData clipboard-data "text/plain"))) + +(t/deftest guarded-paste-handlers-do-not-throw-on-missing-clipboardData + (t/testing "event without clipboardData returns nil (no throw)" + (t/is (nil? (guarded-get-data-text #js {}))) + (t/is (nil? (guarded-get-data-text-plain #js {})))) + (t/testing "event with explicit nil clipboardData returns nil" + (t/is (nil? (guarded-get-data-text #js {:clipboardData nil}))) + (t/is (nil? (guarded-get-data-text-plain #js {:clipboardData nil})))) + (t/testing "event with valid clipboardData returns the text" + (let [text-cb (fn [t] (if (= t "text") "hello" nil)) + text-plain-cb (fn [t] (if (= t "text/plain") "hello" nil)) + cb #js {:getData (fn [t] (if (= t "text") "hello" nil))} + cbp #js {:getData (fn [t] (if (= t "text/plain") "hello" nil))}] + (t/is (= "hello" (guarded-get-data-text #js {:clipboardData cb}))) + (t/is (= "hello" (guarded-get-data-text-plain #js {:clipboardData cbp})))))) + +;; Mirrors the fixed styles-fn body in main/ui/workspace/shapes/text/editor.cljs. +;; The fix adds an (and content ...) guard so getText and getData are never +;; called on a nil content object. +(defn- guarded-styles-fn-branch + "Returns the data that styles-fn would use for a given content. When content + is nil, the function falls back to the styles-only branch and returns :fallback + (the real function calls legacy.txt/styles-to-attrs which we don't exercise + here — we only verify the guard itself prevents the getText/getData throws)." + [content] + (if (and content (= (.getText ^js content) "")) + (-> ^js (.getData content) + (.toJS) + (js->clj :keywordize-keys true)) + :fallback)) + +(t/deftest guarded-styles-fn-branch-does-not-throw-on-nil-content + (t/testing "nil content falls back to the styles branch (no throw)" + (t/is (= :fallback (guarded-styles-fn-branch nil))) + (t/is (= :fallback (guarded-styles-fn-branch js/undefined))))) diff --git a/frontend/test/frontend_tests/util/dom/dnd_test.cljs b/frontend/test/frontend_tests/util/dom/dnd_test.cljs new file mode 100644 index 0000000000..60c7ad725d --- /dev/null +++ b/frontend/test/frontend_tests/util/dom/dnd_test.cljs @@ -0,0 +1,23 @@ +;; 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 INC + +(ns frontend-tests.util.dom.dnd-test + (:require + [app.util.dom.dnd :as dnd] + [cljs.test :as t :include-macros true])) + +(t/deftest get-data-returns-nil-when-event-has-no-dataTransfer + (t/testing "event without dataTransfer" + (t/is (nil? (dnd/get-data #js {})))) + (t/testing "event with explicit nil dataTransfer" + (t/is (nil? (dnd/get-data #js {:dataTransfer nil})))) + (t/testing "explicit data-type also returns nil for missing dataTransfer" + (t/is (nil? (dnd/get-data #js {} "penpot/data"))))) + +(t/deftest get-data-reads-from-dataTransfer + (t/testing "dataTransfer with matching key returns the value (non-decoded type)" + (let [dt #js {:getData (fn [_type] "hello")}] + (t/is (= "hello" (dnd/get-data #js {:dataTransfer dt} "text/plain")))))) diff --git a/frontend/test/frontend_tests/util_text_editor_test.cljs b/frontend/test/frontend_tests/util_text_editor_test.cljs new file mode 100644 index 0000000000..0823a77d10 --- /dev/null +++ b/frontend/test/frontend_tests/util_text_editor_test.cljs @@ -0,0 +1,18 @@ +;; 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 INC + +(ns frontend-tests.util-text-editor-test + (:require + [app.util.text-editor :as te] + [cljs.test :as t :include-macros true])) + +(t/deftest get-editor-block-data-returns-nil-for-nil-block + (t/is (nil? (te/get-editor-block-data nil))) + (t/is (nil? (te/get-editor-block-data js/undefined)))) + +(t/deftest get-editor-block-type-returns-nil-for-nil-block + (t/is (nil? (te/get-editor-block-type nil))) + (t/is (nil? (te/get-editor-block-type js/undefined)))) diff --git a/frontend/test/frontend_tests/util_zip_test.cljs b/frontend/test/frontend_tests/util_zip_test.cljs new file mode 100644 index 0000000000..7cf2d6f609 --- /dev/null +++ b/frontend/test/frontend_tests/util_zip_test.cljs @@ -0,0 +1,44 @@ +;; 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 INC + +(ns frontend-tests.util-zip-test + (:require + [app.util.zip :as-alias uz] + [app.worker.import :as worker.import] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.mock :as mock] + [promesa.core :as p])) + +(t/deftest read-as-text-nil-entry-raises-typed-error + (t/testing "read-as-text guards against nil entry" + (t/is (thrown-with-msg? js/Error #"nil" + (app.util.zip/read-as-text nil))))) + +(t/deftest read-zip-manifest-missing-throws-validation-error + (t/async + done + (t/testing "read-zip-manifest rejects ZIPs without manifest.json" + (mock/with-mocks + {app.util.zip/get-entry (mock/stub (fn [_ _] (p/resolved nil)))} + (fn [done'] + (->> (worker.import/read-zip-manifest #js {}) + (rx/subs! + (fn [_] + (t/is false "expected validation error to be thrown") + (done')) + (fn [err] + (let [data (ex-data err)] + (t/is (= :invalid-penpot-file (:code data)) + "missing manifest.json raises typed :invalid-penpot-file error") + (t/is (string? (:hint data)) + "missing manifest.json error carries a :hint") + (t/is (re-find #"manifest\.json" (:hint data)) + "missing manifest.json :hint mentions manifest.json") + (t/is (nil? (re-find #"getData" (:hint data))) + "missing manifest.json :hint does not leak the raw TypeError text") + (done')))))) + done)))) \ No newline at end of file diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 2155b71895..1f0017d87f 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1490,6 +1490,9 @@ msgstr "The fonts %s could not be loaded" msgid "errors.cannot-upload" msgstr "Cannot upload the media file." +msgid "errors.circular-library-reference" +msgstr "Cannot add library: this would create a circular dependency" + #: src/app/main/ui/comments.cljs:737, src/app/main/ui/comments.cljs:767, src/app/main/ui/comments.cljs:864 msgid "errors.character-limit-exceeded" msgstr "Character limit exceeded" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index ef0ef18989..3c2655035b 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1501,6 +1501,9 @@ msgstr "No se han podido cargar las fuentes %s" msgid "errors.cannot-upload" msgstr "No se puede cargar el archivo multimedia." +msgid "errors.circular-library-reference" +msgstr "No se puede añadir la biblioteca: crearía una dependencia circular" + #: src/app/main/ui/comments.cljs:737, src/app/main/ui/comments.cljs:767, src/app/main/ui/comments.cljs:864 msgid "errors.character-limit-exceeded" msgstr "Se ha superado el límite de caracteres" diff --git a/mcp/README.md b/mcp/README.md index 3efc6255e7..846ee4d128 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -272,6 +272,7 @@ The Penpot MCP server can be configured using environment variables. | `PENPOT_MCP_REPL_PORT` | Port for the REPL server (development/debugging) | `4403` | | `PENPOT_MCP_REMOTE_MODE` | Enable remote mode (disables file system access). Set to `true` to enable. | `false` | | `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools. Set to `true` to enable. | `false` | +| `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` | | `PENPOT_MCP_EXPORT_SHAPE_MAX_PARALLEL_REQUESTS` | Maximum number of parallel export shape requests (multi-user mode only). | `0` (no limit) | | `PENPOT_MCP_REDIS_URI` | Redis connection URI (e.g. `redis://host:6379`) enabling multi-instance horizontal scaling via Redis pub/sub task routing (multi-user mode only). When unset, the server runs in single-instance mode, requiring the plugin and MCP client to connect to the same instance. | (unset) | diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index 8a4ee30f25..bd992ec108 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -127,6 +127,7 @@ export class PenpotMcpServer { this.webSocketPort = parseInt(process.env.PENPOT_MCP_WEBSOCKET_PORT ?? "4402", 10); this.replPort = parseInt(process.env.PENPOT_MCP_REPL_PORT ?? "4403", 10); this.tenant = process.env.PENPOT_TENANT ?? "default"; + const toolTimeoutSecs = parseInt(process.env.PENPOT_MCP_TOOL_TIMEOUT_S ?? "120", 10); this.configLoader = new ConfigurationLoader(process.cwd()); this.apiDocs = new ApiDocs(); @@ -147,7 +148,7 @@ export class PenpotMcpServer { this.redisBridge = new RedisBridge(redisUri, this.tenant); } - this.pluginBridge = new PluginBridge(this, this.webSocketPort, this.redisBridge); + this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge); this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); } diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index 1c24547b8f..ac7a2c005c 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -20,8 +20,11 @@ interface ClientConnection { * over these connections. */ export class PluginBridge { + public static readonly MULTIUSER_CONNECTION_ERROR_MESSAGE = `No Penpot instance connected for user token. Please ensure that Penpot is connected and that the MCP client connection is using the correct token.`; + private readonly logger = createLogger("PluginBridge"); private readonly wsServer: WebSocketServer; + private readonly connectedClients: Map = new Map(); private readonly clientsByToken: Map = new Map(); private readonly pendingTasks: Map> = new Map(); @@ -37,12 +40,13 @@ export class PluginBridge { * holding the relevant plugin's WebSocket connection (which may be this same * instance) via Redis, rather than dispatched directly over a local socket. * @param taskTimeoutSecs - Timeout, in seconds, for plugin task execution + * (defaults to {@link DEFAULT_TASK_TIMEOUT_SECS}) */ constructor( public readonly mcpServer: PenpotMcpServer, private port: number, - private readonly redisBridge?: RedisBridge, - private taskTimeoutSecs: number = 30 + private readonly taskTimeoutSecs: number, + private readonly redisBridge?: RedisBridge ) { this.wsServer = new WebSocketServer({ port: port }); this.setupWebSocketHandlers(); @@ -131,9 +135,10 @@ export class PluginBridge { /** * Removes a client connection and releases all resources associated with it. * - * Clears the per-connection keep-alive interval and removes the connection - * from both the socket-keyed and token-keyed indexes. Safe to call with a - * socket that is not (or no longer) registered. + * Clears the per-connection keep-alive interval and removes the connection from the + * socket-keyed index. The token-keyed index entry (and, in multi-instance mode, the + * token's Redis task subscription) is removed only if it is owned by the given + * connection. Safe to call with a socket that is not (or no longer) registered. * * @param ws - The WebSocket whose connection state should be removed */ @@ -145,12 +150,18 @@ export class PluginBridge { clearInterval(connection.pingInterval); this.connectedClients.delete(ws); if (connection.userToken) { - this.clientsByToken.delete(connection.userToken); + // Perform the token-keyed cleanup only if this connection owns the token registration. + // A connection rejected as a duplicate carries the same token but must not remove token associations. + if (this.clientsByToken.get(connection.userToken) !== connection) { + this.logger.debug("Removed connection does not own its token registration; skipping token cleanup"); + } else { + this.clientsByToken.delete(connection.userToken); - if (this.redisBridge) { - this.redisBridge - .unsubscribeFromTasks(connection.userToken) - .catch((error) => this.logger.error(error, "Failed to unsubscribe from Redis task channel")); + if (this.redisBridge) { + this.redisBridge + .unsubscribeFromTasks(connection.userToken) + .catch((error) => this.logger.error(error, "Failed to unsubscribe from Redis task channel")); + } } } } @@ -189,6 +200,35 @@ export class PluginBridge { this.logger.info(`Task ${response.id} completed: success=${response.success}`); } + /** + * Rejects a still-pending task with the given error, releasing its correlation state. + * + * Clears the task's timeout (if armed) and removes the task from the pending-task + * index before rejecting its promise. Safe to call for a task that has already been + * settled (e.g. by a response or a timeout), in which case nothing happens. + * + * @param taskId - The ID of the task to reject + * @param error - The error with which to reject the task + * @returns Whether the task was still pending and has been rejected + */ + private rejectPendingTask(taskId: string, error: Error): boolean { + const pendingTask = this.pendingTasks.get(taskId); + if (!pendingTask) { + return false; + } + + const timeoutHandle = this.taskTimeouts.get(taskId); + if (timeoutHandle) { + clearTimeout(timeoutHandle); + this.taskTimeouts.delete(taskId); + } + this.pendingTasks.delete(taskId); + + pendingTask.rejectWithError(error); + this.logger.info(`Task ${taskId} rejected: ${error.message}`); + return true; + } + /** * Determines the client connection to use for executing a task. * @@ -207,9 +247,7 @@ export class PluginBridge { const connection = this.clientsByToken.get(sessionContext.userToken); if (!connection) { - throw new Error( - `No plugin instance connected for user token. Please ensure the plugin is running and connected with the correct token.` - ); + throw new Error(PluginBridge.MULTIUSER_CONNECTION_ERROR_MESSAGE); } return connection; @@ -257,6 +295,10 @@ export class PluginBridge { * `resolveWithResult`/`rejectWithError` methods. The same correlation and timeout * handling therefore applies regardless of the transport. * + * When routing via Redis, the task is rejected immediately (rather than timing out) + * if the published request reached no instance, i.e. if no instance holds a plugin + * connection for the session's user token, or if publishing fails outright. + * * @param task - The task to dispatch * @param useRedis - Whether to route the request via Redis (multi-instance) rather * than directly over the local WebSocket connection @@ -279,9 +321,17 @@ export class PluginBridge { // register the task for result correlation, then publish the request via Redis this.pendingTasks.set(task.id, task); - void redisBridge.sendTaskRequest(userToken, task.toRequest(), (response) => - this.handlePluginTaskResponse(response) - ); + void redisBridge + .sendTaskRequest(userToken, task.toRequest(), (response) => this.handlePluginTaskResponse(response)) + .then((receiverCount) => { + // fail fast when no instance received the request (no connection with matching user token in any instance) + if (receiverCount === 0) { + this.rejectPendingTask(task.id, new Error(PluginBridge.MULTIUSER_CONNECTION_ERROR_MESSAGE)); + } + }) + .catch((error) => { + this.rejectPendingTask(task.id, error instanceof Error ? error : new Error(String(error))); + }); // on timeout, release the response-channel subscription, since no response // will arrive to trigger its self-unsubscribe. @@ -300,14 +350,13 @@ export class PluginBridge { // Set up a timeout to reject the task if no response is received const timeoutHandle = setTimeout(() => { - const pendingTask = this.pendingTasks.get(task.id); - if (pendingTask) { - this.pendingTasks.delete(task.id); - this.taskTimeouts.delete(task.id); - onTimeout?.(); - pendingTask.rejectWithError( + if ( + this.rejectPendingTask( + task.id, new Error(`Task ${task.id} timed out after ${this.taskTimeoutSecs} seconds`) - ); + ) + ) { + onTimeout?.(); } }, this.taskTimeoutSecs * 1000); diff --git a/mcp/packages/server/src/RedisBridge.ts b/mcp/packages/server/src/RedisBridge.ts index d8117d9242..89c01a4871 100644 --- a/mcp/packages/server/src/RedisBridge.ts +++ b/mcp/packages/server/src/RedisBridge.ts @@ -91,12 +91,16 @@ export class RedisBridge { * @param userToken - The user token identifying the target plugin's request channel * @param request - The serialized plugin task request, passed through verbatim * @param onResponse - Handler invoked with the response when it arrives + * @returns The number of instances that received the request. A count of 0 means no + * instance is subscribed to the token's request channel (i.e. the plugin is not + * connected anywhere); the request was dropped, no response will ever arrive, and + * the response subscription has already been released. */ async sendTaskRequest( userToken: string, request: PluginTaskRequest, onResponse: TaskResponseHandler - ): Promise { + ): Promise { const responseChannel = this.responseChannel(request.id); const requestChannel = this.requestChannel(userToken); @@ -113,7 +117,19 @@ export class RedisBridge { await this.subscriber.subscribe(responseChannel); // publish only once the response subscription is confirmed - await this.publisher.publish(requestChannel, JSON.stringify(request)); + let receiverCount: number; + try { + receiverCount = await this.publisher.publish(requestChannel, JSON.stringify(request)); + } catch (error) { + // the request was never delivered, so no response can arrive + await this.unsubscribeFromResponse(request.id); + throw error; + } + if (receiverCount === 0) { + // no subscriber received the request, so no response can arrive + await this.unsubscribeFromResponse(request.id); + } + return receiverCount; } /** diff --git a/scripts/ci b/scripts/ci new file mode 100755 index 0000000000..df693a9cd0 --- /dev/null +++ b/scripts/ci @@ -0,0 +1,478 @@ +#!/usr/bin/env bash +# scripts/ci - CI orchestration script for Penpot monorepo + +set -euo pipefail + +# Constants +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" +LOG_DIR="$PROJECT_ROOT/.ci-logs" + +# Color codes +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +CYAN='\033[0;36m' +BOLD='\033[1m' +NC='\033[0m' + +# Available modules +ALL_MODULES=("frontend" "backend" "common" "render-wasm" "exporter" "mcp" "plugins" "library") + +# Module commands +declare -A LINT_CMD=( + [frontend]="pnpm run lint:clj && pnpm run lint:js && pnpm run lint:scss" + [backend]="pnpm run lint" + [common]="pnpm run lint:clj" + [render-wasm]="./lint" + [exporter]="pnpm run lint" + [mcp]="" + [plugins]="pnpm run lint" + [library]="pnpm run lint" +) + +declare -A TEST_CMD=( + [frontend]="pnpm run test:quiet" + [backend]="clojure -M:dev:test" + [common]="clojure -M:dev:test && pnpm run test:quiet" + [render-wasm]="./test" + [exporter]="" + [mcp]="pnpm run test" + [plugins]="pnpm run test" + [library]="pnpm run test" +) + +declare -A FMT_CHECK_CMD=( + [frontend]="pnpm run check-fmt:clj && pnpm run check-fmt:js && pnpm run check-fmt:scss" + [backend]="pnpm run check-fmt" + [common]="pnpm run check-fmt:clj && pnpm run check-fmt:js" + [render-wasm]="cargo fmt --check" + [exporter]="pnpm run check-fmt" + [mcp]="pnpm run fmt:check" + [plugins]="pnpm run format:check" + [library]="pnpm run check-fmt" +) + +declare -A FMT_FIX_CMD=( + [frontend]="pnpm run fmt" + [backend]="pnpm run fmt" + [common]="pnpm run fmt:clj && pnpm run fmt:js" + [render-wasm]="cargo fmt" + [exporter]="pnpm run fmt" + [mcp]="pnpm run fmt" + [plugins]="pnpm run format" + [library]="pnpm run fmt" +) + +# Default options +MODULES=() +TASKS=("lint" "test" "fmt") +FIX_MODE=false +FAIL_FAST=false +VERBOSE=true +DRY_RUN=false + +# Results tracking +declare -A RESULTS=() +declare -a FAILED_LOGS=() + +timestamp() { + date "+%H:%M:%S" +} + +log_info() { + echo -e "${BLUE}[$(timestamp)]${NC} $1" +} + +log_success() { + echo -e "${GREEN}[$(timestamp)] ✓${NC} $1" +} + +log_error() { + echo -e "${RED}[$(timestamp)] ✗${NC} $1" +} + +log_warning() { + echo -e "${YELLOW}[$(timestamp)] ⚠${NC} $1" +} + +log_header() { + local module=$1 + local current=$2 + local total=$3 + echo "" + echo -e "${CYAN}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${CYAN} ${BOLD}[$current/$total] $module${NC}" + echo -e "${CYAN}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" +} + +run_task() { + local module=$1 + local task=$2 + local cmd=$3 + local logfile="$LOG_DIR/${module}-${task}.log" + + if [[ -z "$cmd" ]]; then + log_warning "$task: not defined for $module, skipping" + RESULTS["$module:$task"]="SKIPPED" + return 0 + fi + + if [[ "$DRY_RUN" == "true" ]]; then + log_info "$task: would run '$cmd' in $module/" + RESULTS["$module:$task"]="DRY-RUN" + return 0 + fi + + log_info "Running ${BOLD}$task${NC} for $module..." + + mkdir -p "$LOG_DIR" + local start_time + start_time=$(date +%s) + + if [[ "$VERBOSE" == "true" ]]; then + echo -e "${BLUE} cmd: $cmd${NC}" + fi + + local exit_code=0 + (cd "$PROJECT_ROOT/$module" && eval "$cmd") > "$logfile" 2>&1 || exit_code=$? + + local end_time + end_time=$(date +%s) + local duration=$((end_time - start_time)) + + if [[ $exit_code -eq 0 ]]; then + log_success "$task passed for $module ${BLUE}(${duration}s)${NC}" + RESULTS["$module:$task"]="PASSED" + return 0 + else + log_error "$task failed for $module ${BLUE}(${duration}s)${NC}" + echo -e " ${RED}log: $logfile${NC}" + RESULTS["$module:$task"]="FAILED" + FAILED_LOGS+=("$logfile") + if [[ "$VERBOSE" == "true" ]]; then + echo -e "${YELLOW} --- last 30 lines ---${NC}" + tail -n 30 "$logfile" | sed 's/^/ /' + echo -e "${YELLOW} ---------------------${NC}" + fi + return 1 + fi +} + +run_module() { + local module=$1 + local failed=false + + for task in "${TASKS[@]}"; do + local cmd="" + case $task in + lint) + cmd="${LINT_CMD[$module]:-}" + ;; + test) + cmd="${TEST_CMD[$module]:-}" + ;; + fmt) + if [[ "$FIX_MODE" == "true" ]]; then + cmd="${FMT_FIX_CMD[$module]:-}" + else + cmd="${FMT_CHECK_CMD[$module]:-}" + fi + ;; + esac + + if ! run_task "$module" "$task" "$cmd"; then + failed=true + if [[ "$FAIL_FAST" == "true" ]]; then + return 1 + fi + fi + done + + if [[ "$failed" == "true" ]]; then + return 1 + fi + return 0 +} + +print_summary() { + echo "" + echo -e "${CYAN}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${CYAN} ${BOLD}SUMMARY${NC}" + echo -e "${CYAN}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + + local total=0 + local passed=0 + local failed=0 + local skipped=0 + + for module in "${MODULES[@]}"; do + for task in "${TASKS[@]}"; do + local result="${RESULTS[$module:$task]:-N/A}" + total=$((total + 1)) + case $result in + PASSED) + passed=$((passed + 1)) + echo -e " ${GREEN}✓${NC} $module:$task" + ;; + FAILED) + failed=$((failed + 1)) + echo -e " ${RED}✗${NC} $module:$task" + ;; + SKIPPED) + skipped=$((skipped + 1)) + echo -e " ${YELLOW}○${NC} $module:$task" + ;; + DRY-RUN) + echo -e " ${BLUE}~${NC} $module:$task" + ;; + esac + done + done + + echo "" + echo -e " ${BOLD}Total:${NC} $total ${GREEN}Passed:${NC} $passed ${RED}Failed:${NC} $failed ${YELLOW}Skipped:${NC} $skipped" + + if [[ $failed -gt 0 ]]; then + echo "" + echo -e " ${RED}${BOLD}Failed logs:${NC}" + for logfile in "${FAILED_LOGS[@]}"; do + echo -e " ${RED}•${NC} $logfile" + done + return 1 + fi + return 0 +} + +validate_module() { + local mod=$1 + for valid in "${ALL_MODULES[@]}"; do + if [[ "$valid" == "$mod" ]]; then + return 0 + fi + done + return 1 +} + +usage() { + cat <&2 + echo "Run '$(basename "$0") --help' for usage." >&2 + exit 1 + ;; + *) + if validate_module "$1"; then + MODULES+=("$1") + else + echo -e "${RED}Error: Unknown module '$1'${NC}" >&2 + echo "Valid modules: ${ALL_MODULES[*]}" >&2 + exit 1 + fi + shift + ;; + esac + done + + # Clean empty entries from TASKS after removal + TASKS=("${TASKS[@]// /}") + TASKS=("${TASKS[@]/#/}") + local clean_tasks=() + for t in "${TASKS[@]}"; do + if [[ -n "$t" ]]; then + clean_tasks+=("$t") + fi + done + TASKS=("${clean_tasks[@]}") + + # Set modules + if [[ "$run_all" == "true" ]]; then + MODULES=("${ALL_MODULES[@]}") + fi + + # Apply exclusions + if [[ ${#exclude_modules[@]} -gt 0 ]]; then + local filtered=() + for mod in "${MODULES[@]}"; do + local excluded=false + for ex in "${exclude_modules[@]}"; do + if [[ "$mod" == "$ex" ]]; then + excluded=true + break + fi + done + if [[ "$excluded" == "false" ]]; then + filtered+=("$mod") + fi + done + MODULES=("${filtered[@]}") + fi + + # Validate + if [[ ${#MODULES[@]} -eq 0 ]]; then + echo -e "${RED}Error: No modules specified.${NC}" >&2 + echo "Use --all or specify modules: ${ALL_MODULES[*]}" >&2 + exit 1 + fi + + if [[ ${#TASKS[@]} -eq 0 ]]; then + echo -e "${RED}Error: No tasks selected.${NC}" >&2 + exit 1 + fi + + # Print configuration + echo "" + echo -e "${CYAN}${BOLD}Penpot CI Orchestrator${NC}" + echo -e "${CYAN}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e " ${BOLD}Modules:${NC} ${MODULES[*]}" + echo -e " ${BOLD}Tasks:${NC} ${TASKS[*]}" + echo -e " ${BOLD}Fix mode:${NC} $FIX_MODE" + echo -e " ${BOLD}Fail-fast:${NC} $FAIL_FAST" + if [[ "$DRY_RUN" == "true" ]]; then + echo -e " ${BOLD}Mode:${NC} ${YELLOW}DRY RUN${NC}" + fi + echo "" + + # Run + local total_modules=${#MODULES[@]} + local current=0 + local modules_failed=0 + local start_time + start_time=$(date +%s) + + for module in "${MODULES[@]}"; do + current=$((current + 1)) + log_header "$module" "$current" "$total_modules" + + if ! run_module "$module"; then + modules_failed=$((modules_failed + 1)) + if [[ "$FAIL_FAST" == "true" ]]; then + log_error "Fail-fast: stopping due to failure in $module" + break + fi + fi + done + + local end_time + end_time=$(date +%s) + local total_duration=$((end_time - start_time)) + + print_summary || true + + echo "" + echo -e " ${BOLD}Duration:${NC} ${total_duration}s" + echo "" + + if [[ $modules_failed -gt 0 ]]; then + exit 1 + fi + exit 0 +} + +main "$@"