From df8f16ba2cc46830f3971781822804e1ca9954b8 Mon Sep 17 00:00:00 2001
From: Gabriel Horner <gabriel@logseq.com>
Date: Fri, 6 Oct 2023 15:18:37 -0400
Subject: [PATCH] add validation for internal and user properties

by reusing existing schema-types for db properties
---
 deps/db/src/logseq/db/property.cljs           |  8 ++++++
 .../tasks/db_graph/validate_client_db.cljs    | 25 +++++++++++++++++--
 src/main/frontend/components/property.cljs    |  4 +--
 .../frontend/handler/db_based/property.cljs   |  7 ++++--
 src/main/frontend/handler/property.cljs       |  3 ---
 5 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/deps/db/src/logseq/db/property.cljs b/deps/db/src/logseq/db/property.cljs
index b90f283757..a6801ce359 100644
--- a/deps/db/src/logseq/db/property.cljs
+++ b/deps/db/src/logseq/db/property.cljs
@@ -2,6 +2,14 @@
   "Property related fns for DB graphs and frontend/datascript usage"
   (:require [clojure.set :as set]))
 
+(def internal-builtin-schema-types
+  "Valid schema :type only to be used by built-in-properties"
+  #{:keyword :map :coll :any})
+
+(def user-builtin-schema-types
+  "Valid schema :type for users in order they appear in the UI"
+  [:default :number :date :checkbox :url :page :template :enum])
+
 ;; FIXME: no support for built-in-extended-properties
 (def ^:large-vars/data-var built-in-properties
   "Map of built in properties for db graphs. Each property has a config map with
diff --git a/scripts/src/logseq/tasks/db_graph/validate_client_db.cljs b/scripts/src/logseq/tasks/db_graph/validate_client_db.cljs
index 04567dba07..ddf3a205a6 100644
--- a/scripts/src/logseq/tasks/db_graph/validate_client_db.cljs
+++ b/scripts/src/logseq/tasks/db_graph/validate_client_db.cljs
@@ -3,6 +3,7 @@
   (:require [logseq.db.sqlite.cli :as sqlite-cli]
             [logseq.db.sqlite.db :as sqlite-db]
             [logseq.db.schema :as db-schema]
+            [logseq.db.property :as db-property]
             [datascript.core :as d]
             [clojure.string :as string]
             [nbb.core :as nbb]
@@ -79,14 +80,28 @@
     page-attrs
     page-or-block-attrs)))
 
-(def property-page
+(def internal-property
   (vec
    (concat
     [:map {:closed false}]
     [[:block/schema
       [:map
        {:closed false}
-       [:type :keyword]
+       [:type (apply vector :enum (into db-property/internal-builtin-schema-types
+                                        db-property/user-builtin-schema-types))]
+       [:hide? {:optional true} :boolean]
+       [:cardinality {:optional true} [:enum :one :many]]]]]
+    page-attrs
+    page-or-block-attrs)))
+
+(def user-property
+  (vec
+   (concat
+    [:map {:closed false}]
+    [[:block/schema
+      [:map
+       {:closed false}
+       [:type (apply vector :enum db-property/user-builtin-schema-types)]
        [:hide? {:optional true} :boolean]
        [:description {:optional true} :string]
        ;; For any types except for :checkbox :default :template :enum
@@ -98,6 +113,12 @@
     page-attrs
     page-or-block-attrs)))
 
+(def property-page
+  [:multi {:dispatch
+           (fn [m] (contains? db-property/built-in-properties-keys-str (:block/name m)))}
+   [true internal-property]
+   [::m/default user-property]])
+
 (def page-block
   [:multi {:dispatch :block/type}
    [#{"property"} property-page]
diff --git a/src/main/frontend/components/property.cljs b/src/main/frontend/components/property.cljs
index 687ddb089a..bba616f6fa 100644
--- a/src/main/frontend/components/property.cljs
+++ b/src/main/frontend/components/property.cljs
@@ -318,9 +318,9 @@
 
       [:div.grid.grid-cols-4.gap-1.items-center.leading-8
        [:label.col-span-1 "Schema type:"]
-       (let [schema-types (->> (concat property-handler/user-face-builtin-schema-types
+       (let [schema-types (->> (concat db-property/user-builtin-schema-types
                                        (when built-in-property?
-                                         property-handler/internal-builtin-schema-types))
+                                         db-property/internal-builtin-schema-types))
                                (map (comp string/capitalize name))
                                (map (fn [type]
                                       {:label (if (= type "Default") "Text" type)
diff --git a/src/main/frontend/handler/db_based/property.cljs b/src/main/frontend/handler/db_based/property.cljs
index 8fee2905c3..81a11df2e1 100644
--- a/src/main/frontend/handler/db_based/property.cljs
+++ b/src/main/frontend/handler/db_based/property.cljs
@@ -9,6 +9,7 @@
             [frontend.util :as util]
             [logseq.graph-parser.util :as gp-util]
             [logseq.db.sqlite.util :as sqlite-util]
+            [logseq.db.property :as db-property]
             [malli.util :as mu]
             [malli.error :as me]))
 
@@ -57,8 +58,10 @@
    :coll     coll?
    :any      some?})
 
-(def internal-builtin-schema-types #{:keyword :map :coll :any})
-(def user-face-builtin-schema-types [:default :number :date :checkbox :url :page :template :enum])
+(assert (= (set (keys builtin-schema-types))
+           (into db-property/internal-builtin-schema-types
+                 db-property/user-builtin-schema-types))
+        "Built-in schema types must be equal")
 
 ;; schema -> type, cardinality, object's class
 ;;           min, max -> string length, number range, cardinality size limit
diff --git a/src/main/frontend/handler/property.cljs b/src/main/frontend/handler/property.cljs
index 0728df87f6..487c970339 100644
--- a/src/main/frontend/handler/property.cljs
+++ b/src/main/frontend/handler/property.cljs
@@ -8,9 +8,6 @@
             [frontend.state :as state]
             [frontend.db :as db]))
 
-(def user-face-builtin-schema-types db-property-handler/user-face-builtin-schema-types)
-(def internal-builtin-schema-types db-property-handler/internal-builtin-schema-types)
-
 (defn remove-block-property!
   [repo block-id key]
   (if (config/db-based-graph? repo)