diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java index efc6b0b9f..b998d57b3 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java @@ -539,13 +539,80 @@ abstract class SharingManagerImpl return m; } + @FunctionalInterface + private interface DeletableSessionRetriever { + Map getDeletableSessions( + GroupId contactGroup, Map 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 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 messageIds) throws DbException { + return deleteMessages(txn, c, (g, metadata) -> { + // get only sessions from given messageIds + Map 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 metadata; try { @@ -555,18 +622,8 @@ abstract class SharingManagerImpl } // get all sessions and their states - Map 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.getShareableId(), - new DeletableSession(session.getState())); - } + Map sessions = + retriever.getDeletableSessions(g, metadata); // assign protocol messages to their sessions for (Entry entry : metadata.entrySet()) { @@ -585,7 +642,7 @@ abstract class SharingManagerImpl // 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 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 sessions, Set notAcked) - throws DbException { + Collection sessions, Set notAcked, + MessageDeletionChecker checker) throws DbException { // find completed sessions to delete boolean allDeleted = true; for (DeletableSession session : sessions) { @@ -613,7 +670,7 @@ abstract class SharingManagerImpl // 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 return allDeleted; } - @Override - public boolean deleteMessages(Transaction txn, ContactId c, - Set messageIds) throws DbException { - return false; - } - @Override public Set getMessageIds(Transaction txn, ContactId c) throws DbException { diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java index ee02f2c95..20084eb98 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java +++ b/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 - .sendInvitation(forum.getId(), contactId1From0, null, - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -256,9 +258,8 @@ public class ForumSharingIntegrationTest @Test public void testInviteeLeavesAfterFinished() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -320,9 +321,8 @@ public class ForumSharingIntegrationTest @Test public void testSharerLeavesAfterFinished() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, null, - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -398,9 +398,8 @@ public class ForumSharingIntegrationTest @Test public void testSharerLeavesBeforeResponse() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, null, - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, 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 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -486,9 +484,8 @@ public class ForumSharingIntegrationTest public void testSharingSameForumWithEachOtherBeforeAccept() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", 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 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -683,9 +679,8 @@ public class ForumSharingIntegrationTest @Test public void testSyncAfterReSharing() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", clock.currentTimeMillis()); // sync request message sync0To1(1, true); @@ -791,9 +786,8 @@ public class ForumSharingIntegrationTest @Test public void testSessionResetAfterAbort() throws Exception { // send invitation - forumSharingManager0 - .sendInvitation(forum.getId(), contactId1From0, "Hi!", - clock.currentTimeMillis()); + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + "Hi!", 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 m0 = getMessages1From0(); + assertEquals(1, m0.size()); + MessageId messageId = m0.iterator().next().getId(); + Set 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 getMessages1From0() throws DbException { return db0.transactionWithResult(true, txn -> forumSharingManager0 @@ -1003,6 +1052,18 @@ public class ForumSharingIntegrationTest .deleteAllMessages(txn, contactId0From1)); } + private boolean deleteMessages1From0(Set toDelete) + throws DbException { + return db0.transactionWithResult(false, txn -> forumSharingManager0 + .deleteMessages(txn, contactId1From0, toDelete)); + } + + private boolean deleteMessages0From1(Set 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());