From 8f647519b2302ce1df5750f2c79ab66290b99df0 Mon Sep 17 00:00:00 2001 From: Junyi Du Date: Sun, 16 Apr 2023 23:19:34 +0800 Subject: [PATCH] dev: diff-merge error capturing, with unit test case --- src/main/frontend/handler/common/file.cljs | 20 ++++++++------ src/test/frontend/fs/diff_merge_test.cljs | 32 ++++++++++++++++++---- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/main/frontend/handler/common/file.cljs b/src/main/frontend/handler/common/file.cljs index 9990dcc275..c374ab4ba7 100644 --- a/src/main/frontend/handler/common/file.cljs +++ b/src/main/frontend/handler/common/file.cljs @@ -11,7 +11,8 @@ [frontend.context.i18n :refer [t]] [promesa.core :as p] [clojure.string :as string] - [cljs-bean.core :as bean])) + [cljs-bean.core :as bean] + [lambdaisland.glogi :as log])) (defn- page-exists-in-another-file "Conflict of files towards same page" @@ -54,15 +55,18 @@ (validate-existing-file repo-url file-page file-path) (graph-parser/get-blocks-to-delete db file-page file-path retain-uuid-blocks)) -(defn- diff-merge-uuids +(defn- diff-merge-uuids-2ways "Infer new uuids from existing DB data and diff with the new AST Return a list of uuids for the new blocks" [format ast content {:keys [page-name] :as options}] - (let [base-diffblocks (diff-merge/db->diff-blocks page-name) - income-diffblocks (diff-merge/ast->diff-blocks ast content format options) - diff-ops (diff-merge/diff base-diffblocks income-diffblocks) - new-uuids (diff-merge/attachUUID diff-ops (map :uuid base-diffblocks))] - (bean/->clj new-uuids))) + (try + (let [base-diffblocks (diff-merge/db->diff-blocks page-name) + income-diffblocks (diff-merge/ast->diff-blocks ast content format options) + diff-ops (diff-merge/diff base-diffblocks income-diffblocks) + new-uuids (diff-merge/attachUUID diff-ops (map :uuid base-diffblocks))] + (bean/->clj new-uuids)) + (catch js/Error e + (log/error :diff-merge/diff-merge-2way-calling-failed e)))) (defn- reset-file!* "Parse file considering diff-merge with local or remote file @@ -77,7 +81,7 @@ ;; the file is already in db, so we can use the existing file's blocks ;; to do the diff-merge :fs/local-file-change - (graph-parser/parse-file db-conn file content (assoc-in options [:extract-options :resolve-uuid-fn] diff-merge-uuids)) + (graph-parser/parse-file db-conn file content (assoc-in options [:extract-options :resolve-uuid-fn] diff-merge-uuids-2ways)) ;; TODO Junyi: 3 ways to handle remote file change ;; The file is on remote, so we should have diff --git a/src/test/frontend/fs/diff_merge_test.cljs b/src/test/frontend/fs/diff_merge_test.cljs index de72269e5f..e0fb0f7bff 100644 --- a/src/test/frontend/fs/diff_merge_test.cljs +++ b/src/test/frontend/fs/diff_merge_test.cljs @@ -286,7 +286,7 @@ (graph-parser/parse-file conn "bar.md" bar-content {}) (are [ast content page-name uuids] (= (with-redefs [conn/get-db (constantly @conn)] - (#'file-common-handler/diff-merge-uuids :markdown ast content {:page-name page-name + (#'file-common-handler/diff-merge-uuids-2ways :markdown ast content {:page-name page-name :block-pattern "-"})) uuids) @@ -313,12 +313,12 @@ "\t- jkl\n") foo-new-content (str foo-content "- newline\n") new-bar-content (str "- newline\n" bar-content)] - (graph-parser/parse-file conn "foo.md" foo-content {}) - (graph-parser/parse-file conn "bar.md" bar-content {}) + (graph-parser/parse-file conn "foo-persist.md" foo-content {}) + (graph-parser/parse-file conn "bar-persist.md" bar-content {}) ;; Compare if the uuids are the same as those inside DB when the modified content (adding new line) is parsed (are [ast content page-name DB-uuids->new-uuids-fn] (= (with-redefs [conn/get-db (constantly @conn)] - (#'file-common-handler/diff-merge-uuids :markdown ast content {:page-name page-name + (#'file-common-handler/diff-merge-uuids-2ways :markdown ast content {:page-name page-name :block-pattern "-"})) ;; Get all uuids under the page (->> page-name @@ -330,11 +330,31 @@ ;; Append a new line to foo (gp-mldoc/->edn foo-new-content (gp-mldoc/default-config :markdown)) foo-new-content - "foo" + "foo-persist" (fn [db-uuids] (conj db-uuids nil)) ;; Prepend a new line to bar (gp-mldoc/->edn new-bar-content (gp-mldoc/default-config :markdown)) new-bar-content - "bar" + "bar-persist" (fn [db-uuids] (cons nil db-uuids))))) + +(deftest diff-merge-error-capture-test + ;; Any exception thrown in diff-merge-uuids-2ways should be captured and returned a nil + (let [conn (ldb/start-conn) + foo-content (str "- abc\n" + "- def\n") + foo-new-content (str foo-content "- newline\n")] + (graph-parser/parse-file conn "foo-error-cap.md" foo-content {}) + (are [ast content page-name] + (= (with-redefs [conn/get-db (constantly @conn) + ;; Hijack the function to throw an exception + fs-diff/db->diff-blocks #(throw (js/Error. "intentional exception for testing diff-merge-uuids-2ways error capture"))] + (#'file-common-handler/diff-merge-uuids-2ways :markdown ast content {:page-name page-name + :block-pattern "-"})) + nil) + + ;; Append a new line to foo + (gp-mldoc/->edn foo-new-content (gp-mldoc/default-config :markdown)) + foo-new-content + "foo-error-cap")))