From ca1de05fe7247bc8f384a112cc7376c02b7778e9 Mon Sep 17 00:00:00 2001 From: Jamal Kaabi-Mofrad Date: Tue, 10 May 2016 10:49:57 +0000 Subject: [PATCH] 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 --- .../org/alfresco/rest/api/impl/NodesImpl.java | 151 +++++++++++------- .../alfresco/rest/api/tests/NodeApiTest.java | 43 ++++- .../api/tests/client/PublicApiHttpClient.java | 16 +- .../rest/api/tests/util/MultiPartBuilder.java | 16 +- 4 files changed, 155 insertions(+), 71 deletions(-) diff --git a/source/java/org/alfresco/rest/api/impl/NodesImpl.java b/source/java/org/alfresco/rest/api/impl/NodesImpl.java index a7d24d2722..3a18128c5c 100644 --- a/source/java/org/alfresco/rest/api/impl/NodesImpl.java +++ b/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 mapToNodeAspects(List aspectNames) { - Set nodeAspects = new HashSet<>(aspectNames.size()); + Set 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 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 aspectQNames = mapToNodeAspects(aspectNames); + Set 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; - ContentInfo contentInfo = nodeInfo.getContent(); - if (contentInfo != null && contentInfo.getMimeType() != null) - { - mimeType = contentInfo.getMimeType(); - } - else - { - mimeType = mimetypeService.guessMimetype(nodeName); - } - writer.setMimetype(mimeType); + setWriterContentType(writer, new ContentInfoWrapper(nodeInfo.getContent()), nodeRef, false); 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 aspectQNames = mapToNodeAspects(aspectNames); Set existingAspects = nodeService.getAspects(nodeRef); @@ -1295,18 +1287,7 @@ public class NodesImpl implements Nodes } ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, 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(); + setWriterContentType(writer, new ContentInfoWrapper(contentInfo), nodeRef, true); 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 (mimeType == null || !applyMimeType) - { - 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()); - } + // if the mimeType is null, then it will be guessed. + setWriterContentType(writer, new ContentInfoWrapper(content), nodeRef, true); 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()); + } + } + } + } } diff --git a/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java b/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java index 2e2b05fa36..1cf12cfded 100644 --- a/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java +++ b/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"); - BinaryPayload payload = new BinaryPayload(file, MimetypeMap.MIMETYPE_TEXT_PLAIN); + File txtFile = TempFileProvider.createTempFile(inputStream, getClass().getSimpleName(), ".txt"); + 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"); - payload = new BinaryPayload(file, MimetypeMap.MIMETYPE_PDF); + File pdfFile = getResourceFile("quick.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()); diff --git a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiHttpClient.java b/source/test-java/org/alfresco/rest/api/tests/client/PublicApiHttpClient.java index 0facff8780..75025c3e43 100644 --- a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiHttpClient.java +++ b/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.charset = (charset == null) ? "UTF-8" : charset; + this.mimeType = mimeType; + 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; } } diff --git a/source/test-java/org/alfresco/rest/api/tests/util/MultiPartBuilder.java b/source/test-java/org/alfresco/rest/api/tests/util/MultiPartBuilder.java index 574eaac979..5c29317935 100644 --- a/source/test-java/org/alfresco/rest/api/tests/util/MultiPartBuilder.java +++ b/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());