From 7cbfab81ce29477d6fbf43c7ce1696a75c45f6b0 Mon Sep 17 00:00:00 2001 From: Suneet Gupta <89080268+suneet-gupta@users.noreply.github.com> Date: Tue, 21 Mar 2023 14:03:27 +0530 Subject: [PATCH] [ACS-3916][ACS-3917] Added TagId Validation for Get and Delete methods (#1815) * [ACS-3916][ACS-3917] Added TagId Validation for Get and Delete methods * Addressed review comments - 1. Moved duplicate code from ValidateTag methods to a private method 2. Updated the test case --- .../org/alfresco/rest/api/impl/TagsImpl.java | 28 ++++++++++++------- .../alfresco/public-rest-context.xml | 1 + .../alfresco/rest/api/impl/TagsImplTest.java | 21 ++++++++++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/TagsImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/TagsImpl.java index 62b02c572f..e6fec46b07 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/TagsImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/TagsImpl.java @@ -62,6 +62,7 @@ import org.alfresco.rest.framework.resource.parameters.where.QueryHelper; import org.alfresco.rest.framework.resource.parameters.where.QueryImpl; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.tagging.TaggingService; @@ -81,8 +82,10 @@ public class TagsImpl implements Tags private static final String PARAM_WHERE_TAG = "tag"; static final String NOT_A_VALID_TAG = "An invalid parameter has been supplied"; static final String NO_PERMISSION_TO_MANAGE_A_TAG = "Current user does not have permission to manage a tag"; + private final NodeRef tagParentNodeRef = new NodeRef("workspace://SpacesStore/tag:tag-root"); private Nodes nodes; + private NodeService nodeService; private TaggingService taggingService; private TypeConstraint typeConstraint; private AuthorityService authorityService; @@ -96,6 +99,10 @@ public class TagsImpl implements Tags { this.nodes = nodes; } + public void setNodeService(NodeService nodeService) + { + this.nodeService = nodeService; + } public void setTaggingService(TaggingService taggingService) { @@ -200,21 +207,13 @@ public class TagsImpl implements Tags public NodeRef validateTag(String tagId) { NodeRef tagNodeRef = nodes.validateNode(tagId); - if(tagNodeRef == null) - { - throw new EntityNotFoundException(tagId); - } - return tagNodeRef; + return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); } public NodeRef validateTag(StoreRef storeRef, String tagId) { NodeRef tagNodeRef = nodes.validateNode(storeRef, tagId); - if(tagNodeRef == null) - { - throw new EntityNotFoundException(tagId); - } - return tagNodeRef; + return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); } public Tag changeTag(StoreRef storeRef, String tagId, Tag tag) @@ -335,4 +334,13 @@ public class TagsImpl implements Tags } }, Collectors.flatMapping((entry) -> entry.getValue().stream().map(String::toLowerCase), Collectors.toCollection(HashSet::new)))); } + + private NodeRef checkTagRootAsNodePrimaryParent(String tagId, NodeRef tagNodeRef) + { + if ( tagNodeRef == null || !nodeService.getPrimaryParent(tagNodeRef).getParentRef().equals(tagParentNodeRef)) + { + throw new EntityNotFoundException(tagId); + } + return tagNodeRef; + } } diff --git a/remote-api/src/main/resources/alfresco/public-rest-context.xml b/remote-api/src/main/resources/alfresco/public-rest-context.xml index 3460b0a888..d562e174bf 100644 --- a/remote-api/src/main/resources/alfresco/public-rest-context.xml +++ b/remote-api/src/main/resources/alfresco/public-rest-context.xml @@ -858,6 +858,7 @@ + diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/TagsImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/TagsImplTest.java index 16ccb05454..8b668abef1 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/TagsImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/TagsImplTest.java @@ -55,8 +55,10 @@ import org.alfresco.rest.framework.resource.parameters.Paging; import org.alfresco.rest.framework.resource.parameters.Parameters; import org.alfresco.rest.framework.resource.parameters.where.InvalidQueryException; import org.alfresco.rest.framework.tools.RecognizedParamsExtractor; +import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.DuplicateChildNodeNameException; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.tagging.TaggingService; @@ -72,14 +74,20 @@ import org.mockito.junit.MockitoJUnitRunner; public class TagsImplTest { private static final String TAG_ID = "tag-node-id"; + private static final String PARENT_NODE_ID = "tag:tag-root"; private static final String TAG_NAME = "tag-dummy-name"; private static final NodeRef TAG_NODE_REF = new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID.concat("-").concat(TAG_NAME)); + private static final NodeRef TAG_PARENT_NODE_REF = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, PARENT_NODE_ID); private final RecognizedParamsExtractor queryExtractor = new RecognizedParamsExtractor() {}; @Mock private Nodes nodesMock; @Mock + private ChildAssociationRef primaryParentMock; + @Mock + private NodeService nodeServiceMock; + @Mock private AuthorityService authorityServiceMock; @Mock private TaggingService taggingServiceMock; @@ -99,6 +107,7 @@ public class TagsImplTest given(authorityServiceMock.hasAdminAuthority()).willReturn(true); given(nodesMock.validateNode(STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID)).willReturn(TAG_NODE_REF); given(taggingServiceMock.getTagName(TAG_NODE_REF)).willReturn(TAG_NAME); + given(nodeServiceMock.getPrimaryParent(TAG_NODE_REF)).willReturn(primaryParentMock); } @Test @@ -226,6 +235,7 @@ public class TagsImplTest public void testDeleteTagById() { //when + given(primaryParentMock.getParentRef()).willReturn(TAG_PARENT_NODE_REF); objectUnderTest.deleteTagById(STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); then(authorityServiceMock).should().hasAdminAuthority(); @@ -391,6 +401,17 @@ public class TagsImplTest .isEqualTo(expectedTags); } + @Test(expected = EntityNotFoundException.class) + public void testGetTagByIdNotFoundValidation() + { + given(primaryParentMock.getParentRef()).willReturn(TAG_NODE_REF); + objectUnderTest.getTag(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE,TAG_ID); + then(nodeServiceMock).shouldHaveNoInteractions(); + then(nodesMock).should().validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); + then(nodesMock).shouldHaveNoMoreInteractions(); + then(taggingServiceMock).shouldHaveNoInteractions(); + } + private static List> createTagAndNodeRefPairs(final List tagNames) { return tagNames.stream()