From a4705af47bc74913550e4cc86ab0e116beaa9d66 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Thu, 18 Dec 2014 17:47:44 +0100 Subject: [PATCH] SONAR-5955 Extract utility method to convert component keys to UUIDs --- .../server/component/ComponentService.java | 43 +++++++++++++++++++ .../sonar/server/issue/IssueQueryService.java | 24 +++++------ .../component/ComponentServiceMediumTest.java | 40 +++++++++++++++++ .../server/issue/IssueQueryServiceTest.java | 42 ++++++++++++++---- 4 files changed, 129 insertions(+), 20 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java index b8c8f269394..c70a4c90609 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java @@ -20,6 +20,13 @@ package org.sonar.server.component; +import com.google.common.base.Joiner; + +import org.apache.commons.collections.CollectionUtils; +import com.google.common.base.Function; +import com.google.common.base.Function; +import com.google.common.collect.Collections2; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.api.ServerComponent; import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; @@ -30,9 +37,14 @@ import org.sonar.server.db.DbClient; import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import java.util.Collection; +import java.util.List; import java.util.Map; +import static com.google.common.collect.Lists.newArrayList; + public class ComponentService implements ServerComponent { private final DbClient dbClient; @@ -131,4 +143,35 @@ public class ComponentService implements ServerComponent { } } + public Collection componentUuids(@Nullable Collection componentKeys) { + DbSession session = dbClient.openSession(false); + try { + return componentUuids(session, componentKeys, false); + } finally { + session.close(); + } + } + + public Collection componentUuids(DbSession session, @Nullable Collection componentKeys, boolean ignoreMissingComponents) { + Collection componentUuids = newArrayList(); + if (componentKeys != null && !componentKeys.isEmpty()) { + List components = dbClient.componentDao().getByKeys(session, componentKeys); + + if (!ignoreMissingComponents && components.size() < componentKeys.size()) { + Collection foundKeys = Collections2.transform(components, new Function() { + @Override + public String apply(ComponentDto component) { + return component.key(); + } + }); + throw new NotFoundException("The following component keys do not match any component:\n" + + Joiner.on('\n').join(CollectionUtils.subtract(componentKeys, foundKeys))); + } + + for (ComponentDto component : components) { + componentUuids.add(component.uuid()); + } + } + return componentUuids; + } } 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 df2eb281259..2b1648a4423 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 @@ -20,8 +20,10 @@ package org.sonar.server.issue; -import org.apache.commons.lang.ObjectUtils; +import com.google.common.collect.Lists; +import org.sonar.server.component.ComponentService; +import org.apache.commons.lang.ObjectUtils; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Splitter; @@ -52,9 +54,11 @@ import static com.google.common.collect.Lists.newArrayList; public class IssueQueryService implements ServerComponent { private final DbClient dbClient; + private final ComponentService componentService; - public IssueQueryService(DbClient dbClient) { + public IssueQueryService(DbClient dbClient, ComponentService componentService) { this.dbClient = dbClient; + this.componentService = componentService; } public IssueQuery createFromMap(Map params) { @@ -199,16 +203,12 @@ public class IssueQueryService implements ServerComponent { } private Collection componentUuids(DbSession session, @Nullable Collection componentKeys) { - Collection componentUuids = newArrayList(); - if (componentKeys != null) { - for (ComponentDto component : dbClient.componentDao().getByKeys(session, componentKeys)) { - componentUuids.add(component.uuid()); - } - // If unknown components are given, if no components are set then all issues will be return, - // then we add this hack in order to return no issue in this case. - if (!componentKeys.isEmpty() && componentUuids.isEmpty()) { - componentUuids.add(""); - } + Collection componentUuids = Lists.newArrayList(); + componentUuids.addAll(componentService.componentUuids(session, componentKeys, true)); + // If unknown components are given, but no components are found, then all issues will be returned, + // so we add this hack in order to return no issue in this case. + if (componentKeys != null && !componentKeys.isEmpty() && componentUuids.isEmpty()) { + componentUuids.add(""); } return componentUuids; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceMediumTest.java index adeb8d13709..17bd2f515cb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceMediumTest.java @@ -20,6 +20,10 @@ package org.sonar.server.component; +import org.sonar.server.exceptions.NotFoundException; + +import org.fest.assertions.Fail; +import com.google.common.collect.Lists; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; @@ -40,6 +44,7 @@ import org.sonar.server.rule.db.RuleDao; import org.sonar.server.tester.ServerTester; import org.sonar.server.user.MockUserSession; +import java.util.Arrays; import java.util.Map; import static org.fest.assertions.Assertions.assertThat; @@ -277,4 +282,39 @@ public class ComponentServiceMediumTest { service.bulkUpdateKey("sample:root", "sample", "sample2"); } + @Test + public void should_return_project_uuids() throws Exception { + String moduleKey = "sample:root:module"; + ComponentDto module = ComponentTesting.newModuleDto(project).setKey(moduleKey); + tester.get(ComponentDao.class).insert(session, module); + String fileKey = "sample:root:module:Foo.xoo"; + ComponentDto file = ComponentTesting.newFileDto(module).setKey(fileKey); + tester.get(ComponentDao.class).insert(session, file); + session.commit(); + + assertThat(service.componentUuids(Arrays.asList(moduleKey, fileKey))).hasSize(2); + assertThat(service.componentUuids(null)).isEmpty(); + assertThat(service.componentUuids(Arrays.asList())).isEmpty(); + } + + @Test + public void should_fail_on_components_not_found() throws Exception { + String moduleKey = "sample:root:module"; + String fileKey = "sample:root:module:Foo.xoo"; + + try { + service.componentUuids(Arrays.asList(moduleKey, fileKey)); + Fail.fail("Should throw NotFoundException"); + } catch(NotFoundException notFound) { + assertThat(notFound.getMessage()).contains(moduleKey).contains(fileKey); + } + } + + @Test + public void should_fail_silently_on_components_not_found_if_told_so() throws Exception { + String moduleKey = "sample:root:module"; + String fileKey = "sample:root:module:Foo.xoo"; + + assertThat(service.componentUuids(session, Arrays.asList(moduleKey, fileKey), true)).isEmpty(); + } } 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 5f13bb3ea6e..2f524b4f1a6 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 @@ -24,14 +24,20 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; import org.sonar.core.component.ComponentDto; import org.sonar.core.persistence.DbSession; +import org.sonar.server.component.ComponentService; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.db.DbClient; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.Map; import static com.google.common.collect.Lists.newArrayList; @@ -39,6 +45,8 @@ import static com.google.common.collect.Maps.newHashMap; import static java.util.Arrays.asList; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -53,13 +61,25 @@ public class IssueQueryServiceTest { @Mock ComponentDao componentDao; + @Mock + ComponentService componentService; + IssueQueryService issueQueryService; @Before public void setUp() throws Exception { when(dbClient.openSession(false)).thenReturn(session); when(dbClient.componentDao()).thenReturn(componentDao); - issueQueryService = new IssueQueryService(dbClient); + + when(componentService.componentUuids(any(DbSession.class), any(Collection.class), eq(true))).thenAnswer(new Answer>() { + @Override + public Collection answer(InvocationOnMock invocation) throws Throwable { + Collection componentKeys = (Collection) invocation.getArguments()[1]; + return componentKeys == null ? Arrays.asList() : componentKeys; + } + }); + + issueQueryService = new IssueQueryService(dbClient, componentService); } @Test @@ -70,8 +90,10 @@ public class IssueQueryServiceTest { map.put("statuses", newArrayList("CLOSED")); map.put("resolutions", newArrayList("FALSE-POSITIVE")); map.put("resolved", true); - map.put("components", newArrayList("org.apache")); - map.put("componentRoots", newArrayList("org.sonar")); + ArrayList componentKeys = newArrayList("org.apache"); + map.put("components", componentKeys); + ArrayList moduleKeys = newArrayList("org.sonar"); + map.put("componentRoots", moduleKeys); map.put("reporters", newArrayList("marilyn")); map.put("assignees", newArrayList("joanna")); map.put("languages", newArrayList("xoo")); @@ -84,9 +106,9 @@ public class IssueQueryServiceTest { map.put("rules", "squid:AvoidCycle,findbugs:NullReference"); map.put("sort", "CREATION_DATE"); map.put("asc", true); - - when(componentDao.getByKeys(session, newArrayList("org.apache"))).thenReturn(newArrayList(new ComponentDto().setUuid("ABCD"))); - when(componentDao.getByKeys(session, newArrayList("org.sonar"))).thenReturn(newArrayList(new ComponentDto().setUuid("BCDE"))); + + when(componentService.componentUuids(session, componentKeys, true)).thenReturn(newArrayList("ABCD")); + when(componentService.componentUuids(session, moduleKeys, true)).thenReturn(newArrayList("BCDE")); IssueQuery query = issueQueryService.createFromMap(map); assertThat(query.issueKeys()).containsOnly("ABCDE1234"); @@ -113,7 +135,10 @@ public class IssueQueryServiceTest { @Test public void add_unknown_when_no_component_found() { Map map = newHashMap(); - map.put("components", newArrayList("unknown")); + ArrayList componentKeys = newArrayList("unknown"); + map.put("components", componentKeys); + + when(componentService.componentUuids(any(DbSession.class), eq(componentKeys), eq(true))).thenReturn(Arrays.asList()); IssueQuery query = issueQueryService.createFromMap(map); assertThat(query.componentUuids()).containsOnly(""); @@ -131,7 +156,8 @@ public class IssueQueryServiceTest { @Test public void fail_if_components_and_components_uuid_params_are_set_at_the_same_time() { Map map = newHashMap(); - map.put("components", newArrayList("org.apache")); + ArrayList componentKeys = newArrayList("org.apache"); + map.put("components", componentKeys); map.put("componentUuids", newArrayList("ABCD")); try { -- 2.39.5