Merge branch '1483-peer-session-crash' into 'master'

Do not create PeerSession for groups we created

Closes #1483

See merge request briar/briar!1344

briar-api/src/main/java/org/briarproject/briar/api/privategroup/PrivateGroupManager.java

@@ -109,6 +109,11 @@ public interface PrivateGroupManager {
 	Collection<PrivateGroup> getPrivateGroups(Transaction txn)
 			throws DbException;
 
+	/**
+	 * Returns true if the given private group was created by us.
+	 */
+	boolean isOurPrivateGroup(Transaction txn, PrivateGroup g) throws DbException;
+
 	/**
 	 * Returns the text of the private group message with the given ID.
 	 */

briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java

@@ -287,6 +287,13 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook
 		return privateGroups;
 	}
 
+	@Override
+	public boolean isOurPrivateGroup(Transaction txn, PrivateGroup g)
+			throws DbException {
+		LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
+		return localAuthor.getId().equals(g.getCreator().getId());
+	}
+
 	@Override
 	public Collection<PrivateGroup> getPrivateGroups() throws DbException {
 		return db.transactionWithResult(true, this::getPrivateGroups);

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

@@ -128,15 +128,59 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 		// Attach the contact ID to the group
 		clientHelper.setContactId(txn, g.getId(), c.getId());
 		// If the contact belongs to any private groups, create a peer session
-		for (Group pg : db.getGroups(txn, PrivateGroupManager.CLIENT_ID,
+		// or sessions in LEFT state for creator/invitee.
+		for (Group group : db.getGroups(txn, PrivateGroupManager.CLIENT_ID,
 				PrivateGroupManager.MAJOR_VERSION)) {
-			if (privateGroupManager.isMember(txn, pg.getId(), c.getAuthor()))
-				addingMember(txn, pg.getId(), c);
+			if (privateGroupManager
+					.isMember(txn, group.getId(), c.getAuthor())) {
+				PrivateGroup pg =
+						privateGroupManager.getPrivateGroup(txn, group.getId());
+				recreateSession(txn, c, pg, g.getId());
+			}
+		}
+	}
+
+	private void recreateSession(Transaction txn, Contact c,
+			PrivateGroup pg, GroupId contactGroupId) throws DbException {
+		boolean isOur = privateGroupManager.isOurPrivateGroup(txn, pg);
+		boolean isTheirs =
+				c.getAuthor().getId().equals(pg.getCreator().getId());
+		if (isOur || isTheirs) {
+			// we are creator or invitee, create a left session for each role
+			MessageId storageId = createStorageId(txn, contactGroupId);
+			Session<?> session;
+			if (isOur) {
+				session = new CreatorSession(contactGroupId, pg.getId(), null,
+						null, 0, 0, CreatorState.LEFT);
+			} else {
+				session = new InviteeSession(contactGroupId, pg.getId(), null,
+						null, 0, 0, InviteeState.LEFT);
+			}
+			try {
+				storeSession(txn, storageId, session);
+			} catch (FormatException e) {
+				throw new DbException(e);
+			}
+		} else {
+			// we are neither creator nor invitee, create peer session
+			addingMember(txn, pg.getId(), c);
 		}
 	}
 
 	@Override
 	public void removingContact(Transaction txn, Contact c) throws DbException {
+		// mark private groups created by that contact as dissolved
+		for (Group g : db.getGroups(txn, PrivateGroupManager.CLIENT_ID,
+				PrivateGroupManager.MAJOR_VERSION)) {
+			if (privateGroupManager.isMember(txn, g.getId(), c.getAuthor())) {
+				PrivateGroup pg =
+						privateGroupManager.getPrivateGroup(txn, g.getId());
+				// check if contact to be removed is creator of the group
+				if (c.getAuthor().getId().equals(pg.getCreator().getId())) {
+					privateGroupManager.markGroupDissolved(txn, g.getId());
+				}
+			}
+		}
 		// Remove the contact group (all messages will be removed with it)
 		db.removeGroup(txn, getContactGroup(c));
 	}

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

@@ -709,6 +709,41 @@ public class GroupInvitationIntegrationTest
 		assertTrue(deleteMessages0From1(emptySet()).allDeleted());
 	}
 
+	@Test
+	public void testInvitationAfterReAddingContacts() throws Exception {
+		// sync invitation and response back
+		sendInvitation(c0.getClock().currentTimeMillis(), null);
+		sync0To1(1, true);
+		groupInvitationManager1
+				.respondToInvitation(contactId0From1, privateGroup, true);
+		sync1To0(1, true);
+
+		// sync group join messages
+		sync0To1(2, true); // + one invitation protocol join message
+		sync1To0(1, true);
+
+		// inviting again is not possible
+		assertFalse(groupInvitationManager0
+				.isInvitationAllowed(contact1From0, privateGroup.getId()));
+
+		// contacts remove each other
+		removeAllContacts();
+
+		// group gets dissolved for invitee automatically, but not creator
+		assertFalse(groupManager0.isDissolved(privateGroup.getId()));
+		assertTrue(groupManager1.isDissolved(privateGroup.getId()));
+
+		// contacts re-add each other
+		addDefaultContacts();
+
+		// creator can still not invite again
+		assertFalse(groupInvitationManager0
+				.isInvitationAllowed(contact1From0, privateGroup.getId()));
+
+		// finally invitee can remove group without issues
+		groupManager1.removePrivateGroup(privateGroup.getId());
+	}
+
 	private Collection<ConversationMessageHeader> getMessages1From0()
 			throws DbException {
 		return db0.transactionWithResult(true, txn -> groupInvitationManager0

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

@@ -45,6 +45,8 @@ import java.util.Map;
 import javax.annotation.Nullable;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static junit.framework.TestCase.fail;
 import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
 import static org.briarproject.bramble.api.sync.validation.IncomingMessageHook.DeliveryAction.ACCEPT_DO_NOT_SHARE;
@@ -106,7 +108,9 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 	private final ContactId contactId = contact.getId();
 	private final Group localGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
 	private final Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
-	private final Group privateGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
+	private final Group group = getGroup(CLIENT_ID, MAJOR_VERSION);
+	private final PrivateGroup privateGroup = new PrivateGroup(group,
+			getRandomString(5), getAuthor(), getRandomBytes(32));
 	private final BdfDictionary meta = BdfDictionary.of(new BdfEntry("m", "e"));
 	private final Message message = getMessage(contactGroup.getId());
 	private final BdfList body = BdfList.of("body");
@@ -160,9 +164,9 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 			will(returnValue(false));
 			oneOf(db).addGroup(txn, localGroup);
 			oneOf(db).getContacts(txn);
-			will(returnValue(Collections.singletonList(contact)));
+			will(returnValue(singletonList(contact)));
 		}});
-		expectAddingContact(contact);
+		expectAddingContact(contact, emptyList());
 		groupInvitationManager.onDatabaseOpened(txn);
 	}
 
@@ -178,7 +182,8 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 		groupInvitationManager.onDatabaseOpened(txn);
 	}
 
-	private void expectAddingContact(Contact c) throws Exception {
+	private void expectAddingContact(Contact c, Collection<Group> groups)
+			throws Exception {
 		context.checking(new Expectations() {{
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, c);
@@ -193,12 +198,8 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 					.setContactId(txn, contactGroup.getId(), contactId);
 			oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
 					PrivateGroupManager.MAJOR_VERSION);
-			will(returnValue(Collections.singletonList(privateGroup)));
-			oneOf(privateGroupManager).isMember(txn, privateGroup.getId(),
-					c.getAuthor());
-			will(returnValue(true));
+			will(returnValue(groups));
 		}});
-		expectAddingMember(privateGroup.getId(), c);
 	}
 
 	private void expectAddingMember(GroupId g, Contact c) throws Exception {
@@ -252,13 +253,99 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAddingContact() throws Exception {
-		expectAddingContact(contact);
+		expectAddingContact(contact, singletonList(group));
+
+		context.checking(new Expectations() {{
+			oneOf(privateGroupManager)
+					.isMember(txn, privateGroup.getId(), contact.getAuthor());
+			will(returnValue(true));
+			oneOf(privateGroupManager)
+					.getPrivateGroup(txn, privateGroup.getId());
+			will(returnValue(privateGroup));
+			oneOf(privateGroupManager).isOurPrivateGroup(txn, privateGroup);
+			will(returnValue(false));
+		}});
+		// creates PEER session
+		expectAddingMember(privateGroup.getId(), contact);
+
 		groupInvitationManager.addingContact(txn, contact);
 	}
 
 	@Test
-	public void testRemovingContact() throws Exception {
+	public void testAddingContactWhoCreatedGroup() throws Exception {
+		PrivateGroup privateGroup = new PrivateGroup(group,
+				getRandomString(5), contact.getAuthor(), getRandomBytes(32));
+
+		expectAddingContact(contact, singletonList(group));
+
 		context.checking(new Expectations() {{
+			oneOf(privateGroupManager)
+					.isMember(txn, privateGroup.getId(), contact.getAuthor());
+			will(returnValue(true));
+			oneOf(privateGroupManager)
+					.getPrivateGroup(txn, privateGroup.getId());
+			will(returnValue(privateGroup));
+			oneOf(privateGroupManager).isOurPrivateGroup(txn, privateGroup);
+			will(returnValue(false));
+		}});
+		expectCreateStorageId();
+		context.checking(new Expectations() {{
+			oneOf(sessionEncoder)
+					.encodeSession(with(any(InviteeSession.class)));
+			will(returnValue(meta));
+			oneOf(clientHelper)
+					.mergeMessageMetadata(txn, storageMessage.getId(), meta);
+		}});
+
+		groupInvitationManager.addingContact(txn, contact);
+	}
+
+	@Test
+	public void testRemovingContactWithoutCommonGroups() throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
+					PrivateGroupManager.MAJOR_VERSION);
+			will(returnValue(emptyList()));
+			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
+					MAJOR_VERSION, contact);
+			will(returnValue(contactGroup));
+			oneOf(db).removeGroup(txn, contactGroup);
+		}});
+		groupInvitationManager.removingContact(txn, contact);
+	}
+
+	@Test
+	public void testRemovingContactWithCommonGroups() throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
+					PrivateGroupManager.MAJOR_VERSION);
+			will(returnValue(singletonList(group)));
+			oneOf(privateGroupManager).isMember(txn, group.getId(), author);
+			will(returnValue(true));
+			oneOf(privateGroupManager).getPrivateGroup(txn, group.getId());
+			will(returnValue(privateGroup));
+			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
+					MAJOR_VERSION, contact);
+			will(returnValue(contactGroup));
+			oneOf(db).removeGroup(txn, contactGroup);
+		}});
+		groupInvitationManager.removingContact(txn, contact);
+	}
+
+	@Test
+	public void testRemovingContactWhoIsCreatorOfCommonGroup()
+			throws Exception {
+		PrivateGroup privateGroup = new PrivateGroup(group,
+				getRandomString(5), contact.getAuthor(), getRandomBytes(32));
+		context.checking(new Expectations() {{
+			oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
+					PrivateGroupManager.MAJOR_VERSION);
+			will(returnValue(singletonList(group)));
+			oneOf(privateGroupManager).isMember(txn, group.getId(), author);
+			will(returnValue(true));
+			oneOf(privateGroupManager).getPrivateGroup(txn, group.getId());
+			will(returnValue(privateGroup));
+			oneOf(privateGroupManager).markGroupDissolved(txn, group.getId());
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
@@ -355,8 +442,8 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 			BdfDictionary bdfSession) throws Exception {
 		expectParseMessageMetadata();
 		expectGetSession(oneResult, sessionId, contactGroup.getId());
-		Session<?> session =
-				expectHandleMessage(role, messageMetadata, bdfSession, type);
+		Session<?> session = expectHandleMessage(role, messageMetadata,
+				bdfSession, type);
 		expectStoreSession(session, storageMessage.getId());
 	}
 
@@ -569,7 +656,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAcceptInvitationWithGroupId() throws Exception {
-		PrivateGroup pg = new PrivateGroup(privateGroup,
+		PrivateGroup pg = new PrivateGroup(group,
 				getRandomString(MAX_GROUP_NAME_LENGTH), author,
 				getRandomBytes(GROUP_SALT_LENGTH));
 
@@ -579,7 +666,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 
 	@Test
 	public void testDeclineInvitationWithGroupId() throws Exception {
-		PrivateGroup pg = new PrivateGroup(privateGroup,
+		PrivateGroup pg = new PrivateGroup(group,
 				getRandomString(MAX_GROUP_NAME_LENGTH), author,
 				getRandomBytes(GROUP_SALT_LENGTH));
 
@@ -670,7 +757,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 						privateGroup.getId(), time1, "name", author,
 						new byte[0], null, new byte[0], NO_AUTO_DELETE_TIMER);
 		PrivateGroup pg =
-				new PrivateGroup(privateGroup, invite.getGroupName(),
+				new PrivateGroup(group, invite.getGroupName(),
 						invite.getCreator(), invite.getSalt());
 
 		context.checking(new Expectations() {{
@@ -738,7 +825,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 				new InviteMessage(message2.getId(), contactGroup.getId(),
 						privateGroup.getId(), time2, groupName, author, salt,
 						null, getRandomBytes(5), NO_AUTO_DELETE_TIMER);
-		PrivateGroup pg = new PrivateGroup(privateGroup, groupName,
+		PrivateGroup pg = new PrivateGroup(group, groupName,
 				author, salt);
 
 		context.checking(new Expectations() {{
@@ -747,7 +834,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 			oneOf(db).startTransaction(true);
 			will(returnValue(txn));
 			oneOf(db).getContacts(txn);
-			will(returnValue(Collections.singletonList(contact)));
+			will(returnValue(singletonList(contact)));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
@@ -839,7 +926,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
 		expectAddingMember(privateGroup.getId(), contact);
 		context.checking(new Expectations() {{
 			oneOf(db).getContactsByAuthorId(txn, author.getId());
-			will(returnValue(Collections.singletonList(contact)));
+			will(returnValue(singletonList(contact)));
 		}});
 		groupInvitationManager.addingMember(txn, privateGroup.getId(), author);
 	}

briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java

@@ -291,10 +291,10 @@ public abstract class BriarIntegrationTest<C extends BriarIntegrationTestCompone
 		contactManager0.removeContact(contactId2From0);
 		contactManager1.removeContact(contactId0From1);
 		contactManager2.removeContact(contactId0From2);
-		assertNotNull(contactId2From1);
-		contactManager1.removeContact(contactId2From1);
-		assertNotNull(contactId1From2);
-		contactManager2.removeContact(contactId1From2);
+		if (contactId2From1 != null)
+			contactManager1.removeContact(contactId2From1);
+		if (contactId1From2 != null)
+			contactManager2.removeContact(contactId1From2);
 	}
 
 	protected void setAutoDeleteTimer(BriarIntegrationTestComponent component,