Merge pull request #820 from CanalTP/growing-stop-times

documentation/common_ntfs_rules.md

@@ -138,6 +138,9 @@ The following rules apply to every converter, unless otherwise explicitly specif
   * the `line.closing_time` is generated with the biggest arrival time (at the last stop) of all journeys on the lines (+ 24h if the end is earlier than the start time).
   * if a line has several periods without circulation in the day, only the main one (larger and earlier) is used to define the opening and closing times.
   * lines with continuous circulation are indicated by default with an opening at 00:00 and a closing at 23:59.
+* If a trip contains stop times matching any of the two following conditions, the trip is deleted:
+  * if the arrival time of a stop time is greater that the departure time of the same stop time
+  * if the departure time of a stop time is greater that the arrival time of the next stop time
 
 ### Conflicting identifiers
 The model will raise a critical error if identifiers of 2 objects of the same type are identical.

src/enhancers/check_stop_times_order.rs

@@ -0,0 +1,122 @@
+use crate::model::Collections;
+use tracing::warn;
+use typed_index_collection::CollectionWithId;
+
+pub fn check_stop_times_order(collections: &mut Collections) {
+    let vehicle_journeys = collections.vehicle_journeys.take();
+    let mut filtered_vjs = Vec::new();
+    for mut vj in vehicle_journeys {
+        match vj.sort_and_check_stop_times() {
+            Ok(_) => filtered_vjs.push(vj),
+            Err(e) => warn!("{}", e),
+        }
+    }
+    collections.vehicle_journeys = CollectionWithId::new(filtered_vjs)
+        .expect("insert only vehicle journeys that were in a CollectionWithId before");
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::objects::{StopPoint, StopTime, Time, VehicleJourney};
+    use std::str::FromStr;
+
+    fn collections_from_times(
+        (a_arrival, a_departure): (&str, &str),
+        (b_arrival, b_departure): (&str, &str),
+    ) -> Collections {
+        let mut collections = Collections::default();
+        let stop_points = CollectionWithId::from(StopPoint {
+            id: "sp1".to_string(),
+            ..Default::default()
+        });
+        let stop_point_idx = stop_points.get_idx("sp1").unwrap();
+        let stop_times = vec![
+            StopTime {
+                stop_point_idx,
+                sequence: 0,
+                arrival_time: Time::from_str(a_arrival).unwrap(),
+                departure_time: Time::from_str(a_departure).unwrap(),
+                boarding_duration: 0,
+                alighting_duration: 0,
+                pickup_type: 0,
+                drop_off_type: 0,
+                datetime_estimated: false,
+                local_zone_id: None,
+                precision: None,
+            },
+            StopTime {
+                stop_point_idx,
+                sequence: 1,
+                arrival_time: FromStr::from_str(b_arrival).unwrap(),
+                departure_time: FromStr::from_str(b_departure).unwrap(),
+                boarding_duration: 0,
+                alighting_duration: 0,
+                pickup_type: 0,
+                drop_off_type: 0,
+                datetime_estimated: false,
+                local_zone_id: None,
+                precision: None,
+            },
+        ];
+        collections.vehicle_journeys = CollectionWithId::from(VehicleJourney {
+            id: "vj1".to_string(),
+            stop_times,
+            ..Default::default()
+        });
+        collections
+    }
+
+    #[test]
+    fn valid_vj() {
+        let mut collections =
+            collections_from_times(("10:00:00", "10:05:00"), ("11:00:00", "11:05:00"));
+
+        check_stop_times_order(&mut collections);
+
+        assert!(collections.vehicle_journeys.contains_id("vj1"));
+    }
+
+    #[test]
+    fn invalid_growing_stop_times_inside_stop() {
+        testing_logger::setup();
+        let mut collections =
+            collections_from_times(("10:05:00", "10:00:00"), ("11:00:00", "11:05:00"));
+
+        check_stop_times_order(&mut collections);
+
+        assert!(!collections.vehicle_journeys.contains_id("vj1"));
+        testing_logger::validate(|captured_logs| {
+            let error_log = captured_logs
+                .iter()
+                .find(|captured_log| captured_log.level == tracing::log::Level::Warn)
+                .expect("log error expected");
+            assert!(error_log
+                .body
+                .contains("incoherent stop times \'0\' at time \'10:00:00\' for the trip \'vj1\'"));
+        });
+    }
+
+    #[test]
+    fn invalid_growing_stop_times_over_two_stops() {
+        testing_logger::setup();
+        let mut collections =
+            collections_from_times(("10:00:00", "10:05:00"), ("10:03:00", "10:10:00"));
+
+        check_stop_times_order(&mut collections);
+
+        assert!(!collections.vehicle_journeys.contains_id("vj1"));
+        testing_logger::validate(|captured_logs| {
+            for log in captured_logs {
+                dbg!(&log.body);
+            }
+            let error_log = captured_logs
+                .iter()
+                .find(|captured_log| captured_log.level == tracing::log::Level::Warn)
+                .expect("log error expected");
+            assert!(error_log
+                .body
+                .contains("incoherent stop times \'0\' at time \'10:05:00\' for the trip \'vj1\'"));
+        });
+    }
+}

src/enhancers/mod.rs

@@ -1,9 +1,11 @@
 //! This module contains various functions that enhance / cleanup `Collections`
 
 mod adjust_lines_names;
+mod check_stop_times_order;
 mod enhance_pickup_dropoff;
 mod fill_co2;
 
 pub(crate) use adjust_lines_names::adjust_lines_names;
+pub(crate) use check_stop_times_order::check_stop_times_order;
 pub(crate) use enhance_pickup_dropoff::enhance_pickup_dropoff;
 pub(crate) use fill_co2::fill_co2;

src/model.rs

@@ -1232,6 +1232,7 @@ impl Model {
     /// assert!(Model::new(collections).is_ok());
     /// ```
     pub fn new(mut c: Collections) -> Result<Self> {
+        enhancers::check_stop_times_order(&mut c);
         c.comment_deduplication();
         c.clean_comments();
         c.sanitize()?;

src/ntfs/read.rs

@@ -21,7 +21,7 @@ use crate::utils;
 use crate::Result;
 use failure::{bail, ensure, format_err, ResultExt};
 use serde::{Deserialize, Serialize};
-use skip_error::{skip_error_and_error, skip_error_and_warn};
+use skip_error::skip_error_and_warn;
 use std::collections::HashMap;
 use std::convert::TryFrom;
 use tracing::{error, info, warn};
@@ -329,11 +329,6 @@ where
     }
     collections.stop_time_headsigns = headsigns;
     collections.stop_time_ids = stop_time_ids;
-    let mut vehicle_journeys = collections.vehicle_journeys.take();
-    for vj in &mut vehicle_journeys {
-        skip_error_and_error!(vj.sort_and_check_stop_times());
-    }
-    collections.vehicle_journeys = CollectionWithId::new(vehicle_journeys)?;
     Ok(())
 }
 
@@ -882,54 +877,4 @@ mod tests {
             assert_eq!(code.1, "source_code");
         });
     }
-    #[test]
-    fn stop_sequence_growing() {
-        test_in_tmp_dir(|path| {
-            let _ = generate_minimal_ntfs(path);
-            let stop_times_content = "stop_time_id,trip_id,arrival_time,departure_time,stop_id,stop_sequence,pickup_type,drop_off_type,shape_dist_traveled,stop_time_precision\n\
-            1,1,06:00:00,06:00:00,sp:01,1,0,0,,0\n\
-            2,1,06:06:27,06:06:27,sp:02,2,2,1,,1\n\
-            3,1,06:07:27,06:06:27,sp:03,3,2,1,,2\n\
-            4,1,06:08:27,06:06:27,sp:04,3,2,1,,\n\
-            5,1,06:09:27,06:06:27,sp:05,3,2,1,,";
-            create_file_with_content(path, "stop_times.txt", stop_times_content);
-
-            testing_logger::setup();
-            make_collection(path);
-            testing_logger::validate(|captured_logs| {
-                let error_log = captured_logs
-                    .iter()
-                    .find(|captured_log| captured_log.level == tracing::log::Level::Error)
-                    .expect("log error expected");
-                assert!(error_log
-                    .body
-                    .contains("duplicate stop_sequence \'3\' for the trip \'1\'"));
-            });
-        });
-    }
-    #[test]
-    fn stop_times_growing() {
-        test_in_tmp_dir(|path| {
-            let _ = generate_minimal_ntfs(path);
-            let stop_times_content = "stop_time_id,trip_id,arrival_time,departure_time,stop_id,stop_sequence,pickup_type,drop_off_type,shape_dist_traveled,stop_time_precision\n\
-            1,1,06:00:00,06:00:00,sp:01,1,0,0,,0\n\
-            2,1,06:06:27,06:06:27,sp:02,2,2,1,,1\n\
-            3,1,06:06:00,06:06:27,sp:03,3,2,1,,2\n\
-            4,1,06:06:27,06:06:27,sp:04,4,2,1,,\n\
-            5,1,06:06:27,06:06:27,sp:05,5,2,1,,";
-            create_file_with_content(path, "stop_times.txt", stop_times_content);
-
-            testing_logger::setup();
-            make_collection(path);
-            testing_logger::validate(|captured_logs| {
-                let error_log = captured_logs
-                    .iter()
-                    .find(|captured_log| captured_log.level == tracing::log::Level::Error)
-                    .expect("log error expected");
-                assert!(error_log.body.contains(
-                    "incoherent stop times \'2\' at time \'06:06:27\' for the trip \'1\'"
-                ));
-            });
-        });
-    }
 }

src/objects.rs

@@ -2042,4 +2042,85 @@ mod tests {
             epsilon = EPSILON
         );
     }
+
+    mod sort_and_check_stop_times {
+        use super::*;
+
+        fn generate_stop_times(configs: Vec<(u32, &str, &str)>) -> Vec<StopTime> {
+            let stop_points = typed_index_collection::CollectionWithId::from(StopPoint {
+                id: "sp1".to_string(),
+                ..Default::default()
+            });
+            let stop_point_idx = stop_points.get_idx("sp1").unwrap();
+            configs
+                .into_iter()
+                .map(|(sequence, arrival, departure)| StopTime {
+                    stop_point_idx,
+                    sequence,
+                    arrival_time: Time::from_str(arrival).unwrap(),
+                    departure_time: Time::from_str(departure).unwrap(),
+                    boarding_duration: 0,
+                    alighting_duration: 0,
+                    pickup_type: 0,
+                    drop_off_type: 0,
+                    datetime_estimated: false,
+                    local_zone_id: None,
+                    precision: None,
+                })
+                .collect()
+        }
+
+        #[test]
+        fn stop_sequence_growing() {
+            let stop_times = generate_stop_times(vec![
+                (1, "06:00:00", "06:00:00"),
+                (2, "06:06:27", "06:06:27"),
+                (3, "06:07:27", "06:07:27"),
+                (3, "06:08:27", "06:08:27"),
+                (3, "06:09:27", "06:09:27"),
+            ]);
+            let mut vehicle_journey = VehicleJourney {
+                id: "vj1".to_string(),
+                stop_times,
+                ..Default::default()
+            };
+            let error = vehicle_journey.sort_and_check_stop_times().unwrap_err();
+            let _expected_vj_id = "vj1".to_string();
+            let _expected_duplicated_sequence = 2;
+            assert!(matches!(
+                error,
+                StopTimeError::DuplicateStopSequence {
+                    vj_id: _expected_vj_id,
+                    duplicated_sequence: _expected_duplicated_sequence,
+                }
+            ));
+        }
+        #[test]
+        fn stop_times_growing() {
+            let stop_times = generate_stop_times(vec![
+                (1, "06:00:00", "06:00:00"),
+                (2, "06:06:27", "06:06:27"),
+                (3, "06:06:00", "06:06:27"),
+                (4, "06:06:27", "06:06:27"),
+                (5, "06:06:27", "06:06:27"),
+            ]);
+            let mut vehicle_journey = VehicleJourney {
+                id: "vj1".to_string(),
+                stop_times,
+                ..Default::default()
+            };
+            let error = vehicle_journey.sort_and_check_stop_times().unwrap_err();
+            let _expected_vj_id = "vj1".to_string();
+            let _expected_first_incorrect_sequence = 2;
+            let _expected_first_incorrect_time = Time::new(6, 6, 27);
+            assert!(matches!(
+                error,
+                StopTimeError::IncoherentStopTimes {
+                    vj_id: _expected_vj_id,
+                    first_incorrect_sequence: _expected_first_incorrect_sequence,
+                    first_incorrect_time: _expected_first_incorrect_time,
+                },
+            ));
+        }
+    }
 }

src/utils.rs

@@ -121,7 +121,7 @@ where
     T: serde::Deserialize<'de>,
 {
     use serde::Deserialize;
-    Option::<T>::deserialize(de).map(|opt| opt.unwrap_or_else(Default::default))
+    Option::<T>::deserialize(de).map(|opt| opt.unwrap_or_default())
 }
 
 pub fn ser_option_u32_with_default<S>(value: &Option<u32>, serializer: S) -> Result<S::Ok, S::Error>
@@ -147,7 +147,7 @@ pub fn de_without_slashes<'de, D>(deserializer: D) -> Result<String, D::Error>
 where
     D: serde::Deserializer<'de>,
 {
-    de_option_without_slashes(deserializer).map(|opt| opt.unwrap_or_else(Default::default))
+    de_option_without_slashes(deserializer).map(|opt| opt.unwrap_or_default())
 }
 
 pub fn de_option_without_slashes<'de, D>(de: D) -> Result<Option<String>, D::Error>
@@ -165,7 +165,7 @@ where
     Option<T>: serde::Deserialize<'de>,
     T: Default,
 {
-    de_with_invalid_option(de).map(|opt| opt.unwrap_or_else(Default::default))
+    de_with_invalid_option(de).map(|opt| opt.unwrap_or_default())
 }
 
 pub fn de_wkt<'de, D>(deserializer: D) -> Result<geo::Geometry<f64>, D::Error>