Don't build a new connection if it already exists

There was also a lot of unneeded overhead in Connection::new() to get
the path to compare against, so split that out into a settings struct

src/padfoot/connection.rs

@@ -38,20 +38,26 @@ pub const CONN_OBJECT_PATH: &str = "/org/freedesktop/Telepathy/Connection/padfoo
 #[derive(Debug)]
 // A Deltachast connection uses email addresses as handles, and delta's Db IDs
 pub struct Connection {
-    id: String,
     ctx: Arc<RwLock<Context>>,
-
+    settings: ConnSettings,
     state: Arc<RwLock<ConnState>>,
 
     // Used for sending out messages
     msgq: Arc<Mutex<VecDeque<dbus::Message>>>,
 }
 
-impl Connection {
-    pub fn new(params: HashMap<&str, super::Variant>) -> Result<Self, MethodErr> {
+#[derive(Debug)]
+pub struct ConnSettings {
+    account: String,
+    password: String,
+    id: String,
+}
+
+impl ConnSettings {
+    pub fn from_params(params: HashMap<&str, super::Variant>) -> Result<Self, MethodErr> {
         let err = Err(MethodErr::no_arg());
 
-        let acct = match params.get("account") {
+        let account = match params.get("account") {
             Some(variant) => match variant.0.as_str() {
                 Some(str) => str.to_string(),
                 None => return err,
@@ -59,7 +65,7 @@ impl Connection {
             None => return err,
         };
 
-        let id = escape(acct.to_owned());
+        let id = escape(account.to_owned());
 
         let password = match params.get("password") {
             Some(variant) => match variant.0.as_str() {
@@ -69,31 +75,53 @@ impl Connection {
             None => return err,
         };
 
+        Ok(Self {
+            account,
+            password,
+            id,
+        })
+    }
+
+    pub fn id(&self) -> String {
+        self.id.to_owned()
+    }
+
+    pub fn bus(&self) -> String {
+        CONN_BUS_NAME.to_owned() + "." + &self.id
+    }
+
+    pub fn path(&self) -> String {
+        CONN_OBJECT_PATH.to_owned() + "/" + &self.id
+    }
+}
+
+impl Connection {
+    pub fn new(settings: ConnSettings) -> Result<Self, MethodErr> {
         let mut dbfile = directories::ProjectDirs::from("gs", "ur", "telepathy-padfoot")
             .ok_or_else(MethodErr::no_arg)
             .and_then(|p| Ok(p.data_local_dir().to_path_buf()))?;
 
-        dbfile.push(&id);
+        dbfile.push(settings.id());
         dbfile.push("db.sqlite3");
 
         // FIXME: how to give it access to the connection (initialized later)?
         let msgq = Arc::new(Mutex::new(VecDeque::<dbus::Message>::new()));
 
-        let id2 = id.clone();
+        let id = settings.id();
         // Use this if we need to send messages in response to DC events:
         // let msgq2 = msgq.clone();
         let f = move |_c: &Context, e: Event| {
             match e {
-                Event::Info(msg) => println!("Connection<{}>: INFO: {}", id2, msg),
-                Event::Warning(msg) => println!("Connection<{}>: WARN : {}", id2, msg),
+                Event::Info(msg) => println!("Connection<{}>: INFO: {}", id, msg),
+                Event::Warning(msg) => println!("Connection<{}>: WARN : {}", id, msg),
                 Event::Error(msg) | Event::ErrorNetwork(msg) | Event::ErrorSelfNotInGroup(msg) => {
-                    println!("Connection<{}>: ERR : {}", id2, msg)
+                    println!("Connection<{}>: ERR : {}", id, msg)
                 }
                 Event::ConfigureProgress(progress) => {
-                    println!("Connection<{}>: Configuration progress: {}", id2, progress)
+                    println!("Connection<{}>: Configuration progress: {}", id, progress)
                 }
                 Event::ImapConnected(msg) | Event::SmtpConnected(msg) => {
-                    println!("Connection<{}>: Network: {}", id2, msg);
+                    println!("Connection<{}>: Network: {}", id, msg);
                 }
                 /* Unhandled messages:
                     SmtpMessageSent(String),
@@ -115,19 +143,23 @@ impl Connection {
                     SecurejoinInviterProgress
                     SecurejoinJoinerProgress
                 */
-                _ => println!("Connection<{}>: unhandled event received: {:?}", id2, e),
+                _ => println!("Connection<{}>: unhandled event received: {:?}", id, e),
             };
         };
 
         let ctx =
             Context::new(Box::new(f), "telepathy-padfoot".to_string(), dbfile).map_err(|e| {
-                println!("Connection<{}>: couldn't get delta context: {}", id, e);
+                println!(
+                    "Connection<{}>::new(): couldn't get delta context: {}",
+                    settings.id(),
+                    e
+                );
                 MethodErr::no_arg() // FIXME: better error handling
             })?;
 
-        ctx.set_config(Config::Addr, Some(&acct))
+        ctx.set_config(Config::Addr, Some(&settings.account))
             .map_err(|_e| MethodErr::no_arg())?;
-        ctx.set_config(Config::MailPw, Some(&password))
+        ctx.set_config(Config::MailPw, Some(&settings.password))
             .map_err(|_e| MethodErr::no_arg())?;
         ctx.set_config(Config::SentboxWatch, Some(&"Sent"))
             .map_err(|_e| MethodErr::no_arg())?;
@@ -137,7 +169,7 @@ impl Connection {
         };
 
         Ok(Connection {
-            id,
+            settings,
             msgq,
             ctx: Arc::new(RwLock::new(ctx)),
             state: Arc::new(RwLock::new(ConnState::Initial)),
@@ -154,7 +186,7 @@ impl Connection {
         let path = self.path();
         let state = self.state.clone();
         let msgq = self.msgq.clone();
-        let id = self.id.clone();
+        let id = self.id();
         let c_rc = std::rc::Rc::new(self);
 
         let f = dbus::tree::Factory::new_fn::<()>();
@@ -248,12 +280,16 @@ impl Connection {
         // connect() ?
     }
 
+    pub fn id(&self) -> String {
+        self.settings.id.to_string()
+    }
+
     pub fn bus(&self) -> String {
-        CONN_BUS_NAME.to_owned() + "." + &self.id
+        self.settings.bus()
     }
 
     pub fn path(&self) -> String {
-        CONN_OBJECT_PATH.to_owned() + "/" + &self.id
+        self.settings.path()
     }
 }
 

src/padfoot/connection/avatars.rs

@@ -16,12 +16,16 @@ type AvatarRequirementSpec = (Vec<String>, u16, u16, u16, u16, u32);
 // TODO: come back and do this properly, I'm just putting it in for consistency
 impl telepathy::ConnectionInterfaceAvatars for Connection {
     fn get_avatar_requirements(&self) -> Result<AvatarRequirementSpec, MethodErr> {
-        println!("Connection<{}>::get_avatar_requirements()", self.id);
+        println!("Connection<{}>::get_avatar_requirements()", self.id());
         Ok((vec![], 0, 0, 0, 0, 0))
     }
 
     fn get_avatar_tokens(&self, contacts: Vec<u32>) -> Result<Vec<String>, MethodErr> {
-        println!("Connection<{}>::get_avatar_tokens({:?})", self.id, contacts);
+        println!(
+            "Connection<{}>::get_avatar_tokens({:?})",
+            self.id(),
+            contacts
+        );
         Ok(vec![])
     }
 
@@ -31,71 +35,73 @@ impl telepathy::ConnectionInterfaceAvatars for Connection {
     ) -> Result<::std::collections::HashMap<u32, String>, MethodErr> {
         println!(
             "Connection<{}>::get_known_avatar_tokens({:?})",
-            self.id, contacts
+            self.id(),
+            contacts
         );
         Ok(HashMap::<u32, String>::new())
     }
 
     fn request_avatar(&self, contact: u32) -> Result<(Vec<u8>, String), MethodErr> {
-        println!("Connection<{}>::request_avatar({})", self.id, contact);
+        println!("Connection<{}>::request_avatar({})", self.id(), contact);
         Ok((vec![], "".to_string()))
     }
 
     fn request_avatars(&self, contacts: Vec<u32>) -> Result<(), MethodErr> {
-        println!("Connection<{}>::request_avatar({:?})", self.id, contacts);
+        println!("Connection<{}>::request_avatar({:?})", self.id(), contacts);
         Ok(())
     }
 
     fn set_avatar(&self, _avatar: Vec<u8>, mimetype: &str) -> Result<String, MethodErr> {
         println!(
             "Connection<{}>::set_avatar((data), {:?})",
-            self.id, mimetype
+            self.id(),
+            mimetype
         );
         Ok("".to_string())
     }
 
     fn clear_avatar(&self) -> Result<(), MethodErr> {
-        println!("Connection<{}>::clear_avatar()", self.id);
+        println!("Connection<{}>::clear_avatar()", self.id());
         Ok(())
     }
 
     fn supported_avatar_mimetypes(&self) -> Result<Vec<String>, MethodErr> {
-        println!("Connection<{}>::supported_avatar_mimetypes()", self.id);
+        println!("Connection<{}>::supported_avatar_mimetypes()", self.id());
         Ok(vec![])
     }
 
     fn minimum_avatar_height(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::minimum_avatar_height()", self.id);
+        println!("Connection<{}>::minimum_avatar_height()", self.id());
         Ok(0)
     }
 
     fn minimum_avatar_width(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::minimum_avatar_width()", self.id);
+        println!("Connection<{}>::minimum_avatar_width()", self.id());
         Ok(0)
     }
 
     fn recommended_avatar_height(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::recommended_avatar_height()", self.id);
+        println!("Connection<{}>::recommended_avatar_height()", self.id());
         Ok(0)
     }
 
     fn recommended_avatar_width(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::recommended_avatar_width()", self.id);
+        println!("Connection<{}>::recommended_avatar_width()", self.id());
         Ok(0)
     }
 
     fn maximum_avatar_height(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::maximum_avatar_height()", self.id);
+        println!("Connection<{}>::maximum_avatar_height()", self.id());
         Ok(0)
     }
 
     fn maximum_avatar_width(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::maximum_avatar_width()", self.id);
+        println!("Connection<{}>::maximum_avatar_width()", self.id());
         Ok(0)
     }
 
     fn maximum_avatar_bytes(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::maximum_avatar_bytes()", self.id);
+        println!("Connection<{}>::maximum_avatar_bytes()", self.id());
         Ok(0)
     }
 }

src/padfoot/connection/connection.rs

@@ -34,11 +34,11 @@ impl AsRef<dyn telepathy::Connection + 'static> for std::rc::Rc<Connection> {
 impl telepathy::Connection for Connection {
     // In connect(), we start the threads that drive the deltachat context
     fn connect(&self) -> Result<(), MethodErr> {
-        println!("Connection<{}>::connect()", self.id);
+        println!("Connection<{}>::connect()", self.id());
 
         let inbox_ctx = self.ctx.clone();
         let state = self.state.clone();
-        let id = self.id.clone();
+        let id = self.id();
         let _inbox_thread = thread::spawn(move || {
             while *state.read().unwrap() != ConnState::Disconnected {
                 dc::job::perform_inbox_jobs(&inbox_ctx.read().unwrap());
@@ -56,7 +56,7 @@ impl telepathy::Connection for Connection {
 
         let smtp_ctx = self.ctx.clone();
         let state = self.state.clone();
-        let id = self.id.clone();
+        let id = self.id();
         let _smtp_thread = thread::spawn(move || {
             while *state.read().unwrap() != ConnState::Disconnected {
                 dc::job::perform_smtp_jobs(&smtp_ctx.read().unwrap());
@@ -70,7 +70,7 @@ impl telepathy::Connection for Connection {
 
         let mvbox_ctx = self.ctx.clone();
         let state = self.state.clone();
-        let id = self.id.clone();
+        let id = self.id();
         let _mvbox_thread = thread::spawn(move || {
             while *state.read().unwrap() != ConnState::Disconnected {
                 dc::job::perform_mvbox_fetch(&mvbox_ctx.read().unwrap());
@@ -84,7 +84,7 @@ impl telepathy::Connection for Connection {
 
         let sentbox_ctx = self.ctx.clone();
         let state = self.state.clone();
-        let id = self.id.clone();
+        let id = self.id();
         let _sentbox_thread = thread::spawn(move || {
             while *state.read().unwrap() != ConnState::Disconnected {
                 dc::job::perform_sentbox_fetch(&sentbox_ctx.read().unwrap());
@@ -121,7 +121,7 @@ impl telepathy::Connection for Connection {
     }
 
     fn disconnect(&self) -> Result<(), MethodErr> {
-        println!("Connection<{}>::disconnect()", self.id);
+        println!("Connection<{}>::disconnect()", self.id());
         let ctx = self.ctx.read().unwrap();
 
         let state = self.state.clone();
@@ -140,37 +140,37 @@ impl telepathy::Connection for Connection {
     }
 
     fn interfaces(&self) -> Result<Vec<String>, MethodErr> {
-        println!("Connection<{}>::interfaces()", self.id);
+        println!("Connection<{}>::interfaces()", self.id());
 
         self.get_interfaces()
     }
 
     fn get_interfaces(&self) -> Result<Vec<String>, MethodErr> {
-        println!("Connection<{}>::get_interfaces()", self.id);
+        println!("Connection<{}>::get_interfaces()", self.id());
 
         Ok(connection_interfaces())
     }
 
     fn get_protocol(&self) -> Result<String, MethodErr> {
-        println!("Connection<{}>::get_protocol()", self.id);
+        println!("Connection<{}>::get_protocol()", self.id());
 
         Ok(crate::padfoot::PROTO_NAME.to_string())
     }
 
     fn self_handle(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::self_handle()", self.id);
+        println!("Connection<{}>::self_handle()", self.id());
 
         self.get_self_handle()
     }
 
     fn get_self_handle(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::get_self_handle()", self.id);
+        println!("Connection<{}>::get_self_handle()", self.id());
 
         Ok(dc::constants::DC_CONTACT_ID_SELF)
     }
 
     fn status(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::status()", self.id);
+        println!("Connection<{}>::status()", self.id());
 
         self.get_status()
     }
@@ -185,7 +185,9 @@ impl telepathy::Connection for Connection {
     fn hold_handles(&self, handle_type: u32, handles: Vec<u32>) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::hold_handles({}, {:?})",
-            self.id, handle_type, handles
+            self.id(),
+            handle_type,
+            handles
         );
 
         // Since HasImmortalHandles is true, this doesn't need to do anything
@@ -199,20 +201,24 @@ impl telepathy::Connection for Connection {
     ) -> Result<Vec<String>, MethodErr> {
         println!(
             "Connection<{}>::inspect_handles({}, {:?})",
-            self.id, handle_type, handles
+            self.id(),
+            handle_type,
+            handles
         );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn list_channels(&self) -> Result<Vec<(dbus::Path<'static>, String, u32, u32)>, MethodErr> {
-        println!("Connection<{}>::list_channels()", self.id);
+        println!("Connection<{}>::list_channels()", self.id());
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn release_handles(&self, handle_type: u32, handles: Vec<u32>) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::release_handles({}, {:?})",
-            self.id, handle_type, handles
+            self.id(),
+            handle_type,
+            handles
         );
 
         // Since HasImmortalHandles is true, we don't need to do anything
@@ -228,7 +234,11 @@ impl telepathy::Connection for Connection {
     ) -> Result<dbus::Path<'static>, MethodErr> {
         println!(
             "Connection<{}>::request_channel({}, {}, {}, {})",
-            self.id, type_, handle_type, handle, suppress_handler
+            self.id(),
+            type_,
+            handle_type,
+            handle,
+            suppress_handler
         );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
@@ -240,26 +250,33 @@ impl telepathy::Connection for Connection {
     ) -> Result<Vec<u32>, MethodErr> {
         println!(
             "Connection<{}>::request_handles({}, {:?})",
-            self.id, handle_type, identifiers
+            self.id(),
+            handle_type,
+            identifiers
         );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn add_client_interest(&self, tokens: Vec<&str>) -> Result<(), MethodErr> {
-        println!("Connection<{}>::add_client_interest({:?})", self.id, tokens);
+        println!(
+            "Connection<{}>::add_client_interest({:?})",
+            self.id(),
+            tokens
+        );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn remove_client_interest(&self, tokens: Vec<&str>) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::remove_client_interest({:?})",
-            self.id, tokens
+            self.id(),
+            tokens
         );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn self_id(&self) -> Result<String, MethodErr> {
-        println!("Connection<{}>::self_id()", self.id);
+        println!("Connection<{}>::self_id()", self.id());
 
         let contact = match dc::contact::Contact::get_by_id(
             &self.ctx.read().unwrap(),
@@ -276,7 +293,7 @@ impl telepathy::Connection for Connection {
     }
 
     fn has_immortal_handles(&self) -> Result<bool, MethodErr> {
-        println!("Connection<{}>::has_immortal_handles()", self.id);
+        println!("Connection<{}>::has_immortal_handles()", self.id());
 
         Ok(true)
     }

src/padfoot/connection/contact_list.rs

@@ -28,7 +28,9 @@ impl telepathy::ConnectionInterfaceContactList for Connection {
     ) -> Result<HashMap<u32, HashMap<String, VarArg>>, MethodErr> {
         println!(
             "Connection<{}>::get_contact_list_attributes({:?}, {})",
-            self.id, interfaces, hold
+            self.id(),
+            interfaces,
+            hold
         );
 
         let ctx = &self.ctx.read().unwrap();
@@ -46,7 +48,9 @@ impl telepathy::ConnectionInterfaceContactList for Connection {
     fn request_subscription(&self, contacts: Vec<u32>, message: &str) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::request_subscription({:?}, {})",
-            self.id, contacts, message
+            self.id(),
+            contacts,
+            message
         );
         Ok(())
     }
@@ -54,50 +58,51 @@ impl telepathy::ConnectionInterfaceContactList for Connection {
     fn authorize_publication(&self, contacts: Vec<u32>) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::authorize_publication({:?})",
-            self.id, contacts
+            self.id(),
+            contacts
         );
         Ok(())
     }
     fn remove_contacts(&self, contacts: Vec<u32>) -> Result<(), MethodErr> {
-        println!("Connection<{}>::remove_contacts({:?})", self.id, contacts);
+        println!("Connection<{}>::remove_contacts({:?})", self.id(), contacts);
         Ok(())
     }
     fn unsubscribe(&self, contacts: Vec<u32>) -> Result<(), MethodErr> {
-        println!("Connection<{}>::unsubscribe({:?})", self.id, contacts);
+        println!("Connection<{}>::unsubscribe({:?})", self.id(), contacts);
         Ok(())
     }
 
     fn unpublish(&self, contacts: Vec<u32>) -> Result<(), MethodErr> {
-        println!("Connection<{}>::unpublish({:?})", self.id, contacts);
+        println!("Connection<{}>::unpublish({:?})", self.id(), contacts);
         Ok(())
     }
 
     fn download(&self) -> Result<(), MethodErr> {
-        println!("Connection<{}>::download()", self.id);
+        println!("Connection<{}>::download()", self.id());
         Ok(())
     }
 
     fn contact_list_state(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::contact_list_state()", self.id);
+        println!("Connection<{}>::contact_list_state()", self.id());
         Ok(3) // Success
     }
 
     fn contact_list_persists(&self) -> Result<bool, MethodErr> {
-        println!("Connection<{}>::contact_list_persists()", self.id);
+        println!("Connection<{}>::contact_list_persists()", self.id());
         Ok(true)
     }
 
     fn can_change_contact_list(&self) -> Result<bool, MethodErr> {
-        println!("Connection<{}>::can_change_contact_list()", self.id);
+        println!("Connection<{}>::can_change_contact_list()", self.id());
         Ok(false)
     }
     fn request_uses_message(&self) -> Result<bool, MethodErr> {
-        println!("Connection<{}>::request_uses_message()", self.id);
+        println!("Connection<{}>::request_uses_message()", self.id());
         Ok(false)
     }
 
     fn download_at_connection(&self) -> Result<bool, MethodErr> {
-        println!("Connection<{}>::download_at_connection()", self.id);
+        println!("Connection<{}>::download_at_connection()", self.id());
         Ok(false)
     }
 }

src/padfoot/connection/contacts.rs

@@ -25,7 +25,10 @@ impl telepathy::ConnectionInterfaceContacts for Connection {
     ) -> Result<HashMap<u32, HashMap<String, VarArgs>>, MethodErr> {
         println!(
             "Connection<{}>::get_contact_attributes({:?}, {:?}, {})",
-            self.id, handles, interfaces, hold
+            self.id(),
+            handles,
+            interfaces,
+            hold
         );
 
         let mut out = HashMap::<u32, HashMap<String, VarArgs>>::new();
@@ -73,13 +76,15 @@ impl telepathy::ConnectionInterfaceContacts for Connection {
     ) -> Result<(u32, HashMap<String, VarArgs>), MethodErr> {
         println!(
             "Connection<{}>::get_contact_by_id({}, {:?})",
-            self.id, identifier, interfaces
+            self.id(),
+            identifier,
+            interfaces
         );
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn contact_attribute_interfaces(&self) -> Result<Vec<String>, MethodErr> {
-        println!("Connection<{}>::contact_attribute_interfaces()", self.id);
+        println!("Connection<{}>::contact_attribute_interfaces()", self.id());
 
         Ok(vec![
             "org.freedesktop.Telepathy.Connection".to_string(),

src/padfoot/connection/requests.rs

@@ -24,7 +24,7 @@ impl telepathy::ConnectionInterfaceRequests for Connection {
         &self,
         request: HashMap<&str, VarArg>,
     ) -> Result<(dbus::Path<'static>, HashMap<String, VarArg>), MethodErr> {
-        println!("Connection<{}>::create_channel({:?})", self.id, request);
+        println!("Connection<{}>::create_channel({:?})", self.id(), request);
 
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
@@ -33,17 +33,17 @@ impl telepathy::ConnectionInterfaceRequests for Connection {
         &self,
         request: HashMap<&str, VarArg>,
     ) -> Result<(bool, dbus::Path<'static>, HashMap<String, VarArg>), MethodErr> {
-        println!("Connection<{}>::ensure_channel({:?})", self.id, request);
+        println!("Connection<{}>::ensure_channel({:?})", self.id(), request);
         Err(MethodErr::no_arg()) // FIXME: should be NotImplemented?
     }
 
     fn channels(&self) -> Result<Vec<ChannelSpec>, MethodErr> {
-        println!("Connection<{}>::channels()", self.id);
+        println!("Connection<{}>::channels()", self.id());
         Ok(vec![])
     }
 
     fn requestable_channel_classes(&self) -> Result<Vec<RequestableChannelSpec>, MethodErr> {
-        println!("Connection<{}>::requestable_channel_classes()", self.id);
+        println!("Connection<{}>::requestable_channel_classes()", self.id());
         Ok(crate::padfoot::requestables())
     }
 }

src/padfoot/connection/simple_presence.rs

@@ -27,7 +27,9 @@ impl telepathy::ConnectionInterfaceSimplePresence for Connection {
     fn set_presence(&self, status: &str, status_message: &str) -> Result<(), MethodErr> {
         println!(
             "Connection<{}>::set_presence({}, {:?})",
-            self.id, status, status_message
+            self.id(),
+            status,
+            status_message
         );
 
         // FIXME: emit a presence changed signal
@@ -39,7 +41,7 @@ impl telepathy::ConnectionInterfaceSimplePresence for Connection {
         &self,
         contacts: Vec<u32>,
     ) -> Result<HashMap<u32, SimplePresenceSpec>, MethodErr> {
-        println!("Connection<{}>::get_presences({:?})", self.id, contacts);
+        println!("Connection<{}>::get_presences({:?})", self.id(), contacts);
 
         let mut out = HashMap::<u32, SimplePresenceSpec>::new();
 
@@ -51,7 +53,7 @@ impl telepathy::ConnectionInterfaceSimplePresence for Connection {
     }
 
     fn statuses(&self) -> Result<HashMap<String, SimpleStatusSpec>, MethodErr> {
-        println!("Connection<{}>::statuses()", self.id);
+        println!("Connection<{}>::statuses()", self.id());
 
         let mut out = HashMap::<String, SimpleStatusSpec>::new();
 
@@ -62,7 +64,7 @@ impl telepathy::ConnectionInterfaceSimplePresence for Connection {
     }
 
     fn maximum_status_message_length(&self) -> Result<u32, MethodErr> {
-        println!("Connection<{}>::maximum_status_message_length()", self.id);
+        println!("Connection<{}>::maximum_status_message_length()", self.id());
         Ok(0)
     }
 }

src/padfoot/connection_manager.rs

@@ -6,6 +6,8 @@ use dbus::tree::MethodErr;
 use std::collections::{HashMap, HashSet};
 use std::sync::mpsc;
 
+use super::{ConnSettings, Connection};
+
 pub const CM_BUS_NAME: &str = "org.freedesktop.Telepathy.ConnectionManager.padfoot";
 pub const CM_CONN_BUS_NAME: &str = "org.freedesktop.Telepathy.Connection.padfoot";
 pub const CM_OBJECT_PATH: &str = "/org/freedesktop/Telepathy/ConnectionManager/padfoot";
@@ -37,22 +39,23 @@ impl ConnectionManager {
 
     fn create_connection(
         &self,
-        params: HashMap<&str, super::Variant>,
+        settings: ConnSettings,
     ) -> Result<(String, dbus::Path<'static>), MethodErr> {
-        let conn = super::Connection::new(params)?;
-        let bus = conn.bus();
-        let path = conn.path();
+        let bus = settings.bus();
+        let path = settings.path();
+
+        let mut conns = self.conns.lock().expect("Mutex access");
+
         let dbus_conn_path = dbus::strings::Path::new(path.to_owned())
             .expect("Object path should meet DBUS requirements");
 
-        // If we already have this connection, we can point the user at it.
-        // FIXME: should we update the other params in this case?
-        let mut conns = self.conns.lock().expect("Mutex access");
-
+        // We can't call connect() multiple times on the connection yet
         if conns.contains(&path) {
-            return Ok((bus, dbus_conn_path));
+            return Err(MethodErr::no_arg());
         }
 
+        let conn = Connection::new(settings)?;
+
         // It would be nice to have a single main loop, but thread-per-conn is
         // is easy enough for me to understand in Rust at the moment.
         let (ok_s, ok_r) = mpsc::channel();
@@ -73,7 +76,7 @@ impl ConnectionManager {
                 return Err(e);
             }
 
-            // Otherwise OK
+        // Otherwise OK
         } else {
             return Err(MethodErr::no_arg());
         }
@@ -116,7 +119,7 @@ impl telepathy::ConnectionManager for ConnectionManager {
         println!("CM::request_connection({}, ...)", protocol);
 
         match protocol {
-            super::PROTO_NAME => self.create_connection(params),
+            super::PROTO_NAME => self.create_connection(super::ConnSettings::from_params(params)?),
             _ => Err(MethodErr::no_arg()), // FIXME: should be NotImplemented?
         }
     }