PROTOCOLS-117 MailboxPath should be immutable
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/6a6e74cf Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/6a6e74cf Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/6a6e74cf Branch: refs/heads/master Commit: 6a6e74cf0f869c3f69bbb82991fd703bb0f1c42b Parents: 5218473 Author: benwa <btell...@linagora.com> Authored: Wed Nov 8 10:04:41 2017 +0700 Committer: Antoine Duprat <adup...@linagora.com> Committed: Mon Nov 13 16:23:06 2017 +0100 ---------------------------------------------------------------------- .../apache/james/mailbox/model/MailboxPath.java | 28 ++--------------- .../hbase/mail/HBaseMailboxMapperTest.java | 20 ++++++++---- .../mailbox/store/StoreMailboxManager.java | 32 +++++++++++++------- 3 files changed, 38 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/6a6e74cf/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java index ea5e760..42de9a4 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java @@ -49,9 +49,9 @@ public class MailboxPath { return new MailboxPath(MailboxConstants.USER_NAMESPACE, username, mailboxName); } - private String namespace; - private String user; - private String name; + private final String namespace; + private final String user; + private final String name; public MailboxPath(String namespace, String user, String name) { this.namespace = Optional.ofNullable(namespace) @@ -79,13 +79,6 @@ public class MailboxPath { } /** - * Set the namespace this mailbox is in - */ - public void setNamespace(String namespace) { - this.namespace = namespace; - } - - /** * Get the name of the user who owns the mailbox. This can be null e.g. for * shared mailboxes. * @@ -96,13 +89,6 @@ public class MailboxPath { } /** - * Set the name of the user who owns the mailbox. - */ - public void setUser(String user) { - this.user = user; - } - - /** * Get the name of the mailbox. This is the pure name without user or * namespace, so this is what a user would see in his client. * @@ -113,14 +99,6 @@ public class MailboxPath { } /** - * Set the name of the mailbox. This is the pure name without user or - * namespace, so this is what a user would see in his client. - */ - public void setName(String name) { - this.name = name; - } - - /** * Return a list of MailboxPath representing the hierarchy levels of this * MailboxPath. E.g. INBOX.main.sub would yield * http://git-wip-us.apache.org/repos/asf/james-project/blob/6a6e74cf/mailbox/hbase/src/test/java/org/apache/james/mailbox/hbase/mail/HBaseMailboxMapperTest.java ---------------------------------------------------------------------- diff --git a/mailbox/hbase/src/test/java/org/apache/james/mailbox/hbase/mail/HBaseMailboxMapperTest.java b/mailbox/hbase/src/test/java/org/apache/james/mailbox/hbase/mail/HBaseMailboxMapperTest.java index 078824c..dca113b 100644 --- a/mailbox/hbase/src/test/java/org/apache/james/mailbox/hbase/mail/HBaseMailboxMapperTest.java +++ b/mailbox/hbase/src/test/java/org/apache/james/mailbox/hbase/mail/HBaseMailboxMapperTest.java @@ -160,18 +160,26 @@ public class HBaseMailboxMapperTest { MailboxPath newPath; for (int i = start; i < end; i++) { - newPath = new MailboxPath(path); - newPath.setName(i + newPath.getName() + " " + i); - // test for paths with null user - if (i % 2 == 0) { - newPath.setUser(null); - } + newPath = new MailboxPath(path.getNamespace(), + computeUserName(path.getUser(), i), + computeMailboxName(path.getName(), i)); addMailbox(new HBaseMailbox(newPath, 1234)); } result = mapper.findMailboxWithPathLike(path); assertEquals(end - start + 1, result.size()); } + private String computeUserName(String user, int i) { + if (i % 2 == 0) { + return null; + } + return user; + } + + private String computeMailboxName(String name, int i) { + return i + name + " " + i; + } + /** * Test of list method, of class HBaseMailboxMapper. */ http://git-wip-us.apache.org/repos/asf/james-project/blob/6a6e74cf/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java index b4f6433..7664f84 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java @@ -484,27 +484,25 @@ public class StoreMailboxManager implements MailboxManager { } @Override - public Optional<MailboxId> createMailbox(MailboxPath mailboxPath, final MailboxSession mailboxSession) + public Optional<MailboxId> createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { LOGGER.debug("createMailbox " + mailboxPath); - final int length = mailboxPath.getName().length(); - if (length == 0) { + if (mailboxPath.getName().isEmpty()) { LOGGER.warn("Ignoring mailbox with empty name"); } else { - if (mailboxPath.getName().charAt(length - 1) == getDelimiter()) - mailboxPath.setName(mailboxPath.getName().substring(0, length - 1)); - if (mailboxExists(mailboxPath, mailboxSession)) - throw new MailboxExistsException(mailboxPath.toString()); + MailboxPath sanitizedMailboxPath = sanitizeMailboxPath(mailboxPath); + if (mailboxExists(sanitizedMailboxPath, mailboxSession)) + throw new MailboxExistsException(sanitizedMailboxPath.asString()); // Create parents first // If any creation fails then the mailbox will not be created // TODO: transaction - final List<MailboxId> mailboxIds = new ArrayList<>(); - for (final MailboxPath mailbox : mailboxPath.getHierarchyLevels(getDelimiter())) + List<MailboxId> mailboxIds = new ArrayList<>(); + for (MailboxPath mailbox : sanitizedMailboxPath.getHierarchyLevels(getDelimiter())) locker.executeWithLock(mailboxSession, mailbox, (LockAwareExecution<Void>) () -> { if (!mailboxExists(mailbox, mailboxSession)) { - final Mailbox m = doCreateMailbox(mailbox, mailboxSession); - final MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); + Mailbox m = doCreateMailbox(mailbox, mailboxSession); + MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession); mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(m)))); // notify listeners @@ -521,6 +519,18 @@ public class StoreMailboxManager implements MailboxManager { return Optional.empty(); } + private MailboxPath sanitizeMailboxPath(MailboxPath mailboxPath) { + if (mailboxPath.getName().endsWith(String.valueOf(getDelimiter()))) { + int length = mailboxPath.getName().length(); + String sanitizedName = mailboxPath.getName().substring(0, length - 1); + return new MailboxPath( + mailboxPath.getNamespace(), + mailboxPath.getUser(), + sanitizedName); + } + return mailboxPath; + } + @Override public void deleteMailbox(final MailboxPath mailboxPath, final MailboxSession session) throws MailboxException { LOGGER.info("deleteMailbox " + mailboxPath); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org