From d257bbc9d0fd0c840b3073b6c27ca2c0bfe36cea Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Mon, 3 Oct 2011 16:23:14 +0000 Subject: [PATCH] Merged V3.4-BUG-FIX to HEAD 30744: ALF-9524: WCM: Defining a complex type with 'abstract' attribute to true does not allow other complex types to define elements with the same name - Fix by Pavel 30804: ALF-9524: Fix to NPE by Pavel 30812: ALF-718: Rules Fire Emails before Transaction Commit/Rollback Added tests and moved email sending into a post commit callback. 30820: Fix for ALF-10516 - 'My Sites' dashlet should indicate it is loading rather than say 'No sites' 30824: Fixed ALF-10470: DeclaritiveRegistry is looping continuously and re-initializing - Only replicate removals for the cache. The reset always does a remove first. 30827: ALF-10513 60k Site Performance: Unable to delete user + request not acknowledged for 2 minutes Now about 3 to 4 seconds 30828: Andy H recommended change to lucene indexer values 30831: ALF-718: Fix for InvitationServiceImplTest Fixes behaviour broken by original fix for ALF-718: the send-after-commit does NOT work for the invitation service, so it was necessary to change the code to only send-after-commit in the context of a rule. 30843: Fixed ALF-7698 "Defects in tags picker in SHARE." according to feedback provided in ALF-9953 "Decide order where new items shall appear in the object finder." 30844: ALF-9544 - Inbound email restricts file name to 86 characters or less. used QName.createQNameWithValidLocalName() as suggested. added new EmailServiceImplTest 30849: Fixed ALF-8776 "Rule details dialog handling of apostrophes" 30862: Merged DEV/TEMPORARY to V3.4-BUG-FIX (with improvements) 30856: ALF-10288: Regression of ALF-1997: non domain users cannot bypass SSO in Share using /share/page?pt=login In SSOAuthenticationFilter.doFilter() method added check (PAGE_SERVLET_PATH.equals(req.getServletPath()) && LOGIN_PATH_INFORMATION.equals(req.getPathInfo()). 30864: SMTP Server, To and From address format. - Out standards for from and to address were stuck in the 1980s! 30867: ALF-10517 - 'My Content' slow - performance improvements by reworking Lucene queries used to retrieve recently modified and recently created content for a user - converted queries to fts-alfresco - improvements from 3.5sec per page render down to 1.1 secs 30874: MERGE DEV to V3.4-BUG-FIX 30851 : ALF-9558 - Unchecked Return Value 30882: - ScriptGroup.isRootGroup() now stops if it finds a single parent rather than doing a size on all parent groups. Not sure the logic (not changed by me) is correct as it includes parents that are site groups. - SiteServiceImpl now only gets the specified number of sites. Sometimes it would have return more if AuthorityDAOImpl switched approach used to get containing authorities. - Overloaded getContainingAuthorities to take a size parameter, deprecated other and switched calling code Done for isRootGroup activity. 30910: - Fix build error - Found a better way to use filter in AuthorityDAOImpl.getContainingAuthoritiesInZone so that we don't need to reset the filter, by using the +ve hits from first approach if we switch to second approach. 30925: - ALF-10501 60k Site Performance: Searching for a group to add to a site with a value that matches a few sites takes 2 minutes ALF-10502 60k Site Performance: Admin Console | Groups: search with a value that matches a few sites takes 70 seconds ALF-10504 60k Site Performance: Admin Console | Groups | Browse Groups page appears corrupt as it never finishes loading - All the above were slow as a result of ScriptGroup.isRootGroup() - ScriptGroup.isRootGroup() and ScriptGroup.isAdminGroup() removed - Checked with KevinR that these are not needed any more. - Removed code added on 30 Sep 11 to support isRootGroup (the size parameter to AuthorityDAO.getContainingAuthorities) git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@30935 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../ehcache-custom.xml.sample.cluster | 2 +- config/alfresco/repository.properties | 6 +- .../email/server/EmailServiceImpl.java | 34 +++++- .../email/server/EmailServiceImplTest.java | 110 ++++++++++++++---- .../impl/subetha/SubethaEmailMessage.java | 22 +++- .../impl/subetha/SubethaEmailServer.java | 26 ++++- .../action/executer/MailActionExecuter.java | 38 ++++++ .../preference/PreferenceServiceImpl.java | 7 -- .../repo/rule/RuleServiceCoverageTest.java | 75 +++++++++++- .../alfresco/repo/rule/RuleServiceImpl.java | 46 +++++++- .../repo/security/authority/AuthorityDAO.java | 1 - .../security/authority/AuthorityDAOImpl.java | 35 ++++-- .../authority/script/ScriptGroup.java | 24 ---- source/java/org/alfresco/util/ModelUtil.java | 1 + 14 files changed, 338 insertions(+), 89 deletions(-) diff --git a/config/alfresco/extension/ehcache-custom.xml.sample.cluster b/config/alfresco/extension/ehcache-custom.xml.sample.cluster index c593c77fba..e78ec63bc0 100644 --- a/config/alfresco/extension/ehcache-custom.xml.sample.cluster +++ b/config/alfresco/extension/ehcache-custom.xml.sample.cluster @@ -725,7 +725,7 @@ diff --git a/config/alfresco/repository.properties b/config/alfresco/repository.properties index 178ac98842..72b4c83706 100644 --- a/config/alfresco/repository.properties +++ b/config/alfresco/repository.properties @@ -243,12 +243,12 @@ fts.indexer.batchSize=1000 # Index cache sizes # lucene.indexer.cacheEnabled=true -lucene.indexer.maxDocIdCacheSize=10000 +lucene.indexer.maxDocIdCacheSize=100000 lucene.indexer.maxDocumentCacheSize=100 lucene.indexer.maxIsCategoryCacheSize=-1 lucene.indexer.maxLinkAspectCacheSize=10000 -lucene.indexer.maxParentCacheSize=10000 -lucene.indexer.maxPathCacheSize=10000 +lucene.indexer.maxParentCacheSize=100000 +lucene.indexer.maxPathCacheSize=100000 lucene.indexer.maxTypeCacheSize=10000 # # Properties for merge (not this does not affect the final index segment which will be optimised) diff --git a/source/java/org/alfresco/email/server/EmailServiceImpl.java b/source/java/org/alfresco/email/server/EmailServiceImpl.java index 9b177f537b..77e48aeef1 100644 --- a/source/java/org/alfresco/email/server/EmailServiceImpl.java +++ b/source/java/org/alfresco/email/server/EmailServiceImpl.java @@ -42,6 +42,8 @@ import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.security.AuthorityType; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.extensions.surf.util.ParameterCheck; /** @@ -65,6 +67,14 @@ public class EmailServiceImpl implements EmailService private SearchService searchService; private RetryingTransactionHelper retryingTransactionHelper; private AuthorityService authorityService; + + /** + * The authority that needs to contain the users and groups + * who are allowed to contribute email. + */ + private String emailContributorsAuthority="EMAIL_CONTRIBUTORS"; + + private static Log logger = LogFactory.getLog(EmailServiceImpl.class); private boolean emailInboundEnabled; /** Login of user that is set as unknown. */ @@ -337,10 +347,19 @@ public class EmailServiceImpl implements EmailService { if (unknownUser == null || unknownUser.length() == 0) { + if(logger.isDebugEnabled()) + { + logger.debug("unable to find user for from: " + from); + } throw new EmailMessageException(ERR_UNKNOWN_SOURCE_ADDRESS, from); } else { + if(logger.isDebugEnabled()) + { + logger.debug("unable to find user for from - return anonymous: " + from); + } + userName = unknownUser; } } @@ -375,13 +394,24 @@ public class EmailServiceImpl implements EmailService /** * Check that the user is the member in EMAIL_CONTRIBUTORS group * - * @param userName User name + * @param userName Alfresco user name * @return True if the user is member of the group * @exception EmailMessageException Exception will be thrown if the EMAIL_CONTRIBUTORS group isn't found */ private boolean isEmailContributeUser(String userName) { return this.authorityService.getAuthoritiesForUser(userName).contains( - authorityService.getName(AuthorityType.GROUP, "EMAIL_CONTRIBUTORS")); + authorityService.getName(AuthorityType.GROUP, getEmailContributorsAuthority())); + } + + public void setEmailContributorsAuthority( + String emailContributorsAuthority) + { + this.emailContributorsAuthority = emailContributorsAuthority; + } + + public String getEmailContributorsAuthority() + { + return emailContributorsAuthority; } } diff --git a/source/java/org/alfresco/email/server/EmailServiceImplTest.java b/source/java/org/alfresco/email/server/EmailServiceImplTest.java index 39ef252d9c..1fccc9709f 100644 --- a/source/java/org/alfresco/email/server/EmailServiceImplTest.java +++ b/source/java/org/alfresco/email/server/EmailServiceImplTest.java @@ -18,52 +18,36 @@ */ package org.alfresco.email.server; -import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.InputStream; -import java.io.OutputStream; +import java.io.Serializable; +import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.HashMap; import java.util.Properties; import java.util.Set; -import java.io.Serializable; -import javax.mail.Address; import javax.mail.Message; import javax.mail.Session; import javax.mail.internet.InternetAddress; -import javax.mail.internet.MimeMessage; + +import junit.framework.TestCase; import org.alfresco.email.server.impl.subetha.SubethaEmailMessage; import org.alfresco.model.ContentModel; -import org.alfresco.repo.imap.ImapServiceImpl; import org.alfresco.repo.management.subsystems.ChildApplicationContextFactory; import org.alfresco.repo.security.authentication.AuthenticationUtil; -import org.alfresco.repo.security.person.PersonServiceImpl; -import org.alfresco.service.ServiceRegistry; -import org.alfresco.service.cmr.admin.AdminService; -import org.alfresco.service.cmr.coci.CheckOutCheckInService; -import org.alfresco.service.cmr.email.EmailMessage; import org.alfresco.service.cmr.email.EmailMessageException; import org.alfresco.service.cmr.email.EmailService; -import org.alfresco.service.cmr.lock.LockService; -import org.alfresco.service.cmr.repository.ContentService; -import org.alfresco.service.cmr.repository.CopyService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AuthorityService; -import org.alfresco.service.cmr.security.MutableAuthenticationService; -import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.cmr.security.PersonService; -import org.alfresco.service.cmr.version.VersionService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; -import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; -import org.alfresco.util.BaseSpringTest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.tools.ant.filters.StringInputStream; @@ -71,8 +55,6 @@ import org.springframework.context.ApplicationContext; import com.sun.mail.smtp.SMTPMessage; -import junit.framework.TestCase; - /** * Unit test of EmailServiceImplTest * @author mrogers @@ -205,7 +187,7 @@ public class EmailServiceImplTest extends TestCase InputStream is = new StringInputStream(bos.toString()); assertNotNull("is is null", is); - SubethaEmailMessage m = new SubethaEmailMessage(from, to, is); + SubethaEmailMessage m = new SubethaEmailMessage(is); emailService.importMessage(m); fail("anonymous user not rejected"); @@ -221,6 +203,9 @@ public class EmailServiceImplTest extends TestCase * * Send From the test user TEST_EMAIL to the test user's home */ + { + logger.debug("Step 2"); + String from = TEST_EMAIL; String to = testUserHomeDBID; String content = "hello world"; @@ -241,10 +226,85 @@ public class EmailServiceImplTest extends TestCase InputStream is = new StringInputStream(bos.toString()); assertNotNull("is is null", is); - SubethaEmailMessage m = new SubethaEmailMessage(from, to, is); + SubethaEmailMessage m = new SubethaEmailMessage(is); emailService.importMessage(m); } + + /** + * Step 3 + * + * From with < name@ domain > format + * + * Send From the test user to the test user's home + */ + { + logger.debug("Step 3"); + + String from = " \"Joe Bloggs\" <" + TEST_EMAIL + ">"; + String to = testUserHomeDBID; + String content = "hello world"; + + Session sess = Session.getDefaultInstance(new Properties()); + assertNotNull("sess is null", sess); + SMTPMessage msg = new SMTPMessage(sess); + InternetAddress[] toa = { new InternetAddress(to) }; + + msg.setFrom(new InternetAddress(from)); + msg.setRecipients(Message.RecipientType.TO, toa); + msg.setSubject("JavaMail APIs transport.java Test"); + msg.setContent(content, "text/plain"); + + StringBuffer sb = new StringBuffer(); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + msg.writeTo(System.out); + msg.writeTo(bos); + InputStream is = new StringInputStream(bos.toString()); + assertNotNull("is is null", is); + + SubethaEmailMessage m = new SubethaEmailMessage(is); + + emailService.importMessage(m); + } + +// /** +// * Step 4 +// * +// * From with format +// * +// * RFC3696 +// * +// * Send From the test user to the test user's home +// */ +// { +// logger.debug("Step 4 format"); +// +// String from = "\"Joe Bloggs\" "; +// String to = testUserHomeDBID; +// String content = "hello world"; +// +// Session sess = Session.getDefaultInstance(new Properties()); +// assertNotNull("sess is null", sess); +// SMTPMessage msg = new SMTPMessage(sess); +// InternetAddress[] toa = { new InternetAddress(to) }; +// +// msg.setFrom(new InternetAddress(from)); +// msg.setRecipients(Message.RecipientType.TO, toa); +// msg.setSubject("JavaMail APIs transport.java Test"); +// msg.setContent(content, "text/plain"); +// +// StringBuffer sb = new StringBuffer(); +// ByteArrayOutputStream bos = new ByteArrayOutputStream(); +// msg.writeTo(System.out); +// msg.writeTo(bos); +// InputStream is = new StringInputStream(bos.toString()); +// assertNotNull("is is null", is); +// +// SubethaEmailMessage m = new SubethaEmailMessage(is); +// +// emailService.importMessage(m); +// } + } /** @@ -313,7 +373,7 @@ public class EmailServiceImplTest extends TestCase InputStream is = new StringInputStream(bos.toString()); assertNotNull("is is null", is); - SubethaEmailMessage m = new SubethaEmailMessage(from, to, is); + SubethaEmailMessage m = new SubethaEmailMessage(is); emailService.importMessage(m); diff --git a/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailMessage.java b/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailMessage.java index dc66d19101..5571ba54d8 100644 --- a/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailMessage.java +++ b/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailMessage.java @@ -101,11 +101,8 @@ public class SubethaEmailMessage implements EmailMessage processMimeMessage(mimeMessage); } - public SubethaEmailMessage(String from, String to, InputStream dataInputStream) + public SubethaEmailMessage(InputStream dataInputStream) { - this.to = to; - this.from = from; - MimeMessage mimeMessage = null; try { @@ -136,8 +133,16 @@ public class SubethaEmailMessage implements EmailMessage { throw new EmailMessageException(ERR_NO_FROM_ADDRESS); } + if(addresses[0] instanceof InternetAddress) + { + from = ((InternetAddress)addresses[0]).getAddress(); + } + else + { from = addresses[0].toString(); } + + } if (to == null) { @@ -154,6 +159,15 @@ public class SubethaEmailMessage implements EmailMessage { throw new EmailMessageException(ERR_NO_TO_ADDRESS); } + if(addresses[0] instanceof InternetAddress) + { + to = ((InternetAddress)addresses[0]).getAddress(); + } + else + { + to = addresses[0].toString(); + } + to = addresses[0].toString(); } diff --git a/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailServer.java b/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailServer.java index 8ce2ca15ae..792e05aada 100644 --- a/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailServer.java +++ b/source/java/org/alfresco/email/server/impl/subetha/SubethaEmailServer.java @@ -24,6 +24,9 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import javax.mail.internet.AddressException; +import javax.mail.internet.InternetAddress; + import org.alfresco.email.server.EmailServer; import org.alfresco.service.cmr.email.EmailMessage; import org.alfresco.service.cmr.email.EmailMessageException; @@ -41,7 +44,7 @@ import org.subethamail.smtp.server.SMTPServer; */ public class SubethaEmailServer extends EmailServer { - private final static Log log = LogFactory.getLog(SubethaEmailServer.class); + private final static Log logger = LogFactory.getLog(SubethaEmailServer.class); private SMTPServer serverImpl; @@ -91,7 +94,6 @@ public class SubethaEmailServer extends EmailServer private List EMPTY_LIST = new LinkedList(); - private String from; private MessageContext messageContext; List deliveries = new ArrayList(); @@ -105,11 +107,25 @@ public class SubethaEmailServer extends EmailServer return messageContext; } - public void from(String from) throws RejectException + public void from(String fromString) throws RejectException { - this.from = from; + String from = fromString; + try { + InternetAddress a = new InternetAddress(from); + from = a.getAddress(); + } + catch (AddressException e) + { + } + + try + { + if(logger.isDebugEnabled()) + { + logger.debug("check whether user is allowed to send email from" + from); + } filterSender(from); } catch (EmailMessageException e) @@ -187,7 +203,7 @@ public class SubethaEmailServer extends EmailServer EmailMessage emailMessage; try { - emailMessage = new SubethaEmailMessage(from, delivery.getRecipient(), data); + emailMessage = new SubethaEmailMessage(data); getEmailService().importMessage(emailMessage); } catch (EmailMessageException e) diff --git a/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java b/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java index 2cc261b4ac..655f3f2509 100644 --- a/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java +++ b/source/java/org/alfresco/repo/action/executer/MailActionExecuter.java @@ -36,6 +36,8 @@ import org.alfresco.repo.template.DateCompareMethod; import org.alfresco.repo.template.HasAspectMethod; import org.alfresco.repo.template.I18NMessageMethod; import org.alfresco.repo.template.TemplateNode; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport; +import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.action.ParameterDefinition; @@ -81,6 +83,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase public static final String PARAM_TEMPLATE = "template"; public static final String PARAM_TEMPLATE_MODEL = "template_model"; public static final String PARAM_IGNORE_SEND_FAILURE = "ignore_send_failure"; + public static final String PARAM_SEND_AFTER_COMMIT = "send_after_commit"; /** * From address @@ -297,6 +300,31 @@ public class MailActionExecuter extends ActionExecuterAbstractBase protected void executeImpl( final Action ruleAction, final NodeRef actionedUponNodeRef) + { + if (sendAfterCommit(ruleAction)) + { + AlfrescoTransactionSupport.bindListener(new TransactionListenerAdapter() + { + @Override + public void afterCommit() + { + prepareAndSendEmail(ruleAction, actionedUponNodeRef); + } + }); + } + else + { + prepareAndSendEmail(ruleAction, actionedUponNodeRef); + } + } + + private boolean sendAfterCommit(Action action) + { + Boolean sendAfterCommit = (Boolean) action.getParameterValue(PARAM_SEND_AFTER_COMMIT); + return sendAfterCommit == null ? false : sendAfterCommit.booleanValue(); + } + + private void prepareAndSendEmail(final Action ruleAction, final NodeRef actionedUponNodeRef) { // Create the mime mail message MimeMessagePreparator mailPreparer = new MimeMessagePreparator() @@ -546,6 +574,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase } } + /** * Return true if address has valid format * @param address @@ -658,6 +687,15 @@ public class MailActionExecuter extends ActionExecuterAbstractBase { return lastTestMessage; } + + /** + * Used when test mode is enabled. + * Clears the record of the last message that was sent. + */ + public void clearLastTestMessage() + { + lastTestMessage = null; + } public static class URLHelper { diff --git a/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java b/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java index 640396505d..12d194f6eb 100644 --- a/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java +++ b/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java @@ -28,7 +28,6 @@ import java.util.Map; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; import org.alfresco.repo.content.MimetypeMap; -import org.alfresco.repo.rule.RuleModel; import org.alfresco.repo.security.authentication.AuthenticationContext; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; @@ -256,12 +255,6 @@ public class PreferenceServiceImpl implements PreferenceService contentWriter.setEncoding("UTF-8"); contentWriter.setMimetype(MimetypeMap.MIMETYPE_TEXT_PLAIN); contentWriter.putContent(jsonPrefs.toString()); - - // Lets stop rule inheritance from trying to kick in - we may be in many groups - if (!PreferenceServiceImpl.this.nodeService.hasAspect(personNodeRef, RuleModel.ASPECT_IGNORE_INHERITED_RULES)) - { - PreferenceServiceImpl.this.nodeService.addAspect(personNodeRef, RuleModel.ASPECT_IGNORE_INHERITED_RULES, null); - } } catch (JSONException exception) { diff --git a/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java b/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java index 61775cb7f6..e340e8ef71 100644 --- a/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java +++ b/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java @@ -19,6 +19,7 @@ package org.alfresco.repo.rule; import java.io.File; +import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; @@ -27,6 +28,8 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import javax.mail.MessagingException; +import javax.mail.internet.MimeMessage; import javax.transaction.UserTransaction; import junit.framework.TestCase; @@ -57,6 +60,8 @@ import org.alfresco.repo.dictionary.IndexTokenisationMode; import org.alfresco.repo.dictionary.M2Aspect; import org.alfresco.repo.dictionary.M2Model; import org.alfresco.repo.dictionary.M2Property; +import org.alfresco.repo.management.subsystems.ApplicationContextFactory; +import org.alfresco.repo.node.integrity.IntegrityException; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; @@ -898,11 +903,11 @@ public class RuleServiceCoverageTest extends TestCase * Test: * rule type: inbound * condition: no-condition - * action: mail - * - * Note: this test will be removed from the standard list since it is not currently automated + * action: mail + * @throws MessagingException + * @throws IOException */ - public void xtestMailAction() + public void testMailAction() throws MessagingException, IOException { this.nodeService.addAspect(this.nodeRef, ContentModel.ASPECT_LOCKABLE, null); @@ -919,6 +924,11 @@ public class RuleServiceCoverageTest extends TestCase null); this.ruleService.saveRule(this.nodeRef, rule); + + MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) this.applicationContext + .getBean("OutboundSMTP")).getApplicationContext().getBean("mail"); + mailService.setTestMode(true); + mailService.clearLastTestMessage(); this.nodeService.createNode( this.nodeRef, @@ -928,8 +938,63 @@ public class RuleServiceCoverageTest extends TestCase getContentProperties()).getChildRef(); // An email should appear in the recipients email - // System.out.println(NodeStoreInspector.dumpNodeStore(this.nodeService, this.testStoreRef)); + MimeMessage lastMessage = mailService.retrieveLastTestMessage(); + assertNotNull("Message should have been sent", lastMessage); + System.out.println("Sent email with subject: " + lastMessage.getSubject()); + System.out.println("Sent email with content: " + lastMessage.getContent()); + } + + public void testMailNotSentIfRollback() + { + this.nodeService.addAspect(this.nodeRef, ContentModel.ASPECT_LOCKABLE, null); + + Map params = new HashMap(1); + params.put(MailActionExecuter.PARAM_TO, "alfresco.test@gmail.com"); + params.put(MailActionExecuter.PARAM_SUBJECT, "testMailNotSentIfRollback()"); + params.put(MailActionExecuter.PARAM_TEXT, "This email should NOT have been sent."); + + Rule rule = createRule( + RuleType.INBOUND, + MailActionExecuter.NAME, + params, + NoConditionEvaluator.NAME, + null); + + this.ruleService.saveRule(this.nodeRef, rule); + + String illegalName = "MyName.txt "; // space at end + + MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) this.applicationContext + .getBean("OutboundSMTP")).getApplicationContext().getBean("mail"); + mailService.setTestMode(true); + mailService.clearLastTestMessage(); + + try + { + this.nodeService.createNode( + this.nodeRef, + ContentModel.ASSOC_CHILDREN, + QName.createQName(TEST_NAMESPACE, "children"), + ContentModel.TYPE_CONTENT, + makeNameProperty(illegalName)).getChildRef(); + fail("createNode() should have failed."); + } + catch(IntegrityException e) + { + // Expected exception. + // An email should NOT appear in the recipients email + } + + MimeMessage lastMessage = mailService.retrieveLastTestMessage(); + assertNull("Message should NOT have been sent", lastMessage); + } + + private Map makeNameProperty(String name) + { + Map properties = new HashMap(1); + properties.put(ContentModel.PROP_NAME, name); + return properties; } /** diff --git a/source/java/org/alfresco/repo/rule/RuleServiceImpl.java b/source/java/org/alfresco/repo/rule/RuleServiceImpl.java index 505885659b..18b1c14c79 100644 --- a/source/java/org/alfresco/repo/rule/RuleServiceImpl.java +++ b/source/java/org/alfresco/repo/rule/RuleServiceImpl.java @@ -30,6 +30,8 @@ import java.util.Set; import org.alfresco.model.ContentModel; import org.alfresco.repo.action.ActionModel; import org.alfresco.repo.action.RuntimeActionService; +import org.alfresco.repo.action.executer.CompositeActionExecuter; +import org.alfresco.repo.action.executer.MailActionExecuter; import org.alfresco.repo.cache.NullCache; import org.alfresco.repo.cache.SimpleCache; import org.alfresco.repo.node.NodeServicePolicies; @@ -41,6 +43,7 @@ import org.alfresco.repo.transaction.TransactionListener; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.action.ActionService; import org.alfresco.service.cmr.action.ActionServiceException; +import org.alfresco.service.cmr.action.CompositeAction; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.CopyService; @@ -86,6 +89,13 @@ public class RuleServiceImpl private String ASSOC_NAME_RULES_PREFIX = "rules"; private RegexQNamePattern ASSOC_NAME_RULES_REGEX = new RegexQNamePattern(RuleModel.RULE_MODEL_URI, "^" + ASSOC_NAME_RULES_PREFIX + ".*"); + private static final Set IGNORE_PARENT_ASSOC_TYPES = new HashSet(7); + static + { + IGNORE_PARENT_ASSOC_TYPES.add(ContentModel.ASSOC_MEMBER); + IGNORE_PARENT_ASSOC_TYPES.add(ContentModel.ASSOC_IN_ZONE); + } + private static Log logger = LogFactory.getLog(RuleServiceImpl.class); private NodeService nodeService; @@ -611,6 +621,12 @@ public class RuleServiceImpl List parents = this.runtimeNodeService.getParentAssocs(nodeRef); for (ChildAssociationRef parent : parents) { + // We are not interested in following potentially massive person group membership trees! + if (IGNORE_PARENT_ASSOC_TYPES.contains(parent.getTypeQName())) + { + continue; + } + // Add the inherited rule first for (Rule rule : getInheritedRules(parent.getParentRef(), ruleTypeName, visitedNodeRefs)) { @@ -1167,9 +1183,37 @@ public class RuleServiceImpl // Execute the rule boolean executeAsync = rule.getExecuteAsynchronously(); - this.actionService.executeAction(action, actionedUponNodeRef, true, executeAsync); + // ALF-718: Treats email actions as a special case where they may be performed after the + // current transaction is committed. This only deals with the bug fix and a more thorough approach + // (but one with potentially wide ranging consequences) is to replace the boolean executeAsynchronously + // property on Rules and Actions with an ExecutionTime property - which would + // be an enumerated type with members SYNCHRONOUSLY, SYNCRHONOUSLY_AFTER_COMMIT and ASYNCHRONOUSLY. + // + // NOTE: this code is not at the Action level (i.e. ActionService) since the logic of sending after + // successful commit works in the context of a Rule but not for the InvitationService. + if (action.getActionDefinitionName().equals(CompositeActionExecuter.NAME)) + { + for (Action subAction : ((CompositeAction)action).getActions()) + { + if (subAction.getActionDefinitionName().equals(MailActionExecuter.NAME)) + { + subAction.setParameterValue(MailActionExecuter.PARAM_SEND_AFTER_COMMIT, true); } } + } + else if (action.getActionDefinitionName().equals(MailActionExecuter.NAME)) + { + action.setParameterValue(MailActionExecuter.PARAM_SEND_AFTER_COMMIT, true); + } + + executeAction(action, actionedUponNodeRef, executeAsync); + } + } + + private void executeAction(Action action, NodeRef actionedUponNodeRef, boolean executeAsynchronously) + { + this.actionService.executeAction(action, actionedUponNodeRef, true, executeAsynchronously); + } /** * Determines whether the rule can be executed diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java index d54893b5dc..4ce058100f 100644 --- a/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java +++ b/source/java/org/alfresco/repo/security/authority/AuthorityDAO.java @@ -83,7 +83,6 @@ public interface AuthorityDAO */ Set getContainingAuthorities(AuthorityType type, String name, boolean immediate); - /** * Get a set of authorities with varying filter criteria * diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java index dc3c53cbcf..87e426be9a 100644 --- a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java +++ b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java @@ -782,25 +782,38 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor // If this top down search is not providing an adequate hit count then resort to a naiive unlimited search if (processed >= maxToProcess) { + Set unfilteredResult; + boolean filterZone; if (authority == null) { - return new HashSet(getAuthorities(type, zoneName, null, false, true, new PagingRequest(0, maxResults, null)).getPage()); - } - Set newResult = getContainingAuthorities(type, authority, false); - result.clear(); - int i=0; - for (String container : newResult) - { - if ((filter == null || filter.includeAuthority(container) - && (zoneName == null || getAuthorityZones(container).contains(zoneName)))) + unfilteredResult = new HashSet(getAuthorities(type, zoneName, null, false, true, new PagingRequest(0, filter == null ? maxResults : Integer.MAX_VALUE, null)).getPage()); + if (filter == null) { - result.add(container); + return unfilteredResult; + } + filterZone = false; + } + else + { + unfilteredResult = getContainingAuthorities(type, authority, false); + filterZone = zoneName != null; + } + Set newResult = new TreeSet(result); + int i=newResult.size(); + for (String container : unfilteredResult) + { + // Do not call the filter multiple times on the same result in case it is 'stateful' + if (!result.contains(container) && (filter == null || filter.includeAuthority(container)) + && (!filterZone || getAuthorityZones(container).contains(zoneName))) + { + newResult.add(container); if (++i >= maxResults) { break; } } - } + } + result = newResult; break; } } diff --git a/source/java/org/alfresco/repo/security/authority/script/ScriptGroup.java b/source/java/org/alfresco/repo/security/authority/script/ScriptGroup.java index d43602dbd0..113c224afc 100644 --- a/source/java/org/alfresco/repo/security/authority/script/ScriptGroup.java +++ b/source/java/org/alfresco/repo/security/authority/script/ScriptGroup.java @@ -57,7 +57,6 @@ public class ScriptGroup implements Authority, Serializable private String fullName; private String displayName; private Set childAuthorityNames; - private Boolean isAdmin; private NodeRef groupNodeRef; private Scriptable scope; @@ -411,29 +410,6 @@ public class ScriptGroup implements Authority, Serializable Set parents = authorityService.getContainingAuthorities(AuthorityType.GROUP, fullName, false); return makeScriptGroups(parents, paging, sortBy, serviceRegistry, this.scope); } - - /** - * Is this a root group? - * @return - */ - public boolean isRootGroup() - { - ScriptGroup[] groups = getParentGroups(); - return (groups.length == 0); - } - - /** - * Is this an admin group? - * @return - */ - public boolean isAdminGroup() - { - if (this.isAdmin == null) - { - this.isAdmin = authorityService.isAdminAuthority(fullName); - } - return this.isAdmin; - } /** * Get the number of users contained within this group. diff --git a/source/java/org/alfresco/util/ModelUtil.java b/source/java/org/alfresco/util/ModelUtil.java index f22e27a22d..4fca922f41 100644 --- a/source/java/org/alfresco/util/ModelUtil.java +++ b/source/java/org/alfresco/util/ModelUtil.java @@ -130,6 +130,7 @@ public class ModelUtil { return page(objects, new ScriptPagingDetails(maxItems, skipCount)); } + public static List page(Collection objects, ScriptPagingDetails paging) { int maxItems = paging.getMaxItems();