fix: ensure using :rename-db-idents to rename properties in migrate

src/main/frontend/worker/db/migrate.cljs

@@ -9,9 +9,7 @@
             [logseq.common.config :as common-config]
             [logseq.common.util :as common-util]
             [logseq.db :as ldb]
-            [logseq.db.common.order :as db-order]
             [logseq.db.frontend.class :as db-class]
-            [logseq.db.frontend.db-ident :as db-ident]
             [logseq.db.frontend.property :as db-property]
             [logseq.db.frontend.schema :as db-schema]
             [logseq.db.sqlite.create-graph :as sqlite-create-graph]
@@ -20,20 +18,16 @@
 ;; Frontend migrations
 ;; ===================
 
-(defn- rename-properties-aux
+(defn- rename-properties-fix
   [db props-to-rename]
-  (let [property-tx (map
+  (let [;; update property title/name
+        ;; don't update :db/ident since it's addressed by `:rename-db-idents`
+        property-tx (map
                      (fn [[old new]]
-                       (let [e-new (d/entity db new)
-                             e-old (d/entity db old)]
-                         (if e-new
-                           (when e-old
-                             [:db/retractEntity (:db/id e-old)])
-                           (merge {:db/id (:db/id (d/entity db old))
-                                   :db/ident new}
-                                  (when-let [new-title (get-in db-property/built-in-properties [new :title])]
-                                    {:block/title new-title
-                                     :block/name (common-util/page-name-sanity-lc new-title)})))))
+                       (merge {:db/id (:db/id (d/entity db old))}
+                              (when-let [new-title (get-in db-property/built-in-properties [new :title])]
+                                {:block/title new-title
+                                 :block/name (common-util/page-name-sanity-lc new-title)})))
                      props-to-rename)
         titles-tx (->> (d/datoms db :avet :block/title)
                        (keep (fn [d]
@@ -118,21 +112,20 @@
             ordered-columns-tx
             filters-tx)))
 
-(defn rename-properties
-  [props-to-rename & {:keys [replace-fn]}]
-  (fn [db]
-    (when (ldb/db-based-graph? db)
-      (let [props-tx (rename-properties-aux db props-to-rename)
-            fix-tx (mapcat (fn [[old new]]
-                             ;; can't use datoms b/c user properties aren't indexed
-                             (->> (d/q '[:find ?b ?prop-v :in $ ?prop :where [?b ?prop ?prop-v]] db old)
-                                  (mapcat (fn [[id prop-value]]
-                                            (if (fn? replace-fn)
-                                              (replace-fn id prop-value)
-                                              [[:db/retract id old]
-                                               [:db/add id new prop-value]])))))
-                           props-to-rename)]
-        (concat props-tx fix-tx)))))
+(defn- rename-properties
+  [props-to-rename {:keys [fix]}]
+  {:rename-db-idents (mapv
+                      (fn [[old-ident new-ident]]
+                        {:db-ident-or-block-uuid old-ident
+                         :new-db-ident new-ident})
+                      props-to-rename)
+   :fix (fn [db]
+          (let [common-fix (rename-properties-fix db
+                                                  {:logseq.property.asset/external-src
+                                                   :logseq.property.asset/external-url})
+                additional-fix (when (fn? fix)
+                                 (fix db))]
+            (concat common-fix additional-fix)))})
 
 (comment
   (defn- rename-classes
@@ -147,238 +140,6 @@
                           :block/name (common-util/page-name-sanity-lc new-title)})))
               classes-to-rename)))))
 
-(defn fix-rename-parent-to-extends
-  [db]
-  (let [parent-entity (d/entity db :logseq.property/parent)]
-    (when parent-entity
-      (let [old-p :logseq.property/parent
-            new-p :logseq.property.class/extends
-            f (rename-properties
-               {old-p new-p}
-               {:replace-fn (fn [id prop-value]
-                              (let [page (d/entity db id)
-                                    ;; bad impl, it's not just simple db/ident renaming
-                                    new-p' (if (ldb/class? page) new-p :block/parent)]
-                                [[:db/retract id old-p]
-                                 [:db/add id new-p' prop-value]]))})
-            rename-property-tx (f db)
-            library-page (if-let [page (ldb/get-built-in-page db common-config/library-page-name)]
-                           page
-                           (-> (sqlite-util/build-new-page common-config/library-page-name)
-                               sqlite-create-graph/mark-block-as-built-in))
-            library-id (:block/uuid library-page)
-            library-page-tx (when-not (de/entity? library-page)
-                              [library-page])
-            pages-with-parent (->> (d/datoms db :avet :logseq.property/parent)
-                                   (keep (fn [d]
-                                           (let [e (d/entity db (:e d))]
-                                             (when-not (ldb/class? e)
-                                               e)))))
-            parents (->> pages-with-parent
-                         (map :logseq.property/parent)
-                         (common-util/distinct-by :db/id))
-            top-parents (remove :logseq.property/parent parents)
-            top-parent-ids (set (map :db/id top-parents))
-            move-top-parents-to-library (map (fn [parent]
-                                               {:db/id (:db/id parent)
-                                                :block/parent [:block/uuid library-id]
-                                                :block/order (db-order/gen-key)}) top-parents)
-            update-children-parent-and-order (->> pages-with-parent
-                                                  (remove (fn [page] (top-parent-ids (:db/id page))))
-                                                  (map (fn [page]
-                                                         {:db/id (:db/id page)
-                                                          :block/order (db-order/gen-key)})))]
-        (concat
-         rename-property-tx
-         library-page-tx
-         move-top-parents-to-library
-         update-children-parent-and-order)))))
-
-(defn- retract-property-attributes
-  [id]
-  [[:db/retract id :block/tags :logseq.class/Property]
-   [:db/retract id :logseq.property/type]
-   [:db/retract id :db/cardinality]
-   [:db/retract id :db/valueType]
-   [:db/retract id :db/index]
-   [:db/retract id :logseq.property/classes]
-   [:db/retract id :logseq.property/hide?]
-   [:db/retract id :logseq.property/public?]
-   [:db/retract id :logseq.property/view-context]
-   [:db/retract id :logseq.property/ui-position]
-   [:db/retract id :logseq.property/default-value]
-   [:db/retract id :logseq.property/hide-empty-value]
-   [:db/retract id :logseq.property/enable-history?]])
-
-(defn separate-classes-and-properties
-  [db]
-  ;; find all properties that're classes, create new properties to separate them
-  ;; from classes.
-  (let [class-ids (d/q
-                   '[:find [?b ...]
-                     :where
-                     [?b :block/tags :logseq.class/Property]
-                     [?b :block/tags :logseq.class/Tag]]
-                   db)]
-    (mapcat
-     (fn [id]
-       (let [class (d/entity db id)
-             ident (:db/ident class)
-             new-property (sqlite-util/build-new-property
-                           (:block/title class)
-                           (select-keys class [:logseq.property/type :db/cardinality])
-                           {:title (:block/title class)
-                            :ref-type? true
-                            :properties (merge
-                                         (select-keys class [:logseq.property/hide? :logseq.property/public?
-                                                             :logseq.property/view-context :logseq.property/ui-position
-                                                             :logseq.property/default-value :logseq.property/hide-empty-value :logseq.property/enable-history?])
-                                         {:logseq.property/classes id})})
-             retract-property-attrs (retract-property-attributes id)
-             datoms (if (:db/index class)
-                      (d/datoms db :avet ident)
-                      (filter (fn [d] (= ident (:a d))) (d/datoms db :eavt)))
-             tag-properties (->> (d/datoms db :avet :logseq.property.class/properties id)
-                                 (mapcat (fn [d]
-                                           [[:db/retract (:e d) (:a d) (:v d)]
-                                            [:db/add (:e d) (:a d) [:block/uuid (:block/uuid new-property)]]])))
-             other-properties-tx (mapcat
-                                  (fn [ident]
-                                    (->> (d/datoms db :avet ident id)
-                                         (mapcat (fn [d]
-                                                   [[:db/retract (:e d) (:a d) (:v d)]
-                                                    [:db/add (:e d) (:a d) [:block/uuid (:block/uuid new-property)]]]))))
-                                  [:logseq.property.view/group-by-property :logseq.property.table/pinned-columns])]
-         (concat [new-property]
-                 tag-properties
-                 other-properties-tx
-                 retract-property-attrs
-                 (mapcat
-                  (fn [d]
-                    [[:db/retract (:e d) ident (:v d)]
-                     [:db/add (:e d) (:db/ident new-property) (:v d)]])
-                  datoms))))
-     class-ids)))
-
-(defn fix-tag-properties
-  [db]
-  ;; find all classes that're still used as properties
-  (let [class-ids (d/q
-                   '[:find [?b ...]
-                     :where
-                     [?b :block/tags :logseq.class/Tag]
-                     [?b1 :logseq.property.class/properties ?b]]
-                   db)]
-    (mapcat
-     (fn [id]
-       (let [class (d/entity db id)
-             property-id (first (ldb/page-exists? db (:block/title class) :logseq.class/Property))
-             tag-properties (when property-id
-                              (->> (d/datoms db :avet :logseq.property.class/properties id)
-                                   (mapcat (fn [d]
-                                             [[:db/retract (:e d) (:a d) (:v d)]
-                                              [:db/add (:e d) (:a d) property-id]]))))]
-         tag-properties))
-     class-ids)))
-
-(defn add-missing-db-ident-for-tags
-  [db _sqlite-db]
-  (let [class-ids (d/q
-                   '[:find [?b ...]
-                     :where
-                     [?b :block/tags :logseq.class/Tag]
-                     [(missing? $ ?b :db/ident)]]
-                   db)]
-    (mapcat
-     (fn [id]
-       [[:db/add id :logseq.property.class/extends :logseq.class/Root]
-        [:db/retract id :block/tags :logseq.class/Page]
-        [:db/retract id :block/refs :logseq.class/Page]
-        [:db/retract id :block/path-refs :logseq.class/Page]])
-     class-ids)))
-
-(defn add-missing-db-ident-for-tags2
-  [db]
-  (let [class-ids
-        (d/q
-         '[:find [?b ...]
-           :where
-           [?b :block/tags :logseq.class/Tag]
-           [(missing? $ ?b :db/ident)]]
-         db)]
-    (keep
-     (fn [id]
-       (let [ent (d/entity db id)
-             title (:block/title ent)
-             block-uuid (:block/uuid ent)]
-         (when block-uuid
-           {:db-ident-or-block-uuid block-uuid
-            :new-db-ident (db-ident/replace-db-ident-random-suffix
-                           (db-class/create-user-class-ident-from-name db title)
-                           (subs (str block-uuid) 28))})))
-     class-ids)))
-
-(defn fix-using-properties-as-tags
-  [db]
-  ;; find all properties that're tags
-  (let [property-ids (->>
-                      (d/q
-                       '[:find ?b ?i
-                         :where
-                         [?b :block/tags :logseq.class/Tag]
-                         [?b :db/ident ?i]]
-                       db)
-                      (filter (fn [[_ ident]] (= "user.property" (namespace ident))))
-                      (map first))]
-    (mapcat
-     (fn [id]
-       (into (retract-property-attributes id)
-             [[:db/retract id :logseq.property/parent]]))
-     property-ids)))
-
-(defn fix-using-properties-as-tags2
-  [db]
-  (let [property-ids
-        (->>
-         (d/q
-          '[:find ?b ?i
-            :where
-            [?b :block/tags :logseq.class/Tag]
-            [?b :db/ident ?i]]
-          db)
-         (filter (fn [[_ ident]] (= "user.property" (namespace ident))))
-         (map first))]
-    (keep
-     (fn [id]
-       (let [ent (d/entity db id)
-             title (:block/title ent)
-             block-uuid (:block/uuid ent)]
-         (when block-uuid
-           {:db-ident-or-block-uuid block-uuid
-            :new-db-ident (db-ident/replace-db-ident-random-suffix
-                           (db-class/create-user-class-ident-from-name db title)
-                           (subs (str block-uuid) 28))})))
-     property-ids)))
-
-(defn remove-block-order-for-tags
-  [db]
-  ;; find all properties that're tags
-  (let [tag-ids (d/q
-                 '[:find [?b ...]
-                   :where
-                   [?b :block/tags :logseq.class/Tag]
-                   [?b :block/order]]
-                 db)]
-    (map
-     (fn [id]
-       [:db/retract id :block/order])
-     tag-ids)))
-
-(defn- update-extends-to-cardinality-many
-  [db]
-  (let [extends (d/entity db :logseq.property.class/extends)]
-    [[:db/add (:db/id extends) :db/cardinality :db.cardinality/many]]))
-
 (defn- add-quick-add-page
   [_db]
   (let [page (-> (-> (sqlite-util/build-new-page common-config/quick-add-page-name)
@@ -414,25 +175,11 @@
                  (when (:logseq.property/ui-position e)
                    [:db/retract (:e d) :logseq.property/ui-position]))))))
 
-(defn- rename-external-src-to-external-url
-  [db]
-  (let [f (rename-properties
-           {:logseq.property.asset/external-src :logseq.property.asset/external-url}
-           {})]
-    (f db)))
-
 (def schema-version->updates
   "A vec of tuples defining datascript migrations. Each tuple consists of the
    schema version integer and a migration map. A migration map can have keys of :properties, :classes
    :rename-db-idents and :fix."
-  [["65.0" {:fix separate-classes-and-properties}]
-   ["65.1" {:fix fix-rename-parent-to-extends}]
-   ["65.2" {:fix fix-tag-properties}]
-   ["65.3" {:rename-db-idents add-missing-db-ident-for-tags2 :fix add-missing-db-ident-for-tags}]
-   ["65.4" {:rename-db-idents fix-using-properties-as-tags2 :fix fix-using-properties-as-tags}]
-   ["65.5" {:fix remove-block-order-for-tags}]
-   ["65.6" {:fix update-extends-to-cardinality-many}]
-   ["65.7" {:fix add-quick-add-page}]
+  [["65.7" {:fix add-quick-add-page}]
    ["65.8" {:fix add-missing-page-name}]
    ["65.9" {:properties [:logseq.property.embedding/hnsw-label-updated-at]}]
    ["65.10" {:properties [:block/journal-day :logseq.property.view/sort-groups-by-property :logseq.property.view/sort-groups-desc?]}]
@@ -441,7 +188,9 @@
    ["65.13" {:properties [:logseq.property.asset/width
                           :logseq.property.asset/height]}]
    ["65.14" {:properties [:logseq.property.asset/external-src]}]
-   ["65.15" {:fix rename-external-src-to-external-url}]])
+   ["65.15" (rename-properties {:logseq.property.asset/external-src
+                                :logseq.property.asset/external-url}
+                               {})]])
 
 (let [[major minor] (last (sort (map (comp (juxt :major :minor) db-schema/parse-schema-version first)
                                      schema-version->updates)))]

src/test/frontend/worker/migrate_test.cljs

@@ -1,94 +1,10 @@
 (ns frontend.worker.migrate-test
   (:require ["fs" :as fs-node]
-            [cljs.test :refer [deftest is testing]]
-            [clojure.string :as string]
+            [cljs.test :refer [deftest is]]
             [datascript.core :as d]
             [frontend.worker.db.migrate :as db-migrate]
-            [logseq.common.config :as common-config]
             [logseq.db :as ldb]))
 
-(deftest test-fix-rename-parent-to-extends
-  (testing "Rename parent to extends"
-    (let [db-transit (str (fs-node/readFileSync "src/test/migration/64.8.transit"))
-          db (ldb/read-transit-str db-transit)
-          tx-data (db-migrate/fix-rename-parent-to-extends db)]
-      (is (= (->> tx-data
-                  (map (fn [data]
-                         (cond
-                           (and (map? data) (:block/created-at data))
-                           (dissoc data :block/created-at :block/updated-at)
-                           (and (map? data) (:block/order data))
-                           (dissoc data :block/order)
-                           :else
-                           data))))
-             [{:db/id 35,
-               :db/ident :logseq.property.class/extends,
-               :block/title "Extends",
-               :block/name "extends"}
-              [:db/retract 161 :logseq.property/parent]
-              [:db/add 161 :logseq.property.class/extends 1]
-              [:db/retract 163 :logseq.property/parent]
-              [:db/add 163 :logseq.property.class/extends 162]
-              [:db/retract 139 :logseq.property/parent]
-              [:db/add 139 :logseq.property.class/extends 137]
-              [:db/retract 138 :logseq.property/parent]
-              [:db/add 138 :logseq.property.class/extends 1]
-              [:db/retract 140 :logseq.property/parent]
-              [:db/add 140 :logseq.property.class/extends 1]
-              [:db/retract 158 :logseq.property/parent]
-              [:db/add 158 :block/parent 155]
-              [:db/retract 134 :logseq.property/parent]
-              [:db/add 134 :logseq.property.class/extends 133]
-              [:db/retract 3 :logseq.property/parent]
-              [:db/add 3 :logseq.property.class/extends 1]
-              [:db/retract 142 :logseq.property/parent]
-              [:db/add 142 :logseq.property.class/extends 1]
-              [:db/retract 135 :logseq.property/parent]
-              [:db/add 135 :logseq.property.class/extends 133]
-              [:db/retract 133 :logseq.property/parent]
-              [:db/add 133 :logseq.property.class/extends 1]
-              [:db/retract 162 :logseq.property/parent]
-              [:db/add 162 :logseq.property.class/extends 161]
-              [:db/retract 144 :logseq.property/parent]
-              [:db/add 144 :logseq.property.class/extends 1]
-              [:db/retract 155 :logseq.property/parent]
-              [:db/add 155 :block/parent 154]
-              [:db/retract 165 :logseq.property/parent]
-              [:db/add 165 :logseq.property.class/extends 162]
-              [:db/retract 143 :logseq.property/parent]
-              [:db/add 143 :logseq.property.class/extends 1]
-              [:db/retract 136 :logseq.property/parent]
-              [:db/add 136 :logseq.property.class/extends 1]
-              [:db/retract 2 :logseq.property/parent]
-              [:db/add 2 :logseq.property.class/extends 1]
-              [:db/retract 4 :logseq.property/parent]
-              [:db/add 4 :logseq.property.class/extends 1]
-              [:db/retract 156 :logseq.property/parent]
-              [:db/add 156 :block/parent 155]
-              [:db/retract 141 :logseq.property/parent]
-              [:db/add 141 :logseq.property.class/extends 1]
-              [:db/retract 137 :logseq.property/parent]
-              [:db/add 137 :logseq.property.class/extends 1]
-              {:block/name (string/lower-case common-config/library-page-name),
-               :block/title common-config/library-page-name,
-               :block/uuid #uuid "00000004-1294-7765-6000-000000000000",
-               :block/tags #{:logseq.class/Page},
-               :logseq.property/built-in? true}
-              {:db/id 154,
-               :block/parent
-               [:block/uuid #uuid "00000004-1294-7765-6000-000000000000"],
-               ;; :block/order "a6"
-               }
-              {:db/id 155,
-               ;; :block/order "a7"
-               }
-              {:db/id 156,
-               ;; :block/order "a8"
-               }
-              {:db/id 158,
-               ;; :block/order "a9"
-               }])))))
-
 (deftest ensure-built-in-data-exists!
   (let [db-transit (str (fs-node/readFileSync "src/test/migration/64.8.transit"))
         db (ldb/read-transit-str db-transit)
@@ -104,40 +20,3 @@
     (is (= graph-created-at
            (:kv/value (d/entity @conn :logseq.kv/graph-created-at)))
         "Graph created at not changed by fn")))
-
-(deftest test-separate-classes-and-properties
-  (testing "Separate properties from classes"
-    (let [db-transit (str (fs-node/readFileSync "src/test/migration/65.0.transit"))
-          db (ldb/read-transit-str db-transit)
-          tx-data (db-migrate/separate-classes-and-properties db)
-          new-property (first tx-data)]
-      (is (= (dissoc new-property
-                     :block/updated-at
-                     :block/created-at
-                     :db/ident
-                     :block/uuid
-                     :block/order)
-             {:db/index true,
-              :logseq.property/type :node,
-              :db/valueType :db.type/ref,
-              :block/tags #{:logseq.class/Property},
-              :block/title "Book",
-              :db/cardinality :db.cardinality/one,
-              :logseq.property/classes 156,
-              :block/name "book"}))
-      (is (= (rest tx-data)
-             [[:db/retract 156 :block/tags :logseq.class/Property]
-              [:db/retract 156 :logseq.property/type]
-              [:db/retract 156 :db/cardinality]
-              [:db/retract 156 :db/valueType]
-              [:db/retract 156 :db/index]
-              [:db/retract 156 :logseq.property/classes]
-              [:db/retract 156 :logseq.property/hide?]
-              [:db/retract 156 :logseq.property/public?]
-              [:db/retract 156 :logseq.property/view-context]
-              [:db/retract 156 :logseq.property/ui-position]
-              [:db/retract 156 :logseq.property/default-value]
-              [:db/retract 156 :logseq.property/hide-empty-value]
-              [:db/retract 156 :logseq.property/enable-history?]
-              [:db/retract 157 :user.class/Book-FrG9O7sY 155]
-              [:db/add 157 (:db/ident new-property) 155]])))))

src/test/frontend/worker/rtc/migrate_test.cljs

@@ -41,13 +41,6 @@
             (is (= :logseq.kv/schema-version (get-in last-op [2 :db-ident])) "The schema version key should be correct")
             (is (= (:to-version migration-result) (get-in last-op [2 :value])) "The schema version should be updated to the new version"))))
 
-      (testing "check 65.3"
-        (let [upgrade-result-65-3 (get-specific-result (:upgrade-result-coll migration-result) "65.3")
-              rename-db-idents (set (:rename-db-idents (:migrate-updates upgrade-result-65-3)))
-              rename-db-ident-op-values (set (keep (fn [op] (when (= :rename-db-ident (first op)) (last op))) client-ops))]
-          (is (some? upgrade-result-65-3))
-          (is (= rename-db-idents rename-db-ident-op-values))))
-
       (testing "check 65.10"
         (let [upgrade-result-65-10 (get-specific-result (:upgrade-result-coll migration-result) "65.10")
               {:keys [tx-data db-after]} upgrade-result-65-10]