Fix for ALF-1937. title property text gets cut off after fullstop character found and filename extension is appended during document TransformAction

Also tidied up the handling of names and titles during transform to avoid other potential bugs. Added a Unit Test class to cover the obvious cases.


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

source/java/org/alfresco/repo/action/executer/TransformActionExecuter.java

@@ -246,10 +246,10 @@ public class TransformActionExecuter extends ActionExecuterAbstractBase
             // Adjust the name of the copy
             nodeService.setProperty(copyNodeRef, ContentModel.PROP_NAME, newName);
             String originalTitle = (String)nodeService.getProperty(actionedUponNodeRef, ContentModel.PROP_TITLE);
-            if (originalTitle != null && originalTitle.length() > 0)
+            
+            if (originalTitle != null)
             {
-                String newTitle = transformName(this.mimetypeService, originalTitle, mimeType, false);
-                nodeService.setProperty(copyNodeRef, ContentModel.PROP_TITLE, newTitle);
+                nodeService.setProperty(copyNodeRef, ContentModel.PROP_TITLE, originalTitle);
             }
         }
         
@@ -306,10 +306,18 @@ public class TransformActionExecuter extends ActionExecuterAbstractBase
     }
     
     /**
-     * Transform name from original extension to new extension
+     * Transform a name from original extension to new extension, if appropriate.
+     * If the original name seems to end with a reasonable file extension, then the name
+     * will be transformed such that the old extension is replaced with the new.
+     * Otherwise the name will be returned unaltered.
+     * <P/>
+     * The original name will be deemed to have a reasonable extension if there are one
+     * or more characters after the (required) final dot, none of which are spaces.
      * 
-     * @param original
-     * @param newMimetype
+     * @param mimetypeService the mimetype service
+     * @param original the original name
+     * @param newMimetype the new mime type
+     * @param alwaysAdd if the name has no extension, then add the new one
      * 
      * @return name with new extension as appropriate for the mimetype
      */
@@ -321,20 +329,40 @@ public class TransformActionExecuter extends ActionExecuterAbstractBase
         if (dotIndex > -1)
         {
             // we found it
-            sb.append(original.substring(0, dotIndex));
+            String nameBeforeDot = original.substring(0, dotIndex);
+            String originalExtension = original.substring(dotIndex + 1, original.length());
             
-            // add the new extension
+            // See ALF-1937, which actually relates to cm:title not cm:name.
+            // It is possible that the text after the '.' is not actually an extension
+            // in which case it should not be replaced.
+            boolean originalExtensionIsReasonable = isExtensionReasonable(originalExtension);
             String newExtension = mimetypeService.getExtension(newMimetype);
+            if (originalExtensionIsReasonable)
+            {
+                sb.append(nameBeforeDot);
                 sb.append('.').append(newExtension);
             }
             else
             {
-            // no extension so dont add a new one
+                sb.append(original);
+                if (alwaysAdd == true)
+                {
+                    if (sb.charAt(sb.length() - 1) != '.')
+                    {
+                        sb.append('.');
+                    }
+                    sb.append(newExtension);
+                }
+            }
+        }
+        else
+        {
+            // no extension so don't add a new one
             sb.append(original);
             
             if (alwaysAdd == true)
             {               
-                // add the new extension
+                // add the new extension - defaults to .bin
                 String newExtension = mimetypeService.getExtension(newMimetype);
                 sb.append('.').append(newExtension);
             }
@@ -343,4 +371,20 @@ public class TransformActionExecuter extends ActionExecuterAbstractBase
         return sb.toString();
     }
 
+    /**
+     * Given a String, this method tries to determine whether it is a reasonable filename extension.
+     * There are a number of checks that could be performed here, including checking whether the characters
+     * in the string are all 'letters'. However these are complicated by unicode and other
+     * considerations. Therefore this method adopts a simple approach and returns true if the
+     * string is of non-zero length and contains no spaces.
+     * 
+     * @param potentialExtensionString the string which may be a file extension.
+     * @return <code>true</code> if it is deemed reasonable, else <code>false</code>
+     * @since 3.3
+     */
+    private static boolean isExtensionReasonable(String potentialExtensionString)
+    {
+        return potentialExtensionString.length() > 0 && potentialExtensionString.indexOf(' ') == -1;
+    }
+    
 }

source/java/org/alfresco/repo/action/executer/TransformActionExecuterTest.java

@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2005-2010 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.action.executer;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.List;
+import java.util.Map;
+
+import org.alfresco.repo.content.MimetypeMap;
+import org.alfresco.repo.content.encoding.ContentCharsetFinder;
+import org.alfresco.service.cmr.repository.MimetypeService;
+import org.junit.Test;
+
+/**
+ * This class contains unit tests for {@link TransformActionExecuter}.
+ * 
+ * @author Neil McErlean
+ */
+public class TransformActionExecuterTest
+{
+    @Test
+    public void transformName() throws Exception
+    {
+        MimetypeService dummyMimetypeService = new DummyMimetypeService("txt");
+        
+        // Case 1. A simple name with a reasonable extension.
+        String original = "Letter to Bank Manager.doc";
+        final String newMimetype = MimetypeMap.MIMETYPE_TEXT_PLAIN;
+        String newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, true);
+        assertEquals("Letter to Bank Manager.txt", newName);
+
+        // Case 2. String after '.' is clearly not an extension
+        original = "No.1 - First Document Title";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, true);
+        assertEquals(original + ".txt", newName);
+
+        // Case 2b. String after '.' is clearly not an extension. Don't always add
+        original = "No.1 - First Document Title";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, false);
+        assertEquals(original, newName);
+
+        // Case 3. A name with no extension
+        original = "Letter to Bank Manager";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, true);
+        assertEquals(original + ".txt", newName);
+        
+        // Case 3b. A name with no extension. Don't always add
+        original = "Letter to Bank Manager";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, false);
+        assertEquals(original, newName);
+        
+        // Case 4. A name ending in a dot
+        original = "Letter to Bank Manager.";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, true);
+        assertEquals(original + "txt", newName);
+        
+        // Case 4b. A name ending in a dot. Don't always add
+        original = "Letter to Bank Manager.";
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, newMimetype, false);
+        assertEquals(original, newName);
+        
+        // Case 5. Unknown mime type
+        original = "Letter to Bank Manager.txt";
+        final String unknownMimetype = "Marcel/Marceau";
+        // The real MimetypeService returns a 'bin' extension for unknown mime types.
+        dummyMimetypeService = new DummyMimetypeService("bin");
+        
+        newName = TransformActionExecuter.transformName(dummyMimetypeService, original, unknownMimetype, true);
+        assertEquals("Letter to Bank Manager.bin", newName);
+    }
+}
+
+/**
+ * This is a very simple stub for the MimetypeService.
+ * For this test, we're only interested in the values that come back from
+ * the getExtension call.
+ */
+class DummyMimetypeService implements MimetypeService
+{
+    private final String result;
+    public DummyMimetypeService(String result) { this.result = result; }
+    public ContentCharsetFinder getContentCharsetFinder() { return null; }
+    public Map<String, String> getDisplaysByExtension()   { return null; }
+    public Map<String, String> getDisplaysByMimetype()    { return null; }
+    public String getExtension(String mimetype)           { return result; }
+    public Map<String, String> getExtensionsByMimetype()  { return null; }
+    public String getMimetype(String extension)           { return null; }
+    public List<String> getMimetypes()                    { return null; }
+    public Map<String, String> getMimetypesByExtension()  { return null; }
+    public String guessMimetype(String filename)          { return null; }
+    public boolean isText(String mimetype)                { return false;}
+}