enhance: import a block with properties across graphs

Add test to confirm multiple imports are idempotent. Also fix sqlite.build not handling valid value for :db/cardinality
Bad3r · Bad3r · Jan 2, 2025 · Jan 2, 2025 · Jan 2, 2025 · Jan 2, 2025
commit 574becd8bb32d8d858b7709010076ba87cec2528
diff --git a/deps/db/src/logseq/db/sqlite/build.cljs b/deps/db/src/logseq/db/sqlite/build.cljs
@@ -197,13 +197,18 @@
       true
       (into additional-tx))))
 
-(defn- build-properties-tx [properties page-uuids all-idents]
-  (let [property-db-ids (->> (keys properties)
+(defn- build-properties-tx [properties page-uuids all-idents {:keys [build-existing-tx?]}]
+  (let [properties' (if build-existing-tx?
+                     (->> properties
+                          (remove #(:block/uuid (val %)))
+                          (into {}))
+                     properties)
+        property-db-ids (->> (keys properties')
                              (map #(vector % (new-db-id)))
                              (into {}))
         new-properties-tx (vec
-                           (mapcat (partial build-property-tx properties page-uuids all-idents property-db-ids)
-                                   properties))]
+                           (mapcat (partial build-property-tx properties' page-uuids all-idents property-db-ids)
+                                   properties'))]
     new-properties-tx))
 
 (defn- build-classes-tx [classes properties-config uuid-maps all-idents]
@@ -555,15 +560,13 @@
     {:classes classes' :properties properties'}))
 
 (defn- build-blocks-tx*
-  [{:keys [pages-and-blocks properties graph-namespace auto-create-ontology? build-existing-tx?]
+  [{:keys [pages-and-blocks properties graph-namespace auto-create-ontology?]
     :as options}]
   (let [pages-and-blocks' (pre-build-pages-and-blocks pages-and-blocks properties)
         page-uuids (create-page-uuids pages-and-blocks')
         {:keys [classes properties]} (if auto-create-ontology? (auto-create-ontology options) options)
         all-idents (create-all-idents properties classes graph-namespace)
-        properties-tx (if (and build-existing-tx? (every? qualified-keyword? (keys properties)))
-                        []
-                        (build-properties-tx properties page-uuids all-idents))
+        properties-tx (build-properties-tx properties page-uuids all-idents options)
         classes-tx (build-classes-tx classes properties page-uuids all-idents)
         class-ident->id (->> classes-tx (map (juxt :db/ident :db/id)) (into {}))
         ;; Replace idents with db-ids to avoid any upsert issues

diff --git a/deps/db/src/logseq/db/sqlite/export.cljs b/deps/db/src/logseq/db/sqlite/export.cljs
@@ -7,10 +7,11 @@
             [logseq.db.sqlite.build :as sqlite-build]))
 
 (defn build-entity-export
-  "Given entity id, build an EDN map"
+  "Given entity id, build an EDN export map"
   [db eid]
   (let [entity (d/entity db eid)
-        properties (dissoc (:block/properties entity) :block/tags)
+        properties (dissoc (db-property/properties entity) :block/tags)
+        user-defined-properties (remove db-property/logseq-property? (keys properties))
         result (cond-> (select-keys entity [:block/title])
                  (seq (:block/tags entity))
                  (assoc :build/tags
@@ -29,16 +30,37 @@
                                        :else
                                        v)]))
                              (into {}))))]
-    result))
+    (cond-> {:build/block result}
+      (seq user-defined-properties)
+      (assoc :properties
+             (->> user-defined-properties
+                  (map (fn [ident]
+                         [ident (select-keys (d/entity db ident)
+                                             (-> (disj db-property/schema-properties :logseq.property/classes)
+                                                 (conj :block/title)))]))
+                  (into {}))))))
 
 (defn build-entity-import
   "Given an entity's export map, build the import tx to create it"
-  [db block]
-  (let [opts (cond-> {:pages-and-blocks [{:page (select-keys (:block/page block) [:block/title :block/uuid])
-                                          :blocks [block]}]
+  [db {:build/keys [block] :keys [properties]}]
+  (let [opts (cond-> {:pages-and-blocks [{:page (select-keys (:block/page block) [:block/uuid])
+                                          :blocks [(dissoc block :block/page)]}]
                       :build-existing-tx? true}
-               (seq (:build/properties block))
-               (assoc :properties (into {}
-                                        (map #(vector % (select-keys (d/entity db %) [:logseq.property/type]))
-                                             (keys (:build/properties block))))))]
-    (sqlite-build/build-blocks-tx opts)))
+               (seq properties)
+               (assoc :properties
+                      (->> properties
+                           (map (fn [[k v]]
+                                  (if-let [ent (d/entity db k)]
+                                    [k (assoc v :block/uuid (:block/uuid ent))]
+                                    [k v])))
+                           (into {}))))]
+    (sqlite-build/build-blocks-tx opts)))
+
+(defn merge-export-map
+  "Merges export map with the block that will receive the import"
+  [existing-block export-map]
+  (merge-with merge
+              export-map
+              {:build/block
+               {:block/uuid (:block/uuid existing-block)
+                :block/page (select-keys (:block/page existing-block) [:block/uuid])}}))
diff --git a/deps/db/src/logseq/db/sqlite/util.cljs b/deps/db/src/logseq/db/sqlite/util.cljs
@@ -90,7 +90,7 @@
          :block/uuid (or block-uuid (common-uuid/gen-uuid :db-ident-block-uuid db-ident'))
          :block/title (name prop-name)
          :db/index true
-         :db/cardinality (if (= :many (:db/cardinality prop-schema))
+         :db/cardinality (if (#{:many :db.cardinality/many} (:db/cardinality prop-schema))
                            :db.cardinality/many
                            :db.cardinality/one)
          :block/order (db-order/gen-key)}

diff --git a/deps/db/test/logseq/db/sqlite/build_test.cljs b/deps/db/test/logseq/db/sqlite/build_test.cljs
@@ -94,7 +94,8 @@
                               :blocks [(merge {:block/title "imported block" :block/uuid (:block/uuid block2)}
                                               {:build/properties {:user.property/p1 "foo"}
                                                :build/tags [:user.class/MyClass]})]}]
-          :properties {:user.property/p1 {:logseq.property/type :default}}
+          :properties {:user.property/p1 (select-keys (d/entity @conn :user.property/p1)
+                                                      [:logseq.property/type :block/uuid])}
           :build-existing-tx? true})
         _ (d/transact! conn init-tx2)
         _ (d/transact! conn block-props-tx2)
@@ -112,10 +113,6 @@
           "Block's properties and tags are updated"))
 
     (testing "block with existing user properties and tags"
-      (is (= []
-             (filter #(or (:db/id %) (:db/ident %))
-                     (concat init-tx2 block-props-tx2)))
-          "Tx doesn't try to create new blocks or modify existing idents")
       (is (= "imported block" (:block/title updated-block2)))
       (is (= {:block/tags [:user.class/MyClass]
               :user.property/p1 "foo"}

diff --git a/deps/db/test/logseq/db/sqlite/export_test.cljs b/deps/db/test/logseq/db/sqlite/export_test.cljs
@@ -0,0 +1,87 @@
+(ns logseq.db.sqlite.export-test
+  (:require [cljs.test :refer [deftest is testing]]
+            [datascript.core :as d]
+            [logseq.db.sqlite.export :as sqlite-export]
+            [logseq.db.test.helper :as db-test]))
+
+(deftest import-in-same-graph
+  (let [conn (db-test/create-conn-with-blocks
+              {:properties {:default-many {:logseq.property/type :default :db/cardinality :many}}
+               :pages-and-blocks
+               [{:page {:block/title "page1"}
+                 :blocks [{:block/title "export"
+                           :build/properties {:default-many #{"foo" "bar" "baz"}}}
+                          {:block/title "import"}]}]})
+        export-block (db-test/find-block-by-content @conn "export")
+        import-block* (db-test/find-block-by-content @conn "import")
+        {:keys [init-tx block-props-tx]}
+        (->> (sqlite-export/build-entity-export @conn [:block/uuid (:block/uuid export-block)])
+             (sqlite-export/merge-export-map (db-test/find-block-by-content @conn "import"))
+             (sqlite-export/build-entity-import @conn))
+        _ (assert (empty? block-props-tx) "This is empty for properties that already exist and thus no transacted")
+        _ (d/transact! conn init-tx)
+        import-block (d/entity @conn (:db/id import-block*))]
+    (is (= []
+           (filter #(or (:db/id %) (:db/ident %)) init-tx))
+        "Tx doesn't try to create new blocks or modify existing idents")
+
+    (is (= "export" (:block/title import-block))
+        "imported block title equals exported one")
+    (is (= {:user.property/default-many #{"foo" "bar" "baz"}}
+           (db-test/readable-properties import-block))
+        "imported block properties equals exported one")))
+
+(deftest import-in-different-graph
+  (let [conn (db-test/create-conn-with-blocks
+              {:properties {:num-many {:logseq.property/type :number
+                                       :db/cardinality :many
+                                       :block/title "Num Many"
+                                       :logseq.property/hide? true}}
+               :pages-and-blocks
+               [{:page {:block/title "page1"}
+                 :blocks [{:block/title "export"
+                           :build/properties {:num-many #{3 6 9}}}]}]})
+        conn2 (db-test/create-conn-with-blocks
+               {:pages-and-blocks [{:page {:block/title "page2"}
+                                    :blocks [{:block/title "import"}
+                                             {:block/title "import2"}]}]})
+        export-block (db-test/find-block-by-content @conn "export")
+        import-block* (db-test/find-block-by-content @conn2 "import")
+        {:keys [init-tx block-props-tx] :as _txs}
+        (->> (sqlite-export/build-entity-export @conn [:block/uuid (:block/uuid export-block)])
+             (sqlite-export/merge-export-map import-block*)
+             (sqlite-export/build-entity-import @conn2))
+        _ (assert (nil? (d/entity @conn2 :user.property/num-many)) "Does not have imported property")
+        _ (d/transact! conn2 init-tx)
+        _ (d/transact! conn2 block-props-tx)
+        ;; _ (cljs.pprint/pprint _txs)
+        import-block (d/entity @conn2 (:db/id import-block*))]
+
+    (is (some? (d/entity @conn2 :user.property/num-many))
+        "New user property is imported")
+    (is (= {:logseq.property/hide? true
+            :block/title "Num Many"}
+           (select-keys (d/entity @conn2 :user.property/num-many)
+                             [:block/title :logseq.property/hide?]))
+        "New user property has title and schema properties copied over")
+
+    (is (= "export" (:block/title import-block))
+        "imported block title equals exported one")
+    (is (= {:user.property/num-many #{3 6 9}}
+           (db-test/readable-properties import-block))
+        "imported block properties equals exported one")
+
+    (testing "importing a 2nd time is idempotent"
+      (let [import-block2* (db-test/find-block-by-content @conn2 "import2")
+            {:keys [init-tx block-props-tx] :as _txs}
+            (->> (sqlite-export/build-entity-export @conn [:block/uuid (:block/uuid export-block)])
+                 (sqlite-export/merge-export-map import-block2*)
+                 (sqlite-export/build-entity-import @conn2))
+            _ (assert (empty? block-props-tx) "This is empty for properties that already exist and thus no transacted")
+            _ (d/transact! conn2 init-tx)
+            import-block2 (d/entity @conn2 (:db/id import-block2*))]
+        (is (= "export" (:block/title import-block2))
+            "imported block title equals exported one")
+        (is (= {:user.property/num-many #{3 6 9}}
+               (db-test/readable-properties import-block))
+            "imported block properties equals exported one")))))
diff --git a/src/main/frontend/handler/common/developer.cljs b/src/main/frontend/handler/common/developer.cljs
@@ -75,13 +75,15 @@
     (notification/show! "Copied block's data!" :success)))
 
 (defn- import-submit [block import-input _]
-  (let [new-block (edn/read-string @import-input)
-        updated-block (merge (select-keys block [:block/uuid])
-                             {:block/page (select-keys (:block/page block) [:block/title :block/uuid])}
-                             new-block)
-        {:keys [init-tx]} (sqlite-export/build-entity-import (db/get-db) updated-block)]
-    (pprint/pprint init-tx)
-    (db/transact! (state/get-current-repo) init-tx {:save-block true})
+  (let [export-map (edn/read-string @import-input)
+        export-map' (sqlite-export/merge-export-map block export-map)
+        {:keys [init-tx block-props-tx] :as tx} (sqlite-export/build-entity-import (db/get-db) export-map')]
+    (pprint/pprint tx)
+    (p/do
+      ;; FIXME: Choose better metadata so that undo works consistently
+      (db/transact! (state/get-current-repo) init-tx {:save-block true})
+      (when (seq block-props-tx)
+        (db/transact! (state/get-current-repo) block-props-tx {:save-block true})))
     ;; Also close cmd-k
     (shui/dialog-close-all!)))
 
@@ -92,7 +94,8 @@
     (shui/dialog-open!
      [:div
       [:label.flex.my-2 "Import into block with text " (pr-str (:block/title block))]
-      (shui/textarea {:placeholder "Import EDN"
+      (shui/textarea {:placeholder "Import EDN Data"
+                      :rows 10
                       :auto-focus true
                       :on-key-down (fn [e]
                                      (when (= "Enter" (util/ekey e))