From 867753a7e5bc62c2aa291395e90e39adf4ce902d Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 28 Oct 2014 14:49:52 +0100 Subject: [PATCH] SONAR-5787 add sorting of issues per project, file path then line id --- .../org/sonar/server/issue/IssueQuery.java | 9 ++- .../sonar/server/issue/index/IssueDoc.java | 5 ++ .../sonar/server/issue/index/IssueIndex.java | 60 +++++++++++++------ .../server/issue/index/IssueNormalizer.java | 6 +- .../org/sonar/server/issue/IssueTesting.java | 1 + .../issue/index/IssueIndexMediumTest.java | 53 +++++++++++++--- .../org/sonar/core/issue/db/IssueDto.java | 21 +++++++ .../org/sonar/core/issue/db/IssueMapper.xml | 1 + 8 files changed, 127 insertions(+), 29 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java index 41f66724da2..e285b3c9e8b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java @@ -44,7 +44,14 @@ public class IssueQuery { public static final String SORT_BY_ASSIGNEE = "ASSIGNEE"; public static final String SORT_BY_SEVERITY = "SEVERITY"; public static final String SORT_BY_STATUS = "STATUS"; - public static final Set SORTS = ImmutableSet.of(SORT_BY_CREATION_DATE, SORT_BY_UPDATE_DATE, SORT_BY_CLOSE_DATE, SORT_BY_ASSIGNEE, SORT_BY_SEVERITY, SORT_BY_STATUS); + + /** + * Sort by project, file path then line id + */ + public static final String SORT_BY_FILE_LINE = "FILE_LINE"; + + public static final Set SORTS = ImmutableSet.of(SORT_BY_CREATION_DATE, SORT_BY_UPDATE_DATE, SORT_BY_CLOSE_DATE, SORT_BY_ASSIGNEE, SORT_BY_SEVERITY, + SORT_BY_STATUS, SORT_BY_FILE_LINE); private final Collection issueKeys; private final Collection severities; diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java index f34867afd3c..5ae2f503320 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java @@ -187,4 +187,9 @@ public class IssueDoc extends BaseDoc implements Issue { Integer debt = getNullableField(IssueNormalizer.IssueField.DEBT.field()); return (debt != null) ? Duration.create(Long.valueOf(debt)) : null; } + + @CheckForNull + public String filePath() { + return getNullableField(IssueNormalizer.IssueField.FILE_PATH.field()); + } } 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 e2dae7e926c..0f31432e2c2 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 @@ -20,7 +20,9 @@ package org.sonar.server.issue.index; import com.google.common.base.Preconditions; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import org.apache.commons.lang.BooleanUtils; import org.elasticsearch.action.search.SearchRequestBuilder; @@ -28,7 +30,12 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.index.query.*; +import org.elasticsearch.index.query.BoolFilterBuilder; +import org.elasticsearch.index.query.FilterBuilder; +import org.elasticsearch.index.query.FilterBuilders; +import org.elasticsearch.index.query.OrFilterBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder; @@ -42,18 +49,28 @@ import org.sonar.api.web.UserRole; import org.sonar.core.issue.db.IssueDto; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.filter.IssueFilterParameters; -import org.sonar.server.search.*; +import org.sonar.server.search.BaseIndex; +import org.sonar.server.search.FacetValue; +import org.sonar.server.search.IndexDefinition; +import org.sonar.server.search.IndexField; +import org.sonar.server.search.QueryContext; +import org.sonar.server.search.Result; +import org.sonar.server.search.SearchClient; import javax.annotation.Nullable; -import java.util.*; +import java.util.Collection; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; import static com.google.common.collect.Lists.newArrayList; -import static com.google.common.collect.Maps.newHashMap; public class IssueIndex extends BaseIndex { - private Map sortColumns = newHashMap(); + private ListMultimap sortColumns = ArrayListMultimap.create(); public IssueIndex(IssueNormalizer normalizer, SearchClient client) { super(IndexDefinition.ISSUES, normalizer, client); @@ -64,6 +81,9 @@ public class IssueIndex extends BaseIndex { sortColumns.put(IssueQuery.SORT_BY_CREATION_DATE, IssueNormalizer.IssueField.ISSUE_CREATED_AT); sortColumns.put(IssueQuery.SORT_BY_UPDATE_DATE, IssueNormalizer.IssueField.ISSUE_UPDATED_AT); sortColumns.put(IssueQuery.SORT_BY_CLOSE_DATE, IssueNormalizer.IssueField.ISSUE_CLOSE_DATE); + sortColumns.put(IssueQuery.SORT_BY_FILE_LINE, IssueNormalizer.IssueField.PROJECT); + sortColumns.put(IssueQuery.SORT_BY_FILE_LINE, IssueNormalizer.IssueField.FILE_PATH); + sortColumns.put(IssueQuery.SORT_BY_FILE_LINE, IssueNormalizer.IssueField.LINE); } @Override @@ -185,7 +205,7 @@ public class IssueIndex extends BaseIndex { QueryBuilder esQuery = QueryBuilders.matchAllQuery(); BoolFilterBuilder esFilter = FilterBuilders.boolFilter(); Map filters = getFilters(query, options); - for (FilterBuilder filter: filters.values()) { + for (FilterBuilder filter : filters.values()) { if (filter != null) { esFilter.must(filter); } @@ -205,7 +225,7 @@ public class IssueIndex extends BaseIndex { private BoolFilterBuilder getFilter(IssueQuery query, QueryContext options) { BoolFilterBuilder esFilter = FilterBuilders.boolFilter(); - for (FilterBuilder filter: getFilters(query, options).values()) { + for (FilterBuilder filter : getFilters(query, options).values()) { if (filter != null) { esFilter.must(filter); } @@ -278,7 +298,7 @@ public class IssueIndex extends BaseIndex { FilterBuilders.boolFilter() .must(FilterBuilders.termFilter(IssueAuthorizationNormalizer.IssueAuthorizationField.PERMISSION.field(), UserRole.USER), groupsAndUser) .cache(true)) - ); + ); } private void addDatesFilter(Map filters, IssueQuery query) { @@ -383,19 +403,21 @@ public class IssueIndex extends BaseIndex { .subAggregation(facetTopAggregation); } - private void setSorting(IssueQuery query, SearchRequestBuilder esSearch) { /* integrate Query Sort */ String sortField = query.sort(); Boolean asc = query.asc(); if (sortField != null) { - FieldSortBuilder sort = SortBuilders.fieldSort(toIndexField(sortField).sortField()); - if (asc != null && asc) { - sort.order(SortOrder.ASC); - } else { - sort.order(SortOrder.DESC); + List fields = toIndexFields(sortField); + for (IndexField field : fields) { + FieldSortBuilder sort = SortBuilders.fieldSort(field.sortField()); + if (asc != null && asc) { + sort.order(SortOrder.ASC); + } else { + sort.order(SortOrder.DESC); + } + esSearch.addSort(sort); } - esSearch.addSort(sort); } else { esSearch.addSort(IssueNormalizer.IssueField.ISSUE_UPDATED_AT.sortField(), SortOrder.DESC); // deterministic sort when exactly the same updated_at (same millisecond) @@ -403,10 +425,10 @@ public class IssueIndex extends BaseIndex { } } - private IndexField toIndexField(String sort) { - IndexField indexFieldSort = sortColumns.get(sort); - if (indexFieldSort != null) { - return indexFieldSort; + private List toIndexFields(String sort) { + List fields = sortColumns.get(sort); + if (fields != null) { + return fields; } throw new IllegalStateException("Unknown sort field : " + sort); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueNormalizer.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueNormalizer.java index ee27fa1876a..94407f6de54 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueNormalizer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueNormalizer.java @@ -49,7 +49,7 @@ public class IssueNormalizer extends BaseNormalizer { public static final IndexField CREATED_AT = add(IndexField.Type.DATE, "createdAt"); public static final IndexField UPDATED_AT = add(IndexField.Type.DATE, "updatedAt"); - public static final IndexField PROJECT = add(IndexField.Type.STRING, "project"); + public static final IndexField PROJECT = addSortable(IndexField.Type.STRING, "project"); public static final IndexField COMPONENT = add(IndexField.Type.STRING, "component"); public static final IndexField MODULE = add(IndexField.Type.STRING, "module"); public static final IndexField MODULE_PATH = add(IndexField.Type.UUID_PATH, "modulePath"); @@ -63,7 +63,7 @@ public class IssueNormalizer extends BaseNormalizer { public static final IndexField ISSUE_CREATED_AT = addSortable(IndexField.Type.DATE, "issueCreatedAt"); public static final IndexField ISSUE_UPDATED_AT = addSortable(IndexField.Type.DATE, "issueUpdatedAt"); public static final IndexField ISSUE_CLOSE_DATE = addSortable(IndexField.Type.DATE, "issueClosedAt"); - public static final IndexField LINE = add(IndexField.Type.NUMERIC, "line"); + public static final IndexField LINE = addSortable(IndexField.Type.NUMERIC, "line"); public static final IndexField MESSAGE = add(IndexField.Type.STRING, "message"); public static final IndexField RESOLUTION = add(IndexField.Type.STRING, "resolution"); public static final IndexField REPORTER = add(IndexField.Type.STRING, "reporter"); @@ -72,6 +72,7 @@ public class IssueNormalizer extends BaseNormalizer { public static final IndexField SEVERITY_VALUE = addSortable(IndexField.Type.NUMERIC, "severityValue"); public static final IndexField LANGUAGE = add(IndexField.Type.STRING, "language"); public static final IndexField RULE_KEY = add(IndexField.Type.STRING, "ruleKey"); + public static final IndexField FILE_PATH = addSortable(IndexField.Type.STRING, "filePath"); public static final Set ALL_FIELDS = getAllFields(); @@ -125,6 +126,7 @@ public class IssueNormalizer extends BaseNormalizer { update.put(IssueField.DEBT.field(), dto.getDebt()); update.put(IssueField.LANGUAGE.field(), dto.getLanguage()); update.put(IssueField.RULE_KEY.field(), dto.getRuleKey().toString()); + update.put(IssueField.FILE_PATH.field(), dto.getFilePath()); /** Upsert elements */ Map upsert = getUpsertFor(IssueField.ALL_FIELDS, update); 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 c3d2d41840e..dc1eed20ebc 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 @@ -79,6 +79,7 @@ public class IssueTesting { assertThat(issue.language()).isEqualTo(dto.getLanguage()); assertThat(issue.status()).isEqualTo(dto.getStatus()); assertThat(issue.severity()).isEqualTo(dto.getSeverity()); + assertThat(issue.filePath()).isEqualTo(dto.getFilePath()); assertThat(issue.attributes()).isEqualTo(KeyValueFormat.parse(dto.getIssueAttributes())); 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 08e4f05559d..5f06084ab73 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 @@ -49,7 +49,11 @@ import org.sonar.server.issue.db.IssueDao; import org.sonar.server.platform.BackendCleanup; import org.sonar.server.rule.RuleTesting; import org.sonar.server.rule.db.RuleDao; -import org.sonar.server.search.*; +import org.sonar.server.search.FacetValue; +import org.sonar.server.search.IndexDefinition; +import org.sonar.server.search.QueryContext; +import org.sonar.server.search.Result; +import org.sonar.server.search.SearchClient; import org.sonar.server.tester.ServerTester; import org.sonar.server.user.MockUserSession; @@ -420,7 +424,7 @@ public class IssueIndexMediumTest { IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_OPEN), IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_CLOSED), IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_REOPENED) - ); + ); session.commit(); IssueQuery.Builder query = IssueQuery.builder().sort(IssueQuery.SORT_BY_STATUS).asc(true); @@ -444,7 +448,7 @@ public class IssueIndexMediumTest { IssueTesting.newDto(rule, file, project).setSeverity(Severity.MINOR), IssueTesting.newDto(rule, file, project).setSeverity(Severity.CRITICAL), IssueTesting.newDto(rule, file, project).setSeverity(Severity.MAJOR) - ); + ); session.commit(); IssueQuery.Builder query = IssueQuery.builder().sort(IssueQuery.SORT_BY_SEVERITY).asc(true); @@ -547,6 +551,41 @@ public class IssueIndexMediumTest { assertThat(result.getHits().get(2).closeDate()).isNull(); } + @Test + public void sort_by_file_and_line() throws Exception { + db.issueDao().insert(session, + // file Foo.java + IssueTesting.newDto(rule, file, project).setLine(20).setFilePath("src/Foo.java"), + IssueTesting.newDto(rule, file, project).setLine(null).setFilePath("src/Foo.java"), + IssueTesting.newDto(rule, file, project).setLine(25).setFilePath("src/Foo.java"), + + // file Bar.java + IssueTesting.newDto(rule, file, project).setLine(9).setFilePath("src/Bar.java"), + IssueTesting.newDto(rule, file, project).setLine(109).setFilePath("src/Bar.java") + ); + session.commit(); + + // ascending sort -> Bar then Foo + IssueQuery.Builder query = IssueQuery.builder().sort(IssueQuery.SORT_BY_FILE_LINE).asc(true); + Result result = index.search(query.build(), new QueryContext()); + assertThat(result.getHits()).hasSize(5); + assertThat(result.getHits().get(0).line()).isEqualTo(9); + assertThat(result.getHits().get(1).line()).isEqualTo(109); + assertThat(result.getHits().get(2).line()).isEqualTo(20); + assertThat(result.getHits().get(3).line()).isEqualTo(25); + assertThat(result.getHits().get(4).line()).isEqualTo(null); + + // descending sort -> Foo then Bar + query = IssueQuery.builder().sort(IssueQuery.SORT_BY_FILE_LINE).asc(false); + result = index.search(query.build(), new QueryContext()); + assertThat(result.getHits()).hasSize(5); + assertThat(result.getHits().get(0).line()).isEqualTo(25); + assertThat(result.getHits().get(1).line()).isEqualTo(20); + assertThat(result.getHits().get(2).line()).isEqualTo(null); + assertThat(result.getHits().get(3).line()).isEqualTo(109); + assertThat(result.getHits().get(4).line()).isEqualTo(9); + } + @Test public void authorized_issues_on_groups() throws Exception { ComponentDto project1 = ComponentTesting.newProjectDto().setKey("project1"); @@ -629,7 +668,7 @@ public class IssueIndexMediumTest { IssueTesting.newDto(rule, file1, project1), IssueTesting.newDto(rule, file2, project2), IssueTesting.newDto(rule, file2, project3) - ); + ); session.commit(); session.clearCache(); @@ -696,7 +735,8 @@ public class IssueIndexMediumTest { assertThat(db.issueDao().findAfterDate(session, new Date(0))).hasSize(numberOfIssues); assertThat(index.countAll()).isEqualTo(numberOfIssues); - // Clear issue index in order to simulate these issues have been inserted without being indexed in E/S (from a previous version of SQ or from batch) + // Clear issue index in order to simulate these issues have been inserted without being indexed in E/S (from a previous version of SQ or + // from batch) tester.get(BackendCleanup.class).clearIndex(IndexDefinition.ISSUES); tester.clearIndexes(); assertThat(index.countAll()).isEqualTo(0); @@ -722,7 +762,7 @@ public class IssueIndexMediumTest { IssueTesting.newDto(rule, file, project).setAssignee("steph").setStatus(Issue.STATUS_OPEN), // julien should not be returned as the issue is closed IssueTesting.newDto(rule, file, project).setAssignee("julien").setStatus(Issue.STATUS_CLOSED) - ); + ); session.commit(); List results = index.listAssignees(IssueQuery.builder().statuses(newArrayList(Issue.STATUS_OPEN)).build()); @@ -738,7 +778,6 @@ public class IssueIndexMediumTest { assertThat(results.get(2).getValue()).isEqualTo(1); } - @Test public void index_has_4_shards() { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java index 27b24331f05..378c2fca6b2 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java @@ -84,6 +84,7 @@ public final class IssueDto extends Dto implements Serializable { private String moduleUuidPath; private String projectKey; private String projectUuid; + private String filePath; @Override public String getKey() { @@ -118,6 +119,7 @@ public final class IssueDto extends Dto implements Serializable { this.componentUuid = component.uuid(); this.moduleUuid = component.moduleUuid(); this.moduleUuidPath = component.moduleUuidPath(); + this.filePath = component.path(); return this; } @@ -489,6 +491,25 @@ public final class IssueDto extends Dto implements Serializable { return this; } + /** + * Should only be used to persist in E/S + * + * Please use {@link #setProject(org.sonar.core.component.ComponentDto)} instead + */ + public String getFilePath() { + return filePath; + } + + /** + * Should only be used to persist in E/S + * + * Please use {@link #setProject(org.sonar.core.component.ComponentDto)} instead + */ + public IssueDto setFilePath(String filePath) { + this.filePath = filePath; + return this; + } + @Override public String toString() { return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml index 0fe245154a1..0675a5e83b1 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml @@ -36,6 +36,7 @@ p.uuid as componentUuid, p.module_uuid as moduleUuid, p.module_uuid_path as moduleUuidPath, + p.path as filePath, root.kee as projectKey, root.uuid as projectUuid -- 2.39.5