Browse Source

fix duplicate pages in imported graph for page property values

Don't use :build/page for graph exports as it leads to needless
complexity when reconciling referenced nodes
Gabriel Horner 7 months ago
parent
commit
3dd196d8f8

+ 19 - 18
deps/db/src/logseq/db/sqlite/export.cljs

@@ -38,16 +38,16 @@
 
 (defn- buildable-property-value-entity
   "Converts property value to a buildable version"
-  [property-ent pvalue]
-  (cond (ldb/internal-page? pvalue)
+  [property-ent pvalue {:keys [property-value-uuids?]}]
+  (cond (and (not property-value-uuids?) (ldb/internal-page? pvalue))
         ;; Should page properties be pulled here?
         [:build/page (cond-> (shallow-copy-page pvalue)
                        (seq (:block/tags pvalue))
                        (assoc :build/tags (->build-tags (:block/tags pvalue))))]
-        (entity-util/journal? pvalue)
+        (and (not property-value-uuids?) (entity-util/journal? pvalue))
         [:build/page {:build/journal (:block/journal-day pvalue)}]
         :else
-        (if (= :node (:logseq.property/type property-ent))
+        (if (contains? #{:node :date} (:logseq.property/type property-ent))
           ;; Idents take precedence over uuid because they are keep data graph-agnostic
           (if (:db/ident pvalue)
             (:db/ident pvalue)
@@ -61,7 +61,7 @@
 (defn- buildable-properties
   "Originally copied from db-test/readable-properties. Modified so that property values are
    valid sqlite.build EDN"
-  [db ent-properties properties-config]
+  [db ent-properties properties-config options]
   (->> ent-properties
        (map (fn [[k v]]
               [k
@@ -73,17 +73,17 @@
                    (throw (ex-info (str "No closed value found for content: " (pr-str (db-property/property-value-content v))) {:properties properties-config})))
                  (cond
                    (de/entity? v)
-                   (buildable-property-value-entity (d/entity db k) v)
+                   (buildable-property-value-entity (d/entity db k) v options)
                    (and (set? v) (every? de/entity? v))
                    (let [property-ent (d/entity db k)]
-                     (set (map (partial buildable-property-value-entity property-ent) v)))
+                     (set (map #(buildable-property-value-entity property-ent % options) v)))
                    :else
                    v))]))
        (into {})))
 
 (defn- build-export-properties
   "The caller of this fn is responsible for building :build/:property-classes unless shallow-copy?"
-  [db user-property-idents {:keys [include-properties? include-timestamps? include-uuid? shallow-copy?]}]
+  [db user-property-idents {:keys [include-properties? include-timestamps? include-uuid? shallow-copy?] :as options}]
   (let [properties-config-by-ent
         (->> user-property-idents
              (map (fn [ident]
@@ -118,7 +118,7 @@
                     [(:db/ident ent)
                      (cond-> build-property
                        (seq ent-properties)
-                       (assoc :build/properties (buildable-properties db ent-properties properties-config)))])))
+                       (assoc :build/properties (buildable-properties db ent-properties properties-config options)))])))
            (into {}))
       properties-config)))
 
@@ -191,7 +191,7 @@
   (let [ent-properties (dissoc (db-property/properties entity) :block/tags)
         build-tags (when (seq (:block/tags entity)) (->build-tags (:block/tags entity)))
         new-properties (when-not shallow-copy?
-                         (build-node-properties db entity ent-properties (select-keys options [:properties :include-timestamps?])))
+                         (build-node-properties db entity ent-properties (dissoc options :shallow-copy? :include-uuid-fn)))
         build-node (cond-> {:block/title (block-title entity)}
                      (:block/link entity)
                      (assoc :block/link [:block/uuid (:block/uuid (:block/link entity))])
@@ -203,7 +203,7 @@
                      (assoc :build/tags build-tags)
                      (and (not shallow-copy?) (seq ent-properties))
                      (assoc :build/properties
-                            (buildable-properties db ent-properties (merge properties new-properties))))
+                            (buildable-properties db ent-properties (merge properties new-properties) options)))
         new-classes (when-not shallow-copy? (build-node-classes db build-node (:block/tags entity) new-properties))]
     (cond-> {:node build-node}
       (seq new-classes)
@@ -372,8 +372,8 @@
     (merge {::block (:node node-export)}
            block-export)))
 
-(defn- build-page-blocks-export [db page-entity {:keys [properties classes blocks] :as opts}]
-  (let [page-ent-export (build-node-export db page-entity (select-keys opts [:properties :include-uuid-fn :include-timestamps?]))
+(defn- build-page-blocks-export [db page-entity {:keys [properties classes blocks] :as options}]
+  (let [page-ent-export (build-node-export db page-entity (dissoc options :classes :blocks))
         page (merge (dissoc (:node page-ent-export) :block/title)
                     (shallow-copy-page page-entity))
         page-blocks-export {:pages-and-blocks [{:page page :blocks blocks}]
@@ -472,7 +472,7 @@
                       (vector (:db/ident ent)
                               (cond-> (build-export-class ent options)
                                 (seq ent-properties)
-                                (assoc :build/properties (buildable-properties db ent-properties properties)))))))
+                                (assoc :build/properties (buildable-properties db ent-properties properties options)))))))
              (into {}))]
     (cond-> {}
       (seq properties)
@@ -529,16 +529,17 @@
 (defn- build-graph-export
   "Exports whole graph. Has the following options:
    * :include-timestamps? - When set timestamps are included on all blocks"
-  [db options]
-  (let [content-ref-uuids (get-graph-content-ref-uuids db)
-        ontology-options (merge (select-keys options [:include-timestamps?]) {:include-uuid? true})
+  [db options*]
+  (let [options (merge options* {:property-value-uuids? true})
+        content-ref-uuids (get-graph-content-ref-uuids db)
+        ontology-options (merge options {:include-uuid? true})
         ontology-export (build-graph-ontology-export db ontology-options)
         ontology-pvalue-uuids (set (concat (mapcat get-pvalue-uuids (vals (:properties ontology-export)))
                                            (mapcat get-pvalue-uuids (vals (:classes ontology-export)))))
         pages-export (build-graph-pages-export db options)
+        graph-export (merge-export-maps ontology-export pages-export)
         all-ref-uuids (set/union content-ref-uuids ontology-pvalue-uuids (:pvalue-uuids pages-export))
         files (build-graph-files db options)
-        graph-export (merge-export-maps ontology-export pages-export)
         ;; Remove all non-ref uuids after all nodes are built.
         ;; Only way to ensure all pvalue uuids present across block types
         graph-export' (remove-uuids-if-not-ref graph-export all-ref-uuids)]

+ 22 - 6
deps/db/test/logseq/db/sqlite/export_test.cljs

@@ -11,7 +11,8 @@
             [logseq.db.test.helper :as db-test]
             [medley.core :as medley]
             [clojure.walk :as walk]
-            [logseq.common.util :as common-util]))
+            [logseq.common.util :as common-util]
+            [logseq.common.uuid :as common-uuid]))
 
 ;; Test helpers
 ;; ============
@@ -532,9 +533,11 @@
         favorited-uuid (random-uuid)
         block-object-uuid (random-uuid)
         block-object-uuid2 (random-uuid)
+        page-object-uuid (random-uuid)
         closed-value-uuid (random-uuid)
         property-uuid (random-uuid)
         class-uuid (random-uuid)
+        journal-uuid (common-uuid/gen-uuid :journal-page-uuid 19650201)
         original-data
         {:properties
          {:user.property/num {:logseq.property/type :number
@@ -546,6 +549,7 @@
            :build/closed-values [{:value "joy" :uuid closed-value-uuid}
                                  {:value "sad" :uuid (random-uuid)}]}
           :user.property/checkbox {:logseq.property/type :checkbox}
+          :user.property/date {:logseq.property/type :date}
           :user.property/url {:logseq.property/type :url
                               :build/properties {:logseq.property/description "desc for url"}}
           :user.property/node {:logseq.property/type :node
@@ -561,8 +565,16 @@
          [{:page {:block/title "page1"
                   :block/uuid favorited-uuid :build/keep-uuid? true
                   :build/properties {:user.property/checkbox false}}
-           :blocks [{:block/title "b1" :build/properties {:user.property/num 1
-                                                          :user.property/default-closed [:block/uuid closed-value-uuid]}}]}
+           :blocks [{:block/title "b1"
+                     :build/properties {:user.property/num 1
+                                        :user.property/default-closed [:block/uuid closed-value-uuid]
+                                        :user.property/date [:block/uuid journal-uuid]}}
+                    {:block/title "b2" :build/properties {:user.property/node #{[:block/uuid page-object-uuid]}}}
+                    {:block/title "b3" :build/properties {:user.property/node #{[:block/uuid page-object-uuid]}}}]}
+          {:page {:block/title "page object"
+                  :block/uuid page-object-uuid
+                  :build/keep-uuid? true}
+           :blocks []}
           {:page {:block/title "page2" :build/tags [:user.class/MyClass2]}
            :blocks [{:block/title "hola" :block/uuid internal-block-uuid :build/keep-uuid? true}
                     {:block/title "myclass object"
@@ -582,7 +594,10 @@
                       {:block/title (str "class ref to " (page-ref/->page-ref class-uuid))}]}]}
           {:page {:build/journal 20250228 :build/properties {:user.property/num 1}}
            :blocks [{:block/title "journal block"}]}
-          {:page {:build/journal 19650201}, :blocks []}
+          {:page {:build/journal 19650201
+                  :block/uuid journal-uuid
+                  :build/keep-uuid? true}
+           :blocks []}
           ;; built-in pages
           {:page {:block/title "Contents" :build/properties {:logseq.property/built-in? true}}
            :blocks [{:block/title "right sidebar"}]}
@@ -601,8 +616,7 @@
                      :build/properties
                      {:logseq.property.view/type :logseq.property.view/type.list,
                       :logseq.property.view/feature-type :linked-references,
-                      :logseq.property/view-for
-                      [:build/page {:build/journal 19650201}]}}]}]
+                      :logseq.property/view-for [:block/uuid journal-uuid]}}]}]
          ::sqlite-export/graph-files
          [{:file/path "logseq/config.edn"
            :file/content "{:foo :bar}"}
@@ -624,6 +638,8 @@
     ;; (cljs.pprint/pprint (butlast (clojure.data/diff (set (:pages-and-blocks original-data))
     ;;                                                 (set (:pages-and-blocks imported-graph)))))
     (is (= (set (:pages-and-blocks original-data)) (set (:pages-and-blocks imported-graph))))
+    (is (= 1 (count (d/datoms @conn2 :avet :block/title "page object")))
+        "No duplicate pages for pvalue uuids used more than once")
     (is (= (expand-properties (:properties original-data)) (:properties imported-graph)))
     (is (= (expand-classes (:classes original-data)) (:classes imported-graph)))
     (is (= (::sqlite-export/graph-files original-data) (::sqlite-export/graph-files imported-graph))