diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index a980fc3dd5..0f907d0d9d 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -616,6 +616,8 @@ (let [file (get-file-with-summary cfg id)] (assoc file :library-file-ids (db/decode-pgarray library-file-ids #{}))) (catch Throwable cause + (when (instance? InterruptedException cause) + (.interrupt (Thread/currentThread))) (l/error :hint "unable to load shared file" :file-id (str id) :team-id (str team-id) diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 70b18e986a..5074d76e8b 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -11,7 +11,6 @@ [app.common.files.changes-builder :as pcb] [app.common.files.migrations :as fmg] [app.common.files.tokens :as cfo] - [app.common.logging :as l] [app.common.pprint :as pp] [app.common.thumbnails :as thc] [app.common.time :as ct] @@ -3255,19 +3254,6 @@ (t/is (= (:id team) (get-in (meta (:result out)) [:app.loggers.audit/props :team-id]))))) -(defn- wait-for-log-record - "The first captured log record whose message hint is `hint`, waiting up - to two seconds because logging is asynchronous." - [records hint] - (loop [tries 0] - (if-let [record (->> @records - (filter #(= hint (get-in % [::l/props :hint]))) - (first))] - record - (when (< tries 200) - (Thread/sleep 10) - (recur (inc tries)))))) - (t/deftest get-team-shared-files-skips-files-that-fail-to-load (let [profile (th/create-profile* 1) file1 (th/create-file* 1 {:profile-id (:id profile) @@ -3283,26 +3269,16 @@ (doseq [cause [(ex-info "broken file" {:type :assertion}) (AssertionError. "point instance expected") (StackOverflowError.)]] - (let [records (atom []) - broken (fn [cfg id] + (let [broken (fn [cfg id] (if (= id (:id file1)) (throw cause) (get-file cfg id))) - out (try - (add-watch l/log-record ::capture - (fn [_ _ _ record] (swap! records conj record))) - (with-redefs-fn {#'files/get-file-with-summary broken} - #(th/command! {::th/type :get-team-shared-files - ::rpc/profile-id (:id profile) - :team-id (:default-team-id profile)})) - (finally - (remove-watch l/log-record ::capture)))] + out (with-redefs-fn {#'files/get-file-with-summary broken} + #(th/command! {::th/type :get-team-shared-files + ::rpc/profile-id (:id profile) + :team-id (:default-team-id profile)}))] (t/is (th/success? out) (str "skips file failing with " (class cause))) - (t/is (= #{(:id file2)} (into #{} (map :id) (:result out)))) - (t/is (= (str (:id file1)) - (some-> (wait-for-log-record records "unable to load shared file") - (get-in [::l/props :file-id]))) - (str "reports file failing with " (class cause))))))) + (t/is (= #{(:id file2)} (into #{} (map :id) (:result out)))))))) (t/deftest get-team-shared-files-repairs-invalid-children-ids ;; A stored shared file that lists nil and a dangling id in :shapes and diff --git a/common/test/common_tests/files_migrations_test.cljc b/common/test/common_tests/files_migrations_test.cljc index 7d877c671d..bbfb359a7f 100644 --- a/common/test/common_tests/files_migrations_test.cljc +++ b/common/test/common_tests/files_migrations_test.cljc @@ -17,6 +17,7 @@ [app.common.test-helpers.shapes :as ths] [app.common.test-helpers.variants :as thv] [app.common.types.file :as ctf] + [app.common.types.objects-map :as omap] [app.common.types.path :as path] [app.common.types.shape :as cts] [app.common.types.token :as ctt] @@ -888,7 +889,20 @@ data' (cfm/migrate-data data migration-id)] (t/is (identical? (get-in data [:pages-index page-id :objects]) - (get-in data' [:pages-index page-id :objects]))))) + (get-in data' [:pages-index page-id :objects])))) + + ;; Same property through the objects-map wrapper the backend uses on the + ;; unrealized save path, whose `assoc` marks the page as modified. + (let [migration-id "0036-fix-shape-children-ids" + file (-> (thf/sample-file :file1) + (tho/add-frame-with-child :frame1 :child1)) + page-id (thf/current-page-id file) + objects (omap/wrap (get-in (ctf/file-data file) [:pages-index page-id :objects])) + data (assoc-in (ctf/file-data file) [:pages-index page-id :objects] objects) + data' (cfm/migrate-data data migration-id)] + + (t/is (omap/objects-map? objects) "wrapped in an ObjectsMap") + (t/is (identical? objects (get-in data' [:pages-index page-id :objects]))))) (t/deftest migration-0036-fixes-children-ids-of-files-that-applied-0033 ;; Files that applied 0033 before it repaired children never run it @@ -906,6 +920,21 @@ (t/is (not (cfm/need-migration? file')) "new migration recorded") (t/is (= [child1-id] (:shapes (ths/get-shape file' :frame1)))))) +(t/deftest migrations-leave-shapes-without-the-shapes-key-untouched + ;; Only containers have a :shapes key; other shapes come out of the + ;; repair unchanged, without a :shapes key being added. + (doseq [migration-id ["0033-normalize-main-instance-names" + "0036-fix-shape-children-ids"]] + (let [file (-> (thf/sample-file :file1) + (tho/add-frame-with-child :frame1 :child1)) + child (ths/get-shape file :child1) + data' (cfm/migrate-data (ctf/file-data file) migration-id) + child' (ths/get-shape (assoc file :data data') :child1)] + + (t/is (not (contains? child :shapes)) "the fixture shape has no :shapes key") + (t/is (not (contains? child' :shapes)) (str migration-id)) + (t/is (= child child') migration-id)))) + (t/deftest migration-0033-fixes-children-ids-through-file-migration ;; 0032 adds the uuid of a child listed as a string, so the child is ;; listed twice before 0033; the file must still pass validation.