"Confinement" de la librairie Timex (#4385)

* Extract iso-extended parsing to a central place

* Add TODO

* Add wrapper for "{YYYY}{0M}{0D}"

* Add TODO

* Move test calls

* Wrap now() and shift()

* Wrap Timex conversion calls

* Add doctest wrapper

* Add note

* Fix credo warnings

* Add doc

* Update time_wrapper.ex

* Forbid direct use of Timex

Author: thbar

.credo.exs

@@ -129,10 +129,15 @@
 
         # Deprecated checks (these will be deleted after a grace period)
         #
-        {Credo.Check.Readability.Specs, false}
+        {Credo.Check.Readability.Specs, false},
 
         # Custom checks can be created using `mix credo.gen.check`.
         #
+
+        {Credo.Check.Warning.ForbiddenModule,
+         [
+           modules: [Timex]
+         ]}
       ]
     }
   ]

apps/shared/lib/date_time_display.ex

@@ -34,7 +34,7 @@ defmodule Shared.DateTimeDisplay do
   def format_date(nil, _), do: ""
 
   def format_date(date, locale, iso_extended: true) do
-    date |> Timex.parse!("{ISO:Extended}") |> format_date(locale)
+    date |> TimeWrapper.parse!("{ISO:Extended}") |> format_date(locale)
   end
 
   @doc """
@@ -100,7 +100,7 @@ defmodule Shared.DateTimeDisplay do
 
   def format_datetime_to_paris(datetime, locale, options) when is_binary(datetime) do
     datetime
-    |> Timex.parse!("{ISO:Extended}")
+    |> TimeWrapper.parse!("{ISO:Extended}")
     |> format_datetime_to_paris(locale, options)
   end
 
@@ -157,7 +157,7 @@ defmodule Shared.DateTimeDisplay do
 
   def format_time_to_paris(datetime, locale, options) when is_binary(datetime) do
     datetime
-    |> Timex.parse!("{ISO:Extended}")
+    |> TimeWrapper.parse!("{ISO:Extended}")
     |> format_time_to_paris(locale, options)
   end
 
@@ -225,14 +225,11 @@ defmodule Shared.DateTimeDisplay do
 
   @spec convert_to_paris_time(DateTime.t() | NaiveDateTime.t()) :: DateTime.t()
   def convert_to_paris_time(%DateTime{} = dt) do
-    case Timex.Timezone.convert(dt, "Europe/Paris") do
-      %Timex.AmbiguousDateTime{after: dt} -> dt
-      %DateTime{} = dt -> dt
-    end
+    TimeWrapper.convert_to_paris_time(dt)
   end
 
   def convert_to_paris_time(%NaiveDateTime{} = ndt) do
-    ndt |> Timex.Timezone.convert("UTC") |> convert_to_paris_time()
+    ndt |> TimeWrapper.convert("UTC") |> convert_to_paris_time()
   end
 
   defp get_localized_datetime_format("en" = locale, options) do

apps/shared/lib/time_wrapper.ex

@@ -0,0 +1,48 @@
+defmodule TimeWrapper do
+  @moduledoc """
+  This module concentrates all the calls to `Timex` in a single place.
+
+  The idea behind this module is 1. to reduce our dependency on `Timex`, and
+  2. to ideally gradually replace calls by built-in Elixir `DateTime` calls, since
+  `Timex` filled a void in the language that has been partially filled now.
+  """
+
+  # credo:disable-for-this-file Credo.Check.Warning.ForbiddenModule
+
+  def parse!(date_as_string, "{ISO:Extended}" = param) do
+    Timex.parse!(date_as_string, param)
+  end
+
+  def parse!(date_as_string, "{YYYY}{0M}{0D}" = param) do
+    Timex.parse!(date_as_string, param)
+  end
+
+  # NOTE: try not to use this, we will remove it. This is rfc2822 ;
+  # Plug encodes it, but there is no built-in decoder.
+  def parse!(datetime_as_string, "{WDshort}, {D} {Mshort} {YYYY} {h24}:{m}:{s} GMT" = param) do
+    Timex.parse!(datetime_as_string, param)
+  end
+
+  def diff(first, second, :hours = param) do
+    Timex.diff(first, second, param)
+  end
+
+  def now do
+    Timex.now()
+  end
+
+  def shift(dt, months: months) do
+    Timex.shift(dt, months: months)
+  end
+
+  def convert(dt, "UTC") do
+    Timex.Timezone.convert(dt, "UTC")
+  end
+
+  def convert_to_paris_time(dt) do
+    case Timex.Timezone.convert(dt, "Europe/Paris") do
+      %Timex.AmbiguousDateTime{after: dt} -> dt
+      %DateTime{} = dt -> dt
+    end
+  end
+end

apps/shared/test/time_wrapper_test.exs

@@ -0,0 +1,4 @@
+defmodule TimeWrapperTest do
+  use ExUnit.Case, async: true
+  doctest TimeWrapper
+end

apps/transport/lib/jobs/gtfs_to_db.ex

@@ -120,8 +120,8 @@ defmodule Transport.Jobs.GtfsToDB do
           friday: friday = r |> Map.fetch!("friday") |> String.to_integer(),
           saturday: saturday = r |> Map.fetch!("saturday") |> String.to_integer(),
           sunday: sunday = r |> Map.fetch!("sunday") |> String.to_integer(),
-          start_date: r |> Map.fetch!("start_date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
-          end_date: r |> Map.fetch!("end_date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date()
+          start_date: r |> Map.fetch!("start_date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
+          end_date: r |> Map.fetch!("end_date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date()
         }
 
         res
@@ -214,7 +214,7 @@ defmodule Transport.Jobs.GtfsToDB do
           %{
             data_import_id: data_import_id,
             service_id: r |> Map.fetch!("service_id"),
-            date: r |> Map.fetch!("date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
+            date: r |> Map.fetch!("date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
             exception_type: r |> Map.fetch!("exception_type") |> String.to_integer()
           }
         end)

apps/transport/lib/transport_web/live/discussions_live.ex

@@ -115,7 +115,7 @@ defmodule TransportWeb.DiscussionsLive do
       end)
       |> Enum.max(DateTime)
 
-    two_months_ago = DateTime.utc_now() |> Timex.shift(months: -2)
+    two_months_ago = DateTime.utc_now() |> TimeWrapper.shift(months: -2)
     DateTime.compare(two_months_ago, latest_comment_datetime) == :gt
   end
 

apps/transport/test/transport_web/live_views/discussions_live_test.exs

@@ -209,7 +209,7 @@ defmodule Transport.TransportWeb.DiscussionsLiveTest do
   end
 
   defp iso8601_string_x_months_ago(x) do
-    DateTime.utc_now() |> Timex.shift(months: -x) |> DateTime.to_iso8601()
+    DateTime.utc_now() |> TimeWrapper.shift(months: -x) |> DateTime.to_iso8601()
   end
 
   defp discussions do

apps/transport/test/transport_web/routing/headers_and_cookies_test.exs

@@ -24,7 +24,7 @@ defmodule TransportWeb.HeadersAndCookiesTest do
       "expires" => datetime
     } = Plug.Conn.Cookies.decode(header)
 
-    datetime = Timex.parse!(datetime, "{WDshort}, {D} {Mshort} {YYYY} {h24}:{m}:{s} GMT")
-    assert_in_delta Timex.diff(datetime, Timex.now(), :hours), 15 * 24, 1
+    datetime = TimeWrapper.parse!(datetime, "{WDshort}, {D} {Mshort} {YYYY} {h24}:{m}:{s} GMT")
+    assert_in_delta TimeWrapper.diff(datetime, TimeWrapper.now(), :hours), 15 * 24, 1
   end
 end