From 7b979fa11e5230889a7581fa31b95e16754e6723 Mon Sep 17 00:00:00 2001 From: David Caruana Date: Mon, 29 Mar 2010 17:54:30 +0000 Subject: [PATCH] Resolve ALF-1702: CMIS: can't change name of a document via checkout/checkin - modified repository checkin to take into account name change on pwc (if it's changed, the checkin will rename the original) - updated coci unit tests - performed alfresco explorer tests - updated Chemistry TCK to re-enable update of name on checkin test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@19650 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/core-services-context.xml | 3 + .../alfresco/messages/coci-service.properties | 3 +- .../repo/coci/CheckOutCheckInServiceImpl.java | 183 ++++++++++++++---- .../coci/CheckOutCheckInServiceImplTest.java | 42 ++-- .../coci/CheckOutCheckInServiceException.java | 13 ++ 5 files changed, 179 insertions(+), 65 deletions(-) diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index 31636cded0..934844bd4b 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -1155,6 +1155,9 @@ + + + diff --git a/config/alfresco/messages/coci-service.properties b/config/alfresco/messages/coci-service.properties index 112a90b233..aeaa37442f 100644 --- a/config/alfresco/messages/coci-service.properties +++ b/config/alfresco/messages/coci-service.properties @@ -7,4 +7,5 @@ coci_service.err_workingcopy_checkout=A working copy can not be checked out. coci_service.err_not_authenticated=Can not find the currently authenticated user details required by the CheckOutCheckIn service. coci_service.err_workingcopy_has_no_mimetype=Working copy node ({0}) has no mimetype coci_service.err_already_checkedout=This node is already checked out. -coci_service.discussion_for={0} discussion \ No newline at end of file +coci_service.err_cannot_rename=Cannot rename from {0} to {1}. +coci_service.discussion_for={0} discussion diff --git a/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImpl.java b/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImpl.java index a6f3cb73a8..a4d4eb1e3f 100644 --- a/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImpl.java +++ b/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImpl.java @@ -23,7 +23,6 @@ import java.util.HashMap; import java.util.Map; import org.alfresco.error.AlfrescoRuntimeException; -import org.springframework.extensions.surf.util.I18NUtil; import org.alfresco.model.ContentModel; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.version.VersionableAspect; @@ -32,6 +31,9 @@ import org.alfresco.service.cmr.coci.CheckOutCheckInServiceException; import org.alfresco.service.cmr.lock.LockService; import org.alfresco.service.cmr.lock.LockType; import org.alfresco.service.cmr.lock.UnableToReleaseLockException; +import org.alfresco.service.cmr.model.FileExistsException; +import org.alfresco.service.cmr.model.FileFolderService; +import org.alfresco.service.cmr.model.FileNotFoundException; import org.alfresco.service.cmr.repository.AspectMissingException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.ContentData; @@ -43,6 +45,7 @@ import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AuthenticationService; import org.alfresco.service.cmr.version.VersionService; import org.alfresco.service.namespace.QName; +import org.springframework.extensions.surf.util.I18NUtil; /** * Version operations service implementation @@ -61,6 +64,7 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService private static final String MSG_ERR_NOT_AUTHENTICATED = "coci_service.err_not_authenticated"; private static final String MSG_ERR_WORKINGCOPY_HAS_NO_MIMETYPE = "coci_service.err_workingcopy_has_no_mimetype"; private static final String MSG_ALREADY_CHECKEDOUT = "coci_service.err_already_checkedout"; + private static final String MSG_ERR_CANNOT_RENAME = "coci_service.err_cannot_rename"; /** * Extension character, used to recalculate the working copy names @@ -87,6 +91,11 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService */ private CopyService copyService; + /** + * The file folder service + */ + private FileFolderService fileFolderService; + /** * The search service */ @@ -164,7 +173,17 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService { this.searchService = searchService; } - + + /** + * Set the file folder service + * + * @param fileFolderService the file folder service + */ + public void setFileFolderService(FileFolderService fileFolderService) + { + this.fileFolderService = fileFolderService; + } + /** * Sets the versionable aspect behaviour implementation * @@ -175,16 +194,6 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService this.versionableAspect = versionableAspect; } - /** - * Get the working copy label. - * - * @return the working copy label - */ - public String getWorkingCopyLabel() - { - return I18NUtil.getMessage(MSG_WORKING_COPY_LABEL); - } - /** * @see org.alfresco.service.cmr.coci.CheckOutCheckInService#checkout(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.namespace.QName, org.alfresco.service.namespace.QName) */ @@ -194,12 +203,12 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService final QName destinationAssocTypeQName, QName destinationAssocQName) { - LockType lockType = this.lockService.getLockType(nodeRef); + LockType lockType = this.lockService.getLockType(nodeRef); if (LockType.READ_ONLY_LOCK.equals(lockType) == true || getWorkingCopy(nodeRef) != null) - { - throw new CheckOutCheckInServiceException(MSG_ALREADY_CHECKEDOUT); - } - + { + throw new CheckOutCheckInServiceException(MSG_ALREADY_CHECKEDOUT); + } + // Make sure we are no checking out a working copy node if (this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_WORKING_COPY) == true) { @@ -214,27 +223,7 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService // Rename the working copy String copyName = (String)this.nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); - if (this.getWorkingCopyLabel() != null && this.getWorkingCopyLabel().length() != 0) - { - if (copyName != null && copyName.length() != 0) - { - int index = copyName.lastIndexOf(EXTENSION_CHARACTER); - if (index > 0) - { - // Insert the working copy label before the file extension - copyName = copyName.substring(0, index) + " " + getWorkingCopyLabel() + copyName.substring(index); - } - else - { - // Simply append the working copy label onto the end of the existing name - copyName = copyName + " " + getWorkingCopyLabel(); - } - } - else - { - copyName = getWorkingCopyLabel(); - } - } + copyName = createWorkingCopyName(copyName); // Make the working copy final QName copyQName = QName.createQName(destinationAssocQName.getNamespaceURI(), QName.createValidLocalName(copyName)); @@ -368,6 +357,45 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService // Copy the contents of the working copy onto the original this.copyService.copy(workingCopyNodeRef, nodeRef); + // Handle name change on working copy (only for folders/files) + if (fileFolderService.getFileInfo(workingCopyNodeRef) != null) + { + String origName = (String)this.nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + String name = (String)this.nodeService.getProperty(workingCopyNodeRef, ContentModel.PROP_NAME); + if (hasWorkingCopyNameChanged(name, origName)) + { + // ensure working copy has working copy label in its name to avoid name clash + if (!name.contains(" " + getWorkingCopyLabel())) + { + try + { + fileFolderService.rename(workingCopyNodeRef, createWorkingCopyName(name)); + } + catch (FileExistsException e) + { + throw new CheckOutCheckInServiceException(e, MSG_ERR_CANNOT_RENAME, name, createWorkingCopyName(name)); + } + catch (FileNotFoundException e) + { + throw new CheckOutCheckInServiceException(e, MSG_ERR_CANNOT_RENAME, name, createWorkingCopyName(name)); + } + } + try + { + // rename original to changed working name + fileFolderService.rename(nodeRef, getNameFromWorkingCopyName(name)); + } + catch (FileExistsException e) + { + throw new CheckOutCheckInServiceException(e, MSG_ERR_CANNOT_RENAME, origName, getNameFromWorkingCopyName(name)); + } + catch (FileNotFoundException e) + { + throw new CheckOutCheckInServiceException(e, MSG_ERR_CANNOT_RENAME, name, getNameFromWorkingCopyName(name)); + } + } + } + if (versionProperties != null && this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE) == true) { // Create the new version @@ -377,7 +405,7 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService if (keepCheckedOut == false) { // Delete the working copy - this.nodeService.deleteNode(workingCopyNodeRef); + this.nodeService.deleteNode(workingCopyNodeRef); } else { @@ -492,4 +520,81 @@ public class CheckOutCheckInServiceImpl implements CheckOutCheckInService return workingCopy; } + + /** + * Create working copy name + * + * @param name name + * @return working copy name + */ + public String createWorkingCopyName(String name) + { + if (this.getWorkingCopyLabel() != null && this.getWorkingCopyLabel().length() != 0) + { + if (name != null && name.length() != 0) + { + int index = name.lastIndexOf(EXTENSION_CHARACTER); + if (index > 0) + { + // Insert the working copy label before the file extension + name = name.substring(0, index) + " " + getWorkingCopyLabel() + name.substring(index); + } + else + { + // Simply append the working copy label onto the end of the existing name + name = name + " " + getWorkingCopyLabel(); + } + } + else + { + name = getWorkingCopyLabel(); + } + } + return name; + } + + /** + * Get original name from working copy name + * + * @param workingCopyName + * @return original name + */ + private String getNameFromWorkingCopyName(String workingCopyName) + { + String workingCopyLabel = getWorkingCopyLabel(); + String workingCopyLabelRegEx = workingCopyLabel.replaceAll("\\(", "\\\\("); + workingCopyLabelRegEx = workingCopyLabelRegEx.replaceAll("\\)", "\\\\)"); + if (workingCopyName.contains(" " + workingCopyLabel)) + { + workingCopyName = workingCopyName.replaceFirst(" " + workingCopyLabelRegEx, ""); + } + else if (workingCopyName.contains(workingCopyLabel)) + { + workingCopyName = workingCopyName.replaceFirst(workingCopyLabelRegEx, ""); + } + return workingCopyName; + } + + /** + * Has the working copy name changed compared to the original name + * + * @param name working copy name + * @param origName original name + * @return true => if changed + */ + private boolean hasWorkingCopyNameChanged(String workingCopyName, String origName) + { + return !workingCopyName.equals(createWorkingCopyName(origName)); + } + + /** + * Get the working copy label. + * + * @return the working copy label + */ + public String getWorkingCopyLabel() + { + return I18NUtil.getMessage(MSG_WORKING_COPY_LABEL); + } + } diff --git a/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImplTest.java b/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImplTest.java index 751fb173c1..fa1852be08 100644 --- a/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImplTest.java +++ b/source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImplTest.java @@ -126,7 +126,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest ChildAssociationRef childAssocRef = this.nodeService.createNode( rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test"), + QName.createQName("test"), ContentModel.TYPE_CONTENT); this.nodeRef = childAssocRef.getChildRef(); this.nodeService.addAspect(this.nodeRef, ContentModel.ASPECT_TITLED, null); @@ -184,7 +184,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest this.nodeRef, this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}workingCopy")); + QName.createQName("workingCopy")); assertNotNull(workingCopy); //System.out.println(NodeStoreInspector.dumpNodeStore(this.nodeService, this.storeRef)); @@ -197,18 +197,10 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest assertEquals(this.userNodeRef, this.nodeService.getProperty(workingCopy, ContentModel.PROP_WORKING_COPY_OWNER)); // Check that the working copy name has been set correctly - String workingCopyLabel = ((CheckOutCheckInServiceImpl)this.cociService).getWorkingCopyLabel(); - String workingCopyName = (String)this.nodeService.getProperty(workingCopy, PROP_NAME_QNAME); - if (workingCopyLabel == null || workingCopyLabel.length() == 0) - { - assertEquals("myDocument.doc", workingCopyName); - } - else - { - assertEquals( - "myDocument " + workingCopyLabel + ".doc", - workingCopyName); - } + String name = (String)this.nodeService.getProperty(this.nodeRef, PROP_NAME_QNAME); + String workingCopyLabel = ((CheckOutCheckInServiceImpl)this.cociService).createWorkingCopyName(name); + String workingCopyName = (String)this.nodeService.getProperty(workingCopy, PROP_NAME_QNAME); + assertEquals(workingCopyLabel, workingCopyName); // Ensure that the content has been copied correctly ContentReader contentReader = this.contentService.getReader(this.nodeRef, ContentModel.PROP_CONTENT); @@ -269,8 +261,8 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest assertEquals(CONTENT_2, versionContentReader.getContentString()); // Check that the name is not updated during the check-in - assertEquals(TEST_VALUE_NAME, this.nodeService.getProperty(versionNodeRef, PROP_NAME_QNAME)); - assertEquals(TEST_VALUE_NAME, this.nodeService.getProperty(origNodeRef, PROP_NAME_QNAME)); + assertEquals(TEST_VALUE_2, this.nodeService.getProperty(versionNodeRef, PROP_NAME_QNAME)); + assertEquals(TEST_VALUE_2, this.nodeService.getProperty(origNodeRef, PROP_NAME_QNAME)); // Check that the other properties are updated during the check-in assertEquals(TEST_VALUE_3, this.nodeService.getProperty(versionNodeRef, PROP2_QNAME)); @@ -293,7 +285,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest NodeRef translationNodeRef = this.nodeService.createNode( rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}translation"), + QName.createQName("translation"), ContentModel.TYPE_CONTENT).getChildRef(); this.nodeService.addAspect(this.nodeRef, QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "translatable"), null); @@ -304,7 +296,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest this.nodeRef, this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}workingCopy")); + QName.createQName("workingCopy")); // Check it back in again @@ -326,7 +318,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest ChildAssociationRef childAssocRef = this.nodeService.createNode( rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test"), + QName.createQName("test"), ContentModel.TYPE_CONTENT, bagOfProps); NodeRef noVersionNodeRef = childAssocRef.getChildRef(); @@ -398,7 +390,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest NodeRef origNodeRef = this.nodeService.createNode( this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test2"), + QName.createQName("test2"), ContentModel.TYPE_CONTENT).getChildRef(); @@ -440,7 +432,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest NodeRef origNodeRef = this.nodeService.createNode( this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test2"), + QName.createQName("test2"), ContentModel.TYPE_CONTENT).getChildRef(); // Make a copy of the node @@ -448,7 +440,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest origNodeRef, this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test6"), + QName.createQName("test6"), false); NodeRef wk1 = this.cociService.getWorkingCopy(origNodeRef); @@ -488,7 +480,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest this.nodeRef, this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}workingCopy")); + QName.createQName("workingCopy")); assertNotNull(workingCopy); // Try and check the same node out again @@ -498,7 +490,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest this.nodeRef, this.rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}workingCopy2")); + QName.createQName("workingCopy2")); fail("This document has been checked out twice."); } catch (Exception exception) @@ -516,7 +508,7 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest ChildAssociationRef childAssocRef = this.nodeService.createNode( rootNodeRef, ContentModel.ASSOC_CHILDREN, - QName.createQName("{test}test"), + QName.createQName("test"), ContentModel.TYPE_CONTENT, null); final NodeRef testNodeRef = childAssocRef.getChildRef(); diff --git a/source/java/org/alfresco/service/cmr/coci/CheckOutCheckInServiceException.java b/source/java/org/alfresco/service/cmr/coci/CheckOutCheckInServiceException.java index e83fc13f17..f5d3d07796 100644 --- a/source/java/org/alfresco/service/cmr/coci/CheckOutCheckInServiceException.java +++ b/source/java/org/alfresco/service/cmr/coci/CheckOutCheckInServiceException.java @@ -52,4 +52,17 @@ public class CheckOutCheckInServiceException extends AlfrescoRuntimeException { super(message, throwable); } + + /** + * Constructor + * + * @param message the error message + * @param throwable the cause of the exeption + * @param objects message arguments + */ + public CheckOutCheckInServiceException(Throwable throwable, String message, Object ...objects) + { + super(message, objects, throwable); + } + }