Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid overwriting request's params with silk params #26

Closed
wants to merge 2 commits into from

Conversation

volrath
Copy link

@volrath volrath commented Jun 23, 2016

Ring request's :params were overwritten by ring-handler, which assoc its params in the request.

This scenario might be a common one, specially if the user is using something like ring.middlewares.params.wrap-params, for example.

This PR makes sure silk :params are merged to any possible existing request's :params.

I wrote a test for it, let me know if it's enough or if I should test or improve anything else.

Closes #21

If a request have params of its own (given by
`ring.middlewares.params.wrap-params`, for example), they should not be
deleted or overwritten by `ring-handler`.

This commit makes sure `silk` params are merged with any possible
existing request's params.
@volrath
Copy link
Author

volrath commented Jun 23, 2016

Hmm not sure why that failed. I ran lein dev locally and all test passed. I'm fairly new to Clojure, if you can give me a hint I'll fix it.

@@ -34,7 +34,7 @@
(fn [req]
(if-let [params (silk/match rtes (request-map->URL req))]
((-> params :domkm.silk/name get-handler)
(assoc req :params params))
(merge-with merge req {:params params}))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try (update req :params merge params)

@volrath
Copy link
Author

volrath commented Jul 7, 2016

done @domkm

@volrath volrath closed this Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ring-handler clobbers :params
2 participants