Merged FILE-FOLDER-API (5.2.0) to HEAD (5.2)

121563 jkaabimofrad: RA-640: Fixed encoding issue when updating binary content, as well as, multipart uploading.


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

source/java/org/alfresco/rest/api/impl/NodesImpl.java

@@ -105,6 +105,8 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.extensions.surf.util.Content;
 import org.springframework.extensions.webscripts.servlet.FormData;
+import org.springframework.http.InvalidMediaTypeException;
+import org.springframework.http.MediaType;
 
 /**
  * Centralises access to file/folder/node services and maps between representations.
@@ -712,7 +714,7 @@ public class NodesImpl implements Nodes
     
     protected Set<QName> mapToNodeAspects(List<String> aspectNames)
     {
-    	Set<QName> nodeAspects = new HashSet<>(aspectNames.size());
+        Set<QName> nodeAspects = new HashSet<>(aspectNames.size());
 
         for (String aspectName : aspectNames)
         {
@@ -725,7 +727,7 @@ public class NodesImpl implements Nodes
             }
             else 
             {
-            	throw new InvalidArgumentException("Unknown aspect: "+aspectName);
+                throw new InvalidArgumentException("Unknown aspect: " + aspectName);
             }
         }
 
@@ -738,7 +740,7 @@ public class NodesImpl implements Nodes
 
         for (Entry<String, Object> entry : props.entrySet())
         {
-        	String propName = entry.getKey();
+            String propName = entry.getKey();
             QName propQName = createQName(propName);
 
             PropertyDefinition pd = dictionaryService.getProperty(propQName);
@@ -765,7 +767,7 @@ public class NodesImpl implements Nodes
             }
             else 
             {
-            	throw new InvalidArgumentException("Unknown property: "+propName);
+                throw new InvalidArgumentException("Unknown property: " + propName);
             }
         }
 
@@ -986,7 +988,7 @@ public class NodesImpl implements Nodes
         if (aspectNames != null)
         {
             // node aspects - set any additional aspects
-        	Set<QName> aspectQNames = mapToNodeAspects(aspectNames);
+            Set<QName> aspectQNames = mapToNodeAspects(aspectNames);
             for (QName aspectQName : aspectQNames)
             {
                 if (EXCLUDED_ASPECTS.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
@@ -1002,17 +1004,7 @@ public class NodesImpl implements Nodes
         {
             // add empty file
             ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true);
-            String mimeType;
+            setWriterContentType(writer, new ContentInfoWrapper(nodeInfo.getContent()), nodeRef, false);
-            ContentInfo contentInfo = nodeInfo.getContent();
-            if (contentInfo != null && contentInfo.getMimeType() != null)
-            {
-                mimeType = contentInfo.getMimeType();
-            }
-            else
-            {
-                mimeType = mimetypeService.guessMimetype(nodeName);
-            }
-            writer.setMimetype(mimeType);
             writer.putContent("");
         }
 
@@ -1102,7 +1094,7 @@ public class NodesImpl implements Nodes
         if (aspectNames != null)
         {
             // update aspects - note: can be empty (eg. to remove existing aspects+properties) but not cm:auditable, sys:referencable, sys:localized
-        	
+
             Set<QName> aspectQNames = mapToNodeAspects(aspectNames);
 
             Set<QName> existingAspects = nodeService.getAspects(nodeRef);
@@ -1295,18 +1287,7 @@ public class NodesImpl implements Nodes
         }
 
         ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true);
-
+        setWriterContentType(writer, new ContentInfoWrapper(contentInfo), nodeRef, true);
-        String mimeType = contentInfo.getMimeType();
-        if (mimeType == null)
-        {
-            String fileName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_CONTENT);
-            writer.guessMimetype(fileName);
-        }
-        else
-        {
-            writer.setMimetype(mimeType);
-        }
-        writer.guessEncoding();
         writer.putContent(stream);
 
         return getFolderOrDocumentFullInfo(nodeRef,
@@ -1315,6 +1296,33 @@ public class NodesImpl implements Nodes
                     parameters);
     }
 
+    private void setWriterContentType(ContentWriter writer, ContentInfoWrapper contentInfo, NodeRef nodeRef, boolean guessEncodingIfNull)
+    {
+        String mimeType = contentInfo.mimeType;
+        // Manage MimeType
+        if (mimeType == null)
+        {
+            // the mimeType was not provided via the contentInfo, so try to guess
+            final String fileName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
+            mimeType = mimetypeService.guessMimetype(fileName);
+        }
+        writer.setMimetype(mimeType);
+
+        // Manage Encoding
+        if (contentInfo.encoding == null)
+        {
+            if (guessEncodingIfNull)
+            {
+                // the encoding was not provided, so try to guess
+                writer.guessEncoding();
+            }
+        }
+        else
+        {
+            writer.setEncoding(contentInfo.encoding);
+        }
+    }
+
     @Override
     public Node upload(String parentFolderNodeId, FormData formData, Parameters parameters)
     {
@@ -1416,7 +1424,7 @@ public class NodesImpl implements Nodes
                 // else if (overwrite && nodeService.hasAspect(existingFile, ContentModel.ASPECT_VERSIONABLE))
                 // {
                 //     // Upload component was configured to overwrite files if name clashes
-                //     write(existingFile, content, fileName, false, true);
+                //     write(existingFile, content);
                 //
                 //     // Extract the metadata (The overwrite policy controls
                 //     // which if any parts of the document's properties are updated from this)
@@ -1480,7 +1488,7 @@ public class NodesImpl implements Nodes
         NodeRef newFile = createNodeImpl(parentNodeRef, fileName, nodeType, props);
 
         // Write content
-        write(newFile, content, fileName, true, true);
+        write(newFile, content);
 
         // Ensure the file is versionable (autoVersion = true, autoVersionProps = false)
         ensureVersioningEnabled(newFile, true, false);
@@ -1506,40 +1514,13 @@ public class NodesImpl implements Nodes
      *
      * @param nodeRef       the reference to the node having a content property
      * @param content       the content
-     * @param fileName      the uploaded file name
-     * @param applyMimeType If true, apply the mimeType from the Content object,
-     *                      else leave the original mimeType (if the original type is null, then guess the mimeType)
-     * @param guessEncoding If true, guess the encoding from the underlying
-     *                      input stream, else use encoding set in the Content object as supplied
      */
-    protected void write(NodeRef nodeRef, Content content, String fileName, boolean applyMimeType, boolean guessEncoding)
+    protected void write(NodeRef nodeRef, Content content)
     {
         ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true);
-        String mimeType = content.getMimetype();
         // Per RA-637 requirement the mimeType provided by the client takes precedence, however,
-        // if the mimeType is null, then it will be retrieved or guessed.
+        // if the mimeType is null, then it will be guessed.
-        if (mimeType == null || !applyMimeType)
+        setWriterContentType(writer, new ContentInfoWrapper(content), nodeRef, true);
-        {
-            ContentData existingContentData = (ContentData) nodeService.getProperty(nodeRef, ContentModel.PROP_CONTENT);
-            if (existingContentData != null)
-            {
-                mimeType = existingContentData.getMimetype();
-            }
-            else
-            {
-                mimeType = mimetypeService.guessMimetype(fileName);
-            }
-        }
-        writer.setMimetype(mimeType);
-
-        if (guessEncoding)
-        {
-            writer.guessEncoding();
-        }
-        else
-        {
-            writer.setEncoding(content.getEncoding());
-        }
         writer.putContent(content.getInputStream());
     }
 
@@ -1675,4 +1656,52 @@ public class NodesImpl implements Nodes
         return result;
     }
 
+    /**
+     * @author Jamal Kaabi-Mofrad
+     */
+    private static class ContentInfoWrapper
+    {
+        private String mimeType;
+        private String encoding;
+
+        ContentInfoWrapper(BasicContentInfo basicContentInfo)
+        {
+            if (basicContentInfo != null)
+            {
+                this.mimeType = basicContentInfo.getMimeType();
+                this.encoding = basicContentInfo.getEncoding();
+            }
+        }
+
+        ContentInfoWrapper(ContentInfo contentInfo)
+        {
+            if (contentInfo != null)
+            {
+                this.mimeType = contentInfo.getMimeType();
+                this.encoding = contentInfo.getEncoding();
+            }
+        }
+
+        ContentInfoWrapper(Content content)
+        {
+            if (content != null && StringUtils.isNotEmpty(content.getMimetype()))
+            {
+                try
+                {
+                    // TODO I think it makes sense to push contentType parsing into org.springframework.extensions.webscripts.servlet.FormData
+                    MediaType media = MediaType.parseMediaType(content.getMimetype());
+                    this.mimeType = media.getType() + '/' + media.getSubtype();
+
+                    if (media.getCharSet() != null)
+                    {
+                        this.encoding = media.getCharSet().name();
+                    }
+                }
+                catch (InvalidMediaTypeException ime)
+                {
+                    throw new InvalidArgumentException(ime.getMessage());
+                }
+            }
+        }
+    }
 }

source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java

@@ -788,6 +788,8 @@ public class NodeApiTest extends AbstractBaseApiTest
         ContentInfo contentInfo = document.getContent();
         assertNotNull(contentInfo);
         assertEquals(MimetypeMap.MIMETYPE_PDF, contentInfo.getMimeType());
+        // Default encoding
+        assertEquals("UTF-8", contentInfo.getEncoding());
         // Check there is no path info returned.
         // The path info should only be returned when it is requested via a select statement.
         assertNull(document.getPath());
@@ -866,6 +868,26 @@ public class NodeApiTest extends AbstractBaseApiTest
                     .build();
         post(getChildrenUrl(user1Home.getId()), user1, reqBody.getBody(), null, reqBody.getContentType(), 400);
 
+        // User1 uploads a new file
+        reqBody = MultiPartBuilder.create()
+                    .setFileData(new FileData(fileName2, file2, MimetypeMap.MIMETYPE_TEXT_PLAIN, "windows-1252"))
+                    .build();
+        response = post(getChildrenUrl(user1Home.getId()), user1, reqBody.getBody(), null, reqBody.getContentType(), 201);
+        document = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class);
+        // Check the upload response
+        assertEquals(fileName2, document.getName());
+        contentInfo = document.getContent();
+        assertNotNull(contentInfo);
+        assertEquals(MimetypeMap.MIMETYPE_TEXT_PLAIN, contentInfo.getMimeType());
+        assertEquals("windows-1252", contentInfo.getEncoding());
+
+        // Test invalid mimeType
+        reqBody = MultiPartBuilder.create()
+                    .setFileData(new FileData(fileName2, file2, "*/invalidSubType", "ISO-8859-1"))
+                    .setAutoRename(true)
+                    .build();
+        post(getChildrenUrl(user1Home.getId()), user1, reqBody.getBody(), null, reqBody.getContentType(), 400);
+
     }
 
     /**
@@ -1548,8 +1570,8 @@ public class NodeApiTest extends AbstractBaseApiTest
         // Update the empty node's content
         String content = "The quick brown fox jumps over the lazy dog.";
         ByteArrayInputStream inputStream = new ByteArrayInputStream(content.getBytes());
-        File file = TempFileProvider.createTempFile(inputStream, getClass().getSimpleName(), ".txt");
+        File txtFile = TempFileProvider.createTempFile(inputStream, getClass().getSimpleName(), ".txt");
-        BinaryPayload payload = new BinaryPayload(file, MimetypeMap.MIMETYPE_TEXT_PLAIN);
+        BinaryPayload payload = new BinaryPayload(txtFile, MimetypeMap.MIMETYPE_TEXT_PLAIN);
 
         // Try to update a folder!
         putBinary(URL_NODES + f1_nodeId + "/content", user1, payload, null, null, 400);
@@ -1576,6 +1598,8 @@ public class NodeApiTest extends AbstractBaseApiTest
         contentInfo = docResp.getContent();
         assertNotNull(contentInfo);
         assertNotNull(contentInfo.getEncoding());
+        // Default encoding
+        assertEquals("UTF-8", contentInfo.getEncoding());
         assertTrue(contentInfo.getSizeInBytes() > 0);
         assertEquals(MimetypeMap.MIMETYPE_TEXT_PLAIN, contentInfo.getMimeType());
         assertNotNull(contentInfo.getMimeTypeName());
@@ -1587,13 +1611,14 @@ public class NodeApiTest extends AbstractBaseApiTest
         assertEquals(content, response.getResponse());
 
         // Update the node's content again. Also, change the mimeType and make the response to return path!
-        file = getResourceFile("quick.pdf");
+        File pdfFile = getResourceFile("quick.pdf");
-        payload = new BinaryPayload(file, MimetypeMap.MIMETYPE_PDF);
+        payload = new BinaryPayload(pdfFile, MimetypeMap.MIMETYPE_PDF, "ISO-8859-1");
 
         response = putBinary(url + "?select=path", user1, payload, null, null, 200);
         docResp = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class);
         assertEquals(docName, docResp.getName());
         assertNotNull(docResp.getContent());
+        assertNotNull("ISO-8859-1", docResp.getContent());
         assertTrue(docResp.getContent().getSizeInBytes().intValue() > 0);
         assertEquals(MimetypeMap.MIMETYPE_PDF, docResp.getContent().getMimeType());
         PathInfo pathInfo = docResp.getPath();
@@ -1605,6 +1630,16 @@ public class NodeApiTest extends AbstractBaseApiTest
         // check the last element is F1
         assertEquals(f1.getName(), pathElements.get(pathElements.size() - 1).getName());
 
+        // update the original content with different encoding
+        payload = new BinaryPayload(txtFile, MimetypeMap.MIMETYPE_TEXT_PLAIN, "ISO-8859-15");
+        response = putBinary(url, user1, payload, null, null, 200);
+        docResp = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class);
+        assertEquals(docName, docResp.getName());
+        assertNotNull(docResp.getContent());
+        assertNotNull("ISO-8859-15", docResp.getContent());
+        assertTrue(docResp.getContent().getSizeInBytes().intValue() > 0);
+        assertEquals(MimetypeMap.MIMETYPE_TEXT_PLAIN, docResp.getContent().getMimeType());
+
         // Download the file
         response = getSingle(url, user1, null, 200);
         assertNotNull(content, response.getResponse());

source/test-java/org/alfresco/rest/api/tests/client/PublicApiHttpClient.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2014 Alfresco Software Limited.
+ * Copyright (C) 2005-2016 Alfresco Software Limited.
  *
  * This file is part of Alfresco
  *
@@ -880,8 +880,8 @@ public class PublicApiHttpClient
         public BinaryRequestEntity(File file, String mimeType, String charset)
         {
             this.file = file;
-            this.mimeType = (mimeType == null) ? "application/octet-stream" : mimeType;
+            this.mimeType = mimeType;
-            this.charset = (charset == null) ? "UTF-8" : charset;
+            this.charset = charset;
         }
 
         @Override
@@ -918,7 +918,15 @@ public class PublicApiHttpClient
         @Override
         public String getContentType()
         {
-            return mimeType + "; " + charset;
+            if (charset == null)
+            {
+                return mimeType;
+            }
+            if (mimeType == null)
+            {
+                return null;
+            }
+            return mimeType + "; charset=" + charset;
         }
     }
 

source/test-java/org/alfresco/rest/api/tests/util/MultiPartBuilder.java

@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2005-2015 Alfresco Software Limited.
+ *  Copyright (C) 2005-2016 Alfresco Software Limited.
  *
  *  This file is part of Alfresco
  *
@@ -197,12 +197,19 @@ public class MultiPartBuilder
         private final String fileName;
         private final File file;
         private final String mimetype;
+        private final String encoding;
 
         public FileData(String fileName, File file, String mimetype)
+        {
+            this(fileName, file, mimetype, null);
+        }
+
+        public FileData(String fileName, File file, String mimetype, String encoding)
         {
             this.fileName = fileName;
             this.file = file;
             this.mimetype = mimetype;
+            this.encoding = encoding;
         }
 
         public String getFileName()
@@ -219,6 +226,11 @@ public class MultiPartBuilder
         {
             return mimetype;
         }
+
+        public String getEncoding()
+        {
+            return encoding;
+        }
     }
 
     public static class MultiPartRequest
@@ -258,7 +270,7 @@ public class MultiPartBuilder
         {
             FilePart fp = new FilePart("filedata", fileData.getFileName(), fileData.getFile(), fileData.getMimetype(), null);
             // Get rid of the default values added upon FilePart instantiation
-            fp.setCharSet(null);
+            fp.setCharSet(fileData.getEncoding());
             fp.setContentType(fileData.getMimetype());
             parts.add(fp);
             addPartIfNotNull(parts, "filename", fileData.getFileName());