From: Simon Brandhof Date: Tue, 16 Dec 2014 14:03:01 +0000 (+0100) Subject: Simplify the building of sorting criteria of ES requests X-Git-Tag: latest-silver-master-#65~512 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c6a9ef0c9bde8583974dd2823b4f66dc8e9f7956;p=sonarqube.git Simplify the building of sorting criteria of ES requests --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/Sorting.java b/server/sonar-server/src/main/java/org/sonar/server/es/Sorting.java new file mode 100644 index 00000000000..a137a69f4d0 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/es/Sorting.java @@ -0,0 +1,127 @@ +/* + * 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.es; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Lists; +import org.elasticsearch.action.search.SearchRequestBuilder; +import org.elasticsearch.search.sort.FieldSortBuilder; +import org.elasticsearch.search.sort.SortBuilders; +import org.elasticsearch.search.sort.SortOrder; +import org.sonar.server.exceptions.BadRequestException; + +import java.util.List; + +/** + * Construct sorting criteria of ES requests. Sortable fields must be previously + * declared with methods prefixed by add. + */ +public class Sorting { + + private final ListMultimap fields = ArrayListMultimap.create(); + private final List defaultFields = Lists.newArrayList(); + + public Field add(String name) { + Field field = new Field(name); + fields.put(name, field); + return field; + } + + public Field add(String name, String fieldName) { + Field field = new Field(fieldName); + fields.put(name, field); + return field; + } + + public Field addDefault(String fieldName) { + Field field = new Field(fieldName); + defaultFields.add(field); + return field; + } + + public List getFields(String name) { + return fields.get(name); + } + + public void fill(SearchRequestBuilder request, String name, boolean asc) { + List list = fields.get(name); + if (list.isEmpty()) { + throw new BadRequestException("Bad sort field: " + name); + } + doFill(request, list, asc); + } + + public void fillDefault(SearchRequestBuilder request) { + doFill(request, defaultFields, true); + } + + private void doFill(SearchRequestBuilder request, List fields, boolean asc) { + for (Field field : fields) { + FieldSortBuilder sortBuilder = SortBuilders.fieldSort(field.name); + boolean effectiveAsc = asc ? !field.reverse : field.reverse; + sortBuilder.order(effectiveAsc ? SortOrder.ASC : SortOrder.DESC); + boolean effectiveMissingLast = asc ? field.missingLast : !field.missingLast; + sortBuilder.missing(effectiveMissingLast ? "_last" : "_first"); + request.addSort(sortBuilder); + } + } + + public static class Field { + private final String name; + private boolean reverse = false; + private boolean missingLast = false; + + /** + * Default is missing first, same order as requested + */ + public Field(String name) { + this.name = name; + } + + /** + * Mark missing value as moved to the end of sorted results if requested sort is ascending. + */ + public Field missingLast() { + missingLast = true; + return this; + } + + /** + * Mark values as ordered in the opposite direction than the requested sort. + */ + public Field reverse() { + reverse = true; + return this; + } + + public String getName() { + return name; + } + + public boolean isReverse() { + return reverse; + } + + public boolean isMissingLast() { + return missingLast; + } + } +} 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 49c9c472d58..a01f0f9eeae 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 @@ -21,9 +21,7 @@ package org.sonar.server.issue.index; import com.google.common.base.Function; import com.google.common.base.Preconditions; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Collections2; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import org.apache.commons.lang.BooleanUtils; @@ -45,17 +43,14 @@ import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.bucket.terms.Terms.Bucket; import org.elasticsearch.search.aggregations.bucket.terms.Terms.Order; import org.elasticsearch.search.aggregations.bucket.terms.TermsBuilder; -import org.elasticsearch.search.sort.FieldSortBuilder; -import org.elasticsearch.search.sort.SortBuilders; -import org.elasticsearch.search.sort.SortOrder; import org.sonar.api.issue.Issue; import org.sonar.api.rule.Severity; +import org.sonar.server.es.Sorting; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.filter.IssueFilterParameters; 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; @@ -79,21 +74,27 @@ public class IssueIndex extends BaseIndex { private static final int DEFAULT_ISSUE_FACET_SIZE = 5; - private ListMultimap sortColumns = ArrayListMultimap.create(); + private final Sorting sorting; public IssueIndex(SearchClient client) { super(IndexDefinition.ISSUES, null, client); - sortColumns.put(IssueQuery.SORT_BY_ASSIGNEE, IssueNormalizer.IssueField.ASSIGNEE); - sortColumns.put(IssueQuery.SORT_BY_STATUS, IssueNormalizer.IssueField.STATUS); - sortColumns.put(IssueQuery.SORT_BY_SEVERITY, IssueNormalizer.IssueField.SEVERITY_VALUE); - 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); - sortColumns.put(IssueQuery.SORT_BY_FILE_LINE, IssueNormalizer.IssueField.KEY); + sorting = new Sorting(); + sorting.add(IssueQuery.SORT_BY_ASSIGNEE, IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE); + sorting.add(IssueQuery.SORT_BY_STATUS, IssueIndexDefinition.FIELD_ISSUE_STATUS); + sorting.add(IssueQuery.SORT_BY_SEVERITY, IssueIndexDefinition.FIELD_ISSUE_SEVERITY_VALUE); + sorting.add(IssueQuery.SORT_BY_CREATION_DATE, IssueIndexDefinition.FIELD_ISSUE_FUNC_CREATED_AT); + sorting.add(IssueQuery.SORT_BY_UPDATE_DATE, IssueIndexDefinition.FIELD_ISSUE_FUNC_UPDATED_AT); + sorting.add(IssueQuery.SORT_BY_CLOSE_DATE, IssueIndexDefinition.FIELD_ISSUE_FUNC_CLOSED_AT); + sorting.add(IssueQuery.SORT_BY_FILE_LINE, IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID); + sorting.add(IssueQuery.SORT_BY_FILE_LINE, IssueIndexDefinition.FIELD_ISSUE_FILE_PATH); + sorting.add(IssueQuery.SORT_BY_FILE_LINE, IssueIndexDefinition.FIELD_ISSUE_LINE); + sorting.add(IssueQuery.SORT_BY_FILE_LINE, IssueIndexDefinition.FIELD_ISSUE_SEVERITY_VALUE).reverse(); + sorting.add(IssueQuery.SORT_BY_FILE_LINE, IssueIndexDefinition.FIELD_ISSUE_KEY); + + // by default order by updated date and issue key (in order to be deterministic when same ms) + sorting.addDefault(IssueIndexDefinition.FIELD_ISSUE_FUNC_UPDATED_AT).reverse(); + sorting.addDefault(IssueIndexDefinition.FIELD_ISSUE_KEY); } @Override @@ -429,36 +430,14 @@ public class IssueIndex extends BaseIndex { .subAggregation(facetTopAggregation); } - private void setSorting(IssueQuery query, SearchRequestBuilder esSearch) { + private void setSorting(IssueQuery query, SearchRequestBuilder esRequest) { String sortField = query.sort(); if (sortField != null) { - Boolean asc = query.asc(); - List fields = toIndexFields(sortField); - for (IndexField field : fields) { - FieldSortBuilder sortBuilder = SortBuilders.fieldSort(field.sortField()); - // line is optional. When missing, it means zero. - if (asc != null && asc) { - sortBuilder.missing("_first"); - sortBuilder.order(SortOrder.ASC); - } else { - sortBuilder.missing("_last"); - sortBuilder.order(SortOrder.DESC); - } - esSearch.addSort(sortBuilder); - } + boolean asc = BooleanUtils.isTrue(query.asc()); + sorting.fill(esRequest, sortField, asc); } else { - esSearch.addSort(IssueNormalizer.IssueField.ISSUE_UPDATED_AT.sortField(), SortOrder.DESC); - // deterministic sort when exactly the same updated_at (same millisecond) - esSearch.addSort(IssueNormalizer.IssueField.KEY.sortField(), SortOrder.ASC); - } - } - - private List toIndexFields(String sort) { - List fields = sortColumns.get(sort); - if (fields != null) { - return fields; + sorting.fillDefault(esRequest); } - throw new IllegalStateException("Unknown sort field : " + sort); } protected void setPagination(QueryContext options, SearchRequestBuilder esSearch) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/SortingTest.java b/server/sonar-server/src/test/java/org/sonar/server/es/SortingTest.java new file mode 100644 index 00000000000..509b5918b64 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/es/SortingTest.java @@ -0,0 +1,169 @@ +/* + * 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.es; + +import org.apache.commons.lang.reflect.FieldUtils; +import org.elasticsearch.action.search.SearchRequestBuilder; +import org.elasticsearch.client.internal.InternalClient; +import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.sort.SortBuilder; +import org.elasticsearch.search.sort.SortOrder; +import org.junit.Test; +import org.sonar.server.exceptions.BadRequestException; + +import java.util.List; + +import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; +import static org.mockito.Mockito.mock; + +public class SortingTest { + + @Test + public void test_definition() throws Exception { + Sorting sorting = new Sorting(); + sorting.add("fileLine", "file"); + sorting.add("fileLine", "line").missingLast().reverse(); + + List fields = sorting.getFields("fileLine"); + assertThat(fields).hasSize(2); + assertThat(fields.get(0).getName()).isEqualTo("file"); + assertThat(fields.get(0).isReverse()).isFalse(); + assertThat(fields.get(0).isMissingLast()).isFalse(); + + assertThat(fields.get(1).getName()).isEqualTo("line"); + assertThat(fields.get(1).isReverse()).isTrue(); + assertThat(fields.get(1).isMissingLast()).isTrue(); + } + + @Test + public void ascending_sort_on_single_field() throws Exception { + Sorting sorting = new Sorting(); + sorting.add("updatedAt"); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fill(request, "updatedAt", true); + List fields = fields(request); + assertThat(fields).hasSize(1); + expectField(fields.get(0), "updatedAt", "_first", SortOrder.ASC); + } + + @Test + public void descending_sort_on_single_field() throws Exception { + Sorting sorting = new Sorting(); + sorting.add("updatedAt"); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fill(request, "updatedAt", false); + List fields = fields(request); + assertThat(fields).hasSize(1); + expectField(fields.get(0), "updatedAt", "_last", SortOrder.DESC); + } + + @Test + public void ascending_sort_on_single_field_with_missing_in_last_position() throws Exception { + Sorting sorting = new Sorting(); + sorting.add("updatedAt").missingLast(); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fill(request, "updatedAt", true); + List fields = fields(request); + assertThat(fields).hasSize(1); + expectField(fields.get(0), "updatedAt", "_last", SortOrder.ASC); + } + + @Test + public void descending_sort_on_single_field_with_missing_in_last_position() throws Exception { + Sorting sorting = new Sorting(); + sorting.add("updatedAt").missingLast(); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fill(request, "updatedAt", false); + List fields = fields(request); + assertThat(fields).hasSize(1); + expectField(fields.get(0), "updatedAt", "_first", SortOrder.DESC); + } + + @Test + public void sort_on_multiple_fields() throws Exception { + // asc => file asc, line asc, severity desc, key asc + Sorting sorting = new Sorting(); + sorting.add("fileLine", "file"); + sorting.add("fileLine", "line"); + sorting.add("fileLine", "severity").reverse(); + sorting.add("fileLine", "key").missingLast(); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fill(request, "fileLine", true); + List fields = fields(request); + assertThat(fields).hasSize(4); + expectField(fields.get(0), "file", "_first", SortOrder.ASC); + expectField(fields.get(1), "line", "_first", SortOrder.ASC); + expectField(fields.get(2), "severity", "_first", SortOrder.DESC); + expectField(fields.get(3), "key", "_last", SortOrder.ASC); + } + + @Test + public void fail_if_unknown_field() { + Sorting sorting = new Sorting(); + sorting.add("file"); + + try { + sorting.fill(new SearchRequestBuilder(mock(InternalClient.class)), "unknown", true); + fail(); + } catch (BadRequestException e) { + assertThat(e.getMessage()).isEqualTo("Bad sort field: unknown"); + } + } + + @Test + public void default_sorting() throws Exception { + Sorting sorting = new Sorting(); + sorting.addDefault("file"); + + SearchRequestBuilder request = new SearchRequestBuilder(mock(InternalClient.class)); + sorting.fillDefault(request); + List fields = fields(request); + assertThat(fields).hasSize(1); + } + + private void expectField(SortBuilder field, String expectedField, String expectedMissing, SortOrder expectedSort) throws IllegalAccessException { + assertThat(fieldName(field)).isEqualTo(expectedField); + assertThat(missing(field)).isEqualTo(expectedMissing); + assertThat(order(field)).isEqualTo(expectedSort); + } + + private static List fields(SearchRequestBuilder request) throws IllegalAccessException { + SearchSourceBuilder source = request.internalBuilder(); + return (List) FieldUtils.readField(source, "sorts", true); + } + + private static String fieldName(SortBuilder sortBuilder) throws IllegalAccessException { + return (String) FieldUtils.readField(sortBuilder, "fieldName", true); + } + + private static String missing(SortBuilder sortBuilder) throws IllegalAccessException { + return (String) FieldUtils.readField(sortBuilder, "missing", true); + } + + private static SortOrder order(SortBuilder sortBuilder) throws IllegalAccessException { + return (SortOrder) FieldUtils.readField(sortBuilder, "order", true); + } +}