Extract a module for HTTP Date strings

This parsing is used in two different modules now, and there was a bug in it, which tells me it should be in its own module and tested separately.

lib/liberator/evaluator.ex

@@ -162,7 +162,7 @@ defmodule Liberator.Evaluator do
             retry_after_value =
               cond do
                 Timex.is_valid?(retry_after) ->
-                  Timex.format!(retry_after, "%a, %d %b %Y %H:%M:%S GMT", :strftime)
+                  Liberator.HTTPDateTime.format!(retry_after)
                 is_integer(retry_after)->
                   Integer.to_string(retry_after)
                 String.valid?(retry_after) ->

lib/liberator/http_datetime.ex

@@ -0,0 +1,68 @@
+defmodule Liberator.HTTPDateTime do
+  @moduledoc false
+  @moduledoc since: "1.2"
+
+  @strftime_format "%a, %d %b %Y %H:%M:%S GMT"
+
+  @doc """
+  Checks to see if a string is a valid HTTP date.
+
+  ## Examples
+
+      iex> Liberator.HTTPDateTime.valid?("Wed, 21 Oct 2015 07:28:00 GMT")
+      true
+      iex> Liberator.HTTPDateTime.valid?("2015-10-21 07:28:00Z")
+      false
+  """
+  @doc since: "1.2"
+  def valid?(str) do
+    case parse(str) do
+      {:ok, _time} -> true
+      _ -> false
+    end
+  end
+
+  @doc """
+  Parses an HTTP Date string into an Elixir `DateTime` object.
+
+  ## Examples
+
+      iex> Liberator.HTTPDateTime.parse("Wed, 21 Oct 2015 07:28:00 GMT")
+      {:ok, ~U[2015-10-21 07:28:00Z]}
+      iex> Liberator.HTTPDateTime.parse("2015-10-21 07:28:00Z")
+      {:error,  "Expected `weekday abbreviation` at line 1, column 1."}
+  """
+  @doc since: "1.2"
+  def parse(str) do
+    case Timex.parse(str, @strftime_format, :strftime) do
+      {:ok, datetime} -> {:ok, DateTime.from_naive!(datetime, "Etc/UTC")}
+      err -> err
+    end
+  end
+
+  @doc """
+  Like `parse/1` except will raise an error if the string cannot be parsed.
+
+  ## Examples
+
+      iex> Liberator.HTTPDateTime.parse!("Wed, 21 Oct 2015 07:28:00 GMT")
+      ~U[2015-10-21 07:28:00Z]
+  """
+  @doc since: "1.2"
+  def parse!(str) do
+    Timex.parse!(str, @strftime_format, :strftime)
+    |> DateTime.from_naive!("Etc/UTC")
+  end
+
+  @doc """
+  Formats a datetime into an HTTP Date string, and raises if there is an error.
+
+  ## Examples
+
+      iex> Liberator.HTTPDateTime.format!(~U[2015-10-21 07:28:00Z])
+      "Wed, 21 Oct 2015 07:28:00 GMT"
+  """
+  def format!(datetime) do
+    Timex.format!(datetime, @strftime_format, :strftime)
+  end
+end

lib/liberator/resource.ex

@@ -1258,18 +1258,14 @@ defmodule Liberator.Resource do
         conn
         |> get_req_header("if-modified-since")
         |> Enum.at(0)
-        |> Timex.parse("%a, %d %b %Y %H:%M:%S GMT", :strftime)
-        |> case do
-          {:ok, _time} -> true
-          _ -> false
-        end
+        |> Liberator.HTTPDateTime.valid?()
       end
       @impl true
       def modified_since?(conn) do
         conn
         |> get_req_header("if-modified-since")
         |> Enum.at(0)
-        |> Timex.parse!("%a, %d %b %Y %H:%M:%S GMT", :strftime)
+        |> Liberator.HTTPDateTime.parse!()
         |> Timex.before?(last_modified(conn))
       end
       @impl true
@@ -1281,18 +1277,14 @@ defmodule Liberator.Resource do
         conn
         |> get_req_header("if-unmodified-since")
         |> Enum.at(0)
-        |> Timex.parse("%a, %d %b %Y %H:%M:%S GMT", :strftime)
-        |> case do
-          {:ok, _time} -> true
-          _ -> false
-        end
+        |> Liberator.HTTPDateTime.valid?()
       end
       @impl true
       def unmodified_since?(conn) do
         conn
         |> get_req_header("if-unmodified-since")
         |> Enum.at(0)
-        |> Timex.parse!("%a, %d %b %Y %H:%M:%S GMT", :strftime)
+        |> Liberator.HTTPDateTime.parse!()
         |> Timex.after?(last_modified(conn))
       end
 

test/liberator/http_datetime_test.exs

@@ -0,0 +1,20 @@
+defmodule Liberator.HTTPDateTimeTest do
+  use ExUnit.Case
+  doctest Liberator.HTTPDateTime
+
+  describe "parse!/1" do
+    test "raises on error" do
+      assert_raise Timex.Parse.ParseError, "Expected `weekday abbreviation` at line 1, column 1.", fn ->
+        Liberator.HTTPDateTime.parse!("asdf")
+      end
+    end
+  end
+
+  describe "format!/1" do
+    test "raises on error" do
+      assert_raise ArgumentError, fn ->
+        Liberator.HTTPDateTime.format!("asdf")
+      end
+    end
+  end
+end

test/liberator/resource_defaults_test.exs

@@ -1,6 +1,7 @@
 defmodule Liberator.ResourceDefaultsTest do
   use ExUnit.Case, async: true
   use Plug.Test
+  alias Liberator.HTTPDateTime
 
   defmodule MyDefaultResource do
     use Liberator.Resource
@@ -320,23 +321,23 @@ defmodule Liberator.ResourceDefaultsTest do
 
   describe "modified_since?" do
     test "returns true if last modification date is after modification_since" do
-      {:ok, time_str} =
+      time_str =
         Timex.Timezone.get("GMT")
         |> Timex.now()
         |> Timex.shift(days: -1)
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-modified-since", time_str)
 
       assert MyDefaultResource.modified_since?(conn)
     end
     test "returns false if last modification date is before modification_since" do
-      {:ok, time_str} =
+      time_str =
         Timex.Timezone.get("GMT")
         |> Timex.now()
         |> Timex.shift(days: 1)
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-modified-since", time_str)
@@ -347,9 +348,9 @@ defmodule Liberator.ResourceDefaultsTest do
 
   describe "if_modified_since_valid_date?" do
     test "returns true if if_modified_since header contains a valid date" do
-      {:ok, time_str} =
+      time_str =
         Timex.now()
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-modified-since", time_str)
@@ -380,9 +381,9 @@ defmodule Liberator.ResourceDefaultsTest do
 
   describe "if_unmodified_since_valid_date?" do
     test "returns true if if_unmodified_since header contains a valid date" do
-      {:ok, time_str} =
+      time_str =
         Timex.now()
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-unmodified-since", time_str)
@@ -400,23 +401,23 @@ defmodule Liberator.ResourceDefaultsTest do
 
   describe "unmodified_since?" do
     test "returns false if last modification date is after modification_since" do
-      {:ok, time_str} =
+      time_str =
         Timex.Timezone.get("GMT")
         |> Timex.now()
         |> Timex.shift(days: -1)
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-unmodified-since", time_str)
 
       refute MyDefaultResource.unmodified_since?(conn)
     end
     test "returns true if last modification date is before modification_since" do
-      {:ok, time_str} =
+      time_str =
         Timex.Timezone.get("GMT")
         |> Timex.now()
         |> Timex.shift(days: 1)
-        |> Timex.Format.DateTime.Formatters.Strftime.format("%a, %d %b %Y %H:%M:%S GMT")
+        |> HTTPDateTime.format!()
       conn =
         conn(:get, "/")
         |> put_req_header("if-unmodified-since", time_str)