metosin / malli

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

Support type-properties in malli.core/-map-schema and malli.core/-tuple-schema #855

Closed bgalartza closed 1 year ago

bgalartza commented 1 year ago

At the moment the malli.core/-map-schema and malli.core/-tuple-schema IntoSchemas don't have any type-property, and they don't allow setting new ones programmatically.

We would like to be able to specify the type-properties for the mentioned IntoSchemas to be able to extend their functionality by specifying type-wide custom decoders/encoders. The mt/transformer function does allow specifying type-specific encoders/decoders, but it's not good enough when you have a custom registry that references the built-in types.

For example, lets say we have the following schema with a custom registry:

(def test-schema
  (m/schema
   [::object
    [:test
     [::double]]]
   {:registry {::object (m/-map-schema)
               ::double (m/-double-schema)}}))

At decode time we can specify that we want to use a custom decoder for all maps:


(m/decode test-schema {:test 1.0}
          (mt/transformer
           {:name :test
            :decoders {:map {:enter #(prn %)}}}))

But we would like to specify the decoder for all objects, instead of maps.

@ikitommi mentioned in this Clojurians thread that if m/-map-schema would allow custom type-properties we could do the following:

(def test-schema
  (m/schema
   [::object
    [:test
     [::double]]]
   {:registry {::object (m/-map-schema {:type-properties {:decode/test {:enter #(prn %)}}})
               ::double (m/-double-schema)}}))

(m/decode
 test-schema
 {:test 1.0}
 (mt/transformer {:name :test}))

Following his example we tested the approach with malli.core/-collection-schema (which does allow setting the type-properties) and it perfectly fits our use case. We think that others might also benefit of the change, as it would allow extending the types with more functionality without having to re-implement them completely.

ikitommi commented 1 year ago

This would be a non-breaking change. PR most welcome!

bgalartza commented 1 year ago

We have been having a look at the code and we have found that -type-properties method implementation is different in the -simple-schema case and -collection-schema case. We initially thought that using the same implementation as -simple-schema would be enough, but we are no longer sure because we don't fully understand why the -collection-schema implementation uses an atom for the props. And we are not 100% sure if we should implement it that way (using an atom for the props).

ikitommi commented 1 year ago

both -simple-schema and -collection-schema support creating schema instances based on schema properties and childs via a callback function. For maps and tuples, don't think it's needed, so for :map, you can just change:

     (-type-properties [_] (:type-properties opts))

here's a sample about the atom in action with m/-simple-schema:

  (testing "with instance-based type-properties"
    (let [Over (m/-simple-schema
                (fn [{:keys [value]} _]
                  (assert (int? value))
                  {:type :user/over
                   :pred #(and (int? %) (> % value))
                   :type-properties {:error/message (str "should be over " value)
                                     :decode/string mt/-string->long
                                     :json-schema/type "integer"
                                     :json-schema/format "int64"
                                     :json-schema/minimum value}}))]

      (testing "over6"
        (let [schema [Over {:value 6}]]
          (testing "form"
            (is (= [:user/over {:value 6}] (m/form schema))))
          (testing "validation"
            (is (false? (m/validate schema 6)))
            (is (true? (m/validate schema 7))))
          (testing "properties"
            (is (= {:error/message "should be over 6"
                    :decode/string mt/-string->long
                    :json-schema/type "integer"
                    :json-schema/format "int64"
                    :json-schema/minimum 6}
                   (m/type-properties schema)))
            (is (= {:value 6}
                   (m/properties schema))))))

      (testing "over42"
        (let [schema [Over {:value 42}]]
          (testing "form"
            (is (= [:user/over {:value 42}] (m/form schema))))
          (testing "validation"
            (is (false? (m/validate schema 42)))
            (is (true? (m/validate schema 43))))
          (testing "properties"
            (is (= {:error/message "should be over 42"
                    :decode/string mt/-string->long
                    :json-schema/type "integer"
                    :json-schema/format "int64"
                    :json-schema/minimum 42}
                   (m/type-properties schema)))
            (is (= {:value 42}
                   (m/properties schema))))))))
bgalartza commented 1 year ago

PR has been accepted, so I guess we can close the issue.