Remove outdated TODOs, use StateEvent::from_id_value consistently

src/client_server/membership.rs

@@ -545,7 +545,7 @@ async fn join_room_by_id_helper(
         );
 
         // It has enough fields to be called a proper event now
-        let join_event = dbg!(canon_json_stub);
+        let join_event = canon_json_stub;
 
         let send_join_response = server_server::send_request(
             &db.globals,
@@ -577,7 +577,7 @@ async fn join_room_by_id_helper(
                     .expect("a valid EventId can be converted to CanonicalJsonValue"),
             );
 
-            Ok((event_id, serde_json::json!(value))) // TODO CanonicalJsonValue fixup?
+            Ok((event_id, serde_json::json!(value)))
         };
 
         let room_state = send_join_response.room_state.state.iter().map(add_event_id);
@@ -602,7 +602,8 @@ async fn join_room_by_id_helper(
             )))) // Add join event we just created
             .map(|r| {
                 let (event_id, value) = r?;
-                serde_json::from_value::<StateEvent>(value.clone())
+                // TODO remove .clone when I remove debug logging
+                state_res::StateEvent::from_id_value(event_id.clone(), value.clone())
                     .map(|ev| (event_id, Arc::new(ev)))
                     .map_err(|e| {
                         warn!("{}: {}", value, e);
@@ -642,7 +643,9 @@ async fn join_room_by_id_helper(
         .expect("iterative auth check failed on resolved events");
 
         // This removes the control events that failed auth, leaving the resolved
-        // to be mainline sorted
+        // to be mainline sorted. In the actual `state_res::StateResolution::resolve`
+        // function both are removed since these are all events we don't know of
+        // we must keep track of everything to add to our DB.
         let events_to_sort = event_map
             .keys()
             .filter(|id| {

src/pdu.rs

@@ -252,22 +252,27 @@ impl From<&state_res::StateEvent> for PduEvent {
 impl PduEvent {
     pub fn convert_for_state_res(&self) -> Arc<state_res::StateEvent> {
         Arc::new(
-            serde_json::from_value(json!({
-                "event_id": self.event_id,
-                "room_id": self.room_id,
-                "sender": self.sender,
-                "origin_server_ts": self.origin_server_ts,
-                "type": self.kind,
-                "content": self.content,
-                "state_key": self.state_key,
-                "prev_events": self.prev_events,
-                "depth": self.depth,
-                "auth_events": self.auth_events,
-                "redacts": self.redacts,
-                "unsigned": self.unsigned,
-                "hashes": self.hashes,
-                "signatures": self.signatures,
-            }))
+            // For consistency of eventId (just in case) we use the one
+            // generated by conduit for everything.
+            state_res::StateEvent::from_id_value(
+                self.event_id.clone(),
+                json!({
+                    "event_id": self.event_id,
+                    "room_id": self.room_id,
+                    "sender": self.sender,
+                    "origin_server_ts": self.origin_server_ts,
+                    "type": self.kind,
+                    "content": self.content,
+                    "state_key": self.state_key,
+                    "prev_events": self.prev_events,
+                    "depth": self.depth,
+                    "auth_events": self.auth_events,
+                    "redacts": self.redacts,
+                    "unsigned": self.unsigned,
+                    "hashes": self.hashes,
+                    "signatures": self.signatures,
+                }),
+            )
             .expect("all conduit PDUs are state events"),
         )
     }

src/server_server.rs

@@ -26,6 +26,7 @@ use std::{
     collections::BTreeMap,
     convert::{TryFrom, TryInto},
     fmt::Debug,
+    sync::Arc,
     time::{Duration, SystemTime},
 };
 use trust_dns_resolver::AsyncResolver;
@@ -551,7 +552,6 @@ pub async fn send_transaction_message_route<'a>(
                     .iter()
                     .map(|((ev, sk), v)| ((ev.clone(), sk.to_owned()), v.event_id.clone()))
                     .collect::<BTreeMap<_, _>>(),
-                // TODO we may not want the auth events chained in here for resolution?
                 their_current_state
                     .iter()
                     .map(|(_id, v)| ((v.kind(), v.state_key()), v.event_id()))
@@ -750,10 +750,10 @@ pub fn get_user_devices_route<'a>(
 
 /// Generates a correct eventId for the incoming pdu.
 ///
-/// Returns a `state_res::StateEvent` which can be converted freely and has accessor methods.
+/// Returns a tuple of the new `EventId` and the PDU with the eventId inserted as a `serde_json::Value`.
 fn process_incoming_pdu(pdu: &ruma::Raw<ruma::events::pdu::Pdu>) -> (EventId, serde_json::Value) {
-    let mut value = serde_json::from_str(pdu.json().get())
-        .expect("converting raw jsons to values always works");
+    let mut value =
+        serde_json::from_str(pdu.json().get()).expect("A Raw<...> is always valid JSON");
 
     let event_id = EventId::try_from(&*format!(
         "${}",