[core] also delete attachments when deleting select messages

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<MessageId> messageIds) throws DbException {
-		for (MessageId messageId : messageIds) {
+		boolean allDeleted = true;
-			db.deleteMessage(txn, messageId);
+		for (MessageId m : messageIds) {
-			db.deleteMessageMetadata(txn, messageId);
+			// get attachment headers
+			List<AttachmentHeader> 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)

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<MessageId> 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 ->