Address review feedback: use Message field for SDP, remove table id, store ended_timestamp

Co-authored-by: link2xt <18373967+link2xt@users.noreply.github.com>

src/calls.rs

@@ -135,8 +135,18 @@ impl CallInfo {
     }
 
     async fn mark_as_ended(&mut self, context: &Context) -> Result<()> {
-        self.msg.param.set_i64(CALL_ENDED_TIMESTAMP, time());
+        let now = time();
+        self.msg.param.set_i64(CALL_ENDED_TIMESTAMP, now);
         self.msg.update_param(context).await?;
+        
+        // Also store ended timestamp in calls table
+        context
+            .sql
+            .execute(
+                "UPDATE calls SET ended_timestamp=? WHERE msg_id=?",
+                (now, self.msg.id),
+            )
+            .await?;
         Ok(())
     }
 
@@ -152,6 +162,15 @@ impl CallInfo {
         self.msg.param.set_i64(CALL_ENDED_TIMESTAMP, now);
         self.msg.param.set_i64(CALL_CANCELED_TIMESTAMP, now);
         self.msg.update_param(context).await?;
+        
+        // Also store ended timestamp in calls table
+        context
+            .sql
+            .execute(
+                "UPDATE calls SET ended_timestamp=? WHERE msg_id=?",
+                (now, self.msg.id),
+            )
+            .await?;
         Ok(())
     }
 
@@ -193,27 +212,19 @@ impl Context {
         let mut call = Message {
             viewtype: Viewtype::Call,
             text: "Outgoing call".into(),
+            call_sdp_offer: Some(place_call_info.clone()),
             ..Default::default()
         };
         
-        // Set a placeholder parameter so the message is recognized as a call during sending.
-        // This ensures mimefactory can read the SDP during the brief window between
-        // send_msg() starting and the calls table entry being available.
-        // The param will be removed after send_msg() completes.
-        call.param.set(Param::WebrtcRoom, &place_call_info);
         call.id = send_msg(self, chat_id, &mut call).await?;
 
-        // Store SDP offer in calls table and remove from params
+        // Store SDP offer in calls table
         self.sql
             .execute(
                 "INSERT OR REPLACE INTO calls (msg_id, offer_sdp) VALUES (?, ?)",
                 (call.id, &place_call_info),
             )
             .await?;
-        
-        // Remove SDP from message params for privacy
-        call.param.remove(Param::WebrtcRoom);
-        call.update_param(self).await?;
 
         let wait = RINGING_SECONDS;
         task::spawn(Context::emit_end_call_if_unaccepted(

src/message.rs

@@ -443,6 +443,13 @@ pub struct Message {
     pub(crate) location_id: u32,
     pub(crate) error: Option<String>,
     pub(crate) param: Params,
+    
+    /// SDP offer for outgoing calls.
+    /// This field is used to pass the SDP offer to the database
+    /// without storing it in message parameters.
+    /// It is not persisted in the msgs table, only in the calls table.
+    #[serde(skip)]
+    pub(crate) call_sdp_offer: Option<String>,
 }
 
 impl Message {
@@ -572,6 +579,7 @@ impl Message {
                         chat_blocked: row
                             .get::<_, Option<Blocked>>("blocked")?
                             .unwrap_or_default(),
+                        call_sdp_offer: None,
                     };
                     Ok(msg)
                 },

src/mimefactory.rs

@@ -1750,17 +1750,24 @@ impl MimeFactory {
         }
 
         if msg.viewtype == Viewtype::Call {
-            // Get SDP offer from calls table, or fall back to params if not yet migrated
-            let offer_sdp = context
-                .sql
-                .query_row_optional(
-                    "SELECT offer_sdp FROM calls WHERE msg_id=?",
-                    (msg.id,),
-                    |row| row.get::<_, Option<String>>(0),
-                )
-                .await?
-                .flatten()
-                .or_else(|| msg.param.get(Param::WebrtcRoom).map(|s| s.to_string()));
+            // Get SDP offer from the message field (if being sent), calls table, or params (for old messages)
+            let offer_sdp = if let Some(ref offer) = msg.call_sdp_offer {
+                Some(offer.clone())
+            } else if !msg.id.is_unset() {
+                // Try to get from calls table if message is already stored
+                context
+                    .sql
+                    .query_row_optional(
+                        "SELECT offer_sdp FROM calls WHERE msg_id=?",
+                        (msg.id,),
+                        |row| row.get::<_, Option<String>>(0),
+                    )
+                    .await?
+                    .flatten()
+            } else {
+                None
+            }
+            .or_else(|| msg.param.get(Param::WebrtcRoom).map(|s| s.to_string()));
                 
             if let Some(offer_sdp) = offer_sdp {
                 headers.push((

src/sql.rs

@@ -923,10 +923,7 @@ pub async fn housekeeping(context: &Context) -> Result<()> {
 
     // Delete call SDPs for ended calls (older than 24 hours) or trashed calls.
     // We clean up calls that ended more than 24 hours ago to protect privacy
-    // as SDPs contain IP addresses. Ended calls are identified by having
-    // the CALL_ENDED_TIMESTAMP parameter (Param::Arg4='H') set.
-    // The pattern '%H=%' matches this parameter since params are stored as
-    // newline-separated key=value pairs (e.g., "E=123\nH=456\n").
+    // as SDPs contain IP addresses.
     // The ON DELETE CASCADE foreign key handles orphaned entries automatically.
     context
         .sql
@@ -935,8 +932,8 @@ pub async fn housekeeping(context: &Context) -> Result<()> {
                 SELECT calls.msg_id FROM calls 
                 INNER JOIN msgs ON calls.msg_id = msgs.id
                 WHERE msgs.chat_id = ?
-                   OR (msgs.param LIKE '%H=%'
-                       AND msgs.timestamp_sent < ?)
+                   OR (calls.ended_timestamp IS NOT NULL
+                       AND calls.ended_timestamp < ?)
             )",
             (DC_CHAT_ID_TRASH, time() - 86400),
         )

src/sql/migrations.rs

@@ -1343,12 +1343,11 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint);
     if dbversion < migration_version {
         sql.execute_migration(
             "CREATE TABLE calls(
-                id INTEGER PRIMARY KEY AUTOINCREMENT,
-                msg_id INTEGER NOT NULL UNIQUE REFERENCES msgs(id) ON DELETE CASCADE,
+                msg_id INTEGER PRIMARY KEY REFERENCES msgs(id) ON DELETE CASCADE,
                 offer_sdp TEXT,
-                answer_sdp TEXT
-            ) STRICT;
-            CREATE INDEX calls_msg_id_index ON calls (msg_id);",
+                answer_sdp TEXT,
+                ended_timestamp INTEGER
+            ) STRICT;",
             migration_version,
         )
         .await?;