diff --git a/apps/transport/lib/transport_web/api/controllers/stats_controller.ex b/apps/transport/lib/transport_web/api/controllers/stats_controller.ex index 0f1423dee1..8daa2126a1 100644 --- a/apps/transport/lib/transport_web/api/controllers/stats_controller.ex +++ b/apps/transport/lib/transport_web/api/controllers/stats_controller.ex @@ -79,10 +79,9 @@ defmodule TransportWeb.API.StatsController do def new_aom_without_datasets?(%{created_after_2021: true, dataset_types: %{pt: 0}}), do: true def new_aom_without_datasets?(_), do: false - @spec features(Ecto.Query.t()) :: [map()] - def features(q) do - q - |> Repo.all() + @spec features([map()]) :: [map()] + def features(result) do + result |> Enum.reject(fn aom -> is_nil(aom.geometry) or new_aom_without_datasets?(aom) end) |> Enum.map(fn aom -> dataset_types = @@ -136,6 +135,22 @@ defmodule TransportWeb.API.StatsController do |> Enum.to_list() end + @spec bike_scooter_sharing_features([map()]) :: [map()] + def bike_scooter_sharing_features(result) do + result + |> Enum.reject(fn r -> is_nil(r.geometry) end) + |> Enum.map(fn r -> + %{ + "geometry" => r.geometry |> JSON.encode!(), + "type" => "Feature", + # NOTE: there is a bug here - the key is an atom. + # I won't change it now because it would mean more changes somewhere else, maybe. + # `Map.reject(fn({k,v}) -> k == :geometry end)` will do it. + "properties" => Map.take(r, Enum.filter(Map.keys(r), fn k -> k != "geometry" end)) + } + end) + end + defmacro count_aom_types(aom_id, type, include_aggregates: true) do quote do fragment( @@ -219,17 +234,17 @@ defmodule TransportWeb.API.StatsController do end @spec index(Plug.Conn.t(), map()) :: Plug.Conn.t() - def index(%Plug.Conn{} = conn, _params), do: render_features(conn, aom_features_query(), "api-stats-aoms") + def index(%Plug.Conn{} = conn, _params), do: render_features(conn, :aoms, "api-stats-aoms") @spec regions(Plug.Conn.t(), map()) :: Plug.Conn.t() - def regions(%Plug.Conn{} = conn, _params), do: render_features(conn, region_features_query(), "api-stats-regions") + def regions(%Plug.Conn{} = conn, _params), do: render_features(conn, :regions, "api-stats-regions") @spec bike_scooter_sharing(Plug.Conn.t(), map()) :: Plug.Conn.t() def bike_scooter_sharing(%Plug.Conn{} = conn, _params), - do: render_features(conn, bike_scooter_sharing_features()) + do: render_features(conn, :bike_scooter_sharing) @spec quality(Plug.Conn.t(), map()) :: Plug.Conn.t() - def quality(%Plug.Conn{} = conn, _params), do: render_features(conn, quality_features_query(), "api-stats-quality") + def quality(%Plug.Conn{} = conn, _params), do: render_features(conn, :quality, "api-stats-quality") # # (not using @doc because this is a private method and it would then generate a warning ; @@ -248,22 +263,37 @@ defmodule TransportWeb.API.StatsController do # resorting to `send_resp` directly, we leverage `Transport.Shared.ConditionalJSONEncoder` to # skip JSON encoding, signaling the need to do so via a {:skip_json_encoding, data} tuple. # - @spec render_features(Plug.Conn.t(), Ecto.Query.t(), binary()) :: Plug.Conn.t() - defp render_features(conn, query, cache_key) do - comp_fn = fn -> - query - |> features() - |> geojson() - |> Jason.encode!() - end + @spec render_features(Plug.Conn.t(), atom(), binary()) :: Plug.Conn.t() + defp render_features(conn, item, cache_key) do + data = Transport.Cache.fetch(cache_key, fn -> rendered_geojson(item) end) - data = Transport.Cache.fetch(cache_key, comp_fn) + render(conn, data: {:skip_json_encoding, data}) + end + @spec render_features(Plug.Conn.t(), atom()) :: Plug.Conn.t() + defp render_features(conn, item) do + data = rendered_geojson(item) render(conn, data: {:skip_json_encoding, data}) end - defp render_features(conn, data) do - render(conn, data: {:skip_json_encoding, data |> geojson() |> Jason.encode!()}) + def rendered_geojson(item) when item in [:aoms, :regions, :quality] do + case item do + :aoms -> aom_features_query() + :regions -> region_features_query() + :quality -> quality_features_query() + end + |> Repo.all() + |> features() + |> geojson() + |> Jason.encode!() + end + + def rendered_geojson(:bike_scooter_sharing) do + bike_scooter_sharing_features_query() + |> Repo.all() + |> bike_scooter_sharing_features() + |> geojson() + |> Jason.encode!() end @spec aom_features_query :: Ecto.Query.t() @@ -346,32 +376,17 @@ defmodule TransportWeb.API.StatsController do }) end - @spec bike_scooter_sharing_features :: [] - def bike_scooter_sharing_features do - query = - DatasetGeographicView - |> join(:left, [gv], dataset in Dataset, on: dataset.id == gv.dataset_id) - |> select([gv, dataset], %{ - geometry: fragment("ST_Centroid(geom) as geometry"), - names: fragment("array_agg(? order by ? asc)", dataset.custom_title, dataset.custom_title), - slugs: fragment("array_agg(? order by ? asc)", dataset.slug, dataset.custom_title) - }) - |> where([_gv, dataset], dataset.type == "bike-scooter-sharing" and dataset.is_active) - |> group_by(fragment("geometry")) - - query - |> DB.Repo.all() - |> Enum.reject(fn r -> is_nil(r.geometry) end) - |> Enum.map(fn r -> - %{ - "geometry" => r.geometry |> JSON.encode!(), - "type" => "Feature", - # NOTE: there is a bug here - the key is an atom. - # I won't change it now because it would mean more changes somewhere else, maybe. - # `Map.reject(fn({k,v}) -> k == :geometry end)` will do it. - "properties" => Map.take(r, Enum.filter(Map.keys(r), fn k -> k != "geometry" end)) - } - end) + @spec bike_scooter_sharing_features_query :: Ecto.Query.t() + def bike_scooter_sharing_features_query do + DatasetGeographicView + |> join(:left, [gv], dataset in Dataset, on: dataset.id == gv.dataset_id) + |> select([gv, dataset], %{ + geometry: fragment("ST_Centroid(geom) as geometry"), + names: fragment("array_agg(? order by ? asc)", dataset.custom_title, dataset.custom_title), + slugs: fragment("array_agg(? order by ? asc)", dataset.slug, dataset.custom_title) + }) + |> where([_gv, dataset], dataset.type == "bike-scooter-sharing" and dataset.is_active) + |> group_by(fragment("geometry")) end @spec quality_features_query :: Ecto.Query.t() diff --git a/apps/transport/test/transport_web/controllers/api/stats_controller_test.exs b/apps/transport/test/transport_web/controllers/api/stats_controller_test.exs index 3ca3f51445..2db08544e0 100644 --- a/apps/transport/test/transport_web/controllers/api/stats_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/api/stats_controller_test.exs @@ -64,7 +64,9 @@ defmodule TransportWeb.API.StatsControllerTest do } ] - assert TransportWeb.API.StatsController.bike_scooter_sharing_features() == expected + assert TransportWeb.API.StatsController.bike_scooter_sharing_features_query() + |> DB.Repo.all() + |> TransportWeb.API.StatsController.bike_scooter_sharing_features() == expected end test "Quality of AOM data stats", %{conn: conn} do @@ -181,7 +183,9 @@ defmodule TransportWeb.API.StatsControllerTest do assert DB.AOM.created_after_2021?(aom) assert [] == - TransportWeb.API.StatsController.quality_features_query() |> TransportWeb.API.StatsController.features() + TransportWeb.API.StatsController.quality_features_query() + |> DB.Repo.all() + |> TransportWeb.API.StatsController.features() # If created before 2022, it is present even without a dataset aom = aom |> Ecto.Changeset.change(%{composition_res_id: 500}) |> DB.Repo.update!() @@ -189,7 +193,9 @@ defmodule TransportWeb.API.StatsControllerTest do refute DB.AOM.created_after_2021?(aom) assert [%{"properties" => %{"dataset_count" => 0, "nom" => ^aom_nom}}] = - TransportWeb.API.StatsController.quality_features_query() |> TransportWeb.API.StatsController.features() + TransportWeb.API.StatsController.quality_features_query() + |> DB.Repo.all() + |> TransportWeb.API.StatsController.features() # Created in 2022 but with a dataset aom = aom |> Ecto.Changeset.change(%{composition_res_id: 1_200}) |> DB.Repo.update!() @@ -198,7 +204,9 @@ defmodule TransportWeb.API.StatsControllerTest do assert DB.AOM.created_after_2021?(aom) assert [%{"properties" => %{"dataset_types" => %{pt: 1}, "nom" => ^aom_nom}}] = - TransportWeb.API.StatsController.quality_features_query() |> TransportWeb.API.StatsController.features() + TransportWeb.API.StatsController.quality_features_query() + |> DB.Repo.all() + |> TransportWeb.API.StatsController.features() end end @@ -226,7 +234,10 @@ defmodule TransportWeb.API.StatsControllerTest do "quality" => %{"error_level" => "Error"} } } - ] = TransportWeb.API.StatsController.quality_features_query() |> TransportWeb.API.StatsController.features() + ] = + TransportWeb.API.StatsController.quality_features_query() + |> DB.Repo.all() + |> TransportWeb.API.StatsController.features() end test "can load the /stats page", %{conn: conn} do