Allow additional entries in :map schemas

[ikitommi]

trafficstars

Prismatic Schema allows one to mix defined keys and extra keys in a same map. We could get rid of :map-of in favour of supporting predicate keys in :map:

(m/schema
  [:map
   [:id int?]
   [:name string?]
   [int? int?]])

[ikitommi]

related to https://github.com/metosin/malli/issues/31#issuecomment-557893019

[rschmukler]

My leaning on this would be to keep them separate. A few questions that come to mind as "undefined" behaviors if we do merge them:

1. What happens with something like the following? Is it an :or?

[:map 
  [int? int?]
  [string? string?]]

2. What about an open map schema that specifies key predicates? Is it valid if it has non-conforming key-value pairs, so long as both the key and value don't conform?

3. What happens if you call merge on two map-of-like schemas?

To me, although the underlying data structure is similar, the behavior seems quite different - similar to :tuple and :vector.

Also, a user who wants to add arbitrary predicates to a map schema could do so by writing a :fn check. It seems infrequent enough of a need (a map schema, which also conforms to a map-of schema for its "open" values) that I'm not sure if it's worth introducing potentially confusing behavior.

[ikitommi]

1. Like JSON & Plumatic Schema, only one extra entry would be allowed per map
2. Good point, these should be either combined into single key or add rules how to handle cases where both exist. related comments https://github.com/metosin/malli/issues/31#issuecomment-557892061 & https://github.com/metosin/malli/issues/31#issuecomment-557904828. Non trivial anyway :(
3. Another good point. current merge (or union) doesn't handle these. Maybe:

(mu/union [:map-of int? string?] [:map-of string? int?])
; => [:map-of [:or int? string?] [:or string? int?]]

Having a formal syntax for this would allow malli to be more JSON Schema compliant, generating additionalProperties from :fn might not be fun.

[ikitommi]

Collecting alternatives from various sources:

1) don't support

• use :fn to handle extra keys

2) special entry

[:map
  [:x int?]
  [:malli.core/extra [string? string?]]]

3) special property

[:map {:extra [string? string?]}
  [:x int?]]

[ikitommi]

More bad ideas, bloat :closed to support extras:

[:map {:closed false}
  [:x int?]]

[:map {:closed true}
  [:x int?]]

[:map {:closed [string? string?]}
  [:x int?]]

[rschmukler]

Personally lean towards it being property-based. Overloading :closed is an interesting idea.

[esuomi]

I think special property (option 3) would be the one I'd be most comfortable with. The key name could probably be more descriptive, such as :entries/default, :entries/rest or something like that to make the userland code more discussable. Bloating :closed isn't a terrible idea either in principle as that would reduce the total amount of configurable properties, but it doesn't make semantically sense when read out loud; "the map schema is closed with string keys and values".

[ikitommi]

In JSON Schema, it's packed in one (ternary?) key: https://json-schema.org/understanding-json-schema/reference/object.html#properties

[esuomi]

Returning to this as we discussed over the last weekend on Clojurians about supporting LensSchema for :map-of.

From consistency point of view supporting malli.util operations, specifically mu/get-in but also mu/assoc-in and other functions in that namespace implementing the LensSchema for :map-of is quite easy for the -get function:

(-get [_ key default] (if (key-valid? key) value-schema default))

However the -set function gets really confusing for the type-to-type scenario because the expected behavior is really hard to reason - should it throw an exception, always return nill, change the key validator, produce a composite schema of original and a special case for the given key... While certainly possible, I fear the documentation of the behavior would actually be more of a headache than what would be gained, as even though I sort of need that feature now-ish**¤**, even I'm not sure which of those options I just listed I'd actually want it to be.

So to summarize, I'd still go with the third option and additionally either make [:map-of string? int?] a shorthand for [:map {:closed [string? int?]}] or remove :map-of entirely :)

---

¤: By that I mean my side project would love to have the LensSchema support for :map-of, but being a side project there's no actual pressure to get it working any time soon :)

[ikitommi]

:multi just got ::m/default branch. Would be coherent way to do this too:

(def valid?
  (m/validator
    [:multi {:dispatch :type}
     ["object" [:map-of :keyword :string]]
     [::m/default :string]]))

(valid? {:type "object", :key "1", :value "100"})
; => true

(valid? "SUCCESS!")
; => true

(valid? :failure)
; => false

[ikitommi]

Need to implement this to ship #606. Will most likely default to what is described in this comment: https://github.com/metosin/malli/issues/43#issuecomment-797885232

[Deraen]

I think either special entry or property is the best solution. As :multi is using a special entry, should be good here also.

[ikitommi]

I agree. #606 solved this locally with: https://github.com/metosin/malli/blob/master/src/malli/destructure.cljc#L19-L27, but not a generic solution.

[ikitommi]

from Slack:

[:and
 [:map-of ::at-context :any]
 [:map ["@context" {:optional true} ::context]]]

could be presented as:

1) ::m/default with a fallback map-schema

• ::m/default would validate just the extra keys as a map

[:map
 ["@context" {:optional true} ::context]
 [::m/default [:map-of ::at-contect :any]]]

2) ::m/default with a fallback tuple-schema

• ::m/default would validate all extra key-value pairs

[:map
 ["@context" {:optional true} ::context]
 [::m/default [:tuple ::at-contect :any]]]

[cap10morgan]

from Slack:

[:and
 [:map-of ::at-context :any]
 [:map ["@context" {:optional true} ::context]]]

My full use case would be more like:

[:and
 [:map-of ::at-context :any] ; needed for "@context" decode fn
 [:map 
  ["@context" {:optional true} ::context]
  [::m/default [:map-of ::iri ::txn-val]]]]

So if this proposal would support that, I'd be happy to take a crack at implementing it!

[cap10morgan]

Started looking into the implementation of this, and I have a few thoughts:

• We should overload the :closed property with this unless there is a meaningful behavior difference w/ this and :closed true vs. :closed false. Seems like this implies :closed true and doesn't really make sense w/ :closed false? To me the simplest form of this feature makes sense there because you're saying "I'm defining a schema for every key-value pair in the map; a.k.a. it's closed."
• Alternatively if we don't want to overload :closed, we should either define what this means for an open map or document that having this implies :closed true and throw an error if you try to explicitly set :closed false when using this feature. But I don't like that option very much. :)
  • One option for what this could mean under an open map is that any key-values that conform to both schemas (only) will undergo validation, transformation, coercion, etc. but any that don't will just be left alone. I could imagine use cases for that.
• Wherever it's implemented, I think we should just accept the key-schema and value-schema rather than wrapping them in an explicit :map-of unless there are meaningful other schema types that could go there. Having an explicit :map-of implies that other kinds of schemas could make sense there, but I'm not sure that's the case. If I'm wrong about that, I'd be curious what other schema types could make sense there. I see the example of :map-of vs. :tuple in @ikitommi's comment above, but I don't really understand what difference it would make in this context.
  • Related: Would we want to allow something like [::m/default [:or [:map-of ::foo ::bar] [:map-of ::baz ::qux]]]? I like how it keeps the key-value schemas associated w/ each other vs. something like [:map-of [:or ::foo ::baz] [:or ::bar ::qux]] where you could have a k-v that conforms to ::baz ::bar that you didn't intend to be valid.

[ikitommi]

@cap10morgan, I also did a quick check on the impl today, have validation, explain and transformation almost done, will push A draft PR in few minutes. I think "have any schema in ::m/default" is the way to go as we get explain & LensSchema paths correct for free.

(def schema
  [:map
   [:x :boolean]
   [:y :int]
   [::m/default [:map-of :int :int]]])

(m/explain schema {:y "invalid", "123" "123"})
;{:schema [:map [:x :boolean] [:y :int] [:malli.core/default [:map-of :int :int]]],
; :value {:y "invalid", "123" "123"},
; :errors ({:path [:x],
;           :in [:x],
;           :schema [:map [:x :boolean] [:y :int] [:malli.core/default [:map-of :int :int]]],
;           :value nil,
;           :type :malli.core/missing-key}
;          {:path [:y], :in [:y], :schema :int, :value "invalid"}
;          {:path [:malli.core/default 0], :in ["123"], :schema :int, :value "123"}
;          {:path [:malli.core/default 1], :in ["123"], :schema :int, :value "123"})}

(mu/get-in schema [::m/default 1])
; => :int

One could use :fn schema there too for arbitrary constrains on the map.

In transformation, we need to transform the defined keys and the ::m/default keys separately and merge the results:

(m/decode
 [:map
  [:id :int]
  [:name :keyword]
  ["age" :int]
  [::m/default [:map-of [:string {:decode/test str/upper-case}] :string]]]
 {:id 12, :name :kikka, "age" "13", "abba" "jabba"}
 (mt/transformer
  (mt/string-transformer)
  (mt/transformer {:name :test})))
;=> {:id 12, :name :kikka, "age" 13, "ABBA" "jabba"}

[cap10morgan]

@ikitommi I'll leave it in your capable hands then! Thanks for working on this!

[ikitommi]

Was about to "claim" this on slack, but didn't think you would have time for this immediately, thanks for thinking about this! Please check the PR, if the implementation strategy sucks, let's change it. Just learning (and have a strong hunch this is the way).

[ikitommi]

60% done in #871

[ikitommi]

closed via #871