editoast, front: forbid zero-length paths in pathfinding endpoint

[Sh099078]

Fixes #9444

• Editoast: immediately return a 422 Unprocessable Content error at deserialization when the endpoint pathfinding/blocks is called with the same origin and destination.
• Editoast: pathfinding/blocks: fail in post-processing with a new PathfindingInputError::ZeroLengthPath variant when core returns zero-length paths.
• Front: add a new type of stdcm error type when trying to make an STDCM request with the same origin and destination, update the stdcm inputs validation method.

[emersion]

This is not enough to figure out if the path length will be zero. For instance, a UIC+ch could refer to the same OP as a trigram+ch.

[Sh099078]

I am missing some knowledge about what a trigram and an UIC represent and how to correlate them. Would you mind to elaborate or hint at some documentation on them ?

[emersion]

Sure. There are multiple ways to refer to an operational point in a path:

osrd/editoast/editoast_schemas/src/train_schedule/path_item.rs

Line 64 in 9dedb37

pub enum OperationalPointIdentifier {

One way is via a UIC code. The UIC code is a worldwide-unique number which identifies a location. For instance, "87686006" is "Paris-Gare-de-Lyon". In France, the UIC is not enough to precisely identify an operational point: there are multiple operational points in "Paris-Gare-de-Lyon". So a secondary code is added to pin down a specific one (often also called "ch" for "code chantier"). For instance, "BV" often refers to the place where the platforms are in a station ("bâtiment voyageur"). Side note, the UIC is made up of two parts, a country prefix ("87" for France) and the rest is called CI ("code immuable", local French code, e.g. "686006").

Another way is via a trigram. It's a 3-letter code. It's essentially another way to write a CI, perhaps easier to remember for humans. For instance, "PLY" is the trigram for "Paris-Gare-de-Lyon".

Yet another way is via an operational point ID. It's just an opaque string identifier (e.g. "1e083f8b-c7db-48f4-bb12-ac3bb21d8963").

So, both of these refer to the same operational point:

OperationalPointDescription {
    trigram: "PLY",
    secondary_code: "BV",
}
OperationalPointUic {
    uic: 686006,
    secondary_code: "BV",
}

In other words, if these are given as input to the pathfinding algorithm, it'll freak out the same way it would if given two exact same path item locations.

[Sh099078]

Thank you for the explanation ! I added a new error variant ZeroLengthPath that transforms core success responses with zero-length paths into errors instead of checking whether the origin and destination are the same location before executing the pathfinding.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.92%. Comparing base (c155980) to head (30a4ecb).
Report is 1 commits behind head on dev.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #9931      +/-   ##
==========================================
+ Coverage   81.78%   81.92%   +0.13%     
==========================================
  Files        1073      768     -305     
  Lines      106553    76891   -29662     
  Branches      726      731       +5     
==========================================
- Hits        87148    62990   -24158     
+ Misses      19366    13862    -5504     
  Partials       39       39              

Flag	Coverage Δ
editoast	?
front	89.34% <100.00%> (-0.01%)	⬇️
gateway	2.18% <ø> (ø)
osrdyne	3.28% <ø> (ø)
railjson_generator	87.50% <ø> (ø)
tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[woshilapin]

Nice fix. Thank you.

[emersion]

Apart from this, looks good!

[SharglutDev]

Note

This will conflict with #9924

[flomonster]

Should we authorize pathfinding that has a total length higher than 0m but between two steps has a length of 0m?

[SharglutDev]

We don't use ch in the code (too much sncf) but secondary_code (editoast type).
Both uic and secondary_code are defined if location is defined in StdcmPathStep type.
As we checked l.13 that we don't have any path step without location, we can assert that we will have one at this point.

Suggested change

	if (
	'uic' in origin &&
	'uic' in destination &&
	'ch' in origin &&
	'ch' in destination &&
	origin.uic === destination.uic &&
	origin.ch === destination.ch
	) {
	if (
	origin.location!.uic === destination.location!.uic &&
	origin.location!.secondary_code === destination.location!.secondary_code
	) {

[Sh099078]

Fixed in df926d1.

I also put the origin / destination check before the pathfinding response check as a bit of a hotfix, otherwise it would return the standard error when pathfinding fails instead of the new error that is more specific. It might be useful to give to checkStdcmConfigErrors more than just a boolean in case of pathfinding failure ? If it also received the error type returned by Editoast, we could directly return the zero_length_path error instead of doing the current check on the front-end side.

[SharglutDev]

Yep, you're totally right, it should be done here :)
But some errors/warning we want to display doesn't exist on editoast so it won't go away totally.

[SharglutDev]

We could go a little further and check in useStaticPathfinding.ts (probably going to conflict with 2 other PR atm) if origin and destination are the same to avoid launching the pathfinding. There is a check l.57 where we could do the same thing as here (maybe we can extract this check in a util like in stdcm/utils.index.ts)

[Sh099078]

Test added for same origin and destination in 30a4ecb