1
0
Эх сурвалжийг харах

enhance: all user classes have :db/ident

This is important as a user's ontology (properties + classes) is now describable
with unique identifiers. This will allow for easy sharing and
importing of ontologies between users.
Also <create-class! now exists to create classes consistently. Will
be helpful when we want to provide some validation for classes
Gabriel Horner 1 жил өмнө
parent
commit
effde81161

+ 2 - 0
.clj-kondo/config.edn

@@ -139,6 +139,8 @@
              logseq.common.util.macro macro-util
              logseq.db ldb
              logseq.db.frontend.content db-content
+             logseq.db.frontend.class db-class
+             logseq.db.frontend.db-ident db-ident
              logseq.db.frontend.inputs db-inputs
              logseq.db.frontend.order db-order
              logseq.db.frontend.property db-property

+ 4 - 0
deps/db/.carve/ignore

@@ -12,3 +12,7 @@ logseq.db.frontend.rules/rules
 logseq.db.frontend.rules/rules-dependencies
 ;; API
 logseq.db.frontend.inputs/resolve-input
+;; API
+logseq.db.frontend.class/build-new-class
+;; API
+logseq.db.frontend.db-ident/ensure-unique-db-ident

+ 22 - 1
deps/db/src/logseq/db/frontend/class.cljs

@@ -1,5 +1,7 @@
 (ns logseq.db.frontend.class
-  "Class related fns for DB graphs and frontend/datascript usage")
+  "Class related fns for DB graphs and frontend/datascript usage"
+  (:require [logseq.db.sqlite.util :as sqlite-util]
+            [logseq.db.frontend.db-ident :as db-ident]))
 
 (def ^:large-vars/data-var built-in-classes
   "Map of built-in classes for db graphs with their :db/ident as keys"
@@ -14,3 +16,22 @@
                        }
    ;; TODO: Add more classes such as :book, :paper, :movie, :music, :project
    })
+
+(defn create-user-class-ident-from-name
+  "Creates a class :db/ident for a default user namespace"
+  [class-name]
+  (db-ident/create-db-ident-from-name "user.class" class-name))
+
+(defn build-new-class
+  "Builds a new class with a unique :db/ident. Also throws exception for user
+  facing messages when name is invalid"
+  [db page-m]
+  {:pre [(string? (:block/original-name page-m))]}
+  (let [db-ident (try (create-user-class-ident-from-name (:block/original-name page-m))
+                      (catch :default e
+                        (throw (ex-info (str e)
+                                        {:type :notification
+                                         :payload {:message "Failed to create class. Please try a different class name."
+                                                   :type :error}}))))
+        db-ident' (db-ident/ensure-unique-db-ident db db-ident)]
+    (sqlite-util/build-new-class (assoc page-m :db/ident db-ident'))))

+ 44 - 0
deps/db/src/logseq/db/frontend/db_ident.cljs

@@ -0,0 +1,44 @@
+(ns logseq.db.frontend.db-ident
+  "Helper fns for class and property :db/ident"
+  (:require [datascript.core :as d]
+            [clojure.string :as string]))
+
+(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
+          (d/q '[:find [?ident ...]
+                 :in $ ?ident-name
+                 :where
+                 [?b :db/ident ?ident]
+                 [(str ?ident) ?str-ident]
+                 [(clojure.string/starts-with? ?str-ident ?ident-name)]]
+               db
+               (str db-ident "-"))
+          new-ident (if-let [max-num (->> existing-idents
+                                          (keep #(parse-long (string/replace-first (str %) (str db-ident "-") "")))
+                                          (apply max))]
+                      (keyword (namespace db-ident) (str (name db-ident) "-" (inc max-num)))
+                      (keyword (namespace db-ident) (str (name db-ident) "-1")))]
+      new-ident)
+    db-ident))
+
+;; TODO: db ident should obey clojure's rules for keywords
+(defn create-db-ident-from-name
+  "Creates a :db/ident for a class or property by sanitizing the given name.
+
+   NOTE: Only use this when creating a db-ident for a new class/property. Using
+   this in read-only contexts like querying can result in db-ident conflicts"
+  [user-namespace name-string]
+  {:pre [(string? name-string)]}
+  (let [n (-> name-string
+              (string/replace #"(^:\s*|\s*:$)" "")
+              (string/replace #"\s*:\s*$" "")
+              (string/replace-first #"^\d+" "")
+              (string/replace " " "-")
+              (string/replace "#" "")
+              (string/trim))]
+    (assert (seq n) "name is not empty")
+    (keyword user-namespace n)))

+ 1 - 1
deps/db/src/logseq/db/frontend/malli_schema.cljs

@@ -236,7 +236,7 @@
     page-or-block-attrs)))
 
 (def class-attrs
-  [[:db/ident {:optional true} class-ident]
+  [[:db/ident class-ident]
    [:class/parent {:optional true} :int]
    [:class/schema.properties {:optional true} [:set :int]]])
 

+ 3 - 19
deps/db/src/logseq/db/frontend/property.cljs

@@ -2,7 +2,8 @@
   "Property related fns for DB graphs and frontend/datascript usage"
   (:require [datascript.core :as d]
             [clojure.string :as string]
-            [flatland.ordered.map :refer [ordered-map]]))
+            [flatland.ordered.map :refer [ordered-map]]
+            [logseq.db.frontend.db-ident :as db-ident]))
 
 ;; Main property vars
 ;; ==================
@@ -239,27 +240,10 @@
             (when (= (closed-value-name e) value-name)
               e)) values)))
 
-;; TODO: db ident should obey clojure's rules for keywords
-(defn create-db-ident-from-name
-  "Creates a :db/ident by sanitizing the given name.
-
-   NOTE: Only use this when creating a db-ident for a string name. Using this
-   in read-only contexts like querying can result in db-ident conflicts"
-  [user-namespace name-string]
-  (let [n (-> name-string
-              (string/replace #"(^:\s*|\s*:$)" "")
-              (string/replace #"\s*:\s*$" "")
-              (string/replace-first #"^\d+" "")
-              (string/replace " " "-")
-              (string/replace "#" "")
-              (string/trim))]
-    (assert (seq n) "name is not empty")
-    (keyword user-namespace n)))
-
 (defn create-user-property-ident-from-name
   "Creates a property :db/ident for a default user namespace"
   [property-name]
-  (create-db-ident-from-name "user.property" property-name))
+  (db-ident/create-db-ident-from-name "user.property" property-name))
 
 (defn get-class-ordered-properties
   [class-entity]

+ 1 - 1
deps/db/src/logseq/db/sqlite/util.cljs

@@ -105,10 +105,10 @@
         (or ref-type? (contains? (conj db-property-type/ref-property-types :entity) (:type prop-schema)))
         (assoc :db/valueType :db.type/ref))))))
 
-
 (defn build-new-class
   "Build a standard new class so that it is is consistent across contexts"
   [block]
+  {:pre [(qualified-keyword? (:db/ident block))]}
   (block-with-timestamps
    (merge (cond->
            {:block/type "class"

+ 2 - 23
deps/outliner/src/logseq/outliner/property.cljs

@@ -11,6 +11,7 @@
             [logseq.db.frontend.property :as db-property]
             [logseq.db.frontend.property.build :as db-property-build]
             [logseq.db.frontend.property.type :as db-property-type]
+            [logseq.db.frontend.db-ident :as db-ident]
             [logseq.db.sqlite.util :as sqlite-util]
             [logseq.graph-parser.block :as gp-block]
             [logseq.outliner.core :as outliner-core]
@@ -85,28 +86,6 @@
      (when (and old-ref-type? (not ref-type?))
        [:db/retract (:db/id property) :db/valueType])]))
 
-(defn ^:api 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
-          (d/q '[:find [?ident ...]
-                 :in $ ?ident-name
-                 :where
-                 [?b :db/ident ?ident]
-                 [(str ?ident) ?str-ident]
-                 [(clojure.string/starts-with? ?str-ident ?ident-name)]]
-               db
-               (str db-ident "-"))
-          new-ident (if-let [max-num (->> existing-idents
-                                          (keep #(parse-long (string/replace-first (str %) (str db-ident "-") "")))
-                                          (apply max))]
-                      (keyword (namespace db-ident) (str (name db-ident) "-" (inc max-num)))
-                      (keyword (namespace db-ident) (str (name db-ident) "-1")))]
-      new-ident)
-    db-ident))
-
 (defn upsert-property!
   "Updates property if property-id is given. Otherwise creates a property
    with the given property-id or :property-name option. When a property is created
@@ -153,7 +132,7 @@
         property)
       (let [k-name (or (and property-name (name property-name))
                        (name property-id))
-            db-ident' (ensure-unique-db-ident @conn db-ident)]
+            db-ident' (db-ident/ensure-unique-db-ident @conn db-ident)]
         (assert (some? k-name)
                 (prn "property-id: " property-id ", property-name: " property-name))
         (ldb/transact! conn

+ 6 - 4
scripts/src/logseq/tasks/db_graph/create_graph.cljs

@@ -17,7 +17,9 @@
             [nbb.classpath :as cp]
             [logseq.db.frontend.property :as db-property]
             [logseq.db.frontend.order :as db-order]
-            [logseq.db.frontend.property.type :as db-property-type]))
+            [logseq.db.frontend.property.type :as db-property-type]
+            [logseq.db.frontend.class :as db-class]
+            [logseq.db.frontend.db-ident :as db-ident]))
 
 (defn- find-on-classpath [rel-path]
   (some (fn [dir]
@@ -241,7 +243,7 @@
   (let [property-idents (->> (keys properties)
                              (map #(vector %
                                            (if graph-namespace
-                                             (db-property/create-db-ident-from-name (str (name graph-namespace) ".property")
+                                             (db-ident/create-db-ident-from-name (str (name graph-namespace) ".property")
                                                                                     (name %))
                                              (db-property/create-user-property-ident-from-name (name %)))))
                              (into {}))
@@ -249,9 +251,9 @@
         class-idents (->> (keys classes)
                           (map #(vector %
                                         (if graph-namespace
-                                          (db-property/create-db-ident-from-name (str (name graph-namespace) ".class")
+                                          (db-ident/create-db-ident-from-name (str (name graph-namespace) ".class")
                                                                                  (name %))
-                                          (db-property/create-db-ident-from-name "user.class" (name %)))))
+                                          (db-class/create-user-class-ident-from-name (name %)))))
                           (into {}))
         _ (assert (= (count (set (vals class-idents))) (count classes)) "All class db-idents must be unique")
         all-idents (merge property-idents class-idents)]

+ 5 - 5
src/main/frontend/components/cmdk/core.cljs

@@ -512,14 +512,14 @@
         class (when create-class? (get-class-from-input @!input))]
     (p/do!
       (cond
-        create-class? (page-handler/<create! class
-                        {:redirect? false
-                         :create-first-block? false
-                         :class? true})
+        create-class?
+        (page-handler/<create-class! class
+                                     {:redirect? false
+                                      :create-first-block? false})
         create-whiteboard? (whiteboard-handler/<create-new-whiteboard-and-redirect! @!input)
         create-page? (page-handler/<create! @!input {:redirect? true}))
       (if create-class?
-        (state/pub-event! [:class/configure (db/get-page class)])
+        (state/pub-event! [:class/configure (db/get-case-page class)])
         (state/close-modal!)))))
 
 (defn- get-filter-user-input

+ 2 - 3
src/main/frontend/components/property.cljs

@@ -39,9 +39,8 @@
   (when (string? value)
     (let [page-name (string/trim value)]
       (when-not (string/blank? page-name)
-        (p/let [page (page-handler/<create! page-name {:redirect? false
-                                                       :create-first-block? false
-                                                       :class? true})]
+        (p/let [page (page-handler/<create-class! page-name {:redirect? false
+                                                             :create-first-block? false})]
           (:block/uuid page))))))
 
 (rum/defc class-select

+ 11 - 9
src/main/frontend/components/property/value.cljs

@@ -219,15 +219,17 @@
               (or (:block/uuid (ldb/get-case-page (db/get-db) inline-class))
                   (do (log/error :msg "Given inline class does not exist" :inline-class inline-class)
                       nil)))
-            class? (= :block/tags (:db/ident property))]
-        (p/let [page (page-handler/<create! page {:redirect? false
-                                                  :create-first-block? false
-                                                  :tags (if inline-class-uuid
-                                                          [inline-class-uuid]
-                                                          ;; Only 1st class b/c page normally has
-                                                          ;; one of and not all these classes
-                                                          (mapv :block/uuid (take 1 classes)))
-                                                  :class? class?})]
+            class? (= :block/tags (:db/ident property))
+            create-options {:redirect? false
+                            :create-first-block? false
+                            :tags (if inline-class-uuid
+                                    [inline-class-uuid]
+                                    ;; Only 1st class b/c page normally has
+                                    ;; one of and not all these classes
+                                    (mapv :block/uuid (take 1 classes)))}]
+        (p/let [page (if class?
+                       (page-handler/<create-class! page create-options)
+                       (page-handler/<create! page create-options))]
           (:db/id page)))
       id)))
 

+ 36 - 12
src/main/frontend/handler/page.cljs

@@ -41,12 +41,24 @@
             [logseq.graph-parser.db :as gp-db]
             [frontend.modules.outliner.ui :as ui-outliner-tx]
             [frontend.modules.outliner.op :as outliner-op]
-            [frontend.handler.property.util :as pu]))
+            [frontend.handler.property.util :as pu]
+            [logseq.db.frontend.class :as db-class]))
 
 (def create! page-common-handler/create!)
 (def <create! page-common-handler/<create!)
 (def <delete! page-common-handler/<delete!)
 
+(defn <create-class!
+  "Creates a class page and provides class-specific error handling"
+  [title options]
+  (-> (page-common-handler/<create! title (assoc options :class? true))
+      (p/catch (fn [e]
+                 (when (= :notification (:type (ex-data e)))
+                   (notification/show! (get-in (ex-data e) [:payload :message])
+                                       (get-in (ex-data e) [:payload :type])))
+                 ;; Re-throw as we don't want to proceed with a nonexistent class
+                 (throw e)))))
+
 (defn <unfavorite-page!
   [page-name]
   (p/do!
@@ -278,13 +290,23 @@
     (let [current-selected (util/get-selected-text)]
       (cursor/move-cursor-forward input (+ 2 (count current-selected))))))
 
-(defn add-tag [repo block-id tag & {:keys [tag-entity]}]
-  (let [tag-entity (or tag-entity (db/get-page tag))
-        tx-data [[:db/add (:db/id tag-entity) :block/type "class"]
-                 [:db/add [:block/uuid block-id] :block/tags (:db/id tag-entity)]
-                 ;; TODO: Should classes counted as refs
-                 [:db/add [:block/uuid block-id] :block/refs (:db/id tag-entity)]]]
-    (db/transact! repo tx-data {:outliner-op :save-block})))
+(defn add-tag [repo block-id tag-entity]
+  (try
+    (let [tx-data (cond-> []
+                    ;; Don't rebuild an existing class as it changes their :db/ident
+                    (not (contains? (:block/type tag-entity) "class"))
+                    (conj (db-class/build-new-class (conn/get-db)
+                                                    (select-keys tag-entity [:db/id :block/original-name])))
+                    true
+                    (into [[:db/add [:block/uuid block-id] :block/tags (:db/id tag-entity)]
+                         ;; TODO: Should classes counted as refs
+                           [:db/add [:block/uuid block-id] :block/refs (:db/id tag-entity)]]))]
+      (db/transact! repo tx-data {:outliner-op :save-block}))
+    (catch :default e
+      (if (= :notification (:type (ex-data e)))
+        (notification/show! (get-in (ex-data e) [:payload :message])
+                            (get-in (ex-data e) [:payload :type]))
+        (throw e)))))
 
 (defn on-chosen-handler
   [input id _q pos format]
@@ -326,12 +348,14 @@
                (when (and (not (string/blank? tag)) (:block/uuid edit-block))
                  (p/let [tag-entity (get-page-fn tag)
                          _ (when-not tag-entity
-                             (<create! tag {:redirect? false
-                                            :create-first-block? false
-                                            :class? class?}))
+                             (if class?
+                               (<create-class! tag {:redirect false
+                                                    :create-first-block? false})
+                               (<create! tag {:redirect? false
+                                              :create-first-block? false})))
                          tag-entity (get-page-fn tag)]
                    (when class?
-                     (add-tag (state/get-current-repo) (:block/uuid edit-block) tag {:tag-entity tag-entity}))))))
+                     (add-tag (state/get-current-repo) (:block/uuid edit-block) tag-entity))))))
            (editor-handler/insert-command! id
                                            (if class? "" (str "#" wrapped-tag))
                                            format

+ 9 - 9
src/main/frontend/worker/handler/page.cljs

@@ -15,7 +15,8 @@
             [frontend.worker.date :as date]
             [logseq.db.frontend.order :as db-order]
             [logseq.db.frontend.property.util :as db-property-util]
-            [logseq.db.frontend.property.build :as db-property-build]))
+            [logseq.db.frontend.property.build :as db-property-build]
+            [logseq.db.frontend.class :as db-class]))
 
 (defn properties-block
   [repo conn config date-formatter properties format page]
@@ -50,15 +51,14 @@
                             (when (db-property-util/built-in-has-ref-value? k)
                               [k v])))
                     (into {})))]
-          (cond-> [(merge page'
-                         (when class? {:block/type "class"}))]
-           (seq property-vals-tx-m)
-           (into (vals property-vals-tx-m))
-           true
-           (conj (merge {:block/uuid (:block/uuid page)}
+          (cond-> [(if class? (db-class/build-new-class @conn page') page')]
+            (seq property-vals-tx-m)
+            (into (vals property-vals-tx-m))
+            true
+            (conj (merge {:block/uuid (:block/uuid page)}
                        ;; FIXME: Add refs for properties?
-                        properties
-                        (db-property-build/build-properties-with-ref-values property-vals-tx-m)))))
+                         properties
+                         (db-property-build/build-properties-with-ref-values property-vals-tx-m)))))
         (let [file-page (merge page'
                                (when (seq properties) {:block/properties properties}))]
           (if (and (seq properties)

+ 1 - 1
src/test/frontend/test/helper.cljs

@@ -274,4 +274,4 @@ This can be called in synchronous contexts as no async fns should be invoked"
   [repo block-uuid content {:keys [tags]}]
   (editor-handler/save-block! repo block-uuid content)
   (doseq [tag tags]
-    (page-handler/add-tag repo block-uuid tag)))
+    (page-handler/add-tag repo block-uuid (db/get-page tag))))