From d51a370e5ac3b188584797521caf446880d6356d Mon Sep 17 00:00:00 2001 From: "suneet.gupta" Date: Thu, 9 Mar 2023 12:21:14 +0530 Subject: [PATCH] Addressed review comments --- .../org/alfresco/rest/api/impl/TagsImpl.java | 9 +--- .../alfresco/rest/api/impl/TagsImplTest.java | 46 +++---------------- 2 files changed, 9 insertions(+), 46 deletions(-) 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 9b2b0debb6..82a037930d 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 @@ -189,12 +189,7 @@ public class TagsImpl implements Tags public NodeRef validateTag(String tagId) { NodeRef tagNodeRef = nodes.validateNode(tagId); - String tagName = taggingService.getTagName(tagNodeRef); - if( tagNodeRef == null || !taggingService.isTag(tagNodeRef.getStoreRef(), tagName)) - { - throw new EntityNotFoundException(tagId); - } - return tagNodeRef; + return validateTag(tagNodeRef.getStoreRef(), tagId); } public NodeRef validateTag(StoreRef storeRef, String tagId) @@ -245,7 +240,7 @@ public class TagsImpl implements Tags public CollectionWithPagingInfo getTags(String nodeId, Parameters params) { - NodeRef nodeRef = validateTag(nodeId); + NodeRef nodeRef = nodes.validateNode(nodeId); if( nodeRef == null ) { throw new EntityNotFoundException(nodeId); 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 c7dfac5b57..092877459c 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 @@ -29,13 +29,11 @@ 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.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; -import static org.jsoup.helper.Validate.fail; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.*; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; -import static org.mockito.Mockito.verifyNoMoreInteractions; import java.util.Collections; import java.util.List; @@ -46,7 +44,6 @@ import org.alfresco.rest.api.model.Node; 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.NotFoundException; import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.rest.framework.resource.parameters.Parameters; import org.alfresco.service.cmr.repository.DuplicateChildNodeNameException; @@ -56,16 +53,11 @@ 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.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.mockito.exceptions.verification.NeverWantedButInvoked; -import org.mockito.exceptions.verification.NoInteractionsWanted; -import org.mockito.internal.verification.NoInteractions; import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) @@ -96,14 +88,9 @@ public class TagsImplTest public void setup() { given(authorityServiceMock.hasAdminAuthority()).willReturn(true); - given(nodesMock.validateNode(NODE_ID)).willReturn(TAG_NODE_REF); given(nodesMock.validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID)).willReturn(TAG_NODE_REF); - given(nodesMock.getNode(any())).willReturn(createdNodeMock); - given(createdNodeMock.getNodeId()).willReturn(NODE_ID); - given(typeConstraint.matches(any())).willReturn(true); given(taggingServiceMock.getTagName(TAG_NODE_REF)).willReturn(TAG_NAME); } - @Test public void testGetTags() { final List tagNames = List.of("testTag","tag11"); @@ -118,32 +105,16 @@ public class TagsImplTest } @Test - public void testAddTagsToNodeWithNotResponseIndexed() + public void testTagNotFoundValidation() { - //when - assertThrows(EntityNotFoundException.class, () -> objectUnderTest.deleteTagById(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID)); - final List tagNames = List.of("tag1","tag2"); - final List tagsToCreate = createTags(tagNames); - given(taggingServiceMock.addTags(any(), any())).willAnswer(invocation -> createTagAndNodeRefPairs(invocation.getArgument(1))); - final List actualCreatedTags = objectUnderTest.addTags(nodesMock.getNode(any()).getNodeId(),tagsToCreate); - then(taggingServiceMock).should().addTags(TAG_NODE_REF, tagNames); - final List expectedTags = createTagsWithNodeRefs(tagNames); - assertThat(actualCreatedTags) - .isNotNull() - .isEqualTo(expectedTags); + assertThrows(EntityNotFoundException.class, () ->objectUnderTest.validateTag(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, "dummy-id")); } @Test - public void testTagValidationNotFound() + public void testTagFoundValidation() { - try { - assertThrows(EntityNotFoundException.class, () ->objectUnderTest.validateTag(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID)); - // fail("It should throw Not Found Exception"); - } catch (EntityNotFoundException e) { - Assertions.assertThat(e) - .isInstanceOf(EntityNotFoundException.class) - .hasMessage("Wrong value is passed."); - } + given(taggingServiceMock.isTag(any(),any())).willReturn(true); + objectUnderTest.validateTag(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); then(nodesMock).should().validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); then(nodesMock).shouldHaveNoMoreInteractions(); } @@ -156,6 +127,7 @@ public class TagsImplTest then(authorityServiceMock).should().hasAdminAuthority(); then(authorityServiceMock).shouldHaveNoMoreInteractions(); + then(nodesMock).should().validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, TAG_ID); then(nodesMock).shouldHaveNoMoreInteractions(); @@ -188,13 +160,9 @@ public class TagsImplTest then(authorityServiceMock).should().hasAdminAuthority(); then(authorityServiceMock).shouldHaveNoMoreInteractions(); + then(nodesMock).should().validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, "dummy-id"); then(nodesMock).shouldHaveNoMoreInteractions(); - try { - then(taggingServiceMock).shouldHaveNoInteractions(); - } catch (NoInteractionsWanted e) { - assertThat("shouldFilterStackTraceOnVerifyNoInteractions"); - } } @Test