GNOME.org
 

[fractal] backend: force id on every Message

commit 9b6f53786f9a15154008dc85da86734203b6767c
Author: Julian Sparber <julian sparber net>
Date:   Thu Jan 3 11:43:46 2019 +0100

    backend: force id on every Message
    
    Since every message has to have a ID, this drops the Option for id in
    the Message Model

 fractal-gtk/src/appop/message.rs        | 185 ++++++++++++++------------------
 fractal-gtk/src/cache/state.rs          |   5 +-
 fractal-gtk/src/widgets/media_viewer.rs |  11 +-
 fractal-gtk/src/widgets/roomlist.rs     |   5 +-
 fractal-matrix-api/src/backend/room.rs  |  26 +++--
 fractal-matrix-api/src/model/message.rs |  70 ++++++------
 fractal-matrix-api/src/model/room.rs    |   4 +-
 7 files changed, 130 insertions(+), 176 deletions(-)
---
diff --git a/fractal-gtk/src/appop/message.rs b/fractal-gtk/src/appop/message.rs
index 65872d5a..9ef59c7c 100644
--- a/fractal-gtk/src/appop/message.rs
+++ b/fractal-gtk/src/appop/message.rs
@@ -1,9 +1,8 @@
-use chrono::prelude::*;
 use comrak::{markdown_to_html, ComrakOptions};
 use gtk;
 use gtk::prelude::*;
 use lazy_static::lazy_static;
-use std::collections::HashMap;
+use log::error;
 use std::fs;
 use std::path::PathBuf;
 use tree_magic;
@@ -32,7 +31,7 @@ impl AppOp {
         let room = self.rooms.get(room_id)?;
         room.messages
             .iter()
-            .find(|m| m.id == Some(id.to_string()))
+            .find(|m| m.id == id.to_string())
             .cloned()
     }
 
@@ -127,7 +126,7 @@ impl AppOp {
             self.backend
                 .send(BKCommand::MarkAsRead(
                     last_message.room.clone(),
-                    last_message.id.clone()?,
+                    last_message.id.clone(),
                 ))
                 .unwrap();
         }
@@ -140,7 +139,7 @@ impl AppOp {
                 w.destroy();
             }
             m.widget = None;
-            m.msg.id = Some(evid);
+            m.msg.id = evid;
             self.show_room_messages(vec![m.msg.clone()]);
         }
         self.force_dequeue_message();
@@ -186,106 +185,80 @@ impl AppOp {
             return;
         }
 
-        let room = self.active_room.clone();
-        let now = Local::now();
-
-        let mtype = String::from("m.text");
-
-        let mut m = Message {
-            sender: self.uid.clone().unwrap_or_default(),
-            mtype: mtype,
-            body: msg.clone(),
-            room: room.clone().unwrap_or_default(),
-            date: now,
-            thumb: None,
-            url: None,
-            id: None,
-            formatted_body: None,
-            format: None,
-            source: None,
-            receipt: HashMap::new(),
-            redacted: false,
-            in_reply_to: None,
-            extra_content: None,
-        };
+        if let Some(room) = self.active_room.clone() {
+            if let Some(sender) = self.uid.clone() {
+                let body = msg.clone();
+                let mtype = String::from("m.text");
+                let mut m = Message::new(room, sender, body, mtype);
 
-        if msg.starts_with("/me ") {
-            m.body = msg.trim_left_matches("/me ").to_owned();
-            m.mtype = String::from("m.emote");
-        }
+                if msg.starts_with("/me ") {
+                    m.body = msg.trim_left_matches("/me ").to_owned();
+                    m.mtype = String::from("m.emote");
+                }
 
-        // Riot does not properly show emotes with Markdown;
-        // Emotes with markdown have a newline after the username
-        if m.mtype != "m.emote" && self.md_enabled {
-            let mut md_parsed_msg = markdown_to_html(&msg, &ComrakOptions::default());
+                // Riot does not properly show emotes with Markdown;
+                // Emotes with markdown have a newline after the username
+                if m.mtype != "m.emote" && self.md_enabled {
+                    let mut md_parsed_msg = markdown_to_html(&msg, &ComrakOptions::default());
+
+                    // Removing wrap tag: <p>..</p>\n
+                    let limit = md_parsed_msg.len() - 5;
+                    let trim = match (md_parsed_msg.get(0..3), md_parsed_msg.get(limit..)) {
+                        (Some(open), Some(close)) if open == "<p>" && close == "</p>\n" => true,
+                        _ => false,
+                    };
+                    if trim {
+                        md_parsed_msg = md_parsed_msg
+                            .get(3..limit)
+                            .unwrap_or(&md_parsed_msg)
+                            .to_string();
+                    }
 
-            // Removing wrap tag: <p>..</p>\n
-            let limit = md_parsed_msg.len() - 5;
-            let trim = match (md_parsed_msg.get(0..3), md_parsed_msg.get(limit..)) {
-                (Some(open), Some(close)) if open == "<p>" && close == "</p>\n" => true,
-                _ => false,
-            };
-            if trim {
-                md_parsed_msg = md_parsed_msg
-                    .get(3..limit)
-                    .unwrap_or(&md_parsed_msg)
-                    .to_string();
-            }
+                    if md_parsed_msg != msg {
+                        m.formatted_body = Some(md_parsed_msg);
+                        m.format = Some(String::from("org.matrix.custom.html"));
+                    }
+                }
 
-            if md_parsed_msg != msg {
-                m.formatted_body = Some(md_parsed_msg);
-                m.format = Some(String::from("org.matrix.custom.html"));
+                self.add_tmp_room_message(m);
+                self.dequeue_message();
+            } else {
+                error!("Can't send message: No user is logged in");
             }
+        } else {
+            error!("Can't send message: No active room");
         }
-
-        m.id = Some(m.get_txn_id());
-        self.add_tmp_room_message(m.clone());
-        self.dequeue_message();
     }
 
-    pub fn attach_message(&mut self, path: PathBuf) -> Option<()> {
-        let now = Local::now();
-        let room = self.active_room.clone()?;
-        let mime = tree_magic::from_filepath(&path);
-        let mtype = match mime.as_ref() {
-            "image/gif" => "m.image",
-            "image/png" => "m.image",
-            "image/jpeg" => "m.image",
-            "image/jpg" => "m.image",
-            _ => "m.file",
-        };
-        let body = path.file_name().and_then(|s| s.to_str());
-        let path_string = path.to_str()?.to_string();
-
-        let info = match mtype {
-            "m.image" => get_image_media_info(&path_string, mime.as_ref()),
-            _ => None,
-        };
-
-        // TODO: write constructor for Message
-        let mut m = Message {
-            sender: self.uid.clone()?,
-            mtype: mtype.to_string(),
-            body: body?.to_string(),
-            room,
-            date: now,
-            thumb: None,
-            url: Some(path_string),
-            id: None,
-            formatted_body: None,
-            format: None,
-            source: None,
-            receipt: HashMap::new(),
-            redacted: false,
-            in_reply_to: None,
-            extra_content: info,
-        };
-
-        m.id = Some(m.get_txn_id());
-        self.add_tmp_room_message(m);
-        self.dequeue_message();
-
-        Some(())
+    pub fn attach_message(&mut self, path: PathBuf) {
+        if let Some(room) = self.active_room.clone() {
+            if let Some(sender) = self.uid.clone() {
+                let mime = tree_magic::from_filepath(&path);
+                let mtype = match mime.as_ref() {
+                    "image/gif" => "m.image",
+                    "image/png" => "m.image",
+                    "image/jpeg" => "m.image",
+                    "image/jpg" => "m.image",
+                    _ => "m.file",
+                };
+                let body = path
+                    .file_name()
+                    .and_then(|s| s.to_str())
+                    .unwrap_or_default();
+                let path_string = path.to_str().unwrap_or_default();
+
+                let mut m = Message::new(room, sender, body.to_string(), mtype.to_string());
+                if mtype == "m.image" {
+                    m.extra_content = get_image_media_info(path_string, mime.as_ref());
+                }
+                self.add_tmp_room_message(m);
+                self.dequeue_message();
+            } else {
+                error!("Can't send message: No user is logged in");
+            }
+        } else {
+            error!("Can't send message: No active room");
+        }
     }
 
     /// This method is called when a tmp message with an attach is sent correctly
@@ -321,15 +294,13 @@ impl AppOp {
                     || self.rooms.get(&msg.room).map_or(false, |r| r.direct));
 
             if should_notify {
-                if let Some(ref id) = msg.id {
-                    let window: gtk::Window = self
-                        .ui
-                        .builder
-                        .get_object("main_window")
-                        .expect("Can't find main_window in ui file.");
-                    if let Some(app) = window.get_application() {
-                        self.notify(app, &msg.room, id);
-                    }
+                let window: gtk::Window = self
+                    .ui
+                    .builder
+                    .get_object("main_window")
+                    .expect("Can't find main_window in ui file.");
+                if let Some(app) = window.get_application() {
+                    self.notify(app, &msg.room, &msg.id);
                 }
             }
 
@@ -465,7 +436,7 @@ fn create_ui_message(
 ) -> MessageContent {
     MessageContent {
         msg: msg.clone(),
-        id: msg.id.unwrap_or_default(),
+        id: msg.id,
         sender: msg.sender,
         sender_name: name,
         mtype: t,
diff --git a/fractal-gtk/src/cache/state.rs b/fractal-gtk/src/cache/state.rs
index daf1314b..e37fec49 100644
--- a/fractal-gtk/src/cache/state.rs
+++ b/fractal-gtk/src/cache/state.rs
@@ -84,10 +84,7 @@ impl Model for AppRoom {
 
 impl Model for AppMsg {
     fn key(&self) -> String {
-        // messages should have always an ID to be stored in the cache
-        // in other case we'll store with the "msg:room:" key.
-        let id = self.msg.id.clone().unwrap_or_default();
-        format!("msg:{}:{}", self.msg.room, id)
+        format!("msg:{}:{}", self.msg.room, self.msg.id)
     }
 }
 
diff --git a/fractal-gtk/src/widgets/media_viewer.rs b/fractal-gtk/src/widgets/media_viewer.rs
index 7183f1b5..c40ec828 100644
--- a/fractal-gtk/src/widgets/media_viewer.rs
+++ b/fractal-gtk/src/widgets/media_viewer.rs
@@ -351,14 +351,7 @@ impl MediaViewer {
 
         let current_media_index = media_list
             .iter()
-            .position(|media| {
-                media.id.clone().map_or(false, |media_id| {
-                    current_media_msg
-                        .id
-                        .clone()
-                        .map_or(false, |current_media_id| media_id == current_media_id)
-                })
-            })
+            .position(|media| media.id == current_media_msg.id)
             .unwrap_or_default();
 
         MediaViewer {
@@ -728,7 +721,7 @@ fn load_more_media(data: Rc<RefCell<Data>>, builder: gtk::Builder, backend: Send
 
     let msg = data.borrow().media_list[data.borrow().current_media_index].clone();
     let roomid = msg.room.clone();
-    let first_media_id = msg.id.clone();
+    let first_media_id = Some(msg.id.clone());
     let prev_batch = data.borrow().prev_batch.clone();
 
     let (tx, rx): (
diff --git a/fractal-gtk/src/widgets/roomlist.rs b/fractal-gtk/src/widgets/roomlist.rs
index d5c69141..990217e2 100644
--- a/fractal-gtk/src/widgets/roomlist.rs
+++ b/fractal-gtk/src/widgets/roomlist.rs
@@ -12,7 +12,6 @@ use std::collections::HashMap;
 use url::Url;
 
 use crate::globals;
-use crate::types::Message;
 use crate::types::Room;
 use crate::widgets::roomrow::RoomRow;
 use std::sync::{Arc, Mutex, MutexGuard};
@@ -40,7 +39,7 @@ impl RoomUpdated {
     pub fn new(room: Room) -> RoomUpdated {
         let updated = match room.messages.last() {
             Some(l) => l.date,
-            None => Message::default().date,
+            None => Local.ymd(1970, 1, 1).and_hms(0, 0, 0),
         };
 
         RoomUpdated { room, updated }
@@ -315,7 +314,7 @@ impl RoomListGroup {
     pub fn add_rooms(&mut self, mut array: Vec<Room>) {
         array.sort_by_key(|ref x| match x.messages.last() {
             Some(l) => l.date,
-            None => Message::default().date,
+            None => Local.ymd(1970, 1, 1).and_hms(0, 0, 0),
         });
 
         for r in array.iter().rev() {
diff --git a/fractal-matrix-api/src/backend/room.rs b/fractal-matrix-api/src/backend/room.rs
index a2870e79..e83ea8b6 100644
--- a/fractal-matrix-api/src/backend/room.rs
+++ b/fractal-matrix-api/src/backend/room.rs
@@ -155,11 +155,10 @@ pub fn get_room_messages_from_msg(bk: &Backend, roomid: String, msg: Message) ->
     // normal get_room_messages
     let baseu = bk.get_base_url();
     let tk = bk.data.lock().unwrap().access_token.clone();
-    let id = msg.id.unwrap_or_default();
     let tx = bk.internal_tx.clone();
 
     thread::spawn(move || {
-        if let Ok(from) = util::get_prev_batch_from(&baseu, &tk, &roomid, &id) {
+        if let Ok(from) = util::get_prev_batch_from(&baseu, &tk, &roomid, &msg.id) {
             if let Some(t) = tx {
                 t.send(BKCommand::GetRoomMessages(roomid, from)).unwrap();
             }
@@ -228,10 +227,9 @@ pub fn get_message_context(bk: &Backend, msg: Message) -> Result<(), Error> {
     let tx = bk.tx.clone();
     let baseu = bk.get_base_url();
     let roomid = msg.room.clone();
-    let msgid = msg.id.unwrap_or_default();
     let tk = bk.data.lock().unwrap().access_token.clone();
 
-    parse_context(tx, tk, baseu, roomid, &msgid, globals::PAGE_LIMIT)?;
+    parse_context(tx, tk, baseu, roomid, &msg.id, globals::PAGE_LIMIT)?;
 
     Ok(())
 }
@@ -239,9 +237,8 @@ pub fn get_message_context(bk: &Backend, msg: Message) -> Result<(), Error> {
 pub fn send_msg(bk: &Backend, msg: Message) -> Result<(), Error> {
     let roomid = msg.room.clone();
 
-    let id = msg.id.unwrap_or_default();
     let url = bk.url(
-        &format!("rooms/{}/send/m.room.message/{}", roomid, id),
+        &format!("rooms/{}/send/m.room.message/{}", roomid, msg.id),
         vec![],
     )?;
 
@@ -250,16 +247,16 @@ pub fn send_msg(bk: &Backend, msg: Message) -> Result<(), Error> {
         "msgtype": msg.mtype.clone()
     });
 
-    if let Some(u) = msg.url {
+    if let Some(ref u) = msg.url {
         attrs["url"] = json!(u);
     }
 
-    if let (Some(f), Some(f_b)) = (msg.format, msg.formatted_body) {
+    if let (Some(f), Some(f_b)) = (msg.format.as_ref(), msg.formatted_body.as_ref()) {
         attrs["formatted_body"] = json!(f_b);
         attrs["format"] = json!(f);
     }
 
-    if let Some(xctx) = msg.extra_content {
+    if let Some(xctx) = msg.extra_content.as_ref() {
         if let Some(xctx) = xctx.as_object() {
             for (k, v) in xctx {
                 attrs[k] = v.clone();
@@ -274,10 +271,11 @@ pub fn send_msg(bk: &Backend, msg: Message) -> Result<(), Error> {
         &attrs,
         move |js: JsonValue| {
             let evid = js["event_id"].as_str().unwrap_or_default();
-            tx.send(BKResponse::SentMsg(id, evid.to_string())).unwrap();
+            tx.send(BKResponse::SentMsg(msg.id, evid.to_string()))
+                .unwrap();
         },
         |_| {
-            tx.send(BKResponse::SendMsgError(Error::SendMsgError(id)))
+            tx.send(BKResponse::SendMsgError(Error::SendMsgError(msg.id)))
                 .unwrap();
         }
     );
@@ -287,11 +285,10 @@ pub fn send_msg(bk: &Backend, msg: Message) -> Result<(), Error> {
 
 pub fn redact_msg(bk: &Backend, msg: &Message) -> Result<(), Error> {
     let roomid = msg.room.clone();
-    let msgid = msg.id.clone().unwrap_or_default();
-    let txnid = msg.get_txn_id();
+    let txnid = msg.id.clone();
 
     let url = bk.url(
-        &format!("rooms/{}/redact/{}/{}", roomid, msgid, txnid),
+        &format!("rooms/{}/redact/{}/{}", roomid, msg.id, txnid),
         vec![],
     )?;
 
@@ -299,6 +296,7 @@ pub fn redact_msg(bk: &Backend, msg: &Message) -> Result<(), Error> {
         "reason": "Deletion requested by the sender"
     });
 
+    let msgid = msg.id.clone();
     let tx = bk.tx.clone();
     query!(
         "put",
diff --git a/fractal-matrix-api/src/model/message.rs b/fractal-matrix-api/src/model/message.rs
index 04e83588..85a79904 100644
--- a/fractal-matrix-api/src/model/message.rs
+++ b/fractal-matrix-api/src/model/message.rs
@@ -6,6 +6,7 @@ use serde_json::Value as JsonValue;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 
+//FIXME make properties privat
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Message {
     pub sender: String,
@@ -15,7 +16,7 @@ pub struct Message {
     pub room: String,
     pub thumb: Option<String>,
     pub url: Option<String>,
-    pub id: Option<String>,
+    pub id: String,
     pub formatted_body: Option<String>,
     pub format: Option<String>,
     pub source: Option<String>,
@@ -29,34 +30,9 @@ pub struct Message {
     pub extra_content: Option<JsonValue>,
 }
 
-impl Default for Message {
-    fn default() -> Message {
-        Message {
-            sender: String::new(),
-            mtype: String::from("m.text"),
-            body: String::from("default"),
-            date: Local.ymd(1970, 1, 1).and_hms(0, 0, 0),
-            room: String::new(),
-            thumb: None,
-            url: None,
-            id: None,
-            formatted_body: None,
-            format: None,
-            source: None,
-            receipt: HashMap::new(),
-            redacted: false,
-            in_reply_to: None,
-            extra_content: None,
-        }
-    }
-}
-
 impl PartialEq for Message {
     fn eq(&self, other: &Message) -> bool {
-        match (self.id.clone(), other.id.clone()) {
-            (Some(self_id), Some(other_id)) => self_id == other_id,
-            _ => self.sender == other.sender && self.body == other.body,
-        }
+        self.id == other.id
     }
 }
 
@@ -71,15 +47,25 @@ impl PartialOrd for Message {
 }
 
 impl Message {
-    /// Generates an unique transaction id for this message
-    /// The txn_id is generated using the md5sum of a concatenation of the message room id, the
-    /// message body and the date.
-
-    /// 
https://matrix.org/docs/spec/client_server/r0.3.0.html#put-matrix-client-r0-rooms-roomid-send-eventtype-txnid
-    pub fn get_txn_id(&self) -> String {
-        let msg = format!("{}{}{}", self.room, self.body, self.date.to_string());
-        let digest = md5::compute(msg.as_bytes());
-        format!("{:x}", digest)
+    pub fn new(room: String, sender: String, body: String, mtype: String) -> Self {
+        let date = Local::now();
+        Message {
+            id: get_txn_id(&room, &body, &date.to_string()),
+            sender,
+            mtype,
+            body,
+            date,
+            room,
+            thumb: None,
+            url: None,
+            formatted_body: None,
+            format: None,
+            source: None,
+            receipt: HashMap::new(),
+            redacted: false,
+            in_reply_to: None,
+            extra_content: None,
+        }
     }
 
     /// List all supported types. By default a message map a m.room.message event, but there's
@@ -124,7 +110,7 @@ impl Message {
             sender: sender.to_string(),
             date: server_timestamp,
             room: String::from(roomid),
-            id: Some(id.to_string()),
+            id: id.to_string(),
             mtype: type_.to_string(),
             body: String::new(),
             url: None,
@@ -218,3 +204,13 @@ impl Message {
         self.receipt = receipt;
     }
 }
+/// Generates an unique transaction id for this message
+/// The txn_id is generated using the md5sum of a concatenation of the message room id, the
+/// message body and the date.
+
+/// 
https://matrix.org/docs/spec/client_server/r0.3.0.html#put-matrix-client-r0-rooms-roomid-send-eventtype-txnid
+pub fn get_txn_id(room: &str, body: &str, date: &str) -> String {
+    let msg = format!("{}{}{}", room, body, date);
+    let digest = md5::compute(msg.as_bytes());
+    format!("{:x}", digest)
+}
diff --git a/fractal-matrix-api/src/model/room.rs b/fractal-matrix-api/src/model/room.rs
index 05b301c5..83aecd0b 100644
--- a/fractal-matrix-api/src/model/room.rs
+++ b/fractal-matrix-api/src/model/room.rs
@@ -81,7 +81,7 @@ impl Room {
 
         if let Some(receipts) = receipts.clone() {
             for msg in self.messages.iter_mut() {
-                if let Some(r) = msg.id.clone().and_then(|id| receipts.get(&id)) {
+                if let Some(r) = receipts.get(&msg.id) {
                     msg.set_receipt(r.clone());
                 }
             }
@@ -92,7 +92,7 @@ impl Room {
         for msg in self
             .messages
             .iter_mut()
-            .filter(|m| m.id == Some(evid.to_string()))
+            .filter(|m| m.id == evid.to_string())
         {
             msg.receipt.insert(uid.to_string(), 0);
         }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]