This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 3098849c55f58bc70e1cf7c2864aba88b7ce0d4d Author: Benoit Tellier <[email protected]> AuthorDate: Sun Apr 12 15:46:50 2020 +0700 JAMES-3148 Also cleanup AttachmentMessageIdDAO upon deletion --- .../mailbox/cassandra/DeleteMessageListener.java | 9 +++++- .../mail/CassandraAttachmentMessageIdDAO.java | 18 +++++++++++ .../cassandra/CassandraMailboxManagerTest.java | 25 ++++++++++++++- .../mail/CassandraAttachmentMessageIdDAOTest.java | 37 ++++++++++++++++++++++ 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/DeleteMessageListener.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/DeleteMessageListener.java index 15d7b35..31529e6 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/DeleteMessageListener.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/DeleteMessageListener.java @@ -114,7 +114,8 @@ public class DeleteMessageListener implements MailboxListener.GroupMailboxListen return Mono.just(messageId) .filterWhen(this::isReferenced) .flatMap(id -> readMessage(id) - .flatMap(this::deleteUnreferencedAttachments) + .flatMap(message -> deleteUnreferencedAttachments(message).thenReturn(message)) + .flatMap(this::deleteAttachmentMessageIds) .then(messageDAO.delete(messageId))); } @@ -130,6 +131,12 @@ public class DeleteMessageListener implements MailboxListener.GroupMailboxListen .then(); } + private Mono<Void> deleteAttachmentMessageIds(MessageRepresentation message) { + return Flux.fromIterable(message.getAttachments()) + .concatMap(attachment -> attachmentMessageIdDAO.delete(attachment.getAttachmentId(), message.getMessageId())) + .then(); + } + private Mono<Boolean> hasOtherMessagesReferences(MessageRepresentation message, MessageAttachmentRepresentation attachment) { return attachmentMessageIdDAO.getOwnerMessageIds(attachment.getAttachmentId()) .filter(messageId -> !message.getMessageId().equals(messageId)) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAO.java index 8488472..12682c2 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAO.java @@ -38,6 +38,7 @@ import org.apache.james.mailbox.model.MessageId; import com.datastax.driver.core.PreparedStatement; import com.datastax.driver.core.Row; import com.datastax.driver.core.Session; +import com.datastax.driver.core.querybuilder.QueryBuilder; import com.google.common.base.Preconditions; import reactor.core.publisher.Flux; @@ -48,6 +49,7 @@ public class CassandraAttachmentMessageIdDAO { private final CassandraAsyncExecutor cassandraAsyncExecutor; private final PreparedStatement insertStatement; private final PreparedStatement selectStatement; + private final PreparedStatement deleteStatement; private final MessageId.Factory messageIdFactory; @Inject @@ -57,6 +59,7 @@ public class CassandraAttachmentMessageIdDAO { this.selectStatement = prepareSelect(session); this.insertStatement = prepareInsert(session); + this.deleteStatement = prepareDelete(session); } private PreparedStatement prepareInsert(Session session) { @@ -67,6 +70,14 @@ public class CassandraAttachmentMessageIdDAO { .value(MESSAGE_ID, bindMarker(MESSAGE_ID))); } + private PreparedStatement prepareDelete(Session session) { + return session.prepare( + QueryBuilder.delete() + .from(TABLE_NAME) + .where(eq(ATTACHMENT_ID_AS_UUID, bindMarker(ATTACHMENT_ID_AS_UUID))) + .and(eq(MESSAGE_ID, bindMarker(MESSAGE_ID)))); + } + private PreparedStatement prepareSelect(Session session) { return session.prepare(select(FIELDS) .from(TABLE_NAME) @@ -92,4 +103,11 @@ public class CassandraAttachmentMessageIdDAO { .setString(ATTACHMENT_ID, attachmentId.getId()) .setString(MESSAGE_ID, ownerMessageId.serialize())); } + + public Mono<Void> delete(AttachmentId attachmentId, MessageId ownerMessageId) { + return cassandraAsyncExecutor.executeVoid( + deleteStatement.bind() + .setUUID(ATTACHMENT_ID_AS_UUID, attachmentId.asUUID()) + .setString(MESSAGE_ID, ownerMessageId.serialize())); + } } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java index 55eeb7c..3f427a3 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java @@ -34,6 +34,7 @@ import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.cassandra.ids.CassandraId; import org.apache.james.mailbox.cassandra.ids.CassandraMessageId; import org.apache.james.mailbox.cassandra.mail.CassandraAttachmentDAOV2; +import org.apache.james.mailbox.cassandra.mail.CassandraAttachmentMessageIdDAO; import org.apache.james.mailbox.cassandra.mail.CassandraAttachmentOwnerDAO; import org.apache.james.mailbox.cassandra.mail.CassandraMessageDAO; import org.apache.james.mailbox.cassandra.mail.CassandraMessageIdDAO; @@ -128,9 +129,13 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isEmpty(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .doesNotContain(cassandraMessageId); }); } + @Test void deleteShouldUnreferenceMessageMetadata(CassandraCluster cassandraCluster) throws Exception { ComposedMessageId composedMessageId = inboxManager.appendMessage(MessageManager.AppendCommand.builder() @@ -153,6 +158,9 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isEmpty(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .doesNotContain(cassandraMessageId); }); } @@ -187,6 +195,9 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isPresent(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .contains(cassandraMessageId); }); } @@ -214,6 +225,9 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isPresent(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .contains(cassandraMessageId); }); } @@ -248,6 +262,9 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isPresent(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .doesNotContain(cassandraMessageId); }); } @@ -269,16 +286,22 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai SoftAssertions.assertSoftly(softly -> { CassandraMessageId cassandraMessageId = (CassandraMessageId) composedMessageId.getMessageId(); - CassandraId mailboxId = (CassandraId) composedMessageId.getMailboxId(); softly.assertThat(messageDAO(cassandraCluster).retrieveMessage(cassandraMessageId, MessageMapper.FetchType.Metadata) .blockOptional()).isEmpty(); softly.assertThat(attachmentDAO(cassandraCluster).getAttachment(attachmentId).blockOptional()) .isPresent(); + + softly.assertThat(attachmentMessageIdDAO(cassandraCluster).getOwnerMessageIds(attachmentId).collectList().block()) + .doesNotContain(cassandraMessageId); }); } + private CassandraAttachmentMessageIdDAO attachmentMessageIdDAO(CassandraCluster cassandraCluster) { + return new CassandraAttachmentMessageIdDAO(cassandraCluster.getConf(), new CassandraMessageId.Factory()); + } + private CassandraAttachmentDAOV2 attachmentDAO(CassandraCluster cassandraCluster) { return new CassandraAttachmentDAOV2(new HashBlobId.Factory(), cassandraCluster.getConf()); } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAOTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAOTest.java index d7aff6e..271620c 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAOTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMessageIdDAOTest.java @@ -20,6 +20,7 @@ package org.apache.james.mailbox.cassandra.mail; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import org.apache.james.backends.cassandra.CassandraCluster; import org.apache.james.backends.cassandra.CassandraClusterExtension; @@ -63,4 +64,40 @@ class CassandraAttachmentMessageIdDAOTest { .containsExactlyInAnyOrder(messageId1, messageId2) .hasSize(2); } + + @Test + void getOwnerMessageIdsShouldNotReturnDeletedValues() { + CassandraMessageId messageId1 = new CassandraMessageId.Factory().generate(); + AttachmentId attachmentId = AttachmentId.random(); + + testee.storeAttachmentForMessageId(attachmentId, messageId1).block(); + + testee.delete(attachmentId, messageId1).block(); + assertThat(testee.getOwnerMessageIds(attachmentId).collectList().block()) + .isEmpty(); + } + + @Test + void deleteShouldOnlyDeleteSuppliedValues() { + CassandraMessageId messageId1 = new CassandraMessageId.Factory().generate(); + CassandraMessageId messageId2 = new CassandraMessageId.Factory().generate(); + AttachmentId attachmentId = AttachmentId.random(); + + testee.storeAttachmentForMessageId(attachmentId, messageId1).block(); + testee.storeAttachmentForMessageId(attachmentId, messageId2).block(); + + testee.delete(attachmentId, messageId1).block(); + + assertThat(testee.getOwnerMessageIds(attachmentId).collectList().block()) + .containsExactly(messageId2); + } + + @Test + void deleteShouldNotThrowWhenNotExisting() { + CassandraMessageId messageId1 = new CassandraMessageId.Factory().generate(); + AttachmentId attachmentId = AttachmentId.random(); + + assertThatCode(() -> testee.delete(attachmentId, messageId1).block()) + .doesNotThrowAnyException(); + } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
