Add validation for header parameters #160
Conversation
| end | ||
|
|
||
| defp validate_header_params([{"string", _name, _val, _} | parameters]) do | ||
| validate_query_params(parameters) |
There was a problem hiding this comment.
Looks like a copy & paste bug here :)
| defp remove_base_path([_path | rest], [_base_path | base_path_rest]) do | ||
| remove_base_path(rest, base_path_rest) | ||
| end | ||
| end | ||
|
|
||
| defp validate_header_params([]), do: :ok |
There was a problem hiding this comment.
validate_header_params/1 looks very similar to validate_query_params/1.
I think they can be combined into something like validate_parameter_list.
validate_header_params/2 is fine, since it needs to pull the params from a different key in the Conn anyway.
There was a problem hiding this comment.
I'll try to fix this as soon as I can
| defp get_header_value(headers, header_name) do | ||
| header_name_down_case = String.downcase(header_name) | ||
|
|
||
| headers |
There was a problem hiding this comment.
There's a handy function in the erlang std lib to search a list of tuples:
:lists.keyfind(header_name_down_case, 1, headers) || nil
There was a problem hiding this comment.
This is my first opensource contribution and my first month using elixir
Congrats! 🎉 and thankyou ❤️ !
Are the tests ok?
👍 Tests look good to me
Besides the changelog, should I add anything to this PR?
I looks like your editor is auto-formatting the file in the Elixir 1.6 style?
Could you separate it into two commits? One to reformat the file, and another to add the header param validations.
|
@pablobcb I'm happy to help rebasing your changes. Feel free to reach out any time. |
|
@soundmonster feel free to do it, Im currently moving to another city and dont know when I ll have the time to do it |
PR for #155
Add validation for header parameters. The current implementation is full of duplicated code, but due the fact that the algorithm recursively consume the head of parameters until it produces an
:ok, I wasnt able to figure out how to generalize and keep it current structure ...If you want, I can try to wrap everything in
Enum.allor whatever you suggest..This is my first opensource contribution and my first month using elixir, so I'm sorry if I screwed anything up ..
Are the tests ok? Are they in the right place? Besides the changelog, should I add anything to this PR?