From 9b675cf3f5225040639e2d5ac0ea9190a5be4a3f Mon Sep 17 00:00:00 2001 From: Shane Wilton Date: Tue, 10 Nov 2015 16:54:28 -0800 Subject: [PATCH] Add the location to all responses returned --- lib/httpoison.ex | 12 +++++++++--- lib/httpoison/base.ex | 14 ++++++++------ test/httpoison_base_test.exs | 36 +++++++++++++++++++++++++++--------- test/httpoison_test.exs | 14 ++++++++++---- 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/lib/httpoison.ex b/lib/httpoison.ex index 8b6656a..438c271 100644 --- a/lib/httpoison.ex +++ b/lib/httpoison.ex @@ -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 @@ -65,4 +72,3 @@ defmodule HTTPoison do use HTTPoison.Base end - diff --git a/lib/httpoison/base.ex b/lib/httpoison/base.ex index 5ce21b7..6fd85e7 100644 --- a/lib/httpoison/base.ex +++ b/lib/httpoison/base.ex @@ -394,12 +394,13 @@ 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 } } @@ -407,11 +408,12 @@ defmodule HTTPoison.Base do 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 diff --git a/test/httpoison_base_test.exs b/test/httpoison_base_test.exs index 150eca9..c4a45e6 100644 --- a/test/httpoison_base_test.exs +++ b/test/httpoison_base_test.exs @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/test/httpoison_test.exs b/test/httpoison_test.exs index 79f647e..7e8015a 100644 --- a/test/httpoison_test.exs +++ b/test/httpoison_test.exs @@ -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 @@ -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 @@ -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 @@ -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