{"action":"created","issue":{"url":"https://api.github.com/repos/hanami/controller/issues/183","repository_url":"https://api.github.com/repos/hanami/controller","labels_url":"https://api.github.com/repos/hanami/controller/issues/183/labels{/name}","comments_url":"https://api.github.com/repos/hanami/controller/issues/183/comments","events_url":"https://api.github.com/repos/hanami/controller/issues/183/events","html_url":"https://github.com/hanami/controller/issues/183","id":190752655,"number":183,"title":"Consider moving parameters merging from router to controller.","user":{"login":"beauby","id":2215778,"avatar_url":"https://avatars.githubusercontent.com/u/2215778?v=3","gravatar_id":"","url":"https://api.github.com/users/beauby","html_url":"https://github.com/beauby","followers_url":"https://api.github.com/users/beauby/followers","following_url":"https://api.github.com/users/beauby/following{/other_user}","gists_url":"https://api.github.com/users/beauby/gists{/gist_id}","starred_url":"https://api.github.com/users/beauby/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/beauby/subscriptions","organizations_url":"https://api.github.com/users/beauby/orgs","repos_url":"https://api.github.com/users/beauby/repos","events_url":"https://api.github.com/users/beauby/events{/privacy}","received_events_url":"https://api.github.com/users/beauby/received_events","type":"User","site_admin":false},"labels":[{"id":109734474,"url":"https://api.github.com/repos/hanami/controller/labels/discussion","name":"discussion","color":"207de5","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-11-21T15:40:31Z","updated_at":"2016-11-22T12:43:25Z","closed_at":null,"body":"# Current behavior\r\n\r\nCurrently, the router takes care of parsing the request body, symbolizing its keys, and merging it into `env[routers.params]`. Then, the controller action uses it to build its `params` hash that is passed to `Action#call` (it actually merges it into `Rack::Request.new(env).params`, which is the union of GET and POST params (though the latter would be empty when receiving JSON)).\r\n\r\n# Questions\r\n1. Considering that either the POST data is not JSON, in which case `Rack::Request.new(env).params` will contain the POST data, but `env['router.params']` as well, or the POST data is JSON, in which case `Rack::Request.new(env).params` will just be the GET parameters, why not simply merge `env['router.params']` into `Rack::Request.new(env).GET`?\r\n\r\n2. Why merge the router params, the query params, and the post params early (in the router)? Merging it later would allow people to use middleware to modify the POST/GET data independently without causing weird behavior with `Action#params`.\r\n\r\n# Suggestion\r\n\r\nKeep the router params in `env['router.path_params']`, keep the body params in `env['router.parsed_body']`, and keep the GET params in `env['router.query_params']`, and do not symbolize any before hitting the controller (basically do the symbolizing + merging in `BaseParams#initialize`)."},"comment":{"url":"https://api.github.com/repos/hanami/controller/issues/comments/262231243","html_url":"https://github.com/hanami/controller/issues/183#issuecomment-262231243","issue_url":"https://api.github.com/repos/hanami/controller/issues/183","id":262231243,"user":{"login":"beauby","id":2215778,"avatar_url":"https://avatars.githubusercontent.com/u/2215778?v=3","gravatar_id":"","url":"https://api.github.com/users/beauby","html_url":"https://github.com/beauby","followers_url":"https://api.github.com/users/beauby/followers","following_url":"https://api.github.com/users/beauby/following{/other_user}","gists_url":"https://api.github.com/users/beauby/gists{/gist_id}","starred_url":"https://api.github.com/users/beauby/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/beauby/subscriptions","organizations_url":"https://api.github.com/users/beauby/orgs","repos_url":"https://api.github.com/users/beauby/repos","events_url":"https://api.github.com/users/beauby/events{/privacy}","received_events_url":"https://api.github.com/users/beauby/received_events","type":"User","site_admin":false},"created_at":"2016-11-22T12:43:25Z","updated_at":"2016-11-22T12:43:25Z","body":"> Having to depend on hanami-controller or writing their own pre-processing code in non-hanami apps seems a bit inconvenient.\r\n\r\nCurrently they would have to write the same current preprocessing code that is in [Hanami::Action::BaseParams](https://github.com/hanami/controller/blob/master/lib/hanami/action/base_params.rb#L134), that is\r\n```ruby\r\nresult = {}\r\nresult.merge! ::Rack::Request.new(env).params  # merge the GET and POST params\r\nresult.merge! _router_params  # merge the path params (controller and action) AND the POST params\r\n```\r\nWhat I'm suggesting would change what they'd have to write to\r\n```ruby\r\nresult = {}\r\nresult.merge! ::Rack::Request.new(env).GET  # merge the GET ~~and POST params~~\r\nresult.merge! _body_params  # merge the body params\r\nresult.merge! _router_params  # merge the path params (controller and action) ~~AND the POST params~~\r\n```\r\n\r\nBasically, I think it should be up to the controller layer how to expose each kind of parameter.\r\n\r\nThe exact situation I am facing is that I am deserializing a [JSON API](http://jsonapi.org) payload in the request body, so I would like to **replace the body** with the deserialized hash, and have it available to the controller action as normal input, **without remnants from the non-deserialized payload**. Currently I'm achieving that by:\r\n\r\n+ deserializing data from `env['router.parsed_body']`\r\n+ removing all `env['router.parsed_body'].keys` from `env['router.params']`\r\n+ adding my deserialized hash to `env['router.params']`.\r\n\r\nThe issue being that, since the merging of body params into router params has already happened when the deserialization middleware kicks in, modifying `env['router.parsed_body']` will have no effect on the params of the action."}}
{"actor":{"display_login":"beauby"}}