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 e78f7856d4..7023d2625b 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 @@ -189,7 +189,7 @@ public class CategoriesImpl implements Categories @Override public List listCategoriesForNode(final String nodeId, final Parameters parameters) { - final NodeRef contentNodeRef = nodes.validateNode(nodeId); + final NodeRef contentNodeRef = nodes.validateOrLookupNode(nodeId, null); verifyReadPermission(contentNodeRef); verifyNodeType(contentNodeRef); @@ -211,7 +211,7 @@ public class CategoriesImpl implements Categories throw new InvalidArgumentException(NOT_A_VALID_CATEGORY); } - final NodeRef contentNodeRef = nodes.validateNode(nodeId); + final NodeRef contentNodeRef = nodes.validateOrLookupNode(nodeId, null); verifyChangePermission(contentNodeRef); verifyNodeType(contentNodeRef); @@ -237,7 +237,7 @@ public class CategoriesImpl implements Categories public void unlinkNodeFromCategory(final StoreRef storeRef, final String nodeId, final String categoryId, final Parameters parameters) { final NodeRef categoryNodeRef = getCategoryNodeRef(storeRef, categoryId); - final NodeRef contentNodeRef = nodes.validateNode(nodeId); + final NodeRef contentNodeRef = nodes.validateOrLookupNode(nodeId, null); verifyChangePermission(contentNodeRef); verifyNodeType(contentNodeRef); 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..d703b86262 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 @@ -25,11 +25,12 @@ */ package org.alfresco.rest.api.impl; +import static java.util.stream.Collectors.toList; + import static org.alfresco.rest.antlr.WhereClauseParser.EQUALS; import static org.alfresco.rest.antlr.WhereClauseParser.IN; import static org.alfresco.rest.antlr.WhereClauseParser.MATCHES; -import java.util.AbstractList; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -114,44 +115,30 @@ public class TagsImpl implements Tags this.authorityService = authorityService; } - public List addTags(String nodeId, final List tags) - { - NodeRef nodeRef = nodes.validateNode(nodeId); - if(!typeConstraint.matches(nodeRef)) - { - throw new UnsupportedResourceOperationException("Cannot tag this node"); - } + public List addTags(String nodeId, final List tags) + { + NodeRef nodeRef = nodes.validateOrLookupNode(nodeId, null); + if (!typeConstraint.matches(nodeRef)) + { + throw new UnsupportedResourceOperationException("Cannot tag this node"); + } - List tagValues = new AbstractList() + List tagValues = tags.stream().map(Tag::getTag).collect(toList()); + try + { + List> tagNodeRefs = taggingService.addTags(nodeRef, tagValues); + List ret = new ArrayList<>(tags.size()); + for (Pair pair : tagNodeRefs) { - @Override - public String get(int arg0) - { - String tag = tags.get(arg0).getTag(); - return tag; - } - - @Override - public int size() - { - return tags.size(); - } - }; - try - { - List> tagNodeRefs = taggingService.addTags(nodeRef, tagValues); - List ret = new ArrayList(tags.size()); - for(Pair pair : tagNodeRefs) - { - ret.add(new Tag(pair.getSecond(), pair.getFirst())); - } - return ret; + ret.add(new Tag(pair.getSecond(), pair.getFirst())); } - catch(IllegalArgumentException e) - { - throw new InvalidArgumentException(e.getMessage()); - } - } + return ret; + } + catch (IllegalArgumentException e) + { + throw new InvalidArgumentException(e.getMessage()); + } + } public void deleteTag(String nodeId, String tagId) { @@ -254,17 +241,17 @@ public class TagsImpl implements Tags public CollectionWithPagingInfo getTags(String nodeId, Parameters params) { - NodeRef nodeRef = nodes.validateNode(nodeId); - PagingResults> results = taggingService.getTags(nodeRef, Util.getPagingRequest(params.getPaging())); - Integer totalItems = results.getTotalResultCount().getFirst(); - List> page = results.getPage(); - List tags = new ArrayList(page.size()); - for(Pair pair : page) - { - tags.add(new Tag(pair.getFirst(), pair.getSecond())); - } + NodeRef nodeRef = nodes.validateOrLookupNode(nodeId, null); + PagingResults> results = taggingService.getTags(nodeRef, Util.getPagingRequest(params.getPaging())); + Integer totalItems = results.getTotalResultCount().getFirst(); + List> page = results.getPage(); + List tags = new ArrayList<>(page.size()); + for(Pair pair : page) + { + tags.add(new Tag(pair.getFirst(), pair.getSecond())); + } - return CollectionWithPagingInfo.asPaged(params.getPaging(), tags, results.hasMoreItems(), (totalItems == null ? null : totalItems.intValue())); + return CollectionWithPagingInfo.asPaged(params.getPaging(), tags, results.hasMoreItems(), (totalItems == null ? null : totalItems.intValue())); } @Experimental @@ -276,7 +263,7 @@ public class TagsImpl implements Tags .filter(Objects::nonNull) .map(Tag::getTag) .distinct() - .collect(Collectors.toList()); + .collect(toList()); if (CollectionUtils.isEmpty(tagNames)) { @@ -290,7 +277,7 @@ public class TagsImpl implements Tags { tag.setCount(0); } - }).collect(Collectors.toList()); + }).collect(toList()); } private void verifyAdminAuthority() 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 575f0cc0dc..1c7445cb6d 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 @@ -128,7 +128,7 @@ public class CategoriesImplTest { given(authorityServiceMock.hasAdminAuthority()).willReturn(true); given(nodesMock.validateNode(CATEGORY_ID)).willReturn(CATEGORY_NODE_REF); - given(nodesMock.validateNode(CONTENT_NODE_ID)).willReturn(CONTENT_NODE_REF); + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willReturn(CONTENT_NODE_REF); given(nodesMock.isSubClass(any(), any(), anyBoolean())).willReturn(true); given(typeConstraint.matches(any())).willReturn(true); given(permissionServiceMock.hasReadPermission(any())).willReturn(AccessStatus.ALLOWED); @@ -900,7 +900,7 @@ public class CategoriesImplTest // when final List actualLinkedCategories = objectUnderTest.linkNodeToCategories(CONTENT_NODE_ID, categoryLinks, parametersMock); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).should().hasPermission(CONTENT_NODE_REF, PermissionService.CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(typeConstraint).should().matches(CONTENT_NODE_REF); @@ -1011,12 +1011,12 @@ public class CategoriesImplTest @Test public void testLinkNodeToCategories_withInvalidNodeId() { - given(nodesMock.validateNode(CONTENT_NODE_ID)).willThrow(EntityNotFoundException.class); + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willThrow(EntityNotFoundException.class); // when final Throwable actualException = catchThrowable(() -> objectUnderTest.linkNodeToCategories(CONTENT_NODE_ID, List.of(CATEGORY), parametersMock)); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).shouldHaveNoInteractions(); then(nodeServiceMock).shouldHaveNoInteractions(); assertThat(actualException) @@ -1031,7 +1031,7 @@ public class CategoriesImplTest // when final Throwable actualException = catchThrowable(() -> objectUnderTest.linkNodeToCategories(CONTENT_NODE_ID, List.of(CATEGORY), parametersMock)); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).should().hasPermission(CONTENT_NODE_REF, PermissionService.CHANGE_PERMISSIONS); then(nodeServiceMock).shouldHaveNoInteractions(); assertThat(actualException) @@ -1118,7 +1118,7 @@ public class CategoriesImplTest objectUnderTest.unlinkNodeFromCategory(CONTENT_NODE_ID, CATEGORY_ID, parametersMock); then(nodesMock).should().validateNode(CATEGORY_ID); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).should().hasPermission(CONTENT_NODE_REF, PermissionService.CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(typeConstraint).should().matches(CONTENT_NODE_REF); @@ -1155,7 +1155,7 @@ public class CategoriesImplTest // when final List actualCategories = objectUnderTest.listCategoriesForNode(CONTENT_NODE_ID, parametersMock); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).should().hasReadPermission(CONTENT_NODE_REF); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(typeConstraint).should().matches(CONTENT_NODE_REF); @@ -1176,12 +1176,12 @@ public class CategoriesImplTest @Test public void testListCategoriesForNode_withInvalidNodeId() { - given(nodesMock.validateNode(CONTENT_NODE_ID)).willThrow(EntityNotFoundException.class); + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willThrow(EntityNotFoundException.class); // when final Throwable actualException = catchThrowable(() -> objectUnderTest.listCategoriesForNode(CONTENT_NODE_ID, parametersMock)); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(nodeServiceMock).shouldHaveNoInteractions(); assertThat(actualException) .isInstanceOf(EntityNotFoundException.class); @@ -1195,7 +1195,7 @@ public class CategoriesImplTest // when final Throwable actualException = catchThrowable(() -> objectUnderTest.listCategoriesForNode(CONTENT_NODE_ID, parametersMock)); - then(nodesMock).should().validateNode(CONTENT_NODE_ID); + then(nodesMock).should().validateOrLookupNode(CONTENT_NODE_ID, null); then(permissionServiceMock).should().hasReadPermission(CONTENT_NODE_REF); then(nodeServiceMock).shouldHaveNoInteractions(); assertThat(actualException) 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..f874db09e2 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 @@ -25,6 +25,8 @@ */ package org.alfresco.rest.api.impl; +import static java.util.stream.Collectors.toList; + import static org.alfresco.rest.api.impl.TagsImpl.NOT_A_VALID_TAG; import static org.alfresco.rest.api.impl.TagsImpl.NO_PERMISSION_TO_MANAGE_A_TAG; import static org.alfresco.service.cmr.repository.StoreRef.STORE_REF_WORKSPACE_SPACESSTORE; @@ -41,7 +43,6 @@ import static org.mockito.BDDMockito.then; import java.util.Collections; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import org.alfresco.query.PagingRequest; import org.alfresco.query.PagingResults; @@ -50,6 +51,7 @@ import org.alfresco.rest.api.model.Tag; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; +import org.alfresco.rest.framework.core.exceptions.UnsupportedResourceOperationException; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; import org.alfresco.rest.framework.resource.parameters.Paging; import org.alfresco.rest.framework.resource.parameters.Parameters; @@ -63,6 +65,7 @@ import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.tagging.TaggingService; import org.alfresco.util.Pair; +import org.alfresco.util.TypeConstraint; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -77,7 +80,9 @@ public class TagsImplTest 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 static final NodeRef TAG_PARENT_NODE_REF = new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, PARENT_NODE_ID); + private static final String CONTENT_NODE_ID = "content-node-id"; + private static final NodeRef CONTENT_NODE_REF = new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, CONTENT_NODE_ID); private final RecognizedParamsExtractor queryExtractor = new RecognizedParamsExtractor() {}; @@ -97,6 +102,8 @@ public class TagsImplTest private Paging pagingMock; @Mock private PagingResults> pagingResultsMock; + @Mock + private TypeConstraint typeConstraintMock; @InjectMocks private TagsImpl objectUnderTest; @@ -122,7 +129,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().peek(tag -> tag.setCount(0)).collect(toList()); assertEquals(expectedTags, actualTags.getCollection()); } @@ -140,7 +147,7 @@ public class TagsImplTest then(taggingServiceMock).should().findTaggedNodesAndCountByTagName(STORE_REF_WORKSPACE_SPACESSTORE); final List expectedTags = createTagsWithNodeRefs(List.of(TAG_NAME)).stream() .peek(tag -> tag.setCount(0)) - .collect(Collectors.toList()); + .collect(toList()); assertEquals(expectedTags, actualTags.getCollection()); } @@ -395,7 +402,7 @@ public class TagsImplTest final List expectedTags = createTagsWithNodeRefs(tagNames).stream() .peek(tag -> tag.setCount(0)) - .collect(Collectors.toList()); + .collect(toList()); assertThat(actualCreatedTags) .isNotNull() .isEqualTo(expectedTags); @@ -405,18 +412,78 @@ public class TagsImplTest public void testGetTagByIdNotFoundValidation() { given(primaryParentMock.getParentRef()).willReturn(TAG_NODE_REF); - objectUnderTest.getTag(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE,TAG_ID); + objectUnderTest.getTag(STORE_REF_WORKSPACE_SPACESSTORE,TAG_ID); then(nodeServiceMock).shouldHaveNoInteractions(); - then(nodesMock).should().validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); + then(nodesMock).should().validateNode(STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); then(nodesMock).shouldHaveNoMoreInteractions(); then(taggingServiceMock).shouldHaveNoInteractions(); } + @Test + public void testAddTags() + { + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willReturn(CONTENT_NODE_REF); + given(typeConstraintMock.matches(CONTENT_NODE_REF)).willReturn(true); + List> pairs = List.of(new Pair<>("tagA", new NodeRef("tag://A/")), new Pair<>("tagB", new NodeRef("tag://B/"))); + List tagNames = pairs.stream().map(Pair::getFirst).collect(toList()); + List tags = tagNames.stream().map(name -> Tag.builder().tag(name).create()).collect(toList()); + given(taggingServiceMock.addTags(CONTENT_NODE_REF, tagNames)).willReturn(pairs); + + List actual = objectUnderTest.addTags(CONTENT_NODE_ID, tags); + + List expected = pairs.stream().map(pair -> new Tag(pair.getSecond(), pair.getFirst())).collect(toList()); + assertEquals("Unexpected tags returned.", expected, actual); + } + + @Test(expected = InvalidArgumentException.class) + public void testAddTagsToInvalidNode() + { + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willThrow(new InvalidArgumentException()); + List tags = List.of(Tag.builder().tag("tag1").create()); + + objectUnderTest.addTags(CONTENT_NODE_ID, tags); + } + + @Test(expected = UnsupportedResourceOperationException.class) + public void testAddTagsToWrongTypeOfNode() + { + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willReturn(CONTENT_NODE_REF); + given(typeConstraintMock.matches(CONTENT_NODE_REF)).willReturn(false); + + List tags = List.of(Tag.builder().tag("tag1").create()); + + objectUnderTest.addTags(CONTENT_NODE_ID, tags); + } + + @Test + public void testGetTagsForNode() + { + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willReturn(CONTENT_NODE_REF); + given(parametersMock.getPaging()).willReturn(pagingMock); + List> pairs = List.of(new Pair<>(new NodeRef("tag://A/"), "tagA"), new Pair<>(new NodeRef("tag://B/"), "tagB")); + given(taggingServiceMock.getTags(eq(CONTENT_NODE_REF), any(PagingRequest.class))).willReturn(pagingResultsMock); + given(pagingResultsMock.getTotalResultCount()).willReturn(new Pair<>(null, null)); + given(pagingResultsMock.getPage()).willReturn(pairs); + + CollectionWithPagingInfo actual = objectUnderTest.getTags(CONTENT_NODE_ID, parametersMock); + + List tags = pairs.stream().map(pair -> Tag.builder().tag(pair.getSecond()).nodeRef(pair.getFirst()).create()).collect(toList()); + assertEquals(actual.getCollection(), tags); + } + + @Test (expected = InvalidArgumentException.class) + public void testGetTagsFromInvalidNode() + { + given(nodesMock.validateOrLookupNode(CONTENT_NODE_ID, null)).willThrow(new InvalidArgumentException()); + + objectUnderTest.getTags(CONTENT_NODE_ID, parametersMock); + } + private static List> createTagAndNodeRefPairs(final List tagNames) { return tagNames.stream() .map(tagName -> createPair(tagName, new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID.concat("-").concat(tagName)))) - .collect(Collectors.toList()); + .collect(toList()); } private static Pair createPair(final String tagName, final NodeRef nodeRef) @@ -426,12 +493,12 @@ public class TagsImplTest private static List createTags(final List tagNames) { - return tagNames.stream().map(TagsImplTest::createTag).collect(Collectors.toList()); + return tagNames.stream().map(TagsImplTest::createTag).collect(toList()); } private static List createTagsWithNodeRefs(final List tagNames) { - return tagNames.stream().map(TagsImplTest::createTagWithNodeRef).collect(Collectors.toList()); + return tagNames.stream().map(TagsImplTest::createTagWithNodeRef).collect(toList()); } private static Tag createTag(final String tagName)