Merge pull request #492 from woshilapin/clean-route-name

[fix] Remove specific implementations for `route.name` generation in connectors

src/documentation/kv1_to_ntfs_specs.md

@@ -67,12 +67,10 @@ A Route is created from a line and a direction stated in the JOPAXXXXXX.TMI file
 NTFS field | KV1 file | KV1 field | Mapping rule/Comment
 --- | --- | --- | ---
 route_id | JOPAXXXXXX.TMI | *LinePlanningNumber*, *Direction* | This field is prefixed. Concatenation of *LinePlanningNumber* and *Direction* separated by a `:`. Ex: "\<prefix\>:2029:2"
-route_name | JOPAXXXXXX.TMI |  | "[first stop of the first trip] - [last stop of the first trip]" (1)
+route_name | JOPAXXXXXX.TMI |  | See [`common.md`](common.md#general-rules) to generate the `name`
 direction_type | JOPAXXXXXX.TMI | *Direction* | `forward` value when *Direction* is `1` or `A`. `backward` in all other cases.
 line_id | JOPAXXXXXX.TMI | *LinePlanningNumber* | This field is prefixed. Link to the file [lines.txt](#linestxt).
-destination_id |  |  | `stop_id` of the stop_area of the last stop of the first trip.  (1)
-
-(1) The first trip of a route is the one with the smallest `trip_id` value.
+destination_id |  |  | See [`common.md`](common.md#general-rules) to generate the `destination_id`
 
 ### calendar.txt and calendar_dates.txt
 This document specifies a straightforward NTFS conversion using only `calendar_dates.txt` file. The resulting files may be different with the use of an optimizing processing, but the result should be functionally identical.

src/documentation/netex_idfm_to_ntfs.md

@@ -210,9 +210,9 @@ NTFS field | Netex-IDFM element | Mapping rule/Comment
 --- | --- | ---
 route_id | Route/@id | This field is prefixed. 
 line_id | Route/LineRef/@ref | This field is prefixed. 
-route_name | Route/Name | 
+route_name | Route/Name | See [`common.md`](common.md#general-rules) to generate the `name`
 direction_type | Route/DirectionType | The value of this field is used without transformation.
-destination_id |  | The `DirectionRef` of the Route doesn't link to a stop (neither stop_point nor stop_area), thus its value is not used.
+destination_id |  | The `DirectionRef` of the Route doesn't link to a stop (neither stop_point nor stop_area), thus its value is not used. See [`common.md`](common.md#general-rules) to generate the `destination_id`
 
 **ServiceJourneyPattern references**
 

src/documentation/transxchange_to_ntfs_specs.md

@@ -116,12 +116,10 @@ A Route is created from a line and a direction of journey patterns (*StandardSer
 NTFS field | TransXChange element | Mapping rule/Comment
 --- | --- | ---
 route_id | *Services/Service/ServiceCode*, *Services/Service/Line{id}* *StandardService/JourneyPattern/Direction* | This field is prefixed and formed by the concatenation of *Services/Service/ServiceCode*, *Services/Service/Line{id}* and *StandardService/JourneyPattern/Direction* separated by a `:`. Ex. "\<prefix>:1_58_BC:SL1:inbound".
-route_name |  | "[first stop of the first trip] - [last stop of the first trip]" (1)
+route_name |  | See [`common.md`](common.md#general-rules) to generate the `name`
 direction_type | *StandardService/JourneyPattern/Direction* | The value is set to `inbound` or `clockwise` when the specified value for the direction is `inboundAndOutbound` or `circular`, respectively.
 line_id | *Services/Service/ServiceCode*, *Services/Service/Line{id}* | This field is prefixed. Link to the file [lines.txt](#linestxt).
-destination_id |  | `stop_id` of the stop_area of the last stop of the first trip (1). Link to the file [stops.txt](#stopstxt).
-
-(1) The first trip of a route is the first one encontered in the TransXChange file.
+destination_id |  | See [`common.md`](common.md#general-rules) to generate the `destination_id`
 
 ### calendar.txt and calendar_dates.txt
 The validity period of a service is stated in *Services/Service/OperatingPeriod*. In case the validity period is open ended (the *EndDate* is not specified), the end date is defined by a mandatory parameter of this converter.

src/kv1/read.rs

@@ -673,22 +673,6 @@ fn make_vjs_and_stop_times(
     Ok(())
 }
 
-fn get_route_origin_destination<'a>(
-    collections: &'a Collections,
-    route_id: &str,
-) -> Result<(&'a StopPoint, &'a StopPoint)> {
-    let stop_times = &collections
-        .vehicle_journeys
-        .values()
-        .filter(|vj| vj.route_id == route_id)
-        .min_by_key(|vj| &vj.id) // TODO: instead of picking the first trip, find the most frequence origin (or destination) from all the trips
-        .ok_or_else(|| format_err!("vehicle_journeys with route_id={} not found", route_id))?
-        .stop_times;
-    let origin = &collections.stop_points[stop_times[0].stop_point_idx];
-    let destination = &collections.stop_points[stop_times[stop_times.len() - 1].stop_point_idx];
-    Ok((origin, destination))
-}
-
 fn make_routes(collections: &mut Collections, jopas: &[Jopa]) -> Result<()> {
     info!("Generating routes");
     let jopas_map: JopaMap = jopas
@@ -702,18 +686,17 @@ fn make_routes(collections: &mut Collections, jopas: &[Jopa]) -> Result<()> {
         .collect();
     for ((line_id, direction), jopa) in jopas_map {
         let id = jopa.route_id();
-        let (origin, destination) = skip_fail!(get_route_origin_destination(collections, &id));
-        let name = format!("{} - {}", origin.name, destination.name);
-        let destination_stop_area = collections
-            .stop_areas
-            .get(&destination.stop_area_id)
-            .ok_or_else(|| {
-                format_err!(
-                    "The stop point {} doesn't have a corresponding stop area.",
-                    destination.id
-                )
-            })?;
-        let destination_id = Some(destination_stop_area.id.clone());
+        if collections
+            .vehicle_journeys
+            .values()
+            .filter(|vj| vj.route_id == id)
+            .count()
+            == 0
+        {
+            continue;
+        }
+        let name = String::new(); // Auto-generated by Collections::enhance_route_names()
+        let destination_id = None; // Auto-generated by Collections::enhance_route_names()
         let direction_type = if direction == "1" || direction == "A" {
             "forward"
         } else {
@@ -746,6 +729,8 @@ fn route_name_by_direction<'a>(routes: &[&'a Route], direction_type: &str) -> Op
 
 fn make_lines(collections: &mut Collections, lines: &CollectionWithId<Kv1Line>) -> Result<()> {
     info!("Generating lines");
+    // Needs to generate the route names here because `Line` are generated from `Route`
+    collections.enhance_route_names();
     for l in lines.values() {
         let commercial_mode_id = MODES
             .get::<str>(&l.transport_type)

src/transxchange/read.rs

@@ -265,11 +265,9 @@ fn load_lines(
 }
 
 fn create_route(
-    collections: &Collections,
     transxchange: &Element,
     vehicle_journey: &Element,
     lines: &CollectionWithId<Line>,
-    stop_times: &[StopTime],
 ) -> Result<Route> {
     let service = transxchange
         .try_only_child("Services")?
@@ -297,29 +295,13 @@ fn create_route(
         );
     }
     let id = format!("{}:{}", line_id, direction_type);
-    let (first_stop_time, last_stop_time) = stop_times
-        .first()
-        .and_then(|f| stop_times.last().map(|l| (f, l)))
-        .ok_or_else(|| format_err!("Failed to find any StopTime to create the route {}", id))?;
-    let first_stop_point = &collections.stop_points[first_stop_time.stop_point_idx];
-    let last_stop_point = &collections.stop_points[last_stop_time.stop_point_idx];
-    let first_stop_area = collections.stop_areas.get(&first_stop_point.stop_area_id);
-    let first_stop_area_name = first_stop_area
-        .map(|stop_area| stop_area.name.clone())
-        .unwrap_or_else(|| UNDEFINED.to_string());
-    let last_stop_area = collections.stop_areas.get(&last_stop_point.stop_area_id);
-    let last_stop_area_name = last_stop_area
-        .map(|stop_area| stop_area.name.clone())
-        .unwrap_or_else(|| UNDEFINED.to_string());
-    let name = format!("{} - {}", first_stop_area_name, last_stop_area_name);
+    let name = String::new(); // Auto-generated by Collections::enhance_route_names()
     let direction_type = Some(direction_type);
-    let destination_id = last_stop_area.map(|stop_area| stop_area.id.clone());
     Ok(Route {
         id,
         name,
         direction_type,
         line_id,
-        destination_id,
         ..Default::default()
     })
 }
@@ -627,13 +609,7 @@ fn load_routes_vehicle_journeys_calendars(
             &operator_ref,
         )?;
         let company_id = operator.try_only_child("OperatorCode")?.text();
-        let route = create_route(
-            collections,
-            transxchange,
-            vehicle_journey,
-            lines,
-            &stop_times,
-        )?;
+        let route = create_route(transxchange, vehicle_journey, lines)?;
         let route_id = route.id.clone();
         let headsign = journey_pattern
             .only_child("DestinationDisplay")

tests/fixtures/kv12ntfs/output/lines.txt

@@ -1,4 +1,4 @@
 line_id,line_code,line_name,forward_line_name,backward_line_name,line_color,line_text_color,line_sort_order,network_id,commercial_mode_id,geometry_id,line_opening_time,line_closing_time
-prefix:2029,29,"Slagharen, Centrum - Slagharen, Attractiepark","Slagharen, Centrum - Slagharen, Attractiepark","Dedemsvaart, Busstation - Dedemsvaart, Adelaarweg",,,,prefix:SYNTUS,Bus,,,
+prefix:2029,29,"Hoogeveen, Willemskade - Hoogeveen, Willemskade","Hoogeveen, Willemskade - Hoogeveen, Willemskade","Dedemsvaart, Busstation - Dedemsvaart, Adelaarweg",,,,prefix:SYNTUS,Bus,,,
 prefix:2030,30,"Hoogeveen, Station - Hoogeveen, Willemskade","Hoogeveen, Station - Hoogeveen, Willemskade",,,,,prefix:SYNTUS,Bus,,,
 prefix:3170,170,"Epe, Centrum - Epe, Sint Jorisweg","Epe, Centrum - Epe, Sint Jorisweg",,,,,prefix:SYNTUS,Metro,,,

tests/fixtures/kv12ntfs/output/routes.txt

@@ -1,5 +1,5 @@
 route_id,route_name,direction_type,line_id,geometry_id,destination_id
-prefix:2029:1,"Slagharen, Centrum - Slagharen, Attractiepark",forward,prefix:2029,,prefix:20718
+prefix:2029:1,"Hoogeveen, Willemskade - Hoogeveen, Willemskade",forward,prefix:2029,,prefix:20718
 prefix:2029:2,"Dedemsvaart, Busstation - Dedemsvaart, Adelaarweg",backward,prefix:2029,,prefix:20361
 prefix:2030:1,"Hoogeveen, Station - Hoogeveen, Willemskade",forward,prefix:2030,,prefix:20718
 prefix:3170:1,"Epe, Centrum - Epe, Sint Jorisweg",forward,prefix:3170,,prefix:32704