From 05cbcbdba8791ea50f64d1cd4f13d2ac410c2edf Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Tue, 10 Feb 2015 12:37:06 +0100 Subject: [PATCH] SONAR-6065 Enhance coverage on IssueQueryService, refactor a bit --- .../sonar/server/issue/IssueQueryService.java | 123 ++++++------ .../server/issue/IssueQueryServiceTest.java | 182 ++++++++++++++++++ .../ws/SearchActionComponentsMediumTest.java | 2 +- 3 files changed, 245 insertions(+), 62 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 895a3aa8cad..c2a59641dc5 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 @@ -92,13 +92,18 @@ public class IssueQueryService implements ServerComponent { .createdAfter(RubyUtils.toDate(params.get(IssueFilterParameters.CREATED_AFTER))) .createdBefore(RubyUtils.toDate(params.get(IssueFilterParameters.CREATED_BEFORE))); - addComponentParameters(builder, session, - RubyUtils.toBoolean(params.get(IssueFilterParameters.ON_COMPONENT_ONLY)), + Set allComponentUuids = Sets.newHashSet(); + boolean effectiveOnComponentOnly = mergeComponentParameters(session, RubyUtils.toBoolean(params.get(IssueFilterParameters.ON_COMPONENT_ONLY)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENTS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_UUIDS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_KEYS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOT_UUIDS)), RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOTS)), + allComponentUuids); + + addComponentParameters(builder, session, + effectiveOnComponentOnly, + allComponentUuids, RubyUtils.toStrings(params.get(IssueFilterParameters.PROJECT_UUIDS)), RubyUtils.toStrings( ObjectUtils.defaultIfNull( @@ -149,13 +154,18 @@ public class IssueQueryService implements ServerComponent { .createdBefore(request.paramAsDateTime(IssueFilterParameters.CREATED_BEFORE)) .ignorePaging(request.paramAsBoolean(IssueFilterParameters.IGNORE_PAGING)); - addComponentParameters(builder, session, - request.paramAsBoolean(IssueFilterParameters.ON_COMPONENT_ONLY), + Set allComponentUuids = Sets.newHashSet(); + boolean effectiveOnComponentOnly = mergeComponentParameters(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), + allComponentUuids); + + addComponentParameters(builder, session, + effectiveOnComponentOnly, + allComponentUuids, request.paramAsStrings(IssueFilterParameters.PROJECT_UUIDS), request.paramAsStrings(IssueFilterParameters.PROJECT_KEYS), request.paramAsStrings(IssueFilterParameters.MODULE_UUIDS), request.paramAsStrings(IssueFilterParameters.DIRECTORIES), @@ -174,33 +184,62 @@ public class IssueQueryService implements ServerComponent { } } - private void addComponentParameters(IssueQuery.Builder builder, DbSession session, - @Nullable Boolean onComponentOnly, + private boolean mergeComponentParameters(DbSession session, 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, + Set allComponentUuids) { + boolean effectiveOnComponentOnly = false; + + failOnIncompatibleParameters(components, componentUuids, componentKeys, componentRootUuids, componentRoots); + + if (componentRootUuids != null) { + allComponentUuids.addAll(componentRootUuids); + effectiveOnComponentOnly = false; + } else if (componentRoots != null) { + allComponentUuids.addAll(componentUuids(session, componentRoots)); + effectiveOnComponentOnly = false; + } else if (components != null) { + allComponentUuids.addAll(componentUuids(session, components)); + effectiveOnComponentOnly = true; + } else if (componentUuids != null) { + allComponentUuids.addAll(componentUuids); + effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); + } else if (componentKeys != null) { + allComponentUuids.addAll(componentUuids(session, componentKeys)); + effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); + } + return effectiveOnComponentOnly; + } + + protected void failOnIncompatibleParameters( + @Nullable Collection components, + @Nullable Collection componentUuids, + @Nullable Collection componentKeys, + @Nullable Collection componentRootUuids, + @Nullable Collection componentRoots) { + failIfBothParametersSet(componentRootUuids, componentRoots, "componentRoots and componentRootUuids cannot be set simultaneously"); + failIfBothParametersSet(componentUuids, components, "components and componentUuids cannot be set simultaneously"); + failIfBothParametersSet(componentKeys, componentUuids, "componentKeys and componentUuids cannot be set simultaneously"); + } + + private void failIfBothParametersSet(@Nullable Collection uuids, @Nullable Collection keys, String message) { + if (uuids != null && keys != null) { + throw new IllegalArgumentException(message); + } + } + + private void addComponentParameters(IssueQuery.Builder builder, DbSession session, + boolean effectiveOnComponentOnly, + Collection allComponentUuids, @Nullable Collection projectUuids, @Nullable Collection projects, @Nullable Collection moduleUuids, @Nullable Collection directories, @Nullable Collection fileUuids, @Nullable Collection authors) { - Set allComponentUuids = Sets.newHashSet(); - boolean effectiveOnComponentOnly = mergeComponentParameters(session, onComponentOnly, - components, - componentUuids, - componentKeys, - componentRootUuids, - componentRoots, - allComponentUuids); - builder.onComponentOnly(effectiveOnComponentOnly); if (allComponentUuids.isEmpty()) { @@ -237,7 +276,7 @@ public class IssueQueryService implements ServerComponent { addComponentsBelowView(builder, session, authors, projects, projectUuids, moduleUuids, directories, fileUuids); } else if ("DEV".equals(uniqueQualifier)) { // XXX No constant for developer !!! - Collection actualAuthors = authors == null ? dbClient.authorDao().selectScmAccountsByDeveloperUuids(allComponentUuids) : authors; + Collection actualAuthors = authors == null ? dbClient.authorDao().selectScmAccountsByDeveloperUuids(session, allComponentUuids) : authors; addComponentsBelowView(builder, session, actualAuthors, projects, projectUuids, moduleUuids, directories, fileUuids); } else if (Qualifiers.PROJECT.equals(uniqueQualifier)) { builder.projectUuids(allComponentUuids); @@ -263,53 +302,15 @@ 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 authors, @Nullable Collection projects, @Nullable Collection projectUuids, @Nullable Collection moduleUuids, Collection directories, Collection fileUuids) { builder.authors(authors); + failIfBothParametersSet(projectUuids, projects, "projects and projectUuids cannot be set simultaneously"); + if (projectUuids != null) { - if (projects != null) { - throw new IllegalArgumentException("projects and projectUuids cannot be set simultaneously"); - } builder.projectUuids(projectUuids); } else { builder.projectUuids(componentUuids(session, projects)); 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 7c8a05e7f85..8ad75641895 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 @@ -20,7 +20,10 @@ package org.sonar.server.issue; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import org.assertj.core.api.Fail; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,11 +35,14 @@ import org.mockito.stubbing.Answer; import org.sonar.api.resources.Qualifiers; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; +import org.sonar.api.web.UserRole; +import org.sonar.core.component.ComponentDto; import org.sonar.core.persistence.DbSession; import org.sonar.core.user.AuthorDao; import org.sonar.server.component.ComponentService; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.db.DbClient; +import org.sonar.server.user.MockUserSession; import java.util.ArrayList; import java.util.Arrays; @@ -49,7 +55,9 @@ import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -89,6 +97,11 @@ public class IssueQueryServiceTest { issueQueryService = new IssueQueryService(dbClient, componentService); } + @After + public void tearDown() { + MockUserSession.set(); + } + @Test public void create_query_from_parameters() { Map map = newHashMap(); @@ -227,4 +240,173 @@ public class IssueQueryServiceTest { } } + @Test + public void should_search_in_tree_with_component_root_uuids_but_unknown_qualifiers() throws Exception { + Map map = newHashMap(); + map.put("componentRootUuids", newArrayList("ABCD")); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet()); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.onComponentOnly()).isFalse(); + assertThat(query.componentUuids()).contains("ABCD"); + } + + @Test + public void should_search_in_tree_with_component_roots_but_different_qualifiers() throws Exception { + Map map = newHashMap(); + map.put("componentRoots", newArrayList("org.apache.struts:struts", "org.codehaus.sonar:sonar-server")); + + when(componentService.componentUuids(isA(DbSession.class), anyCollection(), eq(true))).thenReturn(Sets.newHashSet("ABCD", "BCDE")); + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newTreeSet(Arrays.asList("TRK", "BRC"))); + + try { + issueQueryService.createFromMap(map); + Fail.failBecauseExceptionWasNotThrown(IllegalArgumentException.class); + } catch (IllegalArgumentException exception) { + assertThat(exception).hasMessage("All components must have the same qualifier, found BRC,TRK"); + } + } + + @Test + public void should_search_in_tree_with_view() throws Exception { + String viewUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentRootUuids", newArrayList(viewUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.VIEW)); + + MockUserSession.set().addProjectUuidPermissions(UserRole.USER, viewUuid); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.viewUuids()).containsExactly(viewUuid); + assertThat(query.onComponentOnly()).isFalse(); + } + + @Test + public void should_search_in_tree_with_subview_but_bad_permissions() throws Exception { + String subViewUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentRootUuids", newArrayList(subViewUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.VIEW)); + + MockUserSession.set(); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.viewUuids()).isNotEmpty().doesNotContain(subViewUuid); + } + + @Test + public void should_search_in_tree_with_project_uuid() throws Exception { + String projectUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(projectUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.PROJECT)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.projectUuids()).containsExactly(projectUuid); + assertThat(query.onComponentOnly()).isFalse(); + } + + @Test + public void should_search_on_component_only_with_project_key() throws Exception { + String projectKey = "org.apache.struts:struts"; + String projectUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentKeys", newArrayList(projectKey)); + map.put("onComponentOnly", true); + + when(componentService.componentUuids(isA(DbSession.class), anyCollection(), eq(true))).thenReturn(Sets.newHashSet(projectUuid)); + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.PROJECT)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.projectUuids()).isEmpty(); + assertThat(query.componentUuids()).containsExactly(projectUuid); + assertThat(query.onComponentOnly()).isTrue(); + } + + @Test + public void should_search_on_developer() throws Exception { + String devUuid = "DEV:anakin.skywalker"; + String login1 = "anakin@skywalker.name"; + String login2 = "darth.vader"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(devUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet("DEV")); + when(authorDao.selectScmAccountsByDeveloperUuids(isA(DbSession.class), anyCollection())).thenReturn(Lists.newArrayList(login1, login2)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.authors()).containsExactly(login1, login2); + } + + @Test + public void should_override_authors_when_searching_on_developer() throws Exception { + String devUuid = "DEV:anakin.skywalker"; + String login = "anakin@skywalker.name"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(devUuid)); + map.put("authors", newArrayList(login)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet("DEV")); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.authors()).containsExactly(login); + } + + @Test + public void should_search_in_tree_with_module_uuid() throws Exception { + String moduleUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(moduleUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.MODULE)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.moduleRootUuids()).containsExactly(moduleUuid); + assertThat(query.onComponentOnly()).isFalse(); + } + + @Test + public void should_search_in_tree_with_directory_uuid() throws Exception { + String directoryUuid = "ABCD"; + String directoryPath = "/some/module/relative/path"; + String moduleUuid = "BCDE"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(directoryUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.DIRECTORY)); + when(componentService.getByUuids(isA(DbSession.class), anyCollection())).thenReturn(Arrays.asList(new ComponentDto().setModuleUuid(moduleUuid).setPath(directoryPath))); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.moduleUuids()).contains(moduleUuid); + assertThat(query.directories()).containsExactly(directoryPath); + assertThat(query.onComponentOnly()).isFalse(); + } + + @Test + public void should_search_on_source_file() throws Exception { + String fileUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(fileUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.FILE)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.fileUuids()).containsExactly(fileUuid); + } + + @Test + public void should_search_on_test_file() throws Exception { + String fileUuid = "ABCD"; + Map map = newHashMap(); + map.put("componentUuids", newArrayList(fileUuid)); + + when(componentService.getDistinctQualifiers(isA(DbSession.class), anyCollection())).thenReturn(Sets.newHashSet(Qualifiers.UNIT_TEST_FILE)); + + IssueQuery query = issueQueryService.createFromMap(map); + assertThat(query.fileUuids()).containsExactly(fileUuid); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java index 294ebb12463..5b3ed067794 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java @@ -342,7 +342,7 @@ public class SearchActionComponentsMediumTest { tester.get(IssueIndexer.class).indexAll(); wsTester.newGetRequest(IssuesWs.API_ENDPOINT, SearchAction.SEARCH_ACTION) - .setParam(IssueFilterParameters.COMPONENT_UUIDS, project.uuid()) + .setParam(IssueFilterParameters.COMPONENT_UUIDS, module.uuid()) .setParam(IssueFilterParameters.MODULE_UUIDS, subModule1.uuid() + "," + subModule3.uuid()) .setParam(WebService.Param.FACETS, "moduleUuids") .execute() -- 2.39.5