🐛 Fix incorrect feature handling on absorb-library! fn

Used in shared flag assignation and library deletion

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

@@ -635,7 +635,7 @@
   (pu/with-open [input (zstd-input-stream input)
                  input (io/data-input-stream input)]
     (binding [*state* (volatile! {:media [] :index {}})]
-      (let [team      (teams/get-team options
+      (let [team      (teams/get-team conn
                                       :profile-id profile-id
                                       :project-id project-id)
 

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

@@ -12,6 +12,7 @@
    [app.common.features :as cfeat]
    [app.common.files.helpers :as cfh]
    [app.common.files.migrations :as fmg]
+   [app.common.logging :as l]
    [app.common.schema :as sm]
    [app.common.schema.desc-js-like :as-alias smdj]
    [app.common.spec :as us]
@@ -186,9 +187,14 @@
 
 (defn load-pointer
   [conn file-id id]
+
+  (dm/assert!
+   "expected valid connection"
+   (db/connection? conn))
+
   (let [{:keys [content]} (db/get conn :file-data-fragment
                                   {:id id :file-id file-id}
-                                  {:columns [:content]
+                                  {::db/columns [:content]
                                    ::db/check-deleted? false})]
     (when-not content
       (ex/raise :type :internal
@@ -199,16 +205,6 @@
 
     (blob/decode content)))
 
-(defn- load-all-pointers!
-  [{:keys [data] :as file}]
-  (doseq [[_id page] (:pages-index data)]
-    (when (pmap/pointer-map? page)
-      (pmap/load! page)))
-  (doseq [[_id component] (:components data)]
-    (when (pmap/pointer-map? component)
-      (pmap/load! component)))
-  file)
-
 (defn persist-pointers!
   [conn file-id]
   (doseq [[id item] @pmap/*tracked*]
@@ -282,24 +278,30 @@
      [:project-id {:optional true} ::sm/uuid]]))
 
 (defn get-file
-  ([conn id] (get-file conn id nil))
+  [conn id & {:keys [project-id migrate?
-  ([conn id project-id]
+                     include-deleted?
-
+                     lock-for-update?]
+              :or {include-deleted? false
+                   lock-for-update? false}}]
   (dm/assert!
    "expected raw connection"
    (db/connection? conn))
 
   (binding [pmap/*load-fn* (partial load-pointer conn id)
-             pmap/*tracked* (atom {})
+            pmap/*tracked* (pmap/create-tracked)
             cfeat/*new*    (atom #{})]
 
     (let [params (merge {:id id}
                         (when (some? project-id)
                           {:project-id project-id}))
 
-           file   (-> (db/get conn :file params)
+          file   (-> (db/get conn :file params
+                             {::db/check-deleted? (not include-deleted?)
+                              ::db/remove-deleted? (not include-deleted?)
+                              ::db/for-update? lock-for-update?})
                      (decode-row)
-                      (fmg/migrate-file))]
+                     (cond-> migrate?
+                       (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
@@ -317,7 +319,7 @@
                        {:id id})
            (persist-pointers! conn id)
            (assoc file :features features))
-         file)))))
+         file))))
 
 (defn get-minimal-file
   [{:keys [::db/pool] :as cfg} id]
@@ -338,12 +340,12 @@
   (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}]
                     (let [perms (get-permissions conn profile-id id)]
                       (check-read-permissions! perms)
-                      (let [team (teams/get-team cfg
+                      (let [team (teams/get-team conn
                                                  :profile-id profile-id
                                                  :project-id project-id
                                                  :file-id id)
 
-                            file (-> (get-file conn id project-id)
+                            file (-> (get-file conn id :project-id project-id)
                                      (assoc :permissions perms)
                                      (check-version!))
 
@@ -502,7 +504,7 @@
               :code :params-validation
               :hint "page-id is required when object-id is provided"))
 
-  (let [team (teams/get-team cfg
+  (let [team (teams/get-team conn
                              :profile-id profile-id
                              :file-id file-id)
 
@@ -583,11 +585,9 @@
           (library-summary [{:keys [id data] :as file}]
             (binding [pmap/*load-fn* (partial load-pointer conn id)]
               (let [load-objects      (fn [component]
-                                   (binding [pmap/*load-fn* (partial load-pointer conn id)]
+                                        (ctf/load-component-objects data component))
-                                     (ctf/load-component-objects data component)))
                     components-sample (-> (assets-sample (ctkl/components data) 4)
-                                          (update :sample
+                                          (update :sample #(mapv load-objects %)))]
-                                                  #(map load-objects %)))]
                 {:components components-sample
                  :media (assets-sample (:media data) 3)
                  :colors (assets-sample (:colors data) 3)
@@ -752,12 +752,12 @@
   (db/tx-run! cfg
               (fn [{:keys [::db/conn] :as cfg}]
                 (check-read-permissions! conn profile-id id)
-                (let [team (teams/get-team cfg
+                (let [team (teams/get-team conn
                                            :profile-id profile-id
                                            :project-id project-id
                                            :file-id id)
 
-                      file (get-file conn id project-id)]
+                      file (get-file conn id :project-id project-id)]
 
                   (-> (cfeat/get-team-enabled-features cf/flags team)
                       (cfeat/check-client-features! (:features params))
@@ -818,16 +818,6 @@
 
 ;; --- MUTATION COMMAND: set-file-shared
 
-(defn- unlink-files!
-  [conn {:keys [id]}]
-  (db/delete! conn :file-library-rel {:library-file-id id}))
-
-(defn- set-file-shared!
-  [conn {:keys [id is-shared] :as params}]
-  (db/update! conn :file
-              {:is-shared is-shared}
-              {:id id}))
-
 (def sql:get-referenced-files
   "SELECT f.id
      FROM file_library_rel AS flr
@@ -836,46 +826,97 @@
       AND (f.deleted_at IS NULL OR f.deleted_at > now())
     ORDER BY f.created_at ASC;")
 
+(defn- absorb-library-by-file!
+  [conn ldata file-id]
+  (binding [pmap/*load-fn* (partial load-pointer conn file-id)
+            pmap/*tracked* (pmap/create-tracked)]
+    (let [file (-> (get-file conn file-id
+                             :include-deleted? true
+                             :lock-for-update? true)
+                   (update :data ctf/absorb-assets ldata))]
+
+      (l/trc :hint "library absorbed"
+             :library-id (str (:id ldata))
+             :file-id (str file-id))
+
+      (db/update! conn :file
+                  {:revn (inc (:revn file))
+                   :data (blob/encode (:data file))
+                   :modified-at (dt/now)}
+                  {:id file-id})
+
+      (persist-pointers! conn file-id))))
+
 (defn- absorb-library!
   "Find all files using a shared library, and absorb all library assets
   into the file local libraries"
   [conn {:keys [id] :as library}]
   (let [ldata (binding [pmap/*load-fn* (partial load-pointer conn id)]
-                (-> library decode-row (process-pointers deref) fmg/migrate-file :data))
+                (-> library (process-pointers deref) :data))
-        rows  (db/exec! conn [sql:get-referenced-files id])]
+        ids   (->> (db/exec! conn [sql:get-referenced-files id])
-    (doseq [file-id (map :id rows)]
+                   (map :id))]
-      (binding [pmap/*load-fn* (partial load-pointer conn file-id)
-                pmap/*tracked* (atom {})]
-        (let [file (-> (db/get-by-id conn :file file-id
-                                     ::db/check-deleted? false
-                                     ::db/remove-deleted? false)
-                       (decode-row)
-                       (load-all-pointers!)
-                       (fmg/migrate-file))
-              data (ctf/absorb-assets (:data file) ldata)]
-          (db/update! conn :file
-                      {:revn (inc (:revn file))
-                       :data (blob/encode data)
-                       :modified-at (dt/now)}
-                      {:id file-id})
-          (persist-pointers! conn file-id))))))
 
-(def ^:private schema:set-file-shared
+    (l/trc :hint "absorbing library"
+           :library-id (str id)
+           :files (str/join "," (map str ids)))
+
+    (run! (partial absorb-library-by-file! conn ldata) ids)))
+
+(def ^:private
+  schema:set-file-shared
+  (sm/define
     [:map {:title "set-file-shared"}
      [:id ::sm/uuid]
-   [:is-shared :boolean]])
+     [:is-shared :boolean]]))
 
 (sv/defmethod ::set-file-shared
   {::doc/added "1.17"
    ::webhooks/event? true
    ::sm/params schema:set-file-shared}
-  [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id is-shared] :as params}]
+  [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id] :as params}]
   (db/with-atomic [conn pool]
     (check-edition-permissions! conn profile-id id)
-    (let [file (set-file-shared! conn params)]
+    (let [file (db/get-by-id conn id {:columns [:id :name :is-shared]})
-      (when-not is-shared
+          file (cond
+                 (and (true? (:is-shared file))
+                      (false? (:is-shared params)))
+                 ;; When we disable shared flag on an already shared
+                 ;; file, we need to perform more complex operation,
+                 ;; so in this case we retrieve the complete file and
+                 ;; perform all required validations.
+                 (let [file (-> (get-file conn id :lock-for-update? true)
+                                (check-version!)
+                                (assoc :is-shared false))
+                       team (teams/get-team conn
+                                            :profile-id profile-id
+                                            :project-id (:project-id file))]
+
+                   (-> (cfeat/get-team-enabled-features cf/flags team)
+                       (cfeat/check-client-features! (:features params))
+                       (cfeat/check-file-features! (:features file)))
+
                    (absorb-library! conn file)
-        (unlink-files! conn file))
+
+                   (db/delete! conn :file-library-rel {:library-file-id id})
+                   (db/update! conn :file
+                               {:is-shared false}
+                               {:id id})
+                   file)
+
+                 (and (false? (:is-shared file))
+                      (true? (:is-shared params)))
+                 (let [file (assoc file :is-shared true)]
+                   (db/update! conn :file
+                               {:is-shared false}
+                               {:id id})
+                   file)
+
+                 :else
+                 (ex/raise :type :validation
+                           :code :invalid-shared-state
+                           :hint "unexpected state found"
+                           :params-is-shared (:is-shared params)
+                           :file-is-shared (:is-shared file)))]
 
       (rph/with-meta
         (select-keys file [:id :name :is-shared])
@@ -886,14 +927,17 @@
 ;; --- MUTATION COMMAND: delete-file
 
 (defn- mark-file-deleted!
-  [conn {:keys [id]}]
+  [conn file-id]
   (db/update! conn :file
               {:deleted-at (dt/now)}
-              {:id id}))
+              {:id file-id}
+              {::db/columns [:id :name :is-shared :project-id :created-at :modified-at]}))
 
-(def ^:private schema:delete-file
+(def ^:private
+  schema:delete-file
+  (sm/define
     [:map {:title "delete-file"}
-   [:id ::sm/uuid]])
+     [:id ::sm/uuid]]))
 
 (sv/defmethod ::delete-file
   {::doc/added "1.17"
@@ -902,9 +946,28 @@
   [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id] :as params}]
   (db/with-atomic [conn pool]
     (check-edition-permissions! conn profile-id id)
-    (let [file (mark-file-deleted! conn params)]
+    (let [file (mark-file-deleted! conn id)]
+
+      ;; NOTE: when a file is a shared library, then we proceed to load
+      ;; the whole file, proceed with feature checking and properly execute
+      ;; the absorb-library procedure
       (when (:is-shared file)
-        (absorb-library! conn file))
+        (let [file (-> (get-file conn id
+                                 :lock-for-update? true
+                                 :include-deleted? true)
+                       (check-version!))
+
+              team (teams/get-team conn
+                                   :profile-id profile-id
+                                   :project-id (:project-id file))]
+
+
+
+          (-> (cfeat/get-team-enabled-features cf/flags team)
+              (cfeat/check-client-features! (:features params))
+              (cfeat/check-file-features! (:features file)))
+
+          (absorb-library! conn file)))
 
       (rph/with-meta (rph/wrap)
         {::audit/props {:project-id (:project-id file)
@@ -923,10 +986,12 @@
   [conn {:keys [file-id library-id] :as params}]
   (db/exec-one! conn [sql:link-file-to-library file-id library-id]))
 
-(def ^:private schema:link-file-to-library
+(def ^:private
+  schema:link-file-to-library
+  (sm/define
     [:map {:title "link-file-to-library"}
      [:file-id ::sm/uuid]
-   [:library-id ::sm/uuid]])
+     [:library-id ::sm/uuid]]))
 
 (sv/defmethod ::link-file-to-library
   {::doc/added "1.17"

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

@@ -100,7 +100,7 @@
   (db/tx-run! cfg
               (fn [{:keys [::db/conn] :as cfg}]
                 (projects/check-edition-permissions! conn profile-id project-id)
-                (let [team     (teams/get-team cfg
+                (let [team     (teams/get-team conn
                                                :profile-id profile-id
                                                :project-id project-id)
                       team-id  (:id team)

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

@@ -46,7 +46,7 @@
   [cfg {:keys [::rpc/profile-id project-id] :as params}]
   (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}]
                     (projects/check-edition-permissions! conn profile-id project-id)
-                    (let [team     (teams/get-team cfg
+                    (let [team     (teams/get-team conn
                                                    :profile-id profile-id
                                                    :project-id project-id)
 

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

@@ -216,7 +216,7 @@
   (db/run! cfg (fn [{:keys [::db/conn] :as cfg}]
                  (files/check-read-permissions! conn profile-id file-id)
 
-                 (let [team     (teams/get-team cfg
+                 (let [team     (teams/get-team conn
                                                 :profile-id profile-id
                                                 :file-id file-id)
 

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

@@ -148,7 +148,7 @@
                     (db/xact-lock! conn id)
 
                     (let [file     (get-file conn id)
-                          team     (teams/get-team cfg
+                          team     (teams/get-team conn
                                                    :profile-id profile-id
                                                    :team-id (:team-id file))
 
@@ -304,9 +304,9 @@
                         (not skip-validate))
                (->> (files/get-file-libraries conn (:id file))
                     (into [file] (map (fn [{:keys [id]}]
-                                        (binding [pmap/*load-fn* (partial files/load-pointer conn id)]
+                                        (binding [pmap/*load-fn* (partial files/load-pointer conn id)
-                                          (-> (db/get conn :file {:id id})
+                                                  pmap/*tracked* nil]
-                                              (files/decode-row)
+                                          (-> (files/get-file conn id :migrate? false)
                                               (files/process-pointers deref) ; ensure all pointers resolved
                                               (fmg/migrate-file))))))
                     (d/index-by :id)))]

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

@@ -317,8 +317,8 @@
       (proj/check-edition-permissions! conn profile-id project-id))
 
     ;; Check the team compatibility
-    (let [orig-team (teams/get-team cfg :profile-id profile-id :project-id (first source))
+    (let [orig-team (teams/get-team conn :profile-id profile-id :project-id (first source))
-          dest-team (teams/get-team cfg :profile-id profile-id :project-id project-id)]
+          dest-team (teams/get-team conn :profile-id profile-id :project-id project-id)]
       (cfeat/check-teams-compatibility! orig-team dest-team))
 
     ;; move all files to the project
@@ -374,8 +374,8 @@
     (teams/check-edition-permissions! conn profile-id team-id)
 
     ;; Check the teams compatibility
-    (let [orig-team (teams/get-team cfg :profile-id profile-id :team-id (:team-id project))
+    (let [orig-team (teams/get-team conn :profile-id profile-id :team-id (:team-id project))
-          dest-team (teams/get-team cfg :profile-id profile-id :team-id team-id)]
+          dest-team (teams/get-team conn :profile-id profile-id :team-id team-id)]
       (cfeat/check-teams-compatibility! orig-team dest-team))
 
     ;; move project to the destination team

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

@@ -149,11 +149,17 @@
 (sv/defmethod ::get-team
   {::doc/added "1.17"
    ::sm/params schema:get-team}
-  [cfg {:keys [::rpc/profile-id id file-id]}]
+  [{:keys [::db/pool]} {:keys [::rpc/profile-id id file-id]}]
-  (db/tx-run! cfg #(get-team % :profile-id profile-id :team-id id :file-id file-id)))
+  (get-team pool :profile-id profile-id :team-id id :file-id file-id))
 
 (defn get-team
   [conn & {:keys [profile-id team-id project-id file-id] :as params}]
+
+  (dm/assert!
+   "connection or pool is mandatory"
+   (or (db/connection? conn)
+       (db/pool? conn)))
+
   (dm/assert!
    "profile-id is mandatory"
    (uuid? profile-id))
@@ -655,7 +661,7 @@
 
 (defn update-team-photo
   [{:keys [::db/pool ::sto/storage] :as cfg} {:keys [profile-id team-id] :as params}]
-  (let [team  (get-team cfg :profile-id profile-id :team-id team-id)
+  (let [team  (get-team pool :profile-id profile-id :team-id team-id)
         photo (profile/upload-photo cfg params)]
 
     (db/with-atomic [conn pool]

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

@@ -86,7 +86,8 @@
    ::doc/added "1.17"
    ::sm/params schema:get-view-only-bundle}
   [system {:keys [::rpc/profile-id file-id share-id] :as params}]
-  (db/run! system (fn [{:keys [::db/conn] :as system}]
+  (db/run! system
+           (fn [{:keys [::db/conn] :as system}]
              (let [perms  (files/get-permissions conn profile-id file-id share-id)
                    params (-> params
                               (assoc ::perms perms)

backend/src/app/util/pointer_map.clj

@@ -60,6 +60,10 @@
 
 (declare create)
 
+(defn create-tracked
+  []
+  (atom {}))
+
 (defprotocol IPointerMap
   (get-id [_])
   (load! [_])

backend/test/backend_tests/helpers.clj

@@ -252,9 +252,10 @@
                                                  params)))))))
 
 (defn mark-file-deleted*
-  ([params] (mark-file-deleted* *system* params))
+  ([params]
+   (mark-file-deleted* *system* params))
   ([conn {:keys [id] :as params}]
-   (#'files/mark-file-deleted! conn {:id id})))
+   (#'files/mark-file-deleted! conn id)))
 
 (defn create-team*
   ([i params] (create-team* *system* i params))

common/src/app/common/files/migrations.cljc

@@ -51,8 +51,9 @@
 (defn migrated?
   [{:keys [data] :as file}]
   (or (::migrated file)
-      (> (:version data)
+      (let [version (:version data)]
-         (::orig-version file))))
+        (> version
+           (::orig-version file version)))))
 
 ;; Default handler, noop
 (defmethod migrate :default [data] data)