From 92643b29c146a50be657ef83ab0d1d010e69ab86 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sun, 14 Jan 2024 20:45:11 +0100 Subject: [PATCH 01/54] :sparkles: Improve internal cache api --- backend/resources/log4j2-devenv.xml | 2 +- backend/src/app/main.clj | 3 +- backend/src/app/redis.clj | 7 +- backend/src/app/rpc/climit.clj | 115 +++++++++++++++------------- backend/src/app/util/cache.clj | 80 ++++++++++--------- 5 files changed, 112 insertions(+), 95 deletions(-) diff --git a/backend/resources/log4j2-devenv.xml b/backend/resources/log4j2-devenv.xml index 7abb7a1885..ca1ab6739a 100644 --- a/backend/resources/log4j2-devenv.xml +++ b/backend/resources/log4j2-devenv.xml @@ -30,7 +30,7 @@ - + diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index c80210a06b..c2f20015c3 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -301,7 +301,8 @@ ::sto/storage (ig/ref ::sto/storage)} :app.rpc/climit - {::mtx/metrics (ig/ref ::mtx/metrics)} + {::mtx/metrics (ig/ref ::mtx/metrics) + ::wrk/executor (ig/ref ::wrk/executor)} :app.rpc/rlimit {::wrk/executor (ig/ref ::wrk/executor)} diff --git a/backend/src/app/redis.clj b/backend/src/app/redis.clj index b730ab1063..58023fe00e 100644 --- a/backend/src/app/redis.clj +++ b/backend/src/app/redis.clj @@ -91,7 +91,7 @@ (s/def ::connect? ::us/boolean) (s/def ::io-threads ::us/integer) (s/def ::worker-threads ::us/integer) -(s/def ::cache some?) +(s/def ::cache cache/cache?) (s/def ::redis (s/keys :req [::resources @@ -168,7 +168,7 @@ (defn- shutdown-resources [{:keys [::resources ::cache ::timer]}] - (cache/invalidate-all! cache) + (cache/invalidate! cache) (when resources (.shutdown ^ClientResources resources)) @@ -211,7 +211,8 @@ (defn get-or-connect [{:keys [::cache] :as state} key options] (us/assert! ::redis state) - (let [connection (cache/get cache key (fn [_] (connect* state options)))] + (let [create (fn [_] (connect* state options)) + connection (cache/get cache key create)] (-> state (dissoc ::cache) (assoc ::connection connection)))) diff --git a/backend/src/app/rpc/climit.clj b/backend/src/app/rpc/climit.clj index d6e4ccb51b..71c64b596a 100644 --- a/backend/src/app/rpc/climit.clj +++ b/backend/src/app/rpc/climit.clj @@ -36,24 +36,14 @@ (-> (str id) (subs 1))) -(defn- create-bulkhead-cache - [config] - (letfn [(load-fn [[id skey]] - (when-let [config (get config id)] - (l/trc :hint "insert into cache" :id (id->str id) :key skey) - (pbh/create :permits (or (:permits config) (:concurrency config)) - :queue (or (:queue config) (:queue-size config)) - :timeout (:timeout config) - :type :semaphore))) - - (on-remove [key _ cause] +(defn- create-cache + [{:keys [::wrk/executor]}] + (letfn [(on-remove [key _ cause] (let [[id skey] key] - (l/trc :hint "evict from cache" :id (id->str id) :key skey :reason (str cause))))] - - (cache/create :executor :same-thread + (l/dbg :hint "destroy limiter" :id (id->str id) :key skey :reason (str cause))))] + (cache/create :executor executor :on-remove on-remove - :keepalive "5m" - :load-fn load-fn))) + :keepalive "5m"))) (s/def ::config/permits ::us/integer) (s/def ::config/queue ::us/integer) @@ -70,7 +60,7 @@ (s/def ::path ::fs/path) (defmethod ig/pre-init-spec ::rpc/climit [_] - (s/keys :req [::mtx/metrics ::path])) + (s/keys :req [::mtx/metrics ::wrk/executor ::path])) (defmethod ig/init-key ::rpc/climit [_ {:keys [::path ::mtx/metrics] :as cfg}] @@ -78,7 +68,7 @@ (when-let [params (some->> path slurp edn/read-string)] (l/inf :hint "initializing concurrency limit" :config (str path)) (us/verify! ::config params) - {::cache (create-bulkhead-cache params) + {::cache (create-cache cfg) ::config params ::mtx/metrics metrics}))) @@ -89,13 +79,17 @@ (s/def ::rpc/climit (s/nilable ::instance)) -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; PUBLIC API -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn- create-limiter + [config [id skey]] + (l/dbg :hint "create limiter" :id (id->str id) :key skey) + (pbh/create :permits (or (:permits config) (:concurrency config)) + :queue (or (:queue config) (:queue-size config)) + :timeout (:timeout config) + :type :semaphore)) -(defn invoke! - [cache metrics id key f] - (if-let [limiter (cache/get cache [id key])] +(defn- invoke! + [config cache metrics id key f] + (if-let [limiter (cache/get cache [id key] (partial create-limiter config))] (let [tpoint (dt/tpoint) labels (into-array String [(id->str id)]) wrapped (fn [] @@ -147,7 +141,7 @@ :queue (:queue stats) :max-permits (:max-permits stats) :max-queue (:max-queue stats)) - (pbh/invoke! limiter wrapped)) + (px/invoke! limiter wrapped)) (catch ExceptionInfo cause (let [{:keys [type code]} (ex-data cause)] (if (= :bulkhead-error type) @@ -160,9 +154,43 @@ (measure! (pbh/get-stats limiter))))) (do - (l/wrn :hint "unable to load limiter" :id (id->str id)) + (l/wrn :hint "no limiter found" :id (id->str id)) (f)))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; MIDDLEWARE +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def noop-fn (constantly nil)) + +(defn wrap + [{:keys [::rpc/climit ::mtx/metrics]} f {:keys [::id ::key-fn] :or {key-fn noop-fn} :as mdata}] + (if (and (some? climit) (some? id)) + (let [cache (::cache climit) + config (::config climit)] + (if-let [config (get config id)] + (do + (l/dbg :hint "instrumenting method" + :limit (id->str id) + :service-name (::sv/name mdata) + :timeout (:timeout config) + :permits (:permits config) + :queue (:queue config) + :keyed? (not= key-fn noop-fn)) + + (fn [cfg params] + (invoke! config cache metrics id (key-fn params) (partial f cfg params)))) + + (do + (l/wrn :hint "no config found for specified queue" :id (id->str id)) + f))) + + f)) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; PUBLIC API +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + (defn configure [{:keys [::rpc/climit]} id] (us/assert! ::rpc/climit climit) @@ -171,37 +199,14 @@ (defn run! "Run a function in context of climit. Intended to be used in virtual threads." - ([{:keys [::id ::cache ::mtx/metrics]} f] - (if (and cache id) - (invoke! cache metrics id nil f) + ([{:keys [::id ::cache ::config ::mtx/metrics]} f] + (if-let [config (get config id)] + (invoke! config cache metrics id nil f) (f))) - ([{:keys [::id ::cache ::mtx/metrics]} f executor] + ([{:keys [::id ::cache ::config ::mtx/metrics]} f executor] (let [f #(p/await! (px/submit! executor f))] - (if (and cache id) - (invoke! cache metrics id nil f) + (if-let [config (get config id)] + (invoke! config cache metrics id nil f) (f))))) -(def noop-fn (constantly nil)) - -(defn wrap - [{:keys [::rpc/climit ::mtx/metrics]} f {:keys [::id ::key-fn] :or {key-fn noop-fn} :as mdata}] - (if (and (some? climit) (some? id)) - (if-let [config (get-in climit [::config id])] - (let [cache (::cache climit)] - (l/dbg :hint "instrumenting method" - :limit (id->str id) - :service-name (::sv/name mdata) - :timeout (:timeout config) - :permits (:permits config) - :queue (:queue config) - :keyed? (not= key-fn noop-fn)) - - (fn [cfg params] - (invoke! cache metrics id (key-fn params) (partial f cfg params)))) - - (do - (l/wrn :hint "no config found for specified queue" :id (id->str id)) - f)) - - f)) diff --git a/backend/src/app/util/cache.clj b/backend/src/app/util/cache.clj index c5aa733e6f..65861e1797 100644 --- a/backend/src/app/util/cache.clj +++ b/backend/src/app/util/cache.clj @@ -9,61 +9,71 @@ (:refer-clojure :exclude [get]) (:require [app.util.time :as dt] - [promesa.core :as p] [promesa.exec :as px]) (:import com.github.benmanes.caffeine.cache.AsyncCache - com.github.benmanes.caffeine.cache.AsyncLoadingCache - com.github.benmanes.caffeine.cache.CacheLoader + com.github.benmanes.caffeine.cache.Cache com.github.benmanes.caffeine.cache.Caffeine com.github.benmanes.caffeine.cache.RemovalListener + com.github.benmanes.caffeine.cache.stats.CacheStats java.time.Duration java.util.concurrent.Executor java.util.function.Function)) (set! *warn-on-reflection* true) -(defn create-listener +(defprotocol ICache + (get [_ k] [_ k load-fn] "get cache entry") + (invalidate! [_] [_ k] "invalidate cache")) + +(defprotocol ICacheStats + (stats [_] "get stats")) + +(defn- create-listener [f] (reify RemovalListener (onRemoval [_ key val cause] (when val (f key val cause))))) -(defn create-loader - [f] - (reify CacheLoader - (load [_ key] - (f key)))) +(defn- get-stats + [^Cache cache] + (let [^CacheStats stats (.stats cache)] + {:hit-rate (.hitRate stats) + :hit-count (.hitCount stats) + :req-count (.requestCount stats) + :miss-count (.missCount stats) + :miss-rate (.missRate stats)})) (defn create - [& {:keys [executor on-remove load-fn keepalive]}] - (as-> (Caffeine/newBuilder) builder - (if on-remove (.removalListener builder (create-listener on-remove)) builder) - (if executor (.executor builder ^Executor (px/resolve-executor executor)) builder) - (if keepalive (.expireAfterAccess builder ^Duration (dt/duration keepalive)) builder) - (if load-fn - (.buildAsync builder ^CacheLoader (create-loader load-fn)) - (.buildAsync builder)))) + [& {:keys [executor on-remove max-size keepalive]}] + (let [cache (as-> (Caffeine/newBuilder) builder + (if (fn? on-remove) (.removalListener builder (create-listener on-remove)) builder) + (if executor (.executor builder ^Executor (px/resolve-executor executor)) builder) + (if keepalive (.expireAfterAccess builder ^Duration (dt/duration keepalive)) builder) + (if (int? max-size) (.maximumSize builder (long max-size)) builder) + (.recordStats builder) + (.buildAsync builder)) + cache (.synchronous ^AsyncCache cache)] + (reify + ICache + (get [_ k] + (.getIfPresent ^Cache cache ^Object k)) + (get [_ k load-fn] + (.get ^Cache cache + ^Object k + ^Function (reify Function + (apply [_ k] + (load-fn k))))) + (invalidate! [_] + (.invalidateAll ^Cache cache)) + (invalidate! [_ k] + (.invalidateAll ^Cache cache ^Object k)) -(defn invalidate-all! - [^AsyncCache cache] - (.invalidateAll (.synchronous cache))) - -(defn get - ([cache key] - (assert (instance? AsyncLoadingCache cache) "should be AsyncLoadingCache instance") - (p/await! (.get ^AsyncLoadingCache cache ^Object key))) - ([cache key not-found-fn] - (assert (instance? AsyncCache cache) "should be AsyncCache instance") - (p/await! (.get ^AsyncCache cache - ^Object key - ^Function (reify - Function - (apply [_ key] - (not-found-fn key))))))) + ICacheStats + (stats [_] + (get-stats cache))))) (defn cache? [o] - (or (instance? AsyncCache o) - (instance? AsyncLoadingCache o))) + (satisfies? ICache o)) From 4fc391763e5c84b43a5cc66862cb114aa45f5420 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sun, 14 Jan 2024 20:45:47 +0100 Subject: [PATCH 02/54] :sparkles: Prevent unexpected exception raising on closing s3 file --- backend/src/app/storage/s3.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/app/storage/s3.clj b/backend/src/app/storage/s3.clj index e019ad2676..92d59988b4 100644 --- a/backend/src/app/storage/s3.clj +++ b/backend/src/app/storage/s3.clj @@ -298,7 +298,7 @@ [path] (proxy [FilterInputStream] [(io/input-stream path)] (close [] - (fs/delete path) + (ex/ignoring (fs/delete path)) (proxy-super close)))) (defn- get-object-data From 944d167bbba1f3bd1369699dd107afa87d739425 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sun, 14 Jan 2024 20:53:03 +0100 Subject: [PATCH 03/54] :sparkles: Simplify SVGO module API --- backend/src/app/config.clj | 5 +- backend/src/app/features/components_v2.clj | 187 ++++++++++---------- backend/src/app/main.clj | 3 +- backend/src/app/rpc/commands/binfile.clj | 4 +- backend/src/app/srepl/components_v2.clj | 192 +++++++++++---------- backend/src/app/svgo.clj | 51 ++---- 6 files changed, 205 insertions(+), 237 deletions(-) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index b4fe60c652..a9e883b8ff 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -209,7 +209,6 @@ (s/def ::telemetry-uri ::us/string) (s/def ::telemetry-with-taiga ::us/boolean) (s/def ::tenant ::us/string) -(s/def ::svgo-max-procs ::us/integer) (s/def ::config (s/keys :opt-un [::secret-key @@ -329,9 +328,7 @@ ::telemetry-uri ::telemetry-referer ::telemetry-with-taiga - ::tenant - - ::svgo-max-procs])) + ::tenant])) (def default-flags [:enable-backend-api-doc diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 8eb6772403..b9cf8a79a9 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -31,6 +31,7 @@ [app.common.types.shape-tree :as ctst] [app.common.uuid :as uuid] [app.db :as db] + [app.db.sql :as sql] [app.features.fdata :as fdata] [app.http.sse :as sse] [app.media :as media] @@ -41,6 +42,7 @@ [app.storage.tmp :as tmp] [app.svgo :as svgo] [app.util.blob :as blob] + [app.util.cache :as cache] [app.util.pointer-map :as pmap] [app.util.time :as dt] [buddy.core.codecs :as bc] @@ -52,20 +54,19 @@ "A dynamic var for setting up state for collect stats globally." nil) -(def ^:dynamic *skip-on-error* - "A dynamic var for setting up the default error behavior." - true) +(def ^:dynamic *cache* + "A dynamic var for setting up a cache instance." + nil) + +(def ^:dynamic *skip-on-graphic-error* + "A dynamic var for setting up the default error behavior for graphics processing." + nil) (def ^:dynamic ^:private *system* "An internal var for making the current `system` available to all internal functions without the need to explicitly pass it top down." nil) -(def ^:dynamic ^:private *max-procs* - "A dynamic variable that can optionally indicates the maxumum number - of concurrent graphics migration processes." - nil) - (def ^:dynamic ^:private *file-stats* "An internal dynamic var for collect stats by file." nil) @@ -576,37 +577,30 @@ (defn- collect-and-persist-images [svg-data file-id] (letfn [(process-image [{:keys [href] :as item}] - (try - (let [item (if (str/starts-with? href "data:") - (let [[mtype data] (parse-datauri href) - size (alength data) - path (tmp/tempfile :prefix "penpot.media.download.") - written (io/write-to-file! data path :size size)] + (let [item (if (str/starts-with? href "data:") + (let [[mtype data] (parse-datauri href) + size (alength data) + path (tmp/tempfile :prefix "penpot.media.download.") + written (io/write-to-file! data path :size size)] - (when (not= written size) - (ex/raise :type :internal - :code :mismatch-write-size - :hint "unexpected state: unable to write to file")) + (when (not= written size) + (ex/raise :type :internal + :code :mismatch-write-size + :hint "unexpected state: unable to write to file")) - (-> item - (assoc :size size) - (assoc :path path) - (assoc :filename "tempfile") - (assoc :mtype mtype))) + (-> item + (assoc :size size) + (assoc :path path) + (assoc :filename "tempfile") + (assoc :mtype mtype))) - (let [result (cmd.media/download-image *system* href)] - (-> (merge item result) - (assoc :name (extract-name href)))))] + (let [result (cmd.media/download-image *system* href)] + (-> (merge item result) + (assoc :name (extract-name href)))))] - ;; The media processing adds the data to the - ;; input map and returns it. - (media/run {:cmd :info :input item})) - - (catch Throwable cause - (l/warn :hint "unexpected exception on processing internal image shape (skiping)" - :cause cause) - (when-not *skip-on-error* - (throw cause))))) + ;; The media processing adds the data to the + ;; input map and returns it. + (media/run {:cmd :info :input item}))) (persist-image [acc {:keys [path size width height mtype href] :as item}] (let [storage (::sto/storage *system*) @@ -642,23 +636,36 @@ (completing persist-image) {}))] (assoc svg-data :image-data images)))) -(defn- get-svg-content +(defn- resolve-sobject-id + [id] + (let [fmobject (db/get *system* :file-media-object {:id id} + {::db/check-deleted false + ::db/remove-deleted false + ::sql/columns [:media-id]})] + (:media-id fmobject))) + +(defn- get-sobject-content [id] (let [storage (::sto/storage *system*) - conn (::db/conn *system*) - fmobject (db/get conn :file-media-object {:id id}) - sobject (sto/get-object storage (:media-id fmobject))] - + sobject (sto/get-object storage id)] (with-open [stream (sto/get-object-data storage sobject)] (slurp stream)))) (defn- create-shapes-for-svg [{:keys [id] :as mobj} file-id objects frame-id position] - (let [svg-text (get-svg-content id) - svg-text (svgo/optimize *system* svg-text) - svg-data (-> (csvg/parse svg-text) - (assoc :name (:name mobj)) - (collect-and-persist-images file-id))] + (let [get-svg (fn [sid] + (let [svg-text (get-sobject-content sid) + svg-text (svgo/optimize *system* svg-text)] + (-> (csvg/parse svg-text) + (assoc :name (:name mobj))))) + + sid (resolve-sobject-id id) + + svg-data (if (cache/cache? *cache*) + (cache/get *cache* sid get-svg) + (get-svg sid)) + + svg-data (collect-and-persist-images file-id)] (sbuilder/create-svg-shapes svg-data position objects frame-id frame-id #{} false))) @@ -717,42 +724,34 @@ (defn- create-media-grid [fdata page-id frame-id grid media-group] - (let [process (fn [mobj position] - (let [position (gpt/add position (gpt/point grid-gap grid-gap)) - tp1 (dt/tpoint)] - (try - (process-media-object fdata page-id frame-id mobj position) - (catch Throwable cause - (l/wrn :hint "unable to process file media object (skiping)" - :file-id (str (:id fdata)) - :id (str (:id mobj)) - :cause cause) - (if-not *skip-on-error* - (throw cause) - nil)) - (finally - (l/trc :hint "graphic processed" - :file-id (str (:id fdata)) - :media-id (str (:id mobj)) - :elapsed (dt/format-duration (tp1)))))))] + (letfn [(process [fdata mobj position] + (let [position (gpt/add position (gpt/point grid-gap grid-gap)) + tp (dt/tpoint)] + (try + (let [changes (process-media-object fdata page-id frame-id mobj position)] + (cp/process-changes fdata changes false)) + (catch Throwable cause + (if *skip-on-graphic-error* + (l/wrn :hint "unable to process file media object (skiping)" + :file-id (str (:id fdata)) + :id (str (:id mobj)) + :cause cause) + (throw cause)) + nil) + (finally + (let [elapsed (tp)] + (l/trc :hint "graphic processed" + :file-id (str (:id fdata)) + :media-id (str (:id mobj)) + :elapsed (dt/format-duration elapsed)))))))] (->> (d/zip media-group grid) - (partition-all (or *max-procs* 1)) - (mapcat (fn [partition] - (->> partition - (map (fn [[mobj position]] - (sse/tap {:type :migration-progress - :section :graphics - :name (:name mobj)}) - (p/vthread (process mobj position)))) - (doall) - (map deref) - (doall)))) - (filter some?) - (reduce (fn [fdata changes] - (-> (assoc-in fdata [:options :components-v2] true) - (cp/process-changes changes false))) - fdata)))) + (reduce (fn [fdata [mobj position]] + (sse/tap {:type :migration-progress + :section :graphics + :name (:name mobj)}) + (or (process fdata mobj position) fdata)) + (assoc-in fdata [:options :components-v2] true))))) (defn- migrate-graphics [fdata] @@ -832,17 +831,12 @@ (decode-row))) (defn- validate-file! - [file libs throw-on-validate?] - (try - (cfv/validate-file! file libs) - (cfv/validate-file-schema! file) - (catch Throwable cause - (if throw-on-validate? - (throw cause) - (l/wrn :hint "migrate:file:validation-error" :cause cause))))) + [file libs] + (cfv/validate-file! file libs) + (cfv/validate-file-schema! file)) (defn- process-file - [{:keys [::db/conn] :as system} id & {:keys [validate? throw-on-validate?]}] + [{:keys [::db/conn] :as system} id & {:keys [validate?]}] (let [file (get-file system id) libs (->> (files/get-file-libraries conn id) @@ -855,7 +849,7 @@ (update :features conj "components/v2")) _ (when validate? - (validate-file! file libs throw-on-validate?)) + (validate-file! file libs)) file (if (contains? (:features file) "fdata/objects-map") (fdata/enable-objects-map file) @@ -901,10 +895,10 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn migrate-file! - [system file-id & {:keys [validate? throw-on-validate? max-procs]}] - (let [tpoint (dt/tpoint)] + [system file-id & {:keys [validate? skip-on-graphic-error?]}] + (let [tpoint (dt/tpoint)] (binding [*file-stats* (atom {}) - *max-procs* max-procs] + *skip-on-graphic-error* skip-on-graphic-error?] (try (l/dbg :hint "migrate:file:start" :file-id (str file-id)) @@ -913,9 +907,7 @@ (fn [system] (binding [*system* system] (fsnap/take-file-snapshot! system {:file-id file-id :label "migration/components-v2"}) - (process-file system file-id - :validate? validate? - :throw-on-validate? throw-on-validate?))))) + (process-file system file-id :validate? validate?))))) (finally (let [elapsed (tpoint) components (get @*file-stats* :processed/components 0) @@ -931,7 +923,7 @@ (some-> *team-stats* (swap! update :processed/files (fnil inc 0))))))))) (defn migrate-team! - [system team-id & {:keys [validate? throw-on-validate? max-procs]}] + [system team-id & {:keys [validate? skip-on-graphic-error?]}] (l/dbg :hint "migrate:team:start" :team-id (dm/str team-id)) @@ -941,9 +933,8 @@ migrate-file (fn [system file-id] (migrate-file! system file-id - :max-procs max-procs :validate? validate? - :throw-on-validate? throw-on-validate?)) + :skip-on-graphics-error? skip-on-graphic-error?)) migrate-team (fn [{:keys [::db/conn] :as system} {:keys [id features] :as team}] (let [features (-> features diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index c2f20015c3..7028be8bfe 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -411,8 +411,7 @@ ::migrations (ig/ref :app.migrations/migrations)} ::svgo/optimizer - {::wrk/executor (ig/ref ::wrk/executor) - ::svgo/max-procs (cf/get :svgo-max-procs)} + {} ::audit.tasks/archive {::props (ig/ref ::setup/props) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 81bb6e10a7..ebebbc42f8 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -664,9 +664,7 @@ (case feature "components/v2" (feat.compv2/migrate-file! options file-id - :max-procs 2 - :validate? validate? - :throw-on-validate? true) + :validate? validate?) "fdata/shape-data-type" nil diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index fec852f082..30b23dcea7 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -10,6 +10,7 @@ [app.common.pprint :as pp] [app.db :as db] [app.features.components-v2 :as feat] + [app.svgo :as svgo] [app.util.time :as dt] [cuerdas.core :as str] [promesa.core :as p] @@ -35,14 +36,10 @@ (fn [_ _ oldv newv] (when (not= (:processed/files oldv) (:processed/files newv)) - (let [total (:total/files newv) - completed (:processed/files newv) - progress (/ (* completed 100.0) total) + (let [completed (:processed/files newv) elapsed (tpoint)] (l/dbg :hint "progress" :completed (:processed/files newv) - :total (:total/files newv) - :progress (str (int progress) "%") :elapsed (dt/format-duration elapsed)))))) (defn- report-progress-teams @@ -50,21 +47,13 @@ (fn [_ _ oldv newv] (when (not= (:processed/teams oldv) (:processed/teams newv)) - (let [total (:total/teams newv) - completed (:processed/teams newv) - progress (/ (* completed 100.0) total) - progress (str (int progress) "%") + (let [completed (:processed/teams newv) elapsed (dt/format-duration (tpoint))] - (when (fn? on-progress) - (on-progress {:total total - :elapsed elapsed - :completed completed - :progress progress})) - + (on-progress {:elapsed elapsed + :completed completed})) (l/dbg :hint "progress" :completed completed - :progress progress :elapsed elapsed))))) (defn- get-total-files @@ -92,7 +81,6 @@ res (db/exec-one! pool [sql])] (:count res))) - (defn- mark-team-migration! [{:keys [::db/pool]} team-id] ;; We execute this out of transaction because we want this @@ -113,24 +101,68 @@ " WHERE id = ?")] (db/exec-one! pool [sql team-id]))) +;; (def ^:private sql:get-teams +;; "SELECT id, features +;; FROM team +;; WHERE deleted_at IS NULL +;; ORDER BY created_at DESC") + +;; (def ^:private sql:get-teams +;; "SELECT t.id, t.features, +;; (SELECT count(*) +;; FROM file_media_object AS fmo +;; JOIN file AS f ON (f.id = fmo.file_id) +;; JOIN project AS p ON (p.id = f.project_id) +;; WHERE p.team_id = t.id +;; AND fmo.mtype = 'image/svg+xml' +;; AND fmo.is_local = false) AS graphics +;; FROM team AS t +;; ORDER BY t.created_at DESC") + + (def ^:private sql:get-teams - "SELECT id, features - FROM team - WHERE deleted_at IS NULL - ORDER BY created_at ASC") + "WITH teams AS ( + SELECT t.id, t.features, + (SELECT count(*) + FROM file_media_object AS fmo + JOIN file AS f ON (f.id = fmo.file_id) + JOIN project AS p ON (p.id = f.project_id) + WHERE p.team_id = t.id + AND fmo.mtype = 'image/svg+xml' + AND fmo.is_local = false) AS graphics + FROM team AS t + ORDER BY 3 ASC + ) + SELECT * FROM teams ") + +(defn- read-pred + [[op val field]] + (let [field (name field)] + (case op + :lt [(str/ffmt "WHERE % < ?" field) val] + :lte [(str/ffmt "WHERE % <= ?" field) val] + :gt [(str/ffmt "WHERE % > ?" field) val] + :gte [(str/ffmt "WHERE % >= ?" field) val] + :eq [(str/ffmt "WHERE % = ?" field) val] + [""]))) (defn- get-teams - [conn] - (->> (db/cursor conn sql:get-teams) - (map feat/decode-row))) + [conn pred] + (let [[sql & params] (read-pred pred)] + (->> (db/cursor conn (apply vector (str sql:get-teams sql) params)) + (map feat/decode-row) + (remove (fn [{:keys [features]}] + (or (contains? features "ephimeral/v2-migration") + (contains? features "components/v2")))) + (map :id)))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PUBLIC API ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn migrate-file! - [system file-id & {:keys [rollback? max-procs] - :or {rollback? true}}] + [system file-id & {:keys [rollback?] :or {rollback? true}}] (l/dbg :hint "migrate:start" :rollback rollback?) (let [tpoint (dt/tpoint) @@ -140,7 +172,7 @@ (binding [feat/*stats* (atom {})] (try (-> (assoc system ::db/rollback rollback?) - (feat/migrate-file! file-id :max-procs max-procs)) + (feat/migrate-file! file-id)) (-> (deref feat/*stats*) (assoc :elapsed (dt/format-duration (tpoint)))) @@ -153,11 +185,11 @@ (l/dbg :hint "migrate:end" :rollback rollback? :elapsed elapsed))))))) (defn migrate-team! - [{:keys [::db/pool] :as system} team-id & {:keys [rollback? skip-on-error validate? max-procs] + [{:keys [::db/pool] :as system} team-id & {:keys [rollback? skip-on-graphic-error? validate? skip-mark?] :or {rollback? true - skip-on-error true - validate? false - max-procs 1} + validate? true + skip-on-graphic-error? false + skip-mark? false} :as opts}] (l/dbg :hint "migrate:start" :rollback rollback?) @@ -165,34 +197,30 @@ (let [team-id (if (string? team-id) (parse-uuid team-id) team-id) - total (get-total-files pool :team-id team-id) - stats (atom {:total/files total}) + stats (atom {}) tpoint (dt/tpoint)] (add-watch stats :progress-report (report-progress-files tpoint)) - (binding [feat/*stats* stats - feat/*skip-on-error* skip-on-error] - + (binding [feat/*stats* stats] (try - (mark-team-migration! system team-id) + (when-not skip-mark? + (mark-team-migration! system team-id)) (-> (assoc system ::db/rollback rollback?) (feat/migrate-team! team-id - :max-procs max-procs :validate? validate? - :throw-on-validate? (not skip-on-error))) - + :skip-on-graphics-error? skip-on-graphic-error?)) (print-stats! (-> (deref feat/*stats*) - (dissoc :total/files) (assoc :elapsed (dt/format-duration (tpoint))))) (catch Throwable cause (l/dbg :hint "migrate:error" :cause cause)) (finally - (unmark-team-migration! system team-id) + (when-not skip-mark? + (unmark-team-migration! system team-id)) (let [elapsed (dt/format-duration (tpoint))] (l/dbg :hint "migrate:end" :rollback rollback? :elapsed elapsed))))))) @@ -202,100 +230,78 @@ This function starts multiple concurrent team migration processes until thw maximum number of jobs is reached which by default has the - value of `1`. This is controled with the `:max-jobs` option. + value of `1`. This is controled with the `:max-jobs` option." - Each tram migration process also can start multiple procs for - graphics migration, the total of that procs is controled with the - `:max-procs` option. - - Internally, the graphics migration process uses SVGO module which by - default has a limited number of maximum concurent - operations (globally), ensure setting up correct number with - PENPOT_SVGO_MAX_PROCS environment variable." - - [{:keys [::db/pool] :as system} & {:keys [max-jobs max-procs max-items + [{:keys [::db/pool] :as system} & {:keys [max-jobs max-items max-time rollback? validate? preset - skip-on-error max-time + pred max-procs skip-mark? on-start on-progress on-error on-end] - :or {validate? false + :or {validate? true rollback? true - skip-on-error true preset :shutdown-on-failure + skip-mark? true max-jobs 1 - max-procs 10 max-items Long/MAX_VALUE} :as opts}] - (let [total (get-total-teams pool) - stats (atom {:total/teams (min total max-items)}) + (let [stats (atom {}) + tpoint (dt/tpoint) + mtime (some-> max-time dt/duration) - tpoint (dt/tpoint) - mtime (some-> max-time dt/duration) - - scope (px/structured-task-scope :preset preset :factory :virtual) - sjobs (ps/create :permits max-jobs) + factory (px/thread-factory :virtual false :prefix "penpot/migration/compv2/") + executor (px/cached-executor :factory factory) + max-procs (or max-procs max-jobs) + sjobs (ps/create :permits max-jobs) + sprocs (ps/create :permits max-procs) migrate-team - (fn [{:keys [id features] :as team}] + (fn [team-id] (ps/acquire! sjobs) (let [ts (tpoint)] - (cond - (and mtime (neg? (compare mtime ts))) + (if (and mtime (neg? (compare mtime ts))) (do (l/inf :hint "max time constraint reached" - :team-id (str id) + :team-id (str team-id) :elapsed (dt/format-duration ts)) (ps/release! sjobs) (reduced nil)) - (or (contains? features "ephimeral/v2-migration") - (contains? features "components/v2")) - (do - (l/dbg :hint "skip team" :team-id (str id)) - (ps/release! sjobs)) - - :else - (px/submit! scope (fn [] + (px/run! executor (fn [] (try - (mark-team-migration! system id) + (when-not skip-mark? + (mark-team-migration! system team-id)) (-> (assoc system ::db/rollback rollback?) - (feat/migrate-team! id - :max-procs max-procs - :validate? validate? - :throw-on-validate? (not skip-on-error))) + (feat/migrate-team! team-id :validate? validate?)) (catch Throwable cause - (l/err :hint "unexpected error on processing team" - :team-id (str id) + (l/err :hint "unexpected error on processing team (skiping)" + :team-id (str team-id) :cause cause)) (finally (ps/release! sjobs) - (unmark-team-migration! system id))))))))] + (when-not skip-mark? + (unmark-team-migration! system team-id)))))))))] (l/dbg :hint "migrate:start" :rollback rollback? - :total total :max-jobs max-jobs - :max-procs max-procs :max-items max-items) (add-watch stats :progress-report (report-progress-teams tpoint on-progress)) (binding [feat/*stats* stats - feat/*skip-on-error* skip-on-error] + svgo/*semaphore* sprocs] (try (when (fn? on-start) - (on-start {:total total :rollback rollback?})) + (on-start {:rollback rollback?})) (db/tx-run! system (fn [{:keys [::db/conn]}] (run! (partial migrate-team) - (->> (get-teams conn) + (->> (get-teams conn pred) (take max-items))))) - (try - (p/await! scope) - (finally - (pu/close! scope))) + ;; Close and await tasks + (pu/close! executor) (if (fn? on-end) (-> (deref stats) diff --git a/backend/src/app/svgo.clj b/backend/src/app/svgo.clj index 70d7c6b2b3..a846fa7680 100644 --- a/backend/src/app/svgo.clj +++ b/backend/src/app/svgo.clj @@ -7,16 +7,10 @@ (ns app.svgo "A SVG Optimizer service" (:require - [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.jsrt :as jsrt] [app.common.logging :as l] - [app.common.spec :as us] [app.worker :as-alias wrk] - [clojure.spec.alpha :as s] [integrant.core :as ig] - [promesa.exec :as px] - [promesa.exec.bulkhead :as bh] [promesa.exec.semaphore :as ps] [promesa.util :as pu])) @@ -26,40 +20,23 @@ nil) (defn optimize - [system data] - (dm/assert! "expect data to be a string" (string? data)) - - (letfn [(optimize-fn [pool] - (jsrt/run! pool - (fn [context] - (jsrt/set! context "svgData" data) - (jsrt/eval! context "penpotSvgo.optimize(svgData, {plugins: ['safeAndFastPreset']})"))))] - (try - (some-> *semaphore* ps/acquire!) - (let [{:keys [::jsrt/pool ::wrk/executor]} (::optimizer system)] - (dm/assert! "expect optimizer instance" (jsrt/pool? pool)) - (px/invoke! executor (partial optimize-fn pool))) - (finally - (some-> *semaphore* ps/release!))))) - -(s/def ::max-procs (s/nilable ::us/integer)) - -(defmethod ig/pre-init-spec ::optimizer [_] - (s/keys :req [::wrk/executor ::max-procs])) - -(defmethod ig/prep-key ::optimizer - [_ cfg] - (merge {::max-procs 20} (d/without-nils cfg))) + [{pool ::optimizer} data] + (try + (some-> *semaphore* ps/acquire!) + (jsrt/run! pool + (fn [context] + (jsrt/set! context "svgData" data) + (jsrt/eval! context "penpotSvgo.optimize(svgData, {plugins: ['safeAndFastPreset']})"))) + (finally + (some-> *semaphore* ps/release!)))) (defmethod ig/init-key ::optimizer - [_ {:keys [::wrk/executor ::max-procs]}] - (l/inf :hint "initializing svg optimizer pool" :max-procs max-procs) - (let [init (jsrt/resource->source "app/common/svg/optimizer.js") - executor (bh/create :type :executor :executor executor :permits max-procs)] - {::jsrt/pool (jsrt/pool :init init) - ::wrk/executor executor})) + [_ _] + (l/inf :hint "initializing svg optimizer pool") + (let [init (jsrt/resource->source "app/common/svg/optimizer.js")] + (jsrt/pool :init init))) (defmethod ig/halt-key! ::optimizer - [_ {:keys [::jsrt/pool]}] + [_ pool] (l/info :hint "stopping svg optimizer pool") (pu/close! pool)) From 161a55e16639b4c9914ba8115d1636fb22cd2ab4 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 23:40:15 +0100 Subject: [PATCH 04/54] :zap: Optimize general case of without-nils Performance gains up to x6 --- common/src/app/common/data.cljc | 11 +++++++++-- common/src/app/common/types/shape.cljc | 7 ++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index d814b14392..c01cdb3d65 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -216,12 +216,19 @@ [coll] (into [] (remove nil?) coll)) + (defn without-nils "Given a map, return a map removing key-value pairs when value is `nil`." - ([] (remove (comp nil? val))) + ([] + (remove (comp nil? val))) ([data] - (into {} (without-nils) data))) + (reduce-kv (fn [data k v] + (if (nil? v) + (dissoc data k) + data)) + data + data))) (defn without-qualified ([] diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index 28289219d4..2397d58458 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -491,7 +491,12 @@ the shape. The props must have :x :y :width :height." [{:keys [type] :as props}] (let [shape (make-minimal-shape type) - shape (merge shape (d/without-nils props)) + + ;; The props can be custom records that does not + ;; work properly with without-nils, so we first make + ;; it plain map for proceed + props (d/without-nils (into {} props)) + shape (merge shape (d/without-nils (into {} props))) shape (case (:type shape) (:bool :path) (setup-path shape) :image (-> shape setup-rect setup-image) From 33ad2d94fbde2c0b039289d15756b85b4c073ff0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 16 Jan 2024 16:59:08 +0100 Subject: [PATCH 05/54] :bug: Add proper default to cx and cy when parsing svg circle elements --- common/src/app/common/svg/shapes_builder.cljc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/src/app/common/svg/shapes_builder.cljc b/common/src/app/common/svg/shapes_builder.cljc index fc4b370ae7..6313939879 100644 --- a/common/src/app/common/svg/shapes_builder.cljc +++ b/common/src/app/common/svg/shapes_builder.cljc @@ -303,6 +303,11 @@ rx (d/nilv r rx) ry (d/nilv r ry) + + ;; There are some svg circles in the internet that does not + ;; have cx and cy attrs, so we default them to 0 + cx (d/nilv cx 0) + cy (d/nilv cy 0) origin (gpt/negate (gpt/point svg-data)) rect (grc/make-rect From b5829982283dfd8700cea7e0474008c1e7ce5400 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 16 Jan 2024 19:18:08 +0100 Subject: [PATCH 06/54] :bug: Add migration for fix bool shapes which does not have :bool-content attr --- common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 65 +++++++++++++-------- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 12e18921eb..8c34b89a41 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 38) +(def version 39) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index c5952d5718..a7ac391518 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -319,18 +319,20 @@ (dissoc :fill-color :fill-opacity)))) (update-container [{:keys [objects] :as container}] - (loop [objects objects - shapes (->> (vals objects) - (filter cfh/image-shape?))] - (if-let [shape (first shapes)] - (let [{:keys [id frame-id] :as shape'} (process-shape shape)] - (if (identical? shape shape') - (recur objects (rest shapes)) - (recur (-> objects - (assoc id shape') - (d/update-when frame-id dissoc :thumbnail)) - (rest shapes)))) - (assoc container :objects objects))))] + (if (contains? container :objects) + (loop [objects (:objects container) + shapes (->> (vals objects) + (filter cfh/image-shape?))] + (if-let [shape (first shapes)] + (let [{:keys [id frame-id] :as shape'} (process-shape shape)] + (if (identical? shape shape') + (recur objects (rest shapes)) + (recur (-> objects + (assoc id shape') + (d/update-when frame-id dissoc :thumbnail)) + (rest shapes)))) + (assoc container :objects objects))) + container))] (-> data (update :pages-index update-vals update-container) @@ -380,7 +382,7 @@ (assign-fills))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -409,7 +411,7 @@ (assoc :fills []))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -424,7 +426,7 @@ (dissoc :position-data))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -440,7 +442,7 @@ (dissoc :position-data))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -527,7 +529,7 @@ (assoc object :frame-id calculated-frame-id))) (update-container [container] - (update container :objects #(update-vals % (partial update-object %))))] + (d/update-when container :objects #(update-vals % (partial update-object %))))] (-> data (update :pages-index update-vals update-container) @@ -565,7 +567,7 @@ (update :content #(txt/transform-nodes invalid-node? fix-node %))))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -580,7 +582,7 @@ object)) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) @@ -613,7 +615,8 @@ object))) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] + (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) @@ -630,7 +633,7 @@ object)) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container)))) @@ -642,7 +645,7 @@ (dissoc object :x :y :width :height) object)) (update-container [container] - (update container :objects update-vals update-object))] + (d/update-when container :objects update-vals update-object))] (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) @@ -694,7 +697,23 @@ shape))) (update-container [container] - (update container :objects update-vals update-shape))] + (d/update-when container :objects update-vals update-shape))] + + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) + + +(defmethod migrate 39 + [data] + (letfn [(update-shape [shape] + (if (and (cfh/bool-shape? shape) + (not (contains? shape :bool-content))) + (assoc shape :bool-content []) + shape)) + + (update-container [container] + (d/update-when container :objects update-vals update-shape))] (-> data (update :pages-index update-vals update-container) From 9b59b9246447041a4046994a69e1168d963bec0c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 00:22:38 +0100 Subject: [PATCH 07/54] :bug: Improve not-found error report on s3 storage backend --- backend/src/app/storage/s3.clj | 23 ++++++++++-------- common/deps.edn | 2 +- common/src/app/common/exceptions.cljc | 35 ++++++++++++++++++++++----- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/backend/src/app/storage/s3.clj b/backend/src/app/storage/s3.clj index 92d59988b4..1bbb38b16a 100644 --- a/backend/src/app/storage/s3.clj +++ b/backend/src/app/storage/s3.clj @@ -51,6 +51,7 @@ software.amazon.awssdk.services.s3.model.DeleteObjectsRequest software.amazon.awssdk.services.s3.model.DeleteObjectsResponse software.amazon.awssdk.services.s3.model.GetObjectRequest + software.amazon.awssdk.services.s3.model.NoSuchKeyException software.amazon.awssdk.services.s3.model.ObjectIdentifier software.amazon.awssdk.services.s3.model.PutObjectRequest software.amazon.awssdk.services.s3.model.S3Error @@ -126,17 +127,19 @@ (defmethod impl/get-object-data :s3 [backend object] (us/assert! ::backend backend) - (letfn [(no-such-key? [cause] - (instance? software.amazon.awssdk.services.s3.model.NoSuchKeyException cause)) - (handle-not-found [cause] - (ex/raise :type :not-found - :code :object-not-found - :hint "s3 object not found" - :cause cause))] - (-> (get-object-data backend object) - (p/catch no-such-key? handle-not-found) - (p/await!)))) + (let [result (p/await (get-object-data backend object))] + (if (ex/exception? result) + (cond + (ex/instance? NoSuchKeyException result) + (ex/raise :type :not-found + :code :object-not-found + :hint "s3 object not found" + :cause result) + :else + (throw result)) + + result))) (defmethod impl/get-object-bytes :s3 [backend object] diff --git a/common/deps.edn b/common/deps.edn index f8c649fab6..61c52e9099 100644 --- a/common/deps.edn +++ b/common/deps.edn @@ -32,7 +32,7 @@ funcool/tubax {:mvn/version "2021.05.20-0"} funcool/cuerdas {:mvn/version "2023.11.09-407"} - funcool/promesa {:git/sha "484b7f5c0d08d817746caa685ed9ac5583eb37fa" + funcool/promesa {:git/sha "0c5ed6ad033515a2df4b55addea044f60e9653d0" :git/url "https://github.com/funcool/promesa"} funcool/datoteka {:mvn/version "3.0.66" diff --git a/common/src/app/common/exceptions.cljc b/common/src/app/common/exceptions.cljc index b01914ca56..2070986fe3 100644 --- a/common/src/app/common/exceptions.cljc +++ b/common/src/app/common/exceptions.cljc @@ -7,10 +7,12 @@ (ns app.common.exceptions "A helpers for work with exceptions." #?(:cljs (:require-macros [app.common.exceptions])) + (:refer-clojure :exclude [instance?]) (:require #?(:clj [clojure.stacktrace :as strace]) [app.common.pprint :as pp] [app.common.schema :as sm] + [clojure.core :as c] [clojure.spec.alpha :as s] [cuerdas.core :as str] [expound.alpha :as expound]) @@ -20,6 +22,9 @@ #?(:clj (set! *warn-on-reflection* true)) +(def ^:dynamic *data-length* 8) +(def ^:dynamic *data-level* 8) + (defmacro error [& {:keys [type hint] :as params}] `(ex-info ~(or hint (name type)) @@ -49,20 +54,38 @@ (defn ex-info? [v] - (instance? #?(:clj clojure.lang.IExceptionInfo :cljs cljs.core.ExceptionInfo) v)) + (c/instance? #?(:clj clojure.lang.IExceptionInfo :cljs cljs.core.ExceptionInfo) v)) (defn error? [v] - (instance? #?(:clj clojure.lang.IExceptionInfo :cljs cljs.core.ExceptionInfo) v)) + (c/instance? #?(:clj clojure.lang.IExceptionInfo :cljs cljs.core.ExceptionInfo) v)) (defn exception? [v] - (instance? #?(:clj java.lang.Throwable :cljs js/Error) v)) + (c/instance? #?(:clj java.lang.Throwable :cljs js/Error) v)) #?(:clj (defn runtime-exception? [v] - (instance? RuntimeException v))) + (c/instance? RuntimeException v))) + +#?(:clj + (defn instance? + [class cause] + (loop [cause cause] + (if (c/instance? class cause) + true + (if-let [cause (ex-cause cause)] + (recur cause) + false))))) + +;; NOTE: idea for a macro for error handling +;; (pu/try-let [cause (p/await (get-object-data backend object))] +;; (ex/instance? NoSuchKeyException cause) +;; (ex/raise :type :not-found +;; :code :object-not-found +;; :hint "s3 object not found" +;; :cause cause)) (defn explain [data & {:as opts}] @@ -91,8 +114,8 @@ data? true explain? true chain? true - data-length 8 - data-level 5}}] + data-length *data-length* + data-level *data-level*}}] (letfn [(print-trace-element [^StackTraceElement e] (let [class (.getClassName e) From f9d63dba00e6e7e0ab667644df5cf41c0e3b71e5 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 16 Jan 2024 19:39:36 +0100 Subject: [PATCH 08/54] :bug: Fix incorrect assumption about parseFloat on fixing percent on parsing and normalizing svg elements --- common/src/app/common/svg.cljc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/src/app/common/svg.cljc b/common/src/app/common/svg.cljc index 6bca95abab..b91e4726f2 100644 --- a/common/src/app/common/svg.cljc +++ b/common/src/app/common/svg.cljc @@ -964,8 +964,7 @@ is-other? #{:r :stroke-width}] (if is-percent? - ;; JS parseFloat removes the % symbol - (let [attr-num (d/parse-double attr-val)] + (let [attr-num (d/parse-double (str/rtrim attr-val "%"))] (str (cond (is-x? attr-key) (fix-coord :x :width attr-num) (is-y? attr-key) (fix-coord :y :height attr-num) From c58302ffc4e50bb446c359acec30af7cba1374c0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 10:00:31 +0100 Subject: [PATCH 09/54] :fire: Remove unnecessary `do` on file validation ns --- common/src/app/common/files/validate.cljc | 105 +++++++++++----------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 514e48ab16..76dea4e749 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -367,64 +367,63 @@ [shape-id file page libraries & {:keys [context] :or {context :not-component}}] (let [shape (ctst/get-shape page shape-id)] (when (some? shape) - (do - (check-geometry shape file page) - (check-parent-children shape file page) - (check-frame shape file page) + (check-geometry shape file page) + (check-parent-children shape file page) + (check-frame shape file page) - (if (ctk/instance-head? shape) - (if (not= :frame (:type shape)) - (report-error :instance-head-not-frame - "Instance head should be a frame" + (if (ctk/instance-head? shape) + (if (not= :frame (:type shape)) + (report-error :instance-head-not-frame + "Instance head should be a frame" + shape file page) + + (if (ctk/instance-root? shape) + (if (ctk/main-instance? shape) + (if (not= context :not-component) + (report-error :root-main-not-allowed + "Root main component not allowed inside other component" + shape file page) + (check-shape-main-root-top shape file page libraries)) + + (if (not= context :not-component) + (report-error :root-copy-not-allowed + "Root copy component not allowed inside other component" + shape file page) + (check-shape-copy-root-top shape file page libraries))) + + (if (ctk/main-instance? shape) + ;; mains can't be nested into mains + (if (or (= context :not-component) (= context :main-top)) + (report-error :nested-main-not-allowed + "Nested main component only allowed inside other component" + shape file page) + (check-shape-main-root-nested shape file page libraries)) + + (if (= context :not-component) + (report-error :nested-copy-not-allowed + "Nested copy component only allowed inside other component" + shape file page) + (check-shape-copy-root-nested shape file page libraries))))) + + (if (ctk/in-component-copy? shape) + (if-not (#{:copy-top :copy-nested :copy-any} context) + (report-error :not-head-copy-not-allowed + "Non-root copy only allowed inside a copy" shape file page) + (check-shape-copy-not-root shape file page libraries)) - (if (ctk/instance-root? shape) - (if (ctk/main-instance? shape) - (if (not= context :not-component) - (report-error :root-main-not-allowed - "Root main component not allowed inside other component" - shape file page) - (check-shape-main-root-top shape file page libraries)) - - (if (not= context :not-component) - (report-error :root-copy-not-allowed - "Root copy component not allowed inside other component" - shape file page) - (check-shape-copy-root-top shape file page libraries))) - - (if (ctk/main-instance? shape) - ;; mains can't be nested into mains - (if (or (= context :not-component) (= context :main-top)) - (report-error :nested-main-not-allowed - "Nested main component only allowed inside other component" - shape file page) - (check-shape-main-root-nested shape file page libraries)) - - (if (= context :not-component) - (report-error :nested-copy-not-allowed - "Nested copy component only allowed inside other component" - shape file page) - (check-shape-copy-root-nested shape file page libraries))))) - - (if (ctk/in-component-copy? shape) - (if-not (#{:copy-top :copy-nested :copy-any} context) - (report-error :not-head-copy-not-allowed - "Non-root copy only allowed inside a copy" + (if (ctn/inside-component-main? (:objects page) shape) + (if-not (#{:main-top :main-nested :main-any} context) + (report-error :not-head-main-not-allowed + "Non-root main only allowed inside a main component" shape file page) - (check-shape-copy-not-root shape file page libraries)) + (check-shape-main-not-root shape file page libraries)) - (if (ctn/inside-component-main? (:objects page) shape) - (if-not (#{:main-top :main-nested :main-any} context) - (report-error :not-head-main-not-allowed - "Non-root main only allowed inside a main component" - shape file page) - (check-shape-main-not-root shape file page libraries)) - - (if (#{:main-top :main-nested :main-any} context) - (report-error :not-component-not-allowed - "Not compoments are not allowed inside a main" - shape file page) - (check-shape-not-component shape file page libraries))))))))) + (if (#{:main-top :main-nested :main-any} context) + (report-error :not-component-not-allowed + "Not compoments are not allowed inside a main" + shape file page) + (check-shape-not-component shape file page libraries)))))))) (defn- check-component "Validate semantic coherence of a component. Report all errors found." From 997441eff3ce7babdc27d2fcba7cac5c8cf82d72 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 00:23:15 +0100 Subject: [PATCH 10/54] :paperclip: Fix typo on validation log message --- common/src/app/common/files/validate.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 76dea4e749..1e62d14ec3 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -105,7 +105,7 @@ (nil? (:selrect shape)) (nil? (:points shape)))) (report-error :invalid-geometry - "Shape greometry is invalid" + "Shape geometry is invalid" shape file page))) (defn- check-parent-children From f73ce6572c79f26b7578be962f0c022102efd74e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 16:41:02 +0100 Subject: [PATCH 11/54] :sparkles: Improve rollback handlong on db ns --- backend/src/app/db.clj | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index 942d01db7d..6e7407e17d 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -517,9 +517,11 @@ (defn rollback! ([conn] - (let [^Connection conn (get-connection conn)] - (l/trc :hint "explicit rollback requested") - (.rollback conn))) + (if (and (map? conn) (::savepoint conn)) + (rollback! conn (::savepoint conn)) + (let [^Connection conn (get-connection conn)] + (l/trc :hint "explicit rollback requested") + (.rollback conn)))) ([conn ^Savepoint sp] (let [^Connection conn (get-connection conn)] (l/trc :hint "explicit rollback requested (savepoint)") @@ -538,8 +540,13 @@ (let [conn (::conn system) sp (savepoint conn)] (try - (let [result (apply f system params)] - (release! conn sp) + (let [system' (-> system + (assoc ::savepoint sp) + (dissoc ::rollback)) + result (apply f system' params)] + (if (::rollback system) + (rollback! conn sp) + (release! conn sp)) result) (catch Throwable cause (.rollback ^Connection conn ^Savepoint sp) @@ -547,8 +554,10 @@ (::pool system) (with-atomic [conn (::pool system)] - (let [system (assoc system ::conn conn) - result (apply f system params)] + (let [system' (-> system + (assoc ::conn conn) + (dissoc ::rollback)) + result (apply f system' params)] (when (::rollback system) (rollback! conn)) result)) From 02d820855371f482333e043f011f782d2c88594b Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 16:34:41 +0100 Subject: [PATCH 12/54] :paperclip: Add temporal repl and log4j config --- backend/resources/log4j2-experiments.xml | 71 ++++++++++++++++++++++++ backend/scripts/repl-test | 49 ++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 backend/resources/log4j2-experiments.xml create mode 100755 backend/scripts/repl-test diff --git a/backend/resources/log4j2-experiments.xml b/backend/resources/log4j2-experiments.xml new file mode 100644 index 0000000000..88542c2774 --- /dev/null +++ b/backend/resources/log4j2-experiments.xml @@ -0,0 +1,71 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/backend/scripts/repl-test b/backend/scripts/repl-test new file mode 100755 index 0000000000..a1333a5317 --- /dev/null +++ b/backend/scripts/repl-test @@ -0,0 +1,49 @@ +#!/usr/bin/env bash + +source /home/penpot/backend/environ +export PENPOT_FLAGS="$PENPOT_FLAGS disable-backend-worker" + +export OPTIONS=" + -A:jmx-remote -A:dev \ + -J-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager \ + -J-Djdk.attach.allowAttachSelf \ + -J-Dlog4j2.configurationFile=log4j2-experiments.xml \ + -J-XX:-OmitStackTraceInFastThrow \ + -J-XX:+UnlockDiagnosticVMOptions \ + -J-XX:+DebugNonSafepoints \ + -J-Djdk.tracePinnedThreads=full \ + -J-Dpolyglot.engine.WarnInterpreterOnly=false \ + -J--enable-preview"; + +# Setup HEAP +#export OPTIONS="$OPTIONS -J-Xms900m -J-Xmx900m -J-XX:+AlwaysPreTouch" +export OPTIONS="$OPTIONS -J-Xms1g -J-Xmx25g" +#export OPTIONS="$OPTIONS -J-Xms900m -J-Xmx900m -J-XX:+AlwaysPreTouch" + +export PENPOT_HTTP_SERVER_IO_THREADS=2 +export PENPOT_HTTP_SERVER_WORKER_THREADS=2 + +# Increase virtual thread pool size +# export OPTIONS="$OPTIONS -J-Djdk.virtualThreadScheduler.parallelism=16" + +# Disable C2 Compiler +# export OPTIONS="$OPTIONS -J-XX:TieredStopAtLevel=1" + +# Disable all compilers +# export OPTIONS="$OPTIONS -J-Xint" + +# Setup GC +export OPTIONS="$OPTIONS -J-XX:+UseG1GC -J-Xlog:gc:logs/gc.log" + + +# Setup GC +#export OPTIONS="$OPTIONS -J-XX:+UseZGC -J-XX:+ZGenerational -J-Xlog:gc:gc.log" + +# Enable ImageMagick v7.x support +# export OPTIONS="-J-Dim4java.useV7=true $OPTIONS"; + +export OPTIONS_EVAL="nil" +# export OPTIONS_EVAL="(set! *warn-on-reflection* true)" + +set -ex +exec clojure $OPTIONS -M -e "$OPTIONS_EVAL" -m rebel-readline.main \ No newline at end of file From 0a5e15b9162a55d855f0c4e3aff6801718c23a13 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sun, 14 Jan 2024 21:08:39 +0100 Subject: [PATCH 13/54] :recycle: Simplify components-v2 migration functions impl --- backend/src/app/features/components_v2.clj | 169 ++++++++++++------ backend/src/app/srepl/components_v2.clj | 191 ++++++++------------- 2 files changed, 192 insertions(+), 168 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index b9cf8a79a9..21a47d05f5 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -20,6 +20,7 @@ [app.common.geom.rect :as grc] [app.common.geom.shapes :as gsh] [app.common.logging :as l] + [app.common.math :as mth] [app.common.svg :as csvg] [app.common.svg.shapes-builder :as sbuilder] [app.common.types.component :as ctk] @@ -48,7 +49,8 @@ [buddy.core.codecs :as bc] [cuerdas.core :as str] [datoteka.io :as io] - [promesa.core :as p])) + [promesa.exec :as px] + [promesa.util :as pu])) (def ^:dynamic *stats* "A dynamic var for setting up state for collect stats globally." @@ -67,6 +69,10 @@ internal functions without the need to explicitly pass it top down." nil) +(def ^:dynamic ^:private *team-id* + "A dynamic var that holds the current processing team-id." + nil) + (def ^:dynamic ^:private *file-stats* "An internal dynamic var for collect stats by file." nil) @@ -575,32 +581,40 @@ (if (> ext-idx 0) (subs filename 0 ext-idx) filename))) (defn- collect-and-persist-images - [svg-data file-id] + [svg-data file-id media-id] (letfn [(process-image [{:keys [href] :as item}] - (let [item (if (str/starts-with? href "data:") - (let [[mtype data] (parse-datauri href) - size (alength data) - path (tmp/tempfile :prefix "penpot.media.download.") - written (io/write-to-file! data path :size size)] + (try + (let [item (if (str/starts-with? href "data:") + (let [[mtype data] (parse-datauri href) + size (alength data) + path (tmp/tempfile :prefix "penpot.media.download.") + written (io/write-to-file! data path :size size)] - (when (not= written size) - (ex/raise :type :internal - :code :mismatch-write-size - :hint "unexpected state: unable to write to file")) + (when (not= written size) + (ex/raise :type :internal + :code :mismatch-write-size + :hint "unexpected state: unable to write to file")) - (-> item - (assoc :size size) - (assoc :path path) - (assoc :filename "tempfile") - (assoc :mtype mtype))) + (-> item + (assoc :size size) + (assoc :path path) + (assoc :filename "tempfile") + (assoc :mtype mtype))) - (let [result (cmd.media/download-image *system* href)] - (-> (merge item result) - (assoc :name (extract-name href)))))] + (let [result (cmd.media/download-image *system* href)] + (-> (merge item result) + (assoc :name (extract-name href)))))] - ;; The media processing adds the data to the - ;; input map and returns it. - (media/run {:cmd :info :input item}))) + ;; The media processing adds the data to the + ;; input map and returns it. + (media/run {:cmd :info :input item})) + (catch Throwable _ + (let [team-id *team-id*] + (l/wrn :hint "unable to process embedded images on svg file" + :team-id (str team-id) + :file-id (str file-id) + :media-id (str media-id))) + nil))) (persist-image [acc {:keys [path size width height mtype href] :as item}] (let [storage (::sto/storage *system*) @@ -639,9 +653,7 @@ (defn- resolve-sobject-id [id] (let [fmobject (db/get *system* :file-media-object {:id id} - {::db/check-deleted false - ::db/remove-deleted false - ::sql/columns [:media-id]})] + {::sql/columns [:media-id]})] (:media-id fmobject))) (defn- get-sobject-content @@ -660,12 +672,11 @@ (assoc :name (:name mobj))))) sid (resolve-sobject-id id) - svg-data (if (cache/cache? *cache*) - (cache/get *cache* sid get-svg) + (cache/get *cache* sid (px/wrap-bindings get-svg)) (get-svg sid)) - svg-data (collect-and-persist-images file-id)] + svg-data (collect-and-persist-images svg-data file-id id)] (sbuilder/create-svg-shapes svg-data position objects frame-id frame-id #{} false))) @@ -724,25 +735,49 @@ (defn- create-media-grid [fdata page-id frame-id grid media-group] - (letfn [(process [fdata mobj position] + (letfn [(process-media-object [fdata mobj position] (let [position (gpt/add position (gpt/point grid-gap grid-gap)) - tp (dt/tpoint)] + tp (dt/tpoint) + err (volatile! false)] (try (let [changes (process-media-object fdata page-id frame-id mobj position)] (cp/process-changes fdata changes false)) + (catch Throwable cause - (if *skip-on-graphic-error* - (l/wrn :hint "unable to process file media object (skiping)" - :file-id (str (:id fdata)) - :id (str (:id mobj)) - :cause cause) - (throw cause)) - nil) + (vreset! err true) + (let [cause (pu/unwrap-exception cause) + edata (ex-data cause) + team-id *team-id*] + (cond + (instance? org.xml.sax.SAXParseException cause) + (l/inf :hint "skip processing media object: invalid svg found" + :team-id (str team-id) + :file-id (str (:id fdata)) + :id (str (:id mobj))) + + (= (:type edata) :not-found) + (l/inf :hint "skip processing media object: underlying object does not exist" + :team-id (str team-id) + :file-id (str (:id fdata)) + :id (str (:id mobj))) + + :else + (let [skip? *skip-on-graphic-error*] + (l/wrn :hint "unable to process file media object" + :skiped skip? + :team-id (str team-id) + :file-id (str (:id fdata)) + :id (str (:id mobj)) + :cause cause) + (when-not skip? + (throw cause)))) + nil)) (finally (let [elapsed (tp)] (l/trc :hint "graphic processed" :file-id (str (:id fdata)) :media-id (str (:id mobj)) + :error @err :elapsed (dt/format-duration elapsed)))))))] (->> (d/zip media-group grid) @@ -750,7 +785,8 @@ (sse/tap {:type :migration-progress :section :graphics :name (:name mobj)}) - (or (process fdata mobj position) fdata)) + (or (process-media-object fdata mobj position) + fdata)) (assoc-in fdata [:options :components-v2] true))))) (defn- migrate-graphics @@ -822,7 +858,6 @@ (update :data fdata/process-pointers deref) (fmg/migrate-file)))) - (defn- get-team [system team-id] (-> (db/get system :team {:id team-id} @@ -870,12 +905,13 @@ (dissoc file :data))) - (def ^:private sql:get-and-lock-team-files "SELECT f.id FROM file AS f JOIN project AS p ON (p.id = f.project_id) WHERE p.team_id = ? + AND p.deleted_at IS NULL + AND f.deleted_at IS NULL FOR UPDATE") (defn- get-and-lock-files @@ -900,14 +936,26 @@ (binding [*file-stats* (atom {}) *skip-on-graphic-error* skip-on-graphic-error?] (try - (l/dbg :hint "migrate:file:start" :file-id (str file-id)) + (l/dbg :hint "migrate:file:start" + :file-id (str file-id) + :validate validate? + :skip-on-graphics-error skip-on-graphic-error?) (let [system (update system ::sto/storage media/configure-assets-storage)] (db/tx-run! system (fn [system] - (binding [*system* system] - (fsnap/take-file-snapshot! system {:file-id file-id :label "migration/components-v2"}) - (process-file system file-id :validate? validate?))))) + (try + (binding [*system* system] + (fsnap/take-file-snapshot! system {:file-id file-id :label "migration/components-v2"}) + (process-file system file-id :validate? validate?)) + + (catch Throwable cause + (let [team-id *team-id*] + (l/wrn :hint "error on processing file" + :team-id (str team-id) + :file-id (str file-id)) + (throw cause))))))) + (finally (let [elapsed (tpoint) components (get @*file-stats* :processed/components 0) @@ -917,6 +965,7 @@ :file-id (str file-id) :graphics graphics :components components + :validate validate? :elapsed (dt/format-duration elapsed)) (some-> *stats* (swap! update :processed/files (fnil inc 0))) @@ -929,6 +978,7 @@ :team-id (dm/str team-id)) (let [tpoint (dt/tpoint) + err (volatile! false) migrate-file (fn [system file-id] @@ -948,7 +998,8 @@ (update-team-features! conn id features)))] - (binding [*team-stats* (atom {})] + (binding [*team-stats* (atom {}) + *team-id* team-id] (try (db/tx-run! system (fn [system] (db/exec-one! system ["SET idle_in_transaction_session_timeout = 0"]) @@ -956,6 +1007,10 @@ (if (contains? (:features team) "components/v2") (l/inf :hint "team already migrated") (migrate-team system team))))) + (catch Throwable cause + (vreset! err true) + (throw cause)) + (finally (let [elapsed (tpoint) components (get @*team-stats* :processed/components 0) @@ -964,9 +1019,21 @@ (some-> *stats* (swap! update :processed/teams (fnil inc 0))) - (l/dbg :hint "migrate:team:end" - :team-id (dm/str team-id) - :files files - :components components - :graphics graphics - :elapsed (dt/format-duration elapsed)))))))) + (if (cache/cache? *cache*) + (let [cache-stats (cache/stats *cache*)] + (l/dbg :hint "migrate:team:end" + :team-id (dm/str team-id) + :files files + :components components + :graphics graphics + :crt (mth/to-fixed (:hit-rate cache-stats) 2) + :crq (str (:req-count cache-stats)) + :error @err + :elapsed (dt/format-duration elapsed))) + + (l/dbg :hint "migrate:team:end" + :team-id (dm/str team-id) + :files files + :components components + :graphics graphics + :elapsed (dt/format-duration elapsed))))))))) diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index 30b23dcea7..7c82d1b1cd 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -10,10 +10,12 @@ [app.common.pprint :as pp] [app.db :as db] [app.features.components-v2 :as feat] + [app.main :as main] [app.svgo :as svgo] + [app.util.cache :as cache] [app.util.time :as dt] + [app.worker :as-alias wrk] [cuerdas.core :as str] - [promesa.core :as p] [promesa.exec :as px] [promesa.exec.semaphore :as ps] [promesa.util :as pu])) @@ -36,8 +38,7 @@ (fn [_ _ oldv newv] (when (not= (:processed/files oldv) (:processed/files newv)) - (let [completed (:processed/files newv) - elapsed (tpoint)] + (let [elapsed (tpoint)] (l/dbg :hint "progress" :completed (:processed/files newv) :elapsed (dt/format-duration elapsed)))))) @@ -56,71 +57,13 @@ :completed completed :elapsed elapsed))))) -(defn- get-total-files - [pool & {:keys [team-id]}] - (if (some? team-id) - (let [sql (str/concat - "SELECT count(f.id) AS count FROM file AS f " - " JOIN project AS p ON (p.id = f.project_id) " - " WHERE p.team_id = ? AND f.deleted_at IS NULL " - " AND p.deleted_at IS NULL") - res (db/exec-one! pool [sql team-id])] - (:count res)) +(def ^:private sql:get-teams-1 + "SELECT id, features + FROM team + WHERE deleted_at IS NULL + ORDER BY created_at DESC") - (let [sql (str/concat - "SELECT count(id) AS count FROM file " - " WHERE deleted_at IS NULL") - res (db/exec-one! pool [sql])] - (:count res)))) - -(defn- get-total-teams - [pool] - (let [sql (str/concat - "SELECT count(id) AS count FROM team " - " WHERE deleted_at IS NULL") - res (db/exec-one! pool [sql])] - (:count res))) - -(defn- mark-team-migration! - [{:keys [::db/pool]} team-id] - ;; We execute this out of transaction because we want this - ;; change to be visible to all other sessions before starting - ;; the migration - (let [sql (str "UPDATE team SET features = " - " array_append(features, 'ephimeral/v2-migration') " - " WHERE id = ?")] - (db/exec-one! pool [sql team-id]))) - -(defn- unmark-team-migration! - [{:keys [::db/pool]} team-id] - ;; We execute this out of transaction because we want this - ;; change to be visible to all other sessions before starting - ;; the migration - (let [sql (str "UPDATE team SET features = " - " array_remove(features, 'ephimeral/v2-migration') " - " WHERE id = ?")] - (db/exec-one! pool [sql team-id]))) - -;; (def ^:private sql:get-teams -;; "SELECT id, features -;; FROM team -;; WHERE deleted_at IS NULL -;; ORDER BY created_at DESC") - -;; (def ^:private sql:get-teams -;; "SELECT t.id, t.features, -;; (SELECT count(*) -;; FROM file_media_object AS fmo -;; JOIN file AS f ON (f.id = fmo.file_id) -;; JOIN project AS p ON (p.id = f.project_id) -;; WHERE p.team_id = t.id -;; AND fmo.mtype = 'image/svg+xml' -;; AND fmo.is_local = false) AS graphics -;; FROM team AS t -;; ORDER BY t.created_at DESC") - - -(def ^:private sql:get-teams +(def ^:private sql:get-teams-2 "WITH teams AS ( SELECT t.id, t.features, (SELECT count(*) @@ -136,20 +79,37 @@ SELECT * FROM teams ") (defn- read-pred - [[op val field]] - (let [field (name field)] - (case op - :lt [(str/ffmt "WHERE % < ?" field) val] - :lte [(str/ffmt "WHERE % <= ?" field) val] - :gt [(str/ffmt "WHERE % > ?" field) val] - :gte [(str/ffmt "WHERE % >= ?" field) val] - :eq [(str/ffmt "WHERE % = ?" field) val] - [""]))) + [entries] + (let [entries (if (and (vector? entries) + (keyword? (first entries))) + [entries] + entries)] + (loop [params [] + queries [] + entries (seq entries)] + (if-let [[op val field] (first entries)] + (let [field (name field) + cond (case op + :lt (str/ffmt "% < ?" field) + :lte (str/ffmt "% <= ?" field) + :gt (str/ffmt "% > ?" field) + :gte (str/ffmt "% >= ?" field) + :eq (str/ffmt "% = ?" field))] + (recur (conj params val) + (conj queries cond) + (rest entries))) + + (let [sql (apply str "WHERE " (str/join " AND " queries))] + (apply vector sql params)))))) (defn- get-teams [conn pred] - (let [[sql & params] (read-pred pred)] - (->> (db/cursor conn (apply vector (str sql:get-teams sql) params)) + (let [sql (if pred + (let [[sql & params] (read-pred pred)] + (apply vector (str sql:get-teams-2 sql) params)) + [sql:get-teams-1])] + + (->> (db/cursor conn sql) (map feat/decode-row) (remove (fn [{:keys [features]}] (or (contains? features "ephimeral/v2-migration") @@ -162,8 +122,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn migrate-file! - [system file-id & {:keys [rollback?] :or {rollback? true}}] - + [file-id & {:keys [rollback? validate?] :or {rollback? true validate? false}}] (l/dbg :hint "migrate:start" :rollback rollback?) (let [tpoint (dt/tpoint) file-id (if (string? file-id) @@ -171,8 +130,8 @@ file-id)] (binding [feat/*stats* (atom {})] (try - (-> (assoc system ::db/rollback rollback?) - (feat/migrate-file! file-id)) + (-> (assoc main/system ::db/rollback rollback?) + (feat/migrate-file! file-id :validate? validate?)) (-> (deref feat/*stats*) (assoc :elapsed (dt/format-duration (tpoint)))) @@ -185,12 +144,10 @@ (l/dbg :hint "migrate:end" :rollback rollback? :elapsed elapsed))))))) (defn migrate-team! - [{:keys [::db/pool] :as system} team-id & {:keys [rollback? skip-on-graphic-error? validate? skip-mark?] - :or {rollback? true - validate? true - skip-on-graphic-error? false - skip-mark? false} - :as opts}] + [team-id & {:keys [rollback? skip-on-graphic-error? validate?] + :or {rollback? true + validate? true + skip-on-graphic-error? false}}] (l/dbg :hint "migrate:start" :rollback rollback?) @@ -204,10 +161,7 @@ (binding [feat/*stats* stats] (try - (when-not skip-mark? - (mark-team-migration! system team-id)) - - (-> (assoc system ::db/rollback rollback?) + (-> (assoc main/system ::db/rollback rollback?) (feat/migrate-team! team-id :validate? validate? :skip-on-graphics-error? skip-on-graphic-error?)) @@ -219,9 +173,6 @@ (l/dbg :hint "migrate:error" :cause cause)) (finally - (when-not skip-mark? - (unmark-team-migration! system team-id)) - (let [elapsed (dt/format-duration (tpoint))] (l/dbg :hint "migrate:end" :rollback rollback? :elapsed elapsed))))))) @@ -232,28 +183,31 @@ until thw maximum number of jobs is reached which by default has the value of `1`. This is controled with the `:max-jobs` option." - [{:keys [::db/pool] :as system} & {:keys [max-jobs max-items max-time - rollback? validate? preset - pred max-procs skip-mark? - on-start on-progress on-error on-end] - :or {validate? true - rollback? true - preset :shutdown-on-failure - skip-mark? true - max-jobs 1 - max-items Long/MAX_VALUE} - :as opts}] + [& {:keys [max-jobs max-items max-time rollback? validate? + pred max-procs cache on-start on-progress on-error on-end + skip-on-graphic-error?] + :or {validate? false + rollback? true + max-jobs 1 + skip-on-graphic-error? true + max-items Long/MAX_VALUE}}] (let [stats (atom {}) tpoint (dt/tpoint) mtime (some-> max-time dt/duration) - factory (px/thread-factory :virtual false :prefix "penpot/migration/compv2/") + factory (px/thread-factory :virtual false :prefix "penpot/migration/") executor (px/cached-executor :factory factory) + max-procs (or max-procs max-jobs) sjobs (ps/create :permits max-jobs) sprocs (ps/create :permits max-procs) + cache (if (int? cache) + (cache/create :executor (::wrk/executor main/system) + :max-items cache) + nil) + migrate-team (fn [team-id] (ps/acquire! sjobs) @@ -268,18 +222,18 @@ (px/run! executor (fn [] (try - (when-not skip-mark? - (mark-team-migration! system team-id)) - (-> (assoc system ::db/rollback rollback?) - (feat/migrate-team! team-id :validate? validate?)) + (-> (assoc main/system ::db/rollback rollback?) + (feat/migrate-team! team-id + :validate? validate? + :skip-on-graphics-error? skip-on-graphic-error?)) + (catch Throwable cause - (l/err :hint "unexpected error on processing team (skiping)" + (l/wrn :hint "unexpected error on processing team (skiping)" :team-id (str team-id) :cause cause)) + (finally - (ps/release! sjobs) - (when-not skip-mark? - (unmark-team-migration! system team-id)))))))))] + (ps/release! sjobs))))))))] (l/dbg :hint "migrate:start" :rollback rollback? @@ -289,14 +243,17 @@ (add-watch stats :progress-report (report-progress-teams tpoint on-progress)) (binding [feat/*stats* stats + feat/*cache* cache svgo/*semaphore* sprocs] (try (when (fn? on-start) (on-start {:rollback rollback?})) - (db/tx-run! system + (db/tx-run! main/system (fn [{:keys [::db/conn]}] - (run! (partial migrate-team) + (db/exec! conn ["SET statement_timeout = 0;"]) + (db/exec! conn ["SET idle_in_transaction_session_timeout = 0;"]) + (run! migrate-team (->> (get-teams conn pred) (take max-items))))) From ec1bcada86887bcbaa9f532b9cd55a351d4c8cf9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 16 Jan 2024 19:29:17 +0100 Subject: [PATCH 14/54] :bug: Fix recent colors on components migration --- backend/src/app/features/components_v2.clj | 15 ++++++++++++--- backend/src/app/srepl/components_v2.clj | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 21a47d05f5..5b25fa3f14 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -21,8 +21,10 @@ [app.common.geom.shapes :as gsh] [app.common.logging :as l] [app.common.math :as mth] + [app.common.schema :as sm] [app.common.svg :as csvg] [app.common.svg.shapes-builder :as sbuilder] + [app.common.types.color :as ctc] [app.common.types.component :as ctk] [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] @@ -113,6 +115,13 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) + fix-recent-colors + (fn [file-data] + (let [valid-color? (sm/validator ::ctc/recent-color)] + (d/update-when file-data :recent-colors + (fn [colors] + (filterv valid-color? colors))))) + fix-orphan-shapes (fn [file-data] ;; Find shapes that are not listed in their parent's children list. @@ -344,6 +353,7 @@ (update :components update-vals fix-container))))] (-> file-data + (fix-recent-colors) (fix-orphan-shapes) (remove-nested-roots) (add-not-nested-roots) @@ -735,7 +745,7 @@ (defn- create-media-grid [fdata page-id frame-id grid media-group] - (letfn [(process-media-object [fdata mobj position] + (letfn [(process [fdata mobj position] (let [position (gpt/add position (gpt/point grid-gap grid-gap)) tp (dt/tpoint) err (volatile! false)] @@ -785,8 +795,7 @@ (sse/tap {:type :migration-progress :section :graphics :name (:name mobj)}) - (or (process-media-object fdata mobj position) - fdata)) + (or (process fdata mobj position) fdata)) (assoc-in fdata [:options :components-v2] true))))) (defn- migrate-graphics diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index 7c82d1b1cd..ddc3207014 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -204,7 +204,7 @@ sprocs (ps/create :permits max-procs) cache (if (int? cache) - (cache/create :executor (::wrk/executor main/system) + (cache/create :executor :same-thread :max-items cache) nil) From c7fa7aa7bc4a69fdf751576449b5a404612c4c79 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 11:45:24 +0100 Subject: [PATCH 15/54] :bug: Add migrations for fix shape geometry missing props --- common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 64 +++++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 8c34b89a41..327414f4e7 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 39) +(def version 41) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index a7ac391518..85f0522f71 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -318,7 +318,7 @@ (= "#7B7D85" fill-color))) (dissoc :fill-color :fill-opacity)))) - (update-container [{:keys [objects] :as container}] + (update-container [container] (if (contains? container :objects) (loop [objects (:objects container) shapes (->> (vals objects) @@ -627,8 +627,8 @@ ;; Ensure all root objects are well formed shapes. (if (= (:id object) uuid/zero) (-> object - (assoc :parent-id uuid/zero - :frame-id uuid/zero) + (assoc :parent-id uuid/zero) + (assoc :frame-id uuid/zero) (cts/setup-shape)) object)) @@ -703,7 +703,6 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) - (defmethod migrate 39 [data] (letfn [(update-shape [shape] @@ -718,3 +717,60 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) + +(defmethod migrate 40 + [data] + (letfn [(update-shape [{:keys [content shapes] :as shape}] + ;; Fix frame shape that in reallity is a path shape + (if (and (cfh/frame-shape? shape) + (contains? shape :selrect) + (seq content) + (not (seq shapes)) + (contains? (first content) :command)) + (-> shape + (assoc :type :path) + (assoc :x nil) + (assoc :y nil) + (assoc :width nil) + (assoc :height nil)) + shape)) + + (update-container [container] + (d/update-when container :objects update-vals update-shape))] + + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) + +(defmethod migrate 41 + [data] + (letfn [(update-shape [shape] + (cond + (or (cfh/bool-shape? shape) + (cfh/path-shape? shape)) + shape + + ;; Fix all shapes that has geometry broken but still + ;; preservers the selrect, so we recalculate the + ;; geometry from selrect. + (and (contains? shape :selrect) + (or (nil? (:x shape)) + (nil? (:y shape)) + (nil? (:width shape)) + (nil? (:height shape)))) + (let [selrect (:selrect shape)] + (-> shape + (assoc :x (:x selrect)) + (assoc :y (:y selrect)) + (assoc :width (:width selrect)) + (assoc :height (:height selrect)))) + + :else + shape)) + + (update-container [container] + (d/update-when container :objects update-vals update-shape))] + + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) From 3d84270f50ee49596a817780c3ae5131da9a4a30 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 12:28:56 +0100 Subject: [PATCH 16/54] :bug: Fix invalid ##Inf value on layout-gap on migrating to comp-v2 --- backend/src/app/features/components_v2.clj | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 5b25fa3f14..ba0f3dd474 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -115,6 +115,30 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) + fix-misc-shape-issues + (fn [file-data] + ;; Find shapes that are not listed in their parent's children list. + ;; Remove them, and also their children + (let [update-shape + (fn [shape] + (cond-> shape + ;; Some shapes has invalid value there + (contains? shape :layout-gap) + (d/update-in-when [:layout-gap :column-gap] + (fn [gap] + (if (or (= gap ##Inf) + (= gap ##-Inf)) + 0 + gap))))) + + update-container + (fn [container] + (d/update-when container :objects update-vals update-shape))] + + (-> file-data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) + fix-recent-colors (fn [file-data] (let [valid-color? (sm/validator ::ctc/recent-color)] @@ -353,6 +377,7 @@ (update :components update-vals fix-container))))] (-> file-data + (fix-misc-shape-issues) (fix-recent-colors) (fix-orphan-shapes) (remove-nested-roots) @@ -865,6 +890,7 @@ (decode-row) (update :data assoc :id id) (update :data fdata/process-pointers deref) + (update :data fdata/process-objects (partial into {})) (fmg/migrate-file)))) (defn- get-team From ba3c42e62cdcc6547bcfff45cef36d8286cd2b3f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 17:25:43 +0100 Subject: [PATCH 17/54] :bug: Fix broken layout and layout-gap props on migrating to comp-v2 --- backend/src/app/features/components_v2.clj | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index ba0f3dd474..0007934bba 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -129,7 +129,16 @@ (if (or (= gap ##Inf) (= gap ##-Inf)) 0 - gap))))) + gap))) + + ;; Fix some broken layout related attrs, probably + ;; of copypaste on flex layout betatest period + (true? (:layout shape)) + (assoc :layout :flex) + + (number? (:layout-gap shape)) + (as-> shape (let [n (:layout-gap shape)] + (assoc shape :layout-gap {:row-gap n :column-gap n}))))) update-container (fn [container] From 3e89a226008e830999313ac60f05be1689e11128 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 18:08:34 +0100 Subject: [PATCH 18/54] :bug: Remove broken and unfixable image shapes on comp-v2 migration --- backend/src/app/features/components_v2.clj | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 0007934bba..baf72e9413 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -115,6 +115,27 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) + + fix-missing-image-metadata + (fn [file-data] + (let [update-object + (fn [objects id shape] + (if (and (cfh/image-shape? shape) + (nil? (:metadata shape))) + (-> objects + (dissoc id) + (d/update-in-when [(:parent-id shape) :shapes] + (fn [shapes] (filterv #(not= id %) shapes)))) + objects)) + + update-page + (fn [page] + (d/update-when page :objects #(reduce-kv update-object % %)))] + + (-> file-data + (update :pages-index update-vals update-page) + (update :components update-vals update-page)))) + fix-misc-shape-issues (fn [file-data] ;; Find shapes that are not listed in their parent's children list. @@ -388,6 +409,7 @@ (-> file-data (fix-misc-shape-issues) (fix-recent-colors) + (fix-missing-image-metadata) (fix-orphan-shapes) (remove-nested-roots) (add-not-nested-roots) From 6ad6e6f8564eeddc495b0babfde864dea42c0331 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 23:52:58 +0100 Subject: [PATCH 19/54] :bug: Fix objects-map and pointer-map issues on file crud --- backend/src/app/features/fdata.clj | 4 +- backend/src/app/rpc/commands/files.clj | 46 ++++++++---- backend/src/app/rpc/commands/files_create.clj | 73 ++++++++++--------- backend/src/app/rpc/commands/files_update.clj | 31 ++++++-- backend/src/app/util/pointer_map.clj | 56 ++++++++------ backend/test/backend_tests/rpc_file_test.clj | 29 +++++--- common/src/app/common/data.cljc | 2 +- common/src/app/common/files/migrations.cljc | 4 + common/src/app/common/types/page.cljc | 11 +-- 9 files changed, 153 insertions(+), 103 deletions(-) diff --git a/backend/src/app/features/fdata.clj b/backend/src/app/features/fdata.clj index 68e58833cd..8a57a1aa17 100644 --- a/backend/src/app/features/fdata.clj +++ b/backend/src/app/features/fdata.clj @@ -27,7 +27,7 @@ (update :data (fn [fdata] (-> fdata (update :pages-index update-vals update-fn) - (update :components update-vals update-fn)))) + (d/update-when :components update-vals update-fn)))) (update :features conj "fdata/objects-map")))) (defn process-objects @@ -110,6 +110,6 @@ (update :data (fn [fdata] (-> fdata (update :pages-index update-vals pmap/wrap) - (update :components pmap/wrap)))) + (d/update-when :components pmap/wrap)))) (update :features conj "fdata/pointer-map"))) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index f6b6c615da..f2e6a19899 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -226,23 +226,37 @@ [{:keys [::db/conn] :as cfg} {:keys [id] :as file}] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) pmap/*tracked* (pmap/create-tracked)] - (let [file (fmg/migrate-file file)] + (let [;; For avoid unnecesary overhead of creating multiple pointers and + ;; handly internally with objects map in their worst case (when + ;; probably all shapes and all pointers will be readed in any + ;; case), we just realize/resolve them before applying the + ;; migration to the file + file (-> file + (update :data feat.fdata/process-pointers deref) + (update :data feat.fdata/process-objects (partial into {})) + (fmg/migrate-file)) - ;; NOTE: when file is migrated, we break the rule of no perform - ;; mutations on get operations and update the file with all - ;; migrations applied - ;; - ;; NOTE: the following code will not work on read-only mode, it - ;; is a known issue; we keep is not implemented until we really - ;; need this - (when (fmg/migrated? file) - (db/update! conn :file - {:data (blob/encode (:data file)) - :features (db/create-array conn "text" (:features file))} - {:id id}) + ;; When file is migrated, we break the rule of no perform + ;; mutations on get operations and update the file with all + ;; migrations applied + ;; + ;; WARN: he following code will not work on read-only mode, + ;; it is a known issue; we keep is not implemented until we + ;; really need this. + file (if (contains? (:features file) "fdata/objects-map") + (feat.fdata/enable-objects-map file) + file) + file (if (contains? (:features file) "fdata/pointer-map") + (feat.fdata/enable-pointer-map file) + file)] - (when (contains? (:features file) "fdata/pointer-map") - (feat.fdata/persist-pointers! cfg id))) + (db/update! conn :file + {:data (blob/encode (:data file)) + :features (db/create-array conn "text" (:features file))} + {:id id}) + + (when (contains? (:features file) "fdata/pointer-map") + (feat.fdata/persist-pointers! cfg id)) file))) @@ -266,7 +280,7 @@ ::db/remove-deleted (not include-deleted?) ::sql/for-update lock-for-update?}) (decode-row))] - (if migrate? + (if (and migrate? (fmg/need-migration? file)) (migrate-file cfg file) file))) diff --git a/backend/src/app/rpc/commands/files_create.clj b/backend/src/app/rpc/commands/files_create.clj index 59273f033e..dbbd1c04d1 100644 --- a/backend/src/app/rpc/commands/files_create.clj +++ b/backend/src/app/rpc/commands/files_create.clj @@ -18,14 +18,12 @@ [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] [app.rpc :as-alias rpc] - [app.rpc.commands.files :as files] [app.rpc.commands.projects :as projects] [app.rpc.commands.teams :as teams] [app.rpc.doc :as-alias doc] [app.rpc.permissions :as perms] [app.rpc.quotes :as quotes] [app.util.blob :as blob] - [app.util.objects-map :as omap] [app.util.pointer-map :as pmap] [app.util.services :as sv] [app.util.time :as dt] @@ -50,47 +48,52 @@ "expected a valid connection" (db/connection? conn)) - (let [id (or id (uuid/next)) + (binding [pmap/*tracked* (pmap/create-tracked) + cfeat/*current* features] + (let [id (or id (uuid/next)) - pointers (pmap/create-tracked) - pmap? (contains? features "fdata/pointer-map") - omap? (contains? features "fdata/objects-map") - - data (binding [pmap/*tracked* pointers - cfeat/*current* features - cfeat/*wrap-with-objects-map-fn* (if omap? omap/wrap identity) - cfeat/*wrap-with-pointer-map-fn* (if pmap? pmap/wrap identity)] - (if create-page + data (if create-page (ctf/make-file-data id) - (ctf/make-file-data id nil))) + (ctf/make-file-data id nil)) - features (->> (set/difference features cfeat/frontend-only-features) - (db/create-array conn "text")) + file {:id id + :project-id project-id + :name name + :revn revn + :is-shared is-shared + :data data + :features features + :ignore-sync-until ignore-sync-until + :modified-at modified-at + :deleted-at deleted-at} - file (db/insert! conn :file - (d/without-nils - {:id id - :project-id project-id - :name name - :revn revn - :is-shared is-shared - :data (blob/encode data) - :features features - :ignore-sync-until ignore-sync-until - :modified-at modified-at - :deleted-at deleted-at}))] + file (if (contains? features "fdata/objects-map") + (feat.fdata/enable-objects-map file) + file) - (binding [pmap/*tracked* pointers] - (feat.fdata/persist-pointers! cfg id)) + file (if (contains? features "fdata/pointer-map") + (feat.fdata/enable-pointer-map file) + file) - (->> (assoc params :file-id id :role :owner) - (create-file-role! conn)) + file (d/without-nils file)] - (db/update! conn :project - {:modified-at (dt/now)} - {:id project-id}) + (db/insert! conn :file + (-> file + (update :data blob/encode) + (update :features db/encode-pgarray conn "text")) + {::db/return-keys false}) - (files/decode-row file))) + (when (contains? features "fdata/pointer-map") + (feat.fdata/persist-pointers! cfg id)) + + (->> (assoc params :file-id id :role :owner) + (create-file-role! conn)) + + (db/update! conn :project + {:modified-at (dt/now)} + {:id project-id}) + + file))) (def ^:private schema:create-file [:map {:title "create-file"} diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 03e1b04da4..134a127946 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -292,9 +292,20 @@ (let [file (update file :data (fn [data] (-> data (blob/decode) - (assoc :id (:id file)) - (fmg/migrate-data) - (d/without-nils)))) + (assoc :id (:id file))))) + + ;; For avoid unnecesary overhead of creating multiple pointers + ;; and handly internally with objects map in their worst + ;; case (when probably all shapes and all pointers will be + ;; readed in any case), we just realize/resolve them before + ;; applying the migration to the file + file (if (fmg/need-migration? file) + (-> file + (update :data feat.fdata/process-pointers deref) + (update :data feat.fdata/process-objects (partial into {})) + (fmg/migrate-file)) + file) + ;; WARNING: this ruins performance; maybe we need to find ;; some other way to do general validation @@ -305,14 +316,20 @@ (into [file] (map (fn [{:keys [id]}] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id) pmap/*tracked* nil] + ;; We do not resolve the objects maps here + ;; because there is a lower probability that all + ;; shapes needed to be loded into memory, so we + ;; leeave it on lazy status (-> (files/get-file cfg id :migrate? false) (update :data feat.fdata/process-pointers deref) ; ensure all pointers resolved (fmg/migrate-file)))))) (d/index-by :id))) + file (-> (files/check-version! file) (update :revn inc) - (update :data cpc/process-changes changes))] + (update :data cpc/process-changes changes) + (update :data d/without-nils))] (when (contains? cf/flags :soft-file-validation) (soft-validate-file! file libs)) @@ -329,12 +346,10 @@ (val/validate-file-schema! file)) (cond-> file - (and (contains? cfeat/*current* "fdata/objects-map") - (not (contains? cfeat/*previous* "fdata/objects-map"))) + (contains? cfeat/*current* "fdata/objects-map") (feat.fdata/enable-objects-map) - (and (contains? cfeat/*current* "fdata/pointer-map") - (not (contains? cfeat/*previous* "fdata/pointer-map"))) + (contains? cfeat/*current* "fdata/pointer-map") (feat.fdata/enable-pointer-map) :always diff --git a/backend/src/app/util/pointer_map.clj b/backend/src/app/util/pointer_map.clj index 6e8e76b828..bb7b252939 100644 --- a/backend/src/app/util/pointer_map.clj +++ b/backend/src/app/util/pointer_map.clj @@ -166,32 +166,36 @@ (assoc [this key val] (when-not loaded? (load! this)) - (let [odata (assoc odata key val) - mdata (assoc mdata :created-at (dt/now)) - id (if modified? id (uuid/next)) - pmap (PointerMap. id - mdata - odata - true - true)] - (some-> *tracked* (swap! assoc id pmap)) - pmap)) + (let [odata' (assoc odata key val)] + (if (identical? odata odata') + this + (let [mdata (assoc mdata :created-at (dt/now)) + id (if modified? id (uuid/next)) + pmap (PointerMap. id + mdata + odata' + true + true)] + (some-> *tracked* (swap! assoc id pmap)) + pmap)))) (assocEx [_ _ _] (throw (UnsupportedOperationException. "method not implemented"))) (without [this key] (when-not loaded? (load! this)) - (let [odata (dissoc odata key) - mdata (assoc mdata :created-at (dt/now)) - id (if modified? id (uuid/next)) - pmap (PointerMap. id - mdata - odata - true - true)] - (some-> *tracked* (swap! assoc id pmap)) - pmap)) + (let [odata' (dissoc odata key)] + (if (identical? odata odata') + this + (let [mdata (assoc mdata :created-at (dt/now)) + id (if modified? id (uuid/next)) + pmap (PointerMap. id + mdata + odata' + true + true)] + (some-> *tracked* (swap! assoc id pmap)) + pmap)))) Counted (count [this] @@ -206,6 +210,8 @@ (defn create ([] (let [id (uuid/next) + + mdata (assoc *metadata* :created-at (dt/now)) pmap (PointerMap. id mdata {} true true)] (some-> *tracked* (swap! assoc id pmap)) @@ -225,7 +231,15 @@ (do (some-> *tracked* (swap! assoc (get-id data) data)) data) - (into (create) data))) + (let [mdata (assoc (meta data) :created-at (dt/now)) + id (uuid/next) + pmap (PointerMap. id + mdata + data + true + true)] + (some-> *tracked* (swap! assoc id pmap)) + pmap))) (fres/add-handlers! {:name "penpot/pointer-map/v1" diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 6be373a29c..510aadd892 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -166,18 +166,21 @@ :name "test" :id page-id}]) - ;; Check the number of fragments - (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] - (t/is (= 2 (count rows)))) - - ;; Check the number of fragments - (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] - (t/is (= 2 (count rows)))) - - ;; The file-gc should remove unused fragments + ;; The file-gc should mark for remove unused fragments (let [res (th/run-task! :file-gc {:min-age 0})] (t/is (= 1 (:processed res)))) + ;; Check the number of fragments + (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] + (t/is (= 2 (count rows)))) + + ;; The objects-gc should remove unused fragments + (let [res (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 0 (:processed res)))) + + ;; Check the number of fragments + (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] + (t/is (= 2 (count rows)))) ;; Add shape to page that should add a new fragment (update-file! @@ -202,10 +205,14 @@ (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] (t/is (= 3 (count rows)))) - ;; The file-gc should remove unused fragments + ;; The file-gc should mark for remove unused fragments (let [res (th/run-task! :file-gc {:min-age 0})] (t/is (= 1 (:processed res)))) + ;; The objects-gc should remove unused fragments + (let [res (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 0 (:processed res)))) + ;; Check the number of fragments; should be 3 because changes ;; are also holding pointers to fragments; (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] @@ -235,8 +242,6 @@ (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] (t/is (= 2 (count rows))))))) - - (t/deftest file-gc-task-with-thumbnails (letfn [(add-file-media-object [& {:keys [profile-id file-id]}] (let [mfile {:filename "sample.jpg" diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index c01cdb3d65..12e3f7762b 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -7,7 +7,7 @@ (ns app.common.data "A collection if helpers for working with data structures and other data resources." - (:refer-clojure :exclude [read-string hash-map merge name update-vals + (:refer-clojure :exclude [read-string hash-map merge name parse-double group-by iteration concat mapcat parse-uuid max min]) #?(:cljs diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 85f0522f71..499ec20084 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -31,6 +31,10 @@ (defmulti migrate :version) +(defn need-migration? + [{:keys [data]}] + (> cfd/version (:version data 0))) + (defn migrate-data ([data] (migrate-data data version)) ([data to-version] diff --git a/common/src/app/common/types/page.cljc b/common/src/app/common/types/page.cljc index ec24c52a27..6c1d427dff 100644 --- a/common/src/app/common/types/page.cljc +++ b/common/src/app/common/types/page.cljc @@ -7,7 +7,6 @@ (ns app.common.types.page (:require [app.common.data :as d] - [app.common.features :as cfeat] [app.common.schema :as sm] [app.common.types.color :as-alias ctc] [app.common.types.grid :as ctg] @@ -71,13 +70,9 @@ (defn make-empty-page [id name] - (let [wrap-objects-fn cfeat/*wrap-with-objects-map-fn* - wrap-pointer-fn cfeat/*wrap-with-pointer-map-fn*] - (-> empty-page-data - (assoc :id id) - (assoc :name name) - (update :objects wrap-objects-fn) - (wrap-pointer-fn)))) + (-> empty-page-data + (assoc :id id) + (assoc :name name))) ;; --- Helpers for flow From 166d2b7b68d7ed3e6e2a4c0a84ab4ed6e2684593 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 18 Jan 2024 11:56:57 +0100 Subject: [PATCH 20/54] :bug: Fix broken fills and strokes on comp-v2 migration --- backend/src/app/features/components_v2.clj | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index baf72e9413..9af03ab4a7 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -105,6 +105,9 @@ or that are the result of old bugs." [file-data libraries] (let [detached-ids (volatile! #{}) + valid-color? (sm/validator ::ctc/recent-color) + valid-fill? (sm/validator ::cts/fill) + valid-stroke? (sm/validator ::cts/stroke) detach-shape (fn [container shape] @@ -115,7 +118,6 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) - fix-missing-image-metadata (fn [file-data] (let [update-object @@ -152,6 +154,14 @@ 0 gap))) + ;; Fix broken fills + (seq (:fills shape)) + (update :fills (fn [fills] (filterv valid-fill? fills))) + + ;; Fix broken strokes + (seq (:strokes shape)) + (update :strokes (fn [strokes] (filterv valid-stroke? strokes))) + ;; Fix some broken layout related attrs, probably ;; of copypaste on flex layout betatest period (true? (:layout shape)) @@ -171,10 +181,9 @@ fix-recent-colors (fn [file-data] - (let [valid-color? (sm/validator ::ctc/recent-color)] - (d/update-when file-data :recent-colors - (fn [colors] - (filterv valid-color? colors))))) + (d/update-when file-data :recent-colors + (fn [colors] + (filterv valid-color? colors)))) fix-orphan-shapes (fn [file-data] From 5b84054eaaf886dcca11efc5847240ddd06013fa Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 17 Jan 2024 18:09:58 +0100 Subject: [PATCH 21/54] :bug: Fix shape validation schema --- .clj-kondo/config.edn | 1 + common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 34 ++++++++++-------- common/src/app/common/schema/generators.cljc | 10 +++++- common/src/app/common/types/shape.cljc | 38 +++++++++++++------- 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 60606932bf..fe1d14d908 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -5,6 +5,7 @@ promesa.exec.csp/go-loop clojure.core/loop rumext.v2/defc clojure.core/defn promesa.util/with-open clojure.core/with-open + app.common.schema.generators/let clojure.core/let app.common.data/export clojure.core/def app.common.data.macros/get-in clojure.core/get-in app.common.data.macros/with-open clojure.core/with-open diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 327414f4e7..bb448f9975 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 41) +(def version 42) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 499ec20084..982d5b10d0 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -577,21 +577,6 @@ (update :pages-index update-vals update-container) (update :components update-vals update-container)))) -(defmethod migrate 30 - [data] - (letfn [(update-object [object] - (if (and (cfh/frame-shape? object) - (not (:shapes object))) - (assoc object :shapes []) - object)) - - (update-container [container] - (d/update-when container :objects update-vals update-object))] - - (-> data - (update :pages-index update-vals update-container) - (update :components update-vals update-container)))) - (defmethod migrate 31 [data] (letfn [(update-object [object] @@ -778,3 +763,22 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) + +(defmethod migrate 42 + [data] + (letfn [(update-object [object] + (if (and (or (cfh/frame-shape? object) + (cfh/group-shape? object) + (cfh/bool-shape? object)) + (not (:shapes object))) + (assoc object :shapes []) + object)) + + (update-container [container] + (d/update-when container :objects update-vals update-object))] + + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) + + diff --git a/common/src/app/common/schema/generators.cljc b/common/src/app/common/schema/generators.cljc index f1aa8c90fd..83e00bfd87 100644 --- a/common/src/app/common/schema/generators.cljc +++ b/common/src/app/common/schema/generators.cljc @@ -5,7 +5,7 @@ ;; Copyright (c) KALEIDOS INC (ns app.common.schema.generators - (:refer-clojure :exclude [set subseq uuid for filter map]) + (:refer-clojure :exclude [set subseq uuid for filter map let]) #?(:cljs (:require-macros [app.common.schema.generators])) (:require [app.common.schema.registry :as sr] @@ -37,6 +37,10 @@ [& params] `(tp/for-all ~@params)) +(defmacro let + [& params] + `(tg/let ~@params)) + (defn check! [p & {:keys [num] :or {num 20} :as options}] (tc/quick-check num p (assoc options :reporter-fn default-reporter-fn :max-size 50))) @@ -124,6 +128,10 @@ [f g] (tg/fmap f g)) +(defn mcat + [f g] + (tg/bind g f)) + (defn tuple [& opts] (apply tg/tuple opts)) diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index 2397d58458..4f347e9ff7 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -46,6 +46,7 @@ :bool :rect :path + :text :circle :svg-raw :image}) @@ -199,7 +200,7 @@ (sm/define! ::group-attrs [:map {:title "GroupAttrs"} [:type [:= :group]] - [:shapes {:optional true} [:maybe [:vector {:gen/max 10 :gen/min 1} ::sm/uuid]]]]) + [:shapes [:vector {:gen/max 10 :gen/min 1} ::sm/uuid]]]) (sm/define! ::frame-attrs [:map {:title "FrameAttrs"} @@ -212,7 +213,7 @@ (sm/define! ::bool-attrs [:map {:title "BoolAttrs"} [:type [:= :bool]] - [:shapes {:optional true} [:maybe [:vector {:gen/max 10 :gen/min 1} ::sm/uuid]]] + [:shapes [:vector {:gen/max 10 :gen/min 1} ::sm/uuid]] ;; FIXME: improve this schema [:bool-type :keyword] @@ -271,63 +272,63 @@ (sm/define! ::shape-map [:multi {:dispatch :type :title "Shape"} [:group - [:merge {:title "GroupShape"} + [:and {:title "GroupShape"} ::shape-attrs ::minimal-shape-attrs ::group-attrs ::ctsl/layout-child-attrs]] [:frame - [:merge {:title "FrameShape"} + [:and {:title "FrameShape"} ::minimal-shape-attrs ::frame-attrs ::ctsl/layout-attrs ::ctsl/layout-child-attrs]] [:bool - [:merge {:title "BoolShape"} + [:and {:title "BoolShape"} ::shape-attrs ::minimal-shape-attrs ::bool-attrs ::ctsl/layout-child-attrs]] [:rect - [:merge {:title "RectShape"} + [:and {:title "RectShape"} ::shape-attrs ::minimal-shape-attrs ::rect-attrs ::ctsl/layout-child-attrs]] [:circle - [:merge {:title "CircleShape"} + [:and {:title "CircleShape"} ::shape-attrs ::minimal-shape-attrs ::circle-attrs ::ctsl/layout-child-attrs]] [:image - [:merge {:title "ImageShape"} + [:and {:title "ImageShape"} ::shape-attrs ::minimal-shape-attrs ::image-attrs ::ctsl/layout-child-attrs]] [:svg-raw - [:merge {:title "SvgRawShape"} + [:and {:title "SvgRawShape"} ::shape-attrs ::minimal-shape-attrs ::svg-raw-attrs ::ctsl/layout-child-attrs]] [:path - [:merge {:title "PathShape"} + [:and {:title "PathShape"} ::shape-attrs ::minimal-shape-attrs ::path-attrs ::ctsl/layout-child-attrs]] [:text - [:merge {:title "TextShape"} + [:and {:title "TextShape"} ::shape-attrs ::minimal-shape-attrs ::text-attrs @@ -336,7 +337,20 @@ (sm/define! ::shape [:and {:title "Shape" - :gen/gen (->> (sg/generator ::shape-map) + :gen/gen (->> (sg/generator ::minimal-shape-attrs) + (sg/mcat (fn [{:keys [type] :as shape}] + (sg/let [attrs1 (sg/generator ::shape-attrs) + attrs2 (case type + :text (sg/generator ::text-attrs) + :path (sg/generator ::path-attrs) + :svg-raw (sg/generator ::svg-raw-attrs) + :image (sg/generator ::image-attrs) + :circle (sg/generator ::circle-attrs) + :rect (sg/generator ::rect-attrs) + :bool (sg/generator ::bool-attrs) + :group (sg/generator ::group-attrs) + :frame (sg/generator ::frame-attrs))] + (merge attrs1 shape attrs2)))) (sg/fmap map->Shape))} ::shape-map [:fn shape?]]) From 35da01bac98989d0a278ab8a5c2a4b4c0bf2cef0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 18 Jan 2024 11:57:18 +0100 Subject: [PATCH 22/54] :bug: Fix pages with shapes with to too big gemetry vals on comp-v2 migration --- backend/src/app/features/components_v2.clj | 35 ++++++++++++++++++++++ common/src/app/common/schema.cljc | 3 ++ 2 files changed, 38 insertions(+) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 9af03ab4a7..34fb5d8515 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -138,6 +138,40 @@ (update :pages-index update-vals update-page) (update :components update-vals update-page)))) + ;; At some point in time, we had a bug that generated shapes + ;; with huge geometries that did not validate the + ;; schema. Since we don't have a way to fix those shapes, we + ;; simply proceed to delete it. We ignore path type shapes + ;; because they have not been affected by the bug. + fix-big-invalid-shapes + (fn [file-data] + (let [update-object + (fn [objects id shape] + (cond + (or (cfh/path-shape? shape) + (cfh/bool-shape? shape)) + objects + + (or (and (number? (:x shape)) (not (sm/valid-safe-number? (:x shape)))) + (and (number? (:y shape)) (not (sm/valid-safe-number? (:y shape)))) + (and (number? (:width shape)) (not (sm/valid-safe-number? (:width shape)))) + (and (number? (:height shape)) (not (sm/valid-safe-number? (:height shape))))) + (-> objects + (dissoc id) + (d/update-in-when [(:parent-id shape) :shapes] + (fn [shapes] (filterv #(not= id %) shapes)))) + + :else + objects)) + + update-page + (fn [page] + (d/update-when page :objects #(reduce-kv update-object % %)))] + + (-> file-data + (update :pages-index update-vals update-page) + (update :components update-vals update-page)))) + fix-misc-shape-issues (fn [file-data] ;; Find shapes that are not listed in their parent's children list. @@ -419,6 +453,7 @@ (fix-misc-shape-issues) (fix-recent-colors) (fix-missing-image-metadata) + (fix-big-invalid-shapes) (fix-orphan-shapes) (remove-nested-roots) (add-not-nested-roots) diff --git a/common/src/app/common/schema.cljc b/common/src/app/common/schema.cljc index 7fed63b77b..b346f66d29 100644 --- a/common/src/app/common/schema.cljc +++ b/common/src/app/common/schema.cljc @@ -658,6 +658,9 @@ ;; ---- PREDICATES +(def valid-safe-number? + (lazy-validator ::safe-number)) + (def check-safe-int! (check-fn ::safe-int)) From 3bbd2023a423eb9ad199cbe95daa8013054a7ffa Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 19 Jan 2024 15:51:52 +0100 Subject: [PATCH 23/54] :bug: Fix incorrect validation of shape geom attrs Requied validation in a subset of supported shapes --- common/src/app/common/files/validate.cljc | 3 + common/src/app/common/types/shape.cljc | 82 +++++++++++++---------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 1e62d14ec3..ca4d471cb4 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -483,6 +483,9 @@ (sm/lazy-explainer ::ctf/data)) (defn validate-file-schema! + "Validates the file itself, without external dependencies, it + performs the schema checking and some semantical validation of the + content." [{:keys [id data] :as file}] (when-not (valid-fdata? data) (ex/raise :type :validation diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index 4f347e9ff7..6187a7aa7c 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -127,21 +127,24 @@ [:stroke-color-gradient {:optional true} ::ctc/gradient] [:stroke-image {:optional true} ::ctc/image-color]]) -(sm/define! ::minimal-shape-attrs +(sm/define! ::shape-base-attrs [:map {:title "ShapeMinimalRecord"} - [:id {:optional false} ::sm/uuid] - [:name {:optional false} :string] - [:type {:optional false} [::sm/one-of shape-types]] - [:x {:optional false} [:maybe ::sm/safe-number]] - [:y {:optional false} [:maybe ::sm/safe-number]] - [:width {:optional false} [:maybe ::sm/safe-number]] - [:height {:optional false} [:maybe ::sm/safe-number]] - [:selrect {:optional false} ::selrect] - [:points {:optional false} ::points] - [:transform {:optional false} ::gmt/matrix] - [:transform-inverse {:optional false} ::gmt/matrix] - [:parent-id {:optional false} ::sm/uuid] - [:frame-id {:optional false} ::sm/uuid]]) + [:id ::sm/uuid] + [:name :string] + [:type [::sm/one-of shape-types]] + [:selrect ::selrect] + [:points ::points] + [:transform ::gmt/matrix] + [:transform-inverse ::gmt/matrix] + [:parent-id ::sm/uuid] + [:frame-id ::sm/uuid]]) + +(sm/define! ::shape-geom-attrs + [:map {:title "ShapeGeometryAttrs"} + [:x ::sm/safe-number] + [:y ::sm/safe-number] + [:width ::sm/safe-number] + [:height ::sm/safe-number]]) (sm/define! ::shape-attrs [:map {:title "ShapeAttrs"} @@ -273,84 +276,95 @@ [:multi {:dispatch :type :title "Shape"} [:group [:and {:title "GroupShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::group-attrs ::ctsl/layout-child-attrs]] [:frame [:and {:title "FrameShape"} - ::minimal-shape-attrs + ::shape-base-attrs + ::shape-geom-attrs ::frame-attrs ::ctsl/layout-attrs ::ctsl/layout-child-attrs]] [:bool [:and {:title "BoolShape"} + ::shape-base-attrs ::shape-attrs - ::minimal-shape-attrs ::bool-attrs ::ctsl/layout-child-attrs]] [:rect [:and {:title "RectShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::rect-attrs ::ctsl/layout-child-attrs]] [:circle [:and {:title "CircleShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::circle-attrs ::ctsl/layout-child-attrs]] [:image [:and {:title "ImageShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::image-attrs ::ctsl/layout-child-attrs]] [:svg-raw [:and {:title "SvgRawShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::svg-raw-attrs ::ctsl/layout-child-attrs]] [:path [:and {:title "PathShape"} + ::shape-base-attrs ::shape-attrs - ::minimal-shape-attrs ::path-attrs ::ctsl/layout-child-attrs]] [:text [:and {:title "TextShape"} + ::shape-base-attrs + ::shape-geom-attrs ::shape-attrs - ::minimal-shape-attrs ::text-attrs ::ctsl/layout-child-attrs]]]) (sm/define! ::shape [:and {:title "Shape" - :gen/gen (->> (sg/generator ::minimal-shape-attrs) + :gen/gen (->> (sg/generator ::shape-base-attrs) (sg/mcat (fn [{:keys [type] :as shape}] (sg/let [attrs1 (sg/generator ::shape-attrs) - attrs2 (case type - :text (sg/generator ::text-attrs) - :path (sg/generator ::path-attrs) + attrs2 (sg/generator ::shape-geom-attrs) + attrs3 (case type + :text (sg/generator ::text-attrs) + :path (sg/generator ::path-attrs) :svg-raw (sg/generator ::svg-raw-attrs) - :image (sg/generator ::image-attrs) - :circle (sg/generator ::circle-attrs) - :rect (sg/generator ::rect-attrs) - :bool (sg/generator ::bool-attrs) - :group (sg/generator ::group-attrs) - :frame (sg/generator ::frame-attrs))] - (merge attrs1 shape attrs2)))) + :image (sg/generator ::image-attrs) + :circle (sg/generator ::circle-attrs) + :rect (sg/generator ::rect-attrs) + :bool (sg/generator ::bool-attrs) + :group (sg/generator ::group-attrs) + :frame (sg/generator ::frame-attrs))] + (if (or (= type :path) + (= type :bool)) + (merge attrs1 shape attrs3) + (merge attrs1 shape attrs2 attrs3))))) (sg/fmap map->Shape))} ::shape-map [:fn shape?]]) From 1f5991112dc7762a353b58e49236dcb25c1aa042 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 11:06:54 +0100 Subject: [PATCH 24/54] :bug: Add two more fixes to v2 migration --- backend/src/app/features/components_v2.clj | 37 +++++++++++++++++++++ common/src/app/common/files/migrations.cljc | 2 -- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 34fb5d8515..0d5875a143 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -118,6 +118,24 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) + remove-missing-children + (fn [file-data] + ;; Remove any child that does not exist. + (letfn [(fix-container + [container] + (update container :objects update-vals (partial fix-shape container))) + + (fix-shape + [container shape] + (let [objects (:objects container)] + (d/update-when shape :shapes + (fn [shapes] + (d/removev #(nil? (get objects %)) shapes)))))] + + (-> file-data + (update :pages-index update-vals fix-container) + (update :components update-vals fix-container)))) + fix-missing-image-metadata (fn [file-data] (let [update-object @@ -356,6 +374,23 @@ (update :pages-index update-vals fix-container) (update :components update-vals fix-container)))) + fix-path-copies + (fn [file-data] + ;; If the user has created a copy and then converted into a path, detach it + ;; because the synchronization will no longer work. + (letfn [(fix-container [container] + (update container :objects update-vals fix-shape)) + + (fix-shape [shape] + (if (and (ctk/instance-head? shape) + (cfh/path-shape? shape)) + (ctk/detach-shape shape) + shape))] + + (-> file-data + (update :pages-index update-vals fix-container) + (update :components update-vals fix-container)))) + transform-to-frames (fn [file-data] ;; Transform component and copy heads to frames, and set the @@ -450,6 +485,7 @@ (update :components update-vals fix-container))))] (-> file-data + (remove-missing-children) (fix-misc-shape-issues) (fix-recent-colors) (fix-missing-image-metadata) @@ -460,6 +496,7 @@ (fix-orphan-copies) (remap-refs) (fix-copies-of-detached) + (fix-path-copies) (transform-to-frames) (remap-frame-ids) (fix-frame-ids) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 982d5b10d0..1cc3f4c8b6 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -780,5 +780,3 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) - - From 1d21bd34f610360160a4225ccbfa165e495ecd43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 11:07:34 +0100 Subject: [PATCH 25/54] :bug: Check orphan copies before affecting later checks --- backend/src/app/features/components_v2.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 0d5875a143..ed6e205850 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -491,9 +491,9 @@ (fix-missing-image-metadata) (fix-big-invalid-shapes) (fix-orphan-shapes) + (fix-orphan-copies) (remove-nested-roots) (add-not-nested-roots) - (fix-orphan-copies) (remap-refs) (fix-copies-of-detached) (fix-path-copies) From c679b04ad528899959ef49a501813cb461806e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 13:12:20 +0100 Subject: [PATCH 26/54] :bug: Avoid adding empty attributes on update if they doesn't exist --- backend/src/app/features/components_v2.clj | 52 +++++++++++----------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index ed6e205850..47aea68fdd 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -123,7 +123,7 @@ ;; Remove any child that does not exist. (letfn [(fix-container [container] - (update container :objects update-vals (partial fix-shape container))) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] @@ -134,7 +134,7 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-missing-image-metadata (fn [file-data] @@ -154,7 +154,7 @@ (-> file-data (update :pages-index update-vals update-page) - (update :components update-vals update-page)))) + (d/update-when :components update-vals update-page)))) ;; At some point in time, we had a bug that generated shapes ;; with huge geometries that did not validate the @@ -188,7 +188,7 @@ (-> file-data (update :pages-index update-vals update-page) - (update :components update-vals update-page)))) + (d/update-when :components update-vals update-page)))) fix-misc-shape-issues (fn [file-data] @@ -229,7 +229,7 @@ (-> file-data (update :pages-index update-vals update-container) - (update :components update-vals update-container)))) + (d/update-when :components update-vals update-container)))) fix-recent-colors (fn [file-data] @@ -258,13 +258,13 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) remove-nested-roots (fn [file-data] ;; Remove :component-root in head shapes that are nested. (letfn [(fix-container [container] - (update container :objects update-vals (partial fix-shape container))) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] (let [parent (ctst/get-shape container (:parent-id shape))] @@ -275,13 +275,13 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) add-not-nested-roots (fn [file-data] ;; Add :component-root in head shapes that are not nested. (letfn [(fix-container [container] - (update container :objects update-vals (partial fix-shape container))) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] (let [parent (ctst/get-shape container (:parent-id shape))] @@ -292,13 +292,13 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-orphan-copies (fn [file-data] ;; Detach shapes that were inside a copy (have :shape-ref) but now they aren't. (letfn [(fix-container [container] - (update container :objects update-vals (partial fix-shape container))) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] (let [parent (ctst/get-shape container (:parent-id shape))] @@ -310,7 +310,7 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) remap-refs (fn [file-data] @@ -354,14 +354,14 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-copies-of-detached (fn [file-data] ;; Find any copy that is referencing a detached shape inside a component, and ;; undo the nested copy, converting it into a direct copy. (letfn [(fix-container [container] - (update container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals fix-shape)) (fix-shape [shape] (cond-> shape @@ -372,14 +372,14 @@ :component-root)))] (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-path-copies (fn [file-data] ;; If the user has created a copy and then converted into a path, detach it ;; because the synchronization will no longer work. (letfn [(fix-container [container] - (update container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals fix-shape)) (fix-shape [shape] (if (and (ctk/instance-head? shape) @@ -389,14 +389,14 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) transform-to-frames (fn [file-data] ;; Transform component and copy heads to frames, and set the ;; frame-id of its childrens (letfn [(fix-container [container] - (update container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals fix-shape)) (fix-shape [shape] (if (or (nil? (:parent-id shape)) (ctk/instance-head? shape)) @@ -410,7 +410,7 @@ shape))] (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) remap-frame-ids (fn [file-data] @@ -418,7 +418,7 @@ ;; to point to the head instance. (letfn [(fix-container [container] - (update container :objects update-vals (partial fix-shape container))) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] @@ -428,14 +428,14 @@ shape)))] (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-frame-ids (fn [file-data] ;; Ensure that frame-id of all shapes point to the parent or to the frame-id ;; of the parent, and that the destination is indeed a frame. (letfn [(fix-container [container] - (update container :objects #(cfh/reduce-objects % fix-shape %))) + (d/update-when container :objects #(cfh/reduce-objects % fix-shape %))) (fix-shape [objects shape] (let [parent (when (:parent-id shape) @@ -452,7 +452,7 @@ (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container)))) + (d/update-when :components update-vals fix-container)))) fix-component-nil-objects (fn [file-data] @@ -464,14 +464,14 @@ (dissoc component :objects)) component))] (-> file-data - (update :components update-vals fix-component)))) + (d/update-when :components update-vals fix-component)))) fix-false-copies (fn [file-data] ;; Find component heads that are not main-instance but have not :shape-ref. (letfn [(fix-container [container] - (update container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals fix-shape)) (fix-shape [shape] @@ -482,7 +482,7 @@ shape))] (-> file-data (update :pages-index update-vals fix-container) - (update :components update-vals fix-container))))] + (d/update-when :components update-vals fix-container))))] (-> file-data (remove-missing-children) From 02cb75209c78172d3bfbc2cbeddb8ca265142948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 14:30:06 +0100 Subject: [PATCH 27/54] :lipstick: Unify source code style of repair functions --- backend/src/app/features/components_v2.clj | 146 ++++++++++----------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 47aea68fdd..f25d28e21d 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -138,101 +138,101 @@ fix-missing-image-metadata (fn [file-data] - (let [update-object - (fn [objects id shape] - (if (and (cfh/image-shape? shape) - (nil? (:metadata shape))) - (-> objects - (dissoc id) - (d/update-in-when [(:parent-id shape) :shapes] - (fn [shapes] (filterv #(not= id %) shapes)))) - objects)) + ;; Delete broken image shapes with no metadata. + (letfn [(fix-container + [container] + (d/update-when container :objects #(reduce-kv fix-shape % %))) - update-page - (fn [page] - (d/update-when page :objects #(reduce-kv update-object % %)))] + (fix-shape + [objects id shape] + (if (and (cfh/image-shape? shape) + (nil? (:metadata shape))) + (-> objects + (dissoc id) + (d/update-in-when [(:parent-id shape) :shapes] + (fn [shapes] (filterv #(not= id %) shapes)))) + objects))] (-> file-data - (update :pages-index update-vals update-page) - (d/update-when :components update-vals update-page)))) + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) - ;; At some point in time, we had a bug that generated shapes - ;; with huge geometries that did not validate the - ;; schema. Since we don't have a way to fix those shapes, we - ;; simply proceed to delete it. We ignore path type shapes - ;; because they have not been affected by the bug. - fix-big-invalid-shapes + delete-big-geometry-shapes (fn [file-data] - (let [update-object - (fn [objects id shape] - (cond - (or (cfh/path-shape? shape) - (cfh/bool-shape? shape)) - objects + ;; At some point in time, we had a bug that generated shapes + ;; with huge geometries that did not validate the + ;; schema. Since we don't have a way to fix those shapes, we + ;; simply proceed to delete it. We ignore path type shapes + ;; because they have not been affected by the bug. + (letfn [(fix-container + [container] + (d/update-when container :objects #(reduce-kv fix-shape % %))) - (or (and (number? (:x shape)) (not (sm/valid-safe-number? (:x shape)))) - (and (number? (:y shape)) (not (sm/valid-safe-number? (:y shape)))) - (and (number? (:width shape)) (not (sm/valid-safe-number? (:width shape)))) - (and (number? (:height shape)) (not (sm/valid-safe-number? (:height shape))))) - (-> objects - (dissoc id) - (d/update-in-when [(:parent-id shape) :shapes] - (fn [shapes] (filterv #(not= id %) shapes)))) + (fix-shape + [objects id shape] + (cond + (or (cfh/path-shape? shape) + (cfh/bool-shape? shape)) + objects - :else - objects)) + (or (and (number? (:x shape)) (not (sm/valid-safe-number? (:x shape)))) + (and (number? (:y shape)) (not (sm/valid-safe-number? (:y shape)))) + (and (number? (:width shape)) (not (sm/valid-safe-number? (:width shape)))) + (and (number? (:height shape)) (not (sm/valid-safe-number? (:height shape))))) + (-> objects + (dissoc id) + (d/update-in-when [(:parent-id shape) :shapes] + (fn [shapes] (filterv #(not= id %) shapes)))) - update-page - (fn [page] - (d/update-when page :objects #(reduce-kv update-object % %)))] + :else + objects))] (-> file-data - (update :pages-index update-vals update-page) - (d/update-when :components update-vals update-page)))) + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) fix-misc-shape-issues (fn [file-data] - ;; Find shapes that are not listed in their parent's children list. - ;; Remove them, and also their children - (let [update-shape - (fn [shape] - (cond-> shape - ;; Some shapes has invalid value there - (contains? shape :layout-gap) - (d/update-in-when [:layout-gap :column-gap] - (fn [gap] - (if (or (= gap ##Inf) - (= gap ##-Inf)) - 0 - gap))) + (letfn [(fix-container + [container] + (d/update-when container :objects update-vals fix-shape)) - ;; Fix broken fills - (seq (:fills shape)) - (update :fills (fn [fills] (filterv valid-fill? fills))) + (fix-shape + [shape] + (cond-> shape + ;; Some shapes has invalid gap value + (contains? shape :layout-gap) + (d/update-in-when [:layout-gap :column-gap] + (fn [gap] + (if (or (= gap ##Inf) + (= gap ##-Inf)) + 0 + gap))) - ;; Fix broken strokes - (seq (:strokes shape)) - (update :strokes (fn [strokes] (filterv valid-stroke? strokes))) + ;; Fix broken fills + (seq (:fills shape)) + (update :fills (fn [fills] (filterv valid-fill? fills))) - ;; Fix some broken layout related attrs, probably - ;; of copypaste on flex layout betatest period - (true? (:layout shape)) - (assoc :layout :flex) + ;; Fix broken strokes + (seq (:strokes shape)) + (update :strokes (fn [strokes] (filterv valid-stroke? strokes))) - (number? (:layout-gap shape)) - (as-> shape (let [n (:layout-gap shape)] - (assoc shape :layout-gap {:row-gap n :column-gap n}))))) + ;; Fix some broken layout related attrs, probably + ;; of copypaste on flex layout betatest period + (true? (:layout shape)) + (assoc :layout :flex) - update-container - (fn [container] - (d/update-when container :objects update-vals update-shape))] + (number? (:layout-gap shape)) + (as-> shape (let [n (:layout-gap shape)] + (assoc shape :layout-gap {:row-gap n :column-gap n})))))] (-> file-data - (update :pages-index update-vals update-container) - (d/update-when :components update-vals update-container)))) + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) fix-recent-colors (fn [file-data] + ;; Remove invalid colors in :recent-colors (d/update-when file-data :recent-colors (fn [colors] (filterv valid-color? colors)))) @@ -489,7 +489,7 @@ (fix-misc-shape-issues) (fix-recent-colors) (fix-missing-image-metadata) - (fix-big-invalid-shapes) + (delete-big-geometry-shapes) (fix-orphan-shapes) (fix-orphan-copies) (remove-nested-roots) From d69db0b337f4427e61edfa54fee0307568f107aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 15:07:32 +0100 Subject: [PATCH 28/54] :bug: Add one more validation fix in migration --- backend/src/app/features/components_v2.clj | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index f25d28e21d..22ea75c3d2 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -237,6 +237,29 @@ (fn [colors] (filterv valid-color? colors)))) + fix-broken-parents + (fn [file-data] + ;; Find children shapes whose parent-id is not set to the parent that contains them. + ;; Remove them from the parent :shapes list. + (letfn [(fix-container + [container] + (d/update-when container :objects #(reduce-kv fix-shape % %))) + + (fix-shape + [objects id shape] + (reduce (fn [objects child-id] + (let [child (get objects child-id)] + (cond-> objects + (and (some? child) (not= id (:parent-id child))) + (d/update-in-when [id :shapes] + (fn [shapes] (filterv #(not= child-id %) shapes)))))) + objects + (:shapes shape)))] + + (-> file-data + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) + fix-orphan-shapes (fn [file-data] ;; Find shapes that are not listed in their parent's children list. @@ -490,6 +513,7 @@ (fix-recent-colors) (fix-missing-image-metadata) (delete-big-geometry-shapes) + (fix-broken-parents) (fix-orphan-shapes) (fix-orphan-copies) (remove-nested-roots) From 00e894d801dc3b838a1d5664e70d13cd00b55bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 15:23:59 +0100 Subject: [PATCH 29/54] :bug: Add validation fix for duplicated children --- backend/src/app/features/components_v2.clj | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 22ea75c3d2..e4c18a2617 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -118,9 +118,9 @@ (vswap! detached-ids conj (:id shape))) (ctk/detach-shape shape))) - remove-missing-children + fix-bad-children (fn [file-data] - ;; Remove any child that does not exist. + ;; Remove any child that does not exist. And also remove duplicated children. (letfn [(fix-container [container] (d/update-when container :objects update-vals (partial fix-shape container))) @@ -130,7 +130,9 @@ (let [objects (:objects container)] (d/update-when shape :shapes (fn [shapes] - (d/removev #(nil? (get objects %)) shapes)))))] + (->> shapes + (d/removev #(nil? (get objects %))) + (into [] (distinct)))))))] (-> file-data (update :pages-index update-vals fix-container) @@ -508,7 +510,7 @@ (d/update-when :components update-vals fix-container))))] (-> file-data - (remove-missing-children) + (fix-bad-children) (fix-misc-shape-issues) (fix-recent-colors) (fix-missing-image-metadata) From db215254852410349f28f2e69961829c98b8a2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 23 Jan 2024 16:35:54 +0100 Subject: [PATCH 30/54] :bug: Add validation check for duplicated children --- common/src/app/common/files/repair.cljc | 13 +++++++++++++ common/src/app/common/files/validate.cljc | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 8aead05bc2..4a824682e9 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -66,6 +66,19 @@ (pcb/with-file-data file-data) (pcb/update-shapes [(:parent-id shape)] repair-shape)))) +(defmethod repair-error :duplicated-children + [_ {:keys [shape page-id] :as error} file-data _] + (let [repair-shape + (fn [shape] + ; Remove duplicated + (log/debug :hint " -> remove duplicated children") + (update shape :shapes distinct))] + + (log/dbg :hint "repairing shape :duplicated-children" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) + (defmethod repair-error :child-not-found [_ {:keys [shape page-id args] :as error} file-data _] (let [repair-shape diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index ca4d471cb4..01373f93c8 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -26,6 +26,7 @@ #{:invalid-geometry :parent-not-found :child-not-in-parent + :duplicated-children :child-not-found :frame-not-found :invalid-frame @@ -123,6 +124,11 @@ (str/ffmt "Shape % not in parent's children list" (:id shape)) shape file page))) + (when-not (= (count (:shapes shape)) (count (distinct (:shapes shape)))) + (report-error :duplicated-children + (str/ffmt "Shape % has duplicated children" (:id shape)) + shape file page)) + (doseq [child-id (:shapes shape)] (let [child (ctst/get-shape page child-id)] (if (nil? child) From 0d33779c955501a0f7968b4ab4aa86c696b300fa Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sat, 20 Jan 2024 22:25:05 +0100 Subject: [PATCH 31/54] :sparkles: Add support for reporting and partitions on comp-v2 migration code --- backend/src/app/features/components_v2.clj | 38 ++-- backend/src/app/srepl/components_v2.clj | 199 ++++++++++++++++----- common/src/app/common/uuid.cljc | 9 + 3 files changed, 182 insertions(+), 64 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index e4c18a2617..2977a475f7 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -1104,7 +1104,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn migrate-file! - [system file-id & {:keys [validate? skip-on-graphic-error?]}] + [system file-id & {:keys [validate? skip-on-graphic-error? label]}] (let [tpoint (dt/tpoint)] (binding [*file-stats* (atom {}) *skip-on-graphic-error* skip-on-graphic-error?] @@ -1119,7 +1119,9 @@ (fn [system] (try (binding [*system* system] - (fsnap/take-file-snapshot! system {:file-id file-id :label "migration/components-v2"}) + (when (string? label) + (fsnap/take-file-snapshot! system {:file-id file-id + :label (str "migration/" label)})) (process-file system file-id :validate? validate?)) (catch Throwable cause @@ -1145,7 +1147,7 @@ (some-> *team-stats* (swap! update :processed/files (fnil inc 0))))))))) (defn migrate-team! - [system team-id & {:keys [validate? skip-on-graphic-error?]}] + [system team-id & {:keys [validate? skip-on-graphic-error? label]}] (l/dbg :hint "migrate:team:start" :team-id (dm/str team-id)) @@ -1156,30 +1158,30 @@ migrate-file (fn [system file-id] (migrate-file! system file-id + :label label :validate? validate? :skip-on-graphics-error? skip-on-graphic-error?)) migrate-team - (fn [{:keys [::db/conn] :as system} {:keys [id features] :as team}] - (let [features (-> features - (disj "ephimeral/v2-migration") - (conj "components/v2") - (conj "layout/grid") - (conj "styles/v2"))] + (fn [{:keys [::db/conn] :as system} team-id] + (let [{:keys [id features]} (get-team system team-id)] + (if (contains? features "components/v2") + (l/inf :hint "team already migrated") + (let [features (-> features + (disj "ephimeral/v2-migration") + (conj "components/v2") + (conj "layout/grid") + (conj "styles/v2"))] - (run! (partial migrate-file system) - (get-and-lock-files conn id)) + (run! (partial migrate-file system) + (get-and-lock-files conn id)) - (update-team-features! conn id features)))] + (update-team-features! conn id features)))))] (binding [*team-stats* (atom {}) *team-id* team-id] (try - (db/tx-run! system (fn [system] - (db/exec-one! system ["SET idle_in_transaction_session_timeout = 0"]) - (let [team (get-team system team-id)] - (if (contains? (:features team) "components/v2") - (l/inf :hint "team already migrated") - (migrate-team system team))))) + (db/tx-run! system migrate-team team-id) + (catch Throwable cause (vreset! err true) (throw cause)) diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index ddc3207014..5cc30fadfa 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -6,8 +6,10 @@ (ns app.srepl.components-v2 (:require + [app.common.data :as d] [app.common.logging :as l] [app.common.pprint :as pp] + [app.common.uuid :as uuid] [app.db :as db] [app.features.components-v2 :as feat] [app.main :as main] @@ -57,13 +59,15 @@ :completed completed :elapsed elapsed))))) -(def ^:private sql:get-teams-1 - "SELECT id, features - FROM team - WHERE deleted_at IS NULL - ORDER BY created_at DESC") +(def ^:private sql:get-teams-by-created-at + "WITH teams AS ( + SELECT id, features + FROM team + WHERE deleted_at IS NULL + ORDER BY created_at DESC + ) SELECT * FROM TEAMS %(pred)s") -(def ^:private sql:get-teams-2 +(def ^:private sql:get-teams-by-graphics "WITH teams AS ( SELECT t.id, t.features, (SELECT count(*) @@ -74,9 +78,37 @@ AND fmo.mtype = 'image/svg+xml' AND fmo.is_local = false) AS graphics FROM team AS t + WHERE t.deleted_at IS NULL ORDER BY 3 ASC ) - SELECT * FROM teams ") + SELECT * FROM teams %(pred)s") + +(def ^:private sql:get-teams-by-activity + "WITH teams AS ( + SELECT t.id, t.features, + (SELECT coalesce(max(date_trunc('month', f.modified_at)), date_trunc('month', t.modified_at)) + FROM file AS f + JOIN project AS p ON (f.project_id = p.id) + WHERE p.team_id = t.id) AS updated_at, + (SELECT coalesce(count(*), 0) + FROM file AS f + JOIN project AS p ON (f.project_id = p.id) + WHERE p.team_id = t.id) AS total_files + FROM team AS t + WHERE t.deleted_at IS NULL + ORDER BY 3 DESC, 4 DESC + ) + SELECT * FROM teams %(pred)s") + +(def ^:private sql:get-teams-by-report + "WITH teams AS ( + SELECT t.id t.features, mr.name + FROM migration_report AS mr + JOIN team AS t ON (t.id = mr.team_id) + WHERE t.deleted_at IS NULL + AND mr.error IS NOT NULL + ORDER BY mr.created_at + ) SELECT id, features FROM teams %(pred)s") (defn- read-pred [entries] @@ -103,26 +135,62 @@ (apply vector sql params)))))) (defn- get-teams - [conn pred] - (let [sql (if pred - (let [[sql & params] (read-pred pred)] - (apply vector (str sql:get-teams-2 sql) params)) - [sql:get-teams-1])] + [conn query pred] + (let [query (d/nilv query :created-at) + sql (case query + :created-at sql:get-teams-by-created-at + :activity sql:get-teams-by-activity + :graphics sql:get-teams-by-graphics + :report sql:get-teams-by-report) - (->> (db/cursor conn sql) + sql (if pred + (let [[pred-sql & pred-params] (read-pred pred)] + (apply vector + (str/format sql {:pred pred-sql}) + pred-params)) + [(str/format sql {:pred ""})])] + + (->> (db/cursor conn sql {:chunk-size 500}) (map feat/decode-row) (remove (fn [{:keys [features]}] - (or (contains? features "ephimeral/v2-migration") - (contains? features "components/v2")))) + (contains? features "components/v2"))) (map :id)))) +(def ^:private sql:report-table + "CREATE UNLOGGED TABLE IF NOT EXISTS migration_report ( + id bigserial NOT NULL, + label text NOT NULL, + team_id UUID NOT NULL, + error text NULL, + created_at timestamptz NOT NULL DEFAULT now(), + elapsed bigint NOT NULL, + PRIMARY KEY (label, created_at, id) + )") + +(defn- create-report-table! + [system] + (db/exec-one! system [sql:report-table])) + +(defn- clean-reports! + [system label] + (db/delete! system :migration-report {:label label})) + +(defn- report! + [system team-id label elapsed error] + (db/insert! system :migration-report + {:label label + :team-id team-id + :elapsed (inst-ms elapsed) + :error error} + {::db/return-keys false})) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PUBLIC API ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn migrate-file! - [file-id & {:keys [rollback? validate?] :or {rollback? true validate? false}}] + [file-id & {:keys [rollback? validate? label] :or {rollback? true validate? false}}] (l/dbg :hint "migrate:start" :rollback rollback?) (let [tpoint (dt/tpoint) file-id (if (string? file-id) @@ -131,7 +199,9 @@ (binding [feat/*stats* (atom {})] (try (-> (assoc main/system ::db/rollback rollback?) - (feat/migrate-file! file-id :validate? validate?)) + (feat/migrate-file! file-id + :validate? validate? + :label label)) (-> (deref feat/*stats*) (assoc :elapsed (dt/format-duration (tpoint)))) @@ -144,7 +214,7 @@ (l/dbg :hint "migrate:end" :rollback rollback? :elapsed elapsed))))))) (defn migrate-team! - [team-id & {:keys [rollback? skip-on-graphic-error? validate?] + [team-id & {:keys [rollback? skip-on-graphic-error? validate? label] :or {rollback? true validate? true skip-on-graphic-error? false}}] @@ -163,6 +233,7 @@ (try (-> (assoc main/system ::db/rollback rollback?) (feat/migrate-team! team-id + :label label :validate? validate? :skip-on-graphics-error? skip-on-graphic-error?)) (print-stats! @@ -181,17 +252,29 @@ This function starts multiple concurrent team migration processes until thw maximum number of jobs is reached which by default has the - value of `1`. This is controled with the `:max-jobs` option." + value of `1`. This is controled with the `:max-jobs` option. - [& {:keys [max-jobs max-items max-time rollback? validate? + If you want to run this on multiple machines you will need to specify + the total number of partitions and the current partition. + + In order to get the report table populated, you will need to provide + a correct `:label`. That label is also used for persist a file + snaphot before continue with the migration." + [& {:keys [max-jobs max-items max-time rollback? validate? query pred max-procs cache on-start on-progress on-error on-end - skip-on-graphic-error?] + skip-on-graphic-error? label partitions current-partition] :or {validate? false rollback? true max-jobs 1 skip-on-graphic-error? true max-items Long/MAX_VALUE}}] + (when (int? partitions) + (when-not (int? current-partition) + (throw (IllegalArgumentException. "missing `current-partition` parameter"))) + (when-not (<= 0 current-partition partitions) + (throw (IllegalArgumentException. "invalid value on `current-partition` parameter")))) + (let [stats (atom {}) tpoint (dt/tpoint) mtime (some-> max-time dt/duration) @@ -207,8 +290,32 @@ (cache/create :executor :same-thread :max-items cache) nil) - migrate-team + (fn [team-id] + (let [tpoint (dt/tpoint)] + (try + (db/tx-run! (assoc main/system ::db/rollback rollback?) + (fn [system] + (db/exec-one! system ["SET idle_in_transaction_session_timeout = 0"]) + (feat/migrate-team! system team-id + :label label + :validate? validate? + :skip-on-graphics-error? skip-on-graphic-error?))) + + (when (string? label) + (report! main/system label team-id (tpoint) nil)) + + (catch Throwable cause + (l/wrn :hint "unexpected error on processing team (skiping)" + :team-id (str team-id) + :cause cause) + (when (string? label) + (report! main/system label team-id (tpoint) (ex-message cause)))) + + (finally + (ps/release! sjobs))))) + + process-team (fn [team-id] (ps/acquire! sjobs) (let [ts (tpoint)] @@ -220,25 +327,15 @@ (ps/release! sjobs) (reduced nil)) - (px/run! executor (fn [] - (try - (-> (assoc main/system ::db/rollback rollback?) - (feat/migrate-team! team-id - :validate? validate? - :skip-on-graphics-error? skip-on-graphic-error?)) - - (catch Throwable cause - (l/wrn :hint "unexpected error on processing team (skiping)" - :team-id (str team-id) - :cause cause)) - - (finally - (ps/release! sjobs))))))))] + (px/run! executor (partial migrate-team team-id)))))] (l/dbg :hint "migrate:start" + :label label :rollback rollback? :max-jobs max-jobs - :max-items max-items) + :max-items max-items + :partitions partitions + :current-partition current-partition) (add-watch stats :progress-report (report-progress-teams tpoint on-progress)) @@ -249,16 +346,26 @@ (when (fn? on-start) (on-start {:rollback rollback?})) - (db/tx-run! main/system - (fn [{:keys [::db/conn]}] - (db/exec! conn ["SET statement_timeout = 0;"]) - (db/exec! conn ["SET idle_in_transaction_session_timeout = 0;"]) - (run! migrate-team - (->> (get-teams conn pred) - (take max-items))))) + (when (string? label) + (create-report-table! main/system) + (clean-reports! main/system label)) - ;; Close and await tasks - (pu/close! executor) + (db/tx-run! main/system + (fn [{:keys [::db/conn] :as system}] + (db/exec! conn ["SET statement_timeout = 0"]) + (db/exec! conn ["SET idle_in_transaction_session_timeout = 0"]) + + (run! process-team + (->> (get-teams conn query pred) + (take max-items) + (filter (fn [team-id] + (if (and (int? current-partition) (int? partitions)) + (let [partition (mod (uuid/hash-int team-id) partitions)] + (= (dec current-partition) partition)) + true))))) + + ;; Close and await tasks + (pu/close! executor))) (if (fn? on-end) (-> (deref stats) diff --git a/common/src/app/common/uuid.cljc b/common/src/app/common/uuid.cljc index b205c64534..2086a0a5bd 100644 --- a/common/src/app/common/uuid.cljc +++ b/common/src/app/common/uuid.cljc @@ -75,3 +75,12 @@ with base62. It is only safe to use with uuid v4 and penpot custom v8" [id] (impl/short-v8 (dm/str id)))) + +#?(:clj + (defn hash-int + [id] + (let [a (.getMostSignificantBits ^UUID id) + b (.getLeastSignificantBits ^UUID id)] + (+ (clojure.lang.Murmur3/hashLong a) + (clojure.lang.Murmur3/hashLong b))))) + From 4ab4ad96f08d7038700b8d7dd5e17ec744e24f44 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 22 Jan 2024 16:36:23 +0100 Subject: [PATCH 32/54] :bug: Resolve objects-map on srepl/get-file helpers --- backend/src/app/srepl/helpers.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/app/srepl/helpers.clj b/backend/src/app/srepl/helpers.clj index f1a267e48d..5e2a26ee96 100644 --- a/backend/src/app/srepl/helpers.clj +++ b/backend/src/app/srepl/helpers.clj @@ -69,7 +69,8 @@ (fn [system] (binding [pmap/*load-fn* (partial feat.fdata/load-pointer system id)] (-> (files/get-file system id :migrate? migrate?) - (update :data feat.fdata/process-pointers deref)))))) + (update :data feat.fdata/process-pointers deref) + (update :data feat.fdata/process-objects (partial into {}))))))) (defn validate "Validate structure, referencial integrity and semantic coherence of From aaeb8c88686bfb30a12700805678469b3226bc3c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 14:09:52 +0100 Subject: [PATCH 33/54] :bug: Fix components with bool shape as root on comp-v2 migration --- backend/src/app/features/components_v2.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 2977a475f7..266f579b0a 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -408,7 +408,8 @@ (fix-shape [shape] (if (and (ctk/instance-head? shape) - (cfh/path-shape? shape)) + (or (cfh/path-shape? shape) + (cfh/bool-shape? shape))) (ctk/detach-shape shape) shape))] From e4f4ab92217ce328d375d2379344715b2fd6d6cb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 14:34:04 +0100 Subject: [PATCH 34/54] :bug: Fix invalid page flows on comp-v2 migration --- backend/src/app/features/components_v2.clj | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 266f579b0a..78b02da328 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -29,6 +29,7 @@ [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] + [app.common.types.page :as ctp] [app.common.types.pages-list :as ctpl] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] @@ -100,15 +101,16 @@ ;; FILE PREPARATION BEFORE MIGRATION ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(def valid-color? (sm/lazy-validator ::ctc/recent-color)) +(def valid-fill? (sm/lazy-validator ::cts/fill)) +(def valid-stroke? (sm/lazy-validator ::cts/stroke)) +(def valid-flow? (sm/lazy-validator ::ctp/flow)) + (defn- prepare-file-data "Apply some specific migrations or fixes to things that are allowed in v1 but not in v2, or that are the result of old bugs." [file-data libraries] (let [detached-ids (volatile! #{}) - valid-color? (sm/validator ::ctc/recent-color) - valid-fill? (sm/validator ::cts/fill) - valid-stroke? (sm/validator ::cts/stroke) - detach-shape (fn [container shape] ;; Detach a shape. If it's inside a component, add it to detached-ids, for further use. @@ -159,6 +161,14 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) + ;; Some pages has invalid data on flows, we proceed just to + ;; delete them. + delete-invalid-flows + (fn [file-data] + (update file-data :pages-index update-vals + (fn [page] + (d/update-in-when page [:options :flows] #(filterv valid-flow? %))))) + delete-big-geometry-shapes (fn [file-data] ;; At some point in time, we had a bug that generated shapes @@ -511,6 +521,7 @@ (d/update-when :components update-vals fix-container))))] (-> file-data + (delete-invalid-flows) (fix-bad-children) (fix-misc-shape-issues) (fix-recent-colors) From 2950259f971f017f0b50755fffa283f2d6d7dc9a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 15:07:50 +0100 Subject: [PATCH 35/54] :bug: Fix invalid text shapes with invalid nodes --- backend/src/app/features/components_v2.clj | 6 ++---- common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 23 +++++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 78b02da328..422ef171a8 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -205,12 +205,10 @@ fix-misc-shape-issues (fn [file-data] - (letfn [(fix-container - [container] + (letfn [(fix-container [container] (d/update-when container :objects update-vals fix-shape)) - (fix-shape - [shape] + (fix-shape [shape] (cond-> shape ;; Some shapes has invalid gap value (contains? shape :layout-gap) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index bb448f9975..08a590111e 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 42) +(def version 43) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 1cc3f4c8b6..d2213168cb 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -19,6 +19,7 @@ [app.common.geom.shapes.text :as gsht] [app.common.logging :as l] [app.common.math :as mth] + [app.common.schema :as sm] [app.common.svg :as csvg] [app.common.text :as txt] [app.common.types.shape :as cts] @@ -780,3 +781,25 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) + +(def ^:private valid-fill? + (sm/lazy-validator ::cts/fill)) + +(defmethod migrate 43 + [data] + (letfn [(update-text-node [node] + (-> node + (d/update-when :fills #(filterv valid-fill? %)) + (d/without-nils))) + + (update-object [object] + (if (cfh/text-shape? object) + (update object :content #(txt/transform-nodes identity update-text-node %)) + object)) + + (update-container [container] + (d/update-when container :objects update-vals update-object))] + + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) From 3b929041f2d0525344e8db2340afd614b2d3d66d Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 15:22:01 +0100 Subject: [PATCH 36/54] :bug: Fix incorrect percent number parsing on reading svg --- common/src/app/common/svg.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/app/common/svg.cljc b/common/src/app/common/svg.cljc index b91e4726f2..cd7ecf2986 100644 --- a/common/src/app/common/svg.cljc +++ b/common/src/app/common/svg.cljc @@ -980,7 +980,7 @@ (fix-percent-attr-numeric [_ attr-val] (let [is-percent? (str/ends-with? attr-val "%")] (if is-percent? - (str (let [attr-num (d/parse-double attr-val)] + (str (let [attr-num (d/parse-double (str/rtrim attr-val "%"))] (/ attr-num 100))) attr-val))) From 3b0d654b6d616676cc5042abe9fb5ae769937282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Wed, 24 Jan 2024 15:39:25 +0100 Subject: [PATCH 37/54] :lipstick: Review naming and comments --- backend/src/app/features/components_v2.clj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 422ef171a8..e38450f764 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -407,10 +407,10 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) - fix-path-copies + fix-converted-copies (fn [file-data] - ;; If the user has created a copy and then converted into a path, detach it - ;; because the synchronization will no longer work. + ;; If the user has created a copy and then converted into a path or bool, + ;; detach it because the synchronization will no longer work. (letfn [(fix-container [container] (d/update-when container :objects update-vals fix-shape)) @@ -532,7 +532,7 @@ (add-not-nested-roots) (remap-refs) (fix-copies-of-detached) - (fix-path-copies) + (fix-converted-copies) (transform-to-frames) (remap-frame-ids) (fix-frame-ids) From cceb35b0534a95378cfbe271da817917ed852631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Wed, 24 Jan 2024 15:56:31 +0100 Subject: [PATCH 38/54] :bug: Ensure detach in migration fixes always works --- backend/src/app/features/components_v2.clj | 58 ++++++++++++---------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index e38450f764..d2cc547ed9 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -113,7 +113,8 @@ (let [detached-ids (volatile! #{}) detach-shape (fn [container shape] - ;; Detach a shape. If it's inside a component, add it to detached-ids, for further use. + ;; Detach a shape. If it's inside a component, add it to detached-ids. This list + ;; is used later to process any other copy that was referencing a detached copy. (let [is-component? (let [root-shape (ctst/get-shape container (:id container))] (and (some? root-shape) (nil? (:parent-id root-shape))))] (when is-component? @@ -389,36 +390,18 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) - fix-copies-of-detached - (fn [file-data] - ;; Find any copy that is referencing a detached shape inside a component, and - ;; undo the nested copy, converting it into a direct copy. - (letfn [(fix-container [container] - (d/update-when container :objects update-vals fix-shape)) - - (fix-shape [shape] - (cond-> shape - (@detached-ids (:shape-ref shape)) - (dissoc shape - :component-id - :component-file - :component-root)))] - (-> file-data - (update :pages-index update-vals fix-container) - (d/update-when :components update-vals fix-container)))) - fix-converted-copies (fn [file-data] ;; If the user has created a copy and then converted into a path or bool, ;; detach it because the synchronization will no longer work. (letfn [(fix-container [container] - (d/update-when container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals (partial fix-shape container))) - (fix-shape [shape] + (fix-shape [container shape] (if (and (ctk/instance-head? shape) (or (cfh/path-shape? shape) (cfh/bool-shape? shape))) - (ctk/detach-shape shape) + (detach-shape container shape) shape))] (-> file-data @@ -505,15 +488,36 @@ ;; Find component heads that are not main-instance but have not :shape-ref. (letfn [(fix-container [container] - (d/update-when container :objects update-vals fix-shape)) + (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape - [shape] + [container shape] (if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)) (not (ctk/in-component-copy? shape))) - (ctk/detach-shape shape) + (detach-shape container shape) shape))] + (-> file-data + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) + + fix-copies-of-detached + (fn [file-data] + ;; Find any copy that is referencing a shape inside a component that have + ;; been detached in a previous fix. If so, undo the nested copy, converting + ;; it into a direct copy. + ;; + ;; WARNING: THIS SHOULD BE CALLED AT THE END OF THE PROCESS. + (letfn [(fix-container [container] + (d/update-when container :objects update-vals fix-shape)) + + (fix-shape [shape] + (cond-> shape + (@detached-ids (:shape-ref shape)) + (dissoc shape + :component-id + :component-file + :component-root)))] (-> file-data (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container))))] @@ -531,13 +535,13 @@ (remove-nested-roots) (add-not-nested-roots) (remap-refs) - (fix-copies-of-detached) (fix-converted-copies) (transform-to-frames) (remap-frame-ids) (fix-frame-ids) (fix-component-nil-objects) - (fix-false-copies)))) + (fix-false-copies) + (fix-copies-of-detached)))) ; <- Do not add fixes after this one ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; COMPONENTS MIGRATION From 17a208d67bfa2e0eb0c649116d9a7c94a603749d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Wed, 24 Jan 2024 15:59:39 +0100 Subject: [PATCH 39/54] :bug: Add validation fix for false non root copies --- backend/src/app/features/components_v2.clj | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index d2cc547ed9..c09e7251de 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -486,15 +486,18 @@ fix-false-copies (fn [file-data] ;; Find component heads that are not main-instance but have not :shape-ref. + ;; Also shapes that have :shape-ref but are not in a copy. (letfn [(fix-container [container] (d/update-when container :objects update-vals (partial fix-shape container))) (fix-shape [container shape] - (if (and (ctk/instance-head? shape) - (not (ctk/main-instance? shape)) - (not (ctk/in-component-copy? shape))) + (if (or (and (ctk/instance-head? shape) + (not (ctk/main-instance? shape)) + (not (ctk/in-component-copy? shape))) + (and (ctk/in-component-copy? shape) + (nil? (ctn/get-head-shape (:objects container) shape {:allow-main? true})))) (detach-shape container shape) shape))] (-> file-data From 8d0afd8c9682d6fb0114add5d7da4469da557910 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 17:42:02 +0100 Subject: [PATCH 40/54] :bug: Add migration for fix invalid shadows --- backend/src/app/util/time.clj | 1 - common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 26 +++++++++++ common/src/app/common/schema.cljc | 4 +- common/src/app/common/time.cljc | 45 ++++++++++++++++--- common/src/app/common/types/color.cljc | 26 ++++++----- common/src/app/common/types/shape/shadow.cljc | 12 +---- 7 files changed, 86 insertions(+), 30 deletions(-) diff --git a/backend/src/app/util/time.clj b/backend/src/app/util/time.clj index d3611d71e6..7785966245 100644 --- a/backend/src/app/util/time.clj +++ b/backend/src/app/util/time.clj @@ -123,7 +123,6 @@ FileTime (inst-ms* [v] (.toMillis ^FileTime v))) - (defmethod print-method Duration [mv ^java.io.Writer writer] (.write writer (str "#app/duration \"" (str/lower (subs (str mv) 2)) "\""))) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 08a590111e..e35914d73f 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 43) +(def version 44) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index d2213168cb..39d448597c 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -23,6 +23,7 @@ [app.common.svg :as csvg] [app.common.text :as txt] [app.common.types.shape :as cts] + [app.common.types.shape.shadow :as ctss] [app.common.uuid :as uuid] [cuerdas.core :as str])) @@ -803,3 +804,28 @@ (-> data (update :pages-index update-vals update-container) (update :components update-vals update-container)))) + +(def ^:private valid-shadow? + (sm/lazy-validator ::ctss/shadow)) + +(defmethod migrate 44 + [data] + (letfn [(fix-shadow [shadow] + (if (string? (:color shadow)) + (let [color {:color (:color shadow) + :opacity 1}] + (assoc shadow :color color)) + shadow)) + + (update-object [object] + (d/update-when object :shadow + #(into [] + (comp (map fix-shadow) + (filter valid-shadow?)) + %))) + + (update-container [container] + (d/update-when container :objects update-vals update-object))] + (-> data + (update :pages-index update-vals update-container) + (update :components update-vals update-container)))) diff --git a/common/src/app/common/schema.cljc b/common/src/app/common/schema.cljc index b346f66d29..b1e743f643 100644 --- a/common/src/app/common/schema.cljc +++ b/common/src/app/common/schema.cljc @@ -14,6 +14,7 @@ [app.common.schema.generators :as sg] [app.common.schema.openapi :as-alias oapi] [app.common.schema.registry :as sr] + [app.common.time :as tm] [app.common.uri :as u] [app.common.uuid :as uuid] [clojure.core :as c] @@ -625,7 +626,8 @@ {:title "inst" :description "Satisfies Inst protocol" :error/message "expected to be number in safe range" - :gen/gen (sg/small-int) + :gen/gen (->> (sg/small-int) + (sg/fmap (fn [v] (tm/instant v)))) ::oapi/type "number" ::oapi/format "int64"}}) diff --git a/common/src/app/common/time.cljc b/common/src/app/common/time.cljc index c32c82411e..8b1ead444c 100644 --- a/common/src/app/common/time.cljc +++ b/common/src/app/common/time.cljc @@ -4,16 +4,16 @@ ;; ;; Copyright (c) KALEIDOS INC -;; Here we put the time functions that are common between frontend and backend. -;; In the future we may create an unified API for both. - (ns app.common.time + "A new cross-platform date and time API. It should be prefered over + a platform specific implementation found on `app.util.time`." #?(:cljs (:require ["luxon" :as lxn]) :clj (:import - java.time.Instant))) + java.time.Instant + java.time.Duration))) #?(:cljs (def DateTime lxn/DateTime)) @@ -24,4 +24,39 @@ (defn now [] #?(:clj (Instant/now) - :cljs (.local ^js DateTime))) \ No newline at end of file + :cljs (.local ^js DateTime))) + +(defn instant + [s] + #?(:clj (Instant/ofEpochMilli s) + :cljs (.fromMillis ^js DateTime s #js {:zone "local" :setZone false}))) + +#?(:cljs + (extend-protocol IComparable + DateTime + (-compare [it other] + (if ^boolean (.equals it other) + 0 + (if (< (inst-ms it) (inst-ms other)) -1 1))) + + Duration + (-compare [it other] + (if ^boolean (.equals it other) + 0 + (if (< (inst-ms it) (inst-ms other)) -1 1))))) + +#?(:cljs + (extend-protocol cljs.core/Inst + DateTime + (inst-ms* [inst] (.toMillis ^js inst)) + + Duration + (inst-ms* [inst] (.toMillis ^js inst))) + + :clj + (extend-protocol clojure.core/Inst + Duration + (inst-ms* [v] (.toMillis ^Duration v)) + + Instant + (inst-ms* [v] (.toEpochMilli ^Instant v)))) diff --git a/common/src/app/common/types/color.cljc b/common/src/app/common/types/color.cljc index 8049628941..3a726d77a5 100644 --- a/common/src/app/common/types/color.cljc +++ b/common/src/app/common/types/color.cljc @@ -70,18 +70,20 @@ [:offset ::sm/safe-number]]]]]) (sm/define! ::color - [:map {:title "Color"} - [:id {:optional true} ::sm/uuid] - [:name {:optional true} :string] - [:path {:optional true} [:maybe :string]] - [:value {:optional true} [:maybe :string]] - [:color {:optional true} [:maybe ::rgb-color]] - [:opacity {:optional true} [:maybe ::sm/safe-number]] - [:modified-at {:optional true} ::sm/inst] - [:ref-id {:optional true} ::sm/uuid] - [:ref-file {:optional true} ::sm/uuid] - [:gradient {:optional true} [:maybe ::gradient]] - [:image {:optional true} [:maybe ::image-color]]]) + [:and + [:map {:title "Color"} + [:id {:optional true} ::sm/uuid] + [:name {:optional true} :string] + [:path {:optional true} [:maybe :string]] + [:value {:optional true} [:maybe :string]] + [:color {:optional true} [:maybe ::rgb-color]] + [:opacity {:optional true} [:maybe ::sm/safe-number]] + [:modified-at {:optional true} ::sm/inst] + [:ref-id {:optional true} ::sm/uuid] + [:ref-file {:optional true} ::sm/uuid] + [:gradient {:optional true} [:maybe ::gradient]] + [:image {:optional true} [:maybe ::image-color]]] + [::sm/contains-any {:strict true} [:color :gradient :image]]]) (sm/define! ::recent-color [:and diff --git a/common/src/app/common/types/shape/shadow.cljc b/common/src/app/common/types/shape/shadow.cljc index d04886fa3a..cc2fd81c3c 100644 --- a/common/src/app/common/types/shape/shadow.cljc +++ b/common/src/app/common/types/shape/shadow.cljc @@ -7,8 +7,7 @@ (ns app.common.types.shape.shadow (:require [app.common.schema :as sm] - [app.common.types.color :as ctc] - [app.common.types.shape.shadow.color :as-alias shadow-color])) + [app.common.types.color :as ctc])) (def styles #{:drop-shadow :inner-shadow}) @@ -21,11 +20,4 @@ [:blur ::sm/safe-number] [:spread ::sm/safe-number] [:hidden :boolean] - ;;FIXME: reuse color? - [:color - [:map - [:color {:optional true} :string] - [:opacity {:optional true} ::sm/safe-number] - [:gradient {:optional true} [:maybe ::ctc/gradient]] - [:file-id {:optional true} [:maybe ::sm/uuid]] - [:id {:optional true} [:maybe ::sm/uuid]]]]]) + [:color ::ctc/color]]) From 3f97b3a112789ea09f6af4d210c825b89315ca6c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 13:12:15 +0100 Subject: [PATCH 41/54] :bug: Fix minor issues on migration code --- backend/src/app/features/components_v2.clj | 6 ++++++ backend/src/app/srepl/components_v2.clj | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index c09e7251de..915d08ab6e 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -945,6 +945,12 @@ :file-id (str (:id fdata)) :id (str (:id mobj))) + (instance? org.graalvm.polyglot.PolyglotException cause) + (l/inf :hint "skip processing media object: invalid svg found" + :team-id (str team-id) + :file-id (str (:id fdata)) + :id (str (:id mobj))) + (= (:type edata) :not-found) (l/inf :hint "skip processing media object: underlying object does not exist" :team-id (str team-id) diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index 5cc30fadfa..4b042883f4 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -287,7 +287,7 @@ sprocs (ps/create :permits max-procs) cache (if (int? cache) - (cache/create :executor :same-thread + (cache/create :executor executor :max-items cache) nil) migrate-team @@ -303,14 +303,14 @@ :skip-on-graphics-error? skip-on-graphic-error?))) (when (string? label) - (report! main/system label team-id (tpoint) nil)) + (report! main/system team-id label (tpoint) nil)) (catch Throwable cause (l/wrn :hint "unexpected error on processing team (skiping)" :team-id (str team-id) :cause cause) (when (string? label) - (report! main/system label team-id (tpoint) (ex-message cause)))) + (report! main/system team-id label (tpoint) (ex-message cause)))) (finally (ps/release! sjobs))))) From 3986543293ac680ae111675ab99efe47e41cee08 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 20:34:32 +0100 Subject: [PATCH 42/54] :paperclip: Add missing IEquiv implementation for luxon DateTime type --- common/src/app/common/time.cljc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/src/app/common/time.cljc b/common/src/app/common/time.cljc index 8b1ead444c..6cd8601d66 100644 --- a/common/src/app/common/time.cljc +++ b/common/src/app/common/time.cljc @@ -45,6 +45,14 @@ 0 (if (< (inst-ms it) (inst-ms other)) -1 1))))) + +#?(:cljs + (extend-type DateTime + cljs.core/IEquiv + (-equiv [o other] + (and (instance? DateTime other) + (== (.valueOf o) (.valueOf other)))))) + #?(:cljs (extend-protocol cljs.core/Inst DateTime From 326be0df4f60c994bbdd366d596cbe0ffbfd96b0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Jan 2024 20:55:53 +0100 Subject: [PATCH 43/54] :bug: Fix incorrect type supposition on attr inheritance on parsing svg --- common/src/app/common/svg.cljc | 12 ++++--- common/src/app/common/svg/shapes_builder.cljc | 31 +++++++++---------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/common/src/app/common/svg.cljc b/common/src/app/common/svg.cljc index cd7ecf2986..6e4ba6383a 100644 --- a/common/src/app/common/svg.cljc +++ b/common/src/app/common/svg.cljc @@ -872,17 +872,21 @@ transform (update :transform append-transform)))) -(defn inherit-attributes [group-attrs {:keys [attrs] :as node}] +(defn inherit-attributes + [group-attrs {:keys [attrs] :as node}] (if (map? node) - (let [attrs (-> (format-styles attrs) - (add-transform (:transform group-attrs))) + (let [attrs (-> (format-styles attrs) + (add-transform (:transform group-attrs))) + group-attrs (format-styles group-attrs) ;; Don't inherit a property that is already in the style attribute inherit-style (-> (:style group-attrs) (d/without-keys (keys attrs))) inheritable-props (->> inheritable-props (remove #(contains? (:styles attrs) %))) group-attrs (-> group-attrs (assoc :style inherit-style)) - attrs (d/deep-merge (select-keys group-attrs inheritable-props) attrs)] + attrs (-> (select-keys group-attrs inheritable-props) + (d/deep-merge attrs) + (d/without-nils))] (assoc node :attrs attrs)) node)) diff --git a/common/src/app/common/svg/shapes_builder.cljc b/common/src/app/common/svg/shapes_builder.cljc index 6313939879..c219b5e18e 100644 --- a/common/src/app/common/svg/shapes_builder.cljc +++ b/common/src/app/common/svg/shapes_builder.cljc @@ -537,21 +537,20 @@ :image (create-image-shape name frame-id svg-data element) #_other (create-raw-svg name frame-id svg-data element))] - (when (some? shape) - (let [shape (-> shape - (assoc :svg-defs (select-keys defs references)) - (setup-fill) - (setup-stroke) - (setup-opacity) - (setup-other) - (update :svg-attrs (fn [attrs] - (if (empty? (:style attrs)) - (dissoc attrs :style) - attrs))))] - [(cond-> shape - hidden (assoc :hidden true)) + [(-> shape + (assoc :svg-defs (select-keys defs references)) + (setup-fill) + (setup-stroke) + (setup-opacity) + (setup-other) + (update :svg-attrs (fn [attrs] + (if (empty? (:style attrs)) + (dissoc attrs :style) + attrs))) + (cond-> ^boolean hidden + (assoc :hidden true))) - (cond->> (:content element) - (contains? csvg/parent-tags tag) - (mapv #(csvg/inherit-attributes attrs %)))])))))) + (cond->> (:content element) + (contains? csvg/parent-tags tag) + (mapv (partial csvg/inherit-attributes attrs)))]))))) From 1b3e68f4307db58a676880637ce80de4e1f0b7bd Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 09:38:08 +0100 Subject: [PATCH 44/54] :sparkles: Improve partitioning and graphics error skiping mechanism On the migration functions --- backend/src/app/features/components_v2.clj | 4 ++-- backend/src/app/srepl/components_v2.clj | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 915d08ab6e..03068c6fcc 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -1135,7 +1135,7 @@ (l/dbg :hint "migrate:file:start" :file-id (str file-id) :validate validate? - :skip-on-graphics-error skip-on-graphic-error?) + :skip-on-graphic-error skip-on-graphic-error?) (let [system (update system ::sto/storage media/configure-assets-storage)] (db/tx-run! system @@ -1183,7 +1183,7 @@ (migrate-file! system file-id :label label :validate? validate? - :skip-on-graphics-error? skip-on-graphic-error?)) + :skip-on-graphic-error? skip-on-graphic-error?)) migrate-team (fn [{:keys [::db/conn] :as system} team-id] (let [{:keys [id features]} (get-team system team-id)] diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index 4b042883f4..e972f77961 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -235,7 +235,7 @@ (feat/migrate-team! team-id :label label :validate? validate? - :skip-on-graphics-error? skip-on-graphic-error?)) + :skip-on-graphic-error? skip-on-graphic-error?)) (print-stats! (-> (deref feat/*stats*) (assoc :elapsed (dt/format-duration (tpoint))))) @@ -266,6 +266,7 @@ :or {validate? false rollback? true max-jobs 1 + current-partition 1 skip-on-graphic-error? true max-items Long/MAX_VALUE}}] @@ -300,7 +301,7 @@ (feat/migrate-team! system team-id :label label :validate? validate? - :skip-on-graphics-error? skip-on-graphic-error?))) + :skip-on-graphic-error? skip-on-graphic-error?))) (when (string? label) (report! main/system team-id label (tpoint) nil)) @@ -333,9 +334,7 @@ :label label :rollback rollback? :max-jobs max-jobs - :max-items max-items - :partitions partitions - :current-partition current-partition) + :max-items max-items) (add-watch stats :progress-report (report-progress-teams tpoint on-progress)) @@ -359,9 +358,10 @@ (->> (get-teams conn query pred) (take max-items) (filter (fn [team-id] - (if (and (int? current-partition) (int? partitions)) - (let [partition (mod (uuid/hash-int team-id) partitions)] - (= (dec current-partition) partition)) + (if (int? partitions) + (= current-partition (-> (uuid/hash-int team-id) + (mod partitions) + (inc))) true))))) ;; Close and await tasks From 0d5c1811cfe1d07479bbc6f87eb0ea6bc88828c2 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 09:38:47 +0100 Subject: [PATCH 45/54] :bug: Fix edge cases on retrieving href-id on svg to shapes conversion --- common/src/app/common/svg/shapes_builder.cljc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/common/src/app/common/svg/shapes_builder.cljc b/common/src/app/common/svg/shapes_builder.cljc index c219b5e18e..6e495f7258 100644 --- a/common/src/app/common/svg/shapes_builder.cljc +++ b/common/src/app/common/svg/shapes_builder.cljc @@ -507,8 +507,16 @@ att-refs (csvg/find-attr-references attrs) defs (get svg-data :defs) references (csvg/find-def-references defs att-refs) - href-id (-> (or (:href attrs) (:xlink:href attrs) " ") (subs 1)) - use-tag? (and (= :use tag) (contains? defs href-id))] + + href-id (or (:href attrs) (:xlink:href attrs) " ") + href-id (if (and (string? href-id) + (pos? (count href-id))) + (subs href-id 1) + href-id) + + use-tag? (and (= :use tag) + (some? href-id) + (contains? defs href-id))] (if use-tag? (let [;; Merge the data of the use definition with the properties passed as attributes From e6766bac8f33a47a970d1b20fbe25401dfeac2af Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 09:49:40 +0100 Subject: [PATCH 46/54] :sparkles: Set correct order of filtering teams on migration function --- backend/src/app/srepl/components_v2.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/app/srepl/components_v2.clj b/backend/src/app/srepl/components_v2.clj index e972f77961..5e6a697bb7 100644 --- a/backend/src/app/srepl/components_v2.clj +++ b/backend/src/app/srepl/components_v2.clj @@ -356,13 +356,13 @@ (run! process-team (->> (get-teams conn query pred) - (take max-items) (filter (fn [team-id] (if (int? partitions) (= current-partition (-> (uuid/hash-int team-id) (mod partitions) (inc))) - true))))) + true))) + (take max-items))) ;; Close and await tasks (pu/close! executor))) From 66c07e1336087d28a88e5e3fdf0baabab875af58 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 11:32:45 +0100 Subject: [PATCH 47/54] :sparkles: Reapply again all file migrations on comp-v2 migration --- backend/src/app/features/components_v2.clj | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 03068c6fcc..43af932459 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -1052,6 +1052,10 @@ (update :data assoc :id id) (update :data fdata/process-pointers deref) (update :data fdata/process-objects (partial into {})) + (update :data (fn [data] + (if (> (:version data) 22) + (assoc data :version 22) + data))) (fmg/migrate-file)))) (defn- get-team From df4be5106bffeb724ba427b3ffc938c27e4be13f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 11:55:06 +0100 Subject: [PATCH 48/54] :bug: Fix text shapes wrongly converted to path in comp-v2 migration --- backend/src/app/features/components_v2.clj | 33 +++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 43af932459..d6adeb4777 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -33,6 +33,7 @@ [app.common.types.pages-list :as ctpl] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] + [app.common.types.shape.text :as ctsx] [app.common.uuid :as uuid] [app.db :as db] [app.db.sql :as sql] @@ -106,6 +107,9 @@ (def valid-stroke? (sm/lazy-validator ::cts/stroke)) (def valid-flow? (sm/lazy-validator ::ctp/flow)) +(def valid-text-content? + (sm/lazy-validator ::ctsx/content)) + (defn- prepare-file-data "Apply some specific migrations or fixes to things that are allowed in v1 but not in v2, or that are the result of old bugs." @@ -241,6 +245,32 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) + ;; There are some bugs in the past that allows convert text to + ;; path and this fix tries to identify this cases and fix them converting + ;; the shape back to text shape + + fix-text-shapes-converted-to-path + (fn [file-data] + (letfn [(fix-container [container] + (d/update-when container :objects update-vals fix-shape)) + + (fix-shape [shape] + (if (and (cfh/path-shape? shape) + (contains? shape :content) + (some? (:selrect shape)) + (valid-text-content? (:content shape))) + (let [selrect (:selrect shape)] + (-> shape + (assoc :x (:x selrect)) + (assoc :y (:y selrect)) + (assoc :width (:width selrect)) + (assoc :height (:height selrect)) + (assoc :type :text))) + shape))] + (-> file-data + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) + fix-recent-colors (fn [file-data] ;; Remove invalid colors in :recent-colors @@ -509,7 +539,7 @@ ;; Find any copy that is referencing a shape inside a component that have ;; been detached in a previous fix. If so, undo the nested copy, converting ;; it into a direct copy. - ;; + ;; ;; WARNING: THIS SHOULD BE CALLED AT THE END OF THE PROCESS. (letfn [(fix-container [container] (d/update-when container :objects update-vals fix-shape)) @@ -531,6 +561,7 @@ (fix-misc-shape-issues) (fix-recent-colors) (fix-missing-image-metadata) + (fix-text-shapes-converted-to-path) (delete-big-geometry-shapes) (fix-broken-parents) (fix-orphan-shapes) From 70b57f92b416b5376633ad555eb89bfaa7eacc78 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 14:25:46 +0100 Subject: [PATCH 49/54] :bug: Fix broken path content on comp-v2 migration --- backend/src/app/features/components_v2.clj | 38 +++++++++++++++++ common/src/app/common/geom/shapes/path.cljc | 1 + common/src/app/common/types/shape.cljc | 12 +----- common/src/app/common/types/shape/path.cljc | 47 +++++++++++++++++++++ 4 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 common/src/app/common/types/shape/path.cljc diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index d6adeb4777..516db13f81 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -19,6 +19,7 @@ [app.common.geom.point :as gpt] [app.common.geom.rect :as grc] [app.common.geom.shapes :as gsh] + [app.common.geom.shapes.path :as gshp] [app.common.logging :as l] [app.common.math :as mth] [app.common.schema :as sm] @@ -33,6 +34,7 @@ [app.common.types.pages-list :as ctpl] [app.common.types.shape :as cts] [app.common.types.shape-tree :as ctst] + [app.common.types.shape.path :as ctsp] [app.common.types.shape.text :as ctsx] [app.common.uuid :as uuid] [app.db :as db] @@ -110,6 +112,12 @@ (def valid-text-content? (sm/lazy-validator ::ctsx/content)) +(def valid-path-content? + (sm/lazy-validator ::ctsp/content)) + +(def valid-path-segment? + (sm/lazy-validator ::ctsp/segment)) + (defn- prepare-file-data "Apply some specific migrations or fixes to things that are allowed in v1 but not in v2, or that are the result of old bugs." @@ -271,6 +279,35 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) + fix-broken-paths + (fn [file-data] + (letfn [(fix-container [container] + (d/update-when container :objects update-vals fix-shape)) + + (fix-shape [shape] + (if (and (cfh/path-shape? shape) + (seq (:content shape)) + (not (valid-path-content? (:content shape)))) + (let [shape (update shape :content fix-path-content) + [points selrect] (gshp/content->points+selrect shape (:content shape))] + (-> shape + (dissoc :bool-content) + (dissoc :bool-type) + (assoc :points points) + (assoc :selrect selrect))) + shape)) + + (fix-path-content [content] + (let [[seg1 :as content] (filterv valid-path-segment? content)] + (if (and seg1 (not= :move-to (:command seg1))) + (let [params (select-keys (:params seg1) [:x :y])] + (into [{:command :move-to :params params}] content)) + content)))] + + (-> file-data + (update :pages-index update-vals fix-container) + (d/update-when :components update-vals fix-container)))) + fix-recent-colors (fn [file-data] ;; Remove invalid colors in :recent-colors @@ -562,6 +599,7 @@ (fix-recent-colors) (fix-missing-image-metadata) (fix-text-shapes-converted-to-path) + (fix-broken-paths) (delete-big-geometry-shapes) (fix-broken-parents) (fix-orphan-shapes) diff --git a/common/src/app/common/geom/shapes/path.cljc b/common/src/app/common/geom/shapes/path.cljc index 018beaeb39..84f0b52418 100644 --- a/common/src/app/common/geom/shapes/path.cljc +++ b/common/src/app/common/geom/shapes/path.cljc @@ -981,6 +981,7 @@ selrect (-> points (gco/transform-points points-center transform-inverse) (grc/points->rect))] + [points selrect])) (defn open-path? diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index 6187a7aa7c..cdc7ccb163 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -25,6 +25,7 @@ [app.common.types.shape.export :as ctse] [app.common.types.shape.interactions :as ctsi] [app.common.types.shape.layout :as ctsl] + [app.common.types.shape.path :as ctsp] [app.common.types.shape.shadow :as ctss] [app.common.types.shape.text :as ctsx] [app.common.uuid :as uuid] @@ -256,16 +257,7 @@ (sm/define! ::path-attrs [:map {:title "PathAttrs"} [:type [:= :path]] - [:x {:optional true} [:maybe ::sm/safe-number]] - [:y {:optional true} [:maybe ::sm/safe-number]] - [:width {:optional true} [:maybe ::sm/safe-number]] - [:height {:optional true} [:maybe ::sm/safe-number]] - [:content - {:optional true} - [:vector - [:map - [:command :keyword] - [:params {:optional true} [:maybe :map]]]]]]) + [:content ::ctsp/content]]) (sm/define! ::text-attrs [:map {:title "TextAttrs"} diff --git a/common/src/app/common/types/shape/path.cljc b/common/src/app/common/types/shape/path.cljc new file mode 100644 index 0000000000..d633bb85c6 --- /dev/null +++ b/common/src/app/common/types/shape/path.cljc @@ -0,0 +1,47 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.common.types.shape.path + (:require + [app.common.schema :as sm])) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SCHEMA +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(sm/define! ::segment + [:multi {:title "PathSegment" :dispatch :command} + [:line-to + [:map + [:command [:= :line-to]] + [:params + [:map + [:x ::sm/safe-number] + [:y ::sm/safe-number]]]]] + [:close-path + [:map + [:command [:= :close-path]]]] + [:move-to + [:map + [:command [:= :move-to]] + [:params + [:map + [:x ::sm/safe-number] + [:y ::sm/safe-number]]]]] + [:curve-to + [:map + [:command [:= :curve-to]] + [:params + [:map + [:x ::sm/safe-number] + [:y ::sm/safe-number] + [:c1x ::sm/safe-number] + [:c1y ::sm/safe-number] + [:c2x ::sm/safe-number] + [:c2y ::sm/safe-number]]]]]]) + +(sm/define! ::content + [:vector ::segment]) From 75576c341d2ced8e1921863cf36ea5e1bc9ca078 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 14:39:46 +0100 Subject: [PATCH 50/54] :bug: Fix broken bool shapes on comp-v2 migration --- backend/src/app/features/components_v2.clj | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 516db13f81..6c4ea127c7 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -16,6 +16,7 @@ [app.common.files.migrations :as fmg] [app.common.files.shapes-helpers :as cfsh] [app.common.files.validate :as cfv] + [app.common.geom.matrix :as gmt] [app.common.geom.point :as gpt] [app.common.geom.rect :as grc] [app.common.geom.shapes :as gsh] @@ -285,9 +286,10 @@ (d/update-when container :objects update-vals fix-shape)) (fix-shape [shape] - (if (and (cfh/path-shape? shape) - (seq (:content shape)) - (not (valid-path-content? (:content shape)))) + (cond + (and (cfh/path-shape? shape) + (seq (:content shape)) + (not (valid-path-content? (:content shape)))) (let [shape (update shape :content fix-path-content) [points selrect] (gshp/content->points+selrect shape (:content shape))] (-> shape @@ -295,6 +297,29 @@ (dissoc :bool-type) (assoc :points points) (assoc :selrect selrect))) + + ;; When we fount a bool shape with no content, + ;; we convert it to a simple rect + (and (cfh/bool-shape? shape) + (not (seq (:bool-content shape)))) + (let [selrect (or (:selrect shape) + (grc/make-rect)) + points (grc/rect->points selrect)] + (-> shape + (assoc :x (:x selrect)) + (assoc :y (:y selrect)) + (assoc :width (:height selrect)) + (assoc :height (:height selrect)) + (assoc :selrect selrect) + (assoc :points points) + (assoc :type :rect) + (assoc :transform (gmt/matrix)) + (assoc :transform-inverse (gmt/matrix)) + (dissoc :bool-content) + (dissoc :shapes) + (dissoc :content))) + + :else shape)) (fix-path-content [content] From 317f83e3ec849ec850743d2cfd42b831c4921ba1 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 15:02:53 +0100 Subject: [PATCH 51/54] :bug: Fix edge case on parsing svg viewbox --- common/src/app/common/svg/shapes_builder.cljc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/common/src/app/common/svg/shapes_builder.cljc b/common/src/app/common/svg/shapes_builder.cljc index 6e495f7258..343507f770 100644 --- a/common/src/app/common/svg/shapes_builder.cljc +++ b/common/src/app/common/svg/shapes_builder.cljc @@ -57,13 +57,15 @@ clean-value)) (defn- svg-dimensions - [data] - (let [width (dm/get-in data [:attrs :width] 100) - height (dm/get-in data [:attrs :height] 100) - viewbox (or (dm/get-in data [:attrs :viewBox]) + [{:keys [attrs] :as data}] + (let [width (:width attrs 100) + height (:height attrs 100) + viewbox (or (:viewBox attrs) (dm/str "0 0 " width " " height)) - [x y width height] (->> (str/split viewbox #"\s+") + + [x y width height] (->> (str/split viewbox #"[\s,]+") (map d/parse-double)) + width (if (= width 0) 1 width) height (if (= height 0) 1 height)] From f864424d14fb9ffce04156f330d8edaaadc41a66 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 25 Jan 2024 14:50:59 +0100 Subject: [PATCH 52/54] :bug: Fix parent not found --- backend/src/app/features/components_v2.clj | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index 6c4ea127c7..f94dd3d27c 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -615,7 +615,23 @@ :component-root)))] (-> file-data (update :pages-index update-vals fix-container) - (d/update-when :components update-vals fix-container))))] + (d/update-when :components update-vals fix-container)))) + + fix-shape-nil-parent-id + (fn [file-data] + ;; Ensure that parent-id and frame-id are not nil + (letfn [(fix-container [container] + (d/update-when container :objects update-vals fix-shape)) + + (fix-shape [shape] + (let [frame-id (or (:frame-id shape) + uuid/zero) + parent-id (or (:parent-id shape) + frame-id)] + (assoc shape :frame-id frame-id + :parent-id parent-id)))] + (-> file-data + (update :pages-index update-vals fix-container))))] (-> file-data (delete-invalid-flows) @@ -638,6 +654,7 @@ (fix-frame-ids) (fix-component-nil-objects) (fix-false-copies) + (fix-shape-nil-parent-id) (fix-copies-of-detached)))) ; <- Do not add fixes after this one ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From 7ae308c8c9e1ddfde0bf456ecf838dad14194499 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 15:27:37 +0100 Subject: [PATCH 53/54] :bug: Remove page background color it it has an invalid rgb color string --- backend/src/app/features/components_v2.clj | 29 ++++++++++++++++------ 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/backend/src/app/features/components_v2.clj b/backend/src/app/features/components_v2.clj index f94dd3d27c..e5e2673f73 100644 --- a/backend/src/app/features/components_v2.clj +++ b/backend/src/app/features/components_v2.clj @@ -119,6 +119,9 @@ (def valid-path-segment? (sm/lazy-validator ::ctsp/segment)) +(def valid-rgb-color-string? + (sm/lazy-validator ::ctc/rgb-color)) + (defn- prepare-file-data "Apply some specific migrations or fixes to things that are allowed in v1 but not in v2, or that are the result of old bugs." @@ -175,13 +178,25 @@ (update :pages-index update-vals fix-container) (d/update-when :components update-vals fix-container)))) - ;; Some pages has invalid data on flows, we proceed just to - ;; delete them. - delete-invalid-flows + fix-page-invalid-options (fn [file-data] - (update file-data :pages-index update-vals - (fn [page] - (d/update-in-when page [:options :flows] #(filterv valid-flow? %))))) + (letfn [(update-page [page] + (update page :options fix-options)) + + (fix-background [options] + (if (and (contains? options :background) + (not (valid-rgb-color-string? (:background options)))) + (dissoc options :background) + options)) + + (fix-options [options] + (-> options + ;; Some pages has invalid data on flows, we proceed just to + ;; delete them. + (d/update-when :flows #(filterv valid-flow? %)) + (fix-background)))] + + (update file-data :pages-index update-vals update-page))) delete-big-geometry-shapes (fn [file-data] @@ -634,7 +649,7 @@ (update :pages-index update-vals fix-container))))] (-> file-data - (delete-invalid-flows) + (fix-page-invalid-options) (fix-bad-children) (fix-misc-shape-issues) (fix-recent-colors) From 0c8aba6be0d9290580f092e3fd955e2db046ddfb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 25 Jan 2024 15:55:23 +0100 Subject: [PATCH 54/54] :bug: Fix incorrect parsing of svg transform attr --- common/src/app/common/svg.cljc | 39 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/common/src/app/common/svg.cljc b/common/src/app/common/svg.cljc index 6e4ba6383a..e0c718362b 100644 --- a/common/src/app/common/svg.cljc +++ b/common/src/app/common/svg.cljc @@ -31,7 +31,7 @@ (def xml-id-regex #"#([:A-Z_a-z\xC0-\xD6\xD8-\xF6\xF8-\u02FF\u0370-\u037D\u037F-\u1FFF\u200C-\u200D\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD\u10000-\uEFFFF][\.\-\:0-9\xB7A-Z_a-z\xC0-\xD6\xD8-\xF6\xF8-\u02FF\u0300-\u036F\u0370-\u037D\u037F-\u1FFF\u200C-\u200D\u203F-\u2040\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD\u10000-\uEFFFF]*)") (def matrices-regex #"(matrix|translate|scale|rotate|skewX|skewY)\(([^\)]*)\)") -(def number-regex #"[+-]?\d*(\.\d+)?(e[+-]?\d+)?") +(def number-regex #"[+-]?\d*(\.\d+)?([eE][+-]?\d+)?") (def tags-to-remove #{:linearGradient :radialGradient :metadata :mask :clipPath :filter :title}) @@ -759,40 +759,39 @@ ;; Transforms spec: ;; https://www.w3.org/TR/SVG11/single-page.html#coords-TransformAttribute -(defn format-translate-params +(defn- format-translate-params [params] (assert (or (= (count params) 1) (= (count params) 2))) (if (= (count params) 1) [(gpt/point (nth params 0) 0)] [(gpt/point (nth params 0) (nth params 1))])) -(defn format-scale-params +(defn- format-scale-params [params] (assert (or (= (count params) 1) (= (count params) 2))) (if (= (count params) 1) [(gpt/point (nth params 0))] [(gpt/point (nth params 0) (nth params 1))])) -(defn format-rotate-params +(defn- format-rotate-params [params] (assert (or (= (count params) 1) (= (count params) 3)) (str "??" (count params))) (if (= (count params) 1) [(nth params 0) (gpt/point 0 0)] [(nth params 0) (gpt/point (nth params 1) (nth params 2))])) -(defn format-skew-x-params +(defn- format-skew-x-params [params] (assert (= (count params) 1)) [(nth params 0) 0]) -(defn format-skew-y-params +(defn- format-skew-y-params [params] (assert (= (count params) 1)) [0 (nth params 0)]) -(defn to-matrix - [{:keys [type params]}] - (assert (#{"matrix" "translate" "scale" "rotate" "skewX" "skewY"} type)) +(defn- to-matrix + [type params] (case type "matrix" (apply gmt/matrix params) "translate" (apply gmt/translate-matrix (format-translate-params params)) @@ -802,19 +801,17 @@ "skewY" (apply gmt/skew-matrix (format-skew-y-params params)))) (defn parse-transform - [transform-attr] - (if transform-attr - (let [process-matrix - (fn [[_ type params]] - (let [params (->> (re-seq number-regex params) - (filter #(-> % first seq)) - (map (comp d/parse-double first)))] - {:type type :params params})) + [transform] + (if (string? transform) + (->> (re-seq matrices-regex transform) + (map (fn [[_ type params]] + (let [params (->> (re-seq number-regex params) + (map first) + (keep not-empty) + (map d/parse-double))] + (to-matrix type params)))) + (reduce gmt/multiply (gmt/matrix))) - matrices (->> (re-seq matrices-regex transform-attr) - (map process-matrix) - (map to-matrix))] - (reduce gmt/multiply (gmt/matrix) matrices)) (gmt/matrix))) (defn format-move [[x y]] (str "M" x " " y))