From 65b17b37d0b18a6bef8e0c0f6892f0b328e4ef3f Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 22 Sep 2014 08:44:23 +0200 Subject: [PATCH] SONAR-5531 Update issues show ws and execution of bulk change to not use IssueFinder --- .../server/issue/DefaultIssueService.java | 42 +++- .../server/issue/IssueBulkChangeService.java | 101 ++++++++-- .../org/sonar/server/issue/IssueService.java | 2 + .../sonar/server/issue/OldIssueService.java | 5 + .../sonar/server/issue/index/IssueIndex.java | 5 +- .../sonar/server/rule/DefaultRuleFinder.java | 12 ++ .../issue/DefaultIssueServiceMediumTest.java | 11 ++ .../IssueBulkChangeServiceMediumTest.java | 183 ++++++++++++++++++ .../issue/IssueBulkChangeServiceTest.java | 110 ++++++----- .../org/sonar/server/issue/IssueTesting.java | 29 ++- ...viceTest.java => OldIssueServiceTest.java} | 7 +- .../issue/index/IssueIndexMediumTest.java | 46 +++++ .../rule/DefaultRuleFinderMediumTest.java | 11 ++ 13 files changed, 490 insertions(+), 74 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java rename server/sonar-server/src/test/java/org/sonar/server/issue/{OldDefaultIssueServiceTest.java => OldIssueServiceTest.java} (99%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/DefaultIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/DefaultIssueService.java index a48935d271b..a49abf0c583 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/DefaultIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/DefaultIssueService.java @@ -19,9 +19,11 @@ */ package org.sonar.server.issue; +import com.google.common.base.Function; import com.google.common.base.Objects; import com.google.common.base.Strings; import com.google.common.collect.HashMultiset; +import com.google.common.collect.Iterables; import com.google.common.collect.Multiset; import org.apache.commons.lang.StringUtils; import org.sonar.api.issue.ActionPlan; @@ -41,6 +43,7 @@ import org.sonar.core.issue.DefaultIssueBuilder; import org.sonar.core.issue.IssueNotifications; import org.sonar.core.issue.IssueUpdater; import org.sonar.core.issue.db.IssueDao; +import org.sonar.core.issue.db.IssueDto; import org.sonar.core.issue.db.IssueStorage; import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.core.issue.workflow.Transition; @@ -62,6 +65,8 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import static com.google.common.collect.Lists.newArrayList; + public class DefaultIssueService implements IssueService { private final DbClient dbClient; @@ -301,8 +306,8 @@ public class DefaultIssueService implements IssueService { public DefaultIssue getIssueByKey(DbSession session, String key) { // Load from index to check permission - indexClient.get(IssueIndex.class).getByKey(key); - return dbClient.issueDao().getByKey(session, key).toDefaultIssue(); + Issue authorizedIssueIndex = indexClient.get(IssueIndex.class).getByKey(key); + return dbClient.issueDao().getByKey(session, authorizedIssueIndex.key()).toDefaultIssue(); } public DefaultIssue getIssueByKey(String key) { @@ -338,8 +343,37 @@ public class DefaultIssueService implements IssueService { } public org.sonar.server.search.Result search(IssueQuery query, QueryContext options) { - IssueIndex issueIndex = indexClient.get(IssueIndex.class); - return issueIndex.search(query, options); + return indexClient.get(IssueIndex.class).search(query, options); + } + + /** + * Used by the bulk change + * TODO move it to the IssueBulkChangeService when OldIssueService will be removed + */ + @Override + public List search(List issueKeys) { + // Load from index to check permission + List authorizedIndexIssues = search(IssueQuery.builder().issueKeys(issueKeys).build(), new QueryContext().setMaxLimit()).getHits(); + // return ; + List authorizedIssueKeys = newArrayList(Iterables.transform(authorizedIndexIssues, new Function() { + @Override + public String apply(@Nullable Issue input) { + return input != null ? input.key() : null; + } + })); + + DbSession session = dbClient.openSession(false); + try { + List issueDtos = dbClient.issueDao().getByKeys(session, authorizedIssueKeys); + return newArrayList(Iterables.transform(issueDtos, new Function() { + @Override + public Issue apply(@Nullable IssueDto input) { + return input != null ? input.toDefaultIssue() : null; + } + })); + } finally { + session.close(); + } } private void verifyLoggedIn() { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java index 981049b3716..10dc8b4b173 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java @@ -20,43 +20,51 @@ package org.sonar.server.issue; -import org.sonar.core.preview.PreviewCache; - import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.issue.Issue; -import org.sonar.api.issue.IssueQuery; -import org.sonar.api.issue.IssueQueryResult; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; -import org.sonar.api.web.UserRole; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.Rule; +import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.IssueNotifications; import org.sonar.core.issue.db.IssueStorage; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.preview.PreviewCache; +import org.sonar.server.db.DbClient; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.user.UserSession; -import java.util.Date; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import javax.annotation.CheckForNull; + +import java.util.*; import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Sets.newHashSet; public class IssueBulkChangeService { private static final Logger LOG = LoggerFactory.getLogger(IssueBulkChangeService.class); - private final DefaultIssueFinder issueFinder; + private final DbClient dbClient; + private final IssueService issueService; private final IssueStorage issueStorage; + private final DefaultRuleFinder ruleFinder; private final IssueNotifications issueNotifications; private final PreviewCache dryRunCache; private final List actions; - public IssueBulkChangeService(DefaultIssueFinder issueFinder, IssueStorage issueStorage, IssueNotifications issueNotifications, List actions, PreviewCache dryRunCache) { - this.issueFinder = issueFinder; + public IssueBulkChangeService(DbClient dbClient, IssueService issueService, IssueStorage issueStorage, DefaultRuleFinder ruleFinder, + IssueNotifications issueNotifications, List actions, PreviewCache dryRunCache) { + this.dbClient = dbClient; + this.issueService = issueService; this.issueStorage = issueStorage; + this.ruleFinder = ruleFinder; this.issueNotifications = issueNotifications; this.actions = actions; this.dryRunCache = dryRunCache; @@ -68,10 +76,11 @@ public class IssueBulkChangeService { userSession.checkLoggedIn(); IssueBulkChangeResult result = new IssueBulkChangeResult(); - IssueQueryResult issueQueryResult = issueFinder.find(IssueQuery.builder().issueKeys(issueBulkChangeQuery.issues()).pageSize(-1).requiredRole(UserRole.USER).build()); - List issues = issueQueryResult.issues(); - List bulkActions = getActionsToApply(issueBulkChangeQuery, issues, userSession); + List issues = issueService.search(issueBulkChangeQuery.issues()); + Referentials referentials = new Referentials(issues); + + List bulkActions = getActionsToApply(issueBulkChangeQuery, issues, userSession); IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(), userSession.login()); Set concernedProjects = new HashSet(); for (Issue issue : issues) { @@ -86,9 +95,15 @@ public class IssueBulkChangeService { } issueStorage.save((DefaultIssue) issue); if (issueBulkChangeQuery.sendNotifications()) { - issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult); + String projectKey = issue.projectKey(); + if (projectKey != null) { + issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, + referentials.rule(issue.ruleKey()), + referentials.project(projectKey), + referentials.component(issue.componentKey())); + } } - concernedProjects.add(((DefaultIssue) issue).projectKey()); + concernedProjects.add(issue.projectKey()); } } // Purge dryRun cache @@ -156,4 +171,56 @@ public class IssueBulkChangeService { return changeContext; } } + + private class Referentials { + + private final Map rules = newHashMap(); + private final Map components = newHashMap(); + private final Map projects = newHashMap(); + + public Referentials(List issues) { + Set ruleKeys = newHashSet(); + Set componentKeys = newHashSet(); + Set projectKeys = newHashSet(); + + for (Issue issue : issues) { + ruleKeys.add(issue.ruleKey()); + componentKeys.add(issue.componentKey()); + String projectKey = issue.projectKey(); + if (projectKey != null) { + projectKeys.add(projectKey); + } + } + + DbSession session = dbClient.openSession(false); + try { + for (Rule rule : ruleFinder.findByKeys(ruleKeys)) { + rules.put(rule.ruleKey(), rule); + } + + for (ComponentDto file : dbClient.componentDao().getByKeys(session, componentKeys)) { + components.put(file.getKey(), file); + } + + for (ComponentDto project : dbClient.componentDao().getByKeys(session, projectKeys)) { + projects.put(project.getKey(), project); + } + } finally { + session.close(); + } + } + + public Rule rule(RuleKey ruleKey) { + return rules.get(ruleKey); + } + + @CheckForNull + public ComponentDto component(String key) { + return components.get(key); + } + + public ComponentDto project(String key) { + return projects.get(key); + } + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 9314309d122..4b7b313047f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -61,4 +61,6 @@ public interface IssueService extends ServerComponent { DefaultIssue getIssueByKey(String key); + List search(List issues); + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/OldIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/OldIssueService.java index 5e07254f34c..fdb7fa9f563 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/OldIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/OldIssueService.java @@ -307,4 +307,9 @@ public class OldIssueService implements IssueService { public DefaultIssue getIssueByKey(String key) { return (DefaultIssue) loadIssue(key).first(); } + + @Override + public List search(List issues) { + return finder.find(IssueQuery.builder().issueKeys(issues).pageSize(-1).requiredRole(UserRole.USER).build()).issues(); + } } 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 d34959bdb44..f5b67dfca6d 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 @@ -122,12 +122,15 @@ public class IssueIndex extends BaseIndex { } public Result search(IssueQuery query, QueryContext options) { - SearchRequestBuilder esSearch = getClient() .prepareSearch(this.getIndexName()) .setTypes(this.getIndexType()) .setIndices(this.getIndexName()); + // Integrate Pagination + esSearch.setFrom(options.getOffset()); + esSearch.setSize(options.getLimit()); + if (options.isScroll()) { esSearch.setSearchType(SearchType.SCAN); esSearch.setScroll(TimeValue.timeValueMinutes(3)); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java b/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java index 319e0b8e6b3..ef87de0f602 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java @@ -71,6 +71,18 @@ public class DefaultRuleFinder implements RuleFinder { return rules; } + @CheckForNull + public Collection findByKeys(Collection ruleKeys) { + List rules = newArrayList(); + if (ruleKeys.isEmpty()) { + return rules; + } + for (Rule rule : index.getByKeys(ruleKeys)) { + rules.add(toRule(rule)); + } + return rules; + } + @CheckForNull public org.sonar.api.rules.Rule findByKey(RuleKey key) { Rule rule = index.getNullableByKey(key); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueServiceMediumTest.java index 708b5dc80cb..efd6cf718be 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueServiceMediumTest.java @@ -61,6 +61,7 @@ import java.util.Date; import java.util.List; import java.util.UUID; +import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; @@ -375,6 +376,16 @@ public class DefaultIssueServiceMediumTest { assertThat(result.count("UNKNOWN")).isEqualTo(0); } + @Test + public void search_issues() { + IssueDto issue = newIssue(); + tester.get(IssueDao.class).insert(session, issue); + session.commit(); + + List result = service.search(newArrayList(issue.getKey())); + assertThat(result).hasSize(1); + } + private IssueDto newIssue() { return new IssueDto() .setIssueCreationDate(DateUtils.parseDate("2014-09-04")) diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java new file mode 100644 index 00000000000..214a811d881 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java @@ -0,0 +1,183 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.issue; + +import com.google.common.base.Joiner; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.sonar.api.issue.DefaultTransitions; +import org.sonar.api.issue.Issue; +import org.sonar.api.resources.Qualifiers; +import org.sonar.api.resources.Scopes; +import org.sonar.api.security.DefaultGroups; +import org.sonar.api.web.UserRole; +import org.sonar.core.component.ComponentDto; +import org.sonar.core.issue.db.IssueDto; +import org.sonar.core.permission.PermissionFacade; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.rule.RuleDto; +import org.sonar.core.user.UserDto; +import org.sonar.server.component.SnapshotTesting; +import org.sonar.server.component.db.ComponentDao; +import org.sonar.server.component.db.SnapshotDao; +import org.sonar.server.db.DbClient; +import org.sonar.server.issue.db.IssueDao; +import org.sonar.server.rule.RuleTesting; +import org.sonar.server.rule.db.RuleDao; +import org.sonar.server.search.IndexClient; +import org.sonar.server.tester.ServerTester; +import org.sonar.server.user.MockUserSession; +import org.sonar.server.user.UserSession; + +import java.util.Date; +import java.util.List; +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; + +public class IssueBulkChangeServiceMediumTest { + + @ClassRule + public static ServerTester tester = new ServerTester() + .setProperty("sonar.issues.use_es_backend", "true"); + + DbClient db; + IndexClient indexClient; + DbSession session; + IssueBulkChangeService service; + + RuleDto rule; + ComponentDto project; + ComponentDto file; + + UserSession userSession; + + @Before + public void setUp() throws Exception { + tester.clearDbAndIndexes(); + db = tester.get(DbClient.class); + indexClient = tester.get(IndexClient.class); + session = db.openSession(false); + service = tester.get(IssueBulkChangeService.class); + + rule = RuleTesting.newXooX1(); + tester.get(RuleDao.class).insert(session, rule); + + project = new ComponentDto() + .setKey("MyProject") + .setLongName("My Project") + .setQualifier(Qualifiers.PROJECT) + .setScope(Scopes.PROJECT); + tester.get(ComponentDao.class).insert(session, project); + tester.get(SnapshotDao.class).insert(session, SnapshotTesting.createForComponent(project)); + + file = new ComponentDto() + .setProjectId(project.getId()) + .setSubProjectId(project.getId()) + .setKey("MyComponent") + .setLongName("My Component"); + tester.get(ComponentDao.class).insert(session, file); + tester.get(SnapshotDao.class).insert(session, SnapshotTesting.createForComponent(file)); + + // project can be seen by anyone + tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), DefaultGroups.ANYONE, UserRole.USER, session); + db.issueAuthorizationDao().synchronizeAfter(session, new Date(0)); + + userSession = MockUserSession.set() + .setLogin("john") + .addProjectPermissions(UserRole.USER, project.key()); + + session.commit(); + } + + @After + public void after() { + session.close(); + } + + @Test + public void bulk_change() throws Exception { + UserDto user = new UserDto().setLogin("fred").setName("Fred"); + db.userDao().insert(session, user); + + IssueDto issue1 = IssueTesting.newDto(rule, file, project).setAssignee("fabrice"); + IssueDto issue2 = IssueTesting.newDto(rule, file, project).setAssignee("simon"); + tester.get(IssueDao.class).insert(session, issue1, issue2); + session.commit(); + + Map properties = newHashMap(); + properties.put("issues", issue1.getKey() + "," + issue2.getKey()); + properties.put("actions", "assign"); + properties.put("assign.assignee", user.getLogin()); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true); + IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession); + assertThat(result.issuesChanged()).hasSize(2); + assertThat(result.issuesNotChanged()).isEmpty(); + } + + @Test + public void bulk_change_on_500_issues() throws Exception { + List issueKeys = newArrayList(); + for (int i=0; i<500; i++) { + IssueDto issue = IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_OPEN); + tester.get(IssueDao.class).insert(session, issue); + issueKeys.add(issue.getKey()); + } + session.commit(); + + Map properties = newHashMap(); + properties.put("issues", Joiner.on(",").join(issueKeys)); + properties.put("actions", "do_transition"); + properties.put("do_transition.transition", DefaultTransitions.CONFIRM); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, false); + IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession); + assertThat(result.issuesChanged()).hasSize(500); + assertThat(result.issuesNotChanged()).isEmpty(); + } + + @Test + public void fail_if_bulk_change_on_more_than_500_issues() throws Exception { + List issueKeys = newArrayList(); + for (int i=0; i<510; i++) { + issueKeys.add("issue-" + i); + } + + Map properties = newHashMap(); + properties.put("issues", Joiner.on(",").join(issueKeys)); + properties.put("actions", "do_transition"); + properties.put("do_transition.transition", DefaultTransitions.CONFIRM); + + try { + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true); + service.execute(issueBulkChangeQuery, userSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Number of issue keys must be less than 500 (got 510)"); + } + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java index 14a71875b45..5e91277fd42 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java @@ -20,22 +20,25 @@ package org.sonar.server.issue; -import org.sonar.core.preview.PreviewCache; - import org.junit.Before; import org.junit.Test; -import org.mockito.ArgumentCaptor; import org.sonar.api.issue.Issue; -import org.sonar.api.issue.IssueQuery; -import org.sonar.api.issue.IssueQueryResult; import org.sonar.api.issue.condition.Condition; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; -import org.sonar.api.web.UserRole; +import org.sonar.api.resources.Qualifiers; +import org.sonar.api.resources.Scopes; +import org.sonar.api.rules.Rule; +import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.IssueNotifications; import org.sonar.core.issue.db.IssueStorage; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.preview.PreviewCache; +import org.sonar.server.component.db.ComponentDao; +import org.sonar.server.db.DbClient; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.user.MockUserSession; import org.sonar.server.user.UserSession; @@ -44,43 +47,70 @@ import java.util.Map; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Sets.newHashSet; 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.anyListOf; import static org.mockito.Matchers.anyMap; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; public class IssueBulkChangeServiceTest { - private DefaultIssueFinder finder = mock(DefaultIssueFinder.class); - private IssueStorage issueStorage = mock(IssueStorage.class); - private IssueNotifications issueNotifications = mock(IssueNotifications.class); + DbClient dbClient = mock(DbClient.class); + DbSession dbSession = mock(DbSession.class); + + IssueService issueService = mock(IssueService.class); + IssueStorage issueStorage = mock(IssueStorage.class); + DefaultRuleFinder ruleFinder = mock(DefaultRuleFinder.class); + ComponentDao componentDao = mock(ComponentDao.class); + IssueNotifications issueNotifications = mock(IssueNotifications.class); + + IssueBulkChangeService service; - private IssueQueryResult issueQueryResult = mock(IssueQueryResult.class); - private UserSession userSession = MockUserSession.create().setLogin("john").setUserId(10); - private DefaultIssue issue = new DefaultIssue().setKey("ABCD"); + UserSession userSession = MockUserSession.create().setLogin("john").setUserId(10); - private IssueBulkChangeService service; + DefaultIssue issue; + Rule rule; + ComponentDto project; + ComponentDto file; - private List actions; + List actions; @Before public void before() { - when(finder.find(any(IssueQuery.class))).thenReturn(issueQueryResult); - when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) issue)); + when(dbClient.openSession(false)).thenReturn(dbSession); + when(dbClient.componentDao()).thenReturn(componentDao); + + rule = Rule.create("repo", "key"); + when(ruleFinder.findByKeys(newHashSet(rule.ruleKey()))).thenReturn(newArrayList(rule)); + + project = new ComponentDto() + .setId(1L) + .setKey("MyProject") + .setLongName("My Project") + .setQualifier(Qualifiers.PROJECT) + .setScope(Scopes.PROJECT); + when(componentDao.getByKeys(dbSession, newHashSet(project.key()))).thenReturn(newArrayList(project)); + + file = new ComponentDto() + .setId(2L) + .setProjectId(project.getId()) + .setSubProjectId(project.getId()) + .setKey("MyComponent") + .setLongName("My Component"); + when(componentDao.getByKeys(dbSession, newHashSet(file.key()))).thenReturn(newArrayList(file)); + + issue = new DefaultIssue() + .setKey("ABCD") + .setRuleKey(rule.ruleKey()) + .setProjectKey(project.key()) + .setComponentKey(file.key()); + when(issueService.search(anyListOf(String.class))).thenReturn(newArrayList((Issue) issue)); actions = newArrayList(); - - service = new IssueBulkChangeService(finder, issueStorage, issueNotifications, actions, mock(PreviewCache.class)); + service = new IssueBulkChangeService(dbClient, issueService, issueStorage, ruleFinder, issueNotifications, actions, mock(PreviewCache.class)); } @Test @@ -98,7 +128,7 @@ public class IssueBulkChangeServiceTest { verify(issueStorage).save(eq(issue)); verifyNoMoreInteractions(issueStorage); - verify(issueNotifications).sendChanges(eq(issue), any(IssueChangeContext.class), eq(issueQueryResult)); + verify(issueNotifications).sendChanges(eq(issue), any(IssueChangeContext.class), eq(rule), eq(project), eq(file)); verifyNoMoreInteractions(issueNotifications); } @@ -117,8 +147,7 @@ public class IssueBulkChangeServiceTest { verify(issueStorage).save(eq(issue)); verifyNoMoreInteractions(issueStorage); - verify(issueNotifications, never()).sendChanges(eq(issue), any(IssueChangeContext.class), eq(issueQueryResult)); - verifyNoMoreInteractions(issueNotifications); + verifyZeroInteractions(issueNotifications); } @Test @@ -147,7 +176,7 @@ public class IssueBulkChangeServiceTest { @Test public void should_execute_bulk_change_with_comment_only_on_changed_issues() { - when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) new DefaultIssue().setKey("ABCD"), new DefaultIssue().setKey("EFGH"))); + when(issueService.search(anyListOf(String.class))).thenReturn(newArrayList((Issue) new DefaultIssue().setKey("ABCD"), new DefaultIssue().setKey("EFGH"))); Map properties = newHashMap(); properties.put("issues", "ABCD,EFGH"); @@ -197,29 +226,10 @@ public class IssueBulkChangeServiceTest { verify(issueStorage, times(1)).save(eq(issue)); verifyNoMoreInteractions(issueStorage); - verify(issueNotifications, times(1)).sendChanges(eq(issue), any(IssueChangeContext.class), eq(issueQueryResult)); + verify(issueNotifications).sendChanges(eq(issue), any(IssueChangeContext.class), eq(rule), eq(project), eq(file)); verifyNoMoreInteractions(issueNotifications); } - @Test - public void should_load_issues_from_issue_keys_with_maximum_page_size() { - Map properties = newHashMap(); - properties.put("issues", "ABCD,DEFG"); - properties.put("actions", "assign"); - properties.put("assign.assignee", "fred"); - actions.add(new MockAction("assign")); - - IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true); - service.execute(issueBulkChangeQuery, userSession); - - ArgumentCaptor captor = ArgumentCaptor.forClass(IssueQuery.class); - verify(finder).find(captor.capture()); - IssueQuery query = captor.getValue(); - assertThat(query.issueKeys()).containsOnly("ABCD", "DEFG"); - assertThat(query.pageSize()).isEqualTo(IssueQuery.MAX_PAGE_SIZE); - assertThat(query.requiredRole()).isEqualTo(UserRole.USER); - } - @Test public void should_not_execute_bulk_if_issue_does_not_support_action() { Map properties = newHashMap(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueTesting.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueTesting.java index 8711ed81324..ff23bcdd7d4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueTesting.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueTesting.java @@ -19,15 +19,42 @@ */ package org.sonar.server.issue; - import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.Severity; +import org.sonar.api.utils.DateUtils; +import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.db.IssueDto; +import org.sonar.core.rule.RuleDto; + +import java.util.UUID; import static org.fest.assertions.Assertions.assertThat; public class IssueTesting { + /** + * Full IssueDto used to feed database with fake data. Tests must not rely on the + * field contents declared here. They should override the fields they need to test, + * for example: + * 
+   *   issueDao.insert(dbSession, IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE));
+   *
+ */ + public static IssueDto newDto(RuleDto rule, ComponentDto file, ComponentDto project) { + return new IssueDto() + .setKee(UUID.randomUUID().toString()) + .setRule(rule) + .setComponent(file) + .setRootComponent(project) + .setStatus(Issue.STATUS_OPEN) + .setResolution(null) + .setSeverity(Severity.MAJOR) + .setDebt(10L) + .setIssueCreationDate(DateUtils.parseDate("2014-09-04")) + .setIssueUpdateDate(DateUtils.parseDate("2014-12-04")); + } + public static void assertIsEquivalent(IssueDto dto, Issue issue) { assertThat(issue).isNotNull(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/OldDefaultIssueServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java similarity index 99% rename from server/sonar-server/src/test/java/org/sonar/server/issue/OldDefaultIssueServiceTest.java rename to server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java index ceb1db14f4b..adc7258a1e1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/OldDefaultIssueServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java @@ -73,7 +73,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) -public class OldDefaultIssueServiceTest { +public class OldIssueServiceTest { @Mock DefaultIssueFinder finder; @@ -535,4 +535,9 @@ public class OldDefaultIssueServiceTest { assertThat(result.count("INFO")).isEqualTo(1); assertThat(result.count("UNKNOWN")).isEqualTo(0); } + + @Test + public void search_issues() { + assertThat(issueService.search(newArrayList("ABCD"))).hasSize(1); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java index fb95a3f198c..8312ebf1c98 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java @@ -39,6 +39,7 @@ import org.sonar.core.user.UserDto; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.db.DbClient; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.db.IssueDao; import org.sonar.server.rule.RuleTesting; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.search.QueryContext; @@ -47,8 +48,10 @@ import org.sonar.server.tester.ServerTester; import org.sonar.server.user.MockUserSession; import java.util.Date; +import java.util.List; import java.util.UUID; +import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; public class IssueIndexMediumTest { @@ -190,6 +193,49 @@ public class IssueIndexMediumTest { assertThat(result.getHits()).hasSize(2); } + @Test + public void search_with_paging() throws Exception { + for (int i=0; i<12; i++) { + IssueDto issue = createIssue(); + tester.get(IssueDao.class).insert(session, issue); + } + session.commit(); + + IssueQuery.Builder query = IssueQuery.builder(); + // There are 12 issues in total, with 10 issues per page, the page 2 should only contain 2 elements + Result result = index.search(query.build(), new QueryContext().setPage(2, 10)); + assertThat(result.getHits()).hasSize(2); + assertThat(result.getTotal()).isEqualTo(12); + } + + @Test + public void search_with_limit() throws Exception { + for (int i=0; i<20; i++) { + IssueDto issue = createIssue(); + tester.get(IssueDao.class).insert(session, issue); + } + session.commit(); + + IssueQuery.Builder query = IssueQuery.builder(); + Result result = index.search(query.build(), new QueryContext().setLimit(20)); + assertThat(result.getHits()).hasSize(20); + } + + @Test + public void search_with_max_limit() throws Exception { + List issueKeys = newArrayList(); + for (int i=0; i<500; i++) { + IssueDto issue = createIssue(); + tester.get(IssueDao.class).insert(session, issue); + issueKeys.add(issue.getKey()); + } + session.commit(); + + IssueQuery.Builder query = IssueQuery.builder(); + Result result = index.search(query.build(), new QueryContext().setMaxLimit()); + assertThat(result.getHits()).hasSize(500); + } + @Test public void authorized_issues_on_groups() throws Exception { ComponentDto project1 = new ComponentDto() diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/DefaultRuleFinderMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/DefaultRuleFinderMediumTest.java index d014dbbe1b6..c22538525e4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/DefaultRuleFinderMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/DefaultRuleFinderMediumTest.java @@ -21,6 +21,7 @@ package org.sonar.server.rule; import org.fest.assertions.Assertions; import org.junit.*; +import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleQuery; @@ -147,6 +148,16 @@ public class DefaultRuleFinderMediumTest { assertThat(finder.findByIds(newArrayList(2))).hasSize(1); } + @Test + public void find_keys_including_removed_rule() { + assertThat(finder.findByKeys(newArrayList(RuleKey.of("checkstyle", "DisabledCheck")))).hasSize(1); + + // find rule with id 2 is REMOVED + assertThat(finder.findByKeys(newArrayList(RuleKey.of("checkstyle", "com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck")))).hasSize(1); + + assertThat(finder.findByKeys(Collections.emptyList())).isEmpty(); + } + @Test public void find_id_return_null_on_removed_rule() { // find rule with id 2 is REMOVED -- 2.39.5