JAMES-1854 Rework resourceLocator API - Simplify API by using mailAddresses - Merge interface and subclass as genericity is not needed
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/296bb643 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/296bb643 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/296bb643 Branch: refs/heads/master Commit: 296bb643896cb2189c09c12bd573dc13fb72c387 Parents: ac530c0 Author: Benoit Tellier <[email protected]> Authored: Sun Nov 20 23:55:09 2016 +0700 Committer: Benoit Tellier <[email protected]> Committed: Wed Nov 23 18:19:35 2016 +0700 ---------------------------------------------------------------------- .../transport/mailets/ResourceLocatorImpl.java | 62 ------------------ .../apache/james/transport/mailets/Sieve.java | 3 +- .../mailets/jsieve/ResourceLocator.java | 69 ++++++++------------ .../mailets/jsieve/delivery/SieveExecutor.java | 37 ++--------- .../mailets/ResourceLocatorImplTest.java | 57 ---------------- .../transport/mailets/ResourceLocatorTest.java | 69 ++++++++++++++++++++ .../mailets/delivery/SieveIntegrationTest.java | 19 +++++- 7 files changed, 117 insertions(+), 199 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/ResourceLocatorImpl.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/ResourceLocatorImpl.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/ResourceLocatorImpl.java deleted file mode 100644 index bf8978e..0000000 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/ResourceLocatorImpl.java +++ /dev/null @@ -1,62 +0,0 @@ -/**************************************************************** - * 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.transport.mailets; - -import org.apache.james.sieverepository.api.SieveRepository; -import org.apache.james.sieverepository.api.exception.SieveRepositoryException; -import org.apache.james.transport.mailets.jsieve.ResourceLocator; -import org.apache.james.user.api.UsersRepository; -import org.apache.james.user.api.UsersRepositoryException; -import org.joda.time.DateTime; - -import javax.mail.MessagingException; - -public class ResourceLocatorImpl implements ResourceLocator { - - public static ResourceLocatorImpl instanciate(UsersRepository usersRepository, SieveRepository sieveRepository) throws MessagingException { - try { - return new ResourceLocatorImpl(usersRepository.supportVirtualHosting(), sieveRepository); - } catch (UsersRepositoryException e) { - throw new MessagingException("Unable to access UsersRepository", e); - } - } - - private final boolean virtualHosting; - private final SieveRepository sieveRepository; - - public ResourceLocatorImpl(boolean virtualHosting, SieveRepository sieveRepository) { - this.virtualHosting = virtualHosting; - this.sieveRepository = sieveRepository; - } - - public UserSieveInformation get(String uri) throws SieveRepositoryException { - // Use the complete email address for finding the sieve file - uri = uri.substring(2); - - String username; - if (virtualHosting) { - username = uri.substring(0, uri.indexOf("/")); - } else { - username = uri.substring(0, uri.indexOf("@")); - } - - return new UserSieveInformation(sieveRepository.getActivationDateForActiveScript(username), DateTime.now(), sieveRepository.getActive(username)); - } -} http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/Sieve.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/Sieve.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/Sieve.java index 56c7f1d..92c4bc6 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/Sieve.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/Sieve.java @@ -54,7 +54,7 @@ public class Sieve extends GenericMailet { @Inject public Sieve(UsersRepository usersRepository, SieveRepository sieveRepository) throws MessagingException { - this(usersRepository, ResourceLocatorImpl.instanciate(usersRepository, sieveRepository)); + this(usersRepository, new ResourceLocator(sieveRepository, usersRepository)); } public Sieve(UsersRepository usersRepository, ResourceLocator resourceLocator) throws MessagingException { @@ -76,7 +76,6 @@ public class Sieve extends GenericMailet { .build(); sieveExecutor = SieveExecutor.builder() .resourceLocator(resourceLocator) - .usersRepository(usersRepository) .mailetContext(getMailetContext()) .log(log) .sievePoster(new SievePoster(usersRepository, MailboxConstants.INBOX)) http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/ResourceLocator.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/ResourceLocator.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/ResourceLocator.java index 01d38a5..b87b82a 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/ResourceLocator.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/ResourceLocator.java @@ -18,46 +18,17 @@ ****************************************************************/ package org.apache.james.transport.mailets.jsieve; +import org.apache.james.sieverepository.api.SieveRepository; +import org.apache.james.user.api.UsersRepository; +import org.apache.james.user.api.UsersRepositoryException; +import org.apache.mailet.MailAddress; import org.joda.time.DateTime; import java.io.InputStream; -/** - * <p>Experimental API locates resources. - * Used to load Sieve scripts. The base for relative URLs - * should be taken to be the root of the James configuration. - * </p><p> - * Required schemas: - * </p> - * <ul> - * <li><strong>User sieve scripts</strong> - the relative URL scheme - * <code>//<em>user</em>@<em>host</em>/<em>sieve</em> will be used to - * obtain the script - * </ul> - * <p> - * The advantage of using <code>URI</code>s - * and verbs (for example <code>GET</code>, <code>POST</code>) - * are their uniformity. The same API can be used to interface radically - * different resource types and protocols. This allows concise, minimal, - * powerful APIs to be created. Their simplicity is easy to preserved - * across versions. - * </p><p> - * The disadvantage is that this free decouple means that there is - * no gaurantee that the implementations decoupled by this interface - * actually support the same scheme. Issues will be caught only - * at deployment and not at compile time. - * This places a larger burden on the deployer. - * </p><p> - * Either an understanding or a consistent URL mapping scheme may be - * required. For example, <code>//john.smith@localhost/sieve</code> - * may need to be resolved to <code>../apps/james/var/sieve/[email protected]</code> - * when using the file system to store scripts. Note that names <strong>MUST</strong> - * be normalised before resolving on a file system. - * </p> - */ -public interface ResourceLocator { +public class ResourceLocator { - class UserSieveInformation { + public static class UserSieveInformation { private DateTime scriptActivationDate; private DateTime scriptInterpretationDate; private InputStream scriptContent; @@ -81,12 +52,26 @@ public interface ResourceLocator { } } - /** - * GET verb locates and loads a resource. - * @param uri identifies the Sieve script - * @return not null - * @throws Exception when the resource cannot be located - */ - UserSieveInformation get(String uri) throws Exception; + private final SieveRepository sieveRepository; + private final UsersRepository usersRepository; + + public ResourceLocator(SieveRepository sieveRepository, UsersRepository usersRepository) { + this.sieveRepository = sieveRepository; + this.usersRepository = usersRepository; + } + + public UserSieveInformation get(MailAddress mailAddress) throws Exception { + String username = retrieveUsername(mailAddress); + return new UserSieveInformation(sieveRepository.getActivationDateForActiveScript(username), DateTime.now(), sieveRepository.getActive(username)); + } + + private String retrieveUsername(MailAddress mailAddress) { + try { + return usersRepository.getUser(mailAddress); + } catch (UsersRepositoryException e) { + + return mailAddress.asString(); + } + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/delivery/SieveExecutor.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/delivery/SieveExecutor.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/delivery/SieveExecutor.java index e6b7700..feb9c37 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/delivery/SieveExecutor.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/jsieve/delivery/SieveExecutor.java @@ -26,12 +26,9 @@ import javax.mail.MessagingException; import org.apache.commons.logging.Log; import org.apache.james.sieverepository.api.exception.ScriptNotFoundException; -import org.apache.james.transport.mailets.delivery.DeliveryUtils; import org.apache.james.transport.mailets.jsieve.ActionDispatcher; import org.apache.james.transport.mailets.jsieve.ResourceLocator; import org.apache.james.transport.mailets.jsieve.SieveMailAdapter; -import org.apache.james.user.api.UsersRepository; -import org.apache.james.user.api.UsersRepositoryException; import org.apache.jsieve.ConfigurationManager; import org.apache.jsieve.SieveConfigurationException; import org.apache.jsieve.SieveFactory; @@ -53,16 +50,10 @@ public class SieveExecutor { public static class Builder { private MailetContext mailetContext; - private UsersRepository usersRepos; private SievePoster sievePoster; private ResourceLocator resourceLocator; private Log log; - public Builder usersRepository(UsersRepository usersRepository) { - this.usersRepos = usersRepository; - return this; - } - public Builder sievePoster(SievePoster sievePoster) { this.sievePoster = sievePoster; return this; @@ -85,26 +76,23 @@ public class SieveExecutor { public SieveExecutor build() throws MessagingException { Preconditions.checkNotNull(mailetContext); - Preconditions.checkNotNull(usersRepos); Preconditions.checkNotNull(resourceLocator); Preconditions.checkNotNull(log); Preconditions.checkNotNull(sievePoster); - return new SieveExecutor(mailetContext, usersRepos, sievePoster, resourceLocator, log); + return new SieveExecutor(mailetContext, sievePoster, resourceLocator, log); } } private final MailetContext mailetContext; - private final UsersRepository usersRepos; private final SievePoster sievePoster; private final ResourceLocator resourceLocator; private final SieveFactory factory; private final ActionDispatcher actionDispatcher; private final Log log; - public SieveExecutor(MailetContext mailetContext, UsersRepository usersRepos, SievePoster sievePoster, + public SieveExecutor(MailetContext mailetContext, SievePoster sievePoster, ResourceLocator resourceLocator, Log log) throws MessagingException { this.mailetContext = mailetContext; - this.usersRepos = usersRepos; this.sievePoster = sievePoster; this.resourceLocator = resourceLocator; factory = createFactory(log); @@ -131,7 +119,7 @@ public class SieveExecutor { protected void sieveMessage(MailAddress recipient, Mail aMail, Log log) throws MessagingException { try { - ResourceLocator.UserSieveInformation userSieveInformation = resourceLocator.get(getScriptUri(recipient)); + ResourceLocator.UserSieveInformation userSieveInformation = resourceLocator.get(recipient); sieveMessageEvaluate(recipient, aMail, userSieveInformation, log); } catch (ScriptNotFoundException e) { log.info("Can not locate SIEVE script for user " + recipient.asPrettyString()); @@ -147,7 +135,7 @@ public class SieveExecutor { userSieveInformation.getScriptInterpretationDate(), recipient); aMailAdapter.setLog(log); // This logging operation is potentially costly - log.debug("Evaluating " + aMailAdapter.toString() + "against \"" + getScriptUri(recipient) + "\""); + log.debug("Evaluating " + aMailAdapter.toString() + " against \"" + recipient.asPrettyString() + "\""); factory.evaluate(aMailAdapter, factory.parse(userSieveInformation.getScriptContent())); } catch (SieveException ex) { handleFailure(recipient, aMail, ex); @@ -160,24 +148,7 @@ public class SieveExecutor { } } - protected String getScriptUri(MailAddress m) { - return "//" + retrieveUserNameUsedForScriptStorage(m) + "/sieve"; - } - protected void handleFailure(MailAddress recipient, Mail aMail, Exception ex) throws MessagingException, IOException { mailetContext.sendMail(recipient, ImmutableList.of(recipient), SieveFailureMessageComposer.composeMessage(aMail, ex, recipient.toString())); } - - public String retrieveUserNameUsedForScriptStorage(MailAddress mailAddress) { - try { - if (usersRepos.supportVirtualHosting()) { - return mailAddress.asString(); - } else { - return mailAddress.getLocalPart() + "@localhost"; - } - } catch (UsersRepositoryException e) { - log.warn("Can not determine if virtual hosting is used for " + mailAddress.asPrettyString(), e); - return mailAddress.getLocalPart() + "@localhost"; - } - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorImplTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorImplTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorImplTest.java deleted file mode 100644 index adeee4e..0000000 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorImplTest.java +++ /dev/null @@ -1,57 +0,0 @@ -/**************************************************************** - * 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.transport.mailets; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.apache.james.sieverepository.api.SieveRepository; -import org.apache.james.sieverepository.api.exception.ScriptNotFoundException; -import org.junit.Before; -import org.junit.Test; - -import java.io.ByteArrayInputStream; -import java.io.InputStream; - -public class ResourceLocatorImplTest { - - private SieveRepository sieveRepository; - private ResourceLocatorImpl resourceLocator; - - @Before - public void setUp() { - sieveRepository = mock(SieveRepository.class); - resourceLocator = new ResourceLocatorImpl(true, sieveRepository); - } - - @Test(expected = ScriptNotFoundException.class) - public void resourceLocatorImplShouldPropagateScriptNotFound() throws Exception { - when(sieveRepository.getActive("receiver@localhost")).thenThrow(new ScriptNotFoundException()); - resourceLocator.get("//receiver@localhost/sieve"); - } - - @Test - public void resourceLocatorImplShouldWork() throws Exception { - InputStream inputStream = new ByteArrayInputStream(new byte[0]); - when(sieveRepository.getActive("receiver@localhost")).thenReturn(inputStream); - assertThat(resourceLocator.get("//receiver@localhost/sieve").getScriptContent()).isEqualTo(inputStream); - } -} http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorTest.java new file mode 100644 index 0000000..79a12c0 --- /dev/null +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/ResourceLocatorTest.java @@ -0,0 +1,69 @@ +/**************************************************************** + * 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.transport.mailets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.james.sieverepository.api.SieveRepository; +import org.apache.james.sieverepository.api.exception.ScriptNotFoundException; +import org.apache.james.transport.mailets.jsieve.ResourceLocator; +import org.apache.james.user.api.UsersRepository; +import org.apache.mailet.MailAddress; +import org.junit.Before; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; + +public class ResourceLocatorTest { + + public static final String RECEIVER_LOCALHOST = "receiver@localhost"; + private SieveRepository sieveRepository; + private ResourceLocator resourceLocator; + private MailAddress mailAddress; + private UsersRepository usersRepository; + + @Before + public void setUp() throws Exception { + sieveRepository = mock(SieveRepository.class); + usersRepository = mock(UsersRepository.class); + resourceLocator = new ResourceLocator(sieveRepository, usersRepository); + mailAddress = new MailAddress(RECEIVER_LOCALHOST); + } + + @Test(expected = ScriptNotFoundException.class) + public void resourceLocatorImplShouldPropagateScriptNotFound() throws Exception { + when(sieveRepository.getActive(RECEIVER_LOCALHOST)).thenThrow(new ScriptNotFoundException()); + when(usersRepository.getUser(mailAddress)).thenReturn(RECEIVER_LOCALHOST); + + resourceLocator.get(mailAddress); + } + + @Test + public void resourceLocatorImplShouldWork() throws Exception { + InputStream inputStream = new ByteArrayInputStream(new byte[0]); + when(sieveRepository.getActive(RECEIVER_LOCALHOST)).thenReturn(inputStream); + when(usersRepository.getUser(mailAddress)).thenReturn(RECEIVER_LOCALHOST); + + assertThat(resourceLocator.get(mailAddress).getScriptContent()).isEqualTo(inputStream); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/296bb643/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/delivery/SieveIntegrationTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/delivery/SieveIntegrationTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/delivery/SieveIntegrationTest.java index 481b5d5..0e669e9 100644 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/delivery/SieveIntegrationTest.java +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/delivery/SieveIntegrationTest.java @@ -92,10 +92,23 @@ public class SieveIntegrationTest { } @Test + public void serviceShouldNotModifyEmailWhenErrorRetrievingScript() throws Exception { + when(usersRepository.supportVirtualHosting()).thenReturn(true); + when(usersRepository.getUser(new MailAddress(RECEIVER_DOMAIN_COM))).thenReturn(RECEIVER_DOMAIN_COM); + when(resourceLocator.get(new MailAddress(RECEIVER_DOMAIN_COM))).thenThrow(new ScriptNotFoundException()); + + FakeMail mail = createMail(); + testee.service(mail); + + assertThat(mail.getAttribute(MailStore.DELIVERY_PATH_PREFIX + RECEIVER_DOMAIN_COM)).isNull(); + assertThat(mail.getState()).isEqualTo(Mail.DEFAULT); + } + + @Test public void mailShouldBeWellDeliveredByDefaultToUserWhenVirtualHostingIsTurnedOn() throws Exception { + prepareTestUsingScript("org/apache/james/transport/mailets/delivery/keep.script"); when(usersRepository.supportVirtualHosting()).thenReturn(true); when(usersRepository.getUser(new MailAddress(RECEIVER_DOMAIN_COM))).thenReturn(RECEIVER_DOMAIN_COM); - when(resourceLocator.get(RECEIVER_DOMAIN_COM)).thenThrow(new ScriptNotFoundException()); FakeMail mail = createMail(); testee.service(mail); @@ -105,9 +118,9 @@ public class SieveIntegrationTest { @Test public void mailShouldBeWellDeliveredByDefaultToUserWhenvirtualHostingIsTurnedOff() throws Exception { + prepareTestUsingScript("org/apache/james/transport/mailets/delivery/keep.script"); when(usersRepository.supportVirtualHosting()).thenReturn(false); when(usersRepository.getUser(new MailAddress("receiver@localhost"))).thenReturn("receiver"); - when(resourceLocator.get("receiver")).thenThrow(new ScriptNotFoundException()); FakeMail mail = createMail(); testee.service(mail); @@ -914,7 +927,7 @@ public class SieveIntegrationTest { when(usersRepository.supportVirtualHosting()).thenReturn(false); when(usersRepository.getUser(new MailAddress(LOCAL_PART + "@localhost"))).thenReturn(LOCAL_PART); when(usersRepository.getUser(new MailAddress(LOCAL_PART + "@domain.com"))).thenReturn(LOCAL_PART); - when(resourceLocator.get("//" + LOCAL_PART + "@localhost/sieve")).thenReturn(new ResourceLocator.UserSieveInformation(scriptCreationDate, + when(resourceLocator.get(new MailAddress(RECEIVER_DOMAIN_COM))).thenReturn(new ResourceLocator.UserSieveInformation(scriptCreationDate, scriptExecutionDate, ClassLoader.getSystemResourceAsStream(script))); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
