test(undo): add tests and fix

src/main/frontend/db_worker.cljs

@@ -676,11 +676,14 @@
 
   (undo
    [_this repo]
-   (undo-redo/undo repo)
+   (when-let [conn (worker-state/get-datascript-conn repo)]
+     (undo-redo/undo repo conn))
    nil)
+
   (redo
    [_this repo]
-   (undo-redo/redo repo))
+   (when-let [conn (worker-state/get-datascript-conn repo)]
+     (undo-redo/redo repo conn)))
 
   (keep-alive
    [_this]

src/main/frontend/worker/undo_redo.cljs

@@ -40,6 +40,12 @@ when undo this op, this original entity-map will be transacted back into db")
 (sr/defkeyword ::update-block
   "when a block is updated, generate a ::update-block undo-op.")
 
+(sr/defkeyword ::empty-undo-stack
+  "return by undo, when no more undo ops")
+
+(sr/defkeyword ::empty-redo-stack
+  "return by redo, when no more redo ops")
+
 (def ^:private boundary [::boundary])
 
 (def ^:private undo-op-schema
@@ -151,7 +157,7 @@ when undo this op, this original entity-map will be transacted back into db")
 
               :else
               (recur (dec i) (conj r peek-op)))))]
-    [ops (subvec stack 0 i)]))
+    [ops (subvec (vec stack) 0 i)]))
 
 (defn- pop-undo-ops
   [repo]
@@ -161,6 +167,14 @@ when undo this op, this original entity-map will be transacted back into db")
     (swap! repo->undo-stack assoc repo undo-stack*)
     ops))
 
+(defn- empty-undo-stack?
+  [repo]
+  (empty? (@(:undo/repo->undo-stack @worker-state/*state) repo)))
+
+(defn- empty-redo-stack?
+  [repo]
+  (empty? (@(:undo/repo->redo-stack @worker-state/*state) repo)))
+
 (defn- push-redo-ops
   [repo ops]
   (assert (undo-ops-validator ops) ops)
@@ -174,38 +188,45 @@ when undo this op, this original entity-map will be transacted back into db")
     (swap! repo->redo-stack assoc repo redo-stack*)
     ops))
 
+(defn- normal-block?
+  [entity]
+  (and (:block/parent entity)
+       (:block/left entity)))
+
 (defmulti ^:private reverse-apply-op (fn [op _conn _repo] (first op)))
 (defmethod reverse-apply-op ::remove-block
   [op conn repo]
-  (let [[_ {:keys [block-uuid block-entity-map]}] op]
-    (when-let [left-entity (d/entity @conn [:block/uuid (:block/left block-entity-map)])]
-      (let [sibling? (not= (:block/left block-entity-map) (:block/parent block-entity-map))]
-        (outliner-tx/transact!
-         {:gen-undo-op? false
-          :outliner-op :insert-blocks
-          :transact-opts {:repo repo
-                          :conn conn}}
-         (outliner-core/insert-blocks! repo conn
-                                       [(cond-> {:block/uuid block-uuid
-                                                 :block/content (:block/content block-entity-map)
-                                                 :block/format :markdown}
-                                          (:block/created-at block-entity-map)
-                                          (assoc :block/created-at (:block/created-at block-entity-map))
-
-                                          (:block/updated-at block-entity-map)
-                                          (assoc :block/updated-at (:block/updated-at block-entity-map))
-
-                                          (seq (:block/tags block-entity-map))
-                                          (assoc :block/tags (mapv (partial vector :block/uuid)
-                                                                   (:block/tags block-entity-map))))]
-                                       left-entity {:sibling? sibling? :keep-uuid? true}))
-        :push-undo-redo))))
+  (let [[_ {:keys [block-uuid block-entity-map]}] op
+        block-entity (d/entity @conn [:block/uuid block-uuid])]
+    (when-not block-entity ;; this block shouldn't exist now
+      (when-let [left-entity (d/entity @conn [:block/uuid (:block/left block-entity-map)])]
+        (let [sibling? (not= (:block/left block-entity-map) (:block/parent block-entity-map))]
+          (outliner-tx/transact!
+           {:gen-undo-op? false
+            :outliner-op :insert-blocks
+            :transact-opts {:repo repo
+                            :conn conn}}
+           (outliner-core/insert-blocks! repo conn
+                                         [(cond-> {:block/uuid block-uuid
+                                                   :block/content (:block/content block-entity-map)
+                                                   :block/format :markdown}
+                                            (:block/created-at block-entity-map)
+                                            (assoc :block/created-at (:block/created-at block-entity-map))
+
+                                            (:block/updated-at block-entity-map)
+                                            (assoc :block/updated-at (:block/updated-at block-entity-map))
+
+                                            (seq (:block/tags block-entity-map))
+                                            (assoc :block/tags (mapv (partial vector :block/uuid)
+                                                                     (:block/tags block-entity-map))))]
+                                         left-entity {:sibling? sibling? :keep-uuid? true}))
+          :push-undo-redo)))))
 
 (defmethod reverse-apply-op ::insert-block
   [op conn repo]
   (let [[_ {:keys [block-uuid]}] op]
     (when-let [block-entity (d/entity @conn [:block/uuid block-uuid])]
-      (when (empty? (seq (:block/_parent block-entity))) ;if have children, skip
+      (when (empty? (:block/_parent block-entity)) ;if have children, skip
         (outliner-tx/transact!
          {:gen-undo-op? false
           :outliner-op :delete-blocks
@@ -235,53 +256,54 @@ when undo this op, this original entity-map will be transacted back into db")
   [op conn repo]
   (let [[_ {:keys [block-uuid block-origin-content]}] op]
     (when-let [block-entity (d/entity @conn [:block/uuid block-uuid])]
-      (let [new-block (assoc block-entity :block/content block-origin-content)]
-        (outliner-tx/transact!
-         {:gen-undo-op? false
-          :outliner-op :save-block
-          :transact-opts {:repo repo
-                          :conn conn}}
-         (outliner-core/save-block! repo conn
-                                    (common-config/get-date-formatter (worker-state/get-config repo))
-                                    new-block))
-        :push-undo-redo))))
+      (when (normal-block? block-entity)
+        (let [new-block (assoc block-entity :block/content block-origin-content)]
+          (outliner-tx/transact!
+           {:gen-undo-op? false
+            :outliner-op :save-block
+            :transact-opts {:repo repo
+                            :conn conn}}
+           (outliner-core/save-block! repo conn
+                                      (common-config/get-date-formatter (worker-state/get-config repo))
+                                      new-block))
+          :push-undo-redo)))))
 
 (defn undo
-  [repo]
+  [repo conn]
   (if-let [ops (not-empty (pop-undo-ops repo))]
-    (let [conn (worker-state/get-datascript-conn repo)
-          redo-ops-to-push (transient [])]
+    (let [redo-ops-to-push (transient [])]
       (batch-tx/with-batch-tx-mode conn
         (doseq [op ops]
           (let [rev-op (reverse-op @conn op)]
             (when (= :push-undo-redo (reverse-apply-op op conn repo))
               (conj! redo-ops-to-push rev-op)))))
       (when-let [rev-ops (not-empty (persistent! redo-ops-to-push))]
-        (push-redo-ops repo (cons boundary rev-ops))))
-    (prn "No further undo information")))
+        (push-redo-ops repo (cons boundary rev-ops)))
+      nil)
+
+    (when (empty-undo-stack? repo)
+      (prn "No further undo information")
+      ::empty-undo-stack)))
 
 (defn redo
-  [repo]
+  [repo conn]
   (if-let [ops (not-empty (pop-redo-ops repo))]
-    (let [conn (worker-state/get-datascript-conn repo)
-          undo-ops-to-push (transient [])]
+    (let [undo-ops-to-push (transient [])]
       (batch-tx/with-batch-tx-mode conn
         (doseq [op ops]
           (let [rev-op (reverse-op @conn op)]
             (when (= :push-undo-redo (reverse-apply-op op conn repo))
               (conj! undo-ops-to-push rev-op)))))
       (when-let [rev-ops (not-empty (persistent! undo-ops-to-push))]
-        (push-undo-ops repo (cons boundary rev-ops))))
-    (prn "No further redo information")))
+        (push-undo-ops repo (cons boundary rev-ops)))
+      nil)
 
+    (when (empty-redo-stack? repo)
+      (prn "No further redo information")
+      ::empty-redo-stack)))
 
 ;;; listen db changes and push undo-ops
 
-(defn- normal-block?
-  [entity]
-  (and (:block/parent entity)
-       (:block/left entity)))
-
 (defn- entity-datoms=>ops
   [db-before db-after id->attr->datom entity-datoms]
   (when-let [e (ffirst entity-datoms)]
@@ -306,14 +328,27 @@ when undo this op, this original entity-map will be transacted back into db")
 
             (and (or add3? add4?)
                  (normal-block? entity-after))
-            (cond-> [[::move-block
-                      {:block-uuid (:block/uuid entity-after)
-                       :block-origin-left (:block/uuid (:block/left entity-before))
-                       :block-origin-parent (:block/uuid (:block/parent entity-before))}]]
-              (and add2? block-content)
-              (conj [::update-block
-                     {:block-uuid (:block/uuid entity-after)
-                      :block-origin-content (:block/content entity-before)}]))
+            (let [origin-left (:block/left entity-before)
+                  origin-parent (:block/parent entity-before)
+                  origin-left-in-db-after (d/entity db-after [:block/uuid (:block/uuid origin-left)])
+                  origin-parent-in-db-after (d/entity db-after [:block/uuid (:block/uuid origin-parent)])
+                  origin-left-and-parent-available-in-db-after?
+                  (and origin-left-in-db-after origin-parent-in-db-after
+                       (if (not= (:block/uuid origin-left) (:block/uuid origin-parent))
+                         (= (:block/uuid (:block/parent origin-left))
+                            (:block/uuid (:block/parent origin-left-in-db-after)))
+                         true))]
+              (cond-> []
+                origin-left-and-parent-available-in-db-after?
+                (conj [::move-block
+                       {:block-uuid (:block/uuid entity-after)
+                        :block-origin-left (:block/uuid (:block/left entity-before))
+                        :block-origin-parent (:block/uuid (:block/parent entity-before))}])
+
+                (and add2? block-content)
+                (conj [::update-block
+                       {:block-uuid (:block/uuid entity-after)
+                        :block-origin-content (:block/content entity-before)}])))
 
             (and add2? block-content
                  (normal-block? entity-after))
@@ -336,11 +371,13 @@ when undo this op, this original entity-map will be transacted back into db")
 
 ;;; listen db changes and push undo-ops (ends)
 
+(defn clear-undo-redo-stack
+  []
+  (reset! (:undo/repo->undo-stack @worker-state/*state) {})
+  (reset! (:undo/repo->redo-stack @worker-state/*state) {}))
+
 (comment
-  (defn- clear-undo-redo-stack
-    []
-    (reset! (:undo/repo->undo-stack @worker-state/*state) {})
-    (reset! (:undo/repo->redo-stack @worker-state/*state) {}))
+
   (clear-undo-redo-stack)
   (add-watch (:undo/repo->undo-stack @worker-state/*state)
              :xxx

src/test/frontend/test/generators.cljs

@@ -0,0 +1,30 @@
+(ns frontend.test.generators
+  "Generators for block-related data"
+  (:require [clojure.test.check.generators :as gen]
+            [datascript.core :as d]))
+
+
+(defn gen-available-block-uuid
+  [db]
+  (gen/elements
+   (->> (d/q '[:find ?block-uuid
+               :where
+               [?block :block/parent]
+               [?block :block/left]
+               [?block :block/uuid ?block-uuid]]
+             db)
+        (apply concat))))
+
+
+(defn gen-available-parent-left-pair
+  "generate [<parent-uuid> <left-uuid>]"
+  [db]
+  (gen/elements
+   (d/q '[:find ?parent-uuid ?left-uuid
+          :where
+          [?b :block/uuid]
+          [?b :block/parent ?parent]
+          [?b :block/left ?left]
+          [?parent :block/uuid ?parent-uuid]
+          [?left :block/uuid ?left-uuid]]
+        db)))

src/test/frontend/worker/undo_redo_test.cljs

@@ -1,11 +1,136 @@
 (ns frontend.worker.undo-redo-test
-  (:require [frontend.worker.undo-redo :as undo-redo]
-            [clojure.test :refer [deftest]]))
+  (:require [clojure.test :as t :refer [deftest is testing use-fixtures]]
+            [clojure.test.check.generators :as gen]
+            [datascript.core :as d]
+            [frontend.db :as db]
+            [frontend.test.generators :as t.gen]
+            [frontend.test.helper :as test-helper]
+            [frontend.worker.undo-redo :as undo-redo]))
 
+(def init-data (test-helper/initial-test-page-and-blocks))
+(defn start-and-destroy-db
+  [f]
+  (test-helper/db-based-start-and-destroy-db
+   f
+   {:init-data (fn [conn] (d/transact! conn init-data))}))
 
+(use-fixtures :each start-and-destroy-db)
 
-(deftest reverse-op-test
-  ;; TODO: add tests for undo-redo
-  undo-redo/undo
-  undo-redo/redo
-  )
+(def gen-non-exist-block-uuid gen/uuid)
+
+(defn- gen-block-uuid
+  [db & {:keys [non-exist-frequency] :or {non-exist-frequency 1}}]
+  (gen/frequency [[9 (t.gen/gen-available-block-uuid db)] [non-exist-frequency gen-non-exist-block-uuid]]))
+
+(defn- gen-parent-left-pair
+  [db]
+  (gen/frequency [[9 (t.gen/gen-available-parent-left-pair db)] [1 (gen/vector gen-non-exist-block-uuid 2)]]))
+
+(defn- gen-move-block-op
+  [db]
+  (gen/let [block-uuid (gen-block-uuid db)
+            [parent left] (gen-parent-left-pair db)]
+    [:frontend.worker.undo-redo/move-block
+     {:block-uuid block-uuid
+      :block-origin-left left
+      :block-origin-parent parent}]))
+
+(defn- gen-insert-block-op
+  [db]
+  (gen/let [block-uuid (gen-block-uuid db)]
+    [:frontend.worker.undo-redo/insert-block
+     {:block-uuid block-uuid}]))
+
+(defn- gen-remove-block-op
+  [db]
+  (gen/let [block-uuid (gen-block-uuid db {:non-exist-frequency 80})
+            [parent left] (gen-parent-left-pair db)
+            content gen/string-ascii]
+    [:frontend.worker.undo-redo/remove-block
+     {:block-uuid block-uuid
+      :block-entity-map
+      {:block/uuid block-uuid
+       :block/left left
+       :block/parent parent
+       :block/content content}}]))
+
+(defn- gen-update-block-op
+  [db]
+  (gen/let [block-uuid (gen-block-uuid db)
+            content gen/string-ascii]
+    [:frontend.worker.undo-redo/update-block
+     {:block-uuid block-uuid
+      :block-origin-content content}]))
+
+(def gen-boundary (gen/return [:frontend.worker.undo-redo/boundary]))
+
+(defn- gen-op
+  [db & {:keys [insert-block-op move-block-op remove-block-op update-block-op boundary-op]
+         :or {insert-block-op 2
+              move-block-op 2
+              remove-block-op 4
+              update-block-op 2
+              boundary-op 2}}]
+  (gen/frequency [[insert-block-op (gen-insert-block-op db)]
+                  [move-block-op (gen-move-block-op db)]
+                  [remove-block-op (gen-remove-block-op db)]
+                  [update-block-op (gen-update-block-op db)]
+                  [boundary-op gen-boundary]]))
+
+(defn- get-db-block-set
+  [db]
+  (set (d/q '[:find ?uuid ?parent-uuid ?left-uuid
+              :where
+              [?b :block/uuid ?uuid]
+              [?b :block/parent ?parent]
+              [?b :block/left ?left]
+              [?parent :block/uuid ?parent-uuid]
+              [?left :block/uuid ?left-uuid]]
+            db)))
+
+(defn- undo-all-then-redo-all
+  [conn]
+  (loop [i 0]
+    (if (not= :frontend.worker.undo-redo/empty-undo-stack
+              (undo-redo/undo test-helper/test-db-name-db-version conn))
+      (recur (inc i))
+      (prn :undo-count i)))
+
+  (loop []
+    (when (not= :frontend.worker.undo-redo/empty-redo-stack
+                (undo-redo/redo test-helper/test-db-name-db-version conn))
+      (recur))))
+
+(deftest undo-test
+  (let [conn (db/get-db false)
+        all-remove-ops (gen/generate (gen/vector (gen-op @conn {:remove-block-op 1000}) 100))]
+    (#'undo-redo/push-undo-ops test-helper/test-db-name-db-version all-remove-ops)
+    (loop [i 0]
+      (if (not= :frontend.worker.undo-redo/empty-undo-stack
+                (undo-redo/undo test-helper/test-db-name-db-version conn))
+        (recur (inc i))
+        (prn :undo-count i)))
+    (undo-redo/clear-undo-redo-stack)
+    (testing "move blocks"
+      (let [origin-graph-block-set (get-db-block-set @conn)
+            ops (gen/generate (gen/vector (gen-op @conn {:move-block-op 1000 :boundary-op 500}) 1000))]
+        (prn :ops (count ops))
+        (#'undo-redo/push-undo-ops test-helper/test-db-name-db-version ops)
+
+        (undo-all-then-redo-all conn)
+        (undo-all-then-redo-all conn)
+        (undo-all-then-redo-all conn)
+
+        (is (= origin-graph-block-set (get-db-block-set @conn)))))
+
+    (testing "random ops"
+      (let [origin-graph-block-set (get-db-block-set @conn)
+            ops (gen/generate (gen/vector (gen-op @conn) 1000))]
+        (prn :ops (count ops))
+        (#'undo-redo/push-undo-ops test-helper/test-db-name-db-version ops)
+
+        (undo-all-then-redo-all conn)
+        (undo-all-then-redo-all conn)
+        (undo-all-then-redo-all conn)
+
+        (is (= origin-graph-block-set (get-db-block-set @conn)))))))