cleaning

gpx-rs/engine/src/core/gpx/chunk.rs

@@ -1,7 +1,7 @@
-use uuid::Uuid;
-
 use crate::{Trackpoint, Waypoint};
 
+use super::common::uuid_id;
+
 /// A run of items, shared between the successive versions of a [`crate::Chunked`] list: a chunk
 /// that is not touched by an edit is not copied.
 ///
@@ -16,6 +16,9 @@ pub trait Chunk {
     /// A chunk with a new identity, holding `items`.
     fn new(items: Vec<Self::Item>) -> Self;
 
+    /// Gives the chunk a new identity, after its items were changed in place.
+    fn renew(&mut self);
+
     fn items(&self) -> &Vec<Self::Item>;
 
     fn items_mut(&mut self) -> &mut Vec<Self::Item>;
@@ -27,14 +30,7 @@ pub trait Chunk {
 
 const MAX_TRKPT_CHUNK_SIZE: usize = 4096;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct TrackpointChunkId(pub Uuid);
-
-impl Default for TrackpointChunkId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(TrackpointChunkId);
 
 #[derive(Debug, Default)]
 pub struct TrackpointChunk {
@@ -53,6 +49,10 @@ impl Chunk for TrackpointChunk {
         }
     }
 
+    fn renew(&mut self) {
+        self.id = Default::default();
+    }
+
     fn items(&self) -> &Vec<Trackpoint> {
         &self.trkpt
     }
@@ -62,14 +62,7 @@ impl Chunk for TrackpointChunk {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct WaypointChunkId(pub Uuid);
-
-impl Default for WaypointChunkId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(WaypointChunkId);
 
 const MAX_WPT_CHUNK_SIZE: usize = 128;
 
@@ -90,6 +83,10 @@ impl Chunk for WaypointChunk {
         }
     }
 
+    fn renew(&mut self) {
+        self.id = Default::default();
+    }
+
     fn items(&self) -> &Vec<Waypoint> {
         &self.wpt
     }

gpx-rs/engine/src/core/gpx/chunked.rs

@@ -148,7 +148,10 @@ impl<C: Chunk> Chunked<C> {
     fn replace_chunk(&mut self, chunk: usize, f: impl FnOnce(&mut Vec<C::Item>)) {
         let len = self.chunks[chunk].items().len();
         match Rc::get_mut(&mut self.chunks[chunk]) {
-            Some(unique) => f(unique.items_mut()),
+            Some(unique) => {
+                f(unique.items_mut());
+                unique.renew();
+            }
             None => {
                 let mut items = self.chunks[chunk].items().clone();
                 f(&mut items);
@@ -291,7 +294,9 @@ impl<'a, C: Chunk> IntoIterator for &'a Chunked<C> {
 
 pub struct ChunkedIter<'a, C: Chunk> {
     chunked: &'a Chunked<C>,
+    /// The item returned last.
     idx: Option<ChunkIndex>,
+    exhausted: bool,
 }
 
 impl<C: Chunk> Clone for ChunkedIter<'_, C> {
@@ -299,6 +304,7 @@ impl<C: Chunk> Clone for ChunkedIter<'_, C> {
         Self {
             chunked: self.chunked,
             idx: self.idx,
+            exhausted: self.exhausted,
         }
     }
 }
@@ -307,22 +313,307 @@ impl<'a, C: Chunk> ChunkedIter<'a, C> {
     pub fn new(chunked: &'a Chunked<C>) -> Self {
         Self {
             chunked,
-            idx: Default::default(),
+            idx: None,
+            exhausted: false,
         }
     }
+
+    fn move_to(&mut self, idx: Option<ChunkIndex>) -> Option<&'a C::Item> {
+        self.exhausted = idx.is_none();
+        self.idx = idx.or(self.idx);
+        idx.map(|idx| &self.chunked[idx])
+    }
 }
 
 impl<'a, C: Chunk> Iterator for ChunkedIter<'a, C> {
     type Item = &'a C::Item;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.idx = self.chunked.next_index(self.idx);
-        self.idx.map(|idx| &self.chunked[idx])
+        if self.exhausted {
+            return None;
+        }
+        let next = self.chunked.next_index(self.idx);
+        self.move_to(next)
     }
 
     fn nth(&mut self, n: usize) -> Option<Self::Item> {
-        let idx = self.idx.map_or_default(|idx| idx.flat) + n;
-        self.idx = self.chunked.locate(idx);
-        self.idx.map(|idx| &self.chunked[idx])
+        if self.exhausted {
+            return None;
+        }
+        let target = self.idx.map_or(0, |idx| idx.flat + 1).saturating_add(n);
+        let next = self.chunked.locate(target);
+        self.move_to(next)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Chunks of at most 3 numbers, with an identity like the real ones.
+    #[derive(Debug)]
+    struct Numbers {
+        id: usize,
+        items: Vec<u32>,
+    }
+
+    thread_local! {
+        static NEXT_ID: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+    }
+
+    fn next_id() -> usize {
+        NEXT_ID.with(|id| id.replace(id.get() + 1))
+    }
+
+    impl Chunk for Numbers {
+        type Item = u32;
+        const MAX_SIZE: usize = 3;
+
+        fn new(items: Vec<u32>) -> Self {
+            Self {
+                id: next_id(),
+                items,
+            }
+        }
+
+        fn renew(&mut self) {
+            self.id = next_id();
+        }
+
+        fn items(&self) -> &Vec<u32> {
+            &self.items
+        }
+
+        fn items_mut(&mut self) -> &mut Vec<u32> {
+            &mut self.items
+        }
+    }
+
+    /// The numbers `0..n` in chunks of 3.
+    fn numbers(n: u32) -> Chunked<Numbers> {
+        let mut chunked = Chunked::default();
+        for start in (0..n).step_by(3) {
+            chunked.push(Numbers::new((start..(start + 3).min(n)).collect()));
+        }
+        chunked
+    }
+
+    fn values(chunked: &Chunked<Numbers>) -> Vec<u32> {
+        chunked.iter().copied().collect()
+    }
+
+    fn ids(chunked: &Chunked<Numbers>) -> Vec<usize> {
+        chunked.chunks().iter().map(|chunk| chunk.id).collect()
+    }
+
+    fn assert_consistent(chunked: &Chunked<Numbers>) {
+        let sizes: Vec<usize> = chunked.chunks().iter().map(|c| c.items.len()).collect();
+        assert!(
+            sizes
+                .iter()
+                .all(|&size| 0 < size && size <= Numbers::MAX_SIZE)
+        );
+        assert_eq!(chunked.len(), sizes.iter().sum::<usize>());
+        for index in 0..chunked.len() {
+            assert_eq!(chunked.locate(index).unwrap().flat, index);
+        }
+        assert!(chunked.locate(chunked.len()).is_none());
+    }
+
+    #[test]
+    fn test_push_drops_empty_chunks() {
+        let mut chunked = numbers(4);
+        chunked.push(Numbers::new(vec![]));
+        assert_eq!(chunked.chunks().len(), 2);
+        assert_eq!(chunked.len(), 4);
+        assert!(Chunked::<Numbers>::default().is_empty());
+        assert!(!chunked.is_empty());
+    }
+
+    #[test]
+    fn test_locate_index_and_iterate() {
+        let chunked = numbers(7);
+        assert_eq!(
+            chunked.locate(4),
+            Some(ChunkIndex {
+                chunk: 1,
+                pos: 1,
+                flat: 4
+            })
+        );
+        assert_eq!(chunked[4], 4);
+        assert_eq!(chunked[chunked.locate(6).unwrap()], 6);
+        assert!(chunked.locate(7).is_none());
+        assert_eq!(values(&chunked), (0..7).collect::<Vec<_>>());
+        assert_eq!(chunked.iter().count(), 7);
+        assert_eq!((&chunked).into_iter().last(), Some(&6));
+        assert_eq!(values(&Chunked::default()), Vec::<u32>::new());
+    }
+
+    #[test]
+    fn test_first_last_next_and_previous_index() {
+        let chunked = numbers(7);
+        let first = chunked.first_index().unwrap();
+        let last = chunked.last_index().unwrap();
+        assert_eq!((first.flat, last.flat), (0, 6));
+        assert_eq!((last.chunk, last.pos), (2, 0));
+
+        let mut forward = vec![];
+        let mut cur = None;
+        while let Some(next) = chunked.next_index(cur) {
+            forward.push(chunked[next]);
+            cur = Some(next);
+        }
+        assert_eq!(forward, (0..7).collect::<Vec<_>>());
+
+        let mut backward = vec![];
+        let mut cur = None;
+        while let Some(prev) = chunked.prev_index(cur) {
+            backward.push(chunked[prev]);
+            cur = Some(prev);
+        }
+        assert_eq!(backward, (0..7).rev().collect::<Vec<_>>());
+
+        let empty = Chunked::<Numbers>::default();
+        assert!(empty.first_index().is_none() && empty.last_index().is_none());
+    }
+
+    #[test]
+    #[allow(clippy::iter_nth_zero)]
+    fn test_iterator_nth_goes_on_from_the_current_item() {
+        let chunked = numbers(10);
+        let mut iter = chunked.iter();
+        assert_eq!(iter.nth(1), Some(&1));
+        assert_eq!(iter.next(), Some(&2));
+        // skips 3 and 4
+        assert_eq!(iter.nth(2), Some(&5));
+        assert_eq!(iter.nth(0), Some(&6));
+        assert_eq!(iter.nth(2), Some(&9));
+        assert_eq!(iter.nth(0), None);
+        assert_eq!(iter.next(), None, "an exhausted iterator stays exhausted");
+
+        assert_eq!(values(&numbers(10)).iter().skip(4).count(), 6);
+        assert_eq!(
+            chunked.iter().skip(4).copied().collect::<Vec<_>>(),
+            (4..10).collect::<Vec<_>>()
+        );
+        assert_eq!(
+            chunked.iter().step_by(4).copied().collect::<Vec<_>>(),
+            vec![0, 4, 8]
+        );
+    }
+
+    #[test]
+    fn test_splice_replaces_inserts_deletes_and_appends() {
+        let mut chunked = numbers(10);
+        chunked.splice(2, 5, vec![100, 101]);
+        assert_eq!(values(&chunked), [0, 1, 100, 101, 5, 6, 7, 8, 9]);
+        assert_consistent(&chunked);
+
+        chunked.splice(0, 0, vec![50]);
+        assert_eq!(values(&chunked)[..3], [50, 0, 1]);
+        chunked.splice(chunked.len(), chunked.len(), vec![7, 7, 7, 7]);
+        assert_eq!(values(&chunked)[chunked.len() - 5..], [9, 7, 7, 7, 7]);
+        assert_consistent(&chunked);
+
+        let len = chunked.len();
+        chunked.splice(0, len, vec![]);
+        assert!(chunked.is_empty());
+        assert_eq!(chunked.len(), 0);
+    }
+
+    #[test]
+    #[should_panic(expected = "splice range out of bounds")]
+    fn test_splice_out_of_bounds_panics() {
+        numbers(4).splice(2, 5, vec![]);
+    }
+
+    #[test]
+    fn test_splice_keeps_the_chunks_it_does_not_touch() {
+        let mut chunked = numbers(12);
+        let before = ids(&chunked);
+        // inside the second chunk
+        chunked.splice(4, 5, vec![40]);
+        let after = ids(&chunked);
+        assert_eq!(after.len(), before.len());
+        assert_eq!(
+            (after[0], after[2], after[3]),
+            (before[0], before[2], before[3])
+        );
+        assert_ne!(after[1], before[1]);
+    }
+
+    #[test]
+    fn test_edit_replaces_only_the_changed_chunks() {
+        let mut chunked = numbers(9);
+        let before = ids(&chunked);
+
+        // nothing is changed: nothing is replaced
+        assert!(!chunked.edit(|n| *n == 4, |_| false));
+        assert_eq!(ids(&chunked), before);
+
+        // the filter picks the chunk, the hook changes it
+        assert!(chunked.edit(
+            |n| *n == 4,
+            |items| {
+                items[1] = 400;
+                true
+            }
+        ));
+        let after = ids(&chunked);
+        assert_eq!((after[0], after[2]), (before[0], before[2]));
+        assert_ne!(after[1], before[1]);
+        assert_eq!(values(&chunked), [0, 1, 2, 3, 400, 5, 6, 7, 8]);
+
+        // a chunk left empty is dropped
+        assert!(chunked.edit(
+            |n| *n == 0,
+            |items| {
+                items.clear();
+                true
+            }
+        ));
+        assert_eq!(chunked.chunks().len(), 2);
+        assert_consistent(&chunked);
+    }
+
+    #[test]
+    fn test_update_copies_the_chunk_so_that_other_versions_do_not_change() {
+        let mut chunked = numbers(6);
+        let version = chunked.clone();
+        let before = ids(&chunked);
+        chunked.update(4, |n| *n = 40);
+        assert_eq!(values(&chunked), [0, 1, 2, 3, 40, 5]);
+        assert_eq!(values(&version), (0..6).collect::<Vec<_>>());
+        assert_eq!(ids(&version), before);
+        assert_eq!(ids(&chunked)[0], before[0]);
+        assert_ne!(ids(&chunked)[1], before[1]);
+    }
+
+    #[test]
+    fn test_updated_chunks_get_a_new_identity_even_when_nothing_shares_them() {
+        // what is derived from or kept of a chunk is keyed by its identity
+        let mut chunked = numbers(6);
+        let before = ids(&chunked);
+        chunked.update(1, |n| *n += 1);
+        assert_ne!(ids(&chunked)[0], before[0]);
+        assert_eq!(ids(&chunked)[1], before[1]);
+
+        let before = ids(&chunked);
+        chunked.update_all(|_, n| *n += 1);
+        let after = ids(&chunked);
+        assert!(before.iter().zip(&after).all(|(a, b)| a != b));
+    }
+
+    #[test]
+    fn test_update_all_gives_the_index_of_each_item() {
+        let mut chunked = numbers(8);
+        let mut seen = vec![];
+        chunked.update_all(|index, n| {
+            seen.push(index);
+            *n = 10 * index as u32;
+        });
+        assert_eq!(seen, (0..8).collect::<Vec<_>>());
+        assert_eq!(values(&chunked), [0, 10, 20, 30, 40, 50, 60, 70]);
     }
 }

gpx-rs/engine/src/core/gpx/common.rs

@@ -1,3 +1,20 @@
+/// Defines an identifier made of a UUID, which is a new one by default.
+macro_rules! uuid_id {
+    ($(#[$meta:meta])* $name:ident) => {
+        $(#[$meta])*
+        #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+        pub struct $name(pub uuid::Uuid);
+
+        impl Default for $name {
+            fn default() -> Self {
+                Self(uuid::Uuid::new_v4())
+            }
+        }
+    };
+}
+
+pub(crate) use uuid_id;
+
 #[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct Link {
     pub href: String,

gpx-rs/engine/src/core/gpx/file.rs

@@ -1,15 +1,8 @@
-use uuid::Uuid;
-
 use crate::{Link, Track, Waypoints};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct FileId(pub Uuid);
+use super::common::uuid_id;
 
-impl Default for FileId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(FileId);
 
 #[derive(Debug, Default, Clone)]
 pub struct File {

gpx-rs/engine/src/core/gpx/segment.rs

@@ -1,26 +1,13 @@
-use std::ops::{Deref, DerefMut};
-
-use uuid::Uuid;
+use std::ops::Deref;
+use std::rc::Rc;
 
 use crate::{ChunkIndex, Chunked, ChunkedIter, Trackpoint, TrackpointChunk, compute_anchors};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct TrackSegmentId(pub Uuid);
+use super::common::uuid_id;
 
-impl Default for TrackSegmentId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(TrackSegmentId);
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct TrackSegmentRevisionId(pub Uuid);
-
-impl Default for TrackSegmentRevisionId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(TrackSegmentRevisionId);
 
 #[derive(Debug, Default, Clone)]
 pub struct TrackSegment {
@@ -42,17 +29,37 @@ impl Deref for TrackSegment {
     }
 }
 
-impl DerefMut for TrackSegment {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.points
-    }
-}
-
 impl TrackSegment {
+    /// Adds a chunk after the last trackpoint, which is dropped if it is empty.
+    pub fn push(&mut self, chunk: TrackpointChunk) {
+        self.points.push(chunk);
+        self.rev_id = Default::default();
+    }
+
+    /// Adds a chunk that is shared with something else after the last trackpoint.
+    pub fn push_shared(&mut self, chunk: Rc<TrackpointChunk>) {
+        self.points.push_shared(chunk);
+        self.rev_id = Default::default();
+    }
+
+    /// Applies `f` to the trackpoint at `index`, see [`Chunked::update`]. Panics if there is no
+    /// such trackpoint.
+    pub fn update(&mut self, index: usize, f: impl FnOnce(&mut Trackpoint)) {
+        self.points.update(index, f);
+        self.rev_id = Default::default();
+    }
+
+    /// Applies `f` to every trackpoint, with its index, see [`Chunked::update_all`].
+    pub fn update_all(&mut self, f: impl FnMut(usize, &mut Trackpoint)) {
+        self.points.update_all(f);
+        self.rev_id = Default::default();
+    }
+
     /// Replaces the points in `start..end` by `points`, see [`Chunked::splice`]. The first and
     /// last trackpoints are anchors afterwards.
     pub fn splice(&mut self, start: usize, end: usize, points: Vec<Trackpoint>) {
         self.points.splice(start, end, points);
+        self.rev_id = Default::default();
         self.ensure_end_anchors();
     }
 
@@ -74,7 +81,7 @@ impl TrackSegment {
     /// Makes the trackpoint at `index` an anchor shown from the map zoom level `zoom`. Panics if
     /// there is no such trackpoint.
     pub fn set_anchor(&mut self, index: usize, zoom: u8) {
-        self.points.update(index, |trkpt| trkpt.anchor = Some(zoom));
+        self.update(index, |trkpt| trkpt.anchor = Some(zoom));
     }
 
     /// Sets the anchors of the trackpoints from the details of the path of the segment (see
@@ -82,7 +89,7 @@ impl TrackSegment {
     pub fn compute_anchors(&mut self) {
         let anchors = compute_anchors(self);
         let mut anchors = anchors.into_iter().peekable();
-        self.points.update_all(|index, trkpt| {
+        self.update_all(|index, trkpt| {
             trkpt.anchor = match anchors.peek() {
                 Some(&(anchor, zoom)) if anchor == index => {
                     anchors.next();
@@ -98,6 +105,8 @@ impl TrackSegment {
 mod tests {
     use std::rc::Rc;
 
+    use crate::Chunk;
+
     use super::*;
 
     fn create_track_segment(nb_chunks: usize) -> TrackSegment {
@@ -354,4 +363,49 @@ mod tests {
             idx = next;
         }
     }
+
+    fn points_of(n: usize) -> Vec<Trackpoint> {
+        (0..n).map(|_| Trackpoint::default()).collect()
+    }
+
+    #[test]
+    fn test_every_change_gives_a_new_revision() {
+        type Change = (&'static str, fn(&mut TrackSegment));
+        let changes: [Change; 7] = [
+            ("push", |s| s.push(TrackpointChunk::new(points_of(1)))),
+            ("push_shared", |s| {
+                s.push_shared(Rc::new(TrackpointChunk::new(points_of(1))))
+            }),
+            ("update", |s| s.update(0, |p| p.ele += 1.0)),
+            ("update_all", |s| s.update_all(|_, p| p.ele += 1.0)),
+            ("splice", |s| s.splice(1, 2, points_of(2))),
+            ("set_anchor", |s| s.set_anchor(1, 5)),
+            ("compute_anchors", |s| s.compute_anchors()),
+        ];
+        for (name, change) in changes {
+            let mut trkseg = create_track_segment(3);
+            let before = trkseg.rev_id;
+            change(&mut trkseg);
+            assert_ne!(trkseg.rev_id, before, "{name}");
+        }
+        // reading does not
+        let trkseg = create_track_segment(3);
+        let before = trkseg.rev_id;
+        let _ = trkseg.iter().count();
+        assert_eq!(trkseg.rev_id, before);
+    }
+
+    #[test]
+    fn test_ends_are_anchors_after_a_splice() {
+        let mut trkseg = create_track_segment(3);
+        trkseg.splice(0, 1, points_of(2));
+        let last = trkseg.len() - 1;
+        assert_eq!(trkseg[0].anchor, Some(0));
+        assert_eq!(trkseg[last].anchor, Some(0));
+
+        // nothing to anchor in an empty segment
+        let mut empty = TrackSegment::default();
+        empty.ensure_end_anchors();
+        assert!(empty.is_empty());
+    }
 }

gpx-rs/engine/src/core/gpx/track.rs

@@ -1,15 +1,8 @@
-use uuid::Uuid;
-
 use crate::{Link, TrackSegment};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct TrackId(pub Uuid);
+use super::common::uuid_id;
 
-impl Default for TrackId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(TrackId);
 
 #[derive(Debug, Default, Clone)]
 pub struct Track {

gpx-rs/engine/src/core/gpx/waypoint.rs

@@ -1,15 +1,8 @@
-use uuid::Uuid;
-
 use crate::{Link, LngLat};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
-pub struct WaypointId(pub Uuid);
+use super::common::uuid_id;
 
-impl Default for WaypointId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(WaypointId);
 
 #[derive(Debug, Default, Clone, serde::Serialize, serde::Deserialize)]
 pub struct Waypoint {

gpx-rs/engine/src/core/gpx/waypoints.rs

@@ -1,17 +1,10 @@
 use std::ops::Deref;
 
-use uuid::Uuid;
-
 use crate::{Chunked, Waypoint, WaypointChunk, WaypointId};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct FileWaypointsRevisionId(pub Uuid);
+use super::common::uuid_id;
 
-impl Default for FileWaypointsRevisionId {
-    fn default() -> Self {
-        Self(Uuid::new_v4())
-    }
-}
+uuid_id!(FileWaypointsRevisionId);
 
 /// The waypoints of a file, in chunks that are shared between the versions of the file (see
 /// [`Chunked`]).