Adding Request struct and process_response/1

[ryanwinchester]

First of all, thank you so much for this project, it has been a pleasure using it.

I know this might be a stretch, but I needed to do a bit of a refactor to make it better-suited for some libraries I want to start soon.

I'm hoping we can get some of this in here instead of me making a fork. 😇😅 Let me know if this is a direction you're willing to go and if there are any changes you'd like to see to actually get this merged (since I know I took quite a few liberties, to my own personal tastes 🤣)

[edgurgel]

OH hey! I like the idea! I would probably just try to avoid changing so many things at the same as this will break pretty much every code that is using HTTPoison 😶

Do you think we can simplify this PR so it just adds the Request struct as an option to do the request? Basically adding request/1, get/1 etc? So no changes to the callbacks?

What do you reckon?

[ryanwinchester]

@edgurgel maybe we could make it for a v2.0? 😇

[steffkes]

that's a little bit along the lines of what i've mentioned in #345 (comment), isn't it? (just trying to understand the motivation of the change here)

though i agree with @edgurgel that it might be rather heavy for a single PR - no matter if it's a major version jump or just a minor.

[ryanwinchester]

Yeah I'll take some time and trim it back a bit

[ryanwinchester]

@edgurgel I trimmed it back to mostly just the request struct as well as adding process_response/1 (which should help with #345 as well).

I kept the :request_url key in Response for backwards compatibility, even though it's redundant now.

New Request struct

%HTTPoison.Request{
  method: :get | :post | :put | :patch | :delete | :options | :head,
  url: binary,
  headers: [{atom, binary}] | [{binary, binary}] | %{binary => binary},
  body: binary | {:form, [{atom, any}]} | {:file, binary},
  params: map | keyword | [{binary, binary}],
  options: keyword
}

Updated Response struct:

%HTTPoison.Response{
  status_code: integer,
  body: term,
  headers: list,
  request: Request.t(),
  request_url: binary
}

Possible breaking changes:

1. This should be backwards compatible other than adding a :request key to the response struct but I can't think of a reasonable scenario in which this would be an issue, other than maybe comparisons in tests, if for some reason somebody is comparing whole response structs.

2. Also, I changed the arguments on HTTPoison.Base.request, in order to accept the request struct, which is a public function, but no tests were covering it, so I'm assuming it's okay?

[edgurgel]

@ryanwinchester that's fantastic! I'm pretty sure we can get this released soon. I will review it asap 👍

[ryanwinchester]

I've got a bit of stuff to work out on this, yet, actually.

[ryanwinchester]

Okay, all good 🤞

😇

[ryanwinchester]

I added some callbacks named a little more consistently with eachother, but kept the ones they are replacing.

Implementation:

@deprecated "Use process_request_url/1 instead"
@spec process_url(url) :: url
def process_url(url) do
  HTTPoison.Base.default_process_request_url(url)
end

@spec process_request_url(url) :: url
def process_request_url(url), do: process_url(url)

@deprecated "Use process_response_headers/1 instead"
@spec process_headers(list) :: any
def process_headers(headers), do: headers

@spec process_response_headers(list) :: any
def process_response_headers(headers), do: process_headers(headers)

@deprecated "Use process_response_status_code/1 instead"
@spec process_status_code(integer) :: any
def process_status_code(status_code), do: status_code

@spec process_response_status_code(integer) :: any
def process_response_status_code(status_code), do: process_status_code(status_code)

All the non-deprecated and new process_* callbacks:

process_request_url
process_request_body
process_request_headers
process_request_options
process_request_params
process_response
process_response_headers
process_response_status_code
process_response_body
process_response_chunk

I find this a lot more explicit and less ambiguous about what is being processed.
(and follow better the Principle of least astonishment/surprise)

I don't see it being backwards incompatible yet, since the newly named ones call the old versions unless overridden. Let me know if you want them axed anyway.

[edgurgel]

Thanks for the PR! I really appreciate the time taken to deprecate functions and document everything. I'm hoping to have a new release soon. <3 🎉