From 2c3845bf9d08f2a5f7ec491c2e96e9b5fc207f0b Mon Sep 17 00:00:00 2001 From: George Evangelopoulos Date: Fri, 21 Apr 2023 15:05:21 +0100 Subject: [PATCH] ACS-4025: Throw 400 error when ordering by tag count without including tag count (#1896) * ACS-4025: change exception to throw 400 and add test --- .../org/alfresco/rest/tags/GetTagsTests.java | 35 +- .../org/alfresco/rest/api/impl/TagsImpl.java | 348 +++++++++--------- .../repo/tagging/TaggingServiceImpl.java | 198 +++++----- 3 files changed, 295 insertions(+), 286 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 2648289e60..6f86c78cf2 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 @@ -2,6 +2,7 @@ package org.alfresco.rest.tags; import static org.alfresco.utility.data.RandomData.getRandomName; import static org.alfresco.utility.report.log.Step.STEP; +import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.OK; import java.util.Set; @@ -88,7 +89,7 @@ public class GetTagsTests extends TagsDataPrep .withParams("include=count") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.getEntries().stream() .filter(e -> e.onModel().getTag().equals(folderTagValue) || e.onModel().getTag().equals(documentTagValue)) @@ -105,14 +106,13 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByCountDefaultOrderShouldBeAsc() { - STEP("Get tags and order results by count. Default sort order should be ascending"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("include=count&orderBy=count") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedAscBy("count"); } @@ -122,14 +122,13 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByCountAsc() { - STEP("Get tags and order results by count in ascending order"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("include=count&orderBy=count ASC") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedAscBy("count"); } @@ -139,14 +138,13 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByCountDesc() { - STEP("Get tags and order results by count in descending order"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("include=count&orderBy=count DESC") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedDescBy("count"); } @@ -156,14 +154,13 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByTagDefaultOrderShouldBeAsc() { - STEP("Get tags and order results by tag name. Default sort order should be ascending"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("orderBy=tag") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedAscBy("tag"); } @@ -173,14 +170,13 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByTagAsc() { - STEP("Get tags and order results by tag name in ascending order"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("orderBy=tag ASC") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedAscBy("tag"); } @@ -190,17 +186,30 @@ public class GetTagsTests extends TagsDataPrep @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) public void testGetTags_withOrderByTagDesc() { - STEP("Get tags and order results by tag name in descending order"); returnedCollection = restClient.authenticateUser(adminUserModel) .withParams("orderBy=tag DESC") .withCoreAPI() .getTags(); - restClient.assertStatusCodeIs(HttpStatus.OK); + restClient.assertStatusCodeIs(OK); returnedCollection.assertThat().entriesListIsSortedDescBy("tag"); } + /** + * Ensure that we get a 400 error when we request to order by count without also including the tag count. + */ + @Test(groups = { TestGroup.REST_API, TestGroup.TAGS, TestGroup.REGRESSION }) + public void testGetTags_orderByCountWithoutIncludeCount() + { + restClient.authenticateUser(adminUserModel) + .withParams("orderBy=count") + .withCoreAPI() + .getTags(); + + restClient.assertStatusCodeIs(BAD_REQUEST); + } + @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") 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 ad9992cf09..b0240b23d7 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 @@ -86,40 +86,40 @@ import org.apache.commons.collections.CollectionUtils; */ public class TagsImpl implements Tags { - public static final String PARAM_INCLUDE_COUNT = "count"; - private static final String PARAM_WHERE_TAG = "tag"; - static final String NOT_A_VALID_TAG = "An invalid parameter has been supplied"; - static final String NO_PERMISSION_TO_MANAGE_A_TAG = "Current user does not have permission to manage a tag"; + public static final String PARAM_INCLUDE_COUNT = "count"; + private static final String PARAM_WHERE_TAG = "tag"; + static final String NOT_A_VALID_TAG = "An invalid parameter has been supplied"; + static final String NO_PERMISSION_TO_MANAGE_A_TAG = "Current user does not have permission to manage a tag"; private Nodes nodes; - private NodeService nodeService; - private TaggingService taggingService; - private TypeConstraint typeConstraint; - private AuthorityService authorityService; + private NodeService nodeService; + private TaggingService taggingService; + private TypeConstraint typeConstraint; + private AuthorityService authorityService; - public void setTypeConstraint(TypeConstraint typeConstraint) - { - this.typeConstraint = typeConstraint; - } - - public void setNodes(Nodes nodes) + public void setTypeConstraint(TypeConstraint typeConstraint) { - this.nodes = nodes; - } - public void setNodeService(NodeService nodeService) - { - this.nodeService = nodeService; - } - + this.typeConstraint = typeConstraint; + } + + public void setNodes(Nodes nodes) + { + this.nodes = nodes; + } + public void setNodeService(NodeService nodeService) + { + this.nodeService = nodeService; + } + public void setTaggingService(TaggingService taggingService) { - this.taggingService = taggingService; - } + this.taggingService = taggingService; + } - public void setAuthorityService(AuthorityService authorityService) - { - this.authorityService = authorityService; - } + public void setAuthorityService(AuthorityService authorityService) + { + this.authorityService = authorityService; + } public List addTags(String nodeId, final List tags, final Parameters parameters) { @@ -134,15 +134,15 @@ public class TagsImpl implements Tags { List> tagNodeRefs = taggingService.addTags(nodeRef, tagValues); List ret = new ArrayList<>(tags.size()); - List> tagsCountPairList = taggingService.findTaggedNodesAndCountByTagName(nodeRef.getStoreRef()); - Map tagsCountMap = tagsCountPairList.stream().collect(Collectors.toMap(Pair::getFirst, pair -> Long.valueOf(pair.getSecond()))); + List> tagsCountPairList = taggingService.findTaggedNodesAndCountByTagName(nodeRef.getStoreRef()); + Map tagsCountMap = tagsCountPairList.stream().collect(Collectors.toMap(Pair::getFirst, pair -> Long.valueOf(pair.getSecond()))); for (Pair pair : tagNodeRefs) { - Tag createdTag = new Tag(pair.getSecond(), pair.getFirst()); - if (parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) - { - createdTag.setCount(Optional.ofNullable(tagsCountMap.get(createdTag.getTag())).orElse(0L) + 1); - } + Tag createdTag = new Tag(pair.getSecond(), pair.getFirst()); + if (parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) + { + createdTag.setCount(Optional.ofNullable(tagsCountMap.get(createdTag.getTag())).orElse(0L) + 1); + } ret.add(createdTag); } return ret; @@ -152,109 +152,109 @@ public class TagsImpl implements Tags throw new InvalidArgumentException(e.getMessage()); } } - + public void deleteTag(String nodeId, String tagId) { - NodeRef nodeRef = nodes.validateNode(nodeId); - getTag(STORE_REF_WORKSPACE_SPACESSTORE, tagId, null); - NodeRef existingTagNodeRef = validateTag(tagId); - String tagValue = taggingService.getTagName(existingTagNodeRef); - taggingService.removeTag(nodeRef, tagValue); + NodeRef nodeRef = nodes.validateNode(nodeId); + getTag(STORE_REF_WORKSPACE_SPACESSTORE, tagId, null); + NodeRef existingTagNodeRef = validateTag(tagId); + String tagValue = taggingService.getTagName(existingTagNodeRef); + taggingService.removeTag(nodeRef, tagValue); } @Override - public void deleteTagById(StoreRef storeRef, String tagId) { - verifyAdminAuthority(); + public void deleteTagById(StoreRef storeRef, String tagId) { + verifyAdminAuthority(); - NodeRef tagNodeRef = validateTag(storeRef, tagId); - String tagValue = taggingService.getTagName(tagNodeRef); - taggingService.deleteTag(storeRef, tagValue); - } + NodeRef tagNodeRef = validateTag(storeRef, tagId); + String tagValue = taggingService.getTagName(tagNodeRef); + taggingService.deleteTag(storeRef, tagValue); + } - @Override + @Override public CollectionWithPagingInfo getTags(StoreRef storeRef, Parameters params) { - Paging paging = params.getPaging(); - Pair sorting = !params.getSorting().isEmpty() ? new Pair<>(params.getSorting().get(0).column, params.getSorting().get(0).asc) : null; - Map> namesFilters = resolveTagNamesQuery(params.getQuery()); + Paging paging = params.getPaging(); + Pair sorting = !params.getSorting().isEmpty() ? new Pair<>(params.getSorting().get(0).column, params.getSorting().get(0).asc) : null; + Map> namesFilters = resolveTagNamesQuery(params.getQuery()); - Map results = taggingService.getTags(storeRef, params.getInclude(), sorting, namesFilters.get(EQUALS), namesFilters.get(MATCHES)); + Map results = taggingService.getTags(storeRef, params.getInclude(), sorting, namesFilters.get(EQUALS), namesFilters.get(MATCHES)); - List tagsList = results.entrySet().stream().map(entry -> new Tag(entry.getKey(), (String)nodeService.getProperty(entry.getKey(), ContentModel.PROP_NAME))).collect(Collectors.toList()); + List tagsList = results.entrySet().stream().map(entry -> new Tag(entry.getKey(), (String)nodeService.getProperty(entry.getKey(), ContentModel.PROP_NAME))).collect(Collectors.toList()); - if (params.getInclude().contains(PARAM_INCLUDE_COUNT)) - { - tagsList.forEach(tag -> tag.setCount(results.get(tag.getNodeRef()))); - } + if (params.getInclude().contains(PARAM_INCLUDE_COUNT)) + { + tagsList.forEach(tag -> tag.setCount(results.get(tag.getNodeRef()))); + } - ListBackedPagingResults listBackedPagingResults = new ListBackedPagingResults(tagsList, Util.getPagingRequest(params.getPaging())); + ListBackedPagingResults listBackedPagingResults = new ListBackedPagingResults(tagsList, Util.getPagingRequest(params.getPaging())); - return CollectionWithPagingInfo.asPaged(paging, listBackedPagingResults.getPage(), listBackedPagingResults.hasMoreItems(), (Integer) listBackedPagingResults.getTotalResultCount().getFirst()); + return CollectionWithPagingInfo.asPaged(paging, listBackedPagingResults.getPage(), listBackedPagingResults.hasMoreItems(), (Integer) listBackedPagingResults.getTotalResultCount().getFirst()); } public NodeRef validateTag(String tagId) { - NodeRef tagNodeRef = nodes.validateNode(tagId); - return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); + NodeRef tagNodeRef = nodes.validateNode(tagId); + return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); } public NodeRef validateTag(StoreRef storeRef, String tagId) { - NodeRef tagNodeRef = nodes.validateNode(storeRef, tagId); - return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); + NodeRef tagNodeRef = nodes.validateNode(storeRef, tagId); + return checkTagRootAsNodePrimaryParent(tagId, tagNodeRef); } - /** - * Find the number of times the given tag is used (if requested). - * - * @param storeRef The store the tag is in. - * @param tagName The name of the tag. - * @param parameters The request parameters object containing the includes parameter. - * @return The number of times the tag is applied, or null if "count" wasn't in the include parameter. - */ - private Long findCountIfRequested(StoreRef storeRef, String tagName, Parameters parameters) - { - Long count = null; - if (parameters != null && parameters.getInclude() != null && parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) - { - count = taggingService.findCountByTagName(storeRef, tagName); - } - return count; - } + /** + * Find the number of times the given tag is used (if requested). + * + * @param storeRef The store the tag is in. + * @param tagName The name of the tag. + * @param parameters The request parameters object containing the includes parameter. + * @return The number of times the tag is applied, or null if "count" wasn't in the include parameter. + */ + private Long findCountIfRequested(StoreRef storeRef, String tagName, Parameters parameters) + { + Long count = null; + if (parameters != null && parameters.getInclude() != null && parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) + { + count = taggingService.findCountByTagName(storeRef, tagName); + } + return count; + } @Override public Tag changeTag(StoreRef storeRef, String tagId, Tag tag, Parameters parameters) { - try - { - NodeRef existingTagNodeRef = validateTag(storeRef, tagId); - String existingTagName = taggingService.getTagName(existingTagNodeRef); - Long count = findCountIfRequested(storeRef, existingTagName, parameters); - String newTagName = tag.getTag(); - NodeRef newTagNodeRef = taggingService.changeTag(storeRef, existingTagName, newTagName); - return Tag.builder().nodeRef(newTagNodeRef).tag(newTagName).count(count).create(); - } - catch(NonExistentTagException e) - { - throw new NotFoundException(e.getMessage()); - } - catch(TagExistsException e) - { - throw new ConstraintViolatedException(e.getMessage()); - } - catch(TaggingException e) - { - throw new InvalidArgumentException(e.getMessage()); - } + try + { + NodeRef existingTagNodeRef = validateTag(storeRef, tagId); + String existingTagName = taggingService.getTagName(existingTagNodeRef); + Long count = findCountIfRequested(storeRef, existingTagName, parameters); + String newTagName = tag.getTag(); + NodeRef newTagNodeRef = taggingService.changeTag(storeRef, existingTagName, newTagName); + return Tag.builder().nodeRef(newTagNodeRef).tag(newTagName).count(count).create(); + } + catch(NonExistentTagException e) + { + throw new NotFoundException(e.getMessage()); + } + catch(TagExistsException e) + { + throw new ConstraintViolatedException(e.getMessage()); + } + catch(TaggingException e) + { + throw new InvalidArgumentException(e.getMessage()); + } } - @Override + @Override public Tag getTag(StoreRef storeRef, String tagId, Parameters parameters) { - NodeRef tagNodeRef = validateTag(storeRef, tagId); - String tagName = taggingService.getTagName(tagNodeRef); - Long count = findCountIfRequested(storeRef, tagName, parameters); - return Tag.builder().nodeRef(tagNodeRef).tag(tagName).count(count).create(); + NodeRef tagNodeRef = validateTag(storeRef, tagId); + String tagName = taggingService.getTagName(tagNodeRef); + Long count = findCountIfRequested(storeRef, tagName, parameters); + return Tag.builder().nodeRef(tagNodeRef).tag(tagName).count(count).create(); } @Override @@ -273,80 +273,80 @@ public class TagsImpl implements Tags return CollectionWithPagingInfo.asPaged(params.getPaging(), tags, results.hasMoreItems(), (totalItems == null ? null : totalItems.intValue())); } - @Experimental - @Override - public List createTags(final StoreRef storeRef, final List tags, final Parameters parameters) - { - verifyAdminAuthority(); - final List tagNames = Optional.ofNullable(tags).orElse(Collections.emptyList()).stream() - .filter(Objects::nonNull) - .map(Tag::getTag) - .distinct() - .collect(toList()); + @Experimental + @Override + public List createTags(final StoreRef storeRef, final List tags, final Parameters parameters) + { + verifyAdminAuthority(); + final List tagNames = Optional.ofNullable(tags).orElse(Collections.emptyList()).stream() + .filter(Objects::nonNull) + .map(Tag::getTag) + .distinct() + .collect(toList()); - if (CollectionUtils.isEmpty(tagNames)) - { - throw new InvalidArgumentException(NOT_A_VALID_TAG); - } + if (CollectionUtils.isEmpty(tagNames)) + { + throw new InvalidArgumentException(NOT_A_VALID_TAG); + } - return taggingService.createTags(storeRef, tagNames).stream() - .map(pair -> Tag.builder().tag(pair.getFirst()).nodeRef(pair.getSecond()).create()) - .peek(tag -> { - if (parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) - { - tag.setCount(0L); - } - }).collect(toList()); - } + return taggingService.createTags(storeRef, tagNames).stream() + .map(pair -> Tag.builder().tag(pair.getFirst()).nodeRef(pair.getSecond()).create()) + .peek(tag -> { + if (parameters.getInclude().contains(PARAM_INCLUDE_COUNT)) + { + tag.setCount(0L); + } + }).collect(toList()); + } - private void verifyAdminAuthority() - { - if (!authorityService.hasAdminAuthority()) - { - throw new PermissionDeniedException(NO_PERMISSION_TO_MANAGE_A_TAG); - } - } + private void verifyAdminAuthority() + { + if (!authorityService.hasAdminAuthority()) + { + throw new PermissionDeniedException(NO_PERMISSION_TO_MANAGE_A_TAG); + } + } - /** - * Method resolves where query looking for clauses: EQUALS, IN or MATCHES. - * Expected values for EQUALS and IN will be merged under EQUALS clause. - * @param namesQuery Where query with expected tag name(s). - * @return Map of expected exact and alike tag names. - */ - private Map> resolveTagNamesQuery(final Query namesQuery) - { - if (namesQuery == null || namesQuery == QueryImpl.EMPTY) - { - return Collections.emptyMap(); - } + /** + * Method resolves where query looking for clauses: EQUALS, IN or MATCHES. + * Expected values for EQUALS and IN will be merged under EQUALS clause. + * @param namesQuery Where query with expected tag name(s). + * @return Map of expected exact and alike tag names. + */ + private Map> resolveTagNamesQuery(final Query namesQuery) + { + if (namesQuery == null || namesQuery == QueryImpl.EMPTY) + { + return Collections.emptyMap(); + } - final Map> properties = QueryHelper - .resolve(namesQuery) - .usingOrOperator() - .withoutNegations() - .getProperty(PARAM_WHERE_TAG) - .getExpectedValuesForAnyOf(EQUALS, IN, MATCHES) - .skipNegated(); + final Map> properties = QueryHelper + .resolve(namesQuery) + .usingOrOperator() + .withoutNegations() + .getProperty(PARAM_WHERE_TAG) + .getExpectedValuesForAnyOf(EQUALS, IN, MATCHES) + .skipNegated(); - return properties.entrySet().stream() - .collect(Collectors.groupingBy((entry) -> { - if (entry.getKey() == EQUALS || entry.getKey() == IN) - { - return EQUALS; - } - else - { - return MATCHES; - } - }, Collectors.flatMapping((entry) -> entry.getValue().stream().map(String::toLowerCase), Collectors.toCollection(HashSet::new)))); - } + return properties.entrySet().stream() + .collect(Collectors.groupingBy((entry) -> { + if (entry.getKey() == EQUALS || entry.getKey() == IN) + { + return EQUALS; + } + else + { + return MATCHES; + } + }, Collectors.flatMapping((entry) -> entry.getValue().stream().map(String::toLowerCase), Collectors.toCollection(HashSet::new)))); + } - private NodeRef checkTagRootAsNodePrimaryParent(String tagId, NodeRef tagNodeRef) - { - if ( tagNodeRef == null || !nodeService.getPrimaryParent(tagNodeRef).getParentRef().equals(TAG_ROOT_NODE_REF)) - { - throw new EntityNotFoundException(tagId); - } - return tagNodeRef; - } + private NodeRef checkTagRootAsNodePrimaryParent(String tagId, NodeRef tagNodeRef) + { + if ( tagNodeRef == null || !nodeService.getPrimaryParent(tagNodeRef).getParentRef().equals(TAG_ROOT_NODE_REF)) + { + throw new EntityNotFoundException(tagId); + } + return tagNodeRef; + } } diff --git a/repository/src/main/java/org/alfresco/repo/tagging/TaggingServiceImpl.java b/repository/src/main/java/org/alfresco/repo/tagging/TaggingServiceImpl.java index 5ff3b6ac45..6e75db01db 100644 --- a/repository/src/main/java/org/alfresco/repo/tagging/TaggingServiceImpl.java +++ b/repository/src/main/java/org/alfresco/repo/tagging/TaggingServiceImpl.java @@ -126,7 +126,7 @@ public class TaggingServiceImpl implements TaggingService, private static Log logger = LogFactory.getLog(TaggingServiceImpl.class); - private static Collator collator = Collator.getInstance(); + private static Collator collator = Collator.getInstance(); private NodeService nodeService; private NodeService nodeServiceInternal; @@ -502,8 +502,8 @@ public class TaggingServiceImpl implements TaggingService, { // Lower the case of the tag tag = tag.toLowerCase(); - - return getTagNodeRef(storeRef, tag, true); + + return getTagNodeRef(storeRef, tag, true); } /** @@ -532,33 +532,33 @@ public class TaggingServiceImpl implements TaggingService, public NodeRef changeTag(StoreRef storeRef, String existingTag, String newTag) { - if (existingTag == null) - { - throw new TaggingException("Existing tag cannot be null"); - } - - if (newTag == null || StringUtils.isBlank(newTag)) - { - throw new TaggingException("New tag cannot be blank"); - } + if (existingTag == null) + { + throw new TaggingException("Existing tag cannot be null"); + } - existingTag = existingTag.toLowerCase(); - newTag = newTag.toLowerCase(); + if (newTag == null || StringUtils.isBlank(newTag)) + { + throw new TaggingException("New tag cannot be blank"); + } - if (existingTag.equals(newTag)) - { - throw new TaggingException("New and existing tags are the same"); - } - - if (getTagNodeRef(storeRef, existingTag) == null) - { - throw new NonExistentTagException("Tag " + existingTag + " not found"); - } - - if (getTagNodeRef(storeRef, newTag) != null) - { - throw new TagExistsException("Tag " + newTag + " already exists"); - } + existingTag = existingTag.toLowerCase(); + newTag = newTag.toLowerCase(); + + if (existingTag.equals(newTag)) + { + throw new TaggingException("New and existing tags are the same"); + } + + if (getTagNodeRef(storeRef, existingTag) == null) + { + throw new NonExistentTagException("Tag " + existingTag + " not found"); + } + + if (getTagNodeRef(storeRef, newTag) != null) + { + throw new TagExistsException("Tag " + newTag + " already exists"); + } NodeRef tagNodeRef = getTagNodeRef(storeRef, existingTag); nodeService.setProperty(tagNodeRef, PROP_NAME, newTag); @@ -718,12 +718,12 @@ public class TaggingServiceImpl implements TaggingService, @SuppressWarnings("unchecked") public NodeRef addTag(final NodeRef nodeRef, final String tagName) { - NodeRef newTagNodeRef = null; - - if(tagName == null) - { - throw new IllegalArgumentException("Must provide a non-null tag"); - } + NodeRef newTagNodeRef = null; + + if(tagName == null) + { + throw new IllegalArgumentException("Must provide a non-null tag"); + } updateTagBehaviour.disable(); createTagBehaviour.disable(); @@ -773,7 +773,7 @@ public class TaggingServiceImpl implements TaggingService, */ public List> addTags(NodeRef nodeRef, List tags) { - List> ret = new ArrayList>(); + List> ret = new ArrayList>(); for (String tag : tags) { NodeRef tagNodeRef = addTag(nodeRef, tag); @@ -894,36 +894,36 @@ public class TaggingServiceImpl implements TaggingService, int skipCount = pagingRequest.getSkipCount(); int maxItems = pagingRequest.getMaxItems(); int end = maxItems == Integer.MAX_VALUE ? totalItems : skipCount + maxItems; - int size = (maxItems == Integer.MAX_VALUE ? totalItems : maxItems); + int size = (maxItems == Integer.MAX_VALUE ? totalItems : maxItems); final List> sortedTags = new ArrayList>(size); - // grab all tags and sort (assume fairly low number of tags) - for(NodeRef tagNode : currentTagNodes) - { + // grab all tags and sort (assume fairly low number of tags) + for(NodeRef tagNode : currentTagNodes) + { String tag = (String)this.nodeService.getProperty(tagNode, PROP_NAME); sortedTags.add(new Pair(tagNode, tag)); - } + } Collections.sort(sortedTags, new Comparator>() { - @Override - public int compare(Pair o1, Pair o2) - { - String tag1 = o1.getSecond(); - String tag2 = o2.getSecond(); - return collator.compare(tag1, tag2); - } - }); + @Override + public int compare(Pair o1, Pair o2) + { + String tag1 = o1.getSecond(); + String tag2 = o2.getSecond(); + return collator.compare(tag1, tag2); + } + }); final List> result = new ArrayList>(size); - Iterator> it = sortedTags.iterator(); + Iterator> it = sortedTags.iterator(); for(int count = 0; count < end && it.hasNext(); count++) { - Pair tagPair = it.next(); + Pair tagPair = it.next(); - if(count < skipCount) - { - continue; - } + if(count < skipCount) + { + continue; + } result.add(tagPair); } @@ -932,30 +932,30 @@ public class TaggingServiceImpl implements TaggingService, return new PagingResults>() { - @Override - public List> getPage() - { - return result; - } + @Override + public List> getPage() + { + return result; + } - @Override - public boolean hasMoreItems() - { - return hasMoreItems; - } + @Override + public boolean hasMoreItems() + { + return hasMoreItems; + } - @Override - public Pair getTotalResultCount() - { - Integer total = Integer.valueOf(totalItems); - return new Pair(total, total); - } + @Override + public Pair getTotalResultCount() + { + Integer total = Integer.valueOf(totalItems); + return new Pair(total, total); + } - @Override - public String getQueryExecutionId() - { - return null; - } + @Override + public String getQueryExecutionId() + { + return null; + } }; } } @@ -1028,7 +1028,7 @@ public class TaggingServiceImpl implements TaggingService, { if (tagsByCountMap.isEmpty()) { - throw new QueryException("Tag count should be included when ordering by count"); + throw new IllegalArgumentException("Tag count should be included when ordering by count"); } if (!sorting.getSecond()) @@ -1289,15 +1289,15 @@ public class TaggingServiceImpl implements TaggingService, */ private void getTagScopes(final NodeRef nodeRef, List tagScopes, boolean firstOnly) { - Boolean hasAspect = AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public Boolean doWork() throws Exception - { - return new Boolean(nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TAGSCOPE)); - } - }, AuthenticationUtil.getSystemUserName()); - + Boolean hasAspect = AuthenticationUtil.runAs(new RunAsWork() + { + @Override + public Boolean doWork() throws Exception + { + return new Boolean(nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TAGSCOPE)); + } + }, AuthenticationUtil.getSystemUserName()); + if (Boolean.TRUE.equals(hasAspect) == true) { tagScopes.add(nodeRef); @@ -1308,23 +1308,23 @@ public class TaggingServiceImpl implements TaggingService, } NodeRef parent = AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public NodeRef doWork() throws Exception - { - NodeRef result = null; - ChildAssociationRef assoc = nodeService.getPrimaryParent(nodeRef); - if (assoc != null) - { - result = assoc.getParentRef(); - } - return result; - } - }, AuthenticationUtil.getSystemUserName()); + { + @Override + public NodeRef doWork() throws Exception + { + NodeRef result = null; + ChildAssociationRef assoc = nodeService.getPrimaryParent(nodeRef); + if (assoc != null) + { + result = assoc.getParentRef(); + } + return result; + } + }, AuthenticationUtil.getSystemUserName()); if (parent != null) { - getTagScopes(parent, tagScopes, firstOnly); + getTagScopes(parent, tagScopes, firstOnly); } }