Merge pull request #3633 from penpot/niwinz-develop-bugfixes-5

🐛 Minor bugfixes and logging improvements

backend/resources/log4j2-devenv.xml

@@ -22,7 +22,7 @@
     <Logger name="org.postgresql" level="error" />
 
     <Logger name="app.rpc.commands.binfile" level="debug" />
-    <Logger name="app.storage.tmp" level="debug" />
+    <Logger name="app.storage.tmp" level="info" />
     <Logger name="app.worker" level="trace" />
     <Logger name="app.msgbus" level="info" />
     <Logger name="app.http.websocket" level="info" />
@@ -31,6 +31,7 @@
     <Logger name="app.rpc.rlimit" level="info" />
     <Logger name="app.rpc.climit" level="info" />
     <Logger name="app.rpc.mutations.files" level="info" />
+    <Logger name="app.common.files.migrations" level="debug" />
 
     <Logger name="app.loggers" level="debug" additivity="false">
       <AppenderRef ref="main" level="debug" />

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

@@ -659,20 +659,24 @@
                   :hint "the penpot file seems corrupt, found unexpected uuid (file-id)"))
 
       ;; Update index using with media
-      (l/debug :hint "update index with media" ::l/sync? true)
+      (l/dbg :hint "update index with media" ::l/sync? true)
       (vswap! *state* update :index update-index (map :id media'))
 
       ;; Store file media for later insertion
-      (l/debug :hint "update media references" ::l/sync? true)
+      (l/dbg :hint "update media references" ::l/sync? true)
       (vswap! *state* update :media into (map #(update % :id lookup-index)) media')
 
-      (l/debug :hint "processing file" :file-id file-id ::features features ::l/sync? true)
-
       (binding [ffeat/*current* features
                 ffeat/*wrap-with-objects-map-fn* (if (features "storage/objects-map") omap/wrap identity)
                 ffeat/*wrap-with-pointer-map-fn* (if (features "storage/pointer-map") pmap/wrap identity)
                 pmap/*tracked* (atom {})]
 
+        (l/dbg :hint "processing file"
+               :id file-id
+               :features features
+               :version (-> file :data :version)
+               ::l/sync? true)
+
         (let [file-id' (lookup-index file-id)
               data     (-> (:data file)
                            (assoc :id file-id')
@@ -790,7 +794,7 @@
          index index]
     (if-let [id (first items)]
       (let [new-id (if (::overwrite? *options*) id (uuid/next))]
-        (l/debug :fn "update-index" :id id :new-id new-id ::l/sync? true)
+        (l/trc :fn "update-index" :id id :new-id new-id ::l/sync? true)
         (recur (rest items)
                (assoc index id new-id)))
       index)))

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

@@ -798,11 +798,11 @@
 
 ;; --- MUTATION COMMAND: set-file-shared
 
-(defn unlink-files
+(defn- unlink-files!
-  [conn {:keys [id] :as params}]
+  [conn {:keys [id]}]
   (db/delete! conn :file-library-rel {:library-file-id id}))
 
-(defn set-file-shared
+(defn- set-file-shared!
   [conn {:keys [id is-shared] :as params}]
   (db/update! conn :file
               {:is-shared is-shared}
@@ -813,49 +813,50 @@
      FROM file_library_rel AS flr
     INNER JOIN file AS f ON (f.id = flr.file_id)
     WHERE flr.library_file_id = ?
+      AND (f.deleted_at IS NULL OR f.deleted_at > now())
     ORDER BY f.created_at ASC;")
 
-(defn absorb-library
+(defn- absorb-library!
   "Find all files using a shared library, and absorb all library assets
   into the file local libraries"
-  [conn {:keys [id] :as params}]
+  [conn {:keys [id] :as library}]
-  (let [library (db/get-by-id conn :file id)]
+  (let [ldata (binding [pmap/*load-fn* (partial load-pointer conn id)]
-    (when (:is-shared library)
+                (-> library decode-row (process-pointers deref) pmg/migrate-file :data))
-      (let [ldata (binding [pmap/*load-fn* (partial load-pointer conn id)]
+        rows  (db/exec! conn [sql:get-referenced-files id])]
-                    (-> library decode-row load-all-pointers! pmg/migrate-file :data))
+    (doseq [file-id (map :id rows)]
-            rows  (db/exec! conn [sql:get-referenced-files id])]
+      (binding [pmap/*load-fn* (partial load-pointer conn file-id)
-        (doseq [file-id (map :id rows)]
+                pmap/*tracked* (atom {})]
-          (binding [pmap/*load-fn* (partial load-pointer conn file-id)
+        (let [file (-> (db/get-by-id conn :file file-id
-                    pmap/*tracked* (atom {})]
+                                     ::db/check-deleted? false
-            (let [file (-> (db/get-by-id conn :file file-id
+                                     ::db/remove-deleted? false)
-                                         ::db/check-deleted? false
+                       (decode-row)
-                                         ::db/remove-deleted? false)
+                       (load-all-pointers!)
-                           (decode-row)
+                       (pmg/migrate-file))
-                           (load-all-pointers!)
+              data (ctf/absorb-assets (:data file) ldata)]
-                           (pmg/migrate-file))
+          (db/update! conn :file
-                  data (ctf/absorb-assets (:data file) ldata)]
+                      {:revn (inc (:revn file))
-              (db/update! conn :file
+                       :data (blob/encode data)
-                          {:revn (inc (:revn file))
+                       :modified-at (dt/now)}
-                           :data (blob/encode data)
+                      {:id file-id})
-                           :modified-at (dt/now)}
+          (persist-pointers! conn file-id))))))
-                          {:id file-id})
-              (persist-pointers! conn file-id))))))))
 
-(s/def ::set-file-shared
+(def ^:private schema:set-file-shared
-  (s/keys :req [::rpc/profile-id]
+  [:map {:title "set-file-shared"}
-          :req-un [::id ::is-shared]))
+   [:id ::sm/uuid]
+   [:is-shared :boolean]])
 
 (sv/defmethod ::set-file-shared
   {::doc/added "1.17"
-   ::webhooks/event? true}
+   ::webhooks/event? true
+   ::sm/params schema:set-file-shared}
   [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id is-shared] :as params}]
   (db/with-atomic [conn pool]
     (check-edition-permissions! conn profile-id id)
-    (when-not is-shared
+    (let [file (set-file-shared! conn params)]
-      (absorb-library conn params)
+      (when-not is-shared
-      (unlink-files conn params))
+        (absorb-library! conn file)
+        (unlink-files! conn file))
 
-    (let [file (set-file-shared conn params)]
       (rph/with-meta
         (select-keys file [:id :name :is-shared])
         {::audit/props {:name (:name file)
@@ -864,24 +865,26 @@
 
 ;; --- MUTATION COMMAND: delete-file
 
-(defn mark-file-deleted
+(defn- mark-file-deleted!
-  [conn {:keys [id] :as params}]
+  [conn {:keys [id]}]
   (db/update! conn :file
               {:deleted-at (dt/now)}
               {:id id}))
 
-(s/def ::delete-file
+(def ^:private schema:delete-file
-  (s/keys :req [::rpc/profile-id]
+  [:map {:title "delete-file"}
-          :req-un [::id]))
+   [:id ::sm/uuid]])
 
 (sv/defmethod ::delete-file
   {::doc/added "1.17"
-   ::webhooks/event? true}
+   ::webhooks/event? true
+   ::sm/params schema:delete-file}
   [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id] :as params}]
   (db/with-atomic [conn pool]
     (check-edition-permissions! conn profile-id id)
-    (absorb-library conn params)
+    (let [file (mark-file-deleted! conn params)]
-    (let [file (mark-file-deleted conn params)]
+      (when (:is-shared file)
+        (absorb-library! conn file))
 
       (rph/with-meta (rph/wrap)
         {::audit/props {:project-id (:project-id file)

backend/test/backend_tests/helpers.clj

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

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

@@ -33,7 +33,7 @@
    (if (= (:version data) to-version)
      data
      (let [migrate-fn #(do
-                         (l/trc :hint "migrate file" :id (:id %) :version-from %2 :version-to (inc %2))
+                         (l/dbg :hint "migrate file" :id (:id %) :version-from %2 :version-to (inc %2))
                          (migrate (assoc %1 :version (inc %2))))]
        (reduce migrate-fn data (range (:version data 0) to-version))))))