Skip to content

Commit

Permalink
Add the location to all responses returned
Browse files Browse the repository at this point in the history
  • Loading branch information
QuinnWilton committed Jan 12, 2016
1 parent 87aabe1 commit 9b675cf
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 22 deletions.
12 changes: 9 additions & 3 deletions lib/httpoison.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
defmodule HTTPoison.Response do
defstruct status_code: nil, body: nil, headers: []
@type t :: %__MODULE__{status_code: integer, body: binary, headers: list}
defstruct status_code: nil, body: nil, headers: [], location: nil
@type t :: %__MODULE__{status_code: integer, body: binary, headers: list, location: binary | nil}

@doc """
Returns the location that the response ended up at. This method should be favoured over
returning the location from the struct directly, to protect against future API changes.
"""
@spec location(Response.t) :: binary | nil
def location(response), do: response.location
end

defmodule HTTPoison.AsyncResponse do
Expand Down Expand Up @@ -65,4 +72,3 @@ defmodule HTTPoison do

use HTTPoison.Base
end

14 changes: 8 additions & 6 deletions lib/httpoison/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -394,24 +394,26 @@ defmodule HTTPoison.Base do

case :hackney.request(method, request_url, request_headers,
request_body, hn_options) do
{:ok, status_code, headers, _client} when status_code in [204, 304] ->
response(process_status_code, process_headers, process_response_body, status_code, headers, "")
{:ok, status_code, headers} -> response(process_status_code, process_headers, process_response_body, status_code, headers, "")
{:ok, status_code, headers, client} when status_code in [204, 304] ->
response(process_status_code, process_headers, process_response_body, status_code, headers, "", :hackney.location(client))
{:ok, status_code, headers} -> response(process_status_code, process_headers, process_response_body, status_code, headers, "", nil)
{:ok, status_code, headers, client} ->
location = :hackney.location(client) # FIXME The current version of Hackney (0.8) requires that we read the location before the body.
case :hackney.body(client) do
{:ok, body} -> response(process_status_code, process_headers, process_response_body, status_code, headers, body)
{:ok, body} -> response(process_status_code, process_headers, process_response_body, status_code, headers, body, location)
{:error, reason} -> {:error, %Error{reason: reason} }
end
{:ok, id} -> { :ok, %HTTPoison.AsyncResponse{ id: id } }
{:error, reason} -> {:error, %Error{reason: reason}}
end
end

defp response(process_status_code, process_headers, process_response_body, status_code, headers, body) do
defp response(process_status_code, process_headers, process_response_body, status_code, headers, body, location) do
{:ok, %Response {
status_code: process_status_code.(status_code),
headers: process_headers.(headers),
body: process_response_body.(body)
body: process_response_body.(body),
location: location
} }
end
end
36 changes: 27 additions & 9 deletions test/httpoison_base_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", {:req_headers, []}, {:req_body, "body"}, []],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert Example.post!("localhost", "body") ==
%HTTPoison.Response{ status_code: {:code, 200},
headers: {:headers, "headers"},
body: {:resp_body, "response"} }
body: {:resp_body, "response"},
location: "location" }

assert validate :hackney
end
Expand All @@ -45,11 +47,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", {:req_headers, []}, {:req_body, "body"}, []],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert ExampleDefp.post!("localhost", "body") ==
%HTTPoison.Response{ status_code: {:code, 200},
headers: {:headers, "headers"},
body: {:resp_body, "response"} }
body: {:resp_body, "response"},
location: "location" }

assert validate :hackney
end
Expand All @@ -71,11 +75,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [connect_timeout: 12345]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], timeout: 12345) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -84,11 +90,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [recv_timeout: 12345]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], recv_timeout: 12345) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -97,11 +105,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [proxy: "proxy"]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], proxy: "proxy") ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -110,11 +120,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [proxy_auth: {"username", "password"}, proxy: "proxy"]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], [proxy: "proxy", proxy_auth: {"username", "password"}]) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -123,11 +135,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [ssl_options: [certfile: "certs/client.crt"]]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], ssl: [certfile: "certs/client.crt"]) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -136,11 +150,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [follow_redirect: true]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], follow_redirect: true) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand All @@ -149,11 +165,13 @@ defmodule HTTPoisonBaseTest do
expect(:hackney, :request, [{[:post, "http://localhost", [], "body", [max_redirect: 2]],
{:ok, 200, "headers", :client}}])
expect(:hackney, :body, 1, {:ok, "response"})
expect(:hackney, :location, 1, "location")

assert HTTPoison.post!("localhost", "body", [], max_redirect: 2) ==
%HTTPoison.Response{ status_code: 200,
headers: "headers",
body: "response" }
body: "response",
location: "location" }

assert validate :hackney
end
Expand Down
14 changes: 10 additions & 4 deletions test/httpoison_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ defmodule HTTPoisonTest do
test "get" do
assert_response HTTPoison.get("localhost:8080/deny"), fn(response) ->
assert :erlang.size(response.body) == 197
assert HTTPoison.Response.location(response) == "http://localhost:8080/deny"
end
end

Expand Down Expand Up @@ -65,11 +66,15 @@ defmodule HTTPoisonTest do
end

test "option follow redirect absolute url" do
assert_response HTTPoison.get("http://localhost:8080/redirect-to?url=http%3A%2F%2Flocalhost:8080%2Fget", [], [follow_redirect: true])
assert_response HTTPoison.get("http://localhost:8080/redirect-to?url=http%3A%2F%2Flocalhost:8080%2Fget", [], [follow_redirect: true]), fn(response) ->
assert HTTPoison.Response.location(response) == "http://localhost:8080/get"
end
end

test "option follow redirect relative url" do
assert_response HTTPoison.get("http://localhost:8080/relative-redirect/1", [], [follow_redirect: true])
assert_response HTTPoison.get("http://localhost:8080/relative-redirect/1", [], [follow_redirect: true]), fn(response) ->
assert HTTPoison.Response.location(response) == "/get"
end
end

test "basic_auth hackney option" do
Expand Down Expand Up @@ -102,7 +107,7 @@ defmodule HTTPoisonTest do
test "cached request" do
if_modified = %{"If-Modified-Since" => "Tue, 11 Dec 2012 10:10:24 GMT"}
response = HTTPoison.get!("localhost:8080/cache", if_modified)
assert %HTTPoison.Response{status_code: 304, body: ""} = response
assert %HTTPoison.Response{status_code: 304, body: "", location: "http://localhost:8080/cache"} = response
end

test "send cookies" do
Expand Down Expand Up @@ -140,7 +145,8 @@ defmodule HTTPoisonTest do
assert response.status_code == 200
assert is_binary(response.body)

unless function == nil, do: function.(response)
unless HTTPoison.Response.location(response) == nil, do: assert is_binary(HTTPoison.Response.location(response))
unless function == nil, do: function.(response)
end

defp get_header(headers, key) do
Expand Down

0 comments on commit 9b675cf

Please sign in to comment.