enhance: ensure export EDN is idempotent across import and export

An EDN export should consistently produce the same EDN when imported
into a new graph and re-exported. This adds initial fn, cli option and
test to ensure this. There are some known TODOs. diff-graphs script was
removed as it was being used as a subset of the workflow provided by the
--roundtrip option

.github/workflows/build.yml

@@ -170,8 +170,5 @@ jobs:
       - name: Validate created DB graphs
         run: cd deps/db && yarn nbb-logseq -cp src:../cli/src -m logseq.cli validate -g ../../scripts/properties-graph ../../scripts/schema-graph
 
-      - name: Export a created DB graph
-        run: cd deps/db && yarn nbb-logseq -cp src:../cli/src -m logseq.cli export-edn -g ../../scripts/properties-graph -f properties.edn -T
-
-      - name: Create graph from the export and diff the two graphs
-        run: cd deps/db && yarn nbb-logseq -cp src:../outliner/src:script script/create_graph.cljs ./properties-graph2 properties.edn -iv && yarn nbb-logseq script/diff_graphs.cljs ../../scripts/properties-graph ./properties-graph2 -T
+      - name: Export a created DB graph and confirm the export is idempotent
+        run: cd deps/db && yarn nbb-logseq -cp src:../cli/src -m logseq.cli export-edn -g ../../scripts/properties-graph -f properties.edn --roundtrip

bb.edn

@@ -89,12 +89,6 @@
    :task (apply shell {:dir "deps/db" :extra-env {"ORIGINAL_PWD" (fs/cwd)}}
                 "yarn -s nbb-logseq -cp src:../outliner/src:script script/create_graph.cljs" *command-line-args*)}
 
-  dev:diff-graphs
-  {:doc "Diffs two DB graphs"
-   :requires ([babashka.fs :as fs])
-   :task (apply shell {:dir "deps/db" :extra-env {"ORIGINAL_PWD" (fs/cwd)}}
-                "yarn -s nbb-logseq script/diff_graphs.cljs" *command-line-args*)}
-
   dev:import
   {:doc "Import a file graph to db graph"
    :requires ([babashka.fs :as fs])

deps/cli/src/logseq/cli/commands/export_edn.cljs

@@ -17,24 +17,34 @@
 (defn- build-export-options [options]
   (cond-> {:export-type (:export-type options)}
     (= :graph (:export-type options))
-    (assoc :graph-options (dissoc options :file :export-type :graph))))
+    (assoc :graph-options
+           (select-keys options [:include-timestamps? :exclude-namespaces :exclude-files? :exclude-built-in-pages?]))))
 
 (defn- validate-export
-  [export-map {:keys [catch-validation-errors?]}]
+  [export-map {:keys [catch-validation-errors? roundtrip] :as options}]
   (println "Validating export which can take awhile on large graphs ...")
-  (if-let [error (:error (sqlite-export/validate-export export-map))]
-    (if catch-validation-errors?
-      (js/console.error error)
-      (cli-util/error error))
-    (println "Valid export!")))
+  (let [{:keys [error db]} (sqlite-export/validate-export export-map)]
+    (if error
+      (if catch-validation-errors?
+        (js/console.error error)
+        (cli-util/error error))
+      (println "Valid export!"))
+    (when roundtrip
+      (let [export-map2 (sqlite-export/build-export db (build-export-options options))
+            diff (sqlite-export/diff-exports export-map export-map2)]
+        (if diff
+          (do (println "The exported EDN's have the following diff:")
+              (pprint/pprint diff)
+              (js/process.exit 1))
+          (println "The exported EDN roundtrips successfully!"))))))
 
-(defn- local-export [{{:keys [graph validate] :as options} :opts}]
+(defn- local-export [{{:keys [graph validate roundtrip] :as options} :opts}]
   (when-not graph
     (cli-util/error "Command missing required option 'graph'"))
   (if (fs/existsSync (cli-util/get-graph-path graph))
     (let [conn (apply sqlite-cli/open-db! (cli-util/->open-db-args graph))
           export-map (sqlite-export/build-export @conn (build-export-options options))]
-      (when validate
+      (when (or validate roundtrip)
         (validate-export export-map options))
       (write-export-edn-map export-map options))
     (cli-util/error "Graph" (pr-str graph) "does not exist")))

deps/cli/src/logseq/cli/spec.cljs

@@ -20,6 +20,8 @@
    :exclude-namespaces {:alias :e
                         :coerce #{}
                         :desc "Namespaces to exclude from properties and classes"}
+   :roundtrip {:alias :r
+               :desc "(Dev) Validate export is idempotent by importing into a temp graph and diffing its export"}
    :exclude-built-in-pages? {:alias :b
                              :desc "Exclude built-in pages"}
    :exclude-files? {:alias :F

deps/db/.carve/ignore

@@ -27,6 +27,8 @@ logseq.db.sqlite.export/validate-export
 ;; API
 logseq.db.sqlite.export/build-import
 ;; API
+logseq.db.sqlite.export/diff-exports
+;; API
 logseq.db.common.view/get-property-values
 ;; API
 logseq.db.common.view/get-view-data

deps/db/script/diff_graphs.cljs

@@ -1,67 +0,0 @@
-(ns diff-graphs
-  "A script that diffs two DB graphs through their sqlite.build EDN"
-  (:require [babashka.cli :as cli]
-            [clojure.data :as data]
-            [clojure.pprint :as pprint]
-            [logseq.common.config :as common-config]
-            [logseq.db.common.sqlite-cli :as sqlite-cli]
-            [logseq.db.sqlite.export :as sqlite-export]
-            [nbb.core :as nbb]))
-
-(def spec
-  "Options spec"
-  {:help {:alias :h
-          :desc "Print help"}
-   :exclude-namespaces {:alias :e
-                        :coerce #{}
-                        :desc "Namespaces to exclude from properties and classes"}
-   :exclude-built-in-pages? {:alias :b
-                             :desc "Exclude built-in pages"}
-   :set-diff {:alias :s
-              :desc "Use set to reduce noisy diff caused by ordering"}
-   :include-timestamps? {:alias :T
-                         :desc "Include timestamps in export"}
-   :export-type {:alias :t
-                 :coerce :keyword
-                 :desc "Export type"
-                 :default :graph}})
-
-(defn -main [args]
-  (let [{options :opts args' :args} (cli/parse-args args {:spec spec})
-        [graph-dir graph-dir2] args'
-        _ (when (or (nil? graph-dir) (nil? graph-dir2) (:help options))
-            (println (str "Usage: $0 GRAPH-NAME GRAPH-NAME2 [& ARGS] [OPTIONS]\nOptions:\n"
-                          (cli/format-opts {:spec spec})))
-            (js/process.exit 1))
-        conn (apply sqlite-cli/open-db! (sqlite-cli/->open-db-args graph-dir))
-        conn2 (apply sqlite-cli/open-db! (sqlite-cli/->open-db-args graph-dir2))
-        export-args (cond-> {:export-type (:export-type options)}
-                      (= :graph (:export-type options))
-                      (assoc :graph-options (select-keys options [:include-timestamps? :exclude-namespaces :exclude-built-in-pages?])))
-        export-map (sqlite-export/build-export @conn export-args)
-        export-map2 (sqlite-export/build-export @conn2 export-args)
-        prepare-export-to-diff
-        (fn [m]
-          (cond->
-           (-> m
-               ;; TODO: Fix order of these build keys
-               (update :classes update-vals (fn [m] (update m :build/class-properties sort)))
-               (update :properties update-vals (fn [m] (update m :build/property-classes sort)))
-               (update ::sqlite-export/kv-values
-                       (fn [kvs]
-                         ;; Ignore extra metadata that a copied graph can add
-                         (vec (remove #(#{:logseq.kv/import-type :logseq.kv/imported-at} (:db/ident %)) kvs))))
-              ;; TODO: fix built-in views for schema export
-               (update :pages-and-blocks (fn [pbs]
-                                           (vec (remove #(= (:block/title (:page %)) common-config/views-page-name) pbs)))))
-            (:set-diff options)
-            (update-vals set)))
-        diff (->> (data/diff (prepare-export-to-diff export-map) (prepare-export-to-diff export-map2))
-                  butlast)]
-    (if (= diff [nil nil])
-      (println "The two graphs are equal!")
-      (do (pprint/pprint diff)
-          (js/process.exit 1)))))
-
-(when (= nbb/*file* (nbb/invoked-file))
-  (-main *command-line-args*))

deps/db/src/logseq/db/sqlite/export.cljs

@@ -2,6 +2,7 @@
   "Builds sqlite.build EDN to represent nodes in a graph-agnostic way.
    Useful for exporting and importing across DB graphs"
   (:require [cljs.pprint :as pprint]
+            [clojure.data :as data]
             [clojure.set :as set]
             [clojure.string :as string]
             [clojure.walk :as walk]
@@ -903,7 +904,7 @@
                                                   (::kv-values m))))]
     ;; Only ignore patch if initial version is > 64.8 since this fix started with 64.9
     (if (some-> initial-version (db-schema/compare-schema-version {:major 64 :minor 8}) pos?)
-       m
+      m
       (walk/postwalk
        (fn [e]
          (if (and (keyword? e) (some-> (namespace e) (string/starts-with? "user.")))
@@ -1106,10 +1107,45 @@
           {:keys [init-tx block-props-tx misc-tx] :as _txs} (build-import export-edn @import-conn {})
           _ (d/transact! import-conn (concat init-tx block-props-tx misc-tx))
           validation (db-validate/validate-local-db! @import-conn)]
-      (when-let [errors (seq (:errors validation))]
-        (js/console.error "Exported EDN has the following invalid errors when imported into a new graph:")
-        (pprint/pprint errors)
-        {:error (str "The exported EDN has " (count errors) " validation error(s)")}))
+      (if-let [errors (seq (:errors validation))]
+        (do
+          (js/console.error "Exported EDN has the following invalid errors when imported into a new graph:")
+          (pprint/pprint errors)
+          {:error (str "The exported EDN has " (count errors) " validation error(s)")})
+        {:db @import-conn}))
     (catch :default e
       (js/console.error "Unexpected export-edn validation error:" e)
-      {:error (str "The exported EDN is unexpectedly invalid: " (pr-str (ex-message e)))})))
+      {: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"
+  [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)
+                                       (:build/class-properties m)
+                                       (update :build/class-properties 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
+                     (remove #(#{:logseq.kv/import-type :logseq.kv/imported-at :logseq.kv/local-graph-uuid}
+                               (:db/ident %)))
+                     (sort-by :db/ident)
+                     vec)))))
+
+(defn diff-exports
+  "Given two graph export edns, return a vector of diffs when there is a diff and nil when there is
+   no diff between the two"
+  [export-map export-map2]
+  (let [diff (->> (data/diff (prepare-export-to-diff export-map) (prepare-export-to-diff export-map2))
+                  butlast)]
+    (when-not (= [nil nil] diff)
+      diff)))

deps/db/test/logseq/db/sqlite/export_test.cljs

@@ -601,7 +601,8 @@
     (is (= (expand-classes (:classes original-data)) (:classes imported-nodes)))))
 
 (defn- build-original-graph-data
-  [& {:keys [exclude-namespaces?]}]
+  [& {:keys [exclude-namespaces? add-built-in-pages?]
+      :or {add-built-in-pages? true}}]
   (let [internal-block-uuid (random-uuid)
         favorited-uuid (random-uuid)
         block-pvalue-uuid (random-uuid)
@@ -719,30 +720,7 @@
           {:page {:block/uuid class2-uuid}
            :blocks [{:block/title "class2 block1"}]}
           {:page {:block/uuid property-uuid}
-           :blocks [{:block/title "property block1"}]}
-          ;; built-in pages
-          {:page {:block/title "Library" :build/properties {:logseq.property/built-in? true}}
-           :blocks []}
-          {:page {:block/title "Quick add" :build/properties {:logseq.property/built-in? true
-                                                              :logseq.property/hide? true}}, :blocks []}
-          {:page {:block/title "Contents" :build/properties {:logseq.property/built-in? true}}
-           :blocks [{:block/title "right sidebar"}]}
-          {:page {:block/title common-config/favorites-page-name
-                  :build/properties {:logseq.property/built-in? true, :logseq.property/hide? true}}
-           :blocks [(ldb/build-favorite-tx favorited-uuid)]}
-          {:page {:block/title common-config/views-page-name
-                  :build/properties {:logseq.property/built-in? true, :logseq.property/hide? true}}
-           :blocks [{:block/title "All"
-                     :build/properties {:logseq.property/view-for :logseq.class/Task
-                                        :logseq.property.view/feature-type :class-objects}}
-                    {:block/title "All"
-                     :build/properties {:logseq.property/view-for :user.class/MyClass
-                                        :logseq.property.view/feature-type :class-objects}}
-                    {:block/title "Linked references",
-                     :build/properties
-                     {:logseq.property.view/type :logseq.property.view/type.list,
-                      :logseq.property.view/feature-type :linked-references,
-                      :logseq.property/view-for [:block/uuid journal-uuid]}}]}]
+           :blocks [{:block/title "property block1"}]}]
          ::sqlite-export/graph-files
          [{:file/path "logseq/config.edn"
            :file/content "{:foo :bar}"}
@@ -754,10 +732,35 @@
            :file/content ""}
           {:file/path "logseq/publish.js"
            :file/content ""}]
-         :build-existing-tx? true}]
-    original-data))
-
-(deftest import-graph
+         :build-existing-tx? true}
+        built-in-pages
+        [{:page {:block/title "Library" :build/properties {:logseq.property/built-in? true}}
+          :blocks []}
+         {:page {:block/title "Quick add" :build/properties {:logseq.property/built-in? true
+                                                             :logseq.property/hide? true}}, :blocks []}
+         {:page {:block/title "Contents" :build/properties {:logseq.property/built-in? true}}
+          :blocks [{:block/title "right sidebar"}]}
+         {:page {:block/title common-config/favorites-page-name
+                 :build/properties {:logseq.property/built-in? true, :logseq.property/hide? true}}
+          :blocks [(ldb/build-favorite-tx favorited-uuid)]}
+         {:page {:block/title common-config/views-page-name
+                 :build/properties {:logseq.property/built-in? true, :logseq.property/hide? true}}
+          :blocks [{:block/title "All"
+                    :build/properties {:logseq.property/view-for :logseq.class/Task
+                                       :logseq.property.view/feature-type :class-objects}}
+                   {:block/title "All"
+                    :build/properties {:logseq.property/view-for :user.class/MyClass
+                                       :logseq.property.view/feature-type :class-objects}}
+                   {:block/title "Linked references",
+                    :build/properties
+                    {:logseq.property.view/type :logseq.property.view/type.list,
+                     :logseq.property.view/feature-type :linked-references,
+                     :logseq.property/view-for [:block/uuid journal-uuid]}}]}]]
+    (cond-> original-data
+      add-built-in-pages?
+      (update :pages-and-blocks into built-in-pages))))
+
+(deftest ^:long import-graph
   (let [original-data (build-original-graph-data)
         conn (db-test/create-conn-with-blocks (dissoc original-data ::sqlite-export/graph-files))
         ;; set to an unobtainable version to test this ident
@@ -785,7 +788,7 @@
               (:kv/value (d/entity @conn2 :logseq.kv/schema-version)))
         "Ignored :kv/value is not updated")))
 
-(deftest import-graph-with-timestamps
+(deftest ^:long import-graph-with-timestamps
   (let [original-data* (build-original-graph-data)
         original-data (-> original-data*
                           (update :pages-and-blocks
@@ -815,7 +818,7 @@
     (is (= (::sqlite-export/graph-files original-data) (::sqlite-export/graph-files imported-graph))
         "All :file/path entities are imported")))
 
-(deftest import-graph-with-exclude-namespaces
+(deftest ^:long import-graph-with-exclude-namespaces
   (let [original-data (build-original-graph-data {:exclude-namespaces? true})
         conn (db-test/create-conn-with-blocks (dissoc original-data ::sqlite-export/graph-files))
         _ (d/transact! conn (::sqlite-export/graph-files original-data))
@@ -830,6 +833,21 @@
     (is (= (::sqlite-export/graph-files original-data) (::sqlite-export/graph-files imported-graph))
         "All :file/path entities are imported")))
 
+(deftest ^:long graph-is-idempotent-across-import-and-export
+  ;; FIXME: built-in pages should be idempotent across export and import
+  (let [original-data (build-original-graph-data {:add-built-in-pages? false})
+        conn (db-test/create-conn-with-blocks (dissoc original-data ::sqlite-export/graph-files))
+        _ (d/transact! conn (::sqlite-export/graph-files original-data))
+        export-map (sqlite-export/build-export @conn {:export-type :graph})
+        valid-result (sqlite-export/validate-export export-map)
+        _ (assert (not (:error valid-result)) "No error when importing export-map into new graph")
+        _ (validate-db (:db valid-result))
+        export-map2 (sqlite-export/build-export (:db valid-result) {:export-type :graph})]
+    ;; (cljs.pprint/pprint (sqlite-export/diff-exports export-map export-map2))
+    (is (= nil
+           (sqlite-export/diff-exports export-map export-map2))
+        "No diff between original export and export after importing into a new graph")))
+
 (deftest import-graph-with-different-property-value-cases
   (let [pvalue-uuid1 (random-uuid)
         original-data

scripts/src/logseq/tasks/db_graph/create_graph_with_schema_org.cljs

@@ -335,8 +335,9 @@
 
 (defn- write-export-file [db]
   (let [export-map* (sqlite-export/build-export db {:export-type :graph-ontology})
-        ;; Modify export to provide stable diff like diff-graphs.
-        ;; Remove this when export has stable sort order for these keys
+        ;; 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]