are there any known issues with using spec coercion with a spec that's comprised of s/merge, s/keys and s/map-of? Having an issue where the spec functions as I'd expect at the repl, but not via compojure-api. In the repl, I simulate the data that's being sent via form, and get :clojure.spec.alpha/invalid
(expected), but via the app I'm getting no errors, and the key/val pairs that would cause the error appear to be stripped from the data received by the handler :thinking_face:
@mgrbyte I think c-api strips non-defined keys by default. Is that good or bad?
bad in my case, as I don't know how to do what I need atm 😆
the code is here btw: https://github.com/metosin/compojure-api/blob/master/src/compojure/api/coercion/spec.clj#L16-L35
I have a fairly complex spec, can paste/share the code somewhere.
Essentially: a map of attributes A
. 2 x specs X
and Y
that define combinations of A
that are permitted by a request. I'm trying to make the app return a 4xx response if neither variant of A
is satisfied.
thanks @ikitommi! 🙂 I'll see if I can figure out how to override those transformers at the app level and see if that works... :thinking_face:
have you tried st/merge
?
It's like`s/merge` but works.
no... using vanila s/merge atm
you should try that too,
I've just tried st/merge instead of s/merge, no noticable difference there - pretty sure it's the stripping of data that's the issue
ok. If you can isolate a sample repo/code, I can try to figure out too.
thanks for looking, really appreciate your input and help :thumbsup:
just been reading spec-tools sources, looks like this has already been considered, as there's fail-on-extra-keys-type-decoders
in addition to strip-extra-keys-decoders
.
Although there's a TODO that makes e wary of directly using that: https://github.com/metosin/spec-tools/blob/master/src/spec_tools/transform.cljc#L113
Out of curiosity, what's the rationale why strip variant was chosen as default for c-api?
I think it's great to be able to take request data, convert to EDN/clojure and run it through the spec at the repl; I've been assuming that giving a parameter spec as the metadata for :body-params
or :parameters
would codify the validity of the request/response, the stripping of extra-keys seems to break that invariant?
thanks!
erk, my bad (not= fail-on-extra-keys anything-to-do-with-stripping)
fwiw, I've "fixed" this by: https://github.com/mgrbyte/capi-spec-transforms/commit/47736063b20a1c373140cbaea98acdc6d59a1e54 Hopefully that is "idiomatic" (for some value of idiomatic! :thinking_face:)
@mgrbyte The fix looks good.
about the default of dropping keys… there was a discussion about the default long time ago. Can’t recall why that was chosen. I think at least the option to pass-through all values easy to do. Would you like to do a PR where the spec-coercion could be easily modified to do any of the: strip, fail or nothing?
still alpha, so we can re-visit the decision too.
I will look into doing a PR, possibly this evening. think if it's easy to change, that'd be good. I think the default should be "less surprise" - guessing for folks who are using spec that'd mean not dropping keys, but 🤷
thanks for the links there.
So it looks like my custom coercion wasn't good enough/the complete story
Now values are not being transformed 😞
keywords within map spec specifically
I'm guessing I have to (re)implement the entire Coercion
protocol?
keyword leaf values?
yeah
the leaf specs need to be "decodable", which means Spec Records fow now. there are versions in soec-tools.spec
for most/all predicates
this works when I use :coercion :spec
in api
e.g. spec-tools.spec/keyword?
Oh, hmm.
I can switch between the new custom coercion and the default (`:spec`) and observe the transform works in the later, but not the former
restarting the app in between of course
thats weird.
Could you pr the curent status into the repo?
the example repo or my "real" app?
will try and dup in the example repo, less to look at
just pushed https://github.com/mgrbyte/capi-spec-transforms/commit/38739ade1ddec611ef44c1c2ac654787e87e142e
have to get a 🚌 home now, but will be looking again later tonight to see what I can do. thanks again for taking interest! 😍
thanks @ikitommi - i merged and tested, still doesn't work for me though thinking_face I'm an idiot, made a typo that stopped it working
fwiw, I've just pushed the final working version to master: https://github.com/mgrbyte/capi-spec-transforms/commit/432ed6b3011d2eda2e193a550e6e8062fe932a80
There was one additional issue, in that the (repeat json-transformer)
code I had in options passed to create-coercion
wasn't working, but fixed that easily enough.
@mgrbyte run the code:
default-transformer-stripping-data-issue.handler=> (-> ::variant-a s/spec :keys)
#{:g/cn :p/how :g/bt :g/sp :p/who :p/when :p/why :g/sn}
default-transformer-stripping-data-issue.handler=> (-> ::variant-b s/spec :keys)
#{:g/cn :p/how :g/sp :p/who :p/when :p/why}
default-transformer-stripping-data-issue.handler=>
… with extra-keys strpping:
default-transformer-stripping-data-issue.handler=> (st/conform ::update {:g/sp "s1", :g/cn "cn1", :g/bt "XXXX"} st/strip-extra-keys-transformer)
[:default-transformer-stripping-data-issue.handler/variant-a #:g{:cn "cn1", :sp "s1"}]
so you are right: c-api first strips out keys that are not defined and then checks the keys => the invalid keys are removed and it succeeds.
Normally, this would be good, here - I guess not.
hey @ikitommi I’ve updated my swagger scheme in my routing file and for some reason it’s not rendering the swagger.json, even though one has been returned
the swagger.json gets returned by the API
but there’s an error
and I’ve included the screen where stuff isn’t rendering