[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

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)
+		return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef);
-    	{
-    		throw new EntityNotFoundException(tagId);
-    	}
-    	return tagNodeRef;
     }
     
     public NodeRef validateTag(StoreRef storeRef, String tagId)
     {
     	NodeRef tagNodeRef = nodes.validateNode(storeRef, tagId);
-    	if(tagNodeRef == null)
+		return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef);
-    	{
-    		throw new EntityNotFoundException(tagId);
-    	}
-    	return 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;
+	}
 }

remote-api/src/main/resources/alfresco/public-rest-context.xml

@@ -858,6 +858,7 @@
         <property name="taggingService" ref="TaggingService" />
         <property name="authorityService" ref="AuthorityService" />
         <property name="typeConstraint" ref="nodeTypeConstraint" />
+        <property name="nodeService" ref="NodeService"/>
     </bean>
 
     <bean id="Tags" class="org.springframework.aop.framework.ProxyFactoryBean">

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<Pair<String, NodeRef>> createTagAndNodeRefPairs(final List<String> tagNames)
     {
         return tagNames.stream()