✨ Add general features handling improvements

backend/src/app/features/components_v2.clj

@@ -769,7 +769,7 @@
             fdata (migrate-graphics fdata)]
         (update fdata :options assoc :components-v2 true)))))
 
-(defn- process-fdata
+(defn- prepare-fdata
   [fdata id]
   (-> fdata
       (assoc :id id)
@@ -788,43 +788,46 @@
 
 (defn- process-file
   [{:keys [::db/conn] :as system} id & {:keys [validate? throw-on-validate?]}]
-  (binding [pmap/*tracked* (pmap/create-tracked)
-            pmap/*load-fn* (partial fdata/load-pointer *system* id)]
+  (let [file  (binding [cfeat/*new* (atom #{})
+                        pmap/*load-fn* (partial fdata/load-pointer system id)]
+                (-> (files/get-file system id :migrate? false)
+                    (update :data prepare-fdata id)
+                    (update :features into (deref cfeat/*new*))
+                    (update :features cfeat/migrate-legacy-features)))
 
-    (let [file  (binding [cfeat/*new* (atom #{})]
-                  (-> (files/get-file system id :migrate? false)
-                      (update :data process-fdata id)
-                      (update :features into (deref cfeat/*new*))
-                      (update :features cfeat/migrate-legacy-features)))
+        libs  (->> (files/get-file-libraries conn id)
+                   (into [file] (map (fn [{:keys [id]}]
+                                       (binding [pmap/*load-fn* (partial fdata/load-pointer system id)]
+                                         (-> (files/get-file system id :migrate? false)
+                                             (update :data prepare-fdata id))))))
+                   (d/index-by :id))
 
-          libs  (->> (files/get-file-libraries conn id)
-                     (into [file] (map (fn [{:keys [id]}]
-                                         (binding [pmap/*load-fn* (partial fdata/load-pointer system id)]
-                                           (-> (files/get-file system id :migrate? false)
-                                               (update :data process-fdata id))))))
-                     (d/index-by :id))
+        file  (-> file
+                  (update :data migrate-fdata libs)
+                  (update :features conj "components/v2"))
 
-          pmap? (contains? (:features file) "fdata/pointer-map")
+        _     (when validate?
+                (validate-file! file libs throw-on-validate?))
 
-          file  (-> file
-                    (update :data migrate-fdata libs)
-                    (update :features conj "components/v2")
-                    (cond-> pmap? (fdata/enable-pointer-map)))
-          ]
+        file (if (contains? (:features file) "fdata/objects-map")
+               (fdata/enable-objects-map file)
+               file)
 
-      (when validate?
-        (validate-file! file libs throw-on-validate?))
+        file (if (contains? (:features file) "fdata/pointer-map")
+               (binding [pmap/*tracked* (pmap/create-tracked)]
+                 (let [file (fdata/enable-pointer-map file)]
+                   (fdata/persist-pointers! system id)
+                   file))
+               file)]
 
-      (db/update! conn :file
-                  {:data (blob/encode (:data file))
-                   :features (db/create-array conn "text" (:features file))
-                   :revn (:revn file)}
-                  {:id (:id file)})
+    (db/update! conn :file
+                {:data (blob/encode (:data file))
+                 :features (db/create-array conn "text" (:features file))
+                 :revn (:revn file)}
+                {:id (:id file)}
+                {::db/return-keys? false})
 
-      (when pmap?
-        (fdata/persist-pointers! system id))
-
-      (dissoc file :data))))
+    (dissoc file :data)))
 
 (defn migrate-file!
   [system file-id & {:keys [validate? throw-on-validate?]}]

backend/src/app/rpc/commands/binfile.clj

@@ -701,6 +701,25 @@
               (update :object-id #(str/replace-first % #"^(.*?)/" (str file-id "/")))))
         thumbnails))
 
+(defn- process-fdata
+  [fdata id]
+  (-> fdata
+      (dissoc :recent-colors)
+      (assoc :id id)
+      (cond-> (> (:version fdata) cfd/version)
+        (assoc :version cfd/version))
+      ;; FIXME: We're temporarily activating all
+      ;; migrations because a problem in the
+      ;; environments messed up with the version
+      ;; numbers When this problem is fixed delete
+      ;; the following line
+      (assoc :version 22)
+      (pmg/migrate-data)
+      (update :pages-index relink-shapes)
+      (update :components relink-shapes)
+      (update :media relink-media)))
+
+
 (defmethod read-section :v1/files
   [{:keys [::db/conn ::input ::project-id ::enabled-features ::timestamp ::overwrite?] :as system}]
 
@@ -754,58 +773,40 @@
         (l/dbg :hint "update media references" ::l/sync? true)
         (vswap! *state* update :media into (map #(update % :id lookup-index)) media))
 
-      (binding [pmap/*tracked* (pmap/create-tracked)
-                cfeat/*new* (atom #{})]
-        (let [file (-> file
+      (let [file (binding [cfeat/*new* (atom #{})]
+                   (-> file
                        (assoc :id file-id')
                        (assoc :features features)
                        (assoc :project-id project-id)
                        (assoc :created-at timestamp)
                        (assoc :modified-at timestamp)
                        (dissoc :thumbnails)
-                       (update :data (fn [data]
-                                       (-> data
-                                           (dissoc :recent-colors)
-                                           (assoc :id file-id')
-                                           (cond-> (> (:version data) cfd/version)
-                                             (assoc :version cfd/version))
+                       (update :data process-fdata file-id')
+                       (update :features into (deref cfeat/*new*))))
 
-                                           ;; FIXME: We're temporarily activating all
-                                           ;; migrations because a problem in the
-                                           ;; environments messed up with the version
-                                           ;; numbers When this problem is fixed delete
-                                           ;; the following line
-                                           (assoc :version 22)
-                                           (update :pages-index relink-shapes)
-                                           (update :components relink-shapes)
-                                           (update :media relink-media)
-                                           (pmg/migrate-data)
-                                           (d/without-nils)))))
+            _    (when (contains? cf/flags :file-schema-validation)
+                   (fval/validate-file-schema! file))
 
-              ;; Add to file features all possible features added on
-              ;; migration process.
-              file (update file :features into (deref cfeat/*new*))
+            _    (when (contains? cf/flags :soft-file-schema-validation)
+                   (let [result (ex/try! (fval/validate-file-schema! file))]
+                     (when (ex/exception? result)
+                       (l/error :hint "file schema validation error" :cause result))))
 
-              file (if (contains? cf/flags :file-schema-validation)
-                     (fval/validate-file-schema! file)
-                     file)
+            file (if (contains? (:features file) "fdata/objects-map")
+                   (feat.fdata/enable-objects-map file)
+                   file)
 
-              _    (when (contains? cf/flags :soft-file-schema-validation)
-                     (try
-                       (fval/validate-file-schema! file)
-                       (catch Throwable cause
-                         (l/error :hint "file schema validation error" :cause cause))))
+            file (if (contains? (:features file) "fdata/pointer-map")
+                   (binding [pmap/*tracked* (pmap/create-tracked)]
+                     (let [file (feat.fdata/enable-pointer-map file)]
+                       (feat.fdata/persist-pointers! system file-id')
+                       file))
+                   file)
 
-              file (cond-> file
-                     (contains? (:features file) "fdata/objects-map")
-                     (feat.fdata/enable-objects-map)
 
-                     (contains? (:features file) "fdata/pointer-map")
-                     (feat.fdata/enable-pointer-map))
-
-              file (-> file
-                       (update :features #(db/create-array conn "text" %))
-                       (update :data blob/encode))]
+            file (-> file
+                     (update :features #(db/create-array conn "text" %))
+                     (update :data blob/encode))]
 
           (l/dbg :hint "create file" :id (str file-id') ::l/sync? true)
 
@@ -813,12 +814,10 @@
             (create-or-update-file! conn file)
             (db/insert! conn :file file))
 
-          (feat.fdata/persist-pointers! system file-id')
-
           (when overwrite?
             (db/delete! conn :file-thumbnail {:file-id file-id'}))
 
-          file-id')))))
+          file-id'))))
 
 (defmethod read-section :v1/rels
   [{:keys [::db/conn ::input ::timestamp]}]

backend/src/app/rpc/commands/files.clj

@@ -34,7 +34,6 @@
    [app.util.pointer-map :as pmap]
    [app.util.services :as sv]
    [app.util.time :as dt]
-   [clojure.set :as set]
    [clojure.spec.alpha :as s]
    [cuerdas.core :as str]))
 
@@ -227,7 +226,10 @@
   (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)
             pmap/*tracked* (pmap/create-tracked)
             cfeat/*new*    (atom #{})]
-    (let [file (fmg/migrate-file file)]
+    (let [file (-> (fmg/migrate-file file)
+                   (update :features into (deref cfeat/*new*))
+                   (update :features cfeat/migrate-legacy-features))]
+
       ;; NOTE: when file is migrated, we break the rule of no perform
       ;; mutations on get operations and update the file with all
       ;; migrations applied
@@ -235,16 +237,17 @@
       ;; 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
-      (if (fmg/migrated? file)
-        (let [file     (update file :features cfeat/migrate-legacy-features)
-              features (set/union (deref cfeat/*new*) (:features file))]
-          (db/update! conn :file
-                      {:data (blob/encode (:data file))
-                       :features (db/create-array conn "text" features)}
-                      {:id id})
-          (feat.fdata/persist-pointers! cfg id)
-          (assoc file :features features))
-        file))))
+      (when (fmg/migrated? file)
+        (db/update! conn :file
+                    {:data (blob/encode (:data file))
+                     :features (db/create-array conn "text" (:features file))}
+                    {:id id}
+                    {::db/return-keys? false})
+
+        (when (contains? (:features file) "fdata/pointer-map")
+          (feat.fdata/persist-pointers! cfg id)))
+
+      file)))
 
 (defn get-file
   [{:keys [::db/conn] :as cfg} id & {:keys [project-id migrate?

backend/src/app/rpc/commands/files_update.clj

@@ -182,40 +182,39 @@
 (defn update-file
   [{:keys [::db/conn ::mtx/metrics] :as cfg}
    {:keys [id file features changes changes-with-metadata] :as params}]
-  (binding [cfeat/*current*  features
-            cfeat/*previous* (:features file)]
+  (let [features  (-> features
+                      (set/difference cfeat/frontend-only-features)
+                      (set/union (:features file)))
 
-    (let [features  (-> features
-                        (set/difference cfeat/frontend-only-features)
-                        (set/union (:features file)))
+        update-fn (cond-> update-file*
+                    (contains? features "fdata/pointer-map")
+                    (wrap-with-pointer-map-context)
 
-          update-fn (cond-> update-file*
-                      (contains? features "fdata/pointer-map")
-                      (wrap-with-pointer-map-context)
+                    (contains? features "fdata/objects-map")
+                    (wrap-with-objects-map-context))
 
-                      (contains? features "fdata/objects-map")
-                      (wrap-with-objects-map-context))
+        changes   (if changes-with-metadata
+                    (->> changes-with-metadata (mapcat :changes) vec)
+                    (vec changes))]
 
-          changes   (if changes-with-metadata
-                      (->> changes-with-metadata (mapcat :changes) vec)
-                      (vec changes))]
+    (when (> (:revn params)
+             (:revn file))
+      (ex/raise :type :validation
+                :code :revn-conflict
+                :hint "The incoming revision number is greater that stored version."
+                :context {:incoming-revn (:revn params)
+                          :stored-revn (:revn file)}))
 
-      (when (> (:revn params)
-               (:revn file))
-        (ex/raise :type :validation
-                  :code :revn-conflict
-                  :hint "The incoming revision number is greater that stored version."
-                  :context {:incoming-revn (:revn params)
-                            :stored-revn (:revn file)}))
+    (mtx/run! metrics {:id :update-file-changes :inc (count changes)})
 
-      (mtx/run! metrics {:id :update-file-changes :inc (count changes)})
-
-      (when (not= features (:features file))
-        (let [features (db/create-array conn "text" features)]
-          (db/update! conn :file
-                      {:features features}
-                      {:id id})))
+    (when (not= features (:features file))
+      (let [features (db/create-array conn "text" features)]
+        (db/update! conn :file
+                    {:features features}
+                    {:id id})))
 
+    (binding [cfeat/*current*  features
+              cfeat/*previous* (:features file)]
       (let [file   (assoc file :features features)
             params (-> params
                        (assoc :file file)

backend/src/app/rpc/commands/management.clj

@@ -109,23 +109,37 @@
           (update-fdata [fdata new-id]
             (-> fdata
                 (assoc :id new-id)
+                (feat.fdata/process-pointers deref)
                 (pmg/migrate-data)
                 (update :pages-index relink-shapes)
                 (update :components relink-shapes)
                 (update :media relink-media)
-                (d/without-nils)
-                (feat.fdata/process-pointers pmap/clone)))]
+                (d/without-nils)))]
 
     (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)
               pmap/*tracked* (pmap/create-tracked)
               cfeat/*new*    (atom #{})]
+
       (let [new-id (get index id)
-            file   (-> file
-                       (assoc :id new-id)
-                       (update :data update-fdata new-id)
-                       (update :features into (deref cfeat/*new*))
-                       (update :features cfeat/migrate-legacy-features))]
-        (feat.fdata/persist-pointers! cfg new-id)
+            file   (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg id)
+                             cfeat/*new*    (atom #{})]
+                     (-> file
+                         (assoc :id new-id)
+                         (update :data update-fdata new-id)
+                         (update :features into (deref cfeat/*new*))
+                         (update :features cfeat/migrate-legacy-features)))
+
+            file (if (contains? (:features file) "fdata/objects-map")
+                   (feat.fdata/enable-objects-map file)
+                   file)
+
+            file (if (contains? (:features file) "fdata/pointer-map")
+                   (binding [pmap/*tracked* (pmap/create-tracked)]
+                     (let [file (feat.fdata/enable-pointer-map file)]
+                       (feat.fdata/persist-pointers! cfg (:id file))
+                       file))
+                   file)]
+
         file))))
 
 (def sql:get-used-libraries
@@ -191,20 +205,22 @@
     (db/insert! conn :file
                 (-> file
                     (update :features #(db/create-array conn "text" %))
-                    (update :data blob/encode)))
+                    (update :data blob/encode))
+                {::db/return-keys? false})
 
     (db/insert! conn :file-profile-rel
                 {:file-id (:id file)
                  :profile-id profile-id
                  :is-owner true
                  :is-admin true
-                 :can-edit true})
+                 :can-edit true}
+                {::db/return-keys? false})
 
     (doseq [params flibs]
-      (db/insert! conn :file-library-rel params))
+      (db/insert! conn :file-library-rel params ::db/return-keys? false))
 
     (doseq [params fmeds]
-      (db/insert! conn :file-media-object params))
+      (db/insert! conn :file-media-object params ::db/return-keys? false))
 
     file))
 

common/src/app/common/features.cljc

@@ -51,7 +51,10 @@
     "layout/grid"})
 
 ;; A set of features enabled by default for each file, they are
-;; implicit and are enabled by default and can't be disabled
+;; implicit and are enabled by default and can't be disabled. The
+;; features listed in this set are mainly freatures addedby file
+;; migrations process, so all features referenced in migrations should
+;; be here.
 (def default-enabled-features
   #{"fdata/shape-data-type"})