From 3dec621b1595e816b25de08e94e94adc6dd7c2b9 Mon Sep 17 00:00:00 2001 From: MichalKinas Date: Thu, 13 Apr 2023 09:19:05 +0200 Subject: [PATCH] ACS-4966 Correct path in unit tests, cover child category case --- .../rest/categories/CategoriesPathTests.java | 24 ++++++- .../rest/api/impl/CategoriesImpl.java | 37 +++++++---- .../rest/api/impl/CategoriesImplTest.java | 63 ++++++++++++++----- 3 files changed, 93 insertions(+), 31 deletions(-) diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/categories/CategoriesPathTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/categories/CategoriesPathTests.java index 02311753b3..e958f893ec 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/categories/CategoriesPathTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/categories/CategoriesPathTests.java @@ -53,8 +53,7 @@ public class CategoriesPathTests extends CategoriesRestTest @BeforeClass(alwaysRun = true) public void dataPreparation() throws Exception { - STEP("Create user and site"); - user = dataUser.createRandomTestUser(); + STEP("Create site"); SiteModel site = dataSite.usingUser(user).createPublicRandomSite(); STEP("Create a folder, file in it and a category"); @@ -112,6 +111,27 @@ public class CategoriesPathTests extends CategoriesRestTest .allMatch(cat -> cat.getPath().equals("/categories/General"))); } + /** + * Verify path for child category. + */ + @Test(groups = { TestGroup.REST_API }) + public void testGetChildCategory_includePath() + { + STEP("Create parent and child categories and verify if child path contains parent name"); + final RestCategoryModel parentCategory = createCategoryModelWithId(ROOT_CATEGORY_ID); + final RestCategoryModel childCategory = prepareCategoriesUnder(parentCategory, 2); + final RestCategoryModelsCollection actualCategories = restClient.authenticateUser(user) + .withCoreAPI() + .usingCategory(parentCategory) + .include(INCLUDE_PATH_PARAM) + .getCategoryChildren(); + + restClient.assertStatusCodeIs(OK); + assertTrue(actualCategories.getEntries().stream() + .map(RestCategoryModel::onModel) + .allMatch(cat -> cat.getPath().equals("/categories/General/" + parentCategory.getName()))); + } + /** * Create category and verify that it has a path. */ diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/CategoriesImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/CategoriesImpl.java index efa17be46e..c733d71ab1 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/CategoriesImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/CategoriesImpl.java @@ -69,7 +69,6 @@ import org.apache.commons.lang3.StringUtils; public class CategoriesImpl implements Categories { static final String INCLUDE_COUNT_PARAM = "count"; - static final String INCLUDE_PATH_PARAM = "path"; static final String NOT_A_VALID_CATEGORY = "Node id does not refer to a valid category"; static final String NO_PERMISSION_TO_MANAGE_A_CATEGORY = "Current user does not have permission to manage a category"; static final String NO_PERMISSION_TO_READ_CONTENT = "Current user does not have read permission to content"; @@ -112,9 +111,9 @@ public class CategoriesImpl implements Categories category.setCount(categoriesCount.getOrDefault(category.getId(), 0)); } - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(nodeRef).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } return category; @@ -134,9 +133,9 @@ public class CategoriesImpl implements Categories { category.setCount(0); } - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(nodes.getNode(category.getId()).getNodeRef()).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } }) .collect(Collectors.toList()); @@ -152,9 +151,9 @@ public class CategoriesImpl implements Categories .map(ChildAssociationRef::getChildRef) .map(this::mapToCategory) .peek(category -> { - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(nodes.getNode(category.getId()).getNodeRef()).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } }) .collect(Collectors.toList()); @@ -187,9 +186,9 @@ public class CategoriesImpl implements Categories category.setCount(categoriesCount.getOrDefault(category.getId(), 0)); } - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(categoryNodeRef).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } return category; @@ -226,9 +225,9 @@ public class CategoriesImpl implements Categories .stream() .map(this::mapToCategory) .peek(category -> { - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(nodes.getNode(category.getId()).getNodeRef()).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } }) .collect(Collectors.toList()); @@ -265,9 +264,9 @@ public class CategoriesImpl implements Categories .stream() .map(this::mapToCategory) .peek(category -> { - if (parameters.getInclude().contains(INCLUDE_PATH_PARAM)) + if (parameters.getInclude().contains(Nodes.PARAM_INCLUDE_PATH)) { - category.setPath(nodeService.getPath(nodes.getNode(category.getId()).getNodeRef()).toDisplayPath(nodeService, permissionService)); + category.setPath(getCategoryPath(category)); } }) .collect(Collectors.toList()); @@ -515,4 +514,16 @@ public class CategoriesImpl implements Categories .stream() .collect(Collectors.toMap(pair -> pair.getFirst().toString().replace(idPrefix, StringUtils.EMPTY), Pair::getSecond)); } + + /** + * Get path for a given category in human-readable form. + * + * @param category Category to provide path for. + * @return Path for a category in human-readable form. + */ + private String getCategoryPath(final Category category) + { + final NodeRef categoryNodeRef = nodes.getNode(category.getId()).getNodeRef(); + return nodeService.getPath(categoryNodeRef).toDisplayPath(nodeService, permissionService); + } } diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/CategoriesImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/CategoriesImplTest.java index 4d9a48f91c..9b748d1bb5 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/CategoriesImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/CategoriesImplTest.java @@ -28,7 +28,6 @@ package org.alfresco.rest.api.impl; import static org.alfresco.rest.api.Nodes.PATH_ROOT; import static org.alfresco.rest.api.impl.CategoriesImpl.INCLUDE_COUNT_PARAM; -import static org.alfresco.rest.api.impl.CategoriesImpl.INCLUDE_PATH_PARAM; import static org.alfresco.rest.api.impl.CategoriesImpl.INVALID_NODE_TYPE; import static org.alfresco.rest.api.impl.CategoriesImpl.NOT_A_VALID_CATEGORY; import static org.alfresco.rest.api.impl.CategoriesImpl.NOT_NULL_OR_EMPTY; @@ -61,6 +60,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import org.alfresco.model.ContentModel; +import org.alfresco.repo.transfer.PathHelper; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.model.Category; import org.alfresco.rest.api.model.Node; @@ -260,11 +260,16 @@ public class CategoriesImplTest final QName categoryQName = createCmQNameOf(CATEGORY_NAME); final NodeRef parentCategoryNodeRef = createNodeRefWithId(PARENT_ID); final ChildAssociationRef parentAssociation = createAssociationOf(parentCategoryNodeRef, CATEGORY_NODE_REF, categoryQName); - Path mockPath = new Path(); + final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); given(nodesMock.getNode(any())).willReturn(createNode()); given(nodeServiceMock.getPrimaryParent(any())).willReturn(parentAssociation); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); - given(nodeServiceMock.getPath(CATEGORY_NODE_REF)).willReturn(mockPath); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); + given(nodeServiceMock.getPath(any())).willReturn(mockPath); // when final Category actualCategory = objectUnderTest.getCategoryById(CATEGORY_ID, parametersMock); @@ -273,7 +278,7 @@ public class CategoriesImplTest .isNotNull() .extracting(Category::getPath) .isNotNull() - .isEqualTo(""); + .isEqualTo("//" + mockRootLevel + "//" + mockChildLevel); } @Test @@ -511,11 +516,16 @@ public class CategoriesImplTest final NodeRef parentCategoryNodeRef = createNodeRefWithId(PARENT_ID); final ChildAssociationRef parentAssociation = createAssociationOf(parentCategoryNodeRef, CATEGORY_NODE_REF, categoryQName); final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); given(nodesMock.validateNode(PARENT_ID)).willReturn(parentCategoryNodeRef); given(categoryServiceMock.createCategory(parentCategoryNodeRef, CATEGORY_NAME)).willReturn(categoryNodeRef); given(nodesMock.getNode(any())).willReturn(createNode()); given(nodeServiceMock.getPrimaryParent(any())).willReturn(parentAssociation); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); given(nodeServiceMock.getPath(any())).willReturn(mockPath); final List categoryModels = new ArrayList<>(prepareCategories()); @@ -531,7 +541,7 @@ public class CategoriesImplTest .element(0) .extracting(Category::getPath) .isNotNull() - .isEqualTo(""); + .isEqualTo("//" + mockRootLevel + "//" + mockChildLevel); } @Test @@ -692,9 +702,15 @@ public class CategoriesImplTest final int childrenCount = 3; final List childAssociationRefMocks = prepareChildAssocMocks(childrenCount, parentCategoryNodeRef); final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); + final String resultPath = "//" + mockRootLevel + "//" + mockChildLevel; given(nodeServiceMock.getChildAssocs(parentCategoryNodeRef)).willReturn(childAssociationRefMocks); childAssociationRefMocks.forEach(this::prepareCategoryNodeMocks); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); given(nodeServiceMock.getPath(any())).willReturn(mockPath); // when @@ -705,7 +721,7 @@ public class CategoriesImplTest .hasSize(3) .extracting(Category::getPath) .isNotNull() - .isEqualTo(List.of("", "", "")); + .isEqualTo(List.of(resultPath, resultPath, resultPath)); } @Test @@ -841,11 +857,16 @@ public class CategoriesImplTest final NodeRef parentCategoryNodeRef = createNodeRefWithId(PARENT_ID); final ChildAssociationRef parentAssociation = createAssociationOf(parentCategoryNodeRef, CATEGORY_NODE_REF, categoryQName); final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); given(nodesMock.getNode(any())).willReturn(createNode(categoryNewName)); given(nodeServiceMock.getPrimaryParent(any())).willReturn(parentAssociation); given(nodeServiceMock.moveNode(any(), any(), any(), any())).willReturn(createAssociationOf(parentCategoryNodeRef, CATEGORY_NODE_REF, createCmQNameOf(categoryNewName))); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); - given(nodeServiceMock.getPath(CATEGORY_NODE_REF)).willReturn(mockPath); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); + given(nodeServiceMock.getPath(any())).willReturn(mockPath); // when final Category actualCategory = objectUnderTest.updateCategoryById(CATEGORY_ID, fixedCategory, parametersMock); @@ -854,7 +875,7 @@ public class CategoriesImplTest .isNotNull() .extracting(Category::getPath) .isNotNull() - .isEqualTo(""); + .isEqualTo("//" + mockRootLevel + "//" + mockChildLevel); } @Test @@ -1097,10 +1118,15 @@ public class CategoriesImplTest final NodeRef categoryParentNodeRef = createNodeRefWithId(PARENT_ID); final ChildAssociationRef parentAssociation = createAssociationOf(categoryParentNodeRef, CATEGORY_NODE_REF); final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); given(nodesMock.getNode(any())).willReturn(createNode()); given(nodeServiceMock.getPrimaryParent(any())).willReturn(parentAssociation); given(nodeServiceMock.hasAspect(any(), any())).willReturn(true); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); given(nodeServiceMock.getPath(any())).willReturn(mockPath); // when @@ -1116,7 +1142,7 @@ public class CategoriesImplTest then(nodeServiceMock).should().setProperty(CONTENT_NODE_REF, ContentModel.PROP_CATEGORIES, expectedCategories); then(nodeServiceMock).should().getParentAssocs(categoryParentNodeRef); final List expectedLinkedCategories = List.of(CATEGORY); - expectedLinkedCategories.get(0).setPath(""); + expectedLinkedCategories.get(0).setPath("//" + mockRootLevel + "//" + mockChildLevel); assertThat(actualLinkedCategories) .isNotNull().usingRecursiveComparison() .isEqualTo(expectedLinkedCategories); @@ -1318,10 +1344,15 @@ public class CategoriesImplTest final NodeRef categoryParentNodeRef = createNodeRefWithId(PARENT_ID); final ChildAssociationRef parentAssociation = createAssociationOf(categoryParentNodeRef, CATEGORY_NODE_REF); final Path mockPath = new Path(); + final String mockRootLevel = "/{mockRootLevel}"; + final String mockChildLevel = "/{mockChild}"; + mockPath.append(PathHelper.stringToPath(mockRootLevel)); + mockPath.append(PathHelper.stringToPath(mockChildLevel)); + mockPath.append(PathHelper.stringToPath("/")); given(nodeServiceMock.getProperty(any(), eq(ContentModel.PROP_CATEGORIES))).willReturn((Serializable) List.of(CATEGORY_NODE_REF)); given(nodesMock.getNode(any())).willReturn(createNode()); given(nodeServiceMock.getPrimaryParent(any())).willReturn(parentAssociation); - given(parametersMock.getInclude()).willReturn(List.of(INCLUDE_PATH_PARAM)); + given(parametersMock.getInclude()).willReturn(List.of(Nodes.PARAM_INCLUDE_PATH)); given(nodeServiceMock.getPath(any())).willReturn(mockPath); // when @@ -1341,7 +1372,7 @@ public class CategoriesImplTest then(nodeServiceMock).should().getPath(any()); then(nodeServiceMock).shouldHaveNoMoreInteractions(); final List expectedCategories = List.of(CATEGORY); - expectedCategories.get(0).setPath(""); + expectedCategories.get(0).setPath("//" + mockRootLevel + "//" + mockChildLevel); assertThat(actualCategories) .isNotNull().usingRecursiveComparison() .isEqualTo(expectedCategories);