GBFS : suppression des redirections (#3456)

* GBFS : suppression des redirections

* Update migration to delete data

* Add a catch-all to return 404 responses

apps/gbfs/lib/gbfs/controllers/index_controller.ex

@@ -27,4 +27,10 @@ defmodule GBFS.IndexController do
       }
     }
   end
+
+  def not_found(%Plug.Conn{} = conn, _params) do
+    conn
+    |> put_status(:not_found)
+    |> text("Network not found. See available data: https://transport.data.gouv.fr/datasets?type=bike-scooter-sharing")
+  end
 end

apps/gbfs/lib/gbfs/router.ex

@@ -12,10 +12,6 @@ defmodule GBFS.Router do
     plug(:assign_jcdecaux)
   end
 
-  pipeline :redirect_reseau do
-    plug(:assign_redirect)
-  end
-
   pipeline :index_pipeline do
     plug(:assign_index)
   end
@@ -32,66 +28,9 @@ defmodule GBFS.Router do
     "toulouse" => "Vélô"
   }
 
-  @reseaux_redirects [
-    %{
-      contract_id: "montpellier",
-      redirects: %{
-        "gbfs.json" => "https://montpellier-fr-smoove.klervi.net/gbfs/gbfs.json",
-        "system_information.json" => "https://montpellier-fr-smoove.klervi.net/gbfs/en/system_information.json",
-        "station_information.json" => "https://montpellier-fr-smoove.klervi.net/gbfs/en/station_information.json",
-        "station_status.json" => "https://montpellier-fr-smoove.klervi.net/gbfs/en/station_status.json"
-      }
-    },
-    %{
-      contract_id: "strasbourg",
-      redirects: %{
-        "gbfs.json" => "https://strasbourg-fr-smoove.klervi.net/gbfs/gbfs.json",
-        "system_information.json" => "https://strasbourg-fr-smoove.klervi.net/gbfs/en/system_information.json",
-        "station_information.json" => "https://strasbourg-fr-smoove.klervi.net/gbfs/en/station_information.json",
-        "station_status.json" => "https://strasbourg-fr-smoove.klervi.net/gbfs/en/station_status.json"
-      }
-    },
-    %{
-      contract_id: "rouen",
-      redirects: %{
-        "gbfs.json" => "https://gbfs.urbansharing.com/lovelolibreservice.fr/gbfs.json",
-        "system_information.json" => "https://gbfs.urbansharing.com/lovelolibreservice.fr/system_information.json",
-        "station_information.json" => "https://gbfs.urbansharing.com/lovelolibreservice.fr/station_information.json",
-        "station_status.json" => "https://gbfs.urbansharing.com/lovelolibreservice.fr/station_status.json"
-      }
-    },
-    %{
-      contract_id: "marseille",
-      redirects: %{
-        "gbfs.json" =>
-          "https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/gbfs.json?key=MjE0ZDNmMGEtNGFkZS00M2FlLWFmMWItZGNhOTZhMWQyYzM2",
-        "system_information.json" =>
-          "https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/system_information.json?key=MjE0ZDNmMGEtNGFkZS00M2FlLWFmMWItZGNhOTZhMWQyYzM2",
-        "station_information.json" =>
-          "https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/station_information.json?key=MjE0ZDNmMGEtNGFkZS00M2FlLWFmMWItZGNhOTZhMWQyYzM2",
-        "station_status.json" =>
-          "https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/station_status.json?key=MjE0ZDNmMGEtNGFkZS00M2FlLWFmMWItZGNhOTZhMWQyYzM2"
-      }
-    }
-  ]
-
   scope "/gbfs", GBFS do
     pipe_through(:api)
 
-    scope "/" do
-      pipe_through(:index_pipeline)
-      get("/", IndexController, :index)
-    end
-
-    @reseaux_redirects
-    |> Enum.map(fn %{contract_id: contract_id} ->
-      scope "/" <> contract_id do
-        pipe_through(:redirect_reseau)
-
-        get("/:path", RedirectController, :index, as: contract_id)
-      end
-    end)
-
     scope "/vcub" do
       get("/gbfs.json", VCubController, :index)
       get("/system_information.json", VCubController, :system_information)
@@ -118,6 +57,12 @@ defmodule GBFS.Router do
         get("/station_status.json", JCDecauxController, :station_status, as: contract)
       end
     end)
+
+    scope "/" do
+      pipe_through(:index_pipeline)
+      get("/", IndexController, :index)
+      get("/*path", IndexController, :not_found)
+    end
   end
 
   defp assign_jcdecaux(conn, _) do
@@ -128,19 +73,11 @@ defmodule GBFS.Router do
     |> assign(:contract_name, @reseaux_jcdecaux[contract_id])
   end
 
-  defp assign_redirect(conn, _) do
-    [_, contract_id, _] = conn.path_info
-
-    conn
-    |> assign(:redirect_params, Enum.find(@reseaux_redirects, &(&1.contract_id == contract_id)))
-  end
-
   defp assign_index(conn, _) do
     conn
     |> assign(
       :networks,
-      ["vcub", "vlille"] ++
-        (@reseaux_jcdecaux |> Map.keys()) ++ (@reseaux_redirects |> Enum.map(& &1.contract_id))
+      ["vcub", "vlille"] ++ (@reseaux_jcdecaux |> Map.keys())
     )
   end
 end

apps/gbfs/lib/page_cache.ex

@@ -113,7 +113,7 @@ defmodule PageCache do
   end
 
   def network_name(page_cache_key) do
-    case Regex.named_captures(~r|/gbfs/(?<network>.+)/|, page_cache_key) do
+    case Regex.named_captures(~r|/gbfs/(?<network>[a-zA-Z0-9_-]+)/|, page_cache_key) do
       %{"network" => network} -> network
       _ -> nil
     end

apps/gbfs/test/gbfs/controllers/index_controller_test.exs

@@ -17,4 +17,10 @@ defmodule GBFS.IndexControllerTest do
       assert first_href == "http://localhost/gbfs/vcub/gbfs.json"
     end
   end
+
+  test "404 pages", %{conn: conn} do
+    expected_regex = ~r"^Network not found. See available data:"
+    assert conn |> get("/gbfs/foo") |> text_response(404) =~ expected_regex
+    assert conn |> get("/gbfs/foo/gbfs.json") |> text_response(404) =~ expected_regex
+  end
 end

apps/gbfs/test/gbfs/page_cache_test.exs

@@ -61,6 +61,7 @@ defmodule GBFS.PageCacheTest do
     assert nil == PageCache.network_name("/foo")
     assert nil == PageCache.network_name("/gbfs")
     assert "nantes" == PageCache.network_name("/gbfs/nantes/gbfs.json")
+    assert "nantes" == PageCache.network_name("/gbfs/nantes//gbfs.json")
     assert "nantes" == PageCache.network_name("/gbfs/nantes/station_information.json")
     assert "st_helene" == PageCache.network_name("/gbfs/st_helene/station_information.json")
     assert "cergy-pontoise" == PageCache.network_name("/gbfs/cergy-pontoise/station_information.json")

apps/transport/priv/repo/migrations/20230913130308_metrics_gbfs_target_name.exs

@@ -3,7 +3,7 @@ defmodule DB.Repo.Migrations.MetricsGBFSTargetName do
 
   def up do
     # See https://github.com/etalab/transport-site/issues/3458
-    execute "update metrics set target = replace(target, '/', '') where target like 'gbfs:%/'"
+    execute "delete from metrics where target like 'gbfs:%/'"
   end
 
   def down do