ACS-4779 Ensure we only include the count for tags in the GET response when requested.

packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/tags/GetTagsTests.java

@@ -33,7 +33,7 @@ public class GetTagsTests extends TagsDataPrep
 
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.SANITY, description = "Verify user with Manager role gets tags using REST API and status code is OK (200)")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.SANITY })
-    public void getTagsWithManagerRole() throws Exception
+    public void getTagsWithManagerRole()
     {
         restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteManager));
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -45,7 +45,7 @@ public class GetTagsTests extends TagsDataPrep
     
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION, description = "Verify user with Collaborator role gets tags using REST API and status code is OK (200)")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void getTagsWithCollaboratorRole() throws Exception
+    public void getTagsWithCollaboratorRole()
     {
         restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteCollaborator));
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -57,7 +57,7 @@ public class GetTagsTests extends TagsDataPrep
     
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION, description = "Verify user with Contributor role gets tags using REST API and status code is OK (200)")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void getTagsWithContributorRole() throws Exception
+    public void getTagsWithContributorRole()
     {
         restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteContributor));
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -69,7 +69,7 @@ public class GetTagsTests extends TagsDataPrep
     
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION, description = "Verify user with Consumer role gets tags using REST API and status code is OK (200)")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void getTagsWithConsumerRole() throws Exception
+    public void getTagsWithConsumerRole()
     {
         restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteConsumer));
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -82,7 +82,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.SANITY, description = "Failed authentication get tags call returns status code 401 with Manager role")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.SANITY })
 //    @Bug(id="MNT-16904", description = "It fails only on environment with tenants")
-    public void failedAuthenticationReturnsUnauthorizedStatus() throws Exception
+    public void failedAuthenticationReturnsUnauthorizedStatus()
     {
         restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteManager));
         userModel = dataUser.createRandomTestUser();
@@ -96,7 +96,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "Verify that if maxItems is invalid status code returned is 400")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION})
-    public void maxItemsInvalidValueTest() throws Exception
+    public void maxItemsInvalidValueTest()
     {
         restClient.authenticateUser(adminUserModel).withParams("maxItems=abc").withCoreAPI().getTags();
         restClient.assertStatusCodeIs(HttpStatus.BAD_REQUEST).assertLastError().containsSummary(String.format(RestErrorModel.INVALID_MAXITEMS, "abc"));
@@ -105,7 +105,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "Verify that if skipCount is invalid status code returned is 400")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION})
-    public void skipCountInvalidValueTest() throws Exception
+    public void skipCountInvalidValueTest()
     {
         restClient.authenticateUser(adminUserModel).withParams("skipCount=abc").withCoreAPI().getTags();
         restClient.assertStatusCodeIs(HttpStatus.BAD_REQUEST).assertLastError().containsSummary(String.format(RestErrorModel.INVALID_SKIPCOUNT, "abc"));
@@ -114,7 +114,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "Verify that file tag is retrieved")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION})
-    public void fileTagIsRetrieved() throws Exception
+    public void fileTagIsRetrieved()
     {
         restClient.authenticateUser(adminUserModel);
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -127,7 +127,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "Verify that folder tag is retrieved")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION})
-    public void folderTagIsRetrieved() throws Exception
+    public void folderTagIsRetrieved()
     {
         restClient.authenticateUser(adminUserModel);
         returnedCollection = restClient.withParams("maxItems=10000").withCoreAPI().getTags();
@@ -140,7 +140,7 @@ public class GetTagsTests extends TagsDataPrep
             description = "Verify site Manager is able to get tags using properties parameter."
                     + "Check that properties filter is applied.")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void siteManagerIsAbleToRetrieveTagsWithPropertiesParameter() throws Exception
+    public void siteManagerIsAbleToRetrieveTagsWithPropertiesParameter()
     {
         returnedCollection = restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteManager))
                 .withParams("maxItems=5000&properties=tag").withCoreAPI().getTags();
@@ -154,7 +154,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "With admin get tags and use skipCount parameter. Check pagination")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void useSkipCountCheckPagination() throws Exception
+    public void useSkipCountCheckPagination()
     {
         returnedCollection = restClient.authenticateUser(adminUserModel).withCoreAPI().getTags();
         restClient.assertStatusCodeIs(OK);
@@ -172,7 +172,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "With admin get tags and use maxItems parameter. Check pagination")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void useMaxItemsParameterCheckPagination() throws Exception
+    public void useMaxItemsParameterCheckPagination()
     {
         returnedCollection = restClient.authenticateUser(adminUserModel).withCoreAPI().getTags();
         restClient.assertStatusCodeIs(OK);
@@ -191,7 +191,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "With manager get tags and use high skipCount parameter. Check pagination")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void useHighSkipCountCheckPagination() throws Exception
+    public void useHighSkipCountCheckPagination()
     {
         returnedCollection = restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteManager))
                 .withParams("skipCount=20000").withCoreAPI().getTags();
@@ -207,7 +207,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "With Collaborator user get tags and use maxItems with value zero. Check default error model schema")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void useMaxItemsWithValueZeroCheckDefaultErrorModelSchema() throws Exception
+    public void useMaxItemsWithValueZeroCheckDefaultErrorModelSchema()
     {
         returnedCollection = restClient.authenticateUser(usersWithRoles.getOneUserWithRole(UserRole.SiteCollaborator))
                 .withParams("maxItems=0").withCoreAPI().getTags();
@@ -221,7 +221,7 @@ public class GetTagsTests extends TagsDataPrep
     @TestRail(section = { TestGroup.REST_API, TestGroup.TAGS }, executionType = ExecutionType.REGRESSION,
             description = "With Manager user delete tag. Check it is not retrieved anymore.")
     @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION })
-    public void checkThatDeletedTagIsNotRetrievedAnymore() throws Exception
+    public void checkThatDeletedTagIsNotRetrievedAnymore()
     {
         String removedTag = getRandomName("tag3");
 

remote-api/src/main/java/org/alfresco/rest/api/impl/TagsImpl.java

@@ -181,11 +181,15 @@ public class TagsImpl implements Tags
         Integer totalItems = results.getTotalResultCount().getFirst();
         List<Pair<NodeRef, String>> page = results.getPage();
         List<Tag> tags = new ArrayList<>(page.size());
-        List<Pair<String, Integer>> tagsByCount;
+        for (Pair<NodeRef, String> pair : page)
-        Map<String, Integer> tagsByCountMap = new HashMap<>();
+        {
+            Tag selectedTag = new Tag(pair.getFirst(), pair.getSecond());
+            tags.add(selectedTag);
+        }
         if (params.getInclude().contains(PARAM_INCLUDE_COUNT))
         {
-            tagsByCount = taggingService.findTaggedNodesAndCountByTagName(storeRef);
+            List<Pair<String, Integer>> tagsByCount = taggingService.findTaggedNodesAndCountByTagName(storeRef);
+            Map<String, Integer> tagsByCountMap = new HashMap<>();
             if (tagsByCount != null)
             {
                 for (Pair<String, Integer> tagByCountElem : tagsByCount)
@@ -193,12 +197,7 @@ public class TagsImpl implements Tags
                     tagsByCountMap.put(tagByCountElem.getFirst(), tagByCountElem.getSecond());
                 }
             }
-        }
+            tags.forEach(tag -> tag.setCount(Optional.ofNullable(tagsByCountMap.get(tag.getTag())).orElse(0)));
-        for (Pair<NodeRef, String> pair : page)
-        {
-            Tag selectedTag = new Tag(pair.getFirst(), pair.getSecond());
-            selectedTag.setCount(Optional.ofNullable(tagsByCountMap.get(selectedTag.getTag())).orElse(0));
-            tags.add(selectedTag);
         }
 
         return CollectionWithPagingInfo.asPaged(paging, tags, results.hasMoreItems(), totalItems);

remote-api/src/test/java/org/alfresco/rest/api/impl/TagsImplTest.java

@@ -122,7 +122,7 @@ public class TagsImplTest
 
         then(taggingServiceMock).should().getTags(eq(STORE_REF_WORKSPACE_SPACESSTORE), any(PagingRequest.class), isNull(), isNull());
         then(taggingServiceMock).shouldHaveNoMoreInteractions();
-        final List<Tag> expectedTags = createTagsWithNodeRefs(List.of(TAG_NAME)).stream().peek(tag -> tag.setCount(0)).collect(Collectors.toList());
+        final List<Tag> expectedTags = createTagsWithNodeRefs(List.of(TAG_NAME)).stream().collect(Collectors.toList());
         assertEquals(expectedTags, actualTags.getCollection());
     }
 
@@ -144,6 +144,30 @@ public class TagsImplTest
         assertEquals(expectedTags, actualTags.getCollection());
     }
 
+    /** Check that we can get counts for two tags - one in use and one not applied to any nodes. */
+    @Test
+    public void testGetTags_verifyCountPopulatedCorrectly()
+    {
+        NodeRef tagNodeA = new NodeRef("tag://A/");
+        NodeRef tagNodeB = new NodeRef("tag://B/");
+        List<Pair<NodeRef, String>> tagPairs = List.of(new Pair<>(tagNodeA, "tagA"), new Pair<>(tagNodeB, "tagB"));
+
+        given(parametersMock.getPaging()).willReturn(pagingMock);
+        given(taggingServiceMock.getTags(any(StoreRef.class), any(PagingRequest.class), any(), any())).willReturn(pagingResultsMock);
+        given(pagingResultsMock.getTotalResultCount()).willReturn(new Pair<>(Integer.MAX_VALUE, 0));
+        given(pagingResultsMock.getPage()).willReturn(tagPairs);
+        given(parametersMock.getInclude()).willReturn(List.of("count"));
+        // Only tagA is included in the returned list since tagB is not in use.
+        given(taggingServiceMock.findTaggedNodesAndCountByTagName(STORE_REF_WORKSPACE_SPACESSTORE)).willReturn(List.of(new Pair<>("tagA", 5)));
+
+        final CollectionWithPagingInfo<Tag> actualTags = objectUnderTest.getTags(STORE_REF_WORKSPACE_SPACESSTORE, parametersMock);
+
+        then(taggingServiceMock).should().findTaggedNodesAndCountByTagName(STORE_REF_WORKSPACE_SPACESSTORE);
+        final List<Tag> expectedTags = List.of(Tag.builder().tag("tagA").nodeRef(tagNodeA).count(5).create(),
+                                               Tag.builder().tag("tagB").nodeRef(tagNodeB).count(0).create());
+        assertEquals(expectedTags, actualTags.getCollection());
+    }
+
     @Test
     public void testGetTags_withEqualsClauseWhereQuery()
     {