JAMES-1925 Correct MPT failure on too long mailbox names
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f518297f Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f518297f Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f518297f Branch: refs/heads/master Commit: f518297f992411ba4a08913758ebf7c1337a5b7b Parents: 09934b1 Author: Benoit Tellier <[email protected]> Authored: Tue Feb 14 19:08:36 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Wed Feb 15 13:12:39 2017 +0100 ---------------------------------------------------------------------- .../cassandra/mail/CassandraMailboxMapper.java | 54 ++++++++---- .../mail/CassandraMailboxMapperTest.java | 86 ++++++++++++++++++++ .../imap/scripts/CreateErrorWithLongName.test | 2 +- 3 files changed, 123 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/f518297f/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java index 1f1e163..9ad665a 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Optional; import java.util.StringTokenizer; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -51,8 +52,8 @@ import com.google.common.base.Preconditions; public class CassandraMailboxMapper implements MailboxMapper { public static final String WILDCARD = "%"; - public static final String VALUES_MAY_NOT_BE_LARGER_THAN_64_K = "Index expression values may not be larger than 64K"; + public static final String CLUSTERING_COLUMNS_IS_TOO_LONG = "The sum of all clustering columns is too long"; private final int maxRetry; private final CassandraAsyncExecutor cassandraAsyncExecutor; @@ -87,11 +88,14 @@ public class CassandraMailboxMapper implements MailboxMapper { .orElse(CompletableFuture.completedFuture(Optional.empty()))) .join() .orElseThrow(() -> new MailboxNotFoundException(path)); - } catch (InvalidQueryException e) { - if (StringUtils.containsIgnoreCase(e.getMessage(), VALUES_MAY_NOT_BE_LARGER_THAN_64_K)) { - throw new TooLongMailboxNameException("too long mailbox name"); + } catch (CompletionException e) { + if (e.getCause() instanceof InvalidQueryException) { + if (StringUtils.containsIgnoreCase(e.getCause().getMessage(), VALUES_MAY_NOT_BE_LARGER_THAN_64_K)) { + throw new TooLongMailboxNameException("too long mailbox name"); + } + throw new MailboxException("It has error with cassandra storage", e.getCause()); } - throw new MailboxException("It has error with cassandra storage", e); + throw e; } } @@ -124,20 +128,34 @@ public class CassandraMailboxMapper implements MailboxMapper { CassandraId cassandraId = retrieveId(cassandraMailbox); cassandraMailbox.setMailboxId(cassandraId); - - boolean applied = mailboxDAO.retrieveMailbox(cassandraId) - .thenCompose(optional -> optional - .map(storedMailbox -> mailboxPathDAO.delete(storedMailbox.generateAssociatedPath())) - .orElse(CompletableFuture.completedFuture(null))) - .thenCompose(any -> mailboxPathDAO.save(mailbox.generateAssociatedPath(), cassandraId)) - .thenCompose(result -> { - if (result) { - return mailboxDAO.save(cassandraMailbox).thenApply(any -> result); + try { + boolean applied = mailboxPathDAO.save(mailbox.generateAssociatedPath(), cassandraId) + .thenCompose(result -> { + if (result) { + return mailboxDAO.retrieveMailbox(cassandraId) + .thenCompose(optional -> optional + .map(storedMailbox -> mailboxPathDAO.delete(storedMailbox.generateAssociatedPath())) + .orElse(CompletableFuture.completedFuture(null))) + .thenCompose(any -> mailboxDAO.save(cassandraMailbox).thenApply(_any -> result)); + } + + return CompletableFuture.completedFuture(result); + } + ).join(); + + if (!applied) { + throw new MailboxExistsException(mailbox.generateAssociatedPath().asString()); + } + } catch (CompletionException e) { + if (e.getCause() instanceof InvalidQueryException) { + String errorMessage = e.getCause().getMessage(); + if (StringUtils.containsIgnoreCase(errorMessage, VALUES_MAY_NOT_BE_LARGER_THAN_64_K) || + StringUtils.containsIgnoreCase(errorMessage, CLUSTERING_COLUMNS_IS_TOO_LONG)) { + throw new TooLongMailboxNameException("too long mailbox name"); } - return CompletableFuture.completedFuture(result); - }).join(); - if (!applied) { - throw new MailboxExistsException(mailbox.generateAssociatedPath().asString()); + throw new MailboxException("It has error with cassandra storage", e.getCause()); + } + throw e; } } http://git-wip-us.apache.org/repos/asf/james-project/blob/f518297f/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java new file mode 100644 index 0000000..f793298 --- /dev/null +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java @@ -0,0 +1,86 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.mailbox.cassandra.mail; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +import java.util.Optional; + +import org.apache.commons.lang3.StringUtils; +import org.apache.james.backends.cassandra.CassandraCluster; +import org.apache.james.backends.cassandra.init.CassandraModuleComposite; +import org.apache.james.mailbox.cassandra.mail.CassandraMailboxPathDAO.CassandraIdAndPath; +import org.apache.james.mailbox.cassandra.modules.CassandraAclModule; +import org.apache.james.mailbox.cassandra.modules.CassandraMailboxModule; +import org.apache.james.mailbox.exception.TooLongMailboxNameException; +import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.store.mail.model.Mailbox; +import org.apache.james.mailbox.store.mail.model.impl.SimpleMailbox; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class CassandraMailboxMapperTest { + + public static final int MAX_RETRY = 10; + public static final int UID_VALIDITY = 52; + public static final MailboxPath MAILBOX_PATH = new MailboxPath("#private", "user", "name"); + public static final int THREAD_COUNT = 10; + public static final int OPERATION_COUNT = 10; + private CassandraCluster cassandra; + private CassandraMailboxPathDAO mailboxPathDAO; + private CassandraMailboxMapper testee; + + @Before + public void setUp() { + cassandra = CassandraCluster.create(new CassandraModuleComposite(new CassandraMailboxModule(), new CassandraAclModule())); + cassandra.ensureAllTables(); + + CassandraMailboxDAO mailboxDAO = new CassandraMailboxDAO(cassandra.getConf(), cassandra.getTypesProvider(), MAX_RETRY); + mailboxPathDAO = new CassandraMailboxPathDAO(cassandra.getConf(), cassandra.getTypesProvider()); + testee = new CassandraMailboxMapper(cassandra.getConf(), mailboxDAO, mailboxPathDAO, MAX_RETRY); + } + + @After + public void tearDown() { + cassandra.clearAllTables(); + } + + @Test + public void saveShouldNotRemoveOldMailboxPathWhenCreatingTheNewMailboxPathFails() throws Exception { + testee.save(new SimpleMailbox(MAILBOX_PATH, UID_VALIDITY)); + Mailbox mailbox = testee.findMailboxByPath(MAILBOX_PATH); + + try { + SimpleMailbox newMailbox = new SimpleMailbox(tooLongMailboxPath(mailbox.generateAssociatedPath()), UID_VALIDITY, mailbox.getMailboxId()); + testee.save(newMailbox); + fail("TooLongMailboxNameException expected"); + } catch (TooLongMailboxNameException e) { + } + + Optional<CassandraIdAndPath> cassandraIdAndPath = mailboxPathDAO.retrieveId(MAILBOX_PATH).get(); + assertThat(cassandraIdAndPath.isPresent()).isTrue(); + } + + private MailboxPath tooLongMailboxPath(MailboxPath fromMailboxPath) { + return new MailboxPath(fromMailboxPath, StringUtils.repeat("b", 65537)); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/f518297f/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/CreateErrorWithLongName.test ---------------------------------------------------------------------- diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/CreateErrorWithLongName.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/CreateErrorWithLongName.test index 53b9af6..995fc8e 100644 --- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/CreateErrorWithLongName.test +++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/CreateErrorWithLongName.test @@ -547,7 +547,7 @@ C: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb C: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb C: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb C: bbbbbbbbbbbbbbbbbbbbbbbba -S: A2 BAD DELETE too long mailbox name. Illegal arguments. +S: A2 NO DELETE failed. No such mailbox. C: A3 CREATE newmailbox --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
