reitit
reitit copied to clipboard
Add malli.util/merge call to route data merge
POC, need to figure out API where Malli coercion can extend the merge functionality, as malli.core can't depend on malli call directly, and this must be enabled only when using malli coercion anyway.
Fixes #422
- What's the extension point, Coercion protocol maybe? Or is there something else that should be able to extend this?
- Coercion is itself set in the route-data, so we would need to find out the coercion value for route before merging other values?
- Should the API allow to control any paths, or just :parameters data?
Maybe a simple solution:
router
level options to control route-data merging, provide a function in Malli namespace that can be used for this option. This would need to be set in addition to coercer.
This doesn't allow handling merges if some parts of the route tree use Spec and others Malli etc.
Maybe the Coercion(?) protocol should have a method for merging parameters?
Coercion value is itself set in the route data, it is hard to get the used coercion before we have merged the route data.
Also, what if the top-level uses Reitit coercion and some routes use Spec coercion, which coercion is used to merge the different levels of route data for those routes?
I have a version where Coercion implementations now implement RouteDataMerge protocol which is used to collect route-data parts into one map.
The problems:
- Route data merge is currently tied to :coercion route data, which will currently affect :parameters route data.
- The effective :coercion value needs to be retrieved separately before the merge
- There could be need to control other route data keys than just :parameters
I think better solution would be to just add some kind of declaration or function to router options, to control how keys are merged. This will allow controlling other keys, and doesn't tie this together with coercion values.
This is one way to resolve this, also not happy how coercion spills into core ns. Also, if you have different coercion impls at different parts of the route tree, how does the merge work? doesn't? e.g.
["/root" {:coercion spec, :parameters {:query {:x int?}}}
["/schema" {:coercion schema, :parameters {:query {:y s/Int}}}
|"/malli" {:coercion malli, :parameters {:query [:map [:z :int]]}}]]]
I guess it breaks?
Questions:
- do we need to support multiple coercion impls in one route-tree? => why not
- do we have to have coherent coercion impl in a route tree + parents => YES
One way to have a clean separation of concerns would be to define a generic parameter merge strategy per parameters path, without coercion. e.g. a reitit.ring/router
could say that :parameters
is a special path in which, the values are accumulated into a vector. This way, we have all the schemas separately and could use Coercion impl to merge those in a second sweep.
;; original
["/api" {:coercion malli, :parameters {:query [:map [:x :int]]}}
["/admin" {:parameters {:query [:map [:y :int]]}}
|"/users" {:parameters {:query [:map [:z :int]]}}]]]
; after path & route data merging
["/api/admin/users" {:coercion malli, :parameters [{:query [:map [:x :int]]} {:query [:map [:y :int]]} {:query [:map [:z :int]]}]}]
;; after coercion has merged the params
["/api/admin/users" {:coercion malli, :parameters {:query [:map [:x :int] [:y :int] [:z :int]]}]
what do you think?
... the actual impl for doing the accumulation could be done either a new Protocol or just meta-data. We might need to fork meta-merge
for this.
;; via meta-data
(meta-merge/merge {:parameters ^:accumulate []} {:parameters {:query [:map [:x :int]]}})
; => {:parameters [{:query [:map [:x :int]]}])
;; via protocol (and records)
(meta-merge/accumulator)
; => #meta.merge.Accumulator{:data []}
(meta-merge/merge {:parameters *1} {:parameters {:query [:map [:x :int]]}})
; => {:parameters #meta.merge.Accumulator{:data [{:query [:map [:x :int]]}]})
Replaced by ce06214014499caf86d3abd65884a85e7004b53d