diff --git a/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java b/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java index 8283a520c9..fa71103af2 100644 --- a/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java +++ b/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java @@ -22,9 +22,9 @@ import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Date; import java.util.HashMap; -import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; @@ -555,7 +555,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase return messages; } - List> recipients = getRecipients(ruleAction); + Collection> recipients = getRecipients(ruleAction); Pair from = getFrom(ruleAction); @@ -1141,10 +1141,9 @@ public class MailActionExecuter extends ActionExecuterAbstractBase } @SuppressWarnings("unchecked") - private List> getRecipients(Action ruleAction) + private Collection> getRecipients(Action ruleAction) { - - List> recipients = new LinkedList>(); + Map> recipients = new HashMap>(); // set recipient String to = (String)ruleAction.getParameterValue(PARAM_TO); @@ -1155,7 +1154,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase { locale = getLocaleForUser(to); } - recipients.add(new Pair(to, locale)); + recipients.put(to, new Pair(to, locale)); } else { @@ -1186,7 +1185,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase // Check the user name to be a valid email and we don't need to log an error in this case // ALF-19231 // Validate the email, allowing for local email addresses - if (authority != null && authority.length() != 0) + if ((authority != null) && (authority.length() != 0) && (!recipients.containsKey(authority))) { if (personExists(authority)) { @@ -1194,7 +1193,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase if (address != null && address.length() != 0 && validateAddress(address)) { Locale locale = getLocaleForUser(authority); - recipients.add(new Pair(address, locale)); + recipients.put(authority, new Pair(address, locale)); } else { @@ -1202,13 +1201,13 @@ public class MailActionExecuter extends ActionExecuterAbstractBase if (validateAddresses && emailValidator.isValid(authority)) { Locale locale = getLocaleForUser(authority); - recipients.add(new Pair(authority, locale)); + recipients.put(authority, new Pair(authority, locale)); } } } else { - recipients.add(new Pair(authority, null)); + recipients.put(authority, new Pair(authority, null)); } } } @@ -1227,6 +1226,10 @@ public class MailActionExecuter extends ActionExecuterAbstractBase for (String userAuth : users) { + if (recipients.containsKey(userAuth)) + { + continue; + } if (personExists(userAuth)) { // Check the user name to be a valid email and we don't need to log an error in this case @@ -1236,7 +1239,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase if (address != null && address.length() != 0 && validateAddress(address)) { Locale locale = getLocaleForUser(userAuth); - recipients.add(new Pair(address, locale)); + recipients.put(userAuth, new Pair(address, locale)); } else { @@ -1246,14 +1249,14 @@ public class MailActionExecuter extends ActionExecuterAbstractBase if (userAuth != null && userAuth.length() != 0) { Locale locale = getLocaleForUser(userAuth); - recipients.add(new Pair(userAuth, locale)); + recipients.put(userAuth, new Pair(userAuth, locale)); } } } } else { - recipients.add(new Pair(authority, null)); + recipients.put(userAuth, new Pair(authority, null)); } } } @@ -1274,7 +1277,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase ); } } - return recipients; + return recipients.values(); } public boolean personExists(final String user) diff --git a/source/test-java/org/alfresco/repo/action/executer/AbstractMailActionExecuterTest.java b/source/test-java/org/alfresco/repo/action/executer/AbstractMailActionExecuterTest.java index a6403174f1..57754bbeff 100644 --- a/source/test-java/org/alfresco/repo/action/executer/AbstractMailActionExecuterTest.java +++ b/source/test-java/org/alfresco/repo/action/executer/AbstractMailActionExecuterTest.java @@ -36,6 +36,7 @@ import javax.mail.internet.MimeMessage; import org.alfresco.model.ContentModel; import org.alfresco.repo.management.subsystems.ApplicationContextFactory; import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.tenant.TenantService; import org.alfresco.repo.tenant.TenantUtil; import org.alfresco.repo.tenant.TenantUtil.TenantRunAsWork; @@ -48,6 +49,7 @@ import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.security.AuthorityType; +import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.cmr.security.PersonService; import org.alfresco.service.namespace.QName; import org.alfresco.service.transaction.TransactionService; @@ -90,6 +92,7 @@ public abstract class AbstractMailActionExecuterTest protected static PersonService PERSON_SERVICE; protected static AuthorityService AUTHORITY_SERVICE; protected static NodeService NODE_SERVICE; + protected static PermissionService PERMISSION_SERVICE; protected static boolean WAS_IN_TEST_MODE; @@ -102,6 +105,7 @@ public abstract class AbstractMailActionExecuterTest PERSON_SERVICE = appCtx.getBean("PersonService", PersonService.class); NODE_SERVICE = appCtx.getBean("NodeService", NodeService.class); AUTHORITY_SERVICE = appCtx.getBean("AuthorityService", AuthorityService.class); + PERMISSION_SERVICE = appCtx.getBean("PermissionService", PermissionService.class); WAS_IN_TEST_MODE = ACTION_EXECUTER.isTestMode(); ACTION_EXECUTER.setTestMode(true); @@ -528,6 +532,129 @@ public abstract class AbstractMailActionExecuterTest } } + /** + * Test for MNT-12464 + * @throws Exception + */ + @Test + public void testMultipleIdenticalEmailsToUser() throws Exception + { + final String USER_1 = "user12464_1"; + final String USER_2 = "user12464_2"; + final String USER_3 = "user12464_3"; + final String USER_6 = "user12464_6"; + + final String USER_4_USERNAME = "user12464_4@mnt12464mail.com"; + final String USER_5_USERNAME = "user12464_5@mnt12464mail.com"; + + String[] users = new String[] { USER_1, USER_2, USER_3, USER_4_USERNAME, USER_5_USERNAME }; + + final String GROUP_1_SHORT_NAME = "mnt12464group1"; + final String GROUP_1 = "GROUP_" + GROUP_1_SHORT_NAME; + final String GROUP_2_SHORT_NAME = "mnt12464group2"; + final String GROUP_2 = "GROUP_" + GROUP_2_SHORT_NAME; + + try + { + createUser(USER_1, null); + createUser(USER_2, null); + createUser(USER_3, null); + AUTHORITY_SERVICE.createAuthority(AuthorityType.GROUP, GROUP_1_SHORT_NAME); + AUTHORITY_SERVICE.createAuthority(AuthorityType.GROUP, GROUP_2_SHORT_NAME); + AUTHORITY_SERVICE.addAuthority(GROUP_1, USER_1); + AUTHORITY_SERVICE.addAuthority(GROUP_1, USER_2); + AUTHORITY_SERVICE.addAuthority(GROUP_2, USER_1); + AUTHORITY_SERVICE.addAuthority(GROUP_2, USER_2); + AUTHORITY_SERVICE.addAuthority(GROUP_2, USER_3); + + // these persons should be without emails + PropertyMap personProps = new PropertyMap(); + personProps.put(ContentModel.PROP_USERNAME, USER_4_USERNAME); + personProps.put(ContentModel.PROP_FIRSTNAME, USER_4_USERNAME); + personProps.put(ContentModel.PROP_LASTNAME, USER_4_USERNAME); + PERSON_SERVICE.createPerson(personProps); + AUTHORITY_SERVICE.addAuthority(GROUP_1, USER_4_USERNAME); + AUTHORITY_SERVICE.addAuthority(GROUP_2, USER_4_USERNAME); + + personProps = new PropertyMap(); + personProps.put(ContentModel.PROP_USERNAME, USER_5_USERNAME); + personProps.put(ContentModel.PROP_FIRSTNAME, USER_5_USERNAME); + personProps.put(ContentModel.PROP_LASTNAME, USER_5_USERNAME); + PERSON_SERVICE.createPerson(personProps); + + Action mailAction1 = ACTION_SERVICE.createAction(MailActionExecuter.NAME); + mailAction1.setParameterValue(MailActionExecuter.PARAM_FROM, "some.body@example.com"); + ArrayList toMany1 = new ArrayList(); + toMany1.add(USER_1); + toMany1.add(GROUP_1); + toMany1.add(USER_2); + toMany1.add(GROUP_2); + toMany1.add(USER_3); + toMany1.add(USER_4_USERNAME); + toMany1.add(USER_5_USERNAME); + mailAction1.setParameterValue(MailActionExecuter.PARAM_TO_MANY, toMany1); + mailAction1.setParameterValue(MailActionExecuter.PARAM_SUBJECT, "Testing MNT-12464"); + mailAction1.setParameterValue(MailActionExecuter.PARAM_TEMPLATE, "alfresco/templates/mail/test.txt.ftl"); + mailAction1.setParameterValue(MailActionExecuter.PARAM_TEMPLATE_MODEL, (Serializable) getModel()); + + ACTION_EXECUTER.resetTestSentCount(); + ACTION_SERVICE.executeAction(mailAction1, null); + + Assert.assertEquals("Must be received one letter on each recipient", users.length, ACTION_EXECUTER.getTestSentCount()); + + // testing for GROUP_EVERYONE + + ACTION_EXECUTER.resetTestSentCount(); + everyoneSending(); + int before = ACTION_EXECUTER.getTestSentCount(); + // create one additional user + NodeRef user6 = createUser(USER_6, null); + PERMISSION_SERVICE.setInheritParentPermissions(user6, false); + PERMISSION_SERVICE.deletePermissions(user6); + + // USER_6 not exist for USER_1, but he will be added to recipients + int after = AuthenticationUtil.runAs(new RunAsWork() + { + @Override + public Integer doWork() throws Exception + { + ACTION_EXECUTER.resetTestSentCount(); + everyoneSending(); + return ACTION_EXECUTER.getTestSentCount(); + } + }, USER_1); + + Assert.assertEquals("One additional user was created, quantity of recipients GROUP_EVERYONE must be +1 user", 1, after - before); + } + finally + { + PERSON_SERVICE.deletePerson(USER_1); + PERSON_SERVICE.deletePerson(USER_2); + PERSON_SERVICE.deletePerson(USER_3); + PERSON_SERVICE.deletePerson(USER_4_USERNAME); + PERSON_SERVICE.deletePerson(USER_5_USERNAME); + PERSON_SERVICE.deletePerson(USER_6); + AUTHORITY_SERVICE.deleteAuthority(GROUP_1); + AUTHORITY_SERVICE.deleteAuthority(GROUP_2); + } + } + + private void everyoneSending() + { + Action mailAction = ACTION_SERVICE.createAction(MailActionExecuter.NAME); + mailAction.setParameterValue(MailActionExecuter.PARAM_FROM, "some.body@example.com"); + ArrayList toMany = new ArrayList(); + toMany.add(PERMISSION_SERVICE.getAllAuthorities()); + mailAction.setParameterValue(MailActionExecuter.PARAM_TO_MANY, toMany); + mailAction.setParameterValue(MailActionExecuter.PARAM_SUBJECT, "Testing MNT-12464"); + mailAction.setParameterValue(MailActionExecuter.PARAM_TEMPLATE, "alfresco/templates/mail/test.txt.ftl"); + mailAction.setParameterValue(MailActionExecuter.PARAM_TEMPLATE_MODEL, (Serializable) getModel()); + + ACTION_EXECUTER.resetTestSentCount(); + + ACTION_SERVICE.executeAction(mailAction, null); + } + /** * ACE-2564 */ @@ -537,14 +664,25 @@ public abstract class AbstractMailActionExecuterTest final Serializable recipients = (Serializable) Arrays.asList(BRITISH_USER.getUsername()); final String subject = ""; final String template = "alfresco/templates/mail/test.txt.ftl"; - AuthenticationUtil.pushAuthentication(); AuthenticationUtil.setFullyAuthenticatedUser(EXTERNAL_USER.getUsername()); - MimeMessage message = null; + + try { + + // these persons should be without emails + + + + + + // testing for GROUP_EVERYONE + final String tenantId = getUsersHomeTenant(BRITISH_USER.getUsername()); + + // USER_6 not exist for USER_1, but he will be added to recipients message = TenantUtil.runAsTenant(new TenantRunAsWork() { @Override @@ -563,4 +701,6 @@ public abstract class AbstractMailActionExecuterTest } } + + }