Le site n'utilise plus les geojson des ressources communautaires (#2020)

* add geojson file size information in payload

* function to get a resource related geojson

* same thing for a dataset

* add assign for resource controller

* add assign for dataset controller

* update resource view existing functions accordingly

* plug the new function for resources

* use the new function for dataset details page

* add a test

* add condition on convert_to format

* update test

* format file

* add nil case (can happen in tests)

* don't match, assert equal

* use the updated_at field of resource_history

so we know when was the last check

* update the tests

* use that new information

* update test

* new function to convert seconds to hours and minutes

* finally add the information to the webpage

* mix format

* credo + warning

* simplify test

* correct file_size computation

* use a dedicated field for freshness information

* valide_at should be filled when creating resource !

* change field name

* correct merge issue

Co-authored-by: Thibaut Barrère <[email protected]>

Author: fchabouis

apps/db/lib/db/dataset.ex

@@ -601,6 +601,15 @@ defmodule DB.Dataset do
     |> Repo.all()
   end
 
+  @spec get_resources_related_files(any()) :: map()
+  def get_resources_related_files(%__MODULE__{resources: resources}) when is_list(resources) do
+    resources
+    |> Enum.map(fn %{id: id} = r -> {id, DB.Resource.get_related_files(r)} end)
+    |> Enum.into(%{})
+  end
+
+  def get_resources_related_files(_), do: %{}
+
   @spec validate_territory_mutual_exclusion(Ecto.Changeset.t()) :: Ecto.Changeset.t()
   defp validate_territory_mutual_exclusion(changeset) do
     has_cities =

apps/db/lib/db/resource.ex

@@ -59,7 +59,11 @@ defmodule DB.Resource do
     belongs_to(:dataset, Dataset)
     has_one(:validation, Validation, on_replace: :delete)
     has_many(:logs_validation, LogsValidation, on_replace: :delete, on_delete: :delete_all)
-    has_many(:resource_unavailabilities, ResourceUnavailability, on_replace: :delete, on_delete: :delete_all)
+
+    has_many(:resource_unavailabilities, ResourceUnavailability,
+      on_replace: :delete,
+      on_delete: :delete_all
+    )
   end
 
   defp gtfs_validator, do: Shared.Validation.GtfsValidator.Wrapper.impl()
@@ -535,7 +539,10 @@ defmodule DB.Resource do
   end
 
   @spec ttl(__MODULE__.t()) :: integer() | nil
-  def ttl(%__MODULE__{format: "gbfs", metadata: %{"ttl" => ttl}}) when is_integer(ttl) and ttl >= 0, do: ttl
+  def ttl(%__MODULE__{format: "gbfs", metadata: %{"ttl" => ttl}})
+      when is_integer(ttl) and ttl >= 0,
+      do: ttl
+
   def ttl(_), do: nil
 
   @spec can_direct_download?(__MODULE__.t()) :: boolean
@@ -582,4 +589,30 @@ defmodule DB.Resource do
       where: resource.id == ^id
     )
   end
+
+  @spec get_related_files(__MODULE__.t()) :: map()
+  def get_related_files(%__MODULE__{datagouv_id: resource_datagouv_id}) do
+    %{}
+    |> Map.put(:geojson, get_related_geojson_info(resource_datagouv_id))
+  end
+
+  @spec get_related_geojson_info(binary() | nil) :: %{url: binary(), filesize: binary()} | nil
+  def get_related_geojson_info(nil), do: nil
+
+  def get_related_geojson_info(resource_datagouv_id) do
+    DB.ResourceHistory
+    |> join(:inner, [rh], dc in DB.DataConversion,
+      as: :dc,
+      on: fragment("?::text = ? ->> 'uuid'", dc.resource_history_uuid, rh.payload)
+    )
+    |> select([rh, dc], %{
+      url: fragment("? ->> 'permanent_url'", dc.payload),
+      filesize: fragment("? ->> 'filesize'", dc.payload),
+      resource_history_last_up_to_date_at: rh.last_up_to_date_at
+    })
+    |> where([rh, dc], rh.datagouv_id == ^resource_datagouv_id and dc.convert_to == "GeoJSON")
+    |> order_by([rh, _], desc: rh.inserted_at)
+    |> limit(1)
+    |> DB.Repo.one()
+  end
 end

apps/db/lib/db/resource_history.ex

@@ -8,6 +8,8 @@ defmodule DB.ResourceHistory do
   typed_schema "resource_history" do
     field(:datagouv_id, :string)
     field(:payload, :map)
+    # the last moment we checked and the resource history was corresponding to the real online resource
+    field(:last_up_to_date_at, :utc_datetime_usec)
 
     timestamps(type: :utc_datetime_usec)
   end

apps/db/priv/repo/migrations/20220104092238_add_resource_history_last_up_to_date_at_field.exs

@@ -0,0 +1,9 @@
+defmodule DB.Repo.Migrations.AddResourceHistoryLastUpToDateAtField do
+  use Ecto.Migration
+
+  def change do
+    alter table(:resource_history) do
+      add(:last_up_to_date_at, :utc_datetime_usec)
+    end
+  end
+end

apps/db/test/resource_test.exs

@@ -132,6 +132,49 @@ defmodule DB.ResourceTest do
     assert reasons == %{"content hash has changed" => 1, "no previous validation" => 1}
   end
 
+  test "get resource related geojson infos" do
+    now = DateTime.now!("Etc/UTC")
+
+    # we insert 3 resource history for datagouv_id_1
+    insert_resouce_history("datagouv_id_1", uuid1 = Ecto.UUID.generate(), now, -3600)
+    insert_resouce_history("datagouv_id_1", uuid2 = Ecto.UUID.generate(), now)
+    insert_resouce_history("datagouv_id_1", uuid3 = Ecto.UUID.generate(), now, -3601)
+
+    # and one for datagouv_id_2
+    insert_resouce_history("datagouv_id_2", uuid4 = Ecto.UUID.generate(), now)
+
+    # we insert 1 conversion for each resource history
+    insert_data_conversion(uuid1, "url1", 10)
+    insert_data_conversion(uuid2, "url2", 12)
+    insert_data_conversion(uuid3, "url3", 10)
+    insert_data_conversion(uuid4, "url4", 10)
+
+    assert %{url: "url2", filesize: "12", resource_history_last_up_to_date_at: _} =
+             DB.Resource.get_related_geojson_info("datagouv_id_1")
+
+    assert nil == DB.Resource.get_related_geojson_info("other_id")
+
+    assert %{geojson: %{url: "url2", filesize: "12", resource_history_last_up_to_date_at: _}} =
+             DB.Resource.get_related_files(%DB.Resource{datagouv_id: "datagouv_id_1"})
+  end
+
+  defp insert_resouce_history(datagouv_id, uuid, datetime, time_delta_seconds \\ 0) do
+    insert(:resource_history, %{
+      datagouv_id: datagouv_id,
+      payload: %{uuid: uuid},
+      inserted_at: DateTime.add(datetime, time_delta_seconds, :second)
+    })
+  end
+
+  defp insert_data_conversion(uuid, permanent_url, filesize) do
+    insert(:data_conversion, %{
+      resource_history_uuid: uuid,
+      convert_from: "GTFS",
+      convert_to: "GeoJSON",
+      payload: %{permanent_url: permanent_url, filesize: filesize}
+    })
+  end
+
   test "needs validation with a JSON Schema" do
     schema_name = "etalab/foo"
     resource = insert(:resource, %{schema_name: schema_name})

apps/transport/lib/jobs/gtfs_to_geojson_converter_job.ex

@@ -92,14 +92,17 @@ defmodule Transport.Jobs.SingleGtfsToGeojsonConverterJob do
       geojson_file_name = resource_filename |> geojson_file_name()
       Transport.S3.upload_to_s3!(:history, file, geojson_file_name)
 
+      {:ok, %{size: filesize}} = File.stat(geojson_file_path)
+
       %DataConversion{
         convert_from: "GTFS",
         convert_to: "GeoJSON",
         resource_history_uuid: resource_uuid,
         payload: %{
           filename: geojson_file_name,
           permanent_url: Transport.S3.permanent_url(:history, geojson_file_name),
-          resource_datagouv_id: resource_datagouv_id
+          resource_datagouv_id: resource_datagouv_id,
+          filesize: filesize
         }
       }
       |> Repo.insert!()

apps/transport/lib/jobs/resource_history_job.ex

@@ -118,9 +118,10 @@ defmodule Transport.Jobs.ResourceHistoryJob do
         Transport.S3.upload_to_s3!(:history, body, filename)
         store_resource_history!(resource, data)
 
-      false ->
+      {false, history} ->
         # Good opportunity to add a :telemetry event
         Logger.debug("skipping historization for #{datagouv_id} because resource did not change")
+        touch_resource_history!(history)
     end
   end
 
@@ -142,7 +143,11 @@ defmodule Transport.Jobs.ResourceHistoryJob do
       |> limit(1)
       |> DB.Repo.one()
 
-    is_nil(history) or not is_same_resource?(history, zip_metadata)
+    case {history, is_same_resource?(history, zip_metadata)} do
+      {nil, _} -> true
+      {_history, false} -> true
+      {history, true} -> {false, history}
+    end
   end
 
   @doc """
@@ -154,15 +159,23 @@ defmodule Transport.Jobs.ResourceHistoryJob do
     MapSet.equal?(set_of_sha256(payload["zip_metadata"]), set_of_sha256(zip_metadata))
   end
 
+  def is_same_resource?(nil, _), do: false
+
   def set_of_sha256(items), do: MapSet.new(items |> Enum.map(fn m -> Map.get(m, "sha256") || Map.get(m, :sha256) end))
 
   defp store_resource_history!(%Resource{datagouv_id: datagouv_id}, payload) do
     Logger.debug("Saving ResourceHistory for #{datagouv_id}")
 
-    %DB.ResourceHistory{datagouv_id: datagouv_id, payload: payload}
+    %DB.ResourceHistory{datagouv_id: datagouv_id, payload: payload, last_up_to_date_at: DateTime.utc_now()}
     |> DB.Repo.insert!()
   end
 
+  defp touch_resource_history!(%DB.ResourceHistory{id: id, datagouv_id: datagouv_id} = history) do
+    Logger.debug("Touching unchanged ResourceHistory #{id} for resource datagouv_id #{datagouv_id}")
+
+    history |> Ecto.Changeset.change(%{last_up_to_date_at: DateTime.utc_now()}) |> DB.Repo.update!()
+  end
+
   defp download_path(%Resource{datagouv_id: datagouv_id}) do
     System.tmp_dir!() |> Path.join("resource_#{datagouv_id}_download")
   end

apps/transport/lib/transport_web/controllers/dataset_controller.ex

@@ -43,6 +43,7 @@ defmodule TransportWeb.DatasetController do
 
       conn
       |> assign(:dataset, dataset)
+      |> assign(:resources_related_files, DB.Dataset.get_resources_related_files(dataset))
       |> assign(:territory, territory)
       |> assign(:discussions, Discussions.get(dataset.datagouv_id))
       |> assign(:site, Application.get_env(:oauth2, Authentication)[:site])

apps/transport/lib/transport_web/controllers/resource_controller.ex

@@ -40,6 +40,7 @@ defmodule TransportWeb.ResourceController do
           end
 
         conn
+        |> assign(:related_files, Resource.get_related_files(resource))
         |> assign(:resource, resource)
         |> assign(:other_resources, Resource.other_resources(resource))
         |> assign(:issues, Scrivener.paginate(issues, config))

apps/transport/lib/transport_web/templates/dataset/_resource.html.eex

@@ -1,5 +1,5 @@
 <div class="panel resource <%= valid_panel_class(@resource) %>" title="<%= resource_tooltip_content(@resource) %>">
-  <% has_associated_geojson = ResourceView.has_associated_geojson(@dataset, @resource) %>
+  <% has_associated_geojson = ResourceView.has_associated_geojson(@resources_related_files, @resource.id) %>
   <% has_associated_netex = ResourceView.has_associated_netex(@dataset, @resource) %>
   <h4>
     <%= @resource.title %>

apps/transport/lib/transport_web/templates/dataset/_resources_container.html.eex

@@ -9,7 +9,7 @@
     <% end %>
     <div>
       <div class="ressources-list">
-        <%= render_many order_resources_by_validity(@resources), TransportWeb.DatasetView, "_resource.html", as: :resource, conn: @conn, dataset: assigns[:dataset] %>
+        <%= render_many order_resources_by_validity(@resources), TransportWeb.DatasetView, "_resource.html", as: :resource, conn: @conn, resources_related_files: assigns[:resources_related_files], dataset: assigns[:dataset] %>
       </div>
 
       <%= if assigns[:show_history_section_link] do %>

apps/transport/lib/transport_web/templates/dataset/details.html.eex

@@ -64,7 +64,7 @@
       <% gtfs_official_resources= gtfs_official_resources(@dataset) %>
       <% show_history_section_link= @history_resources !== [] and gtfs_official_resources !== [] %>
 
-      <%= render TransportWeb.DatasetView, "_resources_container.html", conn: @conn, resources: gtfs_official_resources, dataset: @dataset, title: dgettext("page-dataset-details", "GTFS resources"), show_history_section_link: show_history_section_link %>
+      <%= render TransportWeb.DatasetView, "_resources_container.html", conn: @conn, resources: gtfs_official_resources, resources_related_files: @resources_related_files, dataset: @dataset, title: dgettext("page-dataset-details", "GTFS resources"), show_history_section_link: show_history_section_link %>
       <%= render TransportWeb.DatasetView, "_resources_container.html", conn: @conn, resources: gtfs_rt_official_resources(@dataset), title: dgettext("page-dataset-details", "GTFS real time resources") %>
       <%= render TransportWeb.DatasetView, "_resources_container.html", conn: @conn, resources: gbfs_official_resources(@dataset), title: dgettext("page-dataset-details", "GBFS resources") %>
       <%= render TransportWeb.DatasetView, "_resources_container.html", conn: @conn, resources: netex_official_resources(@dataset), title: dgettext("page-dataset-details", "NeTEx resources") %>

apps/transport/lib/transport_web/templates/resource/details.html.eex

@@ -1,5 +1,5 @@
 <section>
-  <% associated_geojson = get_associated_geojson(@resource) %>
+  <% associated_geojson = get_associated_geojson(@related_files) %>
   <% associated_netex = get_associated_netex(@resource) %>
   <div class="grey-background">
     <div class="container">
@@ -26,6 +26,9 @@
               </ul>
               * <%= dgettext("validations", "GeoJSON format contains only the spatial information of the resource (stops coordinates, eventually lines shapes)
               It corresponds to the data you can see on the map below.")%>
+              <%= unless is_nil(associated_geojson.resource_history_last_up_to_date_at) do %>
+                <%= dgettext("validations", "This GeoJSON was up-to-date with the GTFS resource %{hours} ago.", hours: hours_ago(associated_geojson.resource_history_last_up_to_date_at))%>
+              <% end %>
             </li>
           </div>
         <% end %>
@@ -63,13 +66,18 @@
                 'resource-geojson',
                 'resource-geojson-info',
                 "<%= associated_geojson.url %>",
-                "<%= associated_geojson.filesize %>",
+                "<%= associated_geojson.filesize || 0 %>",
                 "<%= dgettext("validations", "Stops visualization is quite big") %>",
                 "<%= dgettext("validations", "Show anyway") %>"
-              )
-            })
-          </script>
-        </div>
+                )
+              })
+              </script>
+              </div>
+              <%= unless is_nil(associated_geojson.resource_history_last_up_to_date_at) do %>
+              <div class="is-centered no-margin">
+                <%= dgettext("validations", "Visualization up-to-date %{hours} ago.", hours: hours_ago(associated_geojson.resource_history_last_up_to_date_at))%>
+              </div>
+              <% end %>
       <% end %>
       <h2 class="mt-48"><%= dgettext("validations", "Validation report")%></h2>
       <div class="panel" id="issues">

apps/transport/lib/transport_web/views/resource_view.ex

@@ -101,25 +101,56 @@ defmodule TransportWeb.ResourceView do
 
   def get_associated_resource(_resource, _format), do: nil
 
-  def has_associated_geojson(dataset, resource) do
-    case get_associated_resource(dataset, resource, "geojson") do
+  def has_associated_geojson(%{} = resources_related_files, resource_id) do
+    resources_related_files
+    |> Map.get(resource_id)
+    |> get_associated_geojson()
+    |> case do
       nil -> false
       _ -> true
     end
   end
 
+  def has_associated_geojson(_, _), do: false
+
   def has_associated_netex(dataset, resource) do
     case get_associated_resource(dataset, resource, "NeTEx") do
       nil -> false
       _ -> true
     end
   end
 
-  def get_associated_geojson(resource) do
-    get_associated_resource(resource, "geojson")
-  end
+  def get_associated_geojson(%{geojson: geojson_url}), do: geojson_url
+  def get_associated_geojson(_), do: nil
 
   def get_associated_netex(resource) do
     get_associated_resource(resource, "NeTEx")
   end
+
+  def hours_ago(utcdatetime) do
+    DateTime.utc_now() |> DateTime.diff(utcdatetime) |> seconds_to_hours_minutes()
+  end
+
+  @doc """
+  Converts seconds to a string showing hours and minutes.
+  Also work for negative input, even if not intended to use it that way.
+
+  iex> seconds_to_hours_minutes(3661)
+  "1 h 1 min"
+  iex> seconds_to_hours_minutes(60)
+  "1 min"
+  iex> seconds_to_hours_minutes(30)
+  "0 min"
+  iex> seconds_to_hours_minutes(-3661)
+  "-1 h 1 min"
+  """
+  @spec seconds_to_hours_minutes(integer()) :: binary()
+  def seconds_to_hours_minutes(seconds) do
+    hours = div(seconds, 3600)
+
+    case hours do
+      0 -> "#{div(seconds, 60)} min"
+      hours -> "#{hours} h #{seconds |> rem(3600) |> div(60) |> abs()} min"
+    end
+  end
 end

apps/transport/priv/gettext/en/LC_MESSAGES/validations.po

@@ -121,3 +121,11 @@ msgid "error"
 msgid_plural "errors"
 msgstr[0] ""
 msgstr[1] ""
+
+#, elixir-format
+msgid "This GeoJSON was up-to-date with the GTFS resource %{hours} ago."
+msgstr ""
+
+#, elixir-format
+msgid "Visualization up-to-date %{hours} ago."
+msgstr ""

apps/transport/priv/gettext/fr/LC_MESSAGES/validations.po

@@ -121,3 +121,11 @@ msgid "error"
 msgid_plural "errors"
 msgstr[0] "erreur"
 msgstr[1] "erreurs"
+
+#, elixir-format
+msgid "This GeoJSON was up-to-date with the GTFS resource %{hours} ago."
+msgstr "Ce GeoJSON était bien à jour avec la ressource GTFS il y a %{hours}."
+
+#, elixir-format
+msgid "Visualization up-to-date %{hours} ago."
+msgstr "Visualisation à jour il y a %{hours}."

apps/transport/priv/gettext/validations.pot

@@ -120,3 +120,11 @@ msgid "error"
 msgid_plural "errors"
 msgstr[0] ""
 msgstr[1] ""
+
+#, elixir-format
+msgid "This GeoJSON was up-to-date with the GTFS resource %{hours} ago."
+msgstr ""
+
+#, elixir-format
+msgid "Visualization up-to-date %{hours} ago."
+msgstr ""