From 24adea11751c621d9f6ae22f0d4070875f045eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Menou?= Date: Tue, 3 Sep 2024 14:50:45 +0200 Subject: [PATCH 1/4] Validateur NeTEx: quelques metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Traquons le nombre de retries et la durée d'une validation. --- .../lib/validators/netex_validator.ex | 35 ++++++++++--------- .../validators/netex_validator_test.exs | 20 +++++++---- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/apps/transport/lib/validators/netex_validator.ex b/apps/transport/lib/validators/netex_validator.ex index 6217d5ed0c..fe864ab191 100644 --- a/apps/transport/lib/validators/netex_validator.ex +++ b/apps/transport/lib/validators/netex_validator.ex @@ -27,19 +27,19 @@ defmodule Transport.Validators.NeTEx do def validate_resource_history(resource_history, filepath) do case validate_with_enroute(filepath) do - {:ok, result_url} -> - insert_validation_results(resource_history.id, result_url) + {:ok, %{url: result_url, retries: retries}} -> + insert_validation_results(resource_history.id, result_url, %{retries: retries}) :ok - {:error, {result_url, errors}} -> - insert_validation_results(resource_history.id, result_url, errors) + {:error, %{details: {result_url, errors}, retries: retries}} -> + insert_validation_results(resource_history.id, result_url, %{retries: retries}, errors) :ok {:error, :unexpected_validation_status} -> Logger.error("Invalid API call to enRoute Chouette Valid") {:error, "enRoute Chouette Valid: Unexpected validation status"} - {:error, :timeout} -> + {:error, %{message: :timeout, retries: _retries}} -> Logger.error("Timeout while fetching result on enRoute Chouette Valid") {:error, "enRoute Chouette Valid: Timeout while fetching results"} end @@ -58,23 +58,23 @@ defmodule Transport.Validators.NeTEx do def validate(url, opts \\ []) do with_url(url, fn filepath -> case validate_with_enroute(filepath, opts) do - {:ok, result_url} -> + {:ok, %{url: result_url, retries: retries}} -> # result_url in metadata? Logger.info("Result URL: #{result_url}") - {:ok, %{"validations" => index_messages([]), "metadata" => %{}}} + {:ok, %{"validations" => index_messages([]), "metadata" => %{retries: retries}}} - {:error, {result_url, errors}} -> + {:error, %{details: {result_url, errors}, retries: retries}} -> Logger.info("Result URL: #{result_url}") # result_url in metadata? - {:ok, %{"validations" => index_messages(errors), "metadata" => %{}}} + {:ok, %{"validations" => index_messages(errors), "metadata" => %{retries: retries}}} {:error, :unexpected_validation_status} -> Logger.error("Invalid API call to enRoute Chouette Valid") - {:error, "enRoute Chouette Valid: Unexpected validation status"} + {:error, %{message: "enRoute Chouette Valid: Unexpected validation status"}} - {:error, :timeout} -> + {:error, %{message: :timeout, retries: retries}} -> Logger.error("Timeout while fetching result on enRoute Chouette Valid") - {:error, "enRoute Chouette Valid: Timeout while fetching results"} + {:error, %{message: "enRoute Chouette Valid: Timeout while fetching results", retries: retries}} end end) end @@ -110,7 +110,7 @@ defmodule Transport.Validators.NeTEx do System.tmp_dir!() |> Path.join("enroute_validation_netex_#{Ecto.UUID.generate()}") end - def insert_validation_results(resource_history_id, result_url, errors \\ []) do + def insert_validation_results(resource_history_id, result_url, metadata, errors \\ []) do result = index_messages(errors) %DB.MultiValidation{ @@ -120,7 +120,8 @@ defmodule Transport.Validators.NeTEx do resource_history_id: resource_history_id, validator_version: validator_version(), command: result_url, - max_error: get_max_severity_error(result) + max_error: get_max_severity_error(result), + metadata: %DB.ResourceMetadata{metadata: metadata} } |> DB.Repo.insert!() end @@ -201,7 +202,7 @@ defmodule Transport.Validators.NeTEx do defp fetch_validation_results(validation_id, retries, opts) do case client().get_a_validation(validation_id) do {:pending, elapsed_seconds} when elapsed_seconds > @timeout -> - {:error, :timeout} + {:error, %{message: :timeout, retries: retries}} {:pending, _elapsed_seconds} -> if Keyword.get(opts, :graceful_retry, true) do @@ -211,10 +212,10 @@ defmodule Transport.Validators.NeTEx do fetch_validation_results(validation_id, retries + 1, opts) {:successful, url} -> - {:ok, url} + {:ok, %{url: url, retries: retries}} value when value in [:warning, :failed] -> - {:error, client().get_messages(validation_id)} + {:error, %{details: client().get_messages(validation_id), retries: retries}} :unexpected_validation_status -> {:error, :unexpected_validation_status} diff --git a/apps/transport/test/transport/validators/netex_validator_test.exs b/apps/transport/test/transport/validators/netex_validator_test.exs index 651c0b8d9b..7ea666d4ae 100644 --- a/apps/transport/test/transport/validators/netex_validator_test.exs +++ b/apps/transport/test/transport/validators/netex_validator_test.exs @@ -43,12 +43,13 @@ defmodule Transport.Validators.NeTExTest do assert :ok == NeTEx.validate_and_save(resource) - multi_validation = DB.MultiValidation |> DB.Repo.get_by(resource_history_id: resource_history.id) + multi_validation = load_multi_validation(resource_history.id) assert multi_validation.command == "http://localhost:9999/chouette-valid/#{validation_id}" assert multi_validation.validator == "enroute-chouette-netex-validator" assert multi_validation.validator_version == "saas-production" assert multi_validation.result == %{} + assert multi_validation.metadata.metadata == %{"retries" => 0} end test "invalid NeTEx" do @@ -61,11 +62,12 @@ defmodule Transport.Validators.NeTExTest do assert :ok == NeTEx.validate_and_save(resource) - multi_validation = DB.MultiValidation |> DB.Repo.get_by(resource_history_id: resource_history.id) + multi_validation = load_multi_validation(resource_history.id) assert multi_validation.command == "http://localhost:9999/chouette-valid/#{validation_id}/messages" assert multi_validation.validator == "enroute-chouette-netex-validator" assert multi_validation.validator_version == "saas-production" + assert multi_validation.metadata.metadata == %{"retries" => 0} assert multi_validation.result == %{ "uic-operating-period" => [ @@ -91,6 +93,12 @@ defmodule Transport.Validators.NeTExTest do ] } end + + defp load_multi_validation(resource_history_id) do + DB.MultiValidation + |> DB.Repo.get_by(resource_history_id: resource_history_id) + |> DB.Repo.preload(:metadata) + end end describe "raw URL" do @@ -100,7 +108,7 @@ defmodule Transport.Validators.NeTExTest do validation_id = expect_create_validation() expect_successful_validation(validation_id) - assert {:ok, %{"validations" => %{}, "metadata" => %{}}} == + assert {:ok, %{"validations" => %{}, "metadata" => %{retries: 0}}} == NeTEx.validate(resource_url) end @@ -136,7 +144,7 @@ defmodule Transport.Validators.NeTExTest do ] } - assert {:ok, %{"validations" => validation_result, "metadata" => %{}}} == NeTEx.validate(resource_url) + assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 0}}} == NeTEx.validate(resource_url) end test "retries" do @@ -162,7 +170,7 @@ defmodule Transport.Validators.NeTExTest do # Let's disable graceful retry as we are mocking the API, otherwise the # test would take almost a minute. - assert {:ok, %{"validations" => validation_result, "metadata" => %{}}} == + assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 3}}} == NeTEx.validate(resource_url, graceful_retry: false) end @@ -175,7 +183,7 @@ defmodule Transport.Validators.NeTExTest do {result, log} = with_log(fn -> NeTEx.validate(resource_url, graceful_retry: false) end) - assert result == {:error, "enRoute Chouette Valid: Timeout while fetching results"} + assert result == {:error, %{message: "enRoute Chouette Valid: Timeout while fetching results", retries: 1}} assert log =~ "[error] Timeout while fetching result on enRoute Chouette Valid" end end From b74f2ea374df9aeb1efc977af0877d602becf6f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Menou?= Date: Tue, 3 Sep 2024 15:43:06 +0200 Subject: [PATCH 2/4] =?UTF-8?q?Validateur=20NeTEx:=20temps=20pass=C3=A9s?= =?UTF-8?q?=20dans=20les=20metadata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../enroute_chouette_valid_client.ex | 31 ++++++++++------ .../lib/validators/netex_validator.ex | 37 +++++++++++++------ .../enroute_chouette_valid_client_test.exs | 12 +++--- .../validators/netex_validator_test.exs | 29 ++++++++------- 4 files changed, 65 insertions(+), 44 deletions(-) diff --git a/apps/transport/lib/validators/enroute_chouette_valid_client.ex b/apps/transport/lib/validators/enroute_chouette_valid_client.ex index 87c34c5818..648a75632e 100644 --- a/apps/transport/lib/validators/enroute_chouette_valid_client.ex +++ b/apps/transport/lib/validators/enroute_chouette_valid_client.ex @@ -7,9 +7,9 @@ defmodule Transport.EnRouteChouetteValidClient.Wrapper do @callback create_a_validation(Path.t()) :: binary() @callback get_a_validation(binary()) :: {:pending, integer()} - | {:successful, binary()} - | :warning - | :failed + | {:successful, binary(), integer()} + | {:warning, integer()} + | {:failed, integer()} | :unexpected_validation_status | :unexpected_datetime_format @callback get_messages(binary()) :: {binary(), map()} @@ -48,28 +48,35 @@ defmodule Transport.EnRouteChouetteValidClient do case response |> Map.fetch!("user_status") do "pending" -> - case {get_datetime(response, "created_at"), get_datetime(response, "updated_at")} do - {{:ok, created_at, _}, {:ok, updated_at, _}} -> - {:pending, DateTime.diff(updated_at, created_at)} - - _ -> - :unexpected_datetime_format + case get_elapsed(response) do + nil -> :unexpected_datetime_format + elapsed -> {:pending, elapsed} end "successful" -> - {:successful, url} + {:successful, url, get_elapsed(response)} "warning" -> - :warning + {:warning, get_elapsed(response)} "failed" -> - :failed + {:failed, get_elapsed(response)} _ -> :unexpected_validation_status end end + defp get_elapsed(response) do + case {get_datetime(response, "created_at"), get_datetime(response, "updated_at")} do + {{:ok, created_at, _}, {:ok, updated_at, _}} -> + DateTime.diff(updated_at, created_at) + + _ -> + nil + end + end + @impl Transport.EnRouteChouetteValidClient.Wrapper def get_messages(validation_id) do url = Path.join([validation_url(validation_id), "messages"]) diff --git a/apps/transport/lib/validators/netex_validator.ex b/apps/transport/lib/validators/netex_validator.ex index fe864ab191..520f256248 100644 --- a/apps/transport/lib/validators/netex_validator.ex +++ b/apps/transport/lib/validators/netex_validator.ex @@ -27,12 +27,19 @@ defmodule Transport.Validators.NeTEx do def validate_resource_history(resource_history, filepath) do case validate_with_enroute(filepath) do - {:ok, %{url: result_url, retries: retries}} -> - insert_validation_results(resource_history.id, result_url, %{retries: retries}) + {:ok, %{url: result_url, elapsed_seconds: elapsed_seconds, retries: retries}} -> + insert_validation_results(resource_history.id, result_url, %{elapsed_seconds: elapsed_seconds, retries: retries}) + :ok - {:error, %{details: {result_url, errors}, retries: retries}} -> - insert_validation_results(resource_history.id, result_url, %{retries: retries}, errors) + {:error, %{details: {result_url, errors}, elapsed_seconds: elapsed_seconds, retries: retries}} -> + insert_validation_results( + resource_history.id, + result_url, + %{elapsed_seconds: elapsed_seconds, retries: retries}, + errors + ) + :ok {:error, :unexpected_validation_status} -> @@ -58,15 +65,21 @@ defmodule Transport.Validators.NeTEx do def validate(url, opts \\ []) do with_url(url, fn filepath -> case validate_with_enroute(filepath, opts) do - {:ok, %{url: result_url, retries: retries}} -> + {:ok, %{url: result_url, elapsed_seconds: elapsed_seconds, retries: retries}} -> # result_url in metadata? Logger.info("Result URL: #{result_url}") - {:ok, %{"validations" => index_messages([]), "metadata" => %{retries: retries}}} - {:error, %{details: {result_url, errors}, retries: retries}} -> + {:ok, + %{"validations" => index_messages([]), "metadata" => %{elapsed_seconds: elapsed_seconds, retries: retries}}} + + {:error, %{details: {result_url, errors}, elapsed_seconds: elapsed_seconds, retries: retries}} -> Logger.info("Result URL: #{result_url}") # result_url in metadata? - {:ok, %{"validations" => index_messages(errors), "metadata" => %{retries: retries}}} + {:ok, + %{ + "validations" => index_messages(errors), + "metadata" => %{elapsed_seconds: elapsed_seconds, retries: retries} + }} {:error, :unexpected_validation_status} -> Logger.error("Invalid API call to enRoute Chouette Valid") @@ -211,11 +224,11 @@ defmodule Transport.Validators.NeTEx do fetch_validation_results(validation_id, retries + 1, opts) - {:successful, url} -> - {:ok, %{url: url, retries: retries}} + {:successful, url, elapsed_seconds} -> + {:ok, %{url: url, elapsed_seconds: elapsed_seconds, retries: retries}} - value when value in [:warning, :failed] -> - {:error, %{details: client().get_messages(validation_id), retries: retries}} + {value, elapsed_seconds} when value in [:warning, :failed] -> + {:error, %{details: client().get_messages(validation_id), elapsed_seconds: elapsed_seconds, retries: retries}} :unexpected_validation_status -> {:error, :unexpected_validation_status} diff --git a/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs b/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs index 39450fac3a..d678b1ae24 100644 --- a/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs +++ b/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs @@ -78,7 +78,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do "started_at": "2024-07-05T14:41:20.680Z", "ended_at": "2024-07-05T14:41:20.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:19.933Z" + "updated_at": "2024-07-05T14:41:24.933Z" } """ @@ -89,7 +89,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do %HTTPoison.Response{status_code: 200, body: response_body} end) - assert {:successful, url} == EnRouteChouetteValidClient.get_a_validation(validation_id) + assert {:successful, url, 5} == EnRouteChouetteValidClient.get_a_validation(validation_id) end test "warning" do @@ -104,7 +104,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do "started_at": "2024-07-05T14:41:20.680Z", "ended_at": "2024-07-05T14:41:20.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:19.933Z" + "updated_at": "2024-07-05T14:41:23.933Z" } """ @@ -115,7 +115,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do %HTTPoison.Response{status_code: 200, body: response_body} end) - assert :warning == EnRouteChouetteValidClient.get_a_validation(validation_id) + assert {:warning, 4} == EnRouteChouetteValidClient.get_a_validation(validation_id) end test "failed" do @@ -130,7 +130,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do "started_at": "2024-07-05T14:41:20.680Z", "ended_at": "2024-07-05T14:41:20.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:19.933Z" + "updated_at": "2024-07-05T14:41:27.933Z" } """ @@ -141,7 +141,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do %HTTPoison.Response{status_code: 200, body: response_body} end) - assert :failed == EnRouteChouetteValidClient.get_a_validation(validation_id) + assert {:failed, 8} == EnRouteChouetteValidClient.get_a_validation(validation_id) end end diff --git a/apps/transport/test/transport/validators/netex_validator_test.exs b/apps/transport/test/transport/validators/netex_validator_test.exs index 7ea666d4ae..bac9da541f 100644 --- a/apps/transport/test/transport/validators/netex_validator_test.exs +++ b/apps/transport/test/transport/validators/netex_validator_test.exs @@ -39,7 +39,7 @@ defmodule Transport.Validators.NeTExTest do {resource, resource_history} = mk_netex_resource() validation_id = expect_create_validation() - expect_successful_validation(validation_id) + expect_successful_validation(validation_id, 12) assert :ok == NeTEx.validate_and_save(resource) @@ -49,14 +49,14 @@ defmodule Transport.Validators.NeTExTest do assert multi_validation.validator == "enroute-chouette-netex-validator" assert multi_validation.validator_version == "saas-production" assert multi_validation.result == %{} - assert multi_validation.metadata.metadata == %{"retries" => 0} + assert multi_validation.metadata.metadata == %{"retries" => 0, "elapsed_seconds" => 12} end test "invalid NeTEx" do {resource, resource_history} = mk_netex_resource() validation_id = expect_create_validation() - expect_failed_validation(validation_id) + expect_failed_validation(validation_id, 31) expect_get_messages(validation_id, @sample_error_messages) @@ -67,7 +67,7 @@ defmodule Transport.Validators.NeTExTest do assert multi_validation.command == "http://localhost:9999/chouette-valid/#{validation_id}/messages" assert multi_validation.validator == "enroute-chouette-netex-validator" assert multi_validation.validator_version == "saas-production" - assert multi_validation.metadata.metadata == %{"retries" => 0} + assert multi_validation.metadata.metadata == %{"retries" => 0, "elapsed_seconds" => 31} assert multi_validation.result == %{ "uic-operating-period" => [ @@ -106,9 +106,9 @@ defmodule Transport.Validators.NeTExTest do resource_url = mk_raw_netex_resource() validation_id = expect_create_validation() - expect_successful_validation(validation_id) + expect_successful_validation(validation_id, 9) - assert {:ok, %{"validations" => %{}, "metadata" => %{retries: 0}}} == + assert {:ok, %{"validations" => %{}, "metadata" => %{retries: 0, elapsed_seconds: 9}}} == NeTEx.validate(resource_url) end @@ -116,7 +116,7 @@ defmodule Transport.Validators.NeTExTest do resource_url = mk_raw_netex_resource() validation_id = expect_create_validation() - expect_failed_validation(validation_id) + expect_failed_validation(validation_id, 25) expect_get_messages(validation_id, @sample_error_messages) @@ -144,7 +144,8 @@ defmodule Transport.Validators.NeTExTest do ] } - assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 0}}} == NeTEx.validate(resource_url) + assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 0, elapsed_seconds: 25}}} == + NeTEx.validate(resource_url) end test "retries" do @@ -154,7 +155,7 @@ defmodule Transport.Validators.NeTExTest do expect_pending_validation(validation_id, 10) expect_pending_validation(validation_id, 20) expect_pending_validation(validation_id, 30) - expect_failed_validation(validation_id) + expect_failed_validation(validation_id, 35) expect_get_messages(validation_id, @sample_error_message) @@ -170,7 +171,7 @@ defmodule Transport.Validators.NeTExTest do # Let's disable graceful retry as we are mocking the API, otherwise the # test would take almost a minute. - assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 3}}} == + assert {:ok, %{"validations" => validation_result, "metadata" => %{retries: 3, elapsed_seconds: 35}}} == NeTEx.validate(resource_url, graceful_retry: false) end @@ -198,14 +199,14 @@ defmodule Transport.Validators.NeTExTest do expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> {:pending, elapsed} end) end - defp expect_successful_validation(validation_id) do + defp expect_successful_validation(validation_id, elapsed) do expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> - {:successful, "http://localhost:9999/chouette-valid/#{validation_id}"} + {:successful, "http://localhost:9999/chouette-valid/#{validation_id}", elapsed} end) end - defp expect_failed_validation(validation_id) do - expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> :failed end) + defp expect_failed_validation(validation_id, elapsed) do + expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> {:failed, elapsed} end) end defp expect_get_messages(validation_id, result) do From d07b07b2d36529434fc1159444be882ce35fbcd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Menou?= Date: Thu, 5 Sep 2024 11:32:01 +0200 Subject: [PATCH 3/4] More idiomatic --- .../enroute_chouette_valid_client.ex | 36 +++++++++---------- .../lib/validators/netex_validator.ex | 3 -- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/apps/transport/lib/validators/enroute_chouette_valid_client.ex b/apps/transport/lib/validators/enroute_chouette_valid_client.ex index 648a75632e..b9aad3bb00 100644 --- a/apps/transport/lib/validators/enroute_chouette_valid_client.ex +++ b/apps/transport/lib/validators/enroute_chouette_valid_client.ex @@ -11,7 +11,6 @@ defmodule Transport.EnRouteChouetteValidClient.Wrapper do | {:warning, integer()} | {:failed, integer()} | :unexpected_validation_status - | :unexpected_datetime_format @callback get_messages(binary()) :: {binary(), map()} def impl, do: Application.get_env(:transport, :enroute_validator_client) @@ -48,32 +47,37 @@ defmodule Transport.EnRouteChouetteValidClient do case response |> Map.fetch!("user_status") do "pending" -> - case get_elapsed(response) do - nil -> :unexpected_datetime_format - elapsed -> {:pending, elapsed} - end + {:pending, get_elapsed!(response)} "successful" -> - {:successful, url, get_elapsed(response)} + {:successful, url, get_elapsed!(response)} "warning" -> - {:warning, get_elapsed(response)} + {:warning, get_elapsed!(response)} "failed" -> - {:failed, get_elapsed(response)} + {:failed, get_elapsed!(response)} _ -> :unexpected_validation_status end end - defp get_elapsed(response) do - case {get_datetime(response, "created_at"), get_datetime(response, "updated_at")} do - {{:ok, created_at, _}, {:ok, updated_at, _}} -> - DateTime.diff(updated_at, created_at) + defp get_elapsed!(response) do + created_at = get_datetime!(response, "created_at") + updated_at = get_datetime!(response, "updated_at") + DateTime.diff(updated_at, created_at) + end - _ -> - nil + defp get_datetime!(map, key) do + raw_date = Map.fetch!(map, key) + + case DateTime.from_iso8601(raw_date) do + {:ok, value, _offset} -> + value + + {:error, reason} -> + raise ArgumentError, "cannot parse #{inspect(raw_date)} as datetime, reasaon: #{inspect(reason)}" end end @@ -85,10 +89,6 @@ defmodule Transport.EnRouteChouetteValidClient do {url, body |> Jason.decode!()} end - defp get_datetime(map, key) do - map |> Map.fetch!(key) |> DateTime.from_iso8601() - end - defp make_file_part(field_name, filepath) do {:file, filepath, {"form-data", [{:name, field_name}, {:filename, Path.basename(filepath)}]}, []} end diff --git a/apps/transport/lib/validators/netex_validator.ex b/apps/transport/lib/validators/netex_validator.ex index 520f256248..44cac53b53 100644 --- a/apps/transport/lib/validators/netex_validator.ex +++ b/apps/transport/lib/validators/netex_validator.ex @@ -232,9 +232,6 @@ defmodule Transport.Validators.NeTEx do :unexpected_validation_status -> {:error, :unexpected_validation_status} - - :unexpected_datetime_format -> - {:error, :unexpected_datetime_format} end end From 645d7b753adb2e23396891d78d442fcc5563342f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Menou?= Date: Thu, 5 Sep 2024 12:15:38 +0200 Subject: [PATCH 4/4] Validateur NeTEx : fix timeout et elapsed_seconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La mesure du temps passé ne fonctionnait pas comme prévu : le champs updated_at n'est en effet pas actualisé chez enRoute. Pour mesurer le temps passé à valider un fichier NeTEx il faut plutôt considérer started_at et ended_at. Ce dernier n'est pas naturellement pas renseigné pour le cas :pending. Le timeout est désormais implémenté par le nombre de retries => 100 retries est donc désormais la limite haute. --- .../enroute_chouette_valid_client.ex | 8 ++++---- .../lib/validators/netex_validator.ex | 7 +++---- .../enroute_chouette_valid_client_test.exs | 16 ++++++++-------- .../validators/netex_validator_test.exs | 18 ++++++++++-------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/apps/transport/lib/validators/enroute_chouette_valid_client.ex b/apps/transport/lib/validators/enroute_chouette_valid_client.ex index b9aad3bb00..5563976080 100644 --- a/apps/transport/lib/validators/enroute_chouette_valid_client.ex +++ b/apps/transport/lib/validators/enroute_chouette_valid_client.ex @@ -6,7 +6,7 @@ defmodule Transport.EnRouteChouetteValidClient.Wrapper do @callback create_a_validation(Path.t()) :: binary() @callback get_a_validation(binary()) :: - {:pending, integer()} + :pending | {:successful, binary(), integer()} | {:warning, integer()} | {:failed, integer()} @@ -47,7 +47,7 @@ defmodule Transport.EnRouteChouetteValidClient do case response |> Map.fetch!("user_status") do "pending" -> - {:pending, get_elapsed!(response)} + :pending "successful" -> {:successful, url, get_elapsed!(response)} @@ -64,8 +64,8 @@ defmodule Transport.EnRouteChouetteValidClient do end defp get_elapsed!(response) do - created_at = get_datetime!(response, "created_at") - updated_at = get_datetime!(response, "updated_at") + created_at = get_datetime!(response, "started_at") + updated_at = get_datetime!(response, "ended_at") DateTime.diff(updated_at, created_at) end diff --git a/apps/transport/lib/validators/netex_validator.ex b/apps/transport/lib/validators/netex_validator.ex index 44cac53b53..e7ec9bea58 100644 --- a/apps/transport/lib/validators/netex_validator.ex +++ b/apps/transport/lib/validators/netex_validator.ex @@ -9,8 +9,7 @@ defmodule Transport.Validators.NeTEx do @no_error "NoError" - # 15 minutes for the validation to complete should be enough. - @timeout 15 * 60 + @max_retries 100 @behaviour Transport.Validators.Validator @@ -214,10 +213,10 @@ defmodule Transport.Validators.NeTEx do defp fetch_validation_results(validation_id, retries, opts) do case client().get_a_validation(validation_id) do - {:pending, elapsed_seconds} when elapsed_seconds > @timeout -> + :pending when retries >= @max_retries -> {:error, %{message: :timeout, retries: retries}} - {:pending, _elapsed_seconds} -> + :pending -> if Keyword.get(opts, :graceful_retry, true) do retries |> poll_interval() |> :timer.sleep() end diff --git a/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs b/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs index d678b1ae24..b571772d53 100644 --- a/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs +++ b/apps/transport/test/transport/validators/enroute_chouette_valid_client_test.exs @@ -52,7 +52,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do "user_status": "pending", "started_at": "2024-07-05T14:41:20.680Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:45:20.933Z" + "updated_at": "2024-07-05T14:41:20.933Z" } """ @@ -63,7 +63,7 @@ defmodule Transport.EnRouteChouetteValidClientTest do %HTTPoison.Response{status_code: 200, body: response_body} end) - assert {:pending, 4 * 60 + 1} == EnRouteChouetteValidClient.get_a_validation(validation_id) + assert :pending == EnRouteChouetteValidClient.get_a_validation(validation_id) end test "successful" do @@ -76,9 +76,9 @@ defmodule Transport.EnRouteChouetteValidClientTest do "rule_set": "french", "user_status": "successful", "started_at": "2024-07-05T14:41:20.680Z", - "ended_at": "2024-07-05T14:41:20.685Z", + "ended_at": "2024-07-05T14:41:25.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:24.933Z" + "updated_at": "2024-07-05T14:41:20.933Z" } """ @@ -102,9 +102,9 @@ defmodule Transport.EnRouteChouetteValidClientTest do "rule_set": "french", "user_status": "warning", "started_at": "2024-07-05T14:41:20.680Z", - "ended_at": "2024-07-05T14:41:20.685Z", + "ended_at": "2024-07-05T14:41:24.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:23.933Z" + "updated_at": "2024-07-05T14:41:20.933Z" } """ @@ -128,9 +128,9 @@ defmodule Transport.EnRouteChouetteValidClientTest do "rule_set": "french", "user_status": "failed", "started_at": "2024-07-05T14:41:20.680Z", - "ended_at": "2024-07-05T14:41:20.685Z", + "ended_at": "2024-07-05T14:41:28.685Z", "created_at": "2024-07-05T14:41:19.933Z", - "updated_at": "2024-07-05T14:41:27.933Z" + "updated_at": "2024-07-05T14:41:20.933Z" } """ diff --git a/apps/transport/test/transport/validators/netex_validator_test.exs b/apps/transport/test/transport/validators/netex_validator_test.exs index bac9da541f..cd41dda0e0 100644 --- a/apps/transport/test/transport/validators/netex_validator_test.exs +++ b/apps/transport/test/transport/validators/netex_validator_test.exs @@ -152,9 +152,9 @@ defmodule Transport.Validators.NeTExTest do resource_url = mk_raw_netex_resource() validation_id = expect_create_validation() - expect_pending_validation(validation_id, 10) - expect_pending_validation(validation_id, 20) - expect_pending_validation(validation_id, 30) + expect_pending_validation(validation_id) + expect_pending_validation(validation_id) + expect_pending_validation(validation_id) expect_failed_validation(validation_id, 35) expect_get_messages(validation_id, @sample_error_message) @@ -179,12 +179,14 @@ defmodule Transport.Validators.NeTExTest do resource_url = mk_raw_netex_resource() validation_id = expect_create_validation() - expect_pending_validation(validation_id, 10) - expect_pending_validation(validation_id, 4510) + + Enum.each(0..100, fn _i -> + expect_pending_validation(validation_id) + end) {result, log} = with_log(fn -> NeTEx.validate(resource_url, graceful_retry: false) end) - assert result == {:error, %{message: "enRoute Chouette Valid: Timeout while fetching results", retries: 1}} + assert result == {:error, %{message: "enRoute Chouette Valid: Timeout while fetching results", retries: 100}} assert log =~ "[error] Timeout while fetching result on enRoute Chouette Valid" end end @@ -195,8 +197,8 @@ defmodule Transport.Validators.NeTExTest do validation_id end - defp expect_pending_validation(validation_id, elapsed) do - expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> {:pending, elapsed} end) + defp expect_pending_validation(validation_id) do + expect(Transport.EnRouteChouetteValidClient.Mock, :get_a_validation, fn ^validation_id -> :pending end) end defp expect_successful_validation(validation_id, elapsed) do