From a41fcdff0f509c5fc14e9ac0d8d112429489d803 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Wed, 22 Mar 2023 15:18:37 +0000 Subject: [PATCH] ACS-4779 Ensure we only include the count for tags in the GET response when requested. --- .../org/alfresco/rest/tags/GetTagsTests.java | 30 +++++++++---------- .../org/alfresco/rest/api/impl/TagsImpl.java | 25 ++++++++-------- .../alfresco/rest/api/impl/TagsImplTest.java | 26 +++++++++++++++- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/tags/GetTagsTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/tags/GetTagsTests.java index cdc6381e54..f4e6a07e67 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/tags/GetTagsTests.java +++ b/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"); 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 e6fec46b07..e3e0fbb822 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 @@ -171,21 +171,25 @@ public class TagsImpl implements Tags taggingService.deleteTag(storeRef, tagValue); } - @Override + @Override public CollectionWithPagingInfo getTags(StoreRef storeRef, Parameters params) { - Paging paging = params.getPaging(); - Map> namesFilters = resolveTagNamesQuery(params.getQuery()); - PagingResults> results = taggingService.getTags(storeRef, Util.getPagingRequest(paging), namesFilters.get(EQUALS), namesFilters.get(MATCHES)); + Paging paging = params.getPaging(); + Map> namesFilters = resolveTagNamesQuery(params.getQuery()); + PagingResults> results = taggingService.getTags(storeRef, Util.getPagingRequest(paging), namesFilters.get(EQUALS), namesFilters.get(MATCHES)); Integer totalItems = results.getTotalResultCount().getFirst(); List> page = results.getPage(); List tags = new ArrayList<>(page.size()); - List> tagsByCount; - Map tagsByCountMap = new HashMap<>(); + for (Pair pair : page) + { + Tag selectedTag = new Tag(pair.getFirst(), pair.getSecond()); + tags.add(selectedTag); + } if (params.getInclude().contains(PARAM_INCLUDE_COUNT)) { - tagsByCount = taggingService.findTaggedNodesAndCountByTagName(storeRef); + List> tagsByCount = taggingService.findTaggedNodesAndCountByTagName(storeRef); + Map tagsByCountMap = new HashMap<>(); if (tagsByCount != null) { for (Pair tagByCountElem : tagsByCount) @@ -193,12 +197,7 @@ public class TagsImpl implements Tags tagsByCountMap.put(tagByCountElem.getFirst(), tagByCountElem.getSecond()); } } - } - for (Pair pair : page) - { - Tag selectedTag = new Tag(pair.getFirst(), pair.getSecond()); - selectedTag.setCount(Optional.ofNullable(tagsByCountMap.get(selectedTag.getTag())).orElse(0)); - tags.add(selectedTag); + tags.forEach(tag -> tag.setCount(Optional.ofNullable(tagsByCountMap.get(tag.getTag())).orElse(0))); } return CollectionWithPagingInfo.asPaged(paging, tags, results.hasMoreItems(), totalItems); 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 8b668abef1..9a5187c3bb 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 @@ -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 expectedTags = createTagsWithNodeRefs(List.of(TAG_NAME)).stream().peek(tag -> tag.setCount(0)).collect(Collectors.toList()); + final List 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> 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 actualTags = objectUnderTest.getTags(STORE_REF_WORKSPACE_SPACESSTORE, parametersMock); + + then(taggingServiceMock).should().findTaggedNodesAndCountByTagName(STORE_REF_WORKSPACE_SPACESSTORE); + final List 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() {