Refactor internals (#2)

* Refactor internals

* Substitute guard for if

* Move pipes in link_str to new lines

lib/scrivener/headers.ex

@@ -17,71 +17,56 @@ defmodule Scrivener.Headers do
       end
   """
 
-  import Plug.Conn
-
-  alias Plug.Conn.Query
-
-  @rels ~w(first last next prev)
+  import Plug.Conn, only: [put_resp_header: 3]
 
   @doc """
   Add HTTP headers for a `Scrivener.Page`.
   """
   @spec paginate(Plug.Conn.t, Scrivener.Page.t) :: Plug.Conn.t
   def paginate(conn, page) do
+    uri = %URI{scheme: Atom.to_string(conn.scheme),
+               host: conn.host,
+               port: conn.port,
+               path: conn.request_path,
+               query: conn.query_string}
     conn
-    |> put_link_header(page)
+    |> put_resp_header("link", build_link_header(uri, page))
     |> put_resp_header("total", Integer.to_string(page.total_entries))
     |> put_resp_header("per-page", Integer.to_string(page.page_size))
   end
 
-  defp put_link_header(conn, page) do
-    link = conn
-           |> pages(page)
-           |> Enum.join(", ")
-
-    put_resp_header(conn, "link", link)
+  @spec build_link_header(URI.t, Scrivener.Page.t) :: String.t
+  defp build_link_header(uri, page) do
+    [link_str(uri, 1, "first"),
+     link_str(uri, page.total_pages, "last")]
+    |> maybe_add_prev(uri, page.page_number, page.total_pages)
+    |> maybe_add_next(uri, page.page_number, page.total_pages)
+    |> Enum.join(", ")
   end
 
-  defp pages(conn, page) do
-    @rels
-    |> Enum.map(&(page_link(conn, page, &1)))
-    |> Enum.reject(&(&1 == ""))
+  defp link_str(%{query: req_query} = uri, page_number, rel) do
+    query =
+      req_query
+      |> URI.decode_query()
+      |> Map.put("page", page_number)
+      |> URI.encode_query()
+    uri_str =
+      %URI{uri | query: query}
+      |> URI.to_string()
+    ~s(<#{uri_str}>; rel="#{rel}")
   end
 
-  defp page_link(conn, %{}, "first"), do: page_link(conn, 1, "first")
-  defp page_link(conn, %{total_pages: total}, "last"), do: page_link(conn, total, "last")
-
-  defp page_link(_conn, %{page_number: page, total_pages: total}, "next") when page == total, do: ""
-  defp page_link(conn, %{page_number: page}, "next"), do: page_link(conn, page + 1, "next")
-
-  defp page_link(_conn, %{page_number: 1}, "prev"), do: ""
-  defp page_link(conn, %{page_number: page}, "prev"), do: page_link(conn, page - 1, "prev")
-
-  defp page_link(conn, page, rel) when is_integer(page) do
-    conn
-    |> paged_url(page)
-    |> page_link(rel)
+  defp maybe_add_prev(links, uri, page_number, total_pages) when 1 < page_number and page_number <= total_pages do
+    [link_str(uri, page_number - 1, "prev") | links]
   end
-
-  defp page_link(url, rel), do: "<#{url}>; rel=\"#{rel}\""
-
-  defp paged_url(conn, page) do
-    conn.query_string
-    |> Query.decode
-    |> Map.put("page", page)
-    |> Query.encode
-    |> url(conn)
+  defp maybe_add_prev(links, _uri, _page_number, _total_pages) do
+    links
   end
 
-  defp url(query, conn) do
-    scheme = Atom.to_string(conn.scheme)
-    scheme = "#{scheme}://"
-
-    unless "" == query, do: query = "?#{query}"
-
-    port = if conn.port in [80, 443], do: "", else: ":#{conn.port}"
-
-    [scheme, conn.host, port, conn.request_path, query]
-    |> Enum.join("")
+  defp maybe_add_next(links, uri, page_number, total_pages) when 1 <= page_number and page_number < total_pages do
+    [link_str(uri, page_number + 1, "next") | links]
+  end
+  defp maybe_add_next(links, _uri, _page_number, _total_pages) do
+    links
   end
 end

test/scrivener/headers_test.exs

@@ -22,7 +22,11 @@ defmodule Scrivener.HeadersTests do
 
     assert headers["total"] == "50"
     assert headers["per-page"] == "10"
-    assert headers["link"] == ~s(<http://www.example.com/test?foo=bar&page=1>; rel="first", <http://www.example.com/test?foo=bar&page=5>; rel="last", <http://www.example.com/test?foo=bar&page=4>; rel="next", <http://www.example.com/test?foo=bar&page=2>; rel="prev")
+    links = String.split(headers["link"], ", ")
+    assert ~s(<http://www.example.com/test?foo=bar&page=1>; rel="first") in links
+    assert ~s(<http://www.example.com/test?foo=bar&page=5>; rel="last") in links
+    assert ~s(<http://www.example.com/test?foo=bar&page=4>; rel="next") in links
+    assert ~s(<http://www.example.com/test?foo=bar&page=2>; rel="prev") in links
   end
 
   test "doesn't include prev link for first page" do