metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 204 forks source link

Support Var-refs #985

Closed ikitommi closed 5 months ago

ikitommi commented 6 months ago

Open Questions

Sample

(require '[malli.core :as m])
(require '[malli.registry :as mr])
(require '[malli.generator :as mg])

;; enable var registry
(mr/set-default-registry!
 (mr/composite-registry
  (m/default-schemas)
  (mr/var-registry)))

(def UserId :string)

(def User
  [:map
   [:id #'UserId]
   [:friends {:optional true} [:set [:ref #'User]]]])

(m/form User)
;[:map 
; [:id #'user/UserId]
; [:friends {:optional true} [:set [:ref #'user/User]]]]

(m/validate User {:id "123", :friends #{{:id "234"}}})
; => true

(mg/sample User {:seed 42})
;({:id ""}
; {:id "5"}
; {:id ""}
; {:id "B62"}
; {:id "Vc16", :friends #{}}
; {:id "E",
;  :friends #{{:id ""}
;             {:id "u", :friends #{}}
;             {:id "", :friends #{}}
;             {:id "GL", :friends #{{:id "i"} {:id "o", :friends #{}}}}
;             {:id "d"}}}
; {:id "", :friends #{{:id ""} {:id "0Ql1", :friends #{{:id "3", :friends #{}}}}}}
; {:id "l"}
; {:id "",
;  :friends #{{:id "R8p", :friends #{}}
;             {:id "d8"}
;             {:id "A2u5k9G6"}
;             {:id ""}
;             {:id "C2", :friends #{{:id "33"} {:id "", :friends #{}} {:id "g4"}}}}}
; {:id "IrcCSYwSO"})

Invalid ref error

((requiring-resolve 'malli.dev/start!))

(m/schema [:ref 123])
invalid-ref
bsless commented 6 months ago

My 2c:

The part I'm most interested in wrt this MR is how var references interact with JSON schema and its derivatives:

ilmoraunio commented 6 months ago

should var-registry be enabled by default?

What would this this mean in terms of code?

If it means running mr/set-default-registry! implicitly using a composite-var-registry, then there also needs to be a way to disable it easily. Also, this would likely constitute a bigger jump in terms of versioning semantics. But I'm carefully for it.

ikitommi commented 6 months ago

@bsless

The part I'm most interested in wrt this MR is how var references interact with JSON schema and its derivatives:

Do they get lifted to definitions?

yes, now with tests. moved all references to strings to ensure they get encoded correctly.

(def UserId :string)

(def User
  [:map {:registry {::location [:tuple :double :double]
                    `description :string}}
   [:id #'UserId]
   ::location
   `description
   [:friends {:optional true} [:set [:ref #'User]]]])

(json-schema/transform User)
;{:type "object",
; :properties {:id {:$ref "#/definitions/malli.json-schema-test~1UserId"},
;              :malli.json-schema-test/location {:$ref "#/definitions/malli.json-schema-test~1location"},
;              malli.json-schema-test/description {:$ref "#/definitions/malli.json-schema-test~1description"},
;              :friends {:type "array", :items {:$ref "#/definitions/malli.json-schema-test~1User"}, :uniqueItems true}},
; :required [:id :malli.json-schema-test/location malli.json-schema-test/description],
; :definitions {"malli.json-schema-test/UserId" {:type "string"},
;               "malli.json-schema-test/location" {:type "array",
;                                                  :items [{:type "number"} {:type "number"}],
;                                                  :additionalItems false},
;               "malli.json-schema-test/description" {:type "string"},
;               "malli.json-schema-test/User" {:type "object",
;                                              :properties {:id {:$ref "#/definitions/malli.json-schema-test~1UserId"},
;                                                           :malli.json-schema-test/location {:$ref "#/definitions/malli.json-schema-test~1location"},
;                                                           malli.json-schema-test/description {:$ref "#/definitions/malli.json-schema-test~1description"},
;                                                           :friends {:type "array",
;                                                                     :items {:$ref "#/definitions/malli.json-schema-test~1User"},
;                                                                     :uniqueItems true}},
;                                              :required [:id
;                                                         :malli.json-schema-test/location
;                                                         malli.json-schema-test/description]}}}

Do they get qualified?

yes.

Should they?

yes. qualified keywords, symbols and Vars are all global in Clojure, so they can and should be global in JSON Schema too.

bsless commented 6 months ago

In that case, I think we'll need properties support to override the namespace qualification, and some "global" property to add a mapping of qualifying namespace to other names when transforming the schema, with some means of conveying it to reitit

ikitommi commented 6 months ago

@bsless so, you would not like to expose the namespaces? just for Vars or also for qualified keywords and symbols?

bsless commented 6 months ago

so, you would not like to expose the namespaces?

Exactly. It's an implementation detail and can just add noise and confusion. Why would the consumer care that my schema is a com.foo.my-service.blash.schema/User? The benefit is it maps directly to the place where the schema is defined. Since it has pros and cons for each option, it makes sense to make it configurable and leave it up to users.

just for Vars or also for qualified keywords and symbols?

With qualified idents you can already give them whatever namespace prefix you like (and some argue you shouldn't use a ::foo anyway), so it's less of an issue.

ikitommi commented 6 months ago

also, added report for the invalid function reg:

register-function-schemas
ikitommi commented 6 months ago

@bsless extracted the requirement to mask Var namespaces in JSON Schema / Swagger into separate ns - https://github.com/metosin/malli/issues/988

@ilmoraunio - didn't push the VarRegistry into defaults. Will sleep over this.