diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java index 473d6a833..775f7c716 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java @@ -491,9 +491,8 @@ abstract class ProtocolEngineImpl return abortWithMessage(txn, s); // Broadcast event informing that contact left ContactId contactId = getContactId(txn, s.getContactGroupId()); - ContactLeftShareableEvent e = - new ContactLeftShareableEvent(s.getShareableId(), - contactId); + ContactLeftShareableEvent e = new ContactLeftShareableEvent( + s.getShareableId(), contactId); txn.attach(e); // Stop sharing the shareable with the contact setShareableVisibility(txn, s, INVISIBLE); diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParser.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParser.java index eec97355f..2467573b6 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParser.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParser.java @@ -13,6 +13,8 @@ interface SessionParser { BdfDictionary getAllSessionsQuery(); + boolean isSession(BdfDictionary d); + Session parseSession(GroupId contactGroupId, BdfDictionary d) throws FormatException; diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParserImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParserImpl.java index ee0c06f61..1c2eaa498 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParserImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SessionParserImpl.java @@ -39,6 +39,11 @@ class SessionParserImpl implements SessionParser { return BdfDictionary.of(new BdfEntry(SESSION_KEY_IS_SESSION, true)); } + @Override + public boolean isSession(BdfDictionary d) { + return d.getBoolean(SESSION_KEY_IS_SESSION, false); + } + @Override public Session parseSession(GroupId contactGroupId, BdfDictionary d) throws FormatException { 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 194bdb41f..0f8848bc5 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 @@ -37,6 +37,7 @@ import org.briarproject.briar.client.ConversationClientImpl; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -541,8 +542,91 @@ abstract class SharingManagerImpl @Override public boolean deleteAllMessages(Transaction txn, ContactId c) throws DbException { - // TODO actually delete messages (#1627 and #1629) - return getMessageIds(txn, c).size() == 0; + // get ID of the contact group + GroupId g = getContactGroup(db.getContact(txn, c)).getId(); + + // get metadata for all messages in the + // (these are sessions *and* protocol messages) + Map metadata; + try { + metadata = clientHelper.getMessageMetadataAsDictionary(txn, g); + } catch (FormatException e) { + throw new DbException(e); + } + + // 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())); + } + + // assign protocol messages to their sessions + for (Entry entry : metadata.entrySet()) { + // skip all sessions, we are only interested in messages + BdfDictionary d = entry.getValue(); + if (sessionParser.isSession(d)) continue; + + // parse message metadata and skip messages not visible in UI + MessageMetadata m; + try { + m = messageParser.parseMetadata(d); + } catch (FormatException e) { + throw new DbException(e); + } + if (!m.isVisibleInConversation()) continue; + + // add visible messages to session + DeletableSession session = sessions.get(m.getShareableId()); + session.messages.add(entry.getKey()); + } + + // get a set of all messages which were not ACKed by the contact + Set notAcked = new HashSet<>(); + for (MessageStatus status : db.getMessageStatus(txn, c, g)) { + if (!status.isSeen()) notAcked.add(status.getMessageId()); + } + boolean allDeleted = + deleteCompletedSessions(txn, sessions.values(), notAcked); + recalculateGroupCount(txn, g); + return allDeleted; + } + + private boolean deleteCompletedSessions(Transaction txn, + Collection sessions, Set notAcked) + throws DbException { + // find completed sessions to delete + boolean allDeleted = true; + for (DeletableSession session : sessions) { + if (session.state.isAwaitingResponse()) { + allDeleted = false; + continue; + } + // we can only delete sessions + // where delivery of all messages was confirmed (aka ACKed) + boolean allAcked = true; + for (MessageId m : session.messages) { + if (notAcked.contains(m)) { + allAcked = false; + allDeleted = false; + break; + } + } + if (allAcked) { + for (MessageId m : session.messages) { + db.deleteMessage(txn, m); + db.deleteMessageMetadata(txn, m); + } + } + } + return allDeleted; } private Set getMessageIds(Transaction txn, ContactId c) @@ -558,6 +642,31 @@ abstract class SharingManagerImpl } } + private void recalculateGroupCount(Transaction txn, GroupId g) + throws DbException { + BdfDictionary query = messageParser.getMessagesVisibleInUiQuery(); + Map results; + try { + results = + clientHelper.getMessageMetadataAsDictionary(txn, g, query); + } catch (FormatException e) { + throw new DbException(e); + } + int msgCount = 0; + int unreadCount = 0; + for (Entry entry : results.entrySet()) { + MessageMetadata meta; + try { + meta = messageParser.parseMetadata(entry.getValue()); + } catch (FormatException e) { + throw new DbException(e); + } + msgCount++; + if (!meta.isRead()) unreadCount++; + } + messageTracker.resetGroupCount(txn, g, msgCount, unreadCount); + } + private static class StoredSession { private final MessageId storageId; @@ -569,4 +678,14 @@ abstract class SharingManagerImpl } } + private static class DeletableSession { + + private final State state; + private final List messages = new ArrayList<>(); + + private DeletableSession(State state) { + this.state = state; + } + } + } diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java index 2d0ed9766..18defd671 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java @@ -15,10 +15,19 @@ import static org.briarproject.bramble.api.sync.Group.Visibility.VISIBLE; enum State { START(0, INVISIBLE), + /** + * The local user has been invited to the shareable, but not yet responded. + */ LOCAL_INVITED(1, INVISIBLE), + /** + * The remote user has been invited to the shareable, but not yet responded. + */ REMOTE_INVITED(2, VISIBLE), SHARING(3, SHARED), LOCAL_LEFT(4, INVISIBLE), + /** + * The local user has left the shareable, but the remote user hasn't. + */ REMOTE_HANGING(5, INVISIBLE); private final int value; @@ -41,6 +50,10 @@ enum State { return this == START; } + public boolean isAwaitingResponse() { + return this == LOCAL_INVITED || this == REMOTE_INVITED; + } + static State fromValue(int value) throws FormatException { for (State s : values()) if (s.value == value) return s; throw new FormatException(); 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 c96fb486a..032f8863d 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 @@ -10,6 +10,7 @@ import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Group; +import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.TestDatabaseConfigModule; @@ -41,6 +42,7 @@ import static junit.framework.Assert.assertNotNull; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.briarproject.briar.api.forum.ForumSharingManager.CLIENT_ID; import static org.briarproject.briar.api.forum.ForumSharingManager.MAJOR_VERSION; +import static org.briarproject.briar.test.BriarTestUtils.assertGroupCount; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -124,9 +126,7 @@ public class ForumSharingIntegrationTest clock.currentTimeMillis()); // check that request message state is correct - Collection messages = - db0.transactionWithResult(true, txn -> forumSharingManager0 - .getMessageHeaders(txn, contactId1From0)); + Collection messages = getMessages1From0(); assertEquals(1, messages.size()); assertMessageState(messages.iterator().next(), true, false, false); @@ -139,8 +139,7 @@ public class ForumSharingIntegrationTest respondToRequest(contactId0From1, true); // check that accept message state is correct - messages = db1.transactionWithResult(true, txn -> forumSharingManager1 - .getMessageHeaders(txn, contactId0From1)); + messages = getMessages0From1(); assertEquals(2, messages.size()); for (ConversationMessageHeader h : messages) { if (h instanceof ConversationResponse) { @@ -158,9 +157,7 @@ public class ForumSharingIntegrationTest assertEquals(1, forumManager1.getForums().size()); // invitee has one invitation message from sharer - Collection list = - db1.transactionWithResult(true, txn -> forumSharingManager1 - .getMessageHeaders(txn, contactId0From1)); + Collection list = getMessages0From1(); assertEquals(2, list.size()); // check other things are alright with the forum message for (ConversationMessageHeader m : list) { @@ -179,9 +176,7 @@ public class ForumSharingIntegrationTest } } // sharer has own invitation message and response - assertEquals(2, db0.transactionWithResult(true, txn -> - forumSharingManager0.getMessageHeaders(txn, contactId1From0)) - .size()); + assertEquals(2, getMessages1From0().size()); // forum can not be shared again Contact c1 = contactManager0.getContact(contactId1From0); assertFalse(forumSharingManager0.canBeShared(forum.getId(), c1)); @@ -216,9 +211,7 @@ public class ForumSharingIntegrationTest assertEquals(0, forumSharingManager1.getInvitations().size()); // invitee has one invitation message from sharer and one response - Collection list = - db1.transactionWithResult(true, txn -> forumSharingManager1 - .getMessageHeaders(txn, contactId0From1)); + Collection list = getMessages0From1(); assertEquals(2, list.size()); // check things are alright with the forum message for (ConversationMessageHeader m : list) { @@ -237,9 +230,7 @@ public class ForumSharingIntegrationTest } } // sharer has own invitation message and response - assertEquals(2, db0.transactionWithResult(true, txn -> - forumSharingManager0.getMessageHeaders(txn, contactId1From0)) - .size()); + assertEquals(2, getMessages1From0().size()); // forum can be shared again Contact c1 = contactManager0.getContact(contactId1From0); assertTrue(forumSharingManager0.canBeShared(forum.getId(), c1)); @@ -507,12 +498,8 @@ public class ForumSharingIntegrationTest .contains(contact0From1)); // and both have each other's invitations (and no response) - assertEquals(2, db0.transactionWithResult(true, txn -> - forumSharingManager0.getMessageHeaders(txn, contactId1From0)) - .size()); - assertEquals(2, db1.transactionWithResult(true, txn -> - forumSharingManager1.getMessageHeaders(txn, contactId0From1)) - .size()); + assertEquals(2, getMessages1From0().size()); + assertEquals(2, getMessages0From1().size()); // there are no more open invitations assertTrue(forumSharingManager0.getInvitations().isEmpty()); @@ -768,10 +755,7 @@ public class ForumSharingIntegrationTest // get invitation MessageId for later MessageId invitationId = null; - Collection list = - db1.transactionWithResult(true, txn -> forumSharingManager1 - .getMessageHeaders(txn, contactId0From1)); - for (ConversationMessageHeader m : list) { + for (ConversationMessageHeader m : getMessages0From1()) { if (m instanceof ForumInvitationRequest) { invitationId = m.getId(); } @@ -829,6 +813,146 @@ public class ForumSharingIntegrationTest .contains(contact0From1)); } + @Test + public void testDeletingAllMessagesWhenCompletingSession() + throws Exception { + // send invitation + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, clock.currentTimeMillis()); + sync0To1(1, true); + eventWaiter.await(TIMEOUT, 1); + + // messages can not be deleted + assertFalse(deleteAllMessages1From0()); + assertFalse(deleteAllMessages0From1()); + + // accept invitation + respondToRequest(contactId0From1, true); + sync1To0(1, true); + eventWaiter.await(TIMEOUT, 1); + + // check that messages are tracked properly + GroupId g1From0 = + forumSharingManager0.getContactGroup(contact1From0).getId(); + GroupId g0From1 = + forumSharingManager1.getContactGroup(contact0From1).getId(); + assertGroupCount(messageTracker0, g1From0, 2, 1); + assertGroupCount(messageTracker1, g0From1, 2, 1); + + // 0 deletes all messages + assertTrue(deleteAllMessages1From0()); + assertEquals(0, getMessages1From0().size()); + assertGroupCount(messageTracker0, g1From0, 0, 0); + + // 1 can not delete all messages, as last one has not been ACKed + assertFalse(deleteAllMessages0From1()); + assertGroupCount(messageTracker1, g0From1, 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(deleteAllMessages0From1()); + assertEquals(0, getMessages0From1().size()); + assertGroupCount(messageTracker1, g0From1, 0, 0); + + // both leave forum and send LEAVE message + forumManager0.removeForum(forum); + sync0To1(1, true); + + // sending invitation is possible again + forumSharingManager1.sendInvitation(forum.getId(), contactId0From1, + null, clock.currentTimeMillis()); + sync1To0(1, true); + eventWaiter.await(TIMEOUT, 1); + + // messages can not be deleted anymore + assertFalse(deleteAllMessages1From0()); + assertEquals(1, getMessages1From0().size()); + assertGroupCount(messageTracker0, g1From0, 1, 1); + assertFalse(deleteAllMessages0From1()); + assertEquals(1, getMessages0From1().size()); + assertGroupCount(messageTracker1, g0From1, 1, 0); + + // 0 accepts re-share + forumSharingManager0.respondToInvitation(forum, contact1From0, true); + sync0To1(1, true); + + // 1 sends an ACK to their last message + sendAcks(c1, c0, contactId0From1, 1); + + // messages can now get deleted again + assertTrue(deleteAllMessages1From0()); + assertEquals(0, getMessages1From0().size()); + assertGroupCount(messageTracker0, g1From0, 0, 0); + assertTrue(deleteAllMessages0From1()); + assertEquals(0, getMessages0From1().size()); + assertGroupCount(messageTracker1, g0From1, 0, 0); + } + + @Test + public void testDeletingAllMessagesAfterDecline() + throws Exception { + // send invitation + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, clock.currentTimeMillis()); + sync0To1(1, true); + eventWaiter.await(TIMEOUT, 1); + + // decline invitation + respondToRequest(contactId0From1, false); + sync1To0(1, true); + eventWaiter.await(TIMEOUT, 1); + + // 0 deletes all messages + assertTrue(deleteAllMessages1From0()); + assertEquals(0, getMessages1From0().size()); + + // 1 can not delete all messages, as last one has not been ACKed + assertFalse(deleteAllMessages0From1()); + + // 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(deleteAllMessages0From1()); + assertEquals(0, getMessages0From1().size()); + + // re-sending invitation is possible + forumSharingManager0.sendInvitation(forum.getId(), contactId1From0, + null, clock.currentTimeMillis()); + sync0To1(1, true); + eventWaiter.await(TIMEOUT, 1); + + // messages can not be deleted anymore + assertFalse(deleteAllMessages1From0()); + assertEquals(1, getMessages1From0().size()); + assertFalse(deleteAllMessages0From1()); + assertEquals(1, getMessages0From1().size()); + } + + private Collection getMessages1From0() + throws DbException { + return db0.transactionWithResult(true, txn -> forumSharingManager0 + .getMessageHeaders(txn, contactId1From0)); + } + + private Collection getMessages0From1() + throws DbException { + return db1.transactionWithResult(true, txn -> forumSharingManager1 + .getMessageHeaders(txn, contactId0From1)); + } + + private boolean deleteAllMessages1From0() throws DbException { + return db0.transactionWithResult(false, txn -> forumSharingManager0 + .deleteAllMessages(txn, contactId1From0)); + } + + private boolean deleteAllMessages0From1() throws DbException { + return db1.transactionWithResult(false, txn -> forumSharingManager1 + .deleteAllMessages(txn, contactId0From1)); + } + private void respondToRequest(ContactId contactId, boolean accept) throws DbException { assertEquals(1, forumSharingManager1.getInvitations().size()); @@ -883,7 +1007,7 @@ public class ForumSharingIntegrationTest } } - void reset() { + private void reset() { requestReceived = responseReceived = responseAccepted = false; requestContactId = responseContactId = null; }