[core] implement subset conversation message deletion for GroupInvitationManager

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

@@ -612,29 +612,82 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 				.getMessageMetadataAsDictionary(txn, contactGroupId, query);
 		Map<GroupId, Visibility> m = new HashMap<>();
 		for (BdfDictionary d : results.values()) {
-			Role role = sessionParser.getRole(d);
-			if (role == CREATOR) {
-				CreatorSession s =
-						sessionParser.parseCreatorSession(contactGroupId, d);
-				m.put(s.getPrivateGroupId(), s.getState().getVisibility());
-			} else if (role == INVITEE) {
-				InviteeSession s =
-						sessionParser.parseInviteeSession(contactGroupId, d);
-				m.put(s.getPrivateGroupId(), s.getState().getVisibility());
-			} else if (role == PEER) {
-				PeerSession s =
-						sessionParser.parsePeerSession(contactGroupId, d);
-				m.put(s.getPrivateGroupId(), s.getState().getVisibility());
-			} else {
-				throw new AssertionError();
-			}
+			Session s = sessionParser.parseSession(contactGroupId, d);
+			m.put(s.getPrivateGroupId(), s.getState().getVisibility());
 		}
 		return m;
 	}
 
+	@FunctionalInterface
+	private interface DeletableSessionRetriever {
+		Map<GroupId, DeletableSession> getDeletableSessions(
+				GroupId contactGroup, Map<MessageId, BdfDictionary> metadata)
+				throws DbException;
+	}
+
+	@FunctionalInterface
+	private interface MessageDeletionChecker {
+		/**
+		 * This is called for all messages belonging to a session.
+		 * It returns true if the given {@link MessageId} causes a problem
+		 * so that the session can not be deleted.
+		 */
+		boolean causesProblem(MessageId messageId) throws DbException;
+	}
+
 	@Override
 	public boolean deleteAllMessages(Transaction txn, ContactId c)
 			throws DbException {
+		return deleteMessages(txn, c, (g, metadata) -> {
+			// get all sessions and their states
+			Map<GroupId, DeletableSession> sessions = new HashMap<>();
+			for (BdfDictionary d : metadata.values()) {
+				if (!sessionParser.isSession(d)) continue;
+				Session session;
+				try {
+					session = sessionParser.parseSession(g, d);
+				} catch (FormatException e) {
+					throw new DbException(e);
+				}
+				sessions.put(session.getPrivateGroupId(),
+						new DeletableSession(session.getState()));
+			}
+			return sessions;
+		}, messageId -> false);
+	}
+
+	@Override
+	public boolean deleteMessages(Transaction txn, ContactId c,
+			Set<MessageId> messageIds) throws DbException {
+		return deleteMessages(txn, c, (g, metadata) -> {
+			// get only sessions from given messageIds
+			Map<GroupId, DeletableSession> sessions = new HashMap<>();
+			for (MessageId messageId : messageIds) {
+				BdfDictionary d = metadata.get(messageId);
+				if (d == null) continue;  // throw new NoSuchMessageException()
+				try {
+					MessageMetadata messageMetadata =
+							messageParser.parseMetadata(d);
+					SessionId sessionId =
+							getSessionId(messageMetadata.getPrivateGroupId());
+					StoredSession ss = getSession(txn, g, sessionId);
+					if (ss == null) throw new DbException();
+					Session session = sessionParser
+							.parseSession(g, metadata.get(ss.storageId));
+					sessions.put(session.getPrivateGroupId(),
+							new DeletableSession(session.getState()));
+				} catch (FormatException e) {
+					throw new DbException(e);
+				}
+			}
+			return sessions;
+			// don't delete sessions if a message is not part of messageIds
+		}, messageId -> !messageIds.contains(messageId));
+	}
+
+	private boolean deleteMessages(Transaction txn, ContactId c,
+			DeletableSessionRetriever retriever, MessageDeletionChecker checker)
+			throws DbException {
 		// get ID of the contact group
 		GroupId g = getContactGroup(db.getContact(txn, c)).getId();
 
@@ -648,25 +701,8 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 		}
 
 		// get all sessions and their states
-		Map<GroupId, DeletableSession> sessions = new HashMap<>();
-		for (BdfDictionary d : metadata.values()) {
-			if (!sessionParser.isSession(d)) continue;
-			Session session;
-			try {
-				Role role = sessionParser.getRole(d);
-				if (role == CREATOR) {
-					session = sessionParser.parseCreatorSession(g, d);
-				} else if (role == INVITEE) {
-					session = sessionParser.parseInviteeSession(g, d);
-				} else if (role == PEER) {
-					session = sessionParser.parsePeerSession(g, d);
-				} else throw new AssertionError("unknown role");
-			} catch (FormatException e) {
-				throw new DbException(e);
-			}
-			sessions.put(session.getPrivateGroupId(),
-					new DeletableSession(session.getState()));
-		}
+		Map<GroupId, DeletableSession> sessions =
+				retriever.getDeletableSessions(g, metadata);
 
 		// assign protocol messages to their sessions
 		for (Entry<MessageId, BdfDictionary> entry : metadata.entrySet()) {
@@ -685,7 +721,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 
 			// add visible messages to session
 			DeletableSession session = sessions.get(m.getPrivateGroupId());
-			session.messages.add(entry.getKey());
+			if (session != null) session.messages.add(entry.getKey());
 		}
 
 		// get a set of all messages which were not ACKed by the contact
@@ -693,15 +729,15 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 		for (MessageStatus status : db.getMessageStatus(txn, c, g)) {
 			if (!status.isSeen()) notAcked.add(status.getMessageId());
 		}
-		boolean allDeleted =
-				deleteCompletedSessions(txn, sessions.values(), notAcked);
+		boolean allDeleted = deleteCompletedSessions(txn, sessions.values(),
+				notAcked, checker);
 		recalculateGroupCount(txn, g);
 		return allDeleted;
 	}
 
 	private boolean deleteCompletedSessions(Transaction txn,
-			Collection<DeletableSession> sessions, Set<MessageId> notAcked)
-			throws DbException {
+			Collection<DeletableSession> sessions, Set<MessageId> notAcked,
+			MessageDeletionChecker checker) throws DbException {
 		// find completed sessions to delete
 		boolean allDeleted = true;
 		for (DeletableSession session : sessions) {
@@ -713,7 +749,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 			// where delivery of all messages was confirmed (aka ACKed)
 			boolean allAcked = true;
 			for (MessageId m : session.messages) {
-				if (notAcked.contains(m)) {
+				if (notAcked.contains(m) || checker.causesProblem(m)) {
 					allAcked = false;
 					allDeleted = false;
 					break;
@@ -729,12 +765,6 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
 		return allDeleted;
 	}
 
-	@Override
-	public boolean deleteMessages(Transaction txn, ContactId c,
-			Set<MessageId> messageIds) throws DbException {
-		return false;
-	}
-
 	@Override
 	public Set<MessageId> getMessageIds(Transaction txn, ContactId c)
 			throws DbException {

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

@@ -17,6 +17,9 @@ interface SessionParser {
 
 	boolean isSession(BdfDictionary d);
 
+	Session parseSession(GroupId contactGroupId, BdfDictionary d)
+			throws FormatException;
+
 	CreatorSession parseCreatorSession(GroupId contactGroupId, BdfDictionary d)
 			throws FormatException;
 

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

@@ -53,6 +53,21 @@ class SessionParserImpl implements SessionParser {
 		return d.getBoolean(SESSION_KEY_IS_SESSION, false);
 	}
 
+	@Override
+	public Session parseSession(GroupId contactGroupId, BdfDictionary d)
+			throws FormatException {
+		Session session;
+		Role role = getRole(d);
+		if (role == CREATOR) {
+			session = parseCreatorSession(contactGroupId, d);
+		} else if (role == INVITEE) {
+			session = parseInviteeSession(contactGroupId, d);
+		} else if (role == PEER) {
+			session = parsePeerSession(contactGroupId, d);
+		} else throw new AssertionError("unknown role");
+		return session;
+	}
+
 	@Override
 	public CreatorSession parseCreatorSession(GroupId contactGroupId,
 			BdfDictionary d) throws FormatException {

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

@@ -2,6 +2,7 @@ package org.briarproject.briar.privategroup.invitation;
 
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.sync.Group;
+import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.TestDatabaseConfigModule;
 import org.briarproject.briar.api.client.ProtocolStateException;
 import org.briarproject.briar.api.conversation.ConversationMessageHeader;
@@ -19,9 +20,12 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 
 import javax.annotation.Nullable;
 
+import static java.util.Collections.emptySet;
 import static org.briarproject.briar.test.BriarTestUtils.assertGroupCount;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -35,6 +39,7 @@ public class GroupInvitationIntegrationTest
 	private PrivateGroupManager groupManager0, groupManager1;
 	private GroupInvitationManager groupInvitationManager0,
 			groupInvitationManager1;
+	private Group g1From0, g0From1;
 
 	@Before
 	@Override
@@ -45,6 +50,8 @@ public class GroupInvitationIntegrationTest
 		groupManager1 = c1.getPrivateGroupManager();
 		groupInvitationManager0 = c0.getGroupInvitationManager();
 		groupInvitationManager1 = c1.getGroupInvitationManager();
+		g1From0 = groupInvitationManager0.getContactGroup(contact1From0);
+		g0From1 = groupInvitationManager1.getContactGroup(contact0From1);
 
 		privateGroup =
 				privateGroupFactory.createPrivateGroup("Testgroup", author0);
@@ -467,8 +474,6 @@ public class GroupInvitationIntegrationTest
 		sync1To0(1, true);
 
 		// check group count
-		Group g1From0 = groupInvitationManager0.getContactGroup(contact1From0);
-		Group g0From1 = groupInvitationManager1.getContactGroup(contact0From1);
 		assertGroupCount(messageTracker0, g1From0.getId(), 2, 1);
 		assertGroupCount(messageTracker1, g0From1.getId(), 2, 1);
 
@@ -563,6 +568,65 @@ public class GroupInvitationIntegrationTest
 		assertGroupCount(messageTracker0, g1From0.getId(), 0, 0);
 	}
 
+	@Test
+	public void testDeletingSomeMessages() throws Exception {
+		// send invitation
+		sendInvitation(clock.currentTimeMillis(), null);
+		sync0To1(1, true);
+
+		// deleting the invitation will fail for both
+		Collection<ConversationMessageHeader> m0 = getMessages1From0();
+		assertEquals(1, m0.size());
+		MessageId messageId = m0.iterator().next().getId();
+		Set<MessageId> toDelete = new HashSet<>();
+		toDelete.add(messageId);
+		assertFalse(deleteMessages1From0(toDelete));
+		assertFalse(deleteMessages0From1(toDelete));
+
+		// respond
+		groupInvitationManager1
+				.respondToInvitation(contactId0From1, privateGroup, false);
+		sync1To0(1, true);
+
+		// both can still not delete the invitation,
+		// because the response was not selected for deletion as well
+		assertFalse(deleteMessages1From0(toDelete));
+		assertFalse(deleteMessages0From1(toDelete));
+
+		// after selecting response, both messages can be deleted by creator
+		m0 = getMessages1From0();
+		assertEquals(2, m0.size());
+		for (ConversationMessageHeader h : m0) {
+			if (!h.getId().equals(messageId)) toDelete.add(h.getId());
+		}
+		assertGroupCount(messageTracker0, g1From0.getId(), 2, 1);
+		assertTrue(deleteMessages1From0(toDelete));
+		assertEquals(0, getMessages1From0().size());
+		// a second time nothing happens
+		assertTrue(deleteMessages1From0(toDelete));
+		assertGroupCount(messageTracker0, g1From0.getId(), 0, 0);
+
+		// 1 can still not delete the messages, as last one has not been ACKed
+		assertFalse(deleteMessages0From1(toDelete));
+		assertEquals(2, getMessages0From1().size());
+		assertGroupCount(messageTracker1, g0From1.getId(), 2, 1);
+
+		// 0 sends an ACK to their last message
+		sendAcks(c0, c1, contactId1From0, 1);
+
+		// 1 can now delete all messages, as last one has been ACKed
+		assertTrue(deleteMessages0From1(toDelete));
+		assertEquals(0, getMessages0From1().size());
+		assertGroupCount(messageTracker1, g0From1.getId(), 0, 0);
+		// a second time nothing happens
+		assertTrue(deleteMessages0From1(toDelete));
+	}
+
+	@Test
+	public void testDeletingEmptySet() throws Exception {
+		assertTrue(deleteMessages0From1(emptySet()));
+	}
+
 	private Collection<ConversationMessageHeader> getMessages1From0()
 			throws DbException {
 		return db0.transactionWithResult(true, txn -> groupInvitationManager0
@@ -585,6 +649,18 @@ public class GroupInvitationIntegrationTest
 				.deleteAllMessages(txn, contactId0From1));
 	}
 
+	private boolean deleteMessages1From0(Set<MessageId> toDelete)
+			throws DbException {
+		return db0.transactionWithResult(false, txn -> groupInvitationManager0
+				.deleteMessages(txn, contactId1From0, toDelete));
+	}
+
+	private boolean deleteMessages0From1(Set<MessageId> toDelete)
+			throws DbException {
+		return db1.transactionWithResult(false, txn -> groupInvitationManager1
+				.deleteMessages(txn, contactId0From1, toDelete));
+	}
+
 	private void sendInvitation(long timestamp, @Nullable String text)
 			throws DbException {
 		byte[] signature = groupInvitationFactory.signInvitation(contact1From0,