From 6c489fbea3803bd4a4c503d9efe40428ab09d9cf Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 28 Oct 2019 10:22:04 -0300 Subject: [PATCH] [core] also delete attachments when deleting select messages --- .../briar/messaging/MessagingManagerImpl.java | 42 ++++++++++++-- .../MessagingManagerIntegrationTest.java | 55 +++++++++++++++++++ 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java index f17becb04..0003b8159 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.data.MetadataParser; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Metadata; +import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.lifecycle.LifecycleManager.OpenDatabaseHook; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -55,6 +56,7 @@ import javax.inject.Inject; import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; +import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; import static org.briarproject.bramble.util.IoUtils.copyAndClose; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT; @@ -438,13 +440,45 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook, @Override public boolean deleteMessages(Transaction txn, ContactId c, Set messageIds) throws DbException { - for (MessageId messageId : messageIds) { - db.deleteMessage(txn, messageId); - db.deleteMessageMetadata(txn, messageId); + boolean allDeleted = true; + for (MessageId m : messageIds) { + // get attachment headers + List headers; + try { + BdfDictionary meta = + clientHelper.getMessageMetadataAsDictionary(txn, m); + Long messageType = meta.getOptionalLong(MSG_KEY_MSG_TYPE); + if (messageType != null && messageType != PRIVATE_MESSAGE) + throw new AssertionError("not supported"); + headers = parseAttachmentHeaders(meta); + } catch (FormatException e) { + throw new DbException(e); + } + // check if all attachments have been delivered + boolean allAttachmentsDelivered = true; + try { + for (AttachmentHeader h : headers) { + if (db.getMessageState(txn, h.getMessageId()) != DELIVERED) + throw new NoSuchMessageException(); + } + } catch (NoSuchMessageException e) { + allAttachmentsDelivered = false; + } + // delete messages, if all attachments were delivered + if (allAttachmentsDelivered) { + for (AttachmentHeader h : headers) { + db.deleteMessage(txn, h.getMessageId()); + db.deleteMessageMetadata(txn, h.getMessageId()); + } + db.deleteMessage(txn, m); + db.deleteMessageMetadata(txn, m); + } else { + allDeleted = false; + } } GroupId g = getContactGroup(db.getContact(txn, c)).getId(); recalculateGroupCount(txn, g); - return true; + return allDeleted; } private void recalculateGroupCount(Transaction txn, GroupId g) diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java index aae06972c..211e7c060 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java @@ -29,7 +29,10 @@ import javax.annotation.Nullable; import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; +import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; +import static org.briarproject.bramble.api.sync.validation.MessageState.PENDING; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.briarproject.briar.test.BriarTestUtils.assertGroupCount; @@ -236,6 +239,58 @@ public class MessagingManagerIntegrationTest } } + @Test + public void testDeleteSomeAttachment() throws Exception { + // send one message with attachment + AttachmentHeader h = addAttachment(c0); + PrivateMessage m = + sendMessage(c0, c1, getRandomString(42), singletonList(h)); + + // attachment exists on both devices, state is set to PENDING + db0.transaction(false, txn -> { + db0.getMessage(txn, h.getMessageId()); + db0.setMessageState(txn, h.getMessageId(), PENDING); + }); + db1.transaction(false, txn -> { + db1.getMessage(txn, h.getMessageId()); + db1.setMessageState(txn, h.getMessageId(), PENDING); + }); + + // deleting message fails (on both sides), + // because attachment is not yet delivered + Set toDelete = singleton(m.getMessage().getId()); + assertFalse(db0.transactionWithResult(false, txn -> + messagingManager0.deleteMessages(txn, contactId, toDelete))); + assertFalse(db1.transactionWithResult(false, txn -> + messagingManager1.deleteMessages(txn, contactId, toDelete))); + + // deliver attachment + db0.transaction(false, + txn -> db0.setMessageState(txn, h.getMessageId(), DELIVERED)); + db1.transaction(false, + txn -> db1.setMessageState(txn, h.getMessageId(), DELIVERED)); + + // deleting message and attachment on both sides works now + assertTrue(db0.transactionWithResult(false, txn -> + messagingManager0.deleteMessages(txn, contactId, toDelete))); + assertTrue(db1.transactionWithResult(false, txn -> + messagingManager1.deleteMessages(txn, contactId, toDelete))); + + // attachment was deleted on both devices + try { + db0.transaction(true, txn -> db0.getMessage(txn, h.getMessageId())); + fail(); + } catch (MessageDeletedException e) { + // expected + } + try { + db1.transaction(true, txn -> db1.getMessage(txn, h.getMessageId())); + fail(); + } catch (MessageDeletedException e) { + // expected + } + } + @Test public void testDeletingEmptySet() throws Exception { assertTrue(db0.transactionWithResult(false, txn ->