Split the 'account sensitive' parts of the Contacts class and move them to an intermediate Contacts_New class.

The Contact class remains the public interface for contact handling. This is only a single step of a longer refactoring to empower the Account class.

src/common/account.py

@@ -20,7 +20,9 @@
 
 class Account(object):
 	
-	def __init__(self, gc_contacts):
+	def __init__(self, name, contacts, gc_contacts):
+		self.name = name
+		self.contacts = contacts
 		self.gc_contacts = gc_contacts
 		
 		

src/common/contacts.py

@@ -174,7 +174,6 @@ class Contact(CommonContact):
 		return False
 
 
-
 class GC_Contact(CommonContact):
 	'''Information concerning each groupchat contact'''
 	def __init__(self, room_jid, account, name='', show='', status='', role='',
@@ -204,29 +203,25 @@ class GC_Contact(CommonContact):
 class Contacts:
 	'''Information concerning all contacts and groupchat contacts'''
 	def __init__(self):
-		self._contacts = {} # list of contacts {acct: {jid1: [C1, C2]}, } one Contact per resource
+		
 		self._metacontact_manager = MetacontactManager(self)
 		self._accounts = {}
 		
 	def change_account_name(self, old_name, new_name):		
-		self._contacts[new_name] = self._contacts[old_name]
-		del self._contacts[old_name]
-		
 		self._accounts[new_name] = self._accounts[old_name]
 		del self._accounts[old_name]
 		
 		self._metacontact_manager.change_account_name(old_name, new_name)
 
-	def add_account(self, account):
+	def add_account(self, account_name):
-		self._contacts[account] = {}
+		self._accounts[account_name] = Account(account_name,
-		self._accounts[account] = Account(GC_Contacts())
+				Contacts_New(), GC_Contacts())
-		self._metacontact_manager.add_account(account)
+		self._metacontact_manager.add_account(account_name)
 
 	def get_accounts(self):
-		return self._contacts.keys()
+		return self._accounts.keys()
 
 	def remove_account(self, account):
-		del self._contacts[account]
 		del self._accounts[account]
 		self._metacontact_manager.remove_account(account)
 
@@ -264,82 +259,41 @@ class Contacts:
 			chatstate=contact.chatstate, last_status_time=contact.last_status_time)
 
 	def add_contact(self, account, contact):
-		assert account == contact.account # migration check
+		if account not in self._accounts:
-		
+			self.add_account(account)
-		# No such account before ?
+		return self._accounts[account].contacts.add_contact(contact)
-		if account not in self._contacts:
-			self._contacts[account] = {contact.jid : [contact]}
-			return
-		# No such jid before ?
-		if contact.jid not in self._contacts[account]:
-			self._contacts[account][contact.jid] = [contact]
-			return
-		contacts = self._contacts[account][contact.jid]
-		# We had only one that was offline, remove it
-		if len(contacts) == 1 and contacts[0].show == 'offline':
-			# Do not use self.remove_contact: it deteles
-			# self._contacts[account][contact.jid]
-			contacts.remove(contacts[0])
-		# If same JID with same resource already exists, use the new one
-		for c in contacts:
-			if c.resource == contact.resource:
-				self.remove_contact(account, c)
-				break
-		contacts.append(contact)
 
 	def remove_contact(self, account, contact):
-		if account not in self._contacts:
+		if account not in self._accounts:
 			return
-		if contact.jid not in self._contacts[account]:
+		return self._accounts[account].contacts.remove_contact(contact)
-			return
-		if contact in self._contacts[account][contact.jid]:
-			self._contacts[account][contact.jid].remove(contact)
-		if len(self._contacts[account][contact.jid]) == 0:
-			del self._contacts[account][contact.jid]
 
 	def remove_jid(self, account, jid, remove_meta=True):
-		'''Removes all contacts for a given jid'''
+		self._accounts[account].contacts.remove_jid(jid)
-		if account not in self._contacts:
-			return
-		if jid not in self._contacts[account]:
-			return
-		del self._contacts[account][jid]
 		if remove_meta:
 			self._metacontact_manager.remove_metacontact(account, jid)
 		
 	def get_contacts(self, account, jid):
-		'''Returns the list of contact instances for this jid.'''
+		return self._accounts[account].contacts.get_contacts(jid)
-		if jid in self._contacts[account]:
-			return self._contacts[account][jid]
-		else:
-			return []
 
 	def get_contact(self, account, jid, resource=None):
-		### WARNING ###
+		return self._accounts[account].contacts.get_contact(jid, resource=resource)
-		# This function returns a *RANDOM* resource if resource = None!
-		# Do *NOT* use if you need to get the contact to which you
-		# send a message for example, as a bare JID in Jabber means
-		# highest available resource, which this function ignores!
-		'''Returns the contact instance for the given resource if it's given else
-		the first contact is no resource is given or None if there is not'''
-		if jid in self._contacts[account]:
-			if not resource:
-				return self._contacts[account][jid][0]
-			for c in self._contacts[account][jid]:
-				if c.resource == resource:
-					return c
-		return None
 
 	def iter_contacts(self, account):
-		if account in self._contacts:
+		for contact in self._accounts[account].contacts.iter_contacts():
-			for jid in self._contacts[account].keys():
-				for contact in self._contacts[account][jid][:]:
 			yield contact
 
 	def get_contact_from_full_jid(self, account, fjid):
-		''' Get Contact object for specific resource of given jid'''
+		return self._accounts[account].contacts.get_contact_from_full_jid(fjid)
-		barejid, resource = common.gajim.get_room_and_nick_from_fjid(fjid)
+	
-		return self.get_contact(account, barejid, resource)
+	def get_first_contact_from_jid(self, account, jid):
+		return self._accounts[account].contacts.get_first_contact_from_jid(jid)
+
+	def get_contacts_from_group(self, account, group):
+		return self._accounts[account].contacts.get_contacts_from_group(group)
+	
+	def get_jid_list(self, account):
+		return self._accounts[account].contacts.get_jid_list()
 
 	def get_highest_prio_contact_from_contacts(self, contacts):
 		if not contacts:
@@ -359,20 +313,6 @@ class Contacts:
 			return contact
 		return self.get_highest_prio_contact_from_contacts(contacts)
 	
-	def get_first_contact_from_jid(self, account, jid):
-		if jid in self._contacts[account]:
-			return self._contacts[account][jid][0]
-		return None
-
-	def get_contacts_from_group(self, account, group):
-		'''Returns all contacts in the given group'''
-		group_contacts = []
-		for jid in self._contacts[account]:
-			contacts = self.get_contacts(account, jid)
-			if group in contacts[0].groups:
-				group_contacts += contacts
-		return group_contacts
-
 	def get_nb_online_total_contacts(self, accounts=[], groups=[]):
 		'''Returns the number of online contacts and the total number of
 		contacts'''
@@ -423,9 +363,6 @@ class Contacts:
 			return False
 		return True
 	
-	def get_jid_list(self, account):
-		return self._contacts[account].keys()
-
 	def __getattr__(self, attr_name):
 		# Only called if self has no attr_name
 		if hasattr(self._metacontact_manager, attr_name):
@@ -460,6 +397,95 @@ class Contacts:
 		return self._accounts[account].gc_contacts.get_nb_role_total_gc_contacts(room_jid, role)
 		
 		
+class Contacts_New():
+	
+	def __init__(self):
+		# list of contacts  {jid1: [C1, C2]}, } one Contact per resource
+		self._contacts = {}
+		
+	def add_contact(self, contact):		
+		if contact.jid not in self._contacts:
+			self._contacts[contact.jid] = [contact]
+			return
+		contacts = self._contacts[contact.jid]
+		# We had only one that was offline, remove it
+		if len(contacts) == 1 and contacts[0].show == 'offline':
+			# Do not use self.remove_contact: it deteles
+			# self._contacts[account][contact.jid]
+			contacts.remove(contacts[0])
+		# If same JID with same resource already exists, use the new one
+		for c in contacts:
+			if c.resource == contact.resource:
+				self.remove_contact(c)
+				break
+		contacts.append(contact)
+		
+	def remove_contact(self, contact):
+		if contact.jid not in self._contacts:
+			return
+		if contact in self._contacts[contact.jid]:
+			self._contacts[contact.jid].remove(contact)
+		if len(self._contacts[contact.jid]) == 0:
+			del self._contacts[contact.jid]
+			
+	def remove_jid(self, jid):
+		'''Removes all contacts for a given jid'''
+		if jid not in self._contacts:
+			return
+		del self._contacts[jid]
+			
+	def get_contacts(self, jid):
+		'''Returns the list of contact instances for this jid.'''
+		if jid in self._contacts:
+			return self._contacts[jid]
+		else:
+			return []
+		
+	def get_contact(self, jid, resource=None):
+		### WARNING ###
+		# This function returns a *RANDOM* resource if resource = None!
+		# Do *NOT* use if you need to get the contact to which you
+		# send a message for example, as a bare JID in Jabber means
+		# highest available resource, which this function ignores!
+		'''Returns the contact instance for the given resource if it's given else
+		the first contact is no resource is given or None if there is not'''
+		if jid in self._contacts:
+			if not resource:
+				return self._contacts[jid][0]
+			for c in self._contacts[jid]:
+				if c.resource == resource:
+					return c
+		return None
+
+	def iter_contacts(self):
+		for jid in self._contacts.keys():
+			for contact in self._contacts[jid][:]:
+				yield contact
+				
+	def get_jid_list(self):
+		return self._contacts.keys()
+				
+	def get_contact_from_full_jid(self, fjid):
+		''' Get Contact object for specific resource of given jid'''
+		barejid, resource = common.gajim.get_room_and_nick_from_fjid(fjid)
+		return self.get_contact(barejid, resource)
+	
+	def get_first_contact_from_jid(self, jid):
+		if jid in self._contacts:
+			return self._contacts[jid][0]
+		return None
+
+	def get_contacts_from_group(self, group):
+		'''Returns all contacts in the given group'''
+		group_contacts = []
+		for jid in self._contacts:
+			contacts = self.get_contacts(jid)
+			if group in contacts[0].groups:
+				group_contacts += contacts
+		return group_contacts
+	
+	
+				
 class GC_Contacts():
 	
 	def __init__(self):

test/unit/test_account.py

@@ -11,9 +11,11 @@ from common.account import Account
 class Test(unittest.TestCase):
 
 	def testInstantiate(self):
-		account = Account(gc_contacts=None)
+		account = Account(name='MyAcc', contacts=None, gc_contacts=None)
 		
+		self.assertEquals('MyAcc', account.name)
 		self.assertTrue(account.gc_contacts is None)
+		self.assertTrue(account.contacts is None)
 
 if __name__ == "__main__":
 	unittest.main()

test/unit/test_contacts.py

@@ -98,6 +98,32 @@ class TestContacts(unittest.TestCase):
 		# Not yet implemented to remain backwart compatible
 		# self.assertEqual(contact, copy, msg="Must be equal")
 		
+	def test_legacy_accounts_handling(self):		
+		self.contacts.add_account("one")
+		self.contacts.add_account("two")
+		
+		self.contacts.change_account_name("two", "old")
+		self.contacts.remove_account("one")
+		
+		self.assertEqual(["old"], self.contacts.get_accounts())
+		
+	def test_legacy_contacts_from_groups(self):
+		jid1 = "test1@gajim.org"
+		jid2 = "test2@gajim.org"
+		account = "account"
+		group = "GroupA"
+		
+		contact1 = self.contacts.create_contact(jid=jid1, account=account,
+			groups=[group])
+		self.contacts.add_contact(account, contact1)
+		
+		contact2 = self.contacts.create_contact(jid=jid2, account=account,
+			groups=[group])
+		self.contacts.add_contact(account, contact2)
+		
+		self.assertEqual(2, len(self.contacts.get_contacts_from_group(account, group)))
+		self.assertEqual(0, len(self.contacts.get_contacts_from_group(account, '')))							
+				
 		
 if __name__ == "__main__":
 		unittest.main()