diff --git a/.serena/memories/clojure/promesa.md b/.serena/memories/clojure/promesa.md new file mode 100644 index 0000000000..75cb5f6642 --- /dev/null +++ b/.serena/memories/clojure/promesa.md @@ -0,0 +1,9 @@ +# Promesa + +Promise library of the monorepo (frontend CLJS and common, `funcool/promesa` 12.0.1 in `common/deps.edn`). Its API has two call styles and mixing them in one expression is the trap: `fmap`/`mcat`/`merr` are **fn first, promise last** (`->>` friendly) and `then`/`then'`/`catch`/`handle` are **promise first, fn last** (`->` friendly). Convention: `fmap`/`mcat`/`merr` inside `->>` pipelines, `then`/`catch` with `->`. A `then` written inside a `->>` reads backwards and hides whether the step carries a value or a promise. +- `fmap` (alias `map`, both `(f p)`) is functor map: `f` returns a plain value. +- `mcat` (alias `mapcat`, both `(f p)`) is monadic bind: `f` must return a promise, and the result mirrors it. +- `then` also unwraps a promise returned by `f`, which is why it covers both cases at the cost of not naming which one it is; `then'` maps without unwrapping. +- `merr` (fn first) and `catch` (promise first) are the error side. +- `all` takes a collection of promises (a lazy seq works); `let` chains sequential bindings. +- Seam with the rx pipelines of the frontend: `beicon.v2.core/from` builds an observable from "arrays, collections, and promise instance" (`external/beicon`), so `(->> p (p/mcat f) (rx/from) (rx/map g))` is the usual bridge (`app.main.data.imports`, `app.util.zip`). diff --git a/.serena/memories/critical-info.md b/.serena/memories/critical-info.md index 6ad09c4f7e..28e1043431 100644 --- a/.serena/memories/critical-info.md +++ b/.serena/memories/critical-info.md @@ -8,6 +8,7 @@ You are working on the GitHub project `penpot/penpot`, a monorepo. - Cross-cutting testing principles, TDD workflow, and anti-patterns: `mem:testing`. - Verified Clojure language behaviors that contradict common assumptions (e.g. `int?` covers `Long`; `integer?` is the general predicate): `mem:clojure/idioms` — read before assuming stdlib predicate semantics. - Read `mem:clojure/design-rules` (function shape, boundary checks, naming) before writing or modifying Clojure/ClojureScript code — not only when designing a solution or an API. +- Promises: which call style to use where (`fmap`/`mcat`/`merr` vs `then`/`catch`) and how they bridge to rx: `mem:clojure/promesa` — read before chaining promises or mixing them with observables. # Development workflow diff --git a/.serena/memories/frontend/dashboard-viewer-subtleties.md b/.serena/memories/frontend/dashboard-viewer-subtleties.md index 9c753d8875..db2158071c 100644 --- a/.serena/memories/frontend/dashboard-viewer-subtleties.md +++ b/.serena/memories/frontend/dashboard-viewer-subtleties.md @@ -5,7 +5,7 @@ - Dashboard initialization fetches projects and fonts for the team, then listens to websocket messages only for global topic `uuid/zero` or the current profile id. - Project fetch replaces each project map completely instead of merging, so fields such as `deleted-at` can disappear cleanly. - Dashboard file/project mutations are often optimistic local updates with fire-and-forget RPC watchers. Bulk permanent delete/restore paths use SSE progress and progress notifications. -- The import wizard runs the package as a job of the backend: it reads the manifest of the file in the main thread (`app.main.data.imports`, no web worker) and follows the job, so its rows paint the milestone of the job (the stage and its counter, with the file in course) instead of the messages the old SSE stream sent. Before the job exists, the rows paint a client-side `:upload` milestone built from the chunked upload (`uploads/upload-blob-chunked` `:on-progress`, chunks done/total), which never comes from the backend; the chunks go one at a time and the row writes them as a percentage of the file, not as a counter. Once created and still waiting for a worker, each entry reports `:queued` ("Queued"); the `:start`/`:retry` events move it to bare progress, and only `:progress` carries a milestone. The export modal follows the same one-job-per-file model, but every file is queued up front (even the ones whose job is created only after the previous one is over) and its queue ends only with a milestone: `:start`/`:retry` do not end it there. Neither modal paints a generic "…ing file…" line any more: each row carries its own status. The `:error` of an import entry is user-facing text (the backend hint, or a translated label), never an i18n key: the wizard renders it as-is, it does not translate it again. The link-later step takes the file id from the `:id` of its resolution entry, never from the key of the resolution map: the map comes from the job result, so what a key holds depends on the result schema of the job-def that wrote it, while the wizard indexes its selection by the id of the entry. The legacy SSE import stays in the backend for other clients. +- The import wizard runs the package as a job of the backend: it reads the manifest of the file in the main thread (`app.main.data.imports`, no web worker) and follows the job, so its header paints the milestone of the job (the stage and its counter) instead of the messages the old SSE stream sent; every file of a container reports the same milestone, so `jp/current-milestone-text` writes it once and the rows never repeat it. Before the job exists, the header paints a client-side `:upload` milestone built from the chunked upload (`uploads/upload-blob-chunked` `:on-progress`, chunks done/total), which never comes from the backend; the chunks go one at a time and it writes them as a percentage of the file, not as a counter. Once created and still waiting for a worker, no row says anything: the header says the import waits for a slot while no job of it has started. The bar of the header measures the whole import, never a file: indeterminate while checking, waiting for a slot or rebuilding, because the client cannot know a global index and total of operations. The upload is the exception — the chunked upload knows its chunks, so the bar is determinate while the containers are sent, weighted by their size (`imp/upload-pct` over `:container-size`, the only percentage the client really knows). The export modal follows the same one-job-per-file model, but every file is queued up front (even the ones whose job is created only after the previous one is over) and its queue ends only with a milestone: `:start`/`:retry` do not end it there. Neither modal paints a generic "…ing file…" line on the rows any more: the rows of the import wizard carry only what is not progress — the name, the size, `Ready` once their container finished uploading, and the error text —, and the button that drops a file only shows while the wizard still offers its import step (the confirmation screen). Above the rows, the import wizard paints a stepper (Check/Upload/Import) plus one phase line: `imp/import-step` derives the step from the dialog status and the entries (upload holds while any file is pending, either `:import-ready` or carrying its own `:upload` milestone), so Check covers analyze, Upload covers the chunked client-side upload, and Import covers queue plus job. A file of a version 3 package shows what it occupies inside its container, computed while analyzing (`imp/file-sizes`): its own `files/…` entries plus the storage objects its media and thumbnails reference, whose metadata is the only link to `objects/…`. The container's own size stays in `:container-size` (what the dropped file weighs) and is what the upload bar weighs, never what a row shows. A file the manifest lists but the container holds no entry of has no size at all, and its row then shows no subtitle: what the package weighs never stands in for it. The subtitle is the size alone, with no extension. While analyzing with no failures nothing renders below the phase header — no rows, no spinner (`imp/awaiting-analysis?`); the header text is the only "Checking…" message. Rows appear once the analysis resolves (one per file inside the package), and a failed analysis takes the error-summary path instead. The ready step and its Continue wait for the end of the analysis (`imp/ready-to-import?`, fed with the completion of `imp/analyze`): a ready file is not enough, because the files still to arrive would never be imported. While rebuilding, the phase title counts the files and `imp/imported-count` counts them done with the job's own `:files` counter, so the number moves while a container rebuilds its files one by one. The `:error` of an import entry is user-facing text (the backend hint, or a translated label), never an i18n key: the wizard renders it as-is, it does not translate it again. The link-later step takes the file id from the `:id` of its resolution entry, never from the key of the resolution map: the map comes from the job result, so what a key holds depends on the result schema of the job-def that wrote it, while the wizard indexes its selection by the id of the entry. The legacy SSE import stays in the backend for other clients. - File creation/duplication strips file `:data` before putting file summaries into dashboard state. ## Viewer diff --git a/.serena/memories/frontend/ui-conventions-and-style-system.md b/.serena/memories/frontend/ui-conventions-and-style-system.md index 1161b55d3a..59390c27e7 100644 --- a/.serena/memories/frontend/ui-conventions-and-style-system.md +++ b/.serena/memories/frontend/ui-conventions-and-style-system.md @@ -12,7 +12,7 @@ - Co-located SCSS modules are preferred. Use `app.main.style/stl` helpers from CLJS and design-system SCSS tokens/mixins instead of legacy global selectors or high-specificity nesting. - Keep CSS specificity low. Avoid nested selectors unless they target elements the component owns; CSS Modules already prevent class-name collisions. -- Prefer CSS logical properties for directional spacing/layout (`padding-inline-start`, etc.). Physical `width`/`height` are still acceptable where they are clearer. +- Prefer CSS logical properties for directional spacing/layout (`padding-inline-start`, etc.). Physical `width`/`height` are still acceptable where they are clearer. Never mix: a stylesheet logical property (`inline-size`) and an inline physical `width` on the same element cascade separately, and the logical mapping wins — a dynamically driven width must be `width` on both sides (import modal progress bar hit this). And an inline style passed from CLJS must be a JS object (`#js {:width …}`): for plain `[:div …]` elements the Rumext macro does not convert the `:style` map at runtime — a plain CLJS map reaches React raw and React never finds the keys (static style literals are converted at compile time, which is why `:style` maps seem to work elsewhere). - Use named design-system variables/tokens for spacing, borders, fixed dimensions, colors, and typography. Avoid hardcoded px/rem values and deprecated `resources/styles/common/refactor/spacing.scss` variables. - Use component-local CSS custom properties for variants and theming instead of one-off Sass variables when a component has multiple visual states. - Prefer DS typography components (`heading*`, `text*`) and typography mixins instead of plain text wrappers or deprecated typography mixins. diff --git a/backend/scripts/_env b/backend/scripts/_env index f777d16a8a..800ad70a08 100644 --- a/backend/scripts/_env +++ b/backend/scripts/_env @@ -92,6 +92,7 @@ export JAVA_OPTS="\ -XX:+UseShenandoahGC \ -XX:+UseCompactObjectHeaders \ -XX:ShenandoahGCMode=generational \ + -XX:NativeMemoryTracking=summary \ -XX:-OmitStackTraceInFastThrow \ --sun-misc-unsafe-memory-access=allow \ --enable-preview \ diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 746dd3e55a..e9c091e872 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -1080,12 +1080,13 @@ :else :detach-libraries)) (defn export-cfg - "The domain cfg of an export: the frozen set of files and how their - libraries are handled." - [cfg {:keys [ids export-type]}] + "The domain cfg of an export: the frozen set of files, how their + libraries are handled, and where the package is written." + [cfg {:keys [file-ids export-type output]}] (-> cfg - (assoc ::ids ids) - (assoc ::export-type export-type))) + (assoc ::file-ids file-ids) + (assoc ::export-type export-type) + (assoc ::output output))) (defn import-cfg "The domain cfg of an import: the destination team/profile/project, the diff --git a/backend/src/app/binfile/jobs.clj b/backend/src/app/binfile/jobs.clj index 950d157220..3bdfe668db 100644 --- a/backend/src/app/binfile/jobs.clj +++ b/backend/src/app/binfile/jobs.clj @@ -5,7 +5,7 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns app.binfile.jobs - "The binfile work a job runs. + "The progress bridge of the binfile jobs. A job has no SSE channel, so the `:progress` points that `bf.v1` and `bf.v3` emit become `progress` events of the job through @@ -17,20 +17,15 @@ so the adapter holds no state between taps. The ids the taps carry are not part of the job event contract. - The adapter checks that the job is still active before it starts, and - the beat checks it on every write: a job that was cancelled, or that + The handlers in `app.tasks.export-binfile` and + `app.tasks.import-binfile` run the core through `run` and + guard it with `check-active`: a job that was cancelled, or that already reached a terminal state, raises an `:interrupt` right there on the run thread, so the work stops at its next event point and the transaction of its handler rolls back with it. A run whose last tap happened before the interruption is discarded once it returns, which is what makes that transaction roll back too." (:require - [app.binfile.common :as bfc] - [app.binfile.v1 :as bf.v1] - [app.binfile.v3 :as bf.v3] - [app.common.exceptions :as ex] - [app.common.time :as ct] - [app.db :as db] [app.jobs :as jobs] [app.util.events :as events])) @@ -39,7 +34,7 @@ The substrate owns both the check and the error; this is the adapter's entry point to it, used by the two handlers before their run and by the - tests." + tests. Answers with the cfg, so a handler can thread it." [cfg context] (jobs/check-active cfg (:id context))) @@ -98,10 +93,13 @@ (assoc :counters (cond-> (or outer-counters {}) (some? own) (assoc stage own)))))) -(defn- with-progress +(defn run "Run `f` with the progress taps of the core turned into milestones of the job. + `f` is called with the cfg, so the adapter forwards what it receives + and a caller can pass the core directly. + Every tap is reported: the substrate throttles the durable events, and a milestone that does not fit loses no information because the next one carries the same state. @@ -117,55 +115,6 @@ :job-id job-id :progress (milestone data))))] (binding [events/*sink* sink] - (let [result (f)] + (let [result (f cfg)] (check-active cfg context) result)))) - -(defn export-files - "Export a set of files into `output` as part of a job, reporting - progress as job events." - [cfg context {:keys [ids export-type output]}] - (check-active cfg context) - (with-progress cfg context - #(bf.v3/export-files! (bfc/export-cfg cfg {:ids ids - :export-type export-type}) - output))) - -(defn- touch-project - "Mark the destination project as modified. - - An import adds files, and the triggers of the schema only touch the - project on an update, so a project that just received files keeps its - old date and stays where it was in the list." - [cfg project-id] - (db/update! cfg :project - {:modified-at (ct/now)} - {:id project-id} - {::db/return-keys false})) - -(defn import-files - "Import the package at `input` as part of a job, reporting progress as - job events. The package was read from the resource of the job, so the - caller resolves the destination team and the adapter only fixes what - the legacy RPC and a job must share. - - Both formats answer with the same shape: the version 1 reader returns - the set of files it created, and the version 3 one returns a map that - already carries them together with the resolution of the libraries." - [cfg context {:keys [profile-id project-id team name input version]}] - (check-active cfg context) - (let [cfg (bfc/import-cfg cfg {:profile-id profile-id - :project-id project-id - :team team - :name name - :input input})] - (with-progress cfg context - #(let [result (case (int version) - 1 (bf.v1/import-files! cfg) - 3 (bf.v3/import-files! cfg) - (ex/raise :type :validation - :code :unsupported-version - :hint "unsupported binfile version" - :version version))] - (touch-project cfg project-id) - result)))) diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index 649494a2c6..76fc870f5d 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -329,13 +329,13 @@ [:v1/metadata :v1/files :v1/rels :v1/sobjects])))) (defmethod write-section :v1/metadata - [{:keys [::output ::bfc/ids ::bfc/include-libraries] :as cfg}] - (if-let [fids (get-files cfg ids)] + [{:keys [::output ::bfc/file-ids ::bfc/include-libraries] :as cfg}] + (if-let [fids (get-files cfg file-ids)] (let [lids (when include-libraries - (bfc/get-libraries cfg ids)) - ids (into fids lids)] - (write-obj! output {:version cf/version :files ids}) - (vswap! bfc/*state* assoc :files ids)) + (bfc/get-libraries cfg file-ids)) + file-ids (into fids lids)] + (write-obj! output {:version cf/version :files file-ids}) + (vswap! bfc/*state* assoc :files file-ids)) (ex/raise :type :not-found :code :files-not-found :hint "unable to retrieve files for export"))) @@ -689,12 +689,12 @@ `::bfc/embed-assets`: instead of including the libraries, embed in the same file library all assets used from external libraries." - [{:keys [::bfc/ids] :as cfg} output] + [{:keys [::bfc/file-ids] :as cfg} output] (dm/assert! - "expected a set of uuid's for `::bfc/ids` parameter" - (and (set? ids) - (every? uuid? ids))) + "expected a set of uuid's for `::bfc/file-ids` parameter" + (and (set? file-ids) + (every? uuid? file-ids))) (dm/assert! "expected instance of jio/IOFactory for `input`" diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index d599f766ed..59c344cf02 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -454,14 +454,15 @@ (write-entry! output path encoded-status))))) (defn- export-files - [{:keys [::bfc/ids ::bfc/export-type ::output] :as cfg}] + [{:keys [::bfc/file-ids ::bfc/export-type ::output] :as cfg}] - (let [original-ids ids - ids (into ids (when (= export-type :include-libraries) (bfc/get-libraries cfg ids))) - rels (if (= export-type :include-libraries) - (->> (bfc/get-files-rels cfg ids) - (mapv (juxt :file-id :library-file-id))) - []) + (let [original-ids file-ids + file-ids (into file-ids (when (= export-type :include-libraries) + (bfc/get-libraries cfg file-ids))) + rels (if (= export-type :include-libraries) + (->> (bfc/get-files-rels cfg file-ids) + (mapv (juxt :file-id :library-file-id))) + []) ;; Compute external libraries: referenced by original files but ;; not included in the export set. Only relevant for :link-later. @@ -486,11 +487,11 @@ (vswap! bfc/*state* assoc :files (d/ordered-map)) ;; Write all the exporting files - (doseq [[index file-id] (d/enumerate ids)] + (doseq [[index file-id] (d/enumerate file-ids)] (-> cfg (assoc ::file-id file-id) (with-outer-counter :files {:current (inc index) - :total (count ids)}) + :total (count file-ids)}) (export-file))) ;; Write manifest file @@ -1384,24 +1385,25 @@ (defn export-files! "Do the exportation of a specified file in custom penpot binary - format. There are some options available for customize the output: + format. The cfg carries what `bfc/export-cfg` froze: `::bfc/file-ids` + (the files to export), `::bfc/export-type` (how linked libraries are + handled) and `::bfc/output` (where the package is written). - `::bfc/export-type`: determines how linked libraries are handled. - Valid values: - `:include-libraries` (include linked libraries), + `::bfc/export-type` determines how linked libraries are handled. + Valid values: `:include-libraries` (include linked libraries), `:merge-libraries` (embed library assets in the file), `:detach-libraries` (treat assets as basic objects), `:link-later` (preserve component metadata for relinking on import)." - [{:keys [::bfc/ids] :as cfg} output] + [{:keys [::bfc/file-ids ::bfc/output] :as cfg}] (assert - (and (set? ids) (every? uuid? ids)) - "expected a set of uuid's for `::bfc/ids` parameter") + (and (set? file-ids) (every? uuid? file-ids)) + "expected a set of uuid's for `::bfc/file-ids` parameter") (assert (satisfies? jio/IOFactory output) - "expected instance of jio/IOFactory for `input`") + "expected instance of jio/IOFactory for `::bfc/output`") (let [id (uuid/next) tp (ct/tpoint) diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index 6995464290..01982605b4 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -359,10 +359,11 @@ (let [path (tmp/tempfile :prefix "penpot.export." :min-age "30m")] (with-open [output (io/output-stream path)] (-> cfg - (assoc ::bfc/ids file-ids) + (assoc ::bfc/file-ids file-ids) (assoc ::bfc/embed-assets embed?) (assoc ::bfc/include-libraries libs?) - (bf.v3/export-files! output))) + (assoc ::bfc/output output) + (bf.v3/export-files!))) (if clone? (let [profile (profile/get-profile pool profile-id) diff --git a/backend/src/app/jobs.clj b/backend/src/app/jobs.clj index caf19b6f4e..271414ebcc 100644 --- a/backend/src/app/jobs.clj +++ b/backend/src/app/jobs.clj @@ -847,7 +847,8 @@ The interrupt is an internal signal, not an error: the caller is a task that must stop the work in course. `heartbeat` calls this when one of its writes finds the job gone, and a caller can also ask explicitly, - which is what the binfile adapter does before and after its run." + which is what the binfile adapter does before and after its run. + Answers with the cfg, so a caller can thread the check into its run." [cfg job-id] (let [job (get-job cfg job-id)] (when-not (and (some? job) (contains? active-statuses (:status job))) @@ -855,7 +856,8 @@ :code :job-interrupted :hint "the job is no longer active" :job-id job-id - :status (or (:status job) "gone"))))) + :status (or (:status job) "gone"))) + cfg)) (defn heartbeat "Keep a running job alive and, optionally, report its progress. diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 593a06a028..35dbf7eb67 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -50,9 +50,10 @@ (let [output (tmp/tempfile*)] (try - (-> (bfc/export-cfg cfg {:ids #{file-id} - :export-type (bfc/resolve-export-type params)}) - (bf.v3/export-files! output)) + (-> (bfc/export-cfg cfg {:file-ids #{file-id} + :export-type (bfc/resolve-export-type params) + :output output}) + (bf.v3/export-files!)) (let [data (sto/content output) object (sto/put-object! storage diff --git a/backend/src/app/tasks/export_binfile.clj b/backend/src/app/tasks/export_binfile.clj index eaa1ba0f55..5ac105a087 100644 --- a/backend/src/app/tasks/export_binfile.clj +++ b/backend/src/app/tasks/export_binfile.clj @@ -8,7 +8,9 @@ "The `:export-binfile` job: it produces the `.penpot` artifact of the set of files frozen in its params." (:require + [app.binfile.common :as bfc] [app.binfile.jobs :as bfj] + [app.binfile.v3 :as bf.v3] [app.common.schema :as sm] [app.db :as db] [app.jobs :as jobs] @@ -64,14 +66,17 @@ (doseq [file-id file-ids] (files/check-read-permissions! cfg profile-id file-id)) - (let [output (tmp/tempfile* :suffix ".penpot")] + (let [output (tmp/tempfile* :suffix ".penpot") + cfg (bfc/export-cfg cfg {:file-ids file-ids + :export-type (:export-type params) + :output output})] (try - (bfj/export-files cfg context {:ids file-ids - :export-type (:export-type params) - :output output}) - (let [resource (js/put-resource cfg profile-id - (assoc artifact :content (sto/content output)))] - {:result (check-result (dissoc resource :resource-id)) + (bfj/check-active cfg context) + (bfj/run cfg context bf.v3/export-files!) + (let [resource (->> (assoc artifact :content (sto/content output)) + (js/put-resource cfg profile-id)) + result (check-result (dissoc resource :resource-id))] + {:result result :resource-id (:resource-id resource)}) (finally (fs/delete output)))))) diff --git a/backend/src/app/tasks/import_binfile.clj b/backend/src/app/tasks/import_binfile.clj index 754bb4bf25..27949b0074 100644 --- a/backend/src/app/tasks/import_binfile.clj +++ b/backend/src/app/tasks/import_binfile.clj @@ -8,9 +8,13 @@ "The `:import-binfile` job: it imports the package the job owns as a resource into the project frozen in its params." (:require + [app.binfile.common :as bfc] [app.binfile.jobs :as bfj] + [app.binfile.v1 :as bf.v1] + [app.binfile.v3 :as bf.v3] [app.common.exceptions :as ex] [app.common.schema :as sm] + [app.common.time :as ct] [app.db :as db] [app.jobs :as jobs] [app.jobs.storage :as js] @@ -101,6 +105,18 @@ :type :validation :code :invalid-result)) +(defn- touch-project + "Mark the destination project as modified. + + An import adds files, and the triggers of the schema only touch the + project on an update, so a project that just received files keeps its + old date and stays where it was in the list." + [cfg project-id] + (db/update! cfg :project + {:modified-at (ct/now)} + {:id project-id} + {::db/return-keys false})) + (defn execute-import "Plain handler, importable and testable without integrant. @@ -124,32 +140,38 @@ (try (projects/check-edition-permissions! cfg profile-id project-id) - (let [team (or (teams/get-team cfg - :profile-id profile-id - :project-id project-id) - (ex/raise :type :not-found - :code :project-not-found - :hint "the destination project is not available" - :profile-id profile-id - :project-id project-id)) - result (db/tx-run! cfg - (fn [tx-cfg] - ;; the core joins this transaction: it - ;; is the one that decides whether the - ;; whole import is kept or rolled back - (check-result - (assoc (bfj/import-files tx-cfg context - {:profile-id profile-id - :project-id project-id - :team team - :name (:name params) - :input input - :version (:version params)}) - :name (:name params) - :version (:version params)))))] - - result) - + (let [team (or (teams/get-team cfg + :profile-id profile-id + :project-id project-id) + (ex/raise :type :not-found + :code :project-not-found + :hint "the destination project is not available" + :profile-id profile-id + :project-id project-id))] + (db/tx-run! cfg (fn [tx-cfg] + ;; the core joins this transaction: it + ;; is the one that decides whether the + ;; whole import is kept or rolled back + (-> (bfc/import-cfg tx-cfg {:profile-id profile-id + :project-id project-id + :team team + :name (:name params) + :input input}) + (bfj/check-active context) + (bfj/run context + (fn [cfg] + (let [result (case (int (:version params)) + 1 (bf.v1/import-files! cfg) + 3 (bf.v3/import-files! cfg) + (ex/raise :type :validation + :code :unsupported-version + :hint "unsupported binfile version" + :version (:version params)))] + (touch-project cfg project-id) + result))) + (assoc :name (:name params)) + (assoc :version (:version params)) + (check-result))))) (finally (fs/delete input) (js/release-input cfg context))))) diff --git a/backend/test/backend_tests/binfile_jobs_test.clj b/backend/test/backend_tests/binfile_jobs_test.clj index e03cb79421..d5f33dbdd9 100644 --- a/backend/test/backend_tests/binfile_jobs_test.clj +++ b/backend/test/backend_tests/binfile_jobs_test.clj @@ -10,6 +10,7 @@ (:require [app.binfile.common :as bfc] [app.binfile.jobs :as bfj] + [app.binfile.v1 :as v1] [app.binfile.v3 :as v3] [app.common.time :as ct] [app.common.uuid :as uuid] @@ -59,6 +60,37 @@ [job-id] (jobs/make-context (jobs/get-job th/*system* job-id))) +(defn- run-export! + "Run an export through the adapter, as the job handler does: the + frozen cfg in, the progress out as job events." + [cfg context file-ids export-type output] + (let [cfg (bfc/export-cfg cfg {:file-ids file-ids + :export-type export-type + :output output})] + (bfj/check-active cfg context) + (bfj/run cfg context v3/export-files!))) + +(defn- run-import! + "Run an import through the adapter, as the job handler does: the + frozen cfg in, the progress out as job events. The project touch + stays in the handler, covered by its own tests." + [cfg context {:keys [profile-id project-id team name input version]}] + (bfj/check-active cfg context) + (let [cfg (bfc/import-cfg cfg {:profile-id profile-id + :project-id project-id + :team team + :name name + :input input})] + (bfj/run cfg context + (fn [cfg] + (case (int version) + 1 (v1/import-files! cfg) + 3 (v3/import-files! cfg) + (throw (ex-info "unsupported binfile version" + {:type :validation + :code :unsupported-version + :version version}))))))) + (defn- progress-events "The progress events of a job, oldest first, with decoded payloads." [job-id] @@ -88,9 +120,9 @@ :library-file-id (:id library)}) (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) (db/update! th/*system* :file {:deleted-at (ct/now)} @@ -118,10 +150,8 @@ job-id (th/create-job* :export-binfile {:profile-id (:id profile)}) output (tmp/tempfile* :suffix ".penpot")] - (bfj/export-files (job-cfg) (job-context job-id) - {:ids #{file-id} - :export-type :detach-libraries - :output output}) + (run-export! (job-cfg) (job-context job-id) + #{file-id} :detach-libraries output) (t/testing "the artifact is a penpot file with its manifest" (t/is (fs/exists? output)) @@ -150,13 +180,13 @@ job-id (th/create-job* :import-binfile {:profile-id (:id profile) :resource-id (:resource-id staged)}) context (job-context job-id) - result (bfj/import-files (job-cfg) context - {:profile-id (:id profile) - :project-id (:default-project-id profile) - :team team - :name "imported" - :input (js/load-input th/*system* context) - :version 3})] + result (run-import! (job-cfg) context + {:profile-id (:id profile) + :project-id (:default-project-id profile) + :team team + :name "imported" + :input (js/load-input th/*system* context) + :version 3})] (t/testing "the package is imported in the destination project" (t/is (= 1 (count (:file-ids result))))) @@ -175,10 +205,9 @@ (with-redefs [jobs/heartbeat (fn [_cfg & {:as options}] (swap! reports conj (:progress options)) 1)] - (bfj/export-files (job-cfg) (job-context job-id) - {:ids #{file-id} - :export-type :detach-libraries - :output (tmp/tempfile* :suffix ".penpot")})) + (run-export! (job-cfg) (job-context job-id) + #{file-id} :detach-libraries + (tmp/tempfile* :suffix ".penpot"))) (t/testing "the file opens the run with its position" (let [event (first @reports)] @@ -207,10 +236,9 @@ ;; the taps themselves, before the adapter turns them into events (with-redefs [events/tap (fn [_type data] (swap! taps conj data))] - (bfj/export-files (job-cfg) (job-context job-id) - {:ids #{file-id} - :export-type :detach-libraries - :output (tmp/tempfile* :suffix ".penpot")})) + (run-export! (job-cfg) (job-context job-id) + #{file-id} :detach-libraries + (tmp/tempfile* :suffix ".penpot"))) (t/testing "the tap that opens the file carries the outer counters" (let [tap (first (filter #(= :file (:section %)) @taps))] @@ -269,13 +297,13 @@ (with-redefs [jobs/heartbeat (fn [_cfg & {:as options}] (swap! reports conj (:progress options)) 1)] - (bfj/import-files (job-cfg) context - {:profile-id (:id profile) - :project-id (:default-project-id profile) - :team team - :name "imported" - :input (js/load-input th/*system* context) - :version 3})) + (run-import! (job-cfg) context + {:profile-id (:id profile) + :project-id (:default-project-id profile) + :team team + :name "imported" + :input (js/load-input th/*system* context) + :version 3})) (t/testing "the run opens with the manifest, which has no units to count" (let [event (first @reports)] @@ -301,8 +329,8 @@ job-id (th/create-job* :export-binfile {:profile-id (:id profile)}) context (job-context job-id)] - (t/testing "a running job is active" - (t/is (nil? (bfj/check-active th/*system* context)))) + (t/testing "a running job is active, and answers with the cfg to thread it" + (t/is (identical? th/*system* (bfj/check-active th/*system* context)))) (t/testing "a cancelled job is not" (th/db-update! :job {:status "cancelled"} {:id job-id}) @@ -314,10 +342,8 @@ (t/testing "so the export stops at its next progress point" (let [output (tmp/tempfile* :suffix ".penpot") cause (try - (bfj/export-files (job-cfg) context - {:ids #{file-id} - :export-type :detach-libraries - :output output}) + (run-export! (job-cfg) context + #{file-id} :detach-libraries output) nil (catch Throwable raised raised))] (t/is (= :interrupt (th/ex-type cause))) @@ -342,14 +368,14 @@ cause (ex-info "the job is no longer active" {:type :interrupt :code :job-interrupted})] (with-redefs [jobs/heartbeat (fn [& _] (throw cause))] - (let [raised (try (@#'bfj/with-progress th/*system* context - (fn [] - (swap! steps conj :before) - ;; the sink calls the beat - ;; inline, so its interrupt - ;; aborts the run here - (events/tap :progress {:section :file}) - (swap! steps conj :after))) + (let [raised (try (bfj/run th/*system* context + (fn [_cfg] + (swap! steps conj :before) + ;; the sink calls the beat + ;; inline, so its interrupt + ;; aborts the run here + (events/tap :progress {:section :file}) + (swap! steps conj :after))) nil (catch Throwable stopped stopped))] (t/is (= :interrupt (:type (ex-data raised)))) diff --git a/backend/test/backend_tests/binfile_svg_test.clj b/backend/test/backend_tests/binfile_svg_test.clj index 12cd353a8d..b954226726 100644 --- a/backend/test/backend_tests/binfile_svg_test.clj +++ b/backend/test/backend_tests/binfile_svg_test.clj @@ -246,10 +246,10 @@ (let [bundle (tmp/tempfile :prefix "penpot-export-" :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream bundle)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream bundle)))) (let [evil (tmp/tempfile :prefix "penpot-evil-" :suffix ".zip") _ (tamper-svg-entry! bundle evil media-id (utf8bytes evil-text) :content-type content-type) @@ -364,7 +364,7 @@ (let [bundle (tmp/tempfile :prefix "penpot-export-v1-" :suffix ".bin")] (v1/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) (assoc ::bfc/include-libraries false)) (io/output-stream bundle)) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 2043ad6a19..7b33186e92 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -284,9 +284,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) (let [result (-> th/*system* (assoc ::bfc/project-id (:default-project-id profile)) @@ -373,9 +373,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) (let [result (-> th/*system* (assoc ::bfc/project-id (:default-project-id profile)) @@ -407,9 +407,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) (let [result (-> th/*system* (assoc ::bfc/project-id (:default-project-id profile)) @@ -473,9 +473,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Read the manifest and check external-libraries (let [manifest (v3/get-manifest th/*system* output)] @@ -508,9 +508,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) ;; Only the file is exported, not the library (let [manifest (v3/get-manifest th/*system* output) @@ -569,9 +569,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :merge-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :merge-libraries) + (assoc ::bfc/output (io/output-stream output)))) ;; Only the file is exported, not the library (let [manifest (v3/get-manifest th/*system* output) @@ -645,9 +645,9 @@ (v3/export-files! (assoc th/*system* - ::bfc/ids #{(:id file)} - ::bfc/export-type :detach-libraries) - (io/output-stream output)) + ::bfc/file-ids #{(:id file)} + ::bfc/export-type :detach-libraries + ::bfc/output (io/output-stream output))) (let [names (zip-entry-names output)] (t/is (contains? names (str "files/" (:id file) "/tokens.json"))) @@ -696,9 +696,9 @@ (v3/export-files! (assoc th/*system* - ::bfc/ids #{(:id file)} - ::bfc/export-type :include-libraries) - (io/output-stream output)) + ::bfc/file-ids #{(:id file)} + ::bfc/export-type :include-libraries + ::bfc/output (io/output-stream output))) (let [files (import-output profile output) library' (get files (:name library)) @@ -733,9 +733,9 @@ (v3/export-files! (assoc th/*system* - ::bfc/ids #{(:id file)} - ::bfc/export-type :include-libraries) - (io/output-stream output)) + ::bfc/file-ids #{(:id file)} + ::bfc/export-type :include-libraries + ::bfc/output (io/output-stream output))) (t/is (contains? (zip-entry-names output) (str "files/" (:id file) "/tokens.json"))) @@ -769,9 +769,9 @@ (v3/export-files! (assoc th/*system* - ::bfc/ids #{(:id file)} - ::bfc/export-type :include-libraries) - (io/output-stream output)) + ::bfc/file-ids #{(:id file)} + ::bfc/export-type :include-libraries + ::bfc/output (io/output-stream output))) (let [files (import-output profile output) file-data (:data (get files (:name file)))] @@ -801,9 +801,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Remove the source library to simulate a cross-environment import ;; where the original library does not exist in the target team. @@ -865,9 +865,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) ;; Remove the source library to simulate a cross-environment import ;; where no matching library exists in the target team. @@ -907,9 +907,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Remove the source library to simulate a cross-environment import ;; where the original library does not exist in the target team. @@ -985,9 +985,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Remove the source library and recreate a matching one owned by owner (db/update! th/*system* :file @@ -1067,9 +1067,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file1) (:id file2)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file1) (:id file2)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Remove the source library and recreate a matching one (db/update! th/*system* :file @@ -1168,9 +1168,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Verify manifest has external-libraries (let [manifest (v3/get-manifest th/*system* output) @@ -1205,9 +1205,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; First import (let [result1 (-> th/*system* @@ -1238,9 +1238,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import with overwrite (file-id set) (let [result (-> th/*system* @@ -1267,9 +1267,9 @@ ;; Export file with link-later from team 1 (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import in team 2 (where library with same name exists) (let [result (-> th/*system* @@ -1300,9 +1300,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import in team 2 (no library exists) (let [result (-> th/*system* @@ -1325,9 +1325,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import in team 2 (library with different name) (let [result (-> th/*system* @@ -1356,9 +1356,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import in team 2 (library exists but not shared) (let [result (-> th/*system* @@ -1381,9 +1381,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete the library in team 2 (db/update! th/*system* :file @@ -1433,9 +1433,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original libraries (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib1)}) @@ -1494,9 +1494,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete both libraries (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib1)}) @@ -1546,9 +1546,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original libraries (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib1)}) @@ -1614,9 +1614,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original libraries (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib1)}) @@ -1680,9 +1680,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file1) (:id file2)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file1) (:id file2)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original library and recreate (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib)}) @@ -1726,9 +1726,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file1) (:id file2)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file1) (:id file2)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original library and recreate (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib)}) @@ -1778,9 +1778,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file1) (:id file2)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file1) (:id file2)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original libraries and recreate (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib-icons)}) @@ -1837,9 +1837,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import as viewer (no edit permission on library) (let [result (-> th/*system* @@ -1879,9 +1879,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import as editor (has edit permission) (let [result (-> th/*system* @@ -1919,9 +1919,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Verify slug in manifest (let [manifest (v3/get-manifest th/*system* output) @@ -1946,9 +1946,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Library with empty slug should be dropped from external-libraries (let [manifest (v3/get-manifest th/*system* output) @@ -1966,9 +1966,9 @@ ;; Export file with no libraries (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Manifest should have no external-libraries (let [manifest (v3/get-manifest th/*system* output)] @@ -2000,9 +2000,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete original and create with different case (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib)}) @@ -2037,9 +2037,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Re-import in same team (let [result (-> th/*system* @@ -2075,9 +2075,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Import in team 2 (no library exists) (let [result (-> th/*system* @@ -2127,9 +2127,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete and recreate library (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib)}) @@ -2182,9 +2182,9 @@ (let [output (tmp/tempfile :suffix ".zip")] (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Delete and create two libraries with same name (db/update! th/*system* :file {:deleted-at (ct/now)} {:id (:id lib)}) @@ -2240,9 +2240,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Re-import in same team (let [result (-> th/*system* @@ -2292,9 +2292,9 @@ ;; Export file with link-later (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/export-type :link-later)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{file-id}) + (assoc ::bfc/export-type :link-later) + (assoc ::bfc/output (io/output-stream output)))) ;; Create a viewer profile (no edit permission on the library) (let [viewer (th/create-profile* th/*system* 2 {}) @@ -2342,10 +2342,10 @@ ;; type should win (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) + (assoc ::bfc/file-ids #{file-id}) (assoc ::bfc/export-type :link-later) - (assoc ::bfc/include-libraries true)) - (io/output-stream output)) + (assoc ::bfc/include-libraries true) + (assoc ::bfc/output (io/output-stream output)))) ;; Verify manifest has external-libraries (only produced by link-later) (let [manifest (v3/get-manifest th/*system* output) @@ -2362,9 +2362,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) ;; Import with max-zip-entries=1 — the exported ZIP has more entries (let [cfg (-> th/*system* @@ -2419,9 +2419,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) ;; Import with max-binary-entry-size=1 — the media object will exceed this (let [cfg (-> th/*system* @@ -2504,10 +2504,10 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream exported)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream exported)))) (let [bombed (tmp/tempfile :suffix ".zip")] (replace-zip-entry! exported bombed @@ -2550,10 +2550,10 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream output)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream output)))) (let [cfg (-> th/*system* (assoc ::bfc/project-id (:default-project-id profile)) @@ -2597,10 +2597,10 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream exported)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream exported)))) (let [sizes (text-entries-sizes exported) max-single (apply max 0 sizes) @@ -2795,10 +2795,10 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id simple) (:id svg)}) + (assoc ::bfc/file-ids #{(:id simple) (:id svg)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream output)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream output)))) ;; import returns the imported file ids plus the library link ;; resolution; the test only needs the ids @@ -2874,10 +2874,10 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream output)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream output)))) ;; import returns the imported file ids plus the library link ;; resolution; the test only needs the ids diff --git a/backend/test/backend_tests/db_test.clj b/backend/test/backend_tests/db_test.clj index 29d3095f32..aa6732ead5 100644 --- a/backend/test/backend_tests/db_test.clj +++ b/backend/test/backend_tests/db_test.clj @@ -18,6 +18,24 @@ (t/use-fixtures :once th/state-init) +(defn- get-consistent-pool-stats + "Read pool stats until total equals active + idle, or attempts run out. + Hikari exposes each counter through a separate MXBean call, so a borrow + or return between the reads can leave a single snapshot inconsistent + (total off by one). Retrying until the pool is quiescent keeps the test + deterministic without weakening the invariant." + [pool] + (loop [attempt 20] + (let [stats (db/pool-stats pool) + total (:total-connections stats) + active (:active-connections stats) + idle (:idle-connections stats)] + (if (or (= total (+ active idle)) + (zero? attempt)) + stats + (do (Thread/sleep 10) + (recur (dec attempt))))))) + (t/deftest pool-requires-metrics ;; Metrics is required by the pool schema: a pool wired without it ;; fails at the boundary (AssertionError when asserts are enabled) or @@ -28,7 +46,7 @@ {::db/uri (java.net.URI. "postgresql://localhost/test")})))) (t/deftest pool-stats-returns-expected-keys - (let [stats (db/pool-stats th/*pool*)] + (let [stats (get-consistent-pool-stats th/*pool*)] (t/testing "all expected keys are present" (t/is (contains? stats :active-connections)) (t/is (contains? stats :idle-connections)) diff --git a/backend/test/backend_tests/offload_file_data_test.clj b/backend/test/backend_tests/offload_file_data_test.clj index f4c99c3310..e7158f0567 100644 --- a/backend/test/backend_tests/offload_file_data_test.clj +++ b/backend/test/backend_tests/offload_file_data_test.clj @@ -453,9 +453,9 @@ (v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) - (assoc ::bfc/export-type :detach-libraries)) - (io/output-stream output)) + (assoc ::bfc/file-ids #{(:id file)}) + (assoc ::bfc/export-type :detach-libraries) + (assoc ::bfc/output (io/output-stream output)))) (let [result (-> th/*system* (assoc ::bfc/project-id (:default-project-id profile)) diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj index f15a261040..0a829d23db 100644 --- a/backend/test/backend_tests/rpc_binfile_test.clj +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -167,10 +167,10 @@ output (tmp/tempfile :suffix ".zip")] (bf.v3/export-files! (-> th/*system* - (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/file-ids #{(:id file)}) (assoc ::bfc/embed-assets false) - (assoc ::bfc/include-libraries false)) - (io/output-stream output)) + (assoc ::bfc/include-libraries false) + (assoc ::bfc/output (io/output-stream output)))) output)) (defn- import-file diff --git a/backend/test/backend_tests/tasks/binfile_jobs_integration_test.clj b/backend/test/backend_tests/tasks/binfile_jobs_integration_test.clj index 4d6277a0ff..07a959aa1a 100644 --- a/backend/test/backend_tests/tasks/binfile_jobs_integration_test.clj +++ b/backend/test/backend_tests/tasks/binfile_jobs_integration_test.clj @@ -249,11 +249,13 @@ (with-redefs [jobs/heartbeat (fn [_cfg & {:as options}] (swap! calls conj options) 1)] - (bfj/export-files th/*system* - (jobs/make-context (jobs/get-job th/*system* job-id)) - {:ids #{file-id} - :export-type :detach-libraries - :output (tmp/tempfile* :suffix ".penpot")})) + (let [cfg (bfc/export-cfg th/*system* + {:file-ids #{file-id} + :export-type :detach-libraries + :output (tmp/tempfile* :suffix ".penpot")}) + context (jobs/make-context (jobs/get-job th/*system* job-id))] + (bfj/check-active cfg context) + (bfj/run cfg context v3/export-files!))) (let [reports (map :progress @calls)] diff --git a/backend/test/backend_tests/tasks/import_binfile_test.clj b/backend/test/backend_tests/tasks/import_binfile_test.clj index eadc44104f..e82d462df3 100644 --- a/backend/test/backend_tests/tasks/import_binfile_test.clj +++ b/backend/test/backend_tests/tasks/import_binfile_test.clj @@ -9,7 +9,6 @@ it leaves behind when it fails." (:require [app.binfile.common :as bfc] - [app.binfile.jobs :as bfj] [app.binfile.v1 :as v1] [app.binfile.v3 :as v3] [app.common.uuid :as uuid] @@ -50,7 +49,7 @@ [file-id] (let [output (tmp/tempfile* :suffix ".penpot")] (v1/export-files! (-> th/*system* - (assoc ::bfc/ids #{file-id}) + (assoc ::bfc/file-ids #{file-id}) (assoc ::bfc/include-libraries false) (assoc ::bfc/embed-assets false)) output) @@ -249,12 +248,12 @@ ;; the core does its work inside the transaction and answers with a result ;; the schema of the task does not accept: the check runs before the ;; transaction commits, so the work is rolled back with it - (with-redefs [bfj/import-files - (fn [tx-cfg _context _params] - (th/create-file* tx-cfg 7 {:profile-id (:id profile) - :project-id project-id - :is-shared false - :name "half imported"}) + (with-redefs [v3/import-files! + (fn [cfg] + (th/create-file* cfg 7 {:profile-id (:id profile) + :project-id project-id + :is-shared false + :name "half imported"}) {:file-ids [(uuid/next)] :resolution "not-a-map"})] diff --git a/frontend/src/app/main/data/exports/files.cljs b/frontend/src/app/main/data/exports/files.cljs index 87d64649d4..905716897c 100644 --- a/frontend/src/app/main/data/exports/files.cljs +++ b/frontend/src/app/main/data/exports/files.cljs @@ -105,7 +105,8 @@ (= "completed" status) (rx/of {:file-id (:id file) :uri (:resource-uri result) - :filename (:name file)}) + :filename (:name file) + :size (:size result)}) (= "failed" status) (rx/of {:file-id (:id file) @@ -121,9 +122,14 @@ :else (rx/empty))))))) + ;; a transport failure carries no job error: a nil `ex-data` would + ;; make the dialog drop the message and leave the row hanging, so a + ;; synthetic error takes its place and the row reports it (rx/catch (fn [cause] (rx/of {:file-id (:id file) - :error (ex-data cause)}))))) + :error (or (ex-data cause) + {:type :internal + :code :unknown-error})}))))) (defn export-files "Start files exportation process. diff --git a/frontend/src/app/main/data/imports.cljs b/frontend/src/app/main/data/imports.cljs index 44cdd681d3..6fd079b015 100644 --- a/frontend/src/app/main/data/imports.cljs +++ b/frontend/src/app/main/data/imports.cljs @@ -25,7 +25,8 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.zip :as uz] [beicon.v2.core :as rx] - [cuerdas.core :as str])) + [cuerdas.core :as str] + [promesa.core :as p])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; MESSAGES OF A FAILURE @@ -98,10 +99,111 @@ (uz/read-as-text entry)))) (rx/map json/decode))) +(defn- segment-of + "The segment of a zip path that follows `prefix`, up to the next slash, + or nil when the path does not start with it." + [prefix path] + (when (str/starts-with? path prefix) + (let [rest (subs path (count prefix)) + idx (str/index-of rest "/")] + (if (some? idx) (subs rest 0 idx) rest)))) + +(defn- file-of-entry + "The file of a container a zip entry belongs to. A container keeps every + file of a package under its own prefix: `files/.json` for its data + and `files//…` for the rest." + [path] + (some-> (segment-of "files/" path) + (str/replace #"\.json$" ""))) + +(defn- object-of-entry + "The storage object a zip entry holds. A container keeps the media and + the thumbnails of its files shared under `objects/`, and both the + metadata (`objects/.json`) and the binary (`objects/.`) + belong to the same object." + [path] + (when-some [segment (segment-of "objects/" path)] + (let [dot (str/index-of segment ".")] + (if (some? dot) (subs segment 0 dot) segment)))) + +(defn- reference-entry? + "Whether an entry of a file is the metadata of one of the media or the + thumbnails that file references, the only place that links a file to the + objects it occupies." + [path] + (or (str/includes? path "/media/") + (str/includes? path "/thumbnails/"))) + +(defn- index-container + "Read the entries of a container in one pass: what every file of the + package occupies by itself, what every storage object occupies, and the + entries that say which objects a file references." + [entries] + (reduce + (fn [index entry] + (let [path (.-filename ^js entry) + size (.-compressedSize ^js entry) + file-id (file-of-entry path)] + (cond + (some? file-id) + (cond-> (update-in index [:files file-id] (fnil + 0) size) + (reference-entry? path) + (update :references conj [file-id entry])) + + :else + (if-some [object-id (object-of-entry path)] + (update-in index [:objects object-id] (fnil + 0) size) + index)))) + {:files {} :objects {} :references []} + entries)) + +(defn- read-objects + "The objects every file of a container references, read from the + metadata of its media and its thumbnails." + [references] + (->> (map (fn [[file-id entry]] + (->> (uz/read-as-text entry) + (p/fmap (fn [text] [file-id (json/decode text)])))) + references) + (p/all) + (p/fmap + (fn [decoded] + (reduce (fn [result [file-id data]] + (->> [:media-id :thumbnail-id] + (keep #(get data %)) + (reduce (fn [result object-id] + (update result file-id (fnil conj #{}) object-id)) + result))) + {} + decoded))))) + +(defn- file-sizes + "How much every file of a container occupies inside it: its own entries + plus the storage objects it references, which the container keeps shared + and which are only linked from the file. The manifest carries neither a + size, so the parts of a package are the only account of what it + occupies." + [entries] + (let [{:keys [files objects references]} (index-container entries)] + (->> (read-objects references) + (p/fmap + (fn [referenced] + (reduce-kv (fn [sizes file-id object-ids] + (update sizes file-id (fnil + 0) + (reduce + 0 (keep objects object-ids)))) + files + referenced)))))) + (defn- analyze-file "The entries of one file the user dropped: one per file of a version 3 - package, one for a version 1 file, and an error for anything else." - [{:keys [uri] :as file}] + package, one for a version 1 file, and an error for anything else. + + A container carries its own weight and the weight of every file in it: + the entry of a file of a version 3 package sizes the file inside its + container, and keeps what the container itself occupies in + `:container-size`, which is what the upload of the package is measured + on." + [{:keys [uri size] :as file}] (let [stream (->> (slurp-uri uri :buffer) (rx/merge-map (fn [body] @@ -110,16 +212,23 @@ (= "application/zip" mtype) (let [zip-reader (uz/reader body)] (->> (read-zip-manifest zip-reader) - (rx/map + (rx/merge-map (fn [manifest] (if (= (:type manifest) "penpot/export-files") - (let [manifest (decode-manifest manifest)] - (assoc file :type :binfile-v3 :files (:files manifest))) - (assoc file :type :unknown)))) + (->> (uz/get-entries zip-reader) + (p/mcat file-sizes) + (rx/from) + (rx/map (fn [sizes] + (-> file + (assoc :type :binfile-v3) + (assoc :container-size size) + (assoc :files (:files (decode-manifest manifest))) + (assoc :sizes sizes))))) + (rx/of (assoc file :type :unknown))))) (rx/finalize (partial uz/close zip-reader)))) (= "application/octet-stream" mtype) - (rx/of (assoc file :type :binfile-v1)) + (rx/of (assoc file :type :binfile-v1 :container-size size)) :else (rx/of (assoc file :type :unknown)))))) @@ -137,19 +246,28 @@ (->> stream (rx/filter (fn [entry] (= :binfile-v3 (:type entry)))) - (rx/merge-map (fn [{:keys [files] :as entry}] + (rx/merge-map (fn [{:keys [files sizes] :as entry}] (->> (rx/from files) (rx/map (fn [file] (-> entry - (dissoc :files) + (dissoc :files :sizes) (assoc :name (:name file)) (assoc :file-id (:id file)) + ;; the size the file itself + ;; occupies inside the + ;; container, never the + ;; container's + (assoc :size (get sizes (str (:id file)))) (assoc :status :success)))))))) (->> stream (rx/filter (fn [data] (= :unknown (:type data)))) (rx/map (fn [_] {:uri (:uri file) + :name (:name file) + :size (:size file) + :container-size (:size file) + :type :unknown :status :error :error (tr "dashboard.import.analyze-error")})))) @@ -273,6 +391,163 @@ (rx/from (entry-messages entries {:status :error :error (cause-message cause (tr "labels.error"))})))))))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; WIZARD STEP, UPLOAD AND FILE SIZE +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn format-file-size + "Human size of a dropped file, like `234MB` or `512KB`. + Returns nil when the size is unknown, so the row shows no subtitle." + [size] + (when (and (some? size) (number? size) (>= size 0)) + (let [kb 1024 + mb (* kb 1024) + gb (* mb 1024)] + (cond + (< size kb) + (str (js/Math.round size) "B") + + (< size mb) + (let [v (/ size kb)] + (str (if (< v 10) + (-> (.toFixed v 1) + (str/replace #"\.0$" "")) + (str (js/Math.round v))) + "KB")) + + (< size gb) + (let [v (/ size mb)] + (str (if (< v 10) + (-> (.toFixed v 1) + (str/replace #"\.0$" "")) + (str (js/Math.round v))) + "MB")) + + :else + (let [v (/ size gb)] + (str (if (< v 10) + (-> (.toFixed v 1) + (str/replace #"\.0$" "")) + (str (js/Math.round v))) + "GB")))))) + +(defn- upload-ratio + "The fraction of the container of an entry already sent: the chunks + uploaded over the chunks the file has, or nil while its upload has not + started." + [entry] + (let [{:keys [current total]} (get-in entry [:progress :counters :upload])] + (when (and (some? current) (some? total) (pos? total)) + (/ current total)))) + +(defn- uploaded-ratio + "The fraction of the container of an entry already uploaded. A container + is uploaded whole before its job exists, so from the moment its entries + leave the upload stage they count as sent: only an entry still sending + chunks, or one that has not started, says less than everything." + [entry] + (cond + (= :upload (get-in entry [:progress :stage])) (or (upload-ratio entry) 0) + (contains? #{:analyze :import-ready} (:status entry)) 0 + :else 1)) + +(defn upload-done? + "Whether the container of an entry finished uploading: the chunks sent + reached the chunks the file has." + [entry] + (let [{:keys [current total]} (get-in entry [:progress :counters :upload])] + (and (some? current) (some? total) (= current total)))) + +(defn upload-pct + "The percentage of the whole upload: the bytes already sent over the + bytes of every container. The containers are weighted by their size, + never averaged, because a `.penpot` is uploaded whole: a container of + 200MB moves the bar as much as four of 50MB." + [entries] + (let [containers (map (comp first :entries) (packages entries)) + total (reduce + 0 (keep :container-size containers))] + (when (pos? total) + (let [sent (reduce + 0 (map (fn [{:keys [container-size] :as entry}] + (* container-size (uploaded-ratio entry))) + containers))] + (js/Math.round (* 100 (/ sent total))))))) + +(defn- files-done + "How many files of a container are already rebuilt. A container that + finished leaves all of its files behind; while it runs, its milestone + says which file it is working on, one based, so the ones before it are + done." + [entries] + (if (every? #(= :import-success (:status %)) entries) + (count entries) + (let [{:keys [current total]} (get-in (first entries) [:progress :counters :files]) + total (or total (count entries))] + (-> (or current 1) dec (max 0) (min total))))) + +(defn imported-count + "How many of the files being imported are already rebuilt: the files of + the containers that finished plus the ones a running container left + behind. A container rebuilds its files one by one, so counting only the + finished containers (or only their files) would leave the number still + for the whole import." + [entries] + (reduce + 0 (map (comp files-done :entries) (packages entries)))) + +(defn waiting-slot? + "Whether every file of the wizard waits for a free slot: the jobs of the + containers exist, but no worker picked them up yet." + [entries] + (and (pos? (count entries)) + (every? #(= :import-queued (:status %)) entries))) + +(defn- pending-upload? + "Whether a file is still to be uploaded, or is being uploaded right now: + the wizard is in its upload step while any file is pending." + [entry] + (or (= :import-ready (:status entry)) + (= :upload (get-in entry [:progress :stage])))) + +(defn import-step + "The wizard step the header paints: `:check` while the entries are + analyzed, `:upload` until every file of every container is uploaded, and + `:import` once the jobs run or the wizard reaches its outcome. + + A file leaves the upload behind when its job exists: the queued message + carries no milestone, and the upload one disappears with it, so the + presence of a milestone is not enough to keep the step in the upload." + [status entries] + (case status + (:analyze :import-ready) + :check + + :import-progress + (if (some pending-upload? entries) + :upload + :import) + + (:import-success :import-error :library-resolution :library-summary) + :import + + :check)) + +(defn awaiting-analysis? + "Whether the wizard hides the rows and shows the checking placeholder + instead: the dialog is still analyzing and no entry failed yet. Entries + that fail during analyze take the error-summary path instead of rows." + [status errors?] + (and (= :analyze status) (not errors?))) + +(defn ready-to-import? + "Whether the wizard can offer its import step: the analysis is over and + every file it found can be imported. A ready file is not enough on its + own: until the analysis ends the wizard does not know how many files the + dropped containers carry, and the ones still to arrive would never be + imported." + [entries analysis-ended?] + (and analysis-ended? + (pos? (count entries)) + (every? #(= :import-ready (:status %)) entries))) + (defn import-files "Import the entries the wizard analyzed, one job per package. diff --git a/frontend/src/app/main/ui/dashboard/import.cljs b/frontend/src/app/main/ui/dashboard/import.cljs index a536033e4b..048c0d7e73 100644 --- a/frontend/src/app/main/ui/dashboard/import.cljs +++ b/frontend/src/app/main/ui/dashboard/import.cljs @@ -29,7 +29,6 @@ [app.main.ui.ds.foundations.typography.heading :refer [heading*]] [app.main.ui.ds.foundations.typography.text :refer [text*]] [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] - [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.jobs.progress :as jp] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] @@ -51,6 +50,7 @@ (let [entries (->> entries (mapv (fn [file] {:name (.-name file) + :size (.-size file) :uri (wapi/create-uri file)})) (not-empty))] (when entries @@ -170,7 +170,7 @@ pending))))) (defn- analyze-entries - [state entries] + [state entries on-analyze-end] (->> (imp/analyze entries) (rx/mapcat #(rx/delay emit-delay (rx/of %))) (rx/filter some?) @@ -179,7 +179,11 @@ (when (some? (:error message)) (st/emit! (ev/event {::ev/name "import-files-error" :error (:error message)}))) - (swap! state update-with-analyze-result message))))) + (swap! state update-with-analyze-result message)) + (constantly nil) + ;; the wizard cannot offer its import step until it knows every + ;; file of every container + on-analyze-end))) (defn- import-files [state library-resolution-data* project-id entries on-job] @@ -201,30 +205,31 @@ (mf/defc import-entry* {::mf/memo true ::mf/private true} - [{:keys [entries entry edition can-be-deleted is-progress on-edit on-change on-delete]}] + [{:keys [entries entry edition on-edit on-change on-delete]}] (let [status (:status entry) ;; FIXME: rename to format format (:type entry) - loading? (or (= :analyze status) - (= :import-queued status) - (= :import-progress status) - (and is-progress (= :import-ready status))) analyze-error? (= :analyze-error status) - import-success? (= :import-success status) import-error? (= :import-error status) import-ready? (= :import-ready status) - level (cond - import-success? :success - import-ready? :success - import-error? :error - analyze-error? :error - loading? nil - :else :default) + progress (:progress entry) + upload? (= :upload (:stage progress)) + + ;; the container the file came in is fully sent, but its job does + ;; not exist yet: the wizard is still uploading the other files + ready? (and (= :import-progress status) + upload? + (imp/upload-done? entry)) + + ;; a file can be dropped while the wizard has not started yet: once + ;; the import runs, its entries are committed + can-be-deleted? (and import-ready? + (> (count entries) 1)) is-shared? (:shared entry) - progress (:progress entry) + subtitle (imp/format-file-size (:size entry)) file-id (:file-id entry) editing? (and (some? file-id) (= edition file-id)) @@ -264,44 +269,42 @@ [:div {:class (stl/css-case :file-entry true - :loading loading? - :success import-success? + :ready ready? + :success (= :import-success status) :error (or import-error? analyze-error?) :editable (and import-ready? (not editing?)))} - [:div {:class (stl/css :file-name)} - (when loading? [:> loader* {:width 26 :title (tr "labels.loading")}]) + [:div {:class (stl/css :file-main)} + [:div {:class (stl/css :file-icon) + :aria-hidden "true"} + [:> icon* {:icon-id i/folder :size "m"}]] - (if editing? - [:div {:class (stl/css :file-name-edit)} - [:input {:type "text" - :auto-focus true - :class (stl/css :file-name-input) - :aria-label (tr "dashboard.import.file-name-label") - :default-value (:name entry) - :on-key-press on-edit-key-press - :on-blur on-edit-blur}]] + [:div {:class (stl/css :file-meta)} + (if editing? + [:div {:class (stl/css :file-name-edit)} + [:input {:type "text" + :auto-focus true + :class (stl/css :file-name-input) + :aria-label (tr "dashboard.import.file-name-label") + :default-value (:name entry) + :on-key-press on-edit-key-press + :on-blur on-edit-blur}]] - [:div {:class (stl/css :file-name-label)} - (if loading? - [:> text* {:class (stl/css :file-name-label) - :as "span" - :typography t/body-medium} - (:name entry) - (when ^boolean is-shared? - [:> icon* {:icon-id i/library :class (stl/css :file-label-icon)}])] - [:> context-notification* - {:level level - :appearance :ghost - :class (stl/css :file-name-notification)} - [:> text* {:class (stl/css :file-name-label) + [:* + [:> text* {:class (stl/css :file-name-label) + :as "span" + :typography t/body-medium} + [:span {:class (stl/css :file-name-text)} + (:name entry)] + (when ^boolean is-shared? + [:> icon* {:icon-id i/library :class (stl/css :file-label-icon)}])] + (when (some? subtitle) + [:> text* {:class (stl/css :file-subtitle) :as "span" - :typography t/body-medium} - (:name entry) - (when ^boolean is-shared? - [:> icon* {:icon-id i/library :class (stl/css :file-label-icon)}])]])]) + :typography t/body-small} + subtitle])])] - (when ^boolean (or editable? can-be-deleted) + (when ^boolean (or editable? can-be-deleted?) [:div {:class (stl/css :edit-entry-buttons)} (when ^boolean editable? [:> icon-button* {:on-click on-edit' @@ -309,38 +312,36 @@ :icon-size "s" :aria-label (tr "labels.edit") :icon i/curve}]) - (when ^boolean can-be-deleted + (when ^boolean can-be-deleted? [:> icon-button* {:on-click on-delete' :variant "ghost" :icon-size "s" :aria-label (tr "labels.delete") - :icon i/delete}])])] - (cond - analyze-error? - [:> text* {:class (stl/css :error-message) - :as "span" - :typography t/body-small} - ;; the message is user-facing text already (a hint or a translated - ;; label), never a key - (if (some? (:error entry)) - (:error entry) - (tr "dashboard.import.analyze-error"))] + :icon i/delete}])]) - import-error? - [:> text* {:class (stl/css :error-message) - :as "span" - :typography t/body-small} - (if (some? (:error entry)) - (:error entry) - (tr "labels.error"))] + ;; a row carries its name and its size, and only the state that is not + ;; progress: the milestone in course and the files already imported are + ;; the business of the header, which writes the milestone once for + ;; every file of a container + (when ^boolean ready? + [:div {:class (stl/css :file-status)} + [:> icon* {:icon-id i/status-tick :size "s"}] + [:> text* {:as "span" + :typography t/body-medium} + (tr "dashboard.import.status.ready")]])] - (and (= :import-queued status) (not import-success?)) - [:div {:class (stl/css :progress-message)} - (tr "jobs.queued")] - - (and (not import-success?) (some? progress)) - [:div {:class (stl/css :progress-message)} - (jp/milestone-text progress :file? true)]) + (when ^boolean (or analyze-error? import-error?) + [:div {:class (stl/css :file-error)} + [:> text* {:class (stl/css :error-message) + :as "span" + :typography t/body-small} + ;; the message is user-facing text already (a hint or a translated + ;; label), never a key + (if (some? (:error entry)) + (:error entry) + (if analyze-error? + (tr "dashboard.import.analyze-error") + (tr "labels.error")))]]) ;; This is legacy code, will be removed when legacy-zip format is removed [:div {:class (stl/css :linked-libraries)} @@ -560,13 +561,126 @@ ;; ── Stage components ──────────────────────────────────────────────── +(mf/defc import-steps* + {::mf/private true} + [{:keys [step]}] + (let [order [:check :upload :import] + label {:check (tr "dashboard.import.steps.check") + :upload (tr "dashboard.import.steps.upload") + :import (tr "dashboard.import.steps.import")} + current (count (take-while #(not= % step) order))] + [:ol {:class (stl/css :import-steps) + :aria-label (tr "dashboard.import.steps.label")} + (for [[idx id] (d/enumerate order)] + (let [done? (< idx current) + active? (= idx current)] + [:li {:key (name id) + :class (stl/css-case :import-step true + :done done? + :active active?) + :aria-current (when active? "step")} + [:span {:class (stl/css :step-marker) + :aria-hidden "true"} + (if done? + [:> icon* {:icon-id i/status-tick :size "s"}] + (str (inc idx)))] + [:> text* {:as "span" + :typography t/body-medium + :class (stl/css :step-label)} + (get label id)] + (when (< idx 2) + [:span {:class (stl/css :step-line) :aria-hidden "true"}])]))])) + +(defn- importing-title + "The title of the phase that rebuilds the files, with the files the + wizard is working on and the ones already done." + [total done] + (dm/str (tr "dashboard.import.importing.title" (i18n/c total)) + " · " + (tr "dashboard.import.importing.done" (i18n/c done)))) + +(mf/defc import-phase-header* + {::mf/private true} + [{:keys [status step entries template]}] + (let [total (+ (count entries) (if (some? template) 1 0)) + ;; the file the job is working on is reported by the job itself, so + ;; the counter moves while a container rebuilds its files one by one + done (imp/imported-count entries) + + phase (cond + (= :analyze status) :check + (= :import-ready status) :ready + (not= :import-progress status) nil + (= :upload step) :upload + (imp/waiting-slot? entries) :queued + :else :importing) + + ;; the bar measures the whole import, never a single file: the + ;; upload of every container put together while they are sent, and + ;; the animated one from there on, because the stages of a job are + ;; not comparable one with another + [title desc pct] + (case phase + :check [(tr "dashboard.import.check.title") + (tr "dashboard.import.check.message") + nil] + :ready [(tr "dashboard.import.ready.title") + (tr "dashboard.import.ready.message") + nil] + :upload [(tr "dashboard.import.upload.title") + (tr "dashboard.import.upload.message") + (imp/upload-pct entries)] + :queued [(tr "dashboard.import.queued.title") + (tr "dashboard.import.queued.message") + nil] + :importing [(importing-title total done) + (tr "dashboard.import.importing.message") + nil] + [nil nil nil]) + + ;; the milestone in course takes the place of the message: the files + ;; of a container share it, so writing it here keeps it once + desc + (or (jp/current-milestone-text entries) desc) + + ;; the bar paints what is running: indeterminate while the wizard + ;; works —checking, waiting for a slot, rebuilding— and determinate + ;; while it uploads, which is the only percentage it really knows + bar? (contains? #{:check :upload :queued :importing} phase)] + + (when (some? phase) + [:div {:class (stl/css :phase-header)} + [:> heading* {:level 3 + :typography t/headline-medium + :class (stl/css :phase-title)} + title] + [:> text* {:as "p" + :typography t/body-medium + :class (stl/css :phase-message)} + desc] + (when bar? + [:div {:class (stl/css-case :progress-bar true + :determinate (some? pct)) + :role "progressbar" + :aria-valuemin 0 + :aria-valuemax 100 + :aria-valuenow pct} + [:div {:class (stl/css :progress-fill) + :style (when (some? pct) + #js {:width (str pct "%")})}]])]))) + (mf/defc import-files-stage* {::mf/private true} - [{:keys [entries template status errors? import-success-total auto-linked-count + [{:keys [entries template status step errors? import-success-total auto-linked-count edition on-edit on-change on-delete on-cancel on-cancel-import on-continue on-accept pending-analysis?]}] [:* + [:> import-steps* {:step step}] [:div {:class (stl/css :modal-content)} + [:> import-phase-header* {:status status + :step step + :entries entries + :template template}] (when (and (= :analyze status) errors?) [:> context-notification* {:level :warning @@ -589,7 +703,8 @@ :class (stl/css :context-notification-error)} (tr "dashboard.import.import-error.disclaimer")]) - (if (or (= :import-error status) (and (= :analyze status) errors?)) + (cond + (or (= :import-error status) (and (= :analyze status) errors?)) [:div {:class (stl/css :import-error-disclaimer)} [:div (tr "dashboard.import.import-error.message1")] [:ul {:class (stl/css :import-error-list)} @@ -613,20 +728,24 @@ (tr "dashboard.import.import-error.unknown-error"))])]))] [:div (tr "dashboard.import.import-error.message2")]] + (imp/awaiting-analysis? status errors?) + ;; while checking, the phase header carries the message: no rows, + ;; no spinners + nil + + :else (for [entry entries] [:> import-entry* {:edition edition :key (dm/str (:uri entry) "/" (:file-id entry)) :entry entry :entries entries - :is-progress (= :import-progress status) :on-edit on-edit :on-change on-change - :on-delete on-delete - :can-be-deleted (> (count entries) 1)}])) + :on-delete on-delete}])) - (when (some? template) - [:> import-entry* {:entry (assoc template :status status) - :can-be-deleted false}])] + (when (and (some? template) + (not (imp/awaiting-analysis? status errors?))) + [:> import-entry* {:entry (assoc template :status status)}])] [:div {:class (stl/css :modal-footer)} [:div {:class (stl/css :action-buttons)} @@ -735,6 +854,13 @@ status* (mf/use-state :analyze) status (deref status*) + ;; the analysis of the dropped files reached its end: only then the + ;; wizard knows every file it is going to import + analysis-ended* + (mf/use-state false) + analysis-ended? + (deref analysis-ended*) + ;; Jobs created while importing, to cancel them on demand, and ;; the subscription to their messages, to stop listening. jobs* (mf/use-state #{}) @@ -970,13 +1096,12 @@ (every? #(contains? visited (:id %)) unresolved-files)) (reset! status* :library-summary))) - (mf/with-effect [entries resolution] + (mf/with-effect [entries resolution analysis-ended?] (cond (some? template) (reset! status* :import-ready) - (and (seq entries) - (every? #(= :import-ready (:status %)) entries)) + (imp/ready-to-import? entries analysis-ended?) (reset! status* :import-ready) (and (seq entries) @@ -993,7 +1118,7 @@ ;; Run analyze operation on component mount (mf/with-effect [] - (let [sub (analyze-entries state* entries)] + (let [sub (analyze-entries state* entries #(reset! analysis-ended* true))] (partial rx/dispose! sub))) [:div {:class (stl/css :modal-overlay)} @@ -1002,7 +1127,7 @@ [:> heading* {:level 2 :typography t/headline-large :class (stl/css :modal-title)} - (tr "dashboard.import")] + (tr "dashboard.import.title")] [:> icon-button* {:variant "ghost" :aria-label (tr "labels.close") :on-click on-cancel @@ -1015,6 +1140,7 @@ {:entries entries :template template :status status + :step (imp/import-step status entries) :errors? errors? :import-success-total import-success-total :auto-linked-count auto-linked-count @@ -1029,23 +1155,27 @@ :pending-analysis? pending-analysis?}] :library-resolution - [:> import-library-resolution-stage* - {:current-unresolved-file current-unresolved-file - :selection selection - :on-select manage-on-select - :on-disconnect manage-on-disconnect - :visited visited - :last-file? last-unresolved-file? - :on-wizard-prev on-wizard-prev - :on-wizard-next on-wizard-next - :on-wizard-skip on-wizard-skip}] + [:* + [:> import-steps* {:step :import}] + [:> import-library-resolution-stage* + {:current-unresolved-file current-unresolved-file + :selection selection + :on-select manage-on-select + :on-disconnect manage-on-disconnect + :visited visited + :last-file? last-unresolved-file? + :on-wizard-prev on-wizard-prev + :on-wizard-next on-wizard-next + :on-wizard-skip on-wizard-skip}]] :library-summary - [:> import-library-summary-stage* - {:resolution resolution - :selection selection - :visited visited - :on-summary-back on-summary-back - :on-confirm-library-links on-confirm-library-links}] + [:* + [:> import-steps* {:step :import}] + [:> import-library-summary-stage* + {:resolution resolution + :selection selection + :visited visited + :on-summary-back on-summary-back + :on-confirm-library-links on-confirm-library-links}]] nil)]])) \ No newline at end of file diff --git a/frontend/src/app/main/ui/dashboard/import.scss b/frontend/src/app/main/ui/dashboard/import.scss index eafdc1ecc6..b67a2fe80a 100644 --- a/frontend/src/app/main/ui/dashboard/import.scss +++ b/frontend/src/app/main/ui/dashboard/import.scss @@ -34,7 +34,7 @@ background-color: var(--color-background-primary); border: $b-2 solid var(--color-background-quaternary); min-block-size: $sz-192; - inline-size: px2rem(640); + inline-size: px2rem(720); max-block-size: px2rem(800); } @@ -56,6 +56,104 @@ .modal-title { color: var(--color-foreground-primary); + text-transform: uppercase; + letter-spacing: 0.02em; +} + +// ################################ +// WIZARD STEPS +// ################################ + +.import-steps { + display: flex; + align-items: center; + gap: 0; + list-style: none; + margin: 0; + padding: 0; +} + +.import-step { + --step-fg-color: var(--color-foreground-secondary); + --step-marker-border: var(--color-foreground-secondary); + + display: flex; + align-items: center; + gap: var(--sp-s); + flex-shrink: 0; + + &:not(:last-child) { + flex: 1 1 auto; + } + + &.active { + --step-fg-color: var(--color-foreground-primary); + --step-marker-border: var(--color-accent-primary); + } + + &.done { + --step-fg-color: var(--color-foreground-secondary); + --step-marker-border: var(--color-accent-primary); + --step-marker-bg-color: var(--color-accent-primary); + --step-marker-fg-color: var(--color-background-secondary); + } +} + +.step-marker { + display: flex; + justify-content: center; + align-items: center; + block-size: $sz-24; + inline-size: $sz-24; + flex-shrink: 0; + border-radius: 50%; + border: $b-1 solid var(--step-marker-border); + background-color: var(--step-marker-bg-color, transparent); + color: var(--step-marker-fg-color, var(--step-marker-border)); + font-size: px2rem(12); + font-weight: 500; + + svg { + block-size: $sz-16; + inline-size: $sz-16; + } +} + +.step-label { + color: var(--step-fg-color); + white-space: nowrap; +} + +.step-line { + flex: 1 1 auto; + block-size: $b-1; + min-inline-size: $sz-32; + margin-inline: var(--sp-m); + background-color: var(--color-background-quaternary); +} + +.import-step.active .step-line, +.import-step.done .step-line { + background-color: var(--color-background-quaternary); +} + +// ################################ +// PHASE HEADER +// ################################ + +.phase-header { + display: flex; + flex-direction: column; + gap: var(--sp-xs); +} + +.phase-title { + color: var(--color-foreground-primary); +} + +.phase-message { + color: var(--color-foreground-secondary); + margin: 0; } .modal-close-btn { @@ -86,8 +184,9 @@ flex-direction: column; gap: var(--sp-m); + &.ready, &.success { - --file-entry-fg-color: var(--color-accent-sucess); + --file-entry-fg-color: var(--color-accent-success); } &.error { @@ -99,16 +198,10 @@ } } -.error-message, -.progress-message { - display: flex; - align-items: center; - min-block-size: $sz-32; - color: var(--file-entry-fg-color); -} - .error-message { + display: flex; align-items: flex-start; + color: var(--file-entry-fg-color); white-space: pre-wrap; overflow-wrap: anywhere; } @@ -153,6 +246,86 @@ color: var(--file-entry-fg-color); } +.file-main { + display: flex; + align-items: center; + gap: var(--sp-m); +} + +.file-icon { + display: flex; + justify-content: center; + align-items: center; + block-size: px2rem(40); + inline-size: px2rem(40); + flex-shrink: 0; + border-radius: $br-8; + background-color: var(--color-background-tertiary); + color: var(--color-foreground-secondary); +} + +.file-meta { + display: flex; + flex-direction: column; + gap: var(--sp-xxs); + flex: 1 1 auto; + min-inline-size: 0; +} + +.file-subtitle { + color: var(--color-foreground-secondary); +} + +.file-status { + display: flex; + align-items: center; + gap: var(--sp-s); + flex-shrink: 0; + color: var(--file-entry-fg-color); + white-space: nowrap; +} + +.file-error { + padding-inline-start: calc(px2rem(40) + var(--sp-m)); +} + +.progress-bar { + block-size: px2rem(4); + border-radius: $br-8; + background-color: var(--color-background-tertiary); + overflow: hidden; +} + +.progress-fill { + block-size: 100%; + border-radius: $br-8; + background-color: var(--color-accent-primary); + + // the client cannot know a global percentage of the import work, so + // the fill slides through the bar while the jobs run; the upload is + // the exception, its chunk counters are real and the inline width + // style drives the fill then. The rule is written as physical `width` + // on purpose: the determinate fill gets its width from an inline + // style, and a logical `inline-size` here would map after the cascade + // and win over it. + width: 40%; + animation: progress-slide 1.2s ease-in-out infinite; + + .determinate & { + animation: none; + } +} + +@keyframes progress-slide { + from { + transform: translateX(-100%); + } + + to { + transform: translateX(250%); + } +} + .edit-entry-buttons { display: flex; align-items: center; @@ -175,7 +348,15 @@ display: flex; align-items: center; gap: var(--sp-m); - flex-grow: 1; + min-inline-size: 0; + color: var(--color-foreground-primary); +} + +// the status sits at the end of the same line: a long name gives way to it +.file-name-text { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .file-label-icon { @@ -214,10 +395,6 @@ --context-notification-bg-color: var(--color-background-primary); } -.file-name-notification { - flex-grow: 1; -} - .import-error-disclaimer { color: var(--color-foreground-secondary); } diff --git a/frontend/src/app/main/ui/exports/files.cljs b/frontend/src/app/main/ui/exports/files.cljs index 8a527c2209..fba7916046 100644 --- a/frontend/src/app/main/ui/exports/files.cljs +++ b/frontend/src/app/main/ui/exports/files.cljs @@ -11,55 +11,107 @@ [app.common.data.macros :as dm] [app.config :as cf] [app.main.data.exports.files :as fexp] + [app.main.data.imports :as imp] [app.main.data.jobs :as dj] [app.main.data.modal :as modal] [app.main.store :as st] [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] - [app.main.ui.ds.foundations.assets.icon :as i] + [app.main.ui.ds.foundations.assets.icon :as i :refer [icon*]] [app.main.ui.ds.foundations.typography :as t] [app.main.ui.ds.foundations.typography.heading :refer [heading*]] [app.main.ui.ds.foundations.typography.text :refer [text*]] [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] - [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.jobs.progress :as jp] - [app.main.ui.notifications.context-notification :refer [context-notification]] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] [beicon.v2.core :as rx] + [cuerdas.core :as str] [rumext.v2 :as mf])) +(defn- same-file? + "Whether an id from a message names a row of the dialog. The ids cross + serialization boundaries, so a message can carry the id as a string + while the row keeps the uuid (or the other way round): both sides are + canonicalized instead of assuming one type." + [a b] + (= (str a) (str b))) + +(defn- error-hint + "What a failed row reports: the message of the failure when it says + something useful, or nil. A string error already is the message; a map + carries it under `:hint`; anything else is unwrapped with `ex-data` + first, which itself can answer a string." + [error] + (let [data (cond + (string? error) error + (map? error) error + :else (ex-data error))] + (some-> (if (string? data) data (:hint data)) + (str/trim) + (not-empty)))) + (defn- mark-file-error - [files file-id] - (mapv #(cond-> % - (= file-id (:id %)) - (assoc :export-error true - :loading false)) + "The export of a file failed: a terminal state that replaces any + other, so a row never reports success and failure at once." + [files file-id error] + (mapv (fn [file] + (if (same-file? file-id (:id file)) + (-> file + (assoc :export-error true + :error error + :loading false) + (dissoc :export-success :size :progress :queued)) + file)) files)) (defn- mark-file-cancelled "The job of a file was cancelled elsewhere: a neutral terminal state, - neither success nor error." + neither success nor error, replacing any other." [files file-id] - (mapv #(cond-> % - (= file-id (:id %)) - (assoc :export-cancelled true - :loading false)) + (mapv (fn [file] + (if (same-file? file-id (:id file)) + (-> file + (assoc :export-cancelled true + :loading false) + (dissoc :export-success :size :export-error :error + :progress :queued)) + file)) + files)) + +(defn- mark-all-cancelled + "Every file still loading is over: its job was just cancelled, or it + was never created because the exports run one after another." + [files] + (mapv (fn [file] + (if (:loading file) + (-> file + (assoc :export-cancelled true + :loading false) + (dissoc :progress :queued)) + file)) files)) (defn- mark-file-success - [files file-id] - (mapv #(cond-> % - (= file-id (:id %)) - (assoc :export-success true - :loading false)) + "The export of a file produced its artifact: a terminal state that + replaces any other, so a row never reports success and failure at + once." + [files file-id size] + (mapv (fn [file] + (if (same-file? file-id (:id file)) + (-> file + (assoc :export-success true + :size size + :loading false) + (dissoc :export-error :error :progress :queued)) + file)) files)) (defn- mark-file-progress "The milestone the job of a file is in, to show it while it runs." [files file-id progress] (mapv #(cond-> % - (= file-id (:id %)) + (same-file? file-id (:id %)) (assoc :progress progress :queued false)) files)) @@ -68,7 +120,7 @@ "The job of a file exists but no worker picked it up yet." [files file-id] (mapv #(cond-> % - (= file-id (:id %)) + (same-file? file-id (:id %)) (assoc :queued true)) files)) @@ -83,53 +135,115 @@ (mf/defc export-entry* {::mf/private true} [{:keys [file]}] - (let [level (cond - (:export-success file) :success - (:export-error file) :error - (:export-cancelled file) :warning - :else :info)] + (let [success? (:export-success file) + error? (:export-error file) + cancelled? (:export-cancelled file) + progress (:progress file) + running? (and (:loading file) (some? progress)) + ;; the job of the file exists but no worker picked it up yet: its + ;; queue only ends with a milestone, so until then the row waits + waiting? (and (:loading file) (nil? progress)) + size (imp/format-file-size (:size file)) + ;; the size of the artifact only exists once the job produced it: + ;; until then the subtitle only says what it will be + subtitle (if (some? size) (dm/str size " .penpot") ".penpot")] [:div {:class (stl/css-case :file-entry true - :loading (:loading file) - :success (:export-success file) - :error (:export-error file))} + :success success? + :error error?)} - (if (:loading file) - [:* - [:div {:class (stl/css :file-name)} - [:> loader* {:width 26 - :title (tr "labels.loading")}] - [:> text* {:class (stl/css :file-name-label) - :as "span" - :typography t/body-large} - (:name file)]] + [:div {:class (stl/css :file-main)} + [:div {:class (stl/css :file-icon) + :aria-hidden "true"} + [:> icon* {:icon-id (if error? i/close i/folder) :size "m"}]] - ;; while the file is being produced: waiting for a worker, then - ;; the milestone of the job - (when (or (some? (:progress file)) (:queued file)) - (let [progress (:progress file)] - [:> text* {:class (stl/css :status-message) - :as "span" - :typography (if (some? progress) t/body-large t/body-medium) - :role "status" - :aria-live "polite"} - (if (some? progress) - (jp/milestone-text progress) - (tr "jobs.queued"))]))] + [:div {:class (stl/css :file-meta)} + [:> text* {:class (stl/css :file-name-label) + :as "span" + :typography t/body-medium} + (:name file)] + [:> text* {:class (stl/css :file-subtitle) + :as "span" + :typography t/body-small} + subtitle]] - [:* - [:> context-notification {:level level - :content (:name file)}] + ;; a row carries its name and what its artifact will weigh, and only + ;; the state that is its own: while the file runs, the milestone and + ;; the bar live under it + (cond + success? + [:div {:class (stl/css :file-status)} + [:> icon* {:icon-id i/status-tick :size "s"}] + [:> text* {:as "span" + :typography t/body-medium} + (tr "files-export-modal.status.exported")]] - ;; a cancelled file is a neutral terminal state, but the label is - ;; what tells the user why no artifact came out - (when (:export-cancelled file) - [:> text* {:class (stl/css :status-message) - :as "span" - :typography t/body-medium - :role "status" - :aria-live "polite"} - (tr "jobs.export-cancelled")])])])) + waiting? + [:div {:class (stl/css :file-status)} + [:> icon* {:icon-id i/clock :size "s"}] + [:> text* {:as "span" + :typography t/body-medium} + (tr "files-export-modal.status.waiting")]])] + + ;; the milestone of the job, with an animated bar: the stages of an + ;; export are not comparable one with another, so the client cannot + ;; know a percentage of the file, only that it moves + (when running? + [:div {:class (stl/css :file-progress)} + [:> text* {:class (stl/css :progress-message) + :as "span" + :typography t/body-small + :role "status" + :aria-live "polite"} + (jp/milestone-text progress)] + [:div {:class (stl/css :progress-bar) + :role "progressbar"} + [:div {:class (stl/css :progress-fill)}]]]) + + (when error? + [:div {:class (stl/css :file-error)} + [:> text* {:class (stl/css :error-message) + :as "span" + :typography t/body-small} + ;; the reason is user-facing text already (the hint of the job), + ;; never a key + (tr "files-export-modal.export-error" + (or (error-hint (:error file)) + (tr "labels.error")))]]) + + ;; a cancelled file is a neutral terminal state, but the label is + ;; what tells the user why no artifact came out + (when cancelled? + [:div {:class (stl/css :file-error)} + [:> text* {:class (stl/css :status-message) + :as "span" + :typography t/body-small + :role "status" + :aria-live "polite"} + (tr "jobs.export-cancelled")]])])) + +(mf/defc export-phase-header* + {::mf/private true} + [{:keys [files]}] + (let [total (count files) + done (count (filterv :export-success files)) + ;; every file is its own job, so the header only counts them: the + ;; progress of each one lives in its row, there is no global bar + running? (boolean (some :loading files))] + [:div {:class (stl/css :phase-header)} + [:> heading* {:level 3 + :typography t/headline-medium + :class (stl/css :phase-title)} + (if running? + (tr "files-export-modal.exporting.title" (i18n/c total)) + (tr "files-export-modal.exported.title" (i18n/c done)))] + (when (or running? (pos? done)) + [:> text* {:as "p" + :typography t/body-medium + :class (stl/css :phase-message)} + (if running? + (tr "files-export-modal.exporting.message") + (tr "files-export-modal.exported.message"))])])) (mf/defc export-dialog {::mf/register modal/components @@ -145,6 +259,10 @@ state (deref state*) selected (:selected state) status (:status state) + ;; files still in flight: the dialog status never leaves + ;; :exporting, so loading rows are the signal of a running + ;; export, not the status + in-progress? (boolean (->> state :files (some :loading))) ;; Jobs still running, as file-id -> job-id, to cancel them on ;; demand; a file that reaches its outcome leaves the map, so @@ -164,7 +282,7 @@ (reset! sub* (->> (fexp/export-files :files files :type selected :on-job #(swap! jobs* assoc (:file-id %) (:job-id %))) (rx/subs! - (fn [{:keys [file-id error filename uri progress queued + (fn [{:keys [file-id error filename uri size progress queued cancelled]}] (cond (some? progress) @@ -175,7 +293,7 @@ (some? error) (swap! jobs* dissoc file-id) - (swap! state* update :files mark-file-error file-id) + (swap! state* update :files mark-file-error file-id error) (true? cancelled) (swap! jobs* dissoc file-id) @@ -186,35 +304,52 @@ (some? uri) (do (swap! jobs* dissoc file-id) - (swap! state* update :files mark-file-success file-id) + (swap! state* update :files mark-file-success file-id size) (dom/trigger-download-uri filename "application/penpot" uri))))))))) - on-cancel-export + on-stop-jobs (mf/use-fn (mf/deps jobs sub) - (fn [event] - (dom/prevent-default event) - ;; stop listening first, so no late message repaints the - ;; entries of a dialog that is going away + (fn [] + ;; stop listening first, so no late message repaints the rows (when (some? sub) (rx/dispose! sub)) ;; only the jobs still in flight are cancelled: finished ones ;; already left the map, and failures are ignored because a job may have just finished on its own (run! dj/cancel-job (vals jobs)) (reset! jobs* {}) - (reset! sub* nil) + (reset! sub* nil))) + + on-cancel-export + (mf/use-fn + (mf/deps on-stop-jobs state*) + (fn [event] + (dom/prevent-default event) + (on-stop-jobs) + ;; the dialog stays open with the terminal states: closing it + ;; is what dismisses it, cancelling whatever is still pending + (swap! state* update :files mark-all-cancelled))) + + on-close + (mf/use-fn + (mf/deps on-stop-jobs) + (fn [event] + (dom/prevent-default event) + ;; closing dismisses the dialog, cancelling whatever is still + ;; pending, so an export never outlives its dialog + (on-stop-jobs) (st/emit! (modal/hide)))) on-cancel (mf/use-fn - (mf/deps status on-cancel-export) + (mf/deps status in-progress? on-cancel-export on-close) (fn [event] - (if (= :exporting status) - ;; closing mid-export stops the jobs, like Cancel + ;; the X cancels like Cancel Export while files are still in + ;; flight, and dismisses like Done once everything reached a + ;; terminal state + (if (and (= :exporting status) in-progress?) (on-cancel-export event) - (do - (dom/prevent-default event) - (st/emit! (modal/hide)))))) + (on-close event)))) on-accept (mf/use-fn @@ -243,7 +378,7 @@ [:> heading* {:level 2 :typography t/headline-large :class (stl/css :modal-title)} - (tr "files-download-modal.title")] + (tr "files-export-modal.title")] [:> icon-button* {:variant "ghost" :aria-label (tr "labels.close") :on-click on-cancel @@ -308,7 +443,7 @@ [:div {:class (stl/css :action-buttons)} [:> button* {:variant "secondary" :type "button" - :on-click on-cancel} + :on-click on-close} (tr "labels.cancel")] [:> button* {:variant "primary" @@ -317,21 +452,20 @@ (tr "labels.continue")]]]] (= status :exporting) - (let [in-progress? (->> state :files (some :loading))] - [:* - [:div {:class (stl/css :modal-content)} - (for [file (:files state)] - [:> export-entry* {:file file :key (dm/str (:id file))}])] + [:* + [:div {:class (stl/css :modal-content)} + [:> export-phase-header* {:files (:files state)}] + (for [file (:files state)] + [:> export-entry* {:file file :key (dm/str (:id file))}])] - [:div {:class (stl/css :modal-footer)} - [:div {:class (stl/css :action-buttons)} - (when in-progress? - [:> button* {:variant "secondary" - :type "button" - :on-click on-cancel-export} - (tr "labels.cancel")]) + [:div {:class (stl/css :modal-footer)} + [:div {:class (stl/css :action-buttons)} + (if in-progress? + [:> button* {:variant "secondary" + :type "button" + :on-click on-cancel-export} + (tr "files-export-modal.cancel-export")] [:> button* {:variant "primary" :type "button" - :disabled in-progress? - :on-click on-cancel} - (tr "labels.close")]]]]))]])) + :on-click on-close} + (tr "files-export-modal.done")])]]])]])) diff --git a/frontend/src/app/main/ui/exports/files.scss b/frontend/src/app/main/ui/exports/files.scss index b2a56e435e..6fa97eae35 100644 --- a/frontend/src/app/main/ui/exports/files.scss +++ b/frontend/src/app/main/ui/exports/files.scss @@ -48,6 +48,12 @@ flex-direction: column; gap: var(--sp-l); margin-block-end: var(--sp-xxl); + + // like the import wizard: the rows scroll and the header and footer + // stay put, instead of the content pushing the footer out of the box + flex: 1; + min-block-size: 0; + overflow: hidden auto; } .modal-content-extended { @@ -153,60 +159,153 @@ margin: 0; } -.file-entry { - --file-entry-color: var(--color-foreground-secondary); +.phase-header { + display: flex; + flex-direction: column; + gap: var(--sp-xs); +} - &.loading { - .file-name { - color: var(--color-foreground-secondary); - } +.phase-title { + color: var(--color-foreground-primary); +} + +.phase-message { + color: var(--color-foreground-secondary); + margin: 0; +} + +.file-entry { + --file-entry-fg-color: var(--color-foreground-secondary); + + display: flex; + flex-direction: column; + gap: var(--sp-m); + + &.success { + --file-entry-fg-color: var(--color-accent-success); } &.error { - .file-name { - color: var(--color-foreground-secondary); + --file-entry-fg-color: var(--color-accent-error); - .file-icon svg { - stroke: var(--color-foreground-secondary); - } - } - } - - &.success { - .file-name { - color: var(--color-foreground-secondary); - - .file-icon svg { - stroke: var(--color-foreground-secondary); - } - } + padding: var(--sp-m); + border-radius: $br-8; + background-color: var(--color-background-error); } } -.file-name { +.file-main { display: flex; align-items: center; gap: var(--sp-m); +} - .file-icon { - display: flex; - justify-content: center; - align-items: center; - block-size: px2rem(16); - inline-size: px2rem(16); - color: var(--color-foreground-secondary); - } +.file-icon { + display: flex; + justify-content: center; + align-items: center; + block-size: px2rem(40); + inline-size: px2rem(40); + flex-shrink: 0; + border-radius: $br-8; + background-color: var(--color-background-tertiary); + color: var(--color-foreground-secondary); - .file-name-label { - display: block; - max-inline-size: 99%; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; + .error & { + background-color: var(--color-accent-error); + color: var(--color-background-error); } } -.icon-status-tick { - fill: none; - stroke: var(--color-accent-primary); +.file-meta { + display: flex; + flex-direction: column; + gap: var(--sp-xxs); + flex: 1 1 auto; + min-inline-size: 0; + + .error & { + color: var(--color-foreground-primary); + } +} + +.file-name-label { + display: block; + max-inline-size: 99%; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + color: var(--color-foreground-primary); +} + +.file-subtitle { + color: var(--color-foreground-secondary); + + .error & { + color: var(--file-entry-fg-color); + } +} + +.file-status { + display: flex; + align-items: center; + gap: var(--sp-s); + flex-shrink: 0; + color: var(--file-entry-fg-color); + white-space: nowrap; +} + +.file-progress { + display: flex; + flex-direction: column; + gap: var(--sp-xs); +} + +.progress-message { + color: var(--color-foreground-secondary); +} + +.progress-bar { + block-size: px2rem(4); + border-radius: $br-8; + background-color: var(--color-background-tertiary); + overflow: hidden; +} + +.progress-fill { + block-size: 100%; + border-radius: $br-8; + background-color: var(--color-accent-primary); + + // the twin of the indeterminate bar of the import wizard: the stages + // of an export are not comparable one with another, so the fill slides + // through the bar while the job of the file runs + width: 40%; + animation: progress-slide 1.2s ease-in-out infinite; +} + +@keyframes progress-slide { + from { + transform: translateX(-100%); + } + + to { + transform: translateX(250%); + } +} + +.file-error { + padding-inline-start: calc(px2rem(40) + var(--sp-m)); + + .error & { + padding-inline-start: 0; + } +} + +.error-message { + display: flex; + align-items: flex-start; + color: var(--file-entry-fg-color); + white-space: pre-wrap; + overflow-wrap: anywhere; } diff --git a/frontend/src/app/main/ui/jobs/progress.cljs b/frontend/src/app/main/ui/jobs/progress.cljs index 83967aa978..c3f1404a66 100644 --- a/frontend/src/app/main/ui/jobs/progress.cljs +++ b/frontend/src/app/main/ui/jobs/progress.cljs @@ -5,8 +5,8 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns app.main.ui.jobs.progress - "How a milestone of a job is written in a row: the stage in course and - its counter. + "How the milestone of a job is written: the stage in course and its + counter. The stage is a stable key of the vocabulary of the worker, never a display string, so it is resolved here with a `case` of literal `tr` @@ -42,28 +42,38 @@ (str current "/" total) (str current)))) +(defn upload-pct + "The whole percentage of the file already uploaded. The chunked upload + knows how many chunks the file has and which one is in course, so + this is the only counter the client can trust as a percentage." + [{:keys [counters]}] + (let [{:keys [current total]} (get counters :upload)] + (when (and (some? current) (some? total) (pos? total)) + (js/Math.round (* 100 (/ current total)))))) + (defn- upload-label "The upload of a file counts chunks, not units of work, so it is written as the percentage of the file already sent instead of a counter." - [{:keys [current total]}] - (when (and (some? current) (some? total) (pos? total)) - (str "(" (js/Math.round (* 100 (/ current total))) "%)"))) + [milestone] + (when-some [pct (upload-pct milestone)] + (str "(" pct "%)"))) (defn milestone-text - "The text of a milestone: the stage in course and its counter, and with - `:file?` the file in course behind it, when the milestone carries its - counter. The upload stage is the exception: it counts chunks, so it is - written as a percentage of the file." - [milestone & {:keys [file?]}] + "The text of a milestone: the stage in course and its counter. The + upload of a file counts chunks, not units of work, so it is written as + the percentage of the file already sent instead of a counter." + [milestone] (when-some [stage (:stage milestone)] - (let [counters (:counters milestone) - own (if (= :upload stage) - (upload-label (get counters :upload)) - (counter-label (get counters stage)))] + (let [own (if (= :upload stage) + (upload-label milestone) + (counter-label (get (:counters milestone) stage)))] (str (stage-label stage) (when (some? own) - (str " " own)) - (when-let [files (and file? - (not= :files stage) - (counter-label (get counters :files)))] - (str " · " (stage-label :files) " " files)))))) + (str " " own)))))) + +(defn current-milestone-text + "The text of the milestone in course, for a caller that writes the + progress once for a whole group of files: every file of a container + reports the same milestone, so a row would repeat it." + [entries] + (some (comp milestone-text :progress) entries)) diff --git a/frontend/test/frontend_tests/data/exports_files_test.cljs b/frontend/test/frontend_tests/data/exports_files_test.cljs index fc648c3b55..9e7f15ebd5 100644 --- a/frontend/test/frontend_tests/data/exports_files_test.cljs +++ b/frontend/test/frontend_tests/data/exports_files_test.cljs @@ -125,7 +125,8 @@ (t/is (= {:file-id (:id target) :uri "http://assets/export" - :filename "My file"} + :filename "My file" + :size 120} (last @seen))))))))) (t/deftest ^:async a-failure-fails-only-its-own-file @@ -165,7 +166,8 @@ (t/testing "the failure does not touch the file that worked" (t/is (= {:file-id (:id one) :uri "http://assets/first" - :filename "First"} + :filename "First" + :size nil} (first (filter #(and (= (:id one) (:file-id %)) (:uri %)) @seen))))) diff --git a/frontend/test/frontend_tests/data/imports_test.cljs b/frontend/test/frontend_tests/data/imports_test.cljs index e73802fe38..e173ec3671 100644 --- a/frontend/test/frontend_tests/data/imports_test.cljs +++ b/frontend/test/frontend_tests/data/imports_test.cljs @@ -137,6 +137,7 @@ (mock/with-mocks* {http/send! (slurp-stub zip-magic) uz/get-entry (mock/stub (fn [_ _] (p/resolved ::entry))) + uz/get-entries (mock/stub (fn [_] (p/resolved #js []))) uz/read-as-text (mock/stub (fn [_] (p/resolved (manifest [first-id "First"] [second-id "Second"])))) uz/close (mock/stub (fn [_] nil))} @@ -153,6 +154,62 @@ (t/testing "all of them share the upload of the file that was dropped" (t/is (every? #(= uri (:uri %)) @seen))))))) +(t/deftest ^:async a-version-3-package-sizes-every-file-with-what-it-occupies + (let [first-id (uuid/next) + second-id (uuid/next) + unknown-id (uuid/next) + object-id (str (uuid/next)) + seen (atom []) + ;; the entries of a container: every file keeps its own under + ;; `files/`, the binaries of its media shared under `objects/`, + ;; and the manifest belongs to the container itself + entries + #js [#js {:filename "manifest.json" + :compressedSize 100} + #js {:filename (str "files/" first-id ".json") + :compressedSize 200} + #js {:filename (str "files/" first-id "/pages/one.json") + :compressedSize 300} + #js {:filename (str "files/" first-id "/media/one.json") + :compressedSize 10 + :content (str "{\"mediaId\": \"" object-id "\"}")} + #js {:filename (str "objects/" object-id ".json") + :compressedSize 20} + #js {:filename (str "objects/" object-id ".png") + :compressedSize 1000} + #js {:filename (str "files/" second-id ".json") + :compressedSize 400}]] + + (await + (mock/with-mocks* + {http/send! (slurp-stub zip-magic) + ;; the manifest is read through the same reader as the rest of the + ;; entries, so the fake carries its text and the mock answers what + ;; every entry holds + uz/get-entry (mock/stub (fn [_ _] + (p/resolved #js {:content (manifest [first-id "First"] + [second-id "Second"] + [unknown-id "Unknown"])}))) + uz/get-entries (mock/stub (fn [_] (p/resolved entries))) + uz/read-as-text (mock/stub (fn [entry] (p/resolved (.-content entry)))) + uz/close (mock/stub (fn [_] nil))} + + (await (hva/observe (imp/analyze [{:uri uri :name "package.penpot" :size 10000}]) + {:on-next #(swap! seen conj %)})))) + + (t/testing "a file occupies its own entries and the objects it references" + ;; 200 + 300 + 10 of its own, and 20 + 1000 of the object its media + ;; points at; the 100 of the manifest belong to the container + (t/is (= [1530 400 nil] (mapv :size @seen)))) + + (t/testing "a file the container does not hold has no size" + ;; the manifest lists it, but no entry of the zip belongs to it: the + ;; row says nothing rather than borrowing what the package weighs + (t/is (nil? (:size (first (filter #(= unknown-id (:file-id %)) @seen)))))) + + (t/testing "what the container occupies is kept apart" + (t/is (= [10000 10000 10000] (mapv :container-size @seen)))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; IMPORTING A PACKAGE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -510,6 +567,190 @@ (rx/push! ws-stream (message (event job-id :end {:outcome "completed"}))) (await done)))))) +(t/deftest format-file-size-writes-human-sizes + (t/testing "unknown sizes show nothing" + (t/is (nil? (imp/format-file-size nil))) + (t/is (nil? (imp/format-file-size -1)))) + + (t/testing "bytes, kilobytes, megabytes and gigabytes" + (t/is (= "0B" (imp/format-file-size 0))) + (t/is (= "512B" (imp/format-file-size 512))) + (t/is (= "1KB" (imp/format-file-size 1024))) + (t/is (= "1.5KB" (imp/format-file-size 1536))) + (t/is (= "512KB" (imp/format-file-size (* 512 1024)))) + (t/is (= "234MB" (imp/format-file-size (* 234 1024 1024)))) + (t/is (= "1.2GB" (imp/format-file-size (* 1.2 1024 1024 1024)))))) + +(defn- uploading + "A file of the wizard while the container it came in is uploaded: the + chunks sent out of the chunks the container has." + [uri size current total] + {:file-id (uuid/next) + :uri uri + :size (* 1024 1024) + :container-size size + :type :binfile-v3 + :status :import-progress + :progress {:stage :upload :counters {:upload {:current current :total total}}}}) + +(t/deftest upload-pct-weighs-every-container-by-its-size + (let [small (* 100 1024 1024) + big (* 300 1024 1024)] + + (t/testing "the bytes sent of every container over the bytes of all of them" + ;; 100MB of 100MB and half of 300MB: 250 of 400, not the average of + ;; 100% and 50% + (t/is (= 63 (imp/upload-pct [(uploading "a" small 4 4) + (uploading "b" big 1 2)])))) + + (t/testing "a container that has not started counts as nothing sent" + (t/is (= 25 (imp/upload-pct [(uploading "a" small 4 4) + {:uri "b" :container-size big :type :binfile-v3 + :status :import-ready}])))) + + (t/testing "the files of one container are one container" + (t/is (= 100 (imp/upload-pct [(uploading "a" big 1 1) + (uploading "a" big 1 1)])))) + + (t/testing "a container with no size leaves the bar indeterminate" + (t/is (nil? (imp/upload-pct []))) + (t/is (nil? (imp/upload-pct [{:uri "a" :type :binfile-v3 :status :import-ready}])))))) + +(t/deftest a-container-past-the-upload-counts-whole + ;; the job of a container is created with its upload as its input, so the + ;; moment the entries leave the upload stage the container is sent + (t/is (= 100 (imp/upload-pct [{:uri "a" :name "a.penpot" :container-size (* 100 1024 1024) + :type :binfile-v1 :status :import-queued} + {:uri "b" :name "b.penpot" :container-size (* 300 1024 1024) + :type :binfile-v1 :status :import-success}])))) + +(t/deftest upload-done?-tells-when-the-chunks-of-a-container-are-sent + (t/testing "the last chunk closes the upload" + (t/is (true? (imp/upload-done? (uploading "a" 100 3 3)))) + (t/is (false? (imp/upload-done? (uploading "a" 100 2 3))))) + + (t/testing "a file that is not uploading yet is not done" + (t/is (false? (imp/upload-done? {:status :import-ready}))) + (t/is (false? (imp/upload-done? {}))))) + +(t/deftest imported-count-counts-the-files-the-job-left-behind + (let [file (fn [uri status progress] + (cond-> {:uri uri :type :binfile-v3 :status status} + (some? progress) (assoc :progress progress)))] + + (t/testing "a container that finished counts all of its files" + (t/is (= 2 (imp/imported-count [(file "a" :import-success nil) + (file "a" :import-success nil)])))) + + (t/testing "a running container counts the files it left behind" + ;; its milestone says it is working on the second file of three + (t/is (= 1 (imp/imported-count [(file "a" :import-progress + {:stage :pages + :counters {:files {:current 2 :total 3} + :pages {:current 1 :total 5}}})])))) + + (t/testing "a container that has not started has nothing done" + (t/is (= 0 (imp/imported-count [(file "a" :import-queued nil)]))) + (t/is (= 0 (imp/imported-count [(file "a" :import-progress + {:stage :pages :counters {}})])))) + + (t/testing "the files of every container add up" + (t/is (= 3 (imp/imported-count [(file "a" :import-success nil) + (file "a" :import-success nil) + (file "b" :import-progress + {:stage :pages + :counters {:files {:current 2 :total 4}}})])))) + + (t/testing "no files, nothing done" + (t/is (= 0 (imp/imported-count [])))))) + +(t/deftest waiting-slot?-is-true-until-a-worker-picks-the-jobs-up + (t/testing "every file queued is a wizard waiting for a slot" + (t/is (true? (imp/waiting-slot? [{:status :import-queued} + {:status :import-queued}])))) + + (t/testing "a job in course leaves the wait behind" + (t/is (false? (imp/waiting-slot? [{:status :import-queued} + {:status :import-progress}])))) + + (t/testing "a file already imported is not waiting either" + (t/is (false? (imp/waiting-slot? [{:status :import-success}])))) + + (t/testing "no files, no wait" + (t/is (false? (imp/waiting-slot? []))))) + +(t/deftest awaiting-analysis-hides-the-rows-while-checking + (t/testing "only a clean analysis shows the placeholder" + (t/is (true? (imp/awaiting-analysis? :analyze false))) + (t/is (false? (imp/awaiting-analysis? :analyze true)))) + (t/testing "any later stage shows the rows" + (t/is (false? (imp/awaiting-analysis? :import-ready false))) + (t/is (false? (imp/awaiting-analysis? :import-progress false))) + (t/is (false? (imp/awaiting-analysis? :import-success false))))) + +(t/deftest ready-to-import?-waits-for-the-whole-analysis + (t/testing "every file found and ready, with the analysis over" + (t/is (true? (imp/ready-to-import? [{:status :import-ready} + {:status :import-ready}] + true)))) + + (t/testing "a file that has not arrived keeps the wizard checking" + ;; the files of a container are only known once its manifest is read, so + ;; offering the import step here would leave the missing one behind + (t/is (false? (imp/ready-to-import? [{:status :import-ready}] false)))) + + (t/testing "a file that cannot be imported is not ready either" + (t/is (false? (imp/ready-to-import? [{:status :import-ready} + {:status :analyze-error}] + true)))) + + (t/testing "no files at all is not ready" + (t/is (false? (imp/ready-to-import? [] true))))) + +(t/deftest import-step-follows-the-wizard-status + (t/testing "analysis paints the check step" + (t/is (= :check (imp/import-step :analyze []))) + (t/is (= :check (imp/import-step :import-ready [])))) + + (t/testing "an upload milestone paints the upload step" + (let [entries [{:file-id (uuid/next) + :status :import-progress + :progress {:stage :upload :counters {:upload {:current 1 :total 2}}}}]] + (t/is (= :upload (imp/import-step :import-progress entries))))) + + (t/testing "a file that has not reported anything yet is still uploading" + ;; the wizard is uploading from the click, before the first chunk arrives + (let [entries [{:file-id (uuid/next) :status :import-ready} + {:file-id (uuid/next) + :status :import-progress + :progress {:stage :upload :counters {:upload {:current 1 :total 2}}}}]] + (t/is (= :upload (imp/import-step :import-progress entries))))) + + (t/testing "a finished upload with its job still to come is uploading" + (let [entries [{:file-id (uuid/next) + :status :import-progress + :progress {:stage :upload :counters {:upload {:current 2 :total 2}}}} + {:file-id (uuid/next) + :status :import-progress + :progress {:stage :upload :counters {:upload {:current 1 :total 4}}}}]] + (t/is (= :upload (imp/import-step :import-progress entries))))) + + (t/testing "queued or job milestones paint the import step" + (let [queued [{:file-id (uuid/next) :status :import-queued}] + started [{:file-id (uuid/next) :status :import-progress}] + pages [{:file-id (uuid/next) + :status :import-progress + :progress {:stage :pages :counters {:pages {:current 1 :total 8}}}}]] + (t/is (= :import (imp/import-step :import-progress queued))) + (t/is (= :import (imp/import-step :import-progress started))) + (t/is (= :import (imp/import-step :import-progress pages))))) + + (t/testing "terminal and resolution stages paint the import step" + (t/is (= :import (imp/import-step :import-success []))) + (t/is (= :import (imp/import-step :import-error []))) + (t/is (= :import (imp/import-step :library-resolution []))) + (t/is (= :import (imp/import-step :library-summary []))))) + (t/deftest ^:async cancel-job-asks-the-server-and-ignores-failures (let [calls (atom []) job-id (uuid/next)] diff --git a/frontend/test/frontend_tests/ui/jobs_progress_test.cljs b/frontend/test/frontend_tests/ui/jobs_progress_test.cljs index 9d25f852af..55937e561e 100644 --- a/frontend/test/frontend_tests/ui/jobs_progress_test.cljs +++ b/frontend/test/frontend_tests/ui/jobs_progress_test.cljs @@ -5,8 +5,7 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns frontend-tests.ui.jobs-progress-test - "How a job milestone is written: the stage and its counter, and the - upload stage as a percentage of the file." + "How a job milestone is written: the stage and its counter." (:require [app.main.ui.jobs.progress :as jp] [app.util.i18n :as i18n] @@ -24,15 +23,7 @@ (t/is (= "TR:jobs.progress.stage.pages 3" (jp/milestone-text {:stage :pages :counters {:pages {:current 3}}})))))) -(t/deftest a-file-milestone-appends-the-file-counter - (with-redefs [i18n/tr stub-tr] - (t/is (= "TR:jobs.progress.stage.pages 3/8 · TR:jobs.progress.stage.files 1/2" - (jp/milestone-text {:stage :pages - :counters {:pages {:current 3 :total 8} - :files {:current 1 :total 2}}} - :file? true))))) - -(t/deftest an-upload-milestone-writes-the-percentage-of-the-file +(t/deftest the-upload-milestone-writes-the-percentage-of-the-file (with-redefs [i18n/tr stub-tr] (t/testing "the chunks sent become a whole percentage" (t/is (= "TR:jobs.progress.stage.upload (33%)" @@ -42,3 +33,22 @@ (t/testing "an upload with no total keeps the bare label" (t/is (= "TR:jobs.progress.stage.upload" (jp/milestone-text {:stage :upload :counters {:upload {:current 1}}})))))) + +(t/deftest the-milestone-in-course-is-the-one-of-the-files-that-report-it + (with-redefs [i18n/tr stub-tr] + (t/testing "the files of a container share their milestone, so one is enough" + (t/is (= "TR:jobs.progress.stage.pages 4/5" + (jp/current-milestone-text + [{:status :import-queued} + {:progress {:stage :pages :counters {:pages {:current 4 :total 5}}}} + {:progress {:stage :pages :counters {:pages {:current 4 :total 5}}}}])))) + + (t/testing "a file with nothing to report says nothing" + (t/is (nil? (jp/current-milestone-text [{:status :import-ready} + {:status :import-queued}]))) + (t/is (nil? (jp/current-milestone-text [])))))) + +(t/deftest a-milestone-without-a-stage-writes-nothing + (with-redefs [i18n/tr stub-tr] + (t/is (nil? (jp/milestone-text nil))) + (t/is (nil? (jp/milestone-text {}))))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 8d5af490a5..2e790e4f14 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -681,6 +681,70 @@ msgstr "Connect selected libraries" msgid "dashboard.import.file-name-label" msgstr "File name" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.title" +msgstr "Import file" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.status.ready" +msgstr "Ready" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.label" +msgstr "Import steps" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.check" +msgstr "Check" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.upload" +msgstr "Upload" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.import" +msgstr "Import" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.check.title" +msgstr "Checking your files..." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.check.message" +msgstr "Making sure each file can be imported. This only takes a moment." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.ready.title" +msgstr "Files ready" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.ready.message" +msgstr "Your files can be imported. Continue to upload them." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.upload.title" +msgstr "Uploading your files..." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.upload.message" +msgstr "Sending your files. Keep this dialog open until the upload ends." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.title" +msgid_plural "dashboard.import.importing.title" +msgstr[0] "Importing 1 file" +msgstr[1] "Importing %s files" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.done" +msgid_plural "dashboard.import.importing.done" +msgstr[0] "1 done" +msgstr[1] "%s done" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.message" +msgstr "Rebuilding pages, components and assets in your project." + #: src/app/main/ui/dashboard/import.cljs:619 msgid "dashboard.import.import-error.check-error" msgstr "We couldn't verify this file." @@ -723,6 +787,14 @@ msgstr "Next file" msgid "dashboard.import.no-perms" msgstr "You don’t have permission to import to this team" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.queued.title" +msgstr "Waiting to start..." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.queued.message" +msgstr "Your files are uploaded. The import starts as soon as a slot is free." + #: src/app/main/ui/dashboard/import.cljs:443 msgid "dashboard.import.resolve-libraries" msgstr "" @@ -2105,9 +2177,37 @@ msgstr "Idea" msgid "feedback.type.issue" msgstr "Issue" -#: src/app/main/ui/exports/files.cljs:140 -msgid "files-download-modal.title" -msgstr "Download files" +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.cancel-export" +msgstr "Cancel export" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.done" +msgstr "Done" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.export-error" +msgstr "Couldn't export: %s" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exported.message" +msgstr "Check your downloads folder." + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exported.title" +msgid_plural "files-export-modal.exported.title" +msgstr[0] "1 file exported" +msgstr[1] "%s files exported" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exporting.message" +msgstr "Each file downloads as soon as it's ready." + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exporting.title" +msgid_plural "files-export-modal.exporting.title" +msgstr[0] "Exporting 1 file" +msgstr[1] "Exporting %s files" #: src/app/main/ui/exports/files.cljs:183 msgid "files-export-modal.options.detach-libraries.message" @@ -2149,6 +2249,18 @@ msgstr "Embed library assets in the file" msgid "files-download-modal.unsaved-changes" msgstr "Some of your latest changes are not saved yet, so this download will not include them." +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.status.exported" +msgstr "Exported" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.status.waiting" +msgstr "Waiting" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.title" +msgstr "Export file" + #: src/app/main/ui/settings/password.cljs:45 msgid "generic.error" msgstr "An error has occurred" @@ -2700,7 +2812,7 @@ msgstr "Uploading file…" #: src/app/main/ui/dashboard/import.cljs:328, src/app/main/ui/exports/files.cljs:103 msgid "jobs.queued" -msgstr "Queued" +msgstr "Waiting job slot" #: src/app/main/ui/dashboard/comments.cljs:94 msgid "label.mark-all-as-read" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 0e75450d92..3da6305f9e 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -680,6 +680,16 @@ msgstr[1] "%s bibliotecas fueron vinculadas automáticamente por nombre." msgid "dashboard.import.bad-url" msgstr "La importación ha fallado. La URL de la plantilla es incorrecta" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.check.message" +msgstr "" +"Estamos comprobando que cada archivo se puede importar. Solo tardará un " +"momento." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.check.title" +msgstr "Comprobando tus archivos..." + #: src/app/main/ui/dashboard/import.cljs:733 msgid "dashboard.import.confirm-library-links" msgstr "Confirmar vínculos de biblioteca" @@ -726,6 +736,22 @@ msgstr[1] "%s ficheros se han importado correctamente." msgid "dashboard.import.import-warning" msgstr "Algunos ficheros contenían objetos erroneos que no han sido importados." +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.done" +msgid_plural "dashboard.import.importing.done" +msgstr[0] "1 hecho" +msgstr[1] "%s hechos" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.message" +msgstr "Reconstruyendo páginas, componentes y recursos en tu proyecto." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.importing.title" +msgid_plural "dashboard.import.importing.title" +msgstr[0] "Importando 1 archivo" +msgstr[1] "Importando %s archivos" + #: src/app/main/ui/dashboard/import.cljs:710 msgid "dashboard.import.next-file" msgstr "Siguiente archivo" @@ -734,6 +760,24 @@ msgstr "Siguiente archivo" msgid "dashboard.import.no-perms" msgstr "No tienes permisos para importar en este equipo" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.queued.message" +msgstr "" +"Tus archivos ya están subidos. La importación empieza en cuanto haya un " +"hueco libre." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.queued.title" +msgstr "Esperando a empezar..." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.ready.message" +msgstr "Tus archivos se pueden importar. Continúa con la subida." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.ready.title" +msgstr "Archivos listos" + #: src/app/main/ui/dashboard/import.cljs:443 msgid "dashboard.import.resolve-libraries" msgstr "" @@ -752,6 +796,26 @@ msgstr "conectar a" msgid "dashboard.import.resolve-libraries.original-library" msgstr "biblioteca original" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.status.ready" +msgstr "Listo" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.check" +msgstr "Comprobación" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.import" +msgstr "Importación" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.label" +msgstr "Pasos de la importación" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.steps.upload" +msgstr "Subida" + #: src/app/main/ui/dashboard/import.cljs:521 msgid "dashboard.import.summary.linked" msgstr "auto-vinculada" @@ -772,6 +836,20 @@ msgstr "Ninguna biblioteca seleccionada" msgid "dashboard.import.summary.original" msgstr "Original" +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.title" +msgstr "Importar archivo" + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.upload.message" +msgstr "" +"Enviando tus archivos. Mantén este diálogo abierto hasta que termine la " +"subida." + +#: src/app/main/ui/dashboard/import.cljs +msgid "dashboard.import.upload.title" +msgstr "Subiendo tus archivos..." + #: src/app/main/ui/dashboard/team.cljs:881 msgid "dashboard.invitation-modal.delete" msgstr "Vas a eliminar las invitaciones para:" @@ -2139,9 +2217,37 @@ msgstr "Idea" msgid "feedback.type.issue" msgstr "Problema" -#: src/app/main/ui/exports/files.cljs:140 -msgid "files-download-modal.title" -msgstr "Descargar ficheros" +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.cancel-export" +msgstr "Cancelar exportación" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.done" +msgstr "Listo" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.export-error" +msgstr "No se ha podido exportar: %s" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exported.message" +msgstr "Revisa tu carpeta de descargas." + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exported.title" +msgid_plural "files-export-modal.exported.title" +msgstr[0] "1 archivo exportado" +msgstr[1] "%s archivos exportados" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exporting.message" +msgstr "Cada archivo se descarga en cuanto está listo." + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.exporting.title" +msgid_plural "files-export-modal.exporting.title" +msgstr[0] "Exportando 1 archivo" +msgstr[1] "Exportando %s archivos" #: src/app/main/ui/exports/files.cljs:183 msgid "files-export-modal.options.detach-libraries.message" @@ -2185,6 +2291,18 @@ msgstr "Incluir librerias compartidas dentro de las librerias del fichero" msgid "files-download-modal.unsaved-changes" msgstr "Algunos de tus últimos cambios aún no se han guardado, así que esta descarga no los incluirá." +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.status.exported" +msgstr "Exportado" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.status.waiting" +msgstr "En espera" + +#: src/app/main/ui/exports/files.cljs +msgid "files-export-modal.title" +msgstr "Exportar archivo" + #: src/app/main/ui/settings/password.cljs:45 msgid "generic.error" msgstr "Ha ocurrido un error" @@ -2738,7 +2856,7 @@ msgstr "Subiendo archivo…" #: src/app/main/ui/dashboard/import.cljs:328, src/app/main/ui/exports/files.cljs:103 msgid "jobs.queued" -msgstr "En cola" +msgstr "Esperando hueco para el job" #: src/app/main/ui/dashboard/comments.cljs:94 msgid "label.mark-all-as-read"