From 639ab0ae52d143cf8b8b82c009c95e69ed2d40d7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Fri, 30 Jan 2015 11:12:28 +0100 Subject: [PATCH] SONAR-5953 Fall back to backwards compatible API for issues search WS (components|componentRoots) --- .../sonar/server/issue/IssueQueryService.java | 97 +++++++++++++------ .../sonar/server/issue/index/IssueIndex.java | 6 +- .../sonar/server/issue/ws/SearchAction.java | 34 ++++--- .../server/issue/IssueQueryServiceTest.java | 9 +- 4 files changed, 98 insertions(+), 48 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java index a1881f88448..1472b728a55 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java @@ -28,6 +28,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import org.apache.commons.lang.BooleanUtils; import org.apache.commons.lang.ObjectUtils; import org.sonar.api.ServerComponent; import org.sonar.api.resources.Qualifiers; @@ -81,7 +82,6 @@ public class IssueQueryService implements ServerComponent { .assignees(RubyUtils.toStrings(params.get(IssueFilterParameters.ASSIGNEES))) .languages(RubyUtils.toStrings(params.get(IssueFilterParameters.LANGUAGES))) .tags(RubyUtils.toStrings(params.get(IssueFilterParameters.TAGS))) - .onComponentOnly(RubyUtils.toBoolean(params.get(IssueFilterParameters.ON_COMPONENT_ONLY))) .assigned(RubyUtils.toBoolean(params.get(IssueFilterParameters.ASSIGNED))) .planned(RubyUtils.toBoolean(params.get(IssueFilterParameters.PLANNED))) .hideRules(RubyUtils.toBoolean(params.get(IssueFilterParameters.HIDE_RULES))) @@ -89,14 +89,11 @@ public class IssueQueryService implements ServerComponent { .createdAfter(RubyUtils.toDate(params.get(IssueFilterParameters.CREATED_AFTER))) .createdBefore(RubyUtils.toDate(params.get(IssueFilterParameters.CREATED_BEFORE))); - addComponentUuids(builder, session, + addComponentParameters(builder, session, + RubyUtils.toBoolean(params.get(IssueFilterParameters.ON_COMPONENT_ONLY)), + RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENTS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_UUIDS)), - RubyUtils.toStrings( - ObjectUtils.defaultIfNull( - params.get(IssueFilterParameters.COMPONENT_KEYS), - params.get(IssueFilterParameters.COMPONENTS) - ) - ), + RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_KEYS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOT_UUIDS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOTS)), RubyUtils.toStrings(params.get(IssueFilterParameters.PROJECT_UUIDS)), @@ -141,7 +138,6 @@ public class IssueQueryService implements ServerComponent { .assignees(request.paramAsStrings(IssueFilterParameters.ASSIGNEES)) .languages(request.paramAsStrings(IssueFilterParameters.LANGUAGES)) .tags(request.paramAsStrings(IssueFilterParameters.TAGS)) - .onComponentOnly(request.paramAsBoolean(IssueFilterParameters.ON_COMPONENT_ONLY)) .assigned(request.paramAsBoolean(IssueFilterParameters.ASSIGNED)) .planned(request.paramAsBoolean(IssueFilterParameters.PLANNED)) .createdAt(request.paramAsDateTime(IssueFilterParameters.CREATED_AT)) @@ -149,9 +145,13 @@ public class IssueQueryService implements ServerComponent { .createdBefore(request.paramAsDateTime(IssueFilterParameters.CREATED_BEFORE)) .ignorePaging(request.paramAsBoolean(IssueFilterParameters.IGNORE_PAGING)); - addComponentUuids(builder, session, - request.paramAsStrings(IssueFilterParameters.COMPONENT_UUIDS), request.paramAsStrings(IssueFilterParameters.COMPONENT_KEYS), - request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOT_UUIDS), request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOTS), + addComponentParameters(builder, session, + request.paramAsBoolean(IssueFilterParameters.ON_COMPONENT_ONLY), + request.paramAsStrings(IssueFilterParameters.COMPONENTS), + request.paramAsStrings(IssueFilterParameters.COMPONENT_UUIDS), + request.paramAsStrings(IssueFilterParameters.COMPONENT_KEYS), + request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOT_UUIDS), + request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOTS), request.paramAsStrings(IssueFilterParameters.PROJECT_UUIDS), request.paramAsStrings(IssueFilterParameters.PROJECT_KEYS), request.paramAsStrings(IssueFilterParameters.MODULE_UUIDS), request.paramAsStrings(IssueFilterParameters.DIRECTORIES), @@ -169,38 +169,44 @@ public class IssueQueryService implements ServerComponent { } } - private void addComponentUuids(IssueQuery.Builder builder, DbSession session, - @Nullable Collection componentUuids, @Nullable Collection components, + private void addComponentParameters(IssueQuery.Builder builder, DbSession session, + @Nullable Boolean onComponentOnly, + @Nullable Collection components, + @Nullable Collection componentUuids, + @Nullable Collection componentKeys, /* * Since 5.1, search of issues is recursive by default (module + submodules), * but "componentKeys" parameter already deprecates "components" parameter, * so queries specifying "componentRoots" must be handled manually */ - @Nullable Collection componentRootUuids, @Nullable Collection componentRoots, + @Nullable Collection componentRootUuids, + @Nullable Collection componentRoots, @Nullable Collection projectUuids, @Nullable Collection projects, @Nullable Collection moduleUuids, @Nullable Collection directories, @Nullable Collection fileUuids) { Set allComponentUuids = Sets.newHashSet(); + boolean effectiveOnComponentOnly = mergeComponentParameters(session, onComponentOnly, + components, + componentUuids, + componentKeys, + componentRootUuids, + componentRoots, + allComponentUuids); - if (componentUuids != null || componentRootUuids != null) { - if (components != null || componentRoots != null) { - throw new IllegalArgumentException("components and componentUuids cannot be set simultaneously"); - } - allComponentUuids.addAll((Collection) ObjectUtils.defaultIfNull(componentUuids, Sets.newHashSet())); - allComponentUuids.addAll((Collection) ObjectUtils.defaultIfNull(componentRootUuids, Sets.newHashSet())); - } else { - Set allComponents = Sets.newHashSet(); - allComponents.addAll((Collection) ObjectUtils.defaultIfNull(components, Sets.newHashSet())); - allComponents.addAll((Collection) ObjectUtils.defaultIfNull(componentRoots, Sets.newHashSet())); - allComponentUuids.addAll(componentUuids(session, allComponents)); - } + builder.onComponentOnly(effectiveOnComponentOnly); if (allComponentUuids.isEmpty()) { builder.setContextualized(false); addComponentsBelowView(builder, session, projects, projectUuids, moduleUuids, directories, fileUuids); } else { + if (effectiveOnComponentOnly) { + builder.setContextualized(false); + builder.componentUuids(allComponentUuids); + return; + } + builder.setContextualized(true); Set qualifiers = componentService.getDistinctQualifiers(session, allComponentUuids); @@ -244,6 +250,43 @@ public class IssueQueryService implements ServerComponent { } } + private boolean mergeComponentParameters(DbSession session, Boolean onComponentOnly, + Collection components, + Collection componentUuids, + Collection componentKeys, + Collection componentRootUuids, + Collection componentRoots, + Set allComponentUuids) { + boolean effectiveOnComponentOnly = false; + + if (componentRootUuids != null) { + if (componentRoots != null) { + throw new IllegalArgumentException("componentRoots and componentRootUuids cannot be set simultaneously"); + } + allComponentUuids.addAll(componentRootUuids); + effectiveOnComponentOnly = false; + } else if (componentRoots != null) { + allComponentUuids.addAll(componentUuids(session, componentRoots)); + effectiveOnComponentOnly = false; + } else if (components != null) { + if (componentUuids != null) { + throw new IllegalArgumentException("components and componentUuids cannot be set simultaneously"); + } + allComponentUuids.addAll(componentUuids(session, components)); + effectiveOnComponentOnly = true; + } else if (componentUuids != null) { + if (componentKeys != null) { + throw new IllegalArgumentException("componentUuids and componentKeys cannot be set simultaneously"); + } + allComponentUuids.addAll(componentUuids); + effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); + } else if (componentKeys != null) { + allComponentUuids.addAll(componentUuids(session, componentKeys)); + effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); + } + return effectiveOnComponentOnly; + } + private void addComponentsBelowView(Builder builder, DbSession session, @Nullable Collection projects, @Nullable Collection projectUuids, @Nullable Collection moduleUuids, Collection directories, Collection fileUuids) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 42414efe6db..688cc6b5ebb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -289,10 +289,14 @@ public class IssueIndex extends BaseIndex { filters.put(FILTER_COMPONENT_ROOT, componentFilter); } } else { + if (query.onComponentOnly()) { + filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, componentFilter); + } else { + filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, fileFilter); + } filters.put(IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID, projectFilter); filters.put(IssueIndexDefinition.FIELD_ISSUE_MODULE_UUID, moduleFilter); filters.put(IssueIndexDefinition.FIELD_ISSUE_DIRECTORY_PATH, directoryFilter); - filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, fileFilter); filters.put("view", viewFilter); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java index 16331c7d8a2..dbf72d30a71 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java @@ -196,21 +196,34 @@ public class SearchAction extends SearchRequestHandler { } private void addComponentRelatedParams(WebService.NewAction action) { + + action.createParam(IssueFilterParameters.ON_COMPONENT_ONLY) + .setDescription("Return only issues at a component's level, not on its descendants (modules, directories, files, etc). " + + "This parameter is only considered when componentKeys or componentUuids is set. " + + "Using the deprecated componentRoots or componentRootUuids parameters will set this parameter to false. " + + "Using the deprecated components parameter will set this parameter to true.") + .setBooleanPossibleValues() + .setDefaultValue("false"); + action.createParam(IssueFilterParameters.COMPONENT_KEYS) - .setDescription("To retrieve issues associated to a specific list of components and their sub-components (comma-separated list of component keys). " + - "A component can be a project, module, directory or file. " + + .setDescription("To retrieve issues associated to a specific list of components sub-components (comma-separated list of component keys). " + + "A component can be a view, developer, project, module, directory or file. " + "If this parameter is set, componentUuids must not be set.") - .setDeprecatedKey(IssueFilterParameters.COMPONENTS) .setExampleValue("org.apache.struts:struts:org.apache.struts.Action"); action.createParam(IssueFilterParameters.COMPONENTS) - .setDescription("Deprecated since 5.1. See componentKeys."); + .setDescription("Deprecated since 5.1. If used, will have the same meaning as componentKeys AND onComponentOnly=true."); action.createParam(IssueFilterParameters.COMPONENT_UUIDS) - .setDescription("To retrieve issues associated to a specific list of components and their sub-components (comma-separated list of component UUIDs). " + + .setDescription("To retrieve issues associated to a specific list of components their sub-components (comma-separated list of component UUIDs). " + INTERNAL_PARAMETER_DISCLAIMER + "A component can be a project, module, directory or file. " + "If this parameter is set, componentKeys must not be set.") .setExampleValue("584a89f2-8037-4f7b-b82c-8b45d2d63fb2"); + action.createParam(IssueFilterParameters.COMPONENT_ROOTS) + .setDescription("Deprecated since 5.1. If used, will have the same meaning as componentKeys AND onComponentOnly=false."); + action.createParam(IssueFilterParameters.COMPONENT_ROOT_UUIDS) + .setDescription("Deprecated since 5.1. If used, will have the same meaning as componentUuids AND onComponentOnly=false."); + action.createParam(IssueFilterParameters.PROJECTS) .setDescription("Deprecated since 5.1. See projectKeys"); action.createParam(IssueFilterParameters.PROJECT_KEYS) @@ -225,10 +238,6 @@ public class SearchAction extends SearchRequestHandler { "Views are not supported. If this parameter is set, projectKeys must not be set.") .setExampleValue("7d8749e8-3070-4903-9188-bdd82933bb92"); - action.createParam(IssueFilterParameters.COMPONENT_ROOTS) - .setDescription("Deprecated since 5.1. Use componentKeys instead, with onComponentOnly=false."); - action.createParam(IssueFilterParameters.COMPONENT_ROOT_UUIDS) - .setDescription("Deprecated since 5.1. Use componentUuids instead, with onComponentOnly=false."); action.createParam(IssueFilterParameters.MODULE_UUIDS) .setDescription("To retrieve issues associated to a specific list of modules (comma-separated list of module UUIDs). " + INTERNAL_PARAMETER_DISCLAIMER + @@ -245,11 +254,6 @@ public class SearchAction extends SearchRequestHandler { .setDescription("To retrieve issues associated to a specific list of files (comma-separated list of file UUIDs). " + INTERNAL_PARAMETER_DISCLAIMER) .setExampleValue("bdd82933-3070-4903-9188-7d8749e8bb92"); - - action.createParam(IssueFilterParameters.ON_COMPONENT_ONLY) - .setDescription("Return only issues at the component's level, not on its descendants (modules, directories, files, etc.)") - .setBooleanPossibleValues() - .setDefaultValue("false"); } @Override @@ -259,7 +263,7 @@ public class SearchAction extends SearchRequestHandler { @Override protected Result doSearch(IssueQuery query, QueryContext context) { - Collection components = query.fileUuids(); + Collection components = query.componentUuids(); if (components != null && components.size() == 1 && BooleanUtils.isTrue(query.ignorePaging())) { context.setShowFullResult(true); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java index b2b811575c2..379e15a484f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java @@ -92,8 +92,8 @@ public class IssueQueryServiceTest { map.put("statuses", newArrayList("CLOSED")); map.put("resolutions", newArrayList("FALSE-POSITIVE")); map.put("resolved", true); - ArrayList componentKeys = newArrayList("org.apache"); - map.put("components", componentKeys); + ArrayList projectKeys = newArrayList("org.apache"); + map.put("projectKeys", projectKeys); ArrayList moduleUuids = newArrayList("BCDE"); map.put("moduleUuids", moduleUuids); map.put("directories", newArrayList("/src/main/java/example")); @@ -103,7 +103,6 @@ public class IssueQueryServiceTest { map.put("assignees", newArrayList("joanna")); map.put("languages", newArrayList("xoo")); map.put("tags", newArrayList("tag1", "tag2")); - map.put("onComponentOnly", true); map.put("assigned", true); map.put("planned", true); map.put("hideRules", true); @@ -142,7 +141,7 @@ public class IssueQueryServiceTest { assertThat(query.assignees()).containsOnly("joanna"); assertThat(query.languages()).containsOnly("xoo"); assertThat(query.tags()).containsOnly("tag1", "tag2"); - assertThat(query.onComponentOnly()).isTrue(); + assertThat(query.onComponentOnly()).isFalse(); assertThat(query.assigned()).isTrue(); assertThat(query.planned()).isTrue(); assertThat(query.hideRules()).isTrue(); @@ -219,7 +218,7 @@ public class IssueQueryServiceTest { issueQueryService.createFromMap(map); fail(); } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("components and componentUuids cannot be set simultaneously"); + assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("componentRoots and componentRootUuids cannot be set simultaneously"); } } -- 2.39.5