Browse Source

fix(sync): malformed remote tx can include a temp entity id

without :block/uuid.
Tienson Qin 1 week ago
parent
commit
8d48e93ccb

+ 45 - 1
src/main/frontend/worker/sync.cljs

@@ -499,6 +499,49 @@
              (= :block/uuid (first x)))
     (second x)))
 
+(defn- drop-anonymous-temp-entity-datoms
+  "Drop malformed temp entities from remote txs.
+   A temp entity must declare one identity attr (:block/uuid or :db/ident)
+   in its :db/add datoms; otherwise it can create anonymous entities that fail validation."
+  [db tx-data]
+  (let [identity-attrs #{:block/uuid :db/ident}
+        temp-id? (fn [x]
+                   (or (string? x)
+                       (and (integer? x) (neg? x))))
+        add-attrs-by-entity
+        (reduce (fn [acc item]
+                  (if (and (vector? item)
+                           (= :db/add (first item))
+                           (>= (count item) 4))
+                    (update acc (second item) (fnil conj #{}) (nth item 2))
+                    acc))
+                {}
+                tx-data)
+        dropped-entities
+        (->> add-attrs-by-entity
+             (keep (fn [[entity attrs]]
+                     (when (and (temp-id? entity)
+                                (empty? (set/intersection identity-attrs attrs)))
+                       entity)))
+             set)]
+    (if (seq dropped-entities)
+      (let [tx-data' (->> tx-data
+                          (remove (fn [item]
+                                    (and (vector? item)
+                                         (>= (count item) 2)
+                                         (contains? dropped-entities (second item)))))
+                          (remove (fn [item]
+                                    (and (vector? item)
+                                         (>= (count item) 4)
+                                         (keyword? (nth item 2))
+                                         (= :db.type/ref (:db/valueType (d/entity db (nth item 2))))
+                                         (contains? dropped-entities (nth item 3))))))]
+        (log/warn :db-sync/drop-anonymous-temp-entities
+                  {:count (count dropped-entities)
+                   :entities dropped-entities})
+        tx-data')
+      tx-data)))
+
 (defn- sanitize-tx-data
   [db tx-data local-deleted-ids]
   (let [sanitized-tx-data (->> tx-data
@@ -1020,7 +1063,8 @@
   [repo client tx-data*]
   (if-let [conn (worker-state/get-datascript-conn repo)]
     (let [tx-data (->> tx-data*
-                       (db-normalize/remove-retract-entity-ref @conn))
+                       (db-normalize/remove-retract-entity-ref @conn)
+                       (#(drop-anonymous-temp-entity-datoms @conn %)))
           local-txs (pending-txs repo)
           reversed-tx-data (get-reverse-tx-data local-txs)
           has-local-changes? (seq reversed-tx-data)

+ 2 - 2
src/test/frontend/components/query_test.cljs

@@ -5,6 +5,6 @@
 (deftest grouped-by-page-result-detection-supports-partial-page-refs
   (let [result [[{:db/id 42}
                  [{:block/uuid (random-uuid)}]]]]
-    (is (true? (#'frontend.components.query/grouped-by-page-result? result true))
+    (is (true? (#'query/grouped-by-page-result? result true))
         "Grouped query results with page refs that only include :db/id should still be recognized")
-    (is (false? (#'frontend.components.query/grouped-by-page-result? result false)))))
+    (is (false? (#'query/grouped-by-page-result? result false)))))

+ 1 - 0
src/test/frontend/db/model_test.cljs

@@ -4,6 +4,7 @@
             [frontend.db :as db]
             [frontend.db.conn :as conn]
             [frontend.db.model :as model]
+            [frontend.db.utils]
             [frontend.test.helper :as test-helper :refer [load-test-files]]))
 
 (use-fixtures :each {:before test-helper/start-test-db!

+ 0 - 38
src/test/frontend/mobile/audio_recorder_test.cljs

@@ -1,38 +0,0 @@
-(ns frontend.mobile.audio-recorder-test
-  (:require [cljs.test :refer [is testing]]
-            [frontend.handler.notification :as notification]
-            [frontend.mobile.audio-recorder :as audio-recorder]
-            [frontend.test.helper :include-macros true :refer [deftest-async]]
-            [logseq.shui.ui :as shui]
-            [promesa.core :as p]))
-
-(deftest-async start-recording-shows-warning-when-microphone-permission-denied
-  (testing "Shows actionable warning and closes recorder popup when mic permission is denied"
-    (let [warning (atom nil)
-          popup-hidden? (atom false)]
-      (p/with-redefs
-       [notification/show! (fn [content & _]
-                             (reset! warning content))
-        shui/popup-hide! (fn []
-                           (reset! popup-hidden? true))]
-        (p/let [_ (audio-recorder/start-recording! #js {:startRecording
-                                                        (fn []
-                                                          (p/rejected (js/Error. "Error accessing the microphone: Permission denied")))})]
-          (is (string? @warning))
-          (is (re-find #"Settings" @warning))
-          (is (true? @popup-hidden?)))))))
-
-(deftest-async start-recording-does-not-show-warning-for-non-permission-errors
-  (testing "Avoids permission warning for unrelated start recording failures"
-    (let [warning (atom nil)
-          popup-hidden? (atom false)]
-      (p/with-redefs
-       [notification/show! (fn [content & _]
-                             (reset! warning content))
-        shui/popup-hide! (fn []
-                           (reset! popup-hidden? true))]
-        (p/let [_ (audio-recorder/start-recording! #js {:startRecording
-                                                        (fn []
-                                                          (p/rejected (js/Error. "Error: No microphone device found")))})]
-          (is (nil? @warning))
-          (is (false? @popup-hidden?)))))))

+ 0 - 79
src/test/frontend/mobile/camera_test.cljs

@@ -1,79 +0,0 @@
-(ns frontend.mobile.camera-test
-  (:require [cljs.test :refer [is testing]]
-            [frontend.handler.editor :as editor-handler]
-            [frontend.handler.notification :as notification]
-            [frontend.mobile.camera :as mobile-camera]
-            [frontend.state :as state]
-            [frontend.test.helper :include-macros true :refer [deftest-async]]
-            [promesa.core :as p]))
-
-(deftest-async embed-photo-uses-provided-id
-  (testing "Uses explicit editor id so capture/upload still works if focused input id is temporarily nil"
-    (let [upload-id (atom nil)]
-      (p/with-redefs
-       [mobile-camera/take-or-choose-photo (fn [] (p/resolved #js {:name "photo.jpeg"}))
-        state/get-edit-block (constantly {:block/format :markdown})
-        editor-handler/upload-asset! (fn [id _files _format _uploading? _drop-or-paste?]
-                                       (reset! upload-id id)
-                                       (p/resolved nil))]
-        (p/let [_ (mobile-camera/embed-photo "editor-id")]
-          (is (= "editor-id" @upload-id)))))))
-
-(deftest-async embed-photo-skips-upload-when-no-photo
-  (testing "Doesn't trigger upload pipeline when camera returns nil photo"
-    (let [upload-called? (atom false)]
-      (p/with-redefs
-       [mobile-camera/take-or-choose-photo (fn [] (p/resolved nil))
-        state/get-edit-block (constantly {:block/format :markdown})
-        editor-handler/upload-asset! (fn [& _]
-                                       (reset! upload-called? true)
-                                       (p/resolved nil))]
-        (p/let [_ (mobile-camera/embed-photo "editor-id")]
-          (is (false? @upload-called?)))))))
-
-(deftest-async embed-photo-still-allows-photo-picking-when-camera-permission-denied
-  (testing "Does not pre-block getPhoto by camera permission so users can still pick existing photos"
-    (let [upload-called? (atom false)
-          get-photo-called? (atom false)
-          warning (atom nil)]
-      (p/with-redefs
-       [mobile-camera/*camera-get-photo* (fn [_]
-                                           (reset! get-photo-called? true)
-                                           (p/resolved #js {:base64String "AA=="}))
-        state/get-edit-block (constantly {:block/format :markdown})
-        notification/show! (fn [content & _]
-                             (reset! warning content))
-        editor-handler/upload-asset! (fn [& _]
-                                       (reset! upload-called? true)
-                                       (p/resolved nil))]
-        (p/let [_ (mobile-camera/embed-photo "editor-id")]
-          (is (true? @get-photo-called?))
-          (is (true? @upload-called?))
-          (is (nil? @warning)))))))
-
-(deftest-async embed-photo-warns-only-for-camera-denied
-  (testing "Shows camera warning only for camera denied errors, not photo-library denied"
-    (let [warning (atom nil)]
-      (p/with-redefs
-       [mobile-camera/*camera-get-photo* (fn [_]
-                                           (p/rejected (js/Error. "User denied access to photos")))
-        state/get-edit-block (constantly {:block/format :markdown})
-        notification/show! (fn [content & _]
-                             (reset! warning content))
-        editor-handler/upload-asset! (fn [& _] (p/resolved nil))]
-        (p/let [_ (mobile-camera/embed-photo "editor-id")]
-          (is (nil? @warning)))))))
-
-(deftest-async embed-photo-warns-when-camera-access-denied
-  (testing "Shows camera warning when take picture is denied by camera permission"
-    (let [warning (atom nil)]
-      (p/with-redefs
-       [mobile-camera/*camera-get-photo* (fn [_]
-                                           (p/rejected (js/Error. "User denied access to camera")))
-        state/get-edit-block (constantly {:block/format :markdown})
-        notification/show! (fn [content & _]
-                             (reset! warning content))
-        editor-handler/upload-asset! (fn [& _] (p/resolved nil))]
-        (p/let [_ (mobile-camera/embed-photo "editor-id")]
-          (is (string? @warning))
-          (is (re-find #"Settings" @warning)))))))

+ 95 - 0
src/test/frontend/worker/db_sync_test.cljs

@@ -10,6 +10,7 @@
             [frontend.worker.sync.crypt :as sync-crypt]
             [logseq.common.config :as common-config]
             [logseq.db :as ldb]
+            [logseq.db.frontend.validate :as db-validate]
             [logseq.db.test.helper :as db-test]
             [logseq.outliner.core :as outliner-core]
             [logseq.outliner.op :as outliner-op]
@@ -423,6 +424,100 @@
             (let [block' (d/entity @conn (:db/id block))]
               (is (= "test" (:block/title block'))))))))))
 
+(deftest ^:long rebase-does-not-leave-anonymous-created-by-entities-test
+  (testing "rebase should not leave entities with timestamps/created-by but without identity attrs"
+    (let [{:keys [conn client-ops-conn parent child1]} (setup-parent-child)
+          child-id (:db/id child1)
+          page-id (:db/id (:block/page parent))]
+      (with-redefs [db-sync/enqueue-local-tx!
+                    (let [orig db-sync/enqueue-local-tx!]
+                      (fn [repo tx-report]
+                        (when-not (:rtc-tx? (:tx-meta tx-report))
+                          (orig repo tx-report))))]
+        (with-datascript-conns conn client-ops-conn
+          (fn []
+            ;; Ensure the deleted block has the same created-by shape from production repros.
+            (d/transact! conn [[:db/add child-id :logseq.property/created-by-ref page-id]])
+            (outliner-core/delete-blocks! conn [(d/entity @conn child-id)] {})
+            (is (seq (#'db-sync/pending-txs test-repo)))
+            (#'db-sync/apply-remote-tx!
+             test-repo
+             nil
+             [[:db/add (:db/id parent) :block/title "parent remote"]])
+            (let [anonymous-ents (->> (d/datoms @conn :avet :logseq.property/created-by-ref)
+                                      (keep (fn [datom]
+                                              (let [ent (d/entity @conn (:e datom))]
+                                                (when (and (nil? (:block/uuid ent))
+                                                           (nil? (:db/ident ent))
+                                                           (some? (:block/created-at ent))
+                                                           (some? (:block/updated-at ent)))
+                                                  (select-keys ent [:db/id :block/created-at :block/updated-at :logseq.property/created-by-ref]))))))
+                  validation (db-validate/validate-local-db! @conn)]
+              (is (empty? anonymous-ents) (str anonymous-ents))
+              (is (empty? (map :entity (:errors validation)))
+                  (str (:errors validation))))))))))
+
+(deftest ^:long rebase-create-then-delete-does-not-leave-anonymous-entities-test
+  (testing "create+delete before sync should not leave anonymous entities after rebase"
+    (let [{:keys [conn client-ops-conn parent]} (setup-parent-child)
+          page-id (:db/id (:block/page parent))]
+      (with-redefs [db-sync/enqueue-local-tx!
+                    (let [orig db-sync/enqueue-local-tx!]
+                      (fn [repo tx-report]
+                        (when-not (:rtc-tx? (:tx-meta tx-report))
+                          (orig repo tx-report))))]
+        (with-datascript-conns conn client-ops-conn
+          (fn []
+            (outliner-core/insert-blocks! conn [{:block/title "temp-rebase-case"}] parent {:sibling? false})
+            (let [temp-block (db-test/find-block-by-content @conn "temp-rebase-case")
+                  temp-id (:db/id temp-block)]
+              (d/transact! conn [[:db/add temp-id :logseq.property/created-by-ref page-id]])
+              (outliner-core/delete-blocks! conn [temp-block] {})
+              (is (>= (count (#'db-sync/pending-txs test-repo)) 2))
+              (#'db-sync/apply-remote-tx!
+               test-repo
+               nil
+               [[:db/add (:db/id parent) :block/title "parent remote 2"]])
+              (let [anonymous-ents (->> (d/datoms @conn :avet :block/created-at)
+                                        (keep (fn [datom]
+                                                (let [ent (d/entity @conn (:e datom))]
+                                                  (when (and (nil? (:block/uuid ent))
+                                                             (nil? (:db/ident ent))
+                                                             (some? (:block/updated-at ent)))
+                                                    (select-keys ent [:db/id :block/created-at :block/updated-at :logseq.property/created-by-ref]))))))
+                    validation (db-validate/validate-local-db! @conn)]
+                (is (empty? anonymous-ents) (str anonymous-ents))
+                (is (empty? (map :entity (:errors validation)))
+                    (str (:errors validation)))))))))))
+
+(deftest ^:long malformed-remote-anonymous-entity-tx-is-ignored-test
+  (testing "remote tx creating anonymous entities should be ignored instead of invalidating db"
+    (let [{:keys [conn parent]} (setup-parent-child)
+          created-by-id (:db/id (:block/page parent))
+          ts 1771435997392
+          malformed-tx [[:db/add "missing-uuid-entity" :block/created-at ts]
+                        [:db/add "missing-uuid-entity" :block/updated-at ts]
+                        [:db/add "missing-uuid-entity" :logseq.property/created-by-ref created-by-id]]]
+      (with-datascript-conns conn nil
+        (fn []
+          (is (nil? (try
+                      (#'db-sync/apply-remote-tx! test-repo nil malformed-tx)
+                      nil
+                      (catch :default e
+                        e))))
+          (let [anonymous-ents (->> (d/datoms @conn :avet :logseq.property/created-by-ref)
+                                    (keep (fn [datom]
+                                            (let [ent (d/entity @conn (:e datom))]
+                                              (when (and (nil? (:block/uuid ent))
+                                                         (nil? (:db/ident ent))
+                                                         (= ts (:block/created-at ent))
+                                                         (= ts (:block/updated-at ent)))
+                                                (select-keys ent [:db/id :block/created-at :block/updated-at :logseq.property/created-by-ref]))))))
+                validation (db-validate/validate-local-db! @conn)]
+            (is (empty? anonymous-ents) (str anonymous-ents))
+            (is (empty? (map :entity (:errors validation)))
+                (str (:errors validation)))))))))
+
 (deftest ^:long offload-large-title-test
   (testing "large titles are offloaded to object storage with placeholder"
     (async done