From 4e9392499d71805b80b126343e504fc6aa40dc33 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 22 Sep 2014 10:48:39 +0200 Subject: [PATCH] SONAR-5531 Bulk change is no more using IssueFinder --- .../server/issue/DefaultIssueService.java | 10 ++++- .../issue/InternalRubyIssueService.java | 23 +++++++++++ .../org/sonar/server/issue/IssueService.java | 3 ++ .../sonar/server/issue/OldIssueService.java | 8 ++-- .../issue/filter/IssueFilterService.java | 11 +++++- .../org/sonar/server/search/QueryContext.java | 2 +- .../issue/DefaultIssueServiceMediumTest.java | 10 +++++ ...java => InternalRubyIssueServiceTest.java} | 38 ++++++++++++++++++- .../server/issue/OldIssueServiceTest.java | 7 ++++ .../issue/filter/IssueFilterServiceTest.java | 13 ++++++- .../app/controllers/issues_controller.rb | 6 ++- .../views/issues/_bulk_change_form.html.erb | 15 +++----- 12 files changed, 127 insertions(+), 19 deletions(-) rename server/sonar-server/src/test/java/org/sonar/server/issue/{InternalRubyDefaultIssueServiceTest.java => InternalRubyIssueServiceTest.java} (94%) 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 a49abf0c583..cb151d9c92f 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 @@ -347,7 +347,15 @@ public class DefaultIssueService implements IssueService { } /** - * Used by the bulk change + * Warning, paging is not taken into account when using this method, max limit is set. (Only used to execute query for bulk change) + * TODO move it to the IssueFilterService when OldIssueService will be removed + */ + public List searchFromQuery(IssueQuery query) { + return indexClient.get(IssueIndex.class).search(query, new QueryContext().setMaxLimit()).getHits(); + } + + /** + * Used by the bulk change : first load issues from E/S to load only authorized issues then load issues from DB in order to update them. * TODO move it to the IssueBulkChangeService when OldIssueService will be removed */ @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index e919ba8f6dc..559e5b0ed1b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -479,6 +479,25 @@ public class InternalRubyIssueService implements ServerComponent { return issueFilterService.execute(issueQuery); } + /** + * Execute issue filter from parameters + */ + public List execute2(Map props) { + IssueQuery issueQuery = PublicRubyIssueService.toQuery(props); + return issueFilterService.execute2(issueQuery); + } + + /** + * Execute issue filter from existing filter with optional overridable parameters + */ + public List execute2(Long issueFilterId, Map overrideProps) { + DefaultIssueFilter issueFilter = issueFilterService.find(issueFilterId, UserSession.get()); + Map props = issueFilterService.deserializeIssueFilterQuery(issueFilter); + overrideProps(props, overrideProps); + IssueQuery issueQuery = PublicRubyIssueService.toQuery(props); + return issueFilterService.execute2(issueQuery); + } + private void overrideProps(Map props, Map overrideProps) { for (Map.Entry entry : overrideProps.entrySet()) { props.put(entry.getKey(), entry.getValue()); @@ -620,4 +639,8 @@ public class InternalRubyIssueService implements ServerComponent { } } + public int maxPageSize(){ + return IssueQuery.MAX_PAGE_SIZE; + } + } 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 4b7b313047f..0d6a6fc10eb 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 @@ -23,6 +23,7 @@ package org.sonar.server.issue; import com.google.common.collect.Multiset; import org.sonar.api.ServerComponent; import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.rule.RuleKey; import org.sonar.core.issue.workflow.Transition; @@ -61,6 +62,8 @@ public interface IssueService extends ServerComponent { DefaultIssue getIssueByKey(String key); + List searchFromQuery(IssueQuery query); + 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 fdb7fa9f563..665572b5208 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 @@ -231,7 +231,7 @@ public class OldIssueService implements IssueService { } Rule rule = findRule(ruleKey); - DefaultIssue issue = (DefaultIssue) new DefaultIssueBuilder() + DefaultIssue issue = new DefaultIssueBuilder() .componentKey(component.getKey()) .projectKey(project.getKey()) .line(line) @@ -288,8 +288,6 @@ public class OldIssueService implements IssueService { } public IssueQueryResult loadIssue(String issueKey) { - // TODO use IssueIndex for ACL - // TODO load DTO IssueQueryResult result = finder.find(IssueQuery.builder().issueKeys(Arrays.asList(issueKey)).requiredRole(UserRole.USER).build()); if (result.issues().size() != 1) { // TODO throw 404 @@ -308,6 +306,10 @@ public class OldIssueService implements IssueService { return (DefaultIssue) loadIssue(key).first(); } + public List searchFromQuery(IssueQuery issueQuery) { + return finder.find(issueQuery).issues(); + } + @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/filter/IssueFilterService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/filter/IssueFilterService.java index 0cc2a2a7c78..315dd89376c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/filter/IssueFilterService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/filter/IssueFilterService.java @@ -25,6 +25,7 @@ import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import org.sonar.api.ServerComponent; +import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.IssueQueryResult; @@ -40,9 +41,11 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.IssueService; import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; + import java.util.List; import java.util.Map; @@ -53,15 +56,17 @@ public class IssueFilterService implements ServerComponent { private final IssueFilterDao filterDao; private final IssueFilterFavouriteDao favouriteDao; private final IssueFinder finder; + private final IssueService issueService; private final AuthorizationDao authorizationDao; private final IssueFilterSerializer serializer; public IssueFilterService(IssueFilterDao filterDao, IssueFilterFavouriteDao favouriteDao, - IssueFinder finder, AuthorizationDao authorizationDao, + IssueFinder finder, IssueService issueService, AuthorizationDao authorizationDao, IssueFilterSerializer serializer) { this.filterDao = filterDao; this.favouriteDao = favouriteDao; this.finder = finder; + this.issueService = issueService; this.authorizationDao = authorizationDao; this.serializer = serializer; } @@ -70,6 +75,10 @@ public class IssueFilterService implements ServerComponent { return createIssueFilterResult(finder.find(issueQuery), issueQuery); } + public List execute2(IssueQuery issueQuery) { + return issueService.searchFromQuery(issueQuery); + } + public DefaultIssueFilter find(Long id, UserSession userSession) { return findIssueFilterDto(id, getLoggedLogin(userSession)).toIssueFilter(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/QueryContext.java b/server/sonar-server/src/main/java/org/sonar/server/search/QueryContext.java index b2cfae1854b..b25d52ff11b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/search/QueryContext.java +++ b/server/sonar-server/src/main/java/org/sonar/server/search/QueryContext.java @@ -31,7 +31,7 @@ import java.util.Set; import static com.google.common.collect.Sets.newHashSet; /** - * Various Elasticsearch request options: paging, sorting, fields and facets + * Various Elasticsearch request options: paging, fields and facets * * @since 4.4 */ 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 efd6cf718be..61b36812273 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 @@ -386,6 +386,16 @@ public class DefaultIssueServiceMediumTest { assertThat(result).hasSize(1); } + @Test + public void search_from_query() { + IssueDto issue = newIssue(); + tester.get(IssueDao.class).insert(session, issue); + session.commit(); + + List result = service.searchFromQuery(IssueQuery.builder().build()); + 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/InternalRubyDefaultIssueServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java similarity index 94% rename from server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyDefaultIssueServiceTest.java rename to server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java index b807c0bc5f2..a6dcce27458 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyDefaultIssueServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java @@ -59,7 +59,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) -public class InternalRubyDefaultIssueServiceTest { +public class InternalRubyIssueServiceTest { @Mock IssueService issueService; @@ -569,6 +569,37 @@ public class InternalRubyDefaultIssueServiceTest { assertThat(issueQuery.pageIndex()).isEqualTo(2); } + @Test + public void execute2_issue_filter_from_issue_query() { + service.execute2(Maps.newHashMap()); + verify(issueFilterService).execute2(any(IssueQuery.class)); + } + + @Test + public void execute2_issue_filter_from_existing_filter() { + Map props = newHashMap(); + props.put("componentRoots", "struts"); + props.put("statuses", "OPEN"); + when(issueFilterService.deserializeIssueFilterQuery(any(DefaultIssueFilter.class))).thenReturn(props); + + Map overrideProps = newHashMap(); + overrideProps.put("statuses", "CLOSED"); + overrideProps.put("resolved", true); + overrideProps.put("pageSize", 20); + overrideProps.put("pageIndex", 2); + service.execute2(10L, overrideProps); + ArgumentCaptor captor = ArgumentCaptor.forClass(IssueQuery.class); + verify(issueFilterService).execute2(captor.capture()); + verify(issueFilterService).find(eq(10L), any(UserSession.class)); + + IssueQuery issueQuery = captor.getValue(); + assertThat(issueQuery.componentRoots()).contains("struts"); + assertThat(issueQuery.statuses()).contains("CLOSED"); + assertThat(issueQuery.resolved()).isTrue(); + assertThat(issueQuery.pageSize()).isEqualTo(20); + assertThat(issueQuery.pageIndex()).isEqualTo(2); + } + @Test public void serialize_filter_query() { Map props = newHashMap(); @@ -652,6 +683,11 @@ public class InternalRubyDefaultIssueServiceTest { verify(changelogService).formatDiffs(eq(fieldDiffs)); } + @Test + public void max_query_size() { + assertThat(service.maxPageSize()).isEqualTo(500); + } + private void checkBadRequestException(Exception e, String key, Object... params) { BadRequestException exception = (BadRequestException) e; Message msg = exception.errors().messages().get(0); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java index adc7258a1e1..c594ffb8af2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/OldIssueServiceTest.java @@ -540,4 +540,11 @@ public class OldIssueServiceTest { public void search_issues() { assertThat(issueService.search(newArrayList("ABCD"))).hasSize(1); } + + @Test + public void search_from_query() { + IssueQuery query = mock(IssueQuery.class); + issueService.searchFromQuery(query); + verify(finder).find(query); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterServiceTest.java index 8248907912c..f61451b499f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/filter/IssueFilterServiceTest.java @@ -41,6 +41,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.IssueService; import org.sonar.server.user.MockUserSession; import org.sonar.server.user.UserSession; @@ -64,10 +65,11 @@ public class IssueFilterServiceTest { IssueFilterDao issueFilterDao = mock(IssueFilterDao.class); IssueFilterFavouriteDao issueFilterFavouriteDao = mock(IssueFilterFavouriteDao.class); IssueFinder issueFinder = mock(IssueFinder.class); + IssueService issueService = mock(IssueService.class); AuthorizationDao authorizationDao = mock(AuthorizationDao.class); IssueFilterSerializer issueFilterSerializer = mock(IssueFilterSerializer.class); UserSession userSession = MockUserSession.create().setLogin("john"); - IssueFilterService service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder, authorizationDao, issueFilterSerializer); + IssueFilterService service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder, issueService, authorizationDao, issueFilterSerializer); @Test public void should_find_by_id() { @@ -527,6 +529,15 @@ public class IssueFilterServiceTest { verify(issueFinder).find(issueQuery); } + @Test + public void should_execute2_from_issue_query() { + IssueQuery issueQuery = IssueQuery.builder().build(); + + service.execute2(issueQuery); + + verify(issueService).searchFromQuery(issueQuery); + } + @Test public void should_find_shared_issue_filter() { when(issueFilterDao.selectSharedFilters()).thenReturn(newArrayList( diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb index 7690cea09be..7bc3a3f5531 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb @@ -152,10 +152,12 @@ class IssuesController < ApplicationController # SONAR-4654 pagination parameters should be remove when loading issues for bulk change issues_query_params.delete('pageIndex') if params[:id] - @issue_filter_result = Internal.issues.execute(params[:id].to_i, issues_query_params) + @issues = Internal.issues.execute2(params[:id].to_i, issues_query_params) else - @issue_filter_result = Internal.issues.execute(issues_query_params) + @issues = Internal.issues.execute2(issues_query_params) end + @projects = Set.new(@issues.map {|issue| issue.projectKey()}) + render :partial => 'issues/bulk_change_form' end diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb index 168d2e5e9ea..729cd13e2cf 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb @@ -1,9 +1,6 @@ <% - issues_result = @issue_filter_result.result - - issues = issues_result.issues - max_page_size_reached = issues_result.issues.size >= issues_result.paging.pageSize() - project_key = issues_result.project(issues.to_a.first).key if !issues.empty? && issues_result.projects().to_a.size == 1 + project_key = @projects.to_a.first if !@projects.empty? && @projects.to_a.size == 1 + max_page_size_reached = @issues.size >= Internal.issues.maxPageSize() transitions_by_issues = {} unresolved_issues = 0 @@ -11,7 +8,7 @@ at_least_one_issue_is_planned = false at_least_one_issue_is_assigned = false all_issues_are_assigned_to_current_user = true - issues.each do |issue| + @issues.each do |issue| transitions = Internal.issues.listTransitions(issue) transitions.each do |transition| issues_for_transition = transitions_by_issues[transition.key] || 0 @@ -28,16 +25,16 @@ end %>
- +
-

<%= message('issue_bulk_change.form.title', {:params => issues.size.to_s}) -%>

+

<%= message('issue_bulk_change.form.title', {:params => @issues.size.to_s}) -%>

<% if max_page_size_reached %> -

<%= message('issue_bulk_change.max_issues_reached', :params => issues.size) -%>

+

<%= message('issue_bulk_change.max_issues_reached', :params => @issues.size) -%>

<% end %>
-- 2.39.5