refactor(chat-history-model): Use sections as day dividers

Using sections simplifies our code a lot. It still needs changes applied
to the reversed list patch, otherwise sections are shifted forward by
one position.

Author: marhkb

src/model/chat_history_item.rs

@@ -1,13 +1,12 @@
 use std::cell::Cell;
 use std::cell::RefCell;
-use std::cmp::Ordering;
 use std::collections::VecDeque;
 
 use gio::prelude::*;
-use gio::subclass::prelude::*;
 use glib::clone;
 use gtk::gio;
 use gtk::glib;
+use gtk::subclass::prelude::*;
 use once_cell::sync::Lazy;
 use thiserror::Error;
 
@@ -28,14 +27,14 @@ mod imp {
     pub(crate) struct ChatHistoryModel {
         pub(super) chat: glib::WeakRef<model::Chat>,
         pub(super) is_loading: Cell<bool>,
-        pub(super) list: RefCell<VecDeque<model::ChatHistoryItem>>,
+        pub(super) list: RefCell<VecDeque<model::Message>>,
     }
 
     #[glib::object_subclass]
     impl ObjectSubclass for ChatHistoryModel {
         const NAME: &'static str = "ChatHistoryModel";
         type Type = super::ChatHistoryModel;
-        type Interfaces = (gio::ListModel,);
+        type Interfaces = (gio::ListModel, gtk::SectionModel);
     }
 
     impl ObjectImpl for ChatHistoryModel {
@@ -60,7 +59,7 @@ mod imp {
 
     impl ListModelImpl for ChatHistoryModel {
         fn item_type(&self) -> glib::Type {
-            model::ChatHistoryItem::static_type()
+            model::Message::static_type()
         }
 
         fn n_items(&self) -> u32 {
@@ -75,6 +74,44 @@ mod imp {
                 .cloned()
         }
     }
+
+    impl SectionModelImpl for ChatHistoryModel {
+        fn section(&self, position: u32) -> (u32, u32) {
+            let list = &*self.list.borrow();
+            let message = list.get(position as usize).unwrap();
+
+            let ymd = glib::DateTime::from_unix_local(message.date() as i64)
+                .unwrap()
+                .ymd();
+
+            (
+                if position == 0 {
+                    0
+                } else {
+                    (0..position)
+                        .rev()
+                        .find(|i| {
+                            ymd != glib::DateTime::from_unix_local(
+                                list.get(*i as usize).unwrap().date() as i64,
+                            )
+                            .unwrap()
+                            .ymd()
+                        })
+                        .map(|i| i + 1)
+                        .unwrap_or(0)
+                },
+                (position + 1..list.len() as u32)
+                    .find(|i| {
+                        ymd != glib::DateTime::from_unix_local(
+                            list.get(*i as usize).unwrap().date() as i64,
+                        )
+                        .unwrap()
+                        .ymd()
+                    })
+                    .unwrap_or(list.len() as u32),
+            )
+        }
+    }
 }
 
 glib::wrapper! {
@@ -108,14 +145,7 @@ impl ChatHistoryModel {
             return Err(ChatHistoryError::AlreadyLoading);
         }
 
-        let oldest_message_id = imp
-            .list
-            .borrow()
-            .iter()
-            .rev()
-            .find_map(|item| item.message())
-            .map(|m| m.id())
-            .unwrap_or_default();
+        let oldest_message_id = imp.list.borrow().back().map(|m| m.id()).unwrap_or_default();
 
         imp.is_loading.set(true);
 
@@ -133,167 +163,36 @@ impl ChatHistoryModel {
         Ok(true)
     }
 
-    fn items_changed(&self, position: u32, removed: u32, added: u32) {
-        let imp = self.imp();
-
-        // Insert day dividers where needed
-        let added = {
-            let position = position as usize;
-            let added = added as usize;
-
-            let mut list = imp.list.borrow_mut();
-            let mut previous_timestamp = if position + 1 < list.len() {
-                list.get(position + 1)
-                    .and_then(|item| item.message_timestamp())
-            } else {
-                None
-            };
-            let mut dividers: Vec<(usize, model::ChatHistoryItem)> = vec![];
-
-            for (index, current) in list.range(position..position + added).enumerate().rev() {
-                if let Some(current_timestamp) = current.message_timestamp() {
-                    if Some(current_timestamp.ymd()) != previous_timestamp.as_ref().map(|t| t.ymd())
-                    {
-                        let divider_pos = position + index + 1;
-                        dividers.push((
-                            divider_pos,
-                            model::ChatHistoryItem::for_day_divider(current_timestamp.clone()),
-                        ));
-                        previous_timestamp = Some(current_timestamp);
-                    }
-                }
-            }
-
-            let dividers_len = dividers.len();
-            for (position, item) in dividers {
-                list.insert(position, item);
-            }
-
-            (added + dividers_len) as u32
-        };
-
-        // Check and remove no more needed day divider after removing messages
-        let removed = {
-            let mut removed = removed as usize;
-
-            if removed > 0 {
-                let mut list = imp.list.borrow_mut();
-                let position = position as usize;
-                let item_before_removed = list.get(position);
-
-                if let Some(model::ChatHistoryItemType::DayDivider(_)) =
-                    item_before_removed.map(|i| i.type_())
-                {
-                    let item_after_removed = if position > 0 {
-                        list.get(position - 1)
-                    } else {
-                        None
-                    };
-
-                    match item_after_removed.map(|item| item.type_()) {
-                        None | Some(model::ChatHistoryItemType::DayDivider(_)) => {
-                            list.remove(position + removed);
-
-                            removed += 1;
-                        }
-                        _ => {}
-                    }
-                }
-            }
-
-            removed as u32
-        };
-
-        // Check and remove no more needed day divider after adding messages
-        let (position, removed) = {
-            let mut removed = removed;
-            let mut position = position as usize;
-
-            if added > 0 && position > 0 {
-                let mut list = imp.list.borrow_mut();
-                let last_added_timestamp = list.get(position).unwrap().message_timestamp().unwrap();
-                let next_item = list.get(position - 1);
-
-                if let Some(model::ChatHistoryItemType::DayDivider(date)) =
-                    next_item.map(|item| item.type_())
-                {
-                    if date.ymd() == last_added_timestamp.ymd() {
-                        list.remove(position - 1);
-
-                        removed += 1;
-                        position -= 1;
-                    }
-                }
-            }
-
-            (position as u32, removed)
-        };
-
-        self.upcast_ref::<gio::ListModel>()
-            .items_changed(position, removed, added);
-    }
-
     fn push_front(&self, message: model::Message) {
-        self.imp()
-            .list
-            .borrow_mut()
-            .push_front(model::ChatHistoryItem::for_message(message));
+        self.imp().list.borrow_mut().push_front(message);
 
         self.items_changed(0, 0, 1);
     }
 
     fn append(&self, messages: Vec<model::Message>) {
         let imp = self.imp();
+
         let added = messages.len();
 
         imp.list.borrow_mut().reserve(added);
 
         for message in messages {
-            imp.list
-                .borrow_mut()
-                .push_back(model::ChatHistoryItem::for_message(message));
+            imp.list.borrow_mut().push_back(message);
         }
 
         let index = imp.list.borrow().len() - added;
         self.items_changed(index as u32, 0, added as u32);
     }
 
     fn remove(&self, message: model::Message) {
-        let imp = self.imp();
-
-        // Put this in a block, so that we only need to borrow the list once and the runtime
-        // borrow checker does not panic in Self::items_changed when it borrows the list again.
-        let index = {
-            let mut list = imp.list.borrow_mut();
-
-            // The elements in this list are ordered. While the day dividers are ordered
-            // only by their date time, the messages are additionally sorted by their id. We
-            // can exploit this by applying a binary search.
-            let index = list
-                .binary_search_by(|m| match m.type_() {
-                    model::ChatHistoryItemType::Message(other_message) => {
-                        message.id().cmp(&other_message.id())
-                    }
-                    model::ChatHistoryItemType::DayDivider(date_time) => {
-                        let ordering = glib::DateTime::from_unix_utc(message.date() as i64)
-                            .unwrap()
-                            .cmp(date_time);
-                        if let Ordering::Equal = ordering {
-                            // We found the day divider of the message. Therefore, the message
-                            // must be among the following elements.
-                            Ordering::Greater
-                        } else {
-                            ordering
-                        }
-                    }
-                })
-                .unwrap();
+        let mut list = self.imp().list.borrow_mut();
 
+        if let Ok(index) = list.binary_search_by(|m| message.id().cmp(&m.id())) {
             list.remove(index);
-            index as u32
-        };
 
-        self.items_changed(index, 1, 0);
+            drop(list);
+            self.items_changed(index as u32, 1, 0);
+        }
     }
 
     pub(crate) fn chat(&self) -> model::Chat {

src/model/chat_history_model.rs

@@ -3,7 +3,6 @@ mod basic_group;
 mod chat;
 mod chat_action;
 mod chat_action_list;
-mod chat_history_item;
 mod chat_history_model;
 mod chat_list;
 mod chat_list_item;
@@ -52,8 +51,6 @@ pub(crate) use self::chat::Chat;
 pub(crate) use self::chat::ChatType;
 pub(crate) use self::chat_action::ChatAction;
 pub(crate) use self::chat_action_list::ChatActionList;
-pub(crate) use self::chat_history_item::ChatHistoryItem;
-pub(crate) use self::chat_history_item::ChatHistoryItemType;
 pub(crate) use self::chat_history_model::ChatHistoryError;
 pub(crate) use self::chat_history_model::ChatHistoryModel;
 pub(crate) use self::chat_list::ChatList;