diff --git a/CHANGES.md b/CHANGES.md index 4096c72816..57d6632603 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -63,6 +63,7 @@ - Fix scroll on colorpicker [Taiga #13623](https://tree.taiga.io/project/penpot/issue/13623) - Fix crash when pasting non-map transit clipboard data [Github #8580](https://github.com/penpot/penpot/pull/8580) - Fix `penpot.openPage()` plugin API not navigating in the same tab; change default to same-tab navigation and allow passing a UUID string instead of a Page object [Github #8520](https://github.com/penpot/penpot/issues/8520) +- Fix old files containing tokens with invalid names [Taiga #13849](https://tree.taiga.io/project/penpot/issue/13849) ## 2.13.3 diff --git a/common/src/app/common/time.cljc b/common/src/app/common/time.cljc index 3784cba8a5..a0576fced8 100644 --- a/common/src/app/common/time.cljc +++ b/common/src/app/common/time.cljc @@ -90,13 +90,22 @@ (Clock/fixed ^Instant (inst instant) ^ZoneId (ZoneId/of "Z")))) - - (defn now [] #?(:clj (Instant/now *clock*) :cljs (new js/Date))) +#?(:clj + (defn tick-millis-clock + "Alternate clock with a resolution of milliseconds instead of the default nanoseconds of the Java clock. + This may be useful if the instant is going to be serialized to DB with fressian (that does not have + resolution enough to store all precission) and need to compare the deserialized value for equality. + + You can replace the global clock (for example in unit tests) with + (alter-var-root #'ct/*clock* (constantly (ct/tick-millis-clock)))" + [] + (Clock/tickMillis (ZoneId/of "Z")))) + ;; --- DURATION (defn- resolve-temporal-unit diff --git a/common/src/app/common/types/token.cljc b/common/src/app/common/types/token.cljc index 55ecc842e7..2766baed5e 100644 --- a/common/src/app/common/types/token.cljc +++ b/common/src/app/common/types/token.cljc @@ -144,6 +144,19 @@ :gen/gen sg/text} token-name-validation-regex]) +(defn clean-token-name + "Remove all forbidden characters from token name and return a valid token name. + This is used for repairing invalid token names in old versions of Penpot." + [name] + (-> name + (str/replace "/" ".") + (str/replace " " "") + (str/replace #"^\$+" "") + (str/replace #"^\.+" "") + (str/replace #"\.+$" "") + (str/replace #"\.\.+" ".") + (str/replace #"[^a-zA-Z0-9$._-]" "?"))) + (def token-ref-validation-regex #"^\{[a-zA-Z0-9_-][a-zA-Z0-9$_-]*(\.[a-zA-Z0-9$_-]+)*\}$") diff --git a/common/src/app/common/types/tokens_lib.cljc b/common/src/app/common/types/tokens_lib.cljc index 63cd87e393..2ce5bd18fd 100644 --- a/common/src/app/common/types/tokens_lib.cljc +++ b/common/src/app/common/types/tokens_lib.cljc @@ -242,17 +242,19 @@ (update-token- [this token-id f] (assert (uuid? token-id) "expected uuid for `token-id`") (if-let [token (get-token- this token-id)] - (let [token' (-> (make-token (f token)) - (assoc :modified-at (ct/now)))] - (TokenSet. id - name - description - (ct/now) - (if (= (:name token) (:name token')) - (assoc tokens (:name token') token') - (-> tokens - (d/oassoc-before (:name token) (:name token') token') - (dissoc (:name token)))))) + (let [token' (f token)] + (if (not= token token') + (let [token' (assoc token' :modified-at (ct/now))] + (TokenSet. id + name + description + (ct/now) + (if (= (:name token) (:name token')) + (assoc tokens (:name token') token') + (-> tokens + (d/oassoc-before (:name token) (:name token') token') + (dissoc (:name token)))))) + this)) this)) (delete-token- [this token-id] @@ -2209,6 +2211,32 @@ Will return a value that matches this schema: (update params :sets d/update-vals migrate-set-node)))) +#?(:clj + (defn- migrate-to-v1-5 + "Migrate the TokensLib data structure internals to v1.5 version; it + expects input from v1.4 version" + [params] + (let [migrate-token + (fn [token] + (let [new-name (-> (get-name token) + (cto/clean-token-name))] + (if (= new-name (get-name token)) + token + (rename token new-name)))) + + migrate-set-node + (fn recurse [node] + (if (token-set? node) + (let [tokens (get-tokens- node)] + (reduce (fn [set token] + + (update-token- set (:id token) migrate-token)) + node + (vals tokens))) + (d/update-vals node recurse)))] + + (update params :sets d/update-vals migrate-set-node)))) + #?(:clj (defn- read-tokens-lib-v1-1 "Reads the tokens lib data structure and ensures that hidden @@ -2224,6 +2252,7 @@ Will return a value that matches this schema: (migrate-to-v1-2) (migrate-to-v1-3) (migrate-to-v1-4) + (migrate-to-v1-5) (map->tokens-lib))))) #?(:clj @@ -2239,6 +2268,7 @@ Will return a value that matches this schema: :active-themes active-themes} (migrate-to-v1-3) (migrate-to-v1-4) + (migrate-to-v1-5) (map->tokens-lib))))) #?(:clj @@ -2254,6 +2284,21 @@ Will return a value that matches this schema: :themes themes :active-themes active-themes} (migrate-to-v1-4) + (migrate-to-v1-5) + (map->tokens-lib))))) + +#?(:clj + (defn- read-tokens-lib-v1-4 + "Reads the tokens lib data structure and fixes token names." + [r] + (let [sets (fres/read-object! r) + themes (fres/read-object! r) + active-themes (fres/read-object! r)] + + (-> {:sets sets + :themes themes + :active-themes active-themes} + (migrate-to-v1-5) (map->tokens-lib))))) #?(:clj @@ -2315,8 +2360,11 @@ Will return a value that matches this schema: {:name "penpot/tokens-lib/v1.3" :rfn read-tokens-lib-v1-3} - ;; CURRENT TOKENS LIB READER & WRITTER {:name "penpot/tokens-lib/v1.4" + :rfn read-tokens-lib-v1-4} + + ;; CURRENT TOKENS LIB READER & WRITTER + {:name "penpot/tokens-lib/v1.5" :class TokensLib :wfn write-tokens-lib :rfn read-tokens-lib})) diff --git a/common/test/common_tests/types/token_migrations_test.cljc b/common/test/common_tests/types/token_migrations_test.cljc new file mode 100644 index 0000000000..087fcabc3a --- /dev/null +++ b/common/test/common_tests/types/token_migrations_test.cljc @@ -0,0 +1,98 @@ +;; 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 common-tests.types.token-migrations-test + #?(:clj + (:require + [app.common.data :as d] + [app.common.test-helpers.ids-map :as thi] + [app.common.time :as ct] + [app.common.types.tokens-lib :as ctob] + [clojure.datafy :refer [datafy]] + [clojure.test :as t]))) + +#?(:clj + (t/deftest test-v1-5-fix-token-names + ;; Use a less precission clock, so `modified-at` dates keep being equal + ;; after serializing from fressian. + (alter-var-root #'ct/*clock* (constantly (ct/tick-millis-clock))) + + (t/testing "empty tokens-lib should not need any action" + (let [tokens-lib (ctob/make-tokens-lib) + tokens-lib' (-> tokens-lib + (datafy) + (#'app.common.types.tokens-lib/migrate-to-v1-5) + (ctob/make-tokens-lib))] + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "tokens with valid names should not need any action" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set :id (thi/new-id! :test-token-set) + :name "test-token-set")) + (ctob/add-token (thi/id :test-token-set) + (ctob/make-token :name "test-token-1" + :type :boolean + :value true)) + (ctob/add-token (thi/id :test-token-set) + (ctob/make-token :name "Test.Token_2" + :type :boolean + :value true)) + (ctob/add-token (thi/id :test-token-set) + (ctob/make-token :name "test.$token.3" + :type :boolean + :value true))) + tokens-lib' (-> tokens-lib + (datafy) + (#'app.common.types.tokens-lib/migrate-to-v1-5) + (ctob/make-tokens-lib))] + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "tokens with invalid names should be repaired" + (let [;; Need to use low level constructors to avoid schema checks + bad-token (ctob/map->Token {:id (thi/new-id! :bad-token) + :name "$.test-token with / and spacesAndSymbols%&." + :type :boolean + :value true + :description "" + :modified-at (ct/now)}) + token-set (ctob/map->token-set {:id (thi/new-id! :token-set) + :name "test-token-set" + :description "" + :modified-at (ct/now) + :tokens (d/ordered-map (ctob/get-name bad-token) bad-token)}) + token-theme (ctob/make-hidden-theme {:modified-at (ct/now)}) + tokens-lib (ctob/map->tokens-lib {:sets (d/ordered-map (str "S-" (ctob/get-name token-set)) token-set) + :themes (d/ordered-map + (:group token-theme) + (d/ordered-map + (:name token-theme) + token-theme)) + :active-themes #{(ctob/get-name token-theme)}}) + + tokens-lib' (-> tokens-lib + (datafy) + (#'app.common.types.tokens-lib/migrate-to-v1-5) + (ctob/make-tokens-lib)) + + expected-name "test-tokenwith.andspacesAndSymbols??" + + token-sets' (ctob/get-set-tree tokens-lib') + token-set' (ctob/get-set-by-name tokens-lib' "test-token-set") + tokens' (ctob/get-tokens tokens-lib' (ctob/get-id token-set')) + bad-token' (ctob/get-token-by-name tokens-lib' "test-token-set" expected-name)] + + (t/is (= (count token-sets') 1)) + (t/is (= (count tokens') 1)) + (t/is (= (ctob/get-id token-set') (ctob/get-id token-set))) + (t/is (= (ctob/get-name token-set') (ctob/get-name token-set))) + (t/is (= (ctob/get-description token-set') (ctob/get-description token-set))) + (t/is (= (ctob/get-modified-at token-set') (ctob/get-modified-at token-set))) + (t/is (= (ctob/get-id bad-token') (ctob/get-id bad-token))) + (t/is (= (ctob/get-name bad-token') expected-name)) + (t/is (= (ctob/get-description bad-token') (ctob/get-description bad-token))) + (t/is (= (ctob/get-modified-at bad-token') (ctob/get-modified-at bad-token))) + (t/is (= (:type bad-token') (:type bad-token))) + (t/is (= (:value bad-token') (:value bad-token))))))) \ No newline at end of file diff --git a/common/test/common_tests/types/token_test.cljc b/common/test/common_tests/types/token_test.cljc index 96e642690c..24b429da51 100644 --- a/common/test/common_tests/types/token_test.cljc +++ b/common/test/common_tests/types/token_test.cljc @@ -10,21 +10,42 @@ [app.common.types.token :as cto] [clojure.test :as t])) -(t/deftest test-valid-token-name-schema +(t/deftest test-valid-token-name ;; Allow regular namespace token names (t/is (true? (sm/validate cto/schema:token-name "Foo"))) (t/is (true? (sm/validate cto/schema:token-name "foo"))) (t/is (true? (sm/validate cto/schema:token-name "FOO"))) (t/is (true? (sm/validate cto/schema:token-name "Foo.Bar.Baz"))) - ;; Disallow trailing tokens + ;; Allow $ inside or at the end of the name, but not at the beginning + (t/is (true? (sm/validate cto/schema:token-name "Foo$Bar$Baz"))) + (t/is (true? (sm/validate cto/schema:token-name "Foo$Bar$Baz$"))) + (t/is (false? (sm/validate cto/schema:token-name "$Foo$Bar$Baz"))) + ;; Disallow starting and trailing dots + (t/is (false? (sm/validate cto/schema:token-name "....Foo.Bar.Baz"))) (t/is (false? (sm/validate cto/schema:token-name "Foo.Bar.Baz...."))) ;; Disallow multiple separator dots (t/is (false? (sm/validate cto/schema:token-name "Foo..Bar.Baz"))) ;; Disallow any special characters (t/is (false? (sm/validate cto/schema:token-name "Hey Foo.Bar"))) - (t/is (false? (sm/validate cto/schema:token-name "Hey😈Foo.Bar"))) - (t/is (false? (sm/validate cto/schema:token-name "Hey%Foo.Bar")))) + (t/is (false? (sm/validate cto/schema:token-name "HeyÅFoo.Bar"))) + (t/is (false? (sm/validate cto/schema:token-name "Hey%Foo.Bar"))) + (t/is (false? (sm/validate cto/schema:token-name "Hey / Foo/Bar")))) +(t/deftest test-clean-token-name + (t/is (= (cto/clean-token-name "Foo") "Foo")) + (t/is (= (cto/clean-token-name "foo") "foo")) + (t/is (= (cto/clean-token-name "FOO") "FOO")) + (t/is (= (cto/clean-token-name "Foo.Bar.Baz") "Foo.Bar.Baz")) + (t/is (= (cto/clean-token-name "Foo$Bar$Baz") "Foo$Bar$Baz")) + (t/is (= (cto/clean-token-name "Foo$Bar$Baz$") "Foo$Bar$Baz$")) + (t/is (= (cto/clean-token-name "$$$Foo$Bar$Baz") "Foo$Bar$Baz")) + (t/is (= (cto/clean-token-name "....Foo.Bar.Baz") "Foo.Bar.Baz")) + (t/is (= (cto/clean-token-name "Foo.Bar.Baz....") "Foo.Bar.Baz")) + (t/is (= (cto/clean-token-name "Foo..Bar...Baz") "Foo.Bar.Baz")) + (t/is (= (cto/clean-token-name "Hey Foo Bar") "HeyFooBar")) + (t/is (= (cto/clean-token-name "HeyÅFoo.Bar") "Hey?Foo.Bar")) + (t/is (= (cto/clean-token-name "Hey%Foo.Bar") "Hey?Foo.Bar")) + (t/is (= (cto/clean-token-name "Hey / Foo/Bar") "Hey.Foo.Bar"))) (t/deftest token-value-with-refs (t/testing "empty value" diff --git a/common/test/common_tests/types/tokens_lib_test.cljc b/common/test/common_tests/types/tokens_lib_test.cljc index 23bed42897..17db384d83 100644 --- a/common/test/common_tests/types/tokens_lib_test.cljc +++ b/common/test/common_tests/types/tokens_lib_test.cljc @@ -11,7 +11,6 @@ #?(:clj [app.common.test-helpers.tokens :as tht]) #?(:clj [clojure.datafy :refer [datafy]]) [app.common.data :as d] - [app.common.path-names :as cpn] [app.common.test-helpers.ids-map :as thi] [app.common.time :as ct] [app.common.transit :as tr]