[core] implement subset conversation message deletion for SharingManager

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

@@ -539,13 +539,80 @@ abstract class SharingManagerImpl<S extends Shareable>
 		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, (contactGroup, 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(contactGroup, d);
+				} catch (FormatException e) {
+					throw new DbException(e);
+				}
+				sessions.put(session.getShareableId(),
+						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.getShareableId());
+					StoredSession ss = getSession(txn, g, sessionId);
+					if (ss == null) throw new DbException();
+					Session session = sessionParser
+							.parseSession(g, metadata.get(ss.storageId));
+					sessions.put(session.getShareableId(),
+							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();
 
-		// get metadata for all messages in the
+		// get metadata for all messages in the group
 		// (these are sessions *and* protocol messages)
 		Map<MessageId, BdfDictionary> metadata;
 		try {
@@ -555,18 +622,8 @@ abstract class SharingManagerImpl<S extends Shareable>
 		}
 
 		// get all sessions and their states
-		Map<GroupId, DeletableSession> sessions = new HashMap<>();
+		Map<GroupId, DeletableSession> sessions =
-		for (BdfDictionary d : metadata.values()) {
+				retriever.getDeletableSessions(g, metadata);
-			if (!sessionParser.isSession(d)) continue;
-			Session session;
-			try {
-				session = sessionParser.parseSession(g, d);
-			} catch (FormatException e) {
-				throw new DbException(e);
-			}
-			sessions.put(session.getShareableId(),
-					new DeletableSession(session.getState()));
-		}
 
 		// assign protocol messages to their sessions
 		for (Entry<MessageId, BdfDictionary> entry : metadata.entrySet()) {
@@ -585,7 +642,7 @@ abstract class SharingManagerImpl<S extends Shareable>
 
 			// add visible messages to session
 			DeletableSession session = sessions.get(m.getShareableId());
-			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
@@ -593,15 +650,15 @@ abstract class SharingManagerImpl<S extends Shareable>
 		for (MessageStatus status : db.getMessageStatus(txn, c, g)) {
 			if (!status.isSeen()) notAcked.add(status.getMessageId());
 		}
-		boolean allDeleted =
+		boolean allDeleted = deleteCompletedSessions(txn, sessions.values(),
-				deleteCompletedSessions(txn, sessions.values(), notAcked);
+				notAcked, checker);
 		recalculateGroupCount(txn, g);
 		return allDeleted;
 	}
 
 	private boolean deleteCompletedSessions(Transaction txn,
-			Collection<DeletableSession> sessions, Set<MessageId> notAcked)
+			Collection<DeletableSession> sessions, Set<MessageId> notAcked,
-			throws DbException {
+			MessageDeletionChecker checker) throws DbException {
 		// find completed sessions to delete
 		boolean allDeleted = true;
 		for (DeletableSession session : sessions) {
@@ -613,7 +670,7 @@ abstract class SharingManagerImpl<S extends Shareable>
 			// 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;
@@ -629,12 +686,6 @@ abstract class SharingManagerImpl<S extends Shareable>
 		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/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java

@@ -35,9 +35,12 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 
 import javax.annotation.Nullable;
 
+import static java.util.Collections.emptySet;
 import static junit.framework.Assert.assertNotNull;
 import static org.briarproject.bramble.util.StringUtils.getRandomString;
 import static org.briarproject.briar.api.forum.ForumSharingManager.CLIENT_ID;
@@ -187,9 +190,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testDeclinedSharing() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, null,
+				null, clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -256,9 +258,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testInviteeLeavesAfterFinished() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -320,9 +321,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testSharerLeavesAfterFinished() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, null,
+				null, clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -398,9 +398,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testSharerLeavesBeforeResponse() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, null,
+				null, clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sharer un-subscribes from forum
 		forumManager0.removeForum(forum);
@@ -445,9 +444,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testSharingSameForumWithEachOther() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -486,9 +484,8 @@ public class ForumSharingIntegrationTest
 	public void testSharingSameForumWithEachOtherBeforeAccept()
 			throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 		sync0To1(1, true);
 		eventWaiter.await(TIMEOUT, 1);
 		assertRequestReceived(listener1, contactId0From1);
@@ -556,9 +553,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testContactRemoved() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -683,9 +679,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testSyncAfterReSharing() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -791,9 +786,8 @@ public class ForumSharingIntegrationTest
 	@Test
 	public void testSessionResetAfterAbort() throws Exception {
 		// send invitation
-		forumSharingManager0
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
-				.sendInvitation(forum.getId(), contactId1From0, "Hi!",
+				"Hi!", clock.currentTimeMillis());
-						clock.currentTimeMillis());
 
 		// sync request message
 		sync0To1(1, true);
@@ -981,6 +975,61 @@ public class ForumSharingIntegrationTest
 		assertEquals(1, getMessages0From1().size());
 	}
 
+	@Test
+	public void testDeletingSomeMessages() throws Exception {
+		// send invitation
+		forumSharingManager0.sendInvitation(forum.getId(), contactId1From0,
+				null, clock.currentTimeMillis());
+		sync0To1(1, true);
+		eventWaiter.await(TIMEOUT, 1);
+
+		// deleting the invitation will fail
+		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));
+
+		// decline invitation
+		respondToRequest(contactId0From1, true);
+		sync1To0(1, true);
+		eventWaiter.await(TIMEOUT, 1);
+
+		// 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
+		m0 = getMessages1From0();
+		assertEquals(2, m0.size());
+		for (ConversationMessageHeader h : m0) {
+			if (!h.getId().equals(messageId)) toDelete.add(h.getId());
+		}
+		assertTrue(deleteMessages1From0(toDelete));
+		assertEquals(0, getMessages1From0().size());
+		// a second time nothing happens
+		assertTrue(deleteMessages1From0(toDelete));
+
+		// 1 can still not delete the messages, as last one has not been ACKed
+		assertFalse(deleteMessages0From1(toDelete));
+
+		// 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());
+		// 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 -> forumSharingManager0
@@ -1003,6 +1052,18 @@ public class ForumSharingIntegrationTest
 				.deleteAllMessages(txn, contactId0From1));
 	}
 
+	private boolean deleteMessages1From0(Set<MessageId> toDelete)
+			throws DbException {
+		return db0.transactionWithResult(false, txn -> forumSharingManager0
+				.deleteMessages(txn, contactId1From0, toDelete));
+	}
+
+	private boolean deleteMessages0From1(Set<MessageId> toDelete)
+			throws DbException {
+		return db1.transactionWithResult(false, txn -> forumSharingManager1
+				.deleteMessages(txn, contactId0From1, toDelete));
+	}
+
 	private void respondToRequest(ContactId contactId, boolean accept)
 			throws DbException {
 		assertEquals(1, forumSharingManager1.getInvitations().size());