From e5fce8814f0531d8d334f8c242acc4823f4181c5 Mon Sep 17 00:00:00 2001 From: Raphael Ouazana Date: Fri, 4 Mar 2016 14:59:56 +0100 Subject: [PATCH] JAMES-1700 Don't fail in account creation if user already exists --- ...lreadyExistInUsersRepositoryException.java | 31 ++++ .../user/lib/AbstractUsersRepository.java | 3 +- server/protocols/jmap/pom.xml | 6 + .../james/jmap/FirstUserConnectionFilter.java | 6 +- .../jmap/FirstUserConnectionFilterTest.java | 165 ++++++++++++++++++ 5 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 server/data/data-api/src/main/java/org/apache/james/user/api/AlreadyExistInUsersRepositoryException.java diff --git a/server/data/data-api/src/main/java/org/apache/james/user/api/AlreadyExistInUsersRepositoryException.java b/server/data/data-api/src/main/java/org/apache/james/user/api/AlreadyExistInUsersRepositoryException.java new file mode 100644 index 00000000000..b507f9e7b11 --- /dev/null +++ b/server/data/data-api/src/main/java/org/apache/james/user/api/AlreadyExistInUsersRepositoryException.java @@ -0,0 +1,31 @@ +/**************************************************************** + * 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.user.api; + +public class AlreadyExistInUsersRepositoryException extends UsersRepositoryException { + + public AlreadyExistInUsersRepositoryException(String msg, Throwable t) { + super(msg, t); + } + + public AlreadyExistInUsersRepositoryException(String msg) { + super(msg); + } +} diff --git a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java index c95b7f632db..b3988ab9937 100644 --- a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java +++ b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java @@ -27,6 +27,7 @@ import org.apache.james.domainlist.api.DomainListException; import org.apache.james.lifecycle.api.Configurable; import org.apache.james.lifecycle.api.LogEnabled; +import org.apache.james.user.api.AlreadyExistInUsersRepositoryException; import org.apache.james.user.api.UsersRepository; import org.apache.james.user.api.UsersRepositoryException; import org.slf4j.Logger; @@ -106,7 +107,7 @@ public void addUser(String username, String password) throws UsersRepositoryExce isValidUsername(username); doAddUser(username, password); } else { - throw new UsersRepositoryException("User with username " + username + " already exist!"); + throw new AlreadyExistInUsersRepositoryException("User with username " + username + " already exists!"); } } diff --git a/server/protocols/jmap/pom.xml b/server/protocols/jmap/pom.xml index 2f31da74e4c..17452310054 100644 --- a/server/protocols/jmap/pom.xml +++ b/server/protocols/jmap/pom.xml @@ -272,6 +272,12 @@ json-path test + + com.googlecode.thread-weaver + threadweaver + 0.2 + test + junit junit diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/FirstUserConnectionFilter.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/FirstUserConnectionFilter.java index 9a44db5053e..f7354f77572 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/FirstUserConnectionFilter.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/FirstUserConnectionFilter.java @@ -37,6 +37,7 @@ import org.apache.james.mailbox.exception.BadCredentialsException; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.user.api.AlreadyExistInUsersRepositoryException; import org.apache.james.user.api.UsersRepository; import org.apache.james.user.api.UsersRepositoryException; import org.slf4j.Logger; @@ -70,12 +71,15 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha chain.doFilter(request, response); } - private void createAccountIfNeeded(MailboxSession session) { + @VisibleForTesting + void createAccountIfNeeded(MailboxSession session) { try { User user = session.getUser(); if (needsAccountCreation(user)) { createAccount(user); } + } catch (AlreadyExistInUsersRepositoryException e) { + // Ignore } catch (UsersRepositoryException|MailboxException e) { throw Throwables.propagate(e); } diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/FirstUserConnectionFilterTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/FirstUserConnectionFilterTest.java index 2da6c0c854c..5d3b4415a7d 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/FirstUserConnectionFilterTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/FirstUserConnectionFilterTest.java @@ -23,22 +23,46 @@ import static org.mockito.Mockito.verify; import java.io.IOException; +import java.util.List; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.james.mailbox.MailboxListener; import org.apache.james.mailbox.MailboxManager; +import org.apache.james.mailbox.MailboxSession; +import org.apache.james.mailbox.MessageManager; +import org.apache.james.mailbox.exception.BadCredentialsException; +import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.mailbox.model.MailboxACL.MailboxACLCommand; +import org.apache.james.mailbox.model.MailboxACL.MailboxACLEntryKey; +import org.apache.james.mailbox.model.MailboxACL.MailboxACLRight; +import org.apache.james.mailbox.model.MailboxACL.MailboxACLRights; +import org.apache.james.mailbox.model.MailboxMetaData; +import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.model.MailboxQuery; +import org.apache.james.mailbox.model.MessageRange; +import org.apache.james.mailbox.store.SimpleMailboxSession; import org.apache.james.user.api.UsersRepositoryException; import org.apache.james.user.lib.mock.InMemoryUsersRepository; import org.junit.Before; import org.junit.Test; +import org.slf4j.Logger; + +import com.google.testing.threadtester.AnnotatedTestRunner; +import com.google.testing.threadtester.ThreadedAfter; +import com.google.testing.threadtester.ThreadedBefore; +import com.google.testing.threadtester.ThreadedMain; +import com.google.testing.threadtester.ThreadedSecondary; public class FirstUserConnectionFilterTest { private FirstUserConnectionFilter sut; private InMemoryUsersRepository usersRepository; + private MailboxSession session; + private MailboxManager mailboxManager; @Before public void setup() { @@ -57,5 +81,146 @@ public void filterShouldDoNothingOnNullSession() throws IOException, ServletExce assertThat(usersRepository.list()).isEmpty(); } + @ThreadedBefore + public void before() { + usersRepository = new InMemoryUsersRepository(); + session = new SimpleMailboxSession(0, "username", null, null, null, ':', null); + mailboxManager = new FakeMailboxManager(session) ; + sut = new FirstUserConnectionFilter(usersRepository, mailboxManager); + } + + @ThreadedMain + public void mainThread() { + sut.createAccountIfNeeded(session); + } + + @ThreadedSecondary + public void secondThread() { + sut.createAccountIfNeeded(session); + } + + @ThreadedAfter + public void after() { + // Exception is thrown if test fails + } + + @Test + public void testConcurrentAccessToFilterShouldNotThrow() { + AnnotatedTestRunner runner = new AnnotatedTestRunner(); + runner.runTests(this.getClass(), FirstUserConnectionFilter.class); + } + + private static class FakeMailboxManager implements MailboxManager { + private MailboxSession mailboxSession; + + public FakeMailboxManager(MailboxSession mailboxSession) { + this.mailboxSession = mailboxSession; + } + + @Override + public void startProcessingRequest(MailboxSession session) { + } + + @Override + public void endProcessingRequest(MailboxSession session) { + } + + @Override + public void addListener(MailboxPath mailboxPath, MailboxListener listener, MailboxSession session) throws MailboxException { + } + + @Override + public void removeListener(MailboxPath mailboxPath, MailboxListener listner, MailboxSession session) throws MailboxException { + } + + @Override + public void addGlobalListener(MailboxListener listener, MailboxSession session) throws MailboxException { + } + + @Override + public void removeGlobalListener(MailboxListener listner, MailboxSession session) throws MailboxException { + } + + @Override + public char getDelimiter() { + return 0; + } + + @Override + public MessageManager getMailbox(MailboxPath mailboxPath, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public void createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { + } + + @Override + public void deleteMailbox(MailboxPath mailboxPath, MailboxSession session) throws MailboxException { + } + + @Override + public void renameMailbox(MailboxPath from, MailboxPath to, MailboxSession session) throws MailboxException { + } + + @Override + public List copyMessages(MessageRange set, MailboxPath from, MailboxPath to, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public List moveMessages(MessageRange set, MailboxPath from, MailboxPath to, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public List search(MailboxQuery expression, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public boolean mailboxExists(MailboxPath mailboxPath, MailboxSession session) throws MailboxException { + return false; + } + + @Override + public MailboxSession createSystemSession(String userName, Logger log) throws BadCredentialsException, MailboxException { + return mailboxSession; + } + + @Override + public MailboxSession login(String userid, String passwd, Logger log) throws BadCredentialsException, MailboxException { + return null; + } + + @Override + public void logout(MailboxSession session, boolean force) throws MailboxException { + } + + @Override + public boolean hasRight(MailboxPath mailboxPath, MailboxACLRight right, MailboxSession session) throws MailboxException { + return false; + } + + @Override + public MailboxACLRights myRights(MailboxPath mailboxPath, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public MailboxACLRights[] listRigths(MailboxPath mailboxPath, MailboxACLEntryKey identifier, MailboxSession session) throws MailboxException { + return null; + } + + @Override + public void setRights(MailboxPath mailboxPath, MailboxACLCommand mailboxACLCommand, MailboxSession session) throws MailboxException { + } + + @Override + public List list(MailboxSession session) throws MailboxException { + return null; + } + + } }