diff --git a/source/java/org/alfresco/repo/tagging/TaggingServiceImpl.java b/source/java/org/alfresco/repo/tagging/TaggingServiceImpl.java index 886962f2c2..ccb404cd9c 100644 --- a/source/java/org/alfresco/repo/tagging/TaggingServiceImpl.java +++ b/source/java/org/alfresco/repo/tagging/TaggingServiceImpl.java @@ -415,8 +415,12 @@ public class TaggingServiceImpl implements TaggingService, // Queue all the before's for removal to the tag scope for (NodeRef beforeNodeRef : beforeNodeRefs) { - String tagName = getTagName(beforeNodeRef); - queueTagUpdate(nodeRef, tagName, false); + // Protect against InvalidNodeRefException(MNT-14453) + if (this.nodeService.exists(beforeNodeRef)) + { + String tagName = getTagName(beforeNodeRef); + queueTagUpdate(nodeRef, tagName, false); + } } } else if (afterNodeRefs != null && beforeNodeRefs != null) @@ -430,7 +434,8 @@ public class TaggingServiceImpl implements TaggingService, // remove the node ref from the after list afterNodeRefs.remove(beforeNodeRef); } - else + // Protect against InvalidNodeRefException(MNT-14453) + else if (this.nodeService.exists(beforeNodeRef)) { String tagName = getTagName(beforeNodeRef); queueTagUpdate(nodeRef, tagName, false); @@ -477,6 +482,15 @@ public class TaggingServiceImpl implements TaggingService, // Lower the case of the tag tag = tag.toLowerCase(); + // Find nodes which are tagged with the 'soon to be deleted' tag. + List taggedNodes = this.findTaggedNodes(storeRef, tag); + + // Clear the tag from the found nodes + for (NodeRef taggedNode : taggedNodes) + { + this.removeTag(taggedNode, tag); + } + NodeRef tagNodeRef = getTagNodeRef(storeRef, tag); if (tagNodeRef != null) { diff --git a/source/java/org/alfresco/service/cmr/tagging/TaggingService.java b/source/java/org/alfresco/service/cmr/tagging/TaggingService.java index 614172831a..45d6026327 100644 --- a/source/java/org/alfresco/service/cmr/tagging/TaggingService.java +++ b/source/java/org/alfresco/service/cmr/tagging/TaggingService.java @@ -94,7 +94,7 @@ public interface TaggingService NodeRef createTag(StoreRef storeRef, String tag); /** - * Delete an existing tag + * Delete an existing tag and any references to it(cascade delete). * * @param storeRef store reference * @param tag tag name diff --git a/source/test-java/org/alfresco/repo/tagging/TaggingServiceImplTest.java b/source/test-java/org/alfresco/repo/tagging/TaggingServiceImplTest.java index 42d167e716..dce52178e9 100644 --- a/source/test-java/org/alfresco/repo/tagging/TaggingServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/tagging/TaggingServiceImplTest.java @@ -46,6 +46,7 @@ import org.alfresco.repo.audit.AuditComponent; import org.alfresco.repo.audit.AuditServiceImpl; import org.alfresco.repo.audit.UserAuditFilter; import org.alfresco.repo.jscript.ClasspathScriptLocation; +import org.alfresco.repo.node.NodeRefPropertyMethodInterceptor; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.JavaBehaviour; import org.alfresco.repo.policy.PolicyComponent; @@ -122,6 +123,8 @@ public class TaggingServiceImplTest extends TestCase private MutableAuthenticationService authenticationService; private AsyncOccurs asyncOccurs; + private NodeRefPropertyMethodInterceptor nodeRefPropInterceptor; + private static StoreRef storeRef; private static NodeRef rootNode; private NodeRef folder; @@ -181,6 +184,7 @@ public class TaggingServiceImplTest extends TestCase this.personService = (PersonService)ctx.getBean("PersonService"); this.permissionService = (PermissionService)ctx.getBean("PermissionService"); this.authenticationService = (MutableAuthenticationService)ctx.getBean("authenticationService"); + this.nodeRefPropInterceptor = (NodeRefPropertyMethodInterceptor)ctx.getBean("nodeRefPropertyInterceptor"); if (init == false) { @@ -2347,4 +2351,43 @@ public class TaggingServiceImplTest extends TestCase } }); } + + /** + * Tests that when the deleteTag() method runs, it will remove invalid references to the deleted tag. + */ + public void testDeleteTag() throws Exception{ + + try{ + // We instruct the 'nodeRefPropInterceptor' to skip processing on the 'get' methods. + // This is needed because this interceptor removes any properties which are invalid. e.g. have been deleted. + // We need to make sure that the 'taggable' property stays put. + nodeRefPropInterceptor.setFilterOnGet(false); + + this.transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback(){ + + @SuppressWarnings("unchecked") + @Override + public Void execute() throws Throwable + { + taggingService.clearTags(folder); + taggingService.addTag(folder, TAG_1); + + // The deleteTag() should remove any reference to the deleted tag + List taggableProperty = (List) nodeService.getProperty(folder, ContentModel.PROP_TAGS); + assertTrue("Our folder should have a reference on one tag.", taggableProperty.size() == 1); + + taggingService.deleteTag(TaggingServiceImplTest.storeRef, TAG_1); + + // The deleteTag() should remove any reference to the deleted tag + taggableProperty = (List) nodeService.getProperty(folder, ContentModel.PROP_TAGS); + assertTrue("Our folder shouldn't have any references left to deleted tags.", taggableProperty.size() == 0); + + return null; + } + }); + } finally{ + nodeRefPropInterceptor.setFilterOnGet(true); + } + } + }