From f008ab5003eac5ce9b0ba9debb20be1e10559814 Mon Sep 17 00:00:00 2001 From: Loup Federico <16464925+Sh099078@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:14:22 +0100 Subject: [PATCH 1/2] editoast, front: forbid zero-length paths - Editoast: treat core pathfinding responses with a length of zero as errors. - Frontend, stdcm: add error notifications when the user enters the same origin and destination. Signed-off-by: Loup Federico <16464925+Sh099078@users.noreply.github.com> --- editoast/openapi.yaml | 8 +++ editoast/src/core/pathfinding.rs | 1 + editoast/src/views/path/pathfinding.rs | 58 ++++++++++++++++++- editoast/src/views/stdcm_logs.rs | 2 +- editoast/src/views/timetable/stdcm.rs | 2 +- editoast/src/views/train_schedule.rs | 2 +- front/public/locales/en/errors.json | 5 ++ front/public/locales/en/stdcm.json | 3 +- front/public/locales/fr/errors.json | 5 ++ front/public/locales/fr/stdcm.json | 3 +- .../stdcm/hooks/useStaticPathfinding.ts | 10 ++++ front/src/applications/stdcm/types.ts | 1 + .../stdcm/utils/checkStdcmConfigErrors.ts | 7 +++ front/src/common/api/generatedEditoastApi.ts | 3 + 14 files changed, 102 insertions(+), 8 deletions(-) diff --git a/editoast/openapi.yaml b/editoast/openapi.yaml index e7f477e3fb9..053ff0a1826 100644 --- a/editoast/openapi.yaml +++ b/editoast/openapi.yaml @@ -8435,6 +8435,14 @@ components: - rolling_stock_not_found rolling_stock_name: type: string + - type: object + required: + - error_type + properties: + error_type: + type: string + enum: + - zero_length_path PathfindingItem: type: object required: diff --git a/editoast/src/core/pathfinding.rs b/editoast/src/core/pathfinding.rs index 74c5ba738fd..8570eb93bd4 100644 --- a/editoast/src/core/pathfinding.rs +++ b/editoast/src/core/pathfinding.rs @@ -133,6 +133,7 @@ pub enum PathfindingInputError { RollingStockNotFound { rolling_stock_name: String, }, + ZeroLengthPath, } // Enum for not-found results and incompatible constraints diff --git a/editoast/src/views/path/pathfinding.rs b/editoast/src/views/path/pathfinding.rs index 4f4918ced7b..a050140da3e 100644 --- a/editoast/src/views/path/pathfinding.rs +++ b/editoast/src/views/path/pathfinding.rs @@ -86,7 +86,12 @@ pub enum PathfindingResult { impl From for PathfindingResult { fn from(core_result: PathfindingCoreResult) -> Self { match core_result { - PathfindingCoreResult::Success(success) => PathfindingResult::Success(success), + PathfindingCoreResult::Success(success) => match success.length { + 0 => PathfindingResult::Failure(PathfindingFailure::PathfindingInputError( + PathfindingInputError::ZeroLengthPath, + )), + _ => PathfindingResult::Success(success), + }, PathfindingCoreResult::NotFoundInBlocks { track_section_ranges, length, @@ -456,6 +461,53 @@ pub mod tests { use crate::views::path::pathfinding::PathfindingResult; use crate::views::test_app::TestAppBuilder; + #[rstest] + async fn pathfinding_fails_when_core_responds_with_zero_length_path() { + let db_pool = DbConnectionPoolV2::for_tests(); + let mut core = MockingClient::new(); + core.stub("/v2/pathfinding/blocks") + .method(reqwest::Method::POST) + .response(StatusCode::OK) + .json(json!({ + "blocks":[], + "routes": [], + "track_section_ranges": [], + "path_item_positions": [], + "length": 0, + "status": "success" + })) + .finish(); + let app = TestAppBuilder::new() + .db_pool(db_pool.clone()) + .core_client(core.into()) + .build(); + let small_infra = create_small_infra(&mut db_pool.get_ok()).await; + + let request = app + .post(format!("/infra/{}/pathfinding/blocks", small_infra.id).as_str()) + .json(&json!({ + "path_items":[ + {"trigram":"WS","secondary_code":"BV"}, + {"trigram":"WS","secondary_code":"BV"} + ], + "rolling_stock_is_thermal":true, + "rolling_stock_loading_gauge":"G1", + "rolling_stock_supported_electrifications":[], + "rolling_stock_supported_signaling_systems":["BAL","BAPR"], + "rolling_stock_maximum_speed":22.00, + "rolling_stock_length":26.00 + })); + + let pathfinding_result: PathfindingResult = + app.fetch(request).assert_status(StatusCode::OK).json_into(); + assert_eq!( + pathfinding_result, + PathfindingResult::Failure(PathfindingFailure::PathfindingInputError( + PathfindingInputError::ZeroLengthPath, + )) + ); + } + #[rstest] async fn pathfinding_with_invalid_path_items_returns_invalid_path_items() { let app = TestAppBuilder::default_app(); @@ -560,7 +612,7 @@ pub mod tests { "routes": [], "track_section_ranges": [], "path_item_positions": [], - "length": 0, + "length": 1, "status": "success" })) .finish(); @@ -593,7 +645,7 @@ pub mod tests { blocks: vec![], routes: vec![], track_section_ranges: vec![], - length: 0, + length: 1, path_item_positions: vec![] }) ); diff --git a/editoast/src/views/stdcm_logs.rs b/editoast/src/views/stdcm_logs.rs index 090bf9fe759..730c00b7370 100644 --- a/editoast/src/views/stdcm_logs.rs +++ b/editoast/src/views/stdcm_logs.rs @@ -262,7 +262,7 @@ mod tests { blocks: vec![], routes: vec![], track_section_ranges: vec![], - length: 0, + length: 1, path_item_positions: vec![0, 10], } } diff --git a/editoast/src/views/timetable/stdcm.rs b/editoast/src/views/timetable/stdcm.rs index 6294b6534bb..13b153495c7 100644 --- a/editoast/src/views/timetable/stdcm.rs +++ b/editoast/src/views/timetable/stdcm.rs @@ -668,7 +668,7 @@ mod tests { blocks: vec![], routes: vec![], track_section_ranges: vec![], - length: 0, + length: 1, path_item_positions: vec![0, 10], } } diff --git a/editoast/src/views/train_schedule.rs b/editoast/src/views/train_schedule.rs index 669c599c88e..b4b82cea3e1 100644 --- a/editoast/src/views/train_schedule.rs +++ b/editoast/src/views/train_schedule.rs @@ -945,7 +945,7 @@ mod tests { "routes": [], "track_section_ranges": [], "path_item_positions": [0,1,2,3], - "length": 0, + "length": 1, "status": "success" })) .finish(); diff --git a/front/public/locales/en/errors.json b/front/public/locales/en/errors.json index 411a3e54f9c..8836b6a84a0 100644 --- a/front/public/locales/en/errors.json +++ b/front/public/locales/en/errors.json @@ -91,6 +91,11 @@ "WrongRailjsonVersionProvided": "Wrong railjson version provided" } }, + "core": { + "pathfinding": { + "ZeroLengthPath": "Path length cannot be null." + } + }, "infra_state": { "FetchError": "Error while fetching the infrastructure loading status" }, diff --git a/front/public/locales/en/stdcm.json b/front/public/locales/en/stdcm.json index 64e72b4db4a..bf8c61a1b74 100644 --- a/front/public/locales/en/stdcm.json +++ b/front/public/locales/en/stdcm.json @@ -114,7 +114,8 @@ "noResults": "No path found", "noScheduledPoint": "The calculation requires either the origin or destination schedule.", "pathfindingFailed": "No path have been found for these waypoints.", - "requestFailed": "Calculation error for last minute train path" + "requestFailed": "Calculation error for last minute train path", + "zeroLengthPath": "The path cannot have a null length." }, "stdcmResults": "Results", "stdcmSimulationReport": "Path simulation report", diff --git a/front/public/locales/fr/errors.json b/front/public/locales/fr/errors.json index 5fcd77da315..608ac7fb216 100644 --- a/front/public/locales/fr/errors.json +++ b/front/public/locales/fr/errors.json @@ -91,6 +91,11 @@ "WrongRailjsonVersionProvided": "Mauvaise version de railjson fournie" } }, + "core": { + "pathfinding": { + "ZeroLengthPath": "La longueur d'un chemin ne peut pas être nulle." + } + }, "infra_state": { "FetchError": "Erreur de récupération de l'état de chargement de l'infrastructure" }, diff --git a/front/public/locales/fr/stdcm.json b/front/public/locales/fr/stdcm.json index e27cac48fde..5279eb1489c 100644 --- a/front/public/locales/fr/stdcm.json +++ b/front/public/locales/fr/stdcm.json @@ -114,7 +114,8 @@ "noResults": "Aucun sillon trouvé", "noScheduledPoint": "Le calcul a besoin de l'horaire d'origine ou de celui de la destination.", "pathfindingFailed": "Aucun chemin n'a été trouvé pour ces points de jalonnement.", - "requestFailed": "Erreur de calcul Sillon de dernière minute" + "requestFailed": "Erreur de calcul Sillon de dernière minute", + "zeroLengthPath": "Le chemin ne peut pas avoir de longueur nulle." }, "stdcmResults": "Résultats", "stdcmSimulationReport": "Fiche simulation", diff --git a/front/src/applications/stdcm/hooks/useStaticPathfinding.ts b/front/src/applications/stdcm/hooks/useStaticPathfinding.ts index 9a447f21eb2..a0eefd584a0 100644 --- a/front/src/applications/stdcm/hooks/useStaticPathfinding.ts +++ b/front/src/applications/stdcm/hooks/useStaticPathfinding.ts @@ -58,6 +58,16 @@ const useStaticPathfinding = (infra?: InfraWithState) => { return; } + // Don't run the pathfinding if the origin and destination are the same: + const origin = pathSteps.at(0)!; + const destination = pathSteps.at(-1)!; + if ( + origin.location!.uic === destination.location!.uic && + origin.location!.secondary_code === destination.location!.secondary_code + ) { + return; + } + const payload = getPathfindingQuery({ infraId: infra.id, rollingStock, diff --git a/front/src/applications/stdcm/types.ts b/front/src/applications/stdcm/types.ts index b674f446072..fa8618aa920 100644 --- a/front/src/applications/stdcm/types.ts +++ b/front/src/applications/stdcm/types.ts @@ -168,6 +168,7 @@ export enum StdcmConfigErrorTypes { PATHFINDING_FAILED = 'pathfindingFailed', BOTH_POINT_SCHEDULED = 'bothPointAreScheduled', NO_SCHEDULED_POINT = 'noScheduledPoint', + ZERO_LENGTH_PATH = 'zeroLengthPath', } export type StdcmConfigErrors = { diff --git a/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts b/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts index 279a2422f53..0d84ed5e5e6 100644 --- a/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts +++ b/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts @@ -23,6 +23,13 @@ const checkStdcmConfigErrors = ( throw new Error('Last step can not be a via'); } + if ( + origin.location!.uic === destination.location!.uic && + origin.location!.secondary_code === destination.location!.secondary_code + ) { + return { errorType: StdcmConfigErrorTypes.ZERO_LENGTH_PATH }; + } + if (pathfindingStateError) { return { errorType: StdcmConfigErrorTypes.PATHFINDING_FAILED }; } diff --git a/front/src/common/api/generatedEditoastApi.ts b/front/src/common/api/generatedEditoastApi.ts index 7919027ee92..966bba96888 100644 --- a/front/src/common/api/generatedEditoastApi.ts +++ b/front/src/common/api/generatedEditoastApi.ts @@ -2700,6 +2700,9 @@ export type PathfindingInputError = | { error_type: 'rolling_stock_not_found'; rolling_stock_name: string; + } + | { + error_type: 'zero_length_path'; }; export type OffsetRange = { end: number; From 2a2142a65a16577b4996795d9b1651885f65693d Mon Sep 17 00:00:00 2001 From: Loup Federico <16464925+Sh099078@users.noreply.github.com> Date: Wed, 22 Jan 2025 14:48:55 +0100 Subject: [PATCH 2/2] front: display stdcm error message even when pathfinding is not called Pathfinding is prevented when some validity checks don't pass, as for example when the user selects the same origin and destination. The error warning was only updated when pathfinging occurs, causing the user warnings not to be displayed when we could detect before launching the pathfinding that its request would be invalid. Update the warnings independently on whether the pathfinding actually happened so that invalid checks preventing the pathfinding also generate their user warnings. Signed-off-by: Loup Federico <16464925+Sh099078@users.noreply.github.com> --- .../stdcm/components/StdcmForm/StdcmConfig.tsx | 13 +++---------- .../stdcm/utils/checkStdcmConfigErrors.ts | 6 +++--- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/front/src/applications/stdcm/components/StdcmForm/StdcmConfig.tsx b/front/src/applications/stdcm/components/StdcmForm/StdcmConfig.tsx index f026b138182..0bf986b6463 100644 --- a/front/src/applications/stdcm/components/StdcmForm/StdcmConfig.tsx +++ b/front/src/applications/stdcm/components/StdcmForm/StdcmConfig.tsx @@ -127,8 +127,7 @@ const StdcmConfig = ({ const markersInfo = useMemo(() => extractMarkersInfo(pathSteps), [pathSteps]); const startSimulation = () => { - const isPathfindingFailed = !!pathfinding && pathfinding.status !== 'success'; - const formErrorsStatus = checkStdcmConfigErrors(isPathfindingFailed, pathSteps, t); + const formErrorsStatus = checkStdcmConfigErrors(pathSteps, t, pathfinding?.status); if (pathfinding?.status === 'success' && !formErrorsStatus) { launchStdcmRequest(); } else { @@ -157,14 +156,8 @@ const StdcmConfig = ({ }; useEffect(() => { - if (pathfinding) { - const formErrorsStatus = checkStdcmConfigErrors( - pathfinding.status !== 'success', - pathSteps, - t - ); - setFormErrors(formErrorsStatus); - } + const formErrorsStatus = checkStdcmConfigErrors(pathSteps, t, pathfinding?.status); + setFormErrors(formErrorsStatus); }, [pathfinding, pathSteps, t]); useEffect(() => { diff --git a/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts b/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts index 0d84ed5e5e6..14b3e39ecbd 100644 --- a/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts +++ b/front/src/applications/stdcm/utils/checkStdcmConfigErrors.ts @@ -6,9 +6,9 @@ import { dateToHHMMSS } from 'utils/date'; import { StdcmConfigErrorTypes, ArrivalTimeTypes, type StdcmConfigErrors } from '../types'; const checkStdcmConfigErrors = ( - pathfindingStateError: boolean, pathSteps: StdcmPathStep[], - t: TFunction + t: TFunction, + pathfindingStatus?: 'success' | 'failure' ): StdcmConfigErrors | undefined => { if (pathSteps.some((step) => !step.location)) { return { errorType: StdcmConfigErrorTypes.MISSING_LOCATION }; @@ -30,7 +30,7 @@ const checkStdcmConfigErrors = ( return { errorType: StdcmConfigErrorTypes.ZERO_LENGTH_PATH }; } - if (pathfindingStateError) { + if (pathfindingStatus && pathfindingStatus === 'failure') { return { errorType: StdcmConfigErrorTypes.PATHFINDING_FAILED }; }