JAMES-2291 Delete should not decrease size when key does not exist
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a9869124 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a9869124 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a9869124 Branch: refs/heads/master Commit: a98691242b9e8f7c32756b842c17a40552b5e198 Parents: 7d190fa Author: Benoit Tellier <[email protected]> Authored: Tue Sep 4 11:46:08 2018 +0700 Committer: Benoit Tellier <[email protected]> Committed: Tue Sep 4 17:09:01 2018 +0700 ---------------------------------------------------------------------- .../mailrepository/MailRepositoryContract.java | 12 +++++++ .../cassandra/CassandraMailRepository.java | 19 +++++++---- .../CassandraMailRepositoryKeysDAO.java | 5 +-- .../CassandraMailRepositoryKeysDAOTest.java | 34 ++++++++++++++------ 4 files changed, 53 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a9869124/server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java ---------------------------------------------------------------------- diff --git a/server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java b/server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java index 3ed562b..0ba857d 100644 --- a/server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java +++ b/server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java @@ -334,6 +334,18 @@ public interface MailRepositoryContract { } @Test + default void removeShouldHaveNoEffectOnSizeWhenUnknownKeys() throws Exception { + MailRepository testee = retrieveRepository(); + + Mail mail1 = createMail(MAIL_1); + testee.store(mail1); + + testee.remove(ImmutableList.of(createMail(UNKNOWN_KEY))); + + assertThat(testee.size()).isEqualTo(1); + } + + @Test default void removeShouldHaveNoEffectForUnknownMail() throws Exception { MailRepository testee = retrieveRepository(); http://git-wip-us.apache.org/repos/asf/james-project/blob/a9869124/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepository.java ---------------------------------------------------------------------- diff --git a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepository.java b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepository.java index dc2db90..d576369 100644 --- a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepository.java +++ b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepository.java @@ -30,6 +30,7 @@ import java.util.Collection; import java.util.Iterator; import java.util.Properties; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import javax.mail.MessagingException; import javax.mail.Session; @@ -193,20 +194,26 @@ public class CassandraMailRepository implements MailRepository { removeAsync(key).join(); } - public CompletableFuture<Void> removeAsync(MailKey key) { - return CompletableFuture.allOf( - keysDAO.remove(url, key), - countDAO.decrement(url)) + private CompletableFuture<Void> removeAsync(MailKey key) { + return keysDAO.remove(url, key) + .thenCompose(this::decreaseSizeIfDeleted) .thenCompose(any -> mailDAO.remove(url, key)); } + private CompletionStage<Void> decreaseSizeIfDeleted(Boolean isDeleted) { + if (isDeleted) { + return countDAO.decrement(url); + } + return CompletableFuture.completedFuture(null); + } + @Override - public long size() throws MessagingException { + public long size() { return countDAO.getCount(url).join(); } @Override - public void removeAll() throws MessagingException { + public void removeAll() { keysDAO.list(url) .thenCompose(stream -> FluentFutureStream.of(stream.map(this::removeAsync)) .completableFuture()) http://git-wip-us.apache.org/repos/asf/james-project/blob/a9869124/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAO.java ---------------------------------------------------------------------- diff --git a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAO.java b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAO.java index 8b01f15..2a640c8 100644 --- a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAO.java +++ b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAO.java @@ -68,6 +68,7 @@ public class CassandraMailRepositoryKeysDAO { private PreparedStatement prepareDelete(Session session) { return session.prepare(delete() .from(KEYS_TABLE_NAME) + .ifExists() .where(eq(REPOSITORY_NAME, bindMarker(REPOSITORY_NAME))) .and(eq(MAIL_KEY, bindMarker(MAIL_KEY)))); } @@ -91,8 +92,8 @@ public class CassandraMailRepositoryKeysDAO { .thenApply(stream -> stream.map(row -> new MailKey(row.getString(MAIL_KEY)))); } - public CompletableFuture<Void> remove(MailRepositoryUrl url, MailKey key) { - return executor.executeVoid(deleteKey.bind() + public CompletableFuture<Boolean> remove(MailRepositoryUrl url, MailKey key) { + return executor.executeReturnApplied(deleteKey.bind() .setString(REPOSITORY_NAME, url.asString()) .setString(MAIL_KEY, key.asString())); } http://git-wip-us.apache.org/repos/asf/james-project/blob/a9869124/server/mailrepository/mailrepository-cassandra/src/test/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAOTest.java ---------------------------------------------------------------------- diff --git a/server/mailrepository/mailrepository-cassandra/src/test/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAOTest.java b/server/mailrepository/mailrepository-cassandra/src/test/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAOTest.java index feebf98..2b303c7 100644 --- a/server/mailrepository/mailrepository-cassandra/src/test/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAOTest.java +++ b/server/mailrepository/mailrepository-cassandra/src/test/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryKeysDAOTest.java @@ -34,7 +34,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @ExtendWith(DockerCassandraExtension.class) -public class CassandraMailRepositoryKeysDAOTest { +class CassandraMailRepositoryKeysDAOTest { static final MailRepositoryUrl URL = MailRepositoryUrl.from("proto://url"); static final MailRepositoryUrl URL2 = MailRepositoryUrl.from("proto://url2"); @@ -51,7 +51,7 @@ public class CassandraMailRepositoryKeysDAOTest { } @BeforeEach - public void setUp() { + void setUp() { testee = new CassandraMailRepositoryKeysDAO(cassandra.getConf(), CassandraUtils.WITH_DEFAULT_CONFIGURATION); } @@ -66,13 +66,13 @@ public class CassandraMailRepositoryKeysDAOTest { } @Test - public void test() { + void listShouldBeEmptyByDefault() { assertThat(testee.list(URL).join()) .isEmpty(); } @Test - public void listShouldReturnEmptyByDefault() { + void listShouldReturnEmptyByDefault() { testee.store(URL, KEY_1).join(); assertThat(testee.list(URL).join()) @@ -80,7 +80,7 @@ public class CassandraMailRepositoryKeysDAOTest { } @Test - public void listShouldNotReturnElementsOfOtherRepositories() { + void listShouldNotReturnElementsOfOtherRepositories() { testee.store(URL, KEY_1).join(); assertThat(testee.list(URL2).join()) @@ -88,7 +88,7 @@ public class CassandraMailRepositoryKeysDAOTest { } @Test - public void listShouldReturnSeveralElements() { + void listShouldReturnSeveralElements() { testee.store(URL, KEY_1).join(); testee.store(URL, KEY_2).join(); testee.store(URL, KEY_3).join(); @@ -98,7 +98,7 @@ public class CassandraMailRepositoryKeysDAOTest { } @Test - public void listShouldNotReturnRemovedElements() { + void listShouldNotReturnRemovedElements() { testee.store(URL, KEY_1).join(); testee.store(URL, KEY_2).join(); testee.store(URL, KEY_3).join(); @@ -110,12 +110,12 @@ public class CassandraMailRepositoryKeysDAOTest { } @Test - public void removeShouldBeIdempotent() { + void removeShouldBeIdempotent() { testee.remove(URL, KEY_2).join(); } @Test - public void removeShouldNotAffectOtherRepositories() { + void removeShouldNotAffectOtherRepositories() { testee.store(URL, KEY_1).join(); testee.remove(URL2, KEY_2).join(); @@ -124,4 +124,20 @@ public class CassandraMailRepositoryKeysDAOTest { .containsOnly(KEY_1); } + @Test + void removeShouldReturnTrueWhenKeyDeleted() { + testee.store(URL, KEY_1).join(); + + boolean isDeleted = testee.remove(URL, KEY_1).join(); + + assertThat(isDeleted).isTrue(); + } + + @Test + void removeShouldReturnFalseWhenKeyNotDeleted() { + boolean isDeleted = testee.remove(URL2, KEY_2).join(); + + assertThat(isDeleted).isFalse(); + } + } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
