diff --git a/source/java/org/alfresco/rest/api/impl/NodesImpl.java b/source/java/org/alfresco/rest/api/impl/NodesImpl.java index 8f293a1781..eaf8777c59 100644 --- a/source/java/org/alfresco/rest/api/impl/NodesImpl.java +++ b/source/java/org/alfresco/rest/api/impl/NodesImpl.java @@ -1158,6 +1158,7 @@ public class NodesImpl implements Nodes String fileName = null; Content content = null; + boolean autoRename = false; boolean overwrite = false; // If a fileName clashes for a versionable file for (FormData.FormField field : formData.getFields()) @@ -1176,9 +1177,13 @@ public class NodesImpl implements Nodes } break; - case "overwrite": - overwrite = Boolean.valueOf(field.getValue()); + case "autorename": + autoRename = Boolean.valueOf(field.getValue()); break; + + // case "overwrite": + // overwrite = Boolean.valueOf(field.getValue()); + // break; } } @@ -1204,20 +1209,25 @@ public class NodesImpl implements Nodes if (existingFile != null) { // File already exists, decide what to do - if (overwrite && nodeService.hasAspect(existingFile, ContentModel.ASPECT_VERSIONABLE)) + if (autoRename) { - // Upload component was configured to overwrite files if name clashes - write(existingFile, content, fileName, false, true); - - // Extract the metadata (The overwrite policy controls - // which if any parts of the document's properties are updated from this) - extractMetadata(existingFile); - - // Do not clean formData temp files to - // allow for retries. Temp files will be deleted later - // when GC call DiskFileItem#finalize() method or by temp file cleaner. - return createUploadResponse(parentNodeRef, existingFile); + fileName = findUniqueName(parentNodeRef, fileName); } + // TODO uncomment when we decide on uploading a new version vs overwriting + // 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); + // + // // Extract the metadata (The overwrite policy controls + // // which if any parts of the document's properties are updated from this) + // extractMetadata(existingFile); + // + // // Do not clean formData temp files to + // // allow for retries. Temp files will be deleted later + // // when GC call DiskFileItem#finalize() method or by temp file cleaner. + // return createUploadResponse(parentNodeRef, existingFile); + // } else { throw new ConstraintViolatedException(fileName + " already exists."); @@ -1395,6 +1405,46 @@ public class NodesImpl implements Nodes } } + /** + * Creates a unique file name, if the upload component was configured to + * find a new unique name for clashing filenames. + * + * @param parentNodeRef the parent node + * @param fileName the original fileName + * @return a new file name + */ + private String findUniqueName(NodeRef parentNodeRef, String fileName) + { + int counter = 1; + String tmpFilename; + NodeRef existingFile; + do + { + int dotIndex = fileName.lastIndexOf('.'); + if (dotIndex == 0) + { + // File didn't have a proper 'name' instead it + // had just a suffix and started with a ".", create "1.txt" + tmpFilename = counter + fileName; + } + else if (dotIndex > 0) + { + // Filename contained ".", create "fileName-1.txt" + tmpFilename = fileName.substring(0, dotIndex) + "-" + counter + fileName.substring(dotIndex); + } + else + { + // Filename didn't contain a dot at all, create "fileName-1" + tmpFilename = fileName + "-" + counter; + } + existingFile = nodeService.getChildByName(parentNodeRef, ContentModel.ASSOC_CONTAINS, tmpFilename); + counter++; + + } while (existingFile != null); + + return tmpFilename; + } + /** * Helper to create a QName from either a fully qualified or short-name QName string * 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 d79f7feb31..d0327ae41a 100644 --- a/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java +++ b/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java @@ -541,7 +541,10 @@ public class NodeApiTest extends AbstractBaseApiTest .setFileData(new FileData(fileName, file, MimetypeMap.MIMETYPE_PDF)); MultiPartRequest reqBody = multiPartBuilder.build(); - // Try to upload + // Try to upload into a non-existent folder + post(getChildrenUrl(UUID.randomUUID().toString()), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 404); + + // Upload response = post(getChildrenUrl(Nodes.PATH_MY), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 201); Document document = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class); // Check the upload response @@ -572,6 +575,27 @@ public class NodeApiTest extends AbstractBaseApiTest assertNotNull(paging); assertEquals("Duplicate file name. The file shouldn't have been uploaded.", numOfNodes + 1, pagingResult.getCount().intValue()); + // Set autoRename=true and upload the same file again + reqBody = MultiPartBuilder.copy(multiPartBuilder) + .setAutoRename(true) + .build(); + + response = post(getChildrenUrl(Nodes.PATH_MY), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 201); + document = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class); + // Check the upload response + assertEquals("quick-1.pdf", document.getName()); + + // upload the same file again + response = post(getChildrenUrl(Nodes.PATH_MY), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 201); + document = jacksonUtil.parseEntry(response.getJsonResponse(), Document.class); + // Check the upload response + assertEquals("quick-2.pdf", document.getName()); + + response = getAll(getChildrenUrl(Nodes.PATH_MY), user1, paging, 200); + pagingResult = parsePaging(response.getJsonResponse()); + assertNotNull(paging); + assertEquals(numOfNodes + 3, pagingResult.getCount().intValue()); + // User2 tries to upload a new file into the user1's home folder. response = getSingle(NodesEntityResource.class, user1, Nodes.PATH_MY, null, 200); Folder user1Home = jacksonUtil.parseEntry(response.getJsonResponse(), Folder.class); @@ -585,13 +609,13 @@ public class NodeApiTest extends AbstractBaseApiTest response = getAll(getChildrenUrl(Nodes.PATH_MY), user1, paging, 200); pagingResult = parsePaging(response.getJsonResponse()); assertNotNull(paging); - assertEquals("Access Denied. The file shouldn't have been uploaded.", numOfNodes + 1, pagingResult.getCount().intValue()); + assertEquals("Access Denied. The file shouldn't have been uploaded.", numOfNodes + 3, pagingResult.getCount().intValue()); // User1 tries to upload a file into a document rather than a folder! post(getChildrenUrl(document.getId()), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 400); // Try to upload a file without defining the required formData - reqBody = MultiPartBuilder.create().build(); + reqBody = MultiPartBuilder.create().setAutoRename(true).build(); post(getChildrenUrl(Nodes.PATH_MY), user1, new String(reqBody.getBody()), null, reqBody.getContentType(), 400); } @@ -645,12 +669,6 @@ public class NodeApiTest extends AbstractBaseApiTest // Upload the same file again to check the name conflicts handling post(getChildrenUrl(folderA_Ref), userOneN1.getId(), new String(reqBody.getBody()), null, reqBody.getContentType(), 409); - // Set overwrite=true and upload the same file again - reqBody = MultiPartBuilder.copy(multiPartBuilder) - .setOverwrite(true) - .build(); - post(getChildrenUrl(folderA_Ref), userOneN1.getId(), new String(reqBody.getBody()), null, reqBody.getContentType(), 201); - response = getAll(getChildrenUrl(folderA_Ref), userOneN1.getId(), paging, 200); pagingResult = parsePaging(response.getJsonResponse()); assertNotNull(paging); 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 2e97181728..0c245905e5 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 @@ -26,7 +26,6 @@ package org.alfresco.rest.api.tests.util; -import static org.junit.Assert.assertNotNull; import org.apache.commons.httpclient.methods.multipart.FilePart; import org.apache.commons.httpclient.methods.multipart.MultipartRequestEntity; @@ -56,8 +55,9 @@ public class MultiPartBuilder private String description; private String contentTypeQNameStr; private List aspects; - private boolean majorVersion; - private boolean overwrite = false; // If a fileName clashes for a versionable file + private Boolean majorVersion; + private Boolean overwrite; + private Boolean autoRename; private MultiPartBuilder() { @@ -76,6 +76,7 @@ public class MultiPartBuilder this.aspects = that.aspects; this.majorVersion = that.majorVersion; this.overwrite = that.overwrite; + this.autoRename = that.autoRename; } public static MultiPartBuilder create() @@ -144,13 +145,19 @@ public class MultiPartBuilder public MultiPartBuilder setMajorVersion(boolean majorVersion) { - this.majorVersion = majorVersion; + this.majorVersion = Boolean.valueOf(majorVersion); return this; } public MultiPartBuilder setOverwrite(boolean overwrite) { - this.overwrite = overwrite; + this.overwrite = Boolean.valueOf(overwrite); + return this; + } + + public MultiPartBuilder setAutoRename(boolean autoRename) + { + this.autoRename = Boolean.valueOf(autoRename); return this; } @@ -247,8 +254,9 @@ public class MultiPartBuilder addPartIfNotNull(parts, "description", description); addPartIfNotNull(parts, "contenttype", contentTypeQNameStr); addPartIfNotNull(parts, "aspects", getAspects(aspects)); - addPartIfNotNull(parts, "majorversion", Boolean.toString(majorVersion)); - addPartIfNotNull(parts, "overwrite", Boolean.toString(overwrite)); + addPartIfNotNull(parts, "majorversion", majorVersion); + addPartIfNotNull(parts, "overwrite", overwrite); + addPartIfNotNull(parts, "autoRename", autoRename); MultipartRequestEntity req = new MultipartRequestEntity(parts.toArray(new Part[parts.size()]), new HttpMethodParams()); @@ -258,11 +266,11 @@ public class MultiPartBuilder return new MultiPartRequest(os.toByteArray(), req.getContentType(), req.getContentLength()); } - private void addPartIfNotNull(List list, String partName, String partValue) + private void addPartIfNotNull(List list, String partName, Object partValue) { if (partValue != null) { - list.add(new StringPart(partName, partValue)); + list.add(new StringPart(partName, partValue.toString())); } } }