fix: adding class properties can result

in clobbered db idents. Also rename fn to make its
purpose explicit

deps/db/src/logseq/db/frontend/property.cljs

@@ -276,8 +276,11 @@
                 e))) values)))
 
 ;; TODO: db ident should obey clojure's rules for keywords
-(defn user-property-ident-from-name
-  "Makes a user property :db/ident from a name by sanitizing the given name"
+(defn create-user-property-ident-from-name
+  "Creates a user property :db/ident from a name by sanitizing the given name.
+
+   NOTE: Only use this when creating a db-ident for a property name. Using this
+   in read-only contexts like querying can result in db-ident conflicts"
   [property-name]
   (let [n (-> (string/lower-case property-name)
               (string/replace #"(^:\s*|\s*:$)" "")

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

@@ -76,7 +76,7 @@
    (assert (keyword? db-ident))
    (let [db-ident' (if (qualified-keyword? db-ident)
                      db-ident
-                     (db-property/user-property-ident-from-name (name db-ident)))
+                     (db-property/create-user-property-ident-from-name (name db-ident)))
          prop-name (or original-name (name db-ident'))]
      (block-with-timestamps
       (cond->

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

@@ -68,7 +68,7 @@
   (->> properties
        (map
         (fn [[prop-name val]]
-          [(db-property/user-property-ident-from-name (name prop-name))
+          [(db-property/create-user-property-ident-from-name (name prop-name))
             ;; set indicates a :many value
            (if (set? val)
              (set (map #(translate-property-value % uuid-maps) val))
@@ -105,7 +105,7 @@
 (defn- build-property-refs [properties property-db-ids]
   (mapv
    (fn [prop-name]
-     (db-property/user-property-ident-from-name (name prop-name)))
+     (db-property/create-user-property-ident-from-name (name prop-name)))
    (keys properties)))
 
 (def current-db-id (atom 0))
@@ -168,7 +168,7 @@
                            (mapcat
                             (fn [[prop-name]]
                               (if (get-in properties [prop-name :closed-values])
-                                (let [db-ident (db-property/user-property-ident-from-name (name prop-name))]
+                                (let [db-ident (db-property/create-user-property-ident-from-name (name prop-name))]
                                   (db-property-util/build-closed-values
                                    db-ident
                                    prop-name

src/main/frontend/handler/db_based/property.cljs

@@ -104,8 +104,9 @@
         {:db/ident ident
          :db/cardinality cardinality})))
 
-(defn ensure-unique-ident
-  "Ensures the given db-ident is unique by querying for any existing ones and adding a suffix if needed"
+(defn- ensure-unique-db-ident
+  "Ensures the given db-ident is unique. If a db-ident conflicts, it is made
+  unique by adding a suffix with a unique number e.g. :db-ident-1 :db-ident-2"
   [db db-ident]
   (if (d/entity db db-ident)
     (let [existing-idents
@@ -130,7 +131,7 @@
    with the given property-id or :property-name option. When a property is created
    it is ensured to have a unique :db/ident"
   [repo property-id schema {:keys [property-name properties]}]
-  (let [db-ident (or property-id (db-property/user-property-ident-from-name property-name))]
+  (let [db-ident (or property-id (db-property/create-user-property-ident-from-name property-name))]
     (assert (qualified-keyword? db-ident))
     (if-let [property (and (qualified-keyword? property-id) (db/entity db-ident))]
       (let [tx-data (->>
@@ -148,7 +149,7 @@
         (db/transact! repo tx-data {:outliner-op :save-block}))
       (let [k-name (or (and property-name (name property-name))
                        (name property-id))
-            db-ident' (ensure-unique-ident (db/get-db repo) db-ident)]
+            db-ident' (ensure-unique-db-ident (db/get-db repo) db-ident)]
         (assert (some? k-name)
                 (prn "property-id: " property-id ", property-name: " property-name))
         (db/transact! repo
@@ -284,19 +285,26 @@
   [repo class-uuid property-id]
   (when-let [class (db/entity repo [:block/uuid class-uuid])]
     (when (contains? (:block/type class) "class")
-      (let [[db-ident property]
+      (let [[db-ident property options]
             ;; strings come from user
             (if (string? property-id)
               (if-let [ent (db/entity [:block/original-name property-id])]
-                [(:db/ident ent) ent]
-                [(db-property/user-property-ident-from-name property-id) nil])
-              [property-id (db/entity property-id)])
+                [(:db/ident ent) ent {}]
+                ;; creates ident beforehand b/c needed in later transact and this avoids
+                ;; making this whole fn async for now
+                [(ensure-unique-db-ident
+                  (db/get-db (state/get-current-repo))
+                  (db-property/create-user-property-ident-from-name property-id))
+                 nil
+                 {:property-name property-id}])
+              [property-id (db/entity property-id) {}])
             property-type (get-in property [:block/schema :type])
-            _ (upsert-property! repo db-ident
+            _ (upsert-property! repo
+                                db-ident
                                 (cond-> (:block/schema property)
                                   (some? property-type)
                                   (assoc :type property-type))
-                                {})]
+                                options)]
         (db/transact! repo
                       [[:db/add (:db/id class) :class/schema.properties db-ident]]
                       {:outliner-op :save-block})))))

src/main/frontend/handler/property/util.cljs

@@ -24,7 +24,7 @@
                         (name key)
                         key)]
     (if (sqlite-util/db-based-graph? repo)
-      (when-let [property (d/entity db (db-property/user-property-ident-from-name property-name))]
+      (when-let [property (d/entity db (db-property/create-user-property-ident-from-name property-name))]
         (get coll (:block/uuid property)))
       (get coll key))))
 

src/test/frontend/handler/db_based/property_test.cljs

@@ -178,11 +178,13 @@
       (db-property-handler/class-add-property! repo c1id :user.property/property-2)
       ;; repeated adding property-2
       (db-property-handler/class-add-property! repo c1id :user.property/property-2)
-      (is (= 2 (count (:class/schema.properties (db/entity (:db/id c1)))))))
+      ;; add new property with same base db-ident as property-1
+      (db-property-handler/class-add-property! repo c1id ":property-1")
+      (is (= 3 (count (:class/schema.properties (db/entity (:db/id c1)))))))
 
     (testing "Class remove property"
       (db-property-handler/class-remove-property! repo c1id :user.property/property-1)
-      (is (= 1 (count (:class/schema.properties (db/entity (:db/id c1)))))))
+      (is (= 2 (count (:class/schema.properties (db/entity (:db/id c1)))))))
     (testing "Add classes to a block"
       (test-helper/save-block! repo fbid "Block 1" {:tags ["class1" "class2" "class3"]})
       (is (= 3 (count (:block/tags (db/entity [:block/uuid fbid]))))))
@@ -199,7 +201,7 @@
                         :class/parent (:db/id c2)}]))
       (db-property-handler/class-add-property! repo c2id :user.property/property-3)
       (db-property-handler/class-add-property! repo c2id :user.property/property-4)
-      (is (= 3 (count (:classes-properties
+      (is (= 4 (count (:classes-properties
                        (db-property-handler/get-block-classes-properties (:db/id (db/entity [:block/uuid fbid]))))))))))