ALF-1878 : Duplicate incoming email Subjects over-write each other

new configuration property email.handler.folder.overwriteDuplicates added, defaults to true so existing behaviour is maintained.

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@30952 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/subsystems/email/InboundSMTP/inboundSMTP-context.xml

@@ -125,7 +125,12 @@
 
 	<bean id="folderEmailMessageHandler"
 		parent="emailMessageHandlerBase"
-		class="org.alfresco.email.server.handler.FolderEmailMessageHandler" />
+		class="org.alfresco.email.server.handler.FolderEmailMessageHandler" >
+	
+		<property name="overwriteDuplicates">
+		    <value>${email.handler.folder.overwriteDuplicates}</value>            
+        </property>
+    </bean>
 
 	<bean id="forumEmailMessageHandler"
 		parent="emailMessageHandlerBase"

config/alfresco/subsystems/email/InboundSMTP/inboundSMTP.properties

@@ -19,3 +19,6 @@ email.server.hideTLS=false
 email.server.enableTLS=true
 # Set this to true to require TLS
 email.server.requireTLS=false
+
+# Should duplicate messages to a folder overwrite each other or be named with a (number)
+email.handler.folder.overwriteDuplicates=true

source/java/org/alfresco/email/server/EmailServiceImplTest.java

@@ -33,12 +33,14 @@ import javax.mail.internet.InternetAddress;
 
 import junit.framework.TestCase;
 
+import org.alfresco.email.server.handler.FolderEmailMessageHandler;
 import org.alfresco.email.server.impl.subetha.SubethaEmailMessage;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.management.subsystems.ChildApplicationContextFactory;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.service.cmr.email.EmailMessageException;
 import org.alfresco.service.cmr.email.EmailService;
+import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.StoreRef;
@@ -47,6 +49,7 @@ import org.alfresco.service.cmr.security.AuthorityService;
 import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.ApplicationContextHelper;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -75,6 +78,7 @@ public class EmailServiceImplTest extends TestCase
     private AuthorityService authorityService;
     private SearchService searchService;
     private NamespaceService namespaceService;
+    private FolderEmailMessageHandler folderEmailMessageHandler;
     
     String TEST_USER="EmailServiceImplTestUser";
     
@@ -93,10 +97,12 @@ public class EmailServiceImplTest extends TestCase
         assertNotNull("emailService", emailService);
         personService = (PersonService)emailCtx.getBean("PersonService");       
         assertNotNull("personService", personService);     
-        searchService = (SearchService)emailCtx.getBean("SearchService");       
-        assertNotNull("searchService", searchService);
         namespaceService = (NamespaceService)emailCtx.getBean("NamespaceService");       
         assertNotNull("namespaceService", namespaceService);
+        searchService = (SearchService)emailCtx.getBean("SearchService");  
+        assertNotNull("searchService", searchService);
+        folderEmailMessageHandler = (FolderEmailMessageHandler) emailCtx.getBean("folderEmailMessageHandler");  
+        assertNotNull("folderEmailMessageHandler", folderEmailMessageHandler);
     }
     
     public void tearDown() throws Exception
@@ -124,6 +130,8 @@ public class EmailServiceImplTest extends TestCase
    public void testFromName() throws Exception
    {
      
+       folderEmailMessageHandler.setOverwriteDuplicates(true);
+       
        logger.debug("Start testFromName");
        
        String TEST_EMAIL="buffy@sunnydale.high";
@@ -318,6 +326,8 @@ public class EmailServiceImplTest extends TestCase
         
         String TEST_EMAIL="buffy@sunnydale.high";
         
+        folderEmailMessageHandler.setOverwriteDuplicates(true);
+        
         // TODO Investigate why setting PROP_EMAIL on createPerson does not work.
         NodeRef person = personService.getPerson(TEST_USER);
         if(person == null)
@@ -379,6 +389,129 @@ public class EmailServiceImplTest extends TestCase
            
     }
     
+    /**
+     * ALF-1878
+     * 
+     * Duplicate incoming email Subjects over-write each other
+     */
+   public void testMultipleMessagesToFolder() throws Exception
+   {
+       logger.debug("Start testFromName");
        
+       String TEST_EMAIL="buffy@sunnydale.high";
+       
+       String TEST_SUBJECT="Practical Bee Keeping";
+       
+       String TEST_LONG_SUBJECT = "This is a very very long name in particular it is greater than eitghty six characters which was a problem explored in ALF-9544";
+       
+       
+       // TODO Investigate why setting PROP_EMAIL on createPerson does not work.
+       NodeRef person = personService.getPerson(TEST_USER);
+       if(person == null)
+       {
+           logger.debug("new person created");
+           Map<QName, Serializable> props = new HashMap<QName, Serializable>();
+           props.put(ContentModel.PROP_USERNAME, TEST_USER);
+           props.put(ContentModel.PROP_EMAIL, TEST_EMAIL);
+           person = personService.createPerson(props);
+       }
+       nodeService.setProperty(person, ContentModel.PROP_EMAIL, TEST_EMAIL);
+
+       Set<String> auths = authorityService.getContainedAuthorities(null, "GROUP_EMAIL_CONTRIBUTORS", true);
+       if(!auths.contains(TEST_USER))
+       {
+           authorityService.addAuthority("GROUP_EMAIL_CONTRIBUTORS", TEST_USER);
+       }
+       
+       String companyHomePathInStore = "/app:company_home"; 
+       String storePath = "workspace://SpacesStore";
+       StoreRef storeRef = new StoreRef(storePath);
+
+       NodeRef storeRootNodeRef = nodeService.getRootNode(storeRef);
+       List<NodeRef> nodeRefs = searchService.selectNodes(storeRootNodeRef, companyHomePathInStore, null, namespaceService, false);
+       NodeRef companyHomeNodeRef = nodeRefs.get(0);
+       assertNotNull("company home is null", companyHomeNodeRef);
+       String companyHomeDBID = ((Long)nodeService.getProperty(companyHomeNodeRef, ContentModel.PROP_NODE_DBID)).toString() + "@Alfresco.com";
+       String testUserDBID = ((Long)nodeService.getProperty(person, ContentModel.PROP_NODE_DBID)).toString() + "@Alfresco.com";
+       NodeRef testUserHomeFolder = (NodeRef)nodeService.getProperty(person, ContentModel.PROP_HOMEFOLDER);
+       assertNotNull("testUserHomeFolder is null", testUserHomeFolder);
+       String testUserHomeDBID = ((Long)nodeService.getProperty(testUserHomeFolder, ContentModel.PROP_NODE_DBID)).toString() + "@Alfresco.com";
+       
+       // Clean up old messages in test folder
+       List<ChildAssociationRef> assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       for(ChildAssociationRef assoc : assocs)
+       {
+           nodeService.deleteNode(assoc.getChildRef());
+       }
+               
+       /**
+        * Send From the test user TEST_EMAIL to the test user's home
+        */
+       String from = 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(TEST_EMAIL));
+       msg.setRecipients(Message.RecipientType.TO, toa);
+       msg.setSubject(TEST_SUBJECT);
+       msg.setContent(content, "text/plain");
+           
+       ByteArrayOutputStream bos = new ByteArrayOutputStream();
+       msg.writeTo(bos);
+       InputStream is = new StringInputStream(bos.toString());
+       assertNotNull("is is null", is);
+   
+       SubethaEmailMessage m = new SubethaEmailMessage(is);   
+       
+       /**
+        * Turn on overwriteDuplicates
+        */
+       logger.debug("Step 1: turn on Overwite Duplicates");
+       folderEmailMessageHandler.setOverwriteDuplicates(true);
+
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 1", 1, assocs.size());
+       assertEquals("name of link not as expected", assocs.get(0).getQName(), QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, TEST_SUBJECT));
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 1", 1, assocs.size());   
+       
+       /**
+        * Turn off overwrite Duplicates
+        */
+       logger.debug("Step 2: turn off Overwite Duplicates");
+       folderEmailMessageHandler.setOverwriteDuplicates(false);
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 2", 2, assocs.size());
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 3", 3, assocs.size());   
+
+       /**
+        * Check assoc rename with long names.   So truncation and rename need to work together.
+        */
+       logger.debug("Step 3: turn off Overwite Duplicates with long subject name");
+       msg.setSubject(TEST_LONG_SUBJECT);
+       ByteArrayOutputStream bos2 = new ByteArrayOutputStream();
+       msg.writeTo(bos2);
+       is = new StringInputStream(bos2.toString());
+       assertNotNull("is is null", is);
+       m = new SubethaEmailMessage(is);   
+       
+       folderEmailMessageHandler.setOverwriteDuplicates(false);
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 4", 4, assocs.size());
+       emailService.importMessage(m);
+       assocs = nodeService.getChildAssocs(testUserHomeFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+       assertEquals("assocs not 5", 5, assocs.size());   
+   }
  
 }

source/java/org/alfresco/email/server/handler/AbstractEmailMessageHandler.java

@@ -24,6 +24,7 @@ import java.io.InputStream;
 import java.io.Serializable;
 import java.io.UnsupportedEncodingException;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.alfresco.error.AlfrescoRuntimeException;
@@ -125,43 +126,43 @@ public abstract class AbstractEmailMessageHandler implements EmailMessageHandler
         this.mimetypeService = mimetypeService;
     }
 
-    /**
+//    /**
-     * @param to Email address which user part specifies node-dbid
+//     * @param to Email address which user part specifies node-dbid
-     * @return Referance to requested node.
+//     * @return Referance to requested node.
-     * @throws InvalidArgumentException The exception is thrown if input string has incorrect format or empty.
+//     * @throws InvalidArgumentException The exception is thrown if input string has incorrect format or empty.
-     */
+//     */
-    protected NodeRef getTargetNode(String to) throws InvalidArgumentException
+//    protected NodeRef getTargetNode(String to) throws InvalidArgumentException
-    {
+//    {
-        if (to == null || to.length() == 0)
+//        if (to == null || to.length() == 0)
-        {
+//        {
-            throw new InvalidArgumentException("Input string has to contain email address.");
+//            throw new InvalidArgumentException("Input string has to contain email address.");
-        }
+//        }
-        String[] parts = to.split("@");
+//        String[] parts = to.split("@");
-        if (parts.length != 2)
+//        if (parts.length != 2)
-        {
+//        {
-            throw new InvalidArgumentException("Incorrect email address format.");
+//            throw new InvalidArgumentException("Incorrect email address format.");
-        }
+//        }
-        try
+//        try
-        {
+//        {
-            Long dbId = Long.parseLong(parts[0]);
+//            Long dbId = Long.parseLong(parts[0]);
-            return nodeService.getNodeRef(dbId);
+//            return nodeService.getNodeRef(dbId);
-        }
+//        }
-        catch (NumberFormatException e)
+//        catch (NumberFormatException e)
-        {
+//        {
-            return null;
+//            return null;
-        }
+//        }
-    }
+//    }
 
-    /**
+//    /**
-     * Write the content to the node 
+//     * Write the content to the node as MIMETYPE TEXT PLAIN.
-     * 
+//     * 
-     * @param nodeRef Target node
+//     * @param nodeRef Target node
-     * @param content Content
+//     * @param content Content
-     */
+//     */
-    protected void writeContent(NodeRef nodeRef, String content)
+//    protected void writeContent(NodeRef nodeRef, String content)
-    {
+//    {
-        writeContent(nodeRef, content, MimetypeMap.MIMETYPE_TEXT_PLAIN);
+//        writeContent(nodeRef, content, MimetypeMap.MIMETYPE_TEXT_PLAIN);
-    }
+//    }
 
     /**
      * Write the string as content to the node.
@@ -189,7 +190,7 @@ public abstract class AbstractEmailMessageHandler implements EmailMessageHandler
      * @param nodeRef Target node.
      * @param content Content stream.
      * @param mimetype MIME content type.
-     * @param encoding Encoding. Can be null for non text based content.
+     * @param encoding Encoding. Can be null for text based content, n which case the best guess.
      */
     protected void writeContent(NodeRef nodeRef, InputStream content, String mimetype, String encoding)
     {
@@ -266,47 +267,82 @@ public abstract class AbstractEmailMessageHandler implements EmailMessageHandler
      * @param nodeService Alfresco Node Service
      * @param parent Parent node
      * @param name Name of the new node
+     * @param overwrite if true then overwrite an existing node with the same name.   if false the name is changed to make it unique.
      * @param assocType Association type that should be set between parent node and the new one.
      * @return Reference to created node
      */
-    protected NodeRef addContentNode(NodeService nodeService, NodeRef parent, String name, QName assocType)
+    protected NodeRef addContentNode(NodeService nodeService, NodeRef parent, String name, QName assocType, boolean overwrite)
     {
-        NodeRef childNodeRef = nodeService.getChildByName(parent, assocType, name);
+        NodeRef childNodeRef = null;
-        if (childNodeRef != null)
-        {
-            // The node is present already.  Make sure the name case is correct
-            nodeService.setProperty(childNodeRef, ContentModel.PROP_NAME, name);
-        }
-        else
-        {
-            Map<QName, Serializable> contentProps = new HashMap<QName, Serializable>();
-            contentProps.put(ContentModel.PROP_NAME, name);
         
-            QName assocName =  QName.createQNameWithValidLocalName(NamespaceService.CONTENT_MODEL_1_0_URI, name); 
+        String workingName = name;
         
-            ChildAssociationRef associationRef = nodeService.createNode(
+        for(int counter = 0; counter < 10000; counter++)
+        {
+            QName safeQName = QName.createQNameWithValidLocalName(NamespaceService.CONTENT_MODEL_1_0_URI, workingName);
+
+            List<ChildAssociationRef> childNodeRefs = nodeService.getChildAssocs(parent, ContentModel.ASSOC_CONTAINS, safeQName);
+        
+            if (childNodeRefs.size() > 0)
+            {
+                if(overwrite)
+                {
+                    childNodeRef=childNodeRefs.get(0).getChildRef();
+                
+                    // Node already exists
+                    // The node is present already.  Make sure the name case is correct
+                    nodeService.setProperty(childNodeRef, ContentModel.PROP_NAME, name);
+                    return childNodeRef;
+                }
+            
+                // Need to work out a new safe name.
+
+                String postFix = " (" + counter + ")";
+            
+                if(name.length() + postFix.length() > QName.MAX_LENGTH )
+                {
+                    workingName =  name.substring(0, QName.MAX_LENGTH-postFix.length()) + postFix;
+                    // Need to truncate name     
+                }
+                else
+                {
+                    workingName = name + postFix;
+                }
+            }
+            else
+            {
+                // Here if child node ref does not already exist
+                Map<QName, Serializable> contentProps = new HashMap<QName, Serializable>();
+                contentProps.put(ContentModel.PROP_NAME, workingName);
+                    
+                ChildAssociationRef associationRef = nodeService.createNode(
                     parent,
                     assocType,
-                    assocName,
+                    safeQName,
                     ContentModel.TYPE_CONTENT,
                     contentProps);
-            childNodeRef = associationRef.getChildRef();
+                childNodeRef = associationRef.getChildRef();
+                    
+                return childNodeRef;
+            }
         }
-        return childNodeRef;
+        throw new AlfrescoRuntimeException("Unable to add new file");
     }
 
     /**
      * Add new node into Alfresco repository with specified parameters.
-     * Node content isn't added. New node will be created with ContentModel.ASSOC_CONTAINS association with parent.
+     * Node content isn't added. 
+     * 
+     * New node will be created with ContentModel.ASSOC_CONTAINS association with parent.
      * 
      * @param nodeService Alfresco Node Service
      * @param parent Parent node
      * @param name Name of the new node
      * @return Reference to created node
      */
-    protected NodeRef addContentNode(NodeService nodeService, NodeRef parent, String name)
+    protected NodeRef addContentNode(NodeService nodeService, NodeRef parent, String name, boolean overwrite)
     {
-        return addContentNode(nodeService, parent, name, ContentModel.ASSOC_CONTAINS);
+        return addContentNode(nodeService, parent, name, ContentModel.ASSOC_CONTAINS, overwrite);
     }
 
     /**
@@ -320,14 +356,13 @@ public abstract class AbstractEmailMessageHandler implements EmailMessageHandler
      */
     protected NodeRef addAttachment(NodeService nodeService, NodeRef folder, NodeRef mainContentNode, String fileName)
     {
-        fileName = getAppropriateNodeName(folder, fileName, ContentModel.ASSOC_CONTAINS);
         
         if (log.isDebugEnabled())
         {
             log.debug("Adding attachment node (name=" + fileName + ").");
         }
         
-        NodeRef attachmentNode = addContentNode(nodeService, folder, fileName);
+        NodeRef attachmentNode = addContentNode(nodeService, folder, fileName, false);
         
         // Add attached aspect
         nodeService.addAspect(mainContentNode, ContentModel.ASPECT_ATTACHABLE, null);
@@ -354,6 +389,7 @@ public abstract class AbstractEmailMessageHandler implements EmailMessageHandler
         if (nodeService.getChildByName(parent, assocType, name) != null)
         {
             name = name + "(1)";
+            
             while (nodeService.getChildByName(parent, assocType, name) != null)
             {
 

source/java/org/alfresco/email/server/handler/AbstractForumEmailMessageHandler.java

@@ -29,6 +29,7 @@ import java.util.Map;
 import org.alfresco.model.ApplicationModel;
 import org.alfresco.model.ContentModel;
 import org.alfresco.model.ForumModel;
+import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.service.cmr.email.EmailMessage;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -92,7 +93,7 @@ public abstract class AbstractForumEmailMessageHandler extends AbstractEmailMess
         }
         else
         {
-            writeContent(postNodeRef, "<The message was empty>");
+            writeContent(postNodeRef, "<The message was empty>", MimetypeMap.MIMETYPE_TEXT_PLAIN);
         }
         addEmailedAspect(postNodeRef, message);
         

source/java/org/alfresco/email/server/handler/FolderEmailMessageHandler.java

@@ -56,6 +56,8 @@ public class FolderEmailMessageHandler extends AbstractEmailMessageHandler
     
     private static final Log log = LogFactory.getLog(FolderEmailMessageHandler.class);
     
+    private boolean overwriteDuplicates = false;
+
     /**
      * {@inheritDoc}
      */
@@ -108,7 +110,7 @@ public class FolderEmailMessageHandler extends AbstractEmailMessageHandler
 
         // Create main content node
         NodeRef contentNodeRef;
-        contentNodeRef = addContentNode(getNodeService(), spaceNodeRef, messageSubject);
+        contentNodeRef = addContentNode(getNodeService(), spaceNodeRef, messageSubject, overwriteDuplicates);
         // Add titled aspect
         addTitledAspect(contentNodeRef, messageSubject, message.getFrom());
         // Add emailed aspect
@@ -182,4 +184,18 @@ public class FolderEmailMessageHandler extends AbstractEmailMessageHandler
             log.debug("Titled aspect has been added.");
         }
     }
+
+    /**
+     * Set the behaviour to be done on detecting a new message with the same subject.
+     * @param overwriteDuplicates
+     */
+    public void setOverwriteDuplicates(boolean overwriteDuplicates)
+    {
+        this.overwriteDuplicates = overwriteDuplicates;
+    }
+
+    public boolean isOverwriteDuplicates()
+    {
+        return overwriteDuplicates;
+    }
 }