Remove redundant checks when adding contacts.

Hooks are now called exactly once per contact.

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java

@@ -65,7 +65,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 	public void createLocalState(Transaction txn) throws DbException {
 		if (db.containsGroup(txn, localGroup.getId())) return;
 		db.addGroup(txn, localGroup);
-		// Ensure we've set things up for any pre-existing contacts
+		// Set things up for any pre-existing contacts
 		for (Contact c : db.getContacts(txn)) addingContact(txn, c);
 	}
 
@@ -73,8 +73,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 	public void addingContact(Transaction txn, Contact c) throws DbException {
 		// Create a group to share with the contact
 		Group g = getContactGroup(c);
-		// Return if we've already set things up for this contact
-		if (db.containsGroup(txn, g.getId())) return;
 		// Store the group and share it with the contact
 		db.addGroup(txn, g);
 		db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);

bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java

@@ -29,6 +29,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import static java.util.Collections.singletonList;
 import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_ID;
 import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_VERSION;
 import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
@@ -87,39 +88,27 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 	@Test
 	public void testCreatesGroupsAtStartup() throws Exception {
 		Transaction txn = new Transaction(null, false);
-		Contact contact1 = getContact(true);
+		Contact contact = getContact(true);
-		Contact contact2 = getContact(true);
+		Group contactGroup = getGroup(CLIENT_ID, CLIENT_VERSION);
-		List<Contact> contacts = Arrays.asList(contact1, contact2);
-		Group contactGroup1 = getGroup(CLIENT_ID, CLIENT_VERSION);
-		Group contactGroup2 = getGroup(CLIENT_ID, CLIENT_VERSION);
 
 		context.checking(new Expectations() {{
 			oneOf(db).containsGroup(txn, localGroup.getId());
 			will(returnValue(false));
 			oneOf(db).addGroup(txn, localGroup);
 			oneOf(db).getContacts(txn);
-			will(returnValue(contacts));
+			will(returnValue(singletonList(contact)));
-			// The first contact's group has already been set up
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
-					CLIENT_VERSION, contact1);
+					CLIENT_VERSION, contact);
-			will(returnValue(contactGroup1));
+			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup1.getId());
+			oneOf(db).addGroup(txn, contactGroup);
-			will(returnValue(true));
+			oneOf(db).setGroupVisibility(txn, contact.getId(),
-			// The second contact's group hasn't been set up
+					contactGroup.getId(), SHARED);
-			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
-					CLIENT_VERSION, contact2);
-			will(returnValue(contactGroup2));
-			oneOf(db).containsGroup(txn, contactGroup2.getId());
-			will(returnValue(false));
-			oneOf(db).addGroup(txn, contactGroup2);
-			oneOf(db).setGroupVisibility(txn, contact2.getId(),
-					contactGroup2.getId(), SHARED);
 		}});
 		// Copy the latest local properties into the group
 		expectGetLocalProperties(txn);
-		expectStoreMessage(txn, contactGroup2.getId(), "foo", fooPropertiesDict,
+		expectStoreMessage(txn, contactGroup.getId(), "foo", fooPropertiesDict,
 				1, true, true);
-		expectStoreMessage(txn, contactGroup2.getId(), "bar", barPropertiesDict,
+		expectStoreMessage(txn, contactGroup.getId(), "bar", barPropertiesDict,
 				1, true, true);
 
 		TransportPropertyManagerImpl t = createInstance();
@@ -151,8 +140,6 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					CLIENT_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup.getId());
-			will(returnValue(false));
 			oneOf(db).addGroup(txn, contactGroup);
 			oneOf(db).setGroupVisibility(txn, contact.getId(),
 					contactGroup.getId(), SHARED);
@@ -538,7 +525,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 					fooPropertiesDict, 1, true, false);
 			// Store the new properties in each contact's group, version 1
 			oneOf(db).getContacts(txn);
-			will(returnValue(Collections.singletonList(contact)));
+			will(returnValue(singletonList(contact)));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					CLIENT_VERSION, contact);
 			will(returnValue(contactGroup));
@@ -597,7 +584,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(db).removeMessage(txn, localGroupUpdateId);
 			// Store the merged properties in each contact's group, version 2
 			oneOf(db).getContacts(txn);
-			will(returnValue(Collections.singletonList(contact)));
+			will(returnValue(singletonList(contact)));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					CLIENT_VERSION, contact);
 			will(returnValue(contactGroup));

briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java

@@ -58,7 +58,7 @@ class MessagingManagerImpl extends ConversationClientImpl
 				CLIENT_VERSION);
 		if (db.containsGroup(txn, localGroup.getId())) return;
 		db.addGroup(txn, localGroup);
-		// Ensure we've set things up for any pre-existing contacts
+		// Set things up for any pre-existing contacts
 		for (Contact c : db.getContacts(txn)) addingContact(txn, c);
 	}
 
@@ -67,8 +67,6 @@ class MessagingManagerImpl extends ConversationClientImpl
 		try {
 			// Create a group to share with the contact
 			Group g = getContactGroup(c);
-			// Return if we've already set things up for this contact
-			if (db.containsGroup(txn, g.getId())) return;
 			// Store the group and share it with the contact
 			db.addGroup(txn, g);
 			db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);

briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java

@@ -100,7 +100,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 				CLIENT_VERSION);
 		if (db.containsGroup(txn, localGroup.getId())) return;
 		db.addGroup(txn, localGroup);
-		// Ensure we've set things up for any pre-existing contacts
+		// Set things up for any pre-existing contacts
 		for (Contact c : db.getContacts(txn)) addingContact(txn, c);
 	}
 
@@ -108,8 +108,6 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 	public void addingContact(Transaction txn, Contact c) throws DbException {
 		// Create a group to share with the contact
 		Group g = getContactGroup(c);
-		// Return if we've already set things up for this contact
-		if (db.containsGroup(txn, g.getId())) return;
 		// Store the group and share it with the contact
 		db.addGroup(txn, g);
 		db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);

briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java

@@ -56,23 +56,17 @@ class BlogSharingManagerImpl extends SharingManagerImpl<Blog>
 		return CLIENT_VERSION;
 	}
 
-	/**
-	 * This is called during each startup for each existing Contact.
-	 */
 	@Override
 	public void addingContact(Transaction txn, Contact c) throws DbException {
-		// Return if we've already set things up for this contact
+		// Create a group to share with the contact
-		if (db.containsGroup(txn, getContactGroup(c).getId())) return;
-
-		// creates a group to share with the contact
 		super.addingContact(txn, c);
 
-		// get our blog and that of Contact c
+		// Get our blog and that of the contact
 		LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
 		Blog ourBlog = blogManager.getPersonalBlog(localAuthor);
 		Blog theirBlog = blogManager.getPersonalBlog(c.getAuthor());
 
-		// pre-share both blogs, if they have not been shared already
+		// Pre-share both blogs, if they have not been shared already
 		try {
 			preShareShareable(txn, c, ourBlog);
 			preShareShareable(txn, c, theirBlog);

briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java

@@ -87,7 +87,7 @@ abstract class SharingManagerImpl<S extends Shareable>
 				getClientVersion());
 		if (db.containsGroup(txn, localGroup.getId())) return;
 		db.addGroup(txn, localGroup);
-		// Ensure we've set things up for any pre-existing contacts
+		// Set things up for any pre-existing contacts
 		for (Contact c : db.getContacts(txn)) addingContact(txn, c);
 	}
 
@@ -96,8 +96,6 @@ abstract class SharingManagerImpl<S extends Shareable>
 		try {
 			// Create a group to share with the contact
 			Group g = getContactGroup(c);
-			// Return if we've already set things up for this contact
-			if (db.containsGroup(txn, g.getId())) return;
 			// Store the group and share it with the contact
 			db.addGroup(txn, g);
 			db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);
@@ -152,17 +150,17 @@ abstract class SharingManagerImpl<S extends Shareable>
 	 */
 	void preShareShareable(Transaction txn, Contact c, S shareable)
 			throws DbException, FormatException {
-		// return if a session already exists with that Contact
+		// Return if a session already exists with the contact
 		GroupId contactGroupId = getContactGroup(c).getId();
 		StoredSession existingSession = getSession(txn, contactGroupId,
 				getSessionId(shareable.getId()));
 		if (existingSession != null) return;
 
-		// add and shares the shareable with the Contact
+		// Add and share the shareable with the contact
 		db.addGroup(txn, shareable.getGroup());
 		db.setGroupVisibility(txn, c.getId(), shareable.getId(), SHARED);
 
-		// initialize session in sharing state
+		// Initialize session in sharing state
 		Session session = new Session(SHARING, contactGroupId,
 				shareable.getId(), null, null, 0, 0);
 		MessageId storageId = createStorageId(txn, contactGroupId);

briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java

@@ -159,7 +159,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 			oneOf(db).getContacts(txn);
 			will(returnValue(Collections.singletonList(contact)));
 		}});
-		expectAddingContact(contact, true);
+		expectAddingContact(contact);
 		groupInvitationManager.createLocalState(txn);
 	}
 
@@ -175,20 +175,14 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 		groupInvitationManager.createLocalState(txn);
 	}
 
-	private void expectAddingContact(Contact c, boolean contactExists)
+	private void expectAddingContact(Contact c) throws Exception {
-			throws Exception {
+		BdfDictionary meta = BdfDictionary
+				.of(new BdfEntry(GROUP_KEY_CONTACT_ID, c.getId().getInt()));
+
 		context.checking(new Expectations() {{
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					CLIENT_VERSION, c);
 			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup.getId());
-			will(returnValue(contactExists));
-		}});
-		if (contactExists) return;
-
-		BdfDictionary meta = BdfDictionary
-				.of(new BdfEntry(GROUP_KEY_CONTACT_ID, c.getId().getInt()));
-		context.checking(new Expectations() {{
 			oneOf(db).addGroup(txn, contactGroup);
 			oneOf(db).setGroupVisibility(txn, c.getId(), contactGroup.getId(),
 					SHARED);
@@ -197,8 +191,8 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 			oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
 					PrivateGroupManager.CLIENT_VERSION);
 			will(returnValue(Collections.singletonList(privateGroup)));
-			oneOf(privateGroupManager)
+			oneOf(privateGroupManager).isMember(txn, privateGroup.getId(),
-					.isMember(txn, privateGroup.getId(), c.getAuthor());
+					c.getAuthor());
 			will(returnValue(true));
 		}});
 		expectAddingMember(privateGroup.getId(), c);
@@ -255,7 +249,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAddingContact() throws Exception {
-		expectAddingContact(contact, false);
+		expectAddingContact(contact);
 		groupInvitationManager.addingContact(txn, contact);
 	}
 

briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java

@@ -91,7 +91,7 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
 	}
 
 	@Test
-	public void testCreateLocalStateFirstTimeWithExistingContactNotSetUp()
+	public void testCreateLocalStateFirstTimeWithExistingContact()
 			throws Exception {
 		Transaction txn = new Transaction(null, false);
 
@@ -119,19 +119,10 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
 		Map<MessageId, BdfDictionary> sessions = Collections.emptyMap();
 
 		context.checking(new Expectations() {{
-			// Check for contact group in BlogSharingManagerImpl
-			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
-					CLIENT_VERSION, contact);
-			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup.getId());
-			will(returnValue(false));
-			// Check for contact group again in SharingManagerImpl
-			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
-					CLIENT_VERSION, contact);
-			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup.getId());
-			will(returnValue(false));
 			// Create the contact group and share it with the contact
+			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
+					CLIENT_VERSION, contact);
+			will(returnValue(contactGroup));
 			oneOf(db).addGroup(txn, contactGroup);
 			oneOf(db).setGroupVisibility(txn, contactId, contactGroup.getId(),
 					SHARED);
@@ -151,33 +142,6 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
 		expectPreShareShareable(txn, contact, blog, sessions);
 	}
 
-	@Test
-	public void testCreateLocalStateFirstTimeWithExistingContactAlreadySetUp()
-			throws Exception {
-		Transaction txn = new Transaction(null, false);
-
-		context.checking(new Expectations() {{
-			// The local group doesn't exist - we need to set things up
-			oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID,
-					CLIENT_VERSION);
-			will(returnValue(localGroup));
-			oneOf(db).containsGroup(txn, localGroup.getId());
-			will(returnValue(false));
-			oneOf(db).addGroup(txn, localGroup);
-			// Get contacts
-			oneOf(db).getContacts(txn);
-			will(returnValue(contacts));
-			// The contact has already been set up
-			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
-					CLIENT_VERSION, contact);
-			will(returnValue(contactGroup));
-			oneOf(db).containsGroup(txn, contactGroup.getId());
-			will(returnValue(true));
-		}});
-
-		blogSharingManager.createLocalState(txn);
-	}
-
 	@Test
 	public void testCreateLocalStateSubsequentTime() throws Exception {
 		Transaction txn = new Transaction(null, false);