🐛 Migration to repair old files containing tokens with invalid names

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
 

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

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$_-]+)*\}$")
 

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}))

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)))))))

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"

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]