Browse Source

enhance: export-edn is idempotent for 3 more keys

For keys with non-ordered collections, use sets to consistently produce
idempotent values. Users can still author with vectors for these keys.
This affects :build/tags, :build/property-classes and
:build/class-extends.  Also remove any hacks around making their values
consistent across import->export cycles
Gabriel Horner 1 day ago
parent
commit
f0f5911ca7

+ 11 - 4
deps/db/src/logseq/db/sqlite/build.cljs

@@ -411,7 +411,7 @@
                         [:block/title :string]
                         [:build/children {:optional true} [:vector [:ref ::block]]]
                         [:build/properties {:optional true} User-properties]
-                        [:build/tags {:optional true} [:vector Class]]
+                        [:build/tags {:optional true} [:or [:set Class] [:vector Class]]]
                         [:build/keep-uuid? {:optional true} :boolean]]}}
    [:page [:and
            [:map
@@ -419,7 +419,7 @@
             [:block/title {:optional true} :string]
             [:build/journal {:optional true} :int]
             [:build/properties {:optional true} User-properties]
-            [:build/tags {:optional true} [:vector Class]]
+            [:build/tags {:optional true} [:or [:set Class] [:vector Class]]]
             [:build/keep-uuid? {:optional true} :boolean]]
            [:fn {:error/message ":block/title, :block/uuid or :build/journal required"
                  :error/path [:block/title]}
@@ -434,13 +434,14 @@
     [:build/properties {:optional true} User-properties]
     [:build/properties-ref-types {:optional true}
      [:map-of :keyword :keyword]]
+    ;; TODO: Make this respect :block/order or allow :set
     [:build/closed-values
      {:optional true}
      [:vector [:map
                [:value [:or :string :double]]
                [:uuid {:optional true} :uuid]
                [:icon {:optional true} :map]]]]
-    [:build/property-classes {:optional true} [:vector Class]]
+    [:build/property-classes {:optional true} [:or [:set Class] [:vector Class]]]
     [:build/keep-uuid? {:optional true} :boolean]]])
 
 (def Classes
@@ -448,13 +449,19 @@
    Class
    [:map
     [:build/properties {:optional true} User-properties]
-    [:build/class-extends {:optional true} [:vector Class]]
+    [:build/class-extends {:optional true} [:or [:set Class] [:vector Class]]]
     [:build/class-properties {:optional true} [:vector Property]]
     [:build/keep-uuid? {:optional true} :boolean]]])
 
 (def Options
+  "Main malli schema that validates a sqlite.build EDN map. If an inner schema
+  uses :vector e.g. :blocks, it's to preserve :block/order-ing for that node's
+  attribute. If an inner schema uses :vector or :set e.g. :build/class-extends,
+  it's to indicate it is order-less and also allow users to write the more
+  familiar vector syntax"
   [:map
    {:closed true}
+   ;; TODO: Make this respect :block/order or allow :set
    [:pages-and-blocks {:optional true} [:vector Page-blocks]]
    [:properties {:optional true} Properties]
    [:classes {:optional true} Classes]

+ 5 - 14
deps/db/src/logseq/db/sqlite/export.cljs

@@ -27,7 +27,7 @@
        ;; These classes are redundant as :build/journal is enough for Journal and Page
        ;; is implied by being in :pages-and-blocks
        (remove #{:logseq.class/Page :logseq.class/Journal})
-       vec))
+       set))
 
 (defn- block-title
   "Get an entity's original title"
@@ -147,7 +147,7 @@
                          (and (not shallow-copy?) include-alias? (:block/alias property))
                          (assoc :block/alias (set (map #(vector :block/uuid (:block/uuid %)) (:block/alias property))))
                          (and (not shallow-copy?) (:logseq.property/classes property))
-                         (assoc :build/property-classes (mapv :db/ident (:logseq.property/classes property)))
+                         (assoc :build/property-classes (set (map :db/ident (:logseq.property/classes property))))
                          (seq closed-values)
                          (assoc :build/closed-values
                                 (mapv #(cond-> {:value (db-property/property-value-content %)
@@ -194,7 +194,7 @@
          (:logseq.property.class/extends class-ent)
          (not= [:logseq.class/Root] (mapv :db/ident (:logseq.property.class/extends class-ent))))
     (assoc :build/class-extends
-           (mapv :db/ident (:logseq.property.class/extends class-ent)))))
+           (set (map :db/ident (:logseq.property.class/extends class-ent))))))
 
 (defn block-property-value? [%]
   (and (map? %) (:build/property-value %)))
@@ -1133,22 +1133,13 @@
       {:error (str "The exported EDN is unexpectedly invalid: " (pr-str (ex-message e)))})))
 
 (defn- prepare-export-to-diff
-  "This prepares a graph's exported edn to be diffed with another"
+  "Prepare a graph's exported edn to be diffed with another"
   [m]
   (-> m
-      ;; TODO: Fix order of these :build/* keys
-      (update :classes update-vals (fn [m]
-                                     (cond-> m
-                                       (:build/class-extends m)
-                                       (update :build/class-extends sort))))
-      (update :properties update-vals (fn [m]
-                                        (cond-> m
-                                          (:build/property-classes m)
-                                          (update :build/property-classes sort))))
       (update ::kv-values
               (fn [kvs]
                 (->> kvs
-                     ;; Ignore extra metadata that a copied graph can add
+                     ;; This varies per copied graph so ignore it
                      (remove #(#{:logseq.kv/import-type :logseq.kv/imported-at :logseq.kv/local-graph-uuid}
                                (:db/ident %)))
                      (sort-by :db/ident)

+ 31 - 27
deps/db/test/logseq/db/sqlite/export_test.cljs

@@ -10,6 +10,7 @@
             [logseq.common.uuid :as common-uuid]
             [logseq.db :as ldb]
             [logseq.db.frontend.validate :as db-validate]
+            [logseq.db.sqlite.build :as sqlite-build]
             [logseq.db.sqlite.export :as sqlite-export]
             [logseq.db.test.helper :as db-test]
             [medley.core :as medley]))
@@ -61,7 +62,8 @@
         imported-page (export-page-and-import-to-another-graph conn conn2 page-title)
         updated-page (db-test/find-page-by-title @conn2 page-title)
         expected-page-and-blocks
-        (update-in (:pages-and-blocks original-data) [0 :blocks] transform-expected-blocks)
+        (-> (:pages-and-blocks original-data)
+            (update-in [0 :blocks] transform-expected-blocks))
         filter-imported-page (if build-journal
                                #(= build-journal (get-in % [:page :build/journal]))
                                #(= (get-in % [:page :block/title]) page-title))]
@@ -91,8 +93,7 @@
     imported-graph))
 
 (defn- expand-properties
-  "Add default values to properties of an input export map to test against a
-  db-based export map"
+  "Modify given properties so that they match properties exported from the imported graph"
   [properties]
   (->> properties
        (map (fn [[k m]]
@@ -100,20 +101,23 @@
                (cond->
                 (merge {:db/cardinality :db.cardinality/one}
                        m)
+                 (:build/property-classes m)
+                 (update :build/property-classes set)
                  (not (:block/title m))
                  (assoc :block/title (name k)))]))
        (into {})))
 
 (defn- expand-classes
-  "Add default values to classes of an input export map to test against a
-  db-based export map"
+  "Modify given classes so that they match classes exported from the imported graph"
   [classes]
   (->> classes
        (map (fn [[k m]]
               [k
                (cond-> m
                  (not (:block/title m))
-                 (assoc :block/title (name k)))]))
+                 (assoc :block/title (name k))
+                 (:build/class-extends m)
+                 (update :build/class-extends set))]))
        (into {})))
 
 (def sort-pages-and-blocks sqlite-export/sort-pages-and-blocks)
@@ -159,7 +163,7 @@
          [{:page {:block/title "page1"}
            :blocks [{:block/title "export"
                      :build/properties {:user.property/default-many #{"foo" "bar" "baz"}}
-                     :build/tags [:user.class/MyClass]}
+                     :build/tags #{:user.class/MyClass}}
                     {:block/title "import"}]}]}
         conn (db-test/create-conn-with-blocks original-data)
         imported-block (export-block-and-import-to-another-block conn conn "export" "import")]
@@ -184,7 +188,7 @@
          [{:page {:block/title "page1"}
            :blocks [{:block/title "export"
                      :build/properties {:user.property/num-many #{3 6 9}}
-                     :build/tags [:user.class/MyClass]}]}]}
+                     :build/tags #{:user.class/MyClass}}]}]}
         conn (db-test/create-conn-with-blocks original-data)
         conn2 (db-test/create-conn-with-blocks
                {:pages-and-blocks [{:page {:block/title "page2"}
@@ -249,10 +253,10 @@
                                         {:block/title "b1ab"}]}
                                       {:block/title "b1b"}]}
                     {:block/title "b2"
-                     :build/tags [:user.class/MyClass]}
+                     :build/tags #{:user.class/MyClass}}
                     {:block/title "some task"
                      :build/properties {:logseq.property/status :logseq.property/status.doing}
-                     :build/tags [:logseq.class/Task]}]}]}
+                     :build/tags #{:logseq.class/Task}}]}]}
         conn (db-test/create-conn-with-blocks original-data)
         conn2 (db-test/create-conn)
         imported-page (export-page-and-import-to-another-graph conn conn2 "page1")]
@@ -388,9 +392,9 @@
          :pages-and-blocks
          [{:page {:block/title "page1"
                   :build/properties {:user.property/p1 "woot"}
-                  :build/tags [:user.class/ChildClass]}
+                  :build/tags #{:user.class/ChildClass}}
            :blocks [{:block/title "child object"
-                     :build/tags [:user.class/ChildClass2]}]}]}
+                     :build/tags #{:user.class/ChildClass2}}]}]}
         conn (db-test/create-conn-with-blocks original-data)
         conn2 (db-test/create-conn)
         imported-page (export-page-and-import-to-another-graph conn conn2 "page1")]
@@ -471,14 +475,14 @@
                      :build/properties {:user.property/date [:build/page {:build/journal 20250203}]}}
                     {:block/title "node block"
                      :build/properties {:user.property/node #{[:build/page {:block/title "page object"
-                                                                            :build/tags [:user.class/MyClass]}]
+                                                                            :build/tags #{:user.class/MyClass}}]
                                                               [:block/uuid block-object-uuid]
                                                               :logseq.class/Task}}}
                     {:block/title "map block"
                      :build/properties {:user.property/map {:foo :bar :num 2}}}]}
           {:page {:block/title "Blocks"}
            :blocks [{:block/title "myclass object"
-                     :build/tags [:user.class/MyClass]
+                     :build/tags #{:user.class/MyClass}
                      :block/uuid block-object-uuid
                      :build/keep-uuid? true}]}]}
         conn (db-test/create-conn-with-blocks original-data)
@@ -570,7 +574,7 @@
                                        :build/properties {:user.property/p1 "ok"}
                                        :build/children [{:block/title "b2"}]}
                                       {:block/title "b3"
-                                       :build/tags [:user.class/class1]
+                                       :build/tags #{:user.class/class1}
                                        :build/children [{:block/title "b4"}]}]}
                             {:page {:block/title "page2"}
                              :blocks [{:block/title "dont export"}]}]}
@@ -655,7 +659,7 @@
                     {: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]}}}
                     {:block/title "Example advanced query",
-                     :build/tags [:logseq.class/Query],
+                     :build/tags #{:logseq.class/Query},
                      :build/properties
                      {:logseq.property/query
                       {:build/property-value :block
@@ -668,24 +672,24 @@
                      {:user.property/url
                       {:build/property-value :block
                        :block/title "https://example.com"
-                       :build/tags [:user.class/MyClass]}}}]}
+                       :build/tags #{:user.class/MyClass}}}}]}
           {:page {:block/title "page object"
                   :block/uuid page-object-uuid
                   :build/keep-uuid? true}
            :blocks []}
-          {:page {:block/title "page2" :build/tags [:user.class/MyClass2]}
+          {: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 1"
-                     :build/tags [:user.class/MyClass]
+                     :build/tags #{:user.class/MyClass}
                      :block/uuid block-pvalue-uuid
                      :build/keep-uuid? true}
                     (cond-> {:block/title "myclass object 2"
-                             :build/tags [:user.class/MyClass]}
+                             :build/tags #{:user.class/MyClass}}
                       (not exclude-namespaces?)
                       (merge {:block/uuid property-pvalue-uuid
                               :build/keep-uuid? true}))
                     {:block/title "myclass object 3"
-                     :build/tags [:user.class/MyClass]
+                     :build/tags #{:user.class/MyClass}
                      :block/uuid page-pvalue-uuid
                      :build/keep-uuid? true}
                     {:block/title "ref blocks"
@@ -849,7 +853,7 @@
            :blocks [{:block/title "block with pvalue that has :build/tags"
                      :build/properties {:user.property/default {:build/property-value :block
                                                                 :block/title "tags pvalue"
-                                                                :build/tags [:user.class/C1]}}}
+                                                                :build/tags #{:user.class/C1}}}}
                     {:block/title "block with pvalue that has a view"
                      :build/properties {:user.property/default {:build/property-value :block
                                                                 :block/title "view pvalue"
@@ -861,7 +865,7 @@
                       #{"yep"
                         {:build/property-value :block
                          :block/title ":many pvalue"
-                         :build/tags [:user.class/C1]}}}}]}
+                         :build/tags #{:user.class/C1}}}}}]}
           {:page {:block/title "$$$views2"}
            :blocks [{:block/title "Unlinked references",
                      :build/properties
@@ -967,25 +971,25 @@
            :blocks [{:block/title "asset block"
                      :block/uuid asset-uuid
                      :build/keep-uuid? true
-                     :build/tags [:logseq.class/Asset]
+                     :build/tags #{:logseq.class/Asset}
                      :build/properties {:logseq.property.asset/type "pdf"
                                         :logseq.property.asset/checksum "abc"
                                         :logseq.property.asset/size 42}}
                     {:block/title "annotation block"
-                     :build/tags [:logseq.class/Pdf-annotation]
+                     :build/tags #{:logseq.class/Pdf-annotation}
                      :build/properties {:logseq.property/asset [:block/uuid asset-uuid]}}]}
           {:page {:block/title "page2"}
            :blocks [{:block/title "asset image block"
                      :block/uuid asset2-uuid
                      :build/keep-uuid? true
-                     :build/tags [:logseq.class/Asset]
+                     :build/tags #{:logseq.class/Asset}
                      :build/properties {:logseq.property.asset/type "png"
                                         :logseq.property.asset/checksum "img-checksum"
                                         :logseq.property.asset/width 100
                                         :logseq.property.asset/height 200
                                         :logseq.property.asset/size 300}}
                     {:block/title "annotation with image"
-                     :build/tags [:logseq.class/Pdf-annotation]
+                     :build/tags #{:logseq.class/Pdf-annotation}
                      :build/properties {:logseq.property.pdf/hl-image [:block/uuid asset2-uuid]}}]}]}
         conn (db-test/create-conn-with-blocks original-data)
         conn2 (db-test/create-conn)

+ 1 - 17
scripts/src/logseq/tasks/db_graph/create_graph_with_schema_org.cljs

@@ -334,23 +334,7 @@
              :desc "Verbose mode"}})
 
 (defn- write-export-file [db]
-  (let [export-map* (sqlite-export/build-export db {:export-type :graph-ontology})
-        ;; Modify export to provide stable diff like prepare-export-to-diff
-        ;; TODO: Remove this when prepare-export-to-diff TODO is done i.e.
-        ;; when export has stable sort order for these keys
-        export-map (-> export-map*
-                       (update :classes update-vals
-                               (fn [m]
-                                 (cond-> m
-                                   (:build/class-extends m)
-                                   (update :build/class-extends (comp vec sort))
-                                   (:build/class-properties m)
-                                   (update :build/class-properties (comp vec sort)))))
-                       (update :properties update-vals
-                               (fn [m]
-                                 (cond-> m
-                                   (:build/property-classes m)
-                                   (update :build/property-classes (comp vec sort))))))]
+  (let [export-map (sqlite-export/build-export db {:export-type :graph-ontology})]
     (fs/writeFileSync "schema.edn"
                       (with-out-str (pprint/pprint export-map)))))