]> source.dussan.org Git - sonarqube.git/commitdiff
Simplify the building of sorting criteria of ES requests
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 16 Dec 2014 14:03:01 +0000 (15:03 +0100)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 16 Dec 2014 14:11:18 +0000 (15:11 +0100)
server/sonar-server/src/main/java/org/sonar/server/es/Sorting.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java
server/sonar-server/src/test/java/org/sonar/server/es/SortingTest.java [new file with mode: 0644]

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 (file)
index 0000000..a137a69
--- /dev/null
@@ -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 <code>add</code>.
+ */
+public class Sorting {
+
+  private final ListMultimap<String, Field> fields = ArrayListMultimap.create();
+  private final List<Field> 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<Field> getFields(String name) {
+    return fields.get(name);
+  }
+
+  public void fill(SearchRequestBuilder request, String name, boolean asc) {
+    List<Field> 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<Field> 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;
+    }
+  }
+}
index 49c9c472d581f75fdfb9e1225e8940ea47dbf17d..a01f0f9eeae994a88d67992890e411fce76a6d29 100644 (file)
@@ -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<Issue, FakeIssueDto, String> {
 
   private static final int DEFAULT_ISSUE_FACET_SIZE = 5;
 
-  private ListMultimap<String, IndexField> 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<Issue, FakeIssueDto, String> {
       .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<IndexField> 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<IndexField> toIndexFields(String sort) {
-    List<IndexField> 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 (file)
index 0000000..509b591
--- /dev/null
@@ -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<Sorting.Field> 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<SortBuilder> 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<SortBuilder> 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<SortBuilder> 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<SortBuilder> 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<SortBuilder> 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<SortBuilder> 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<SortBuilder> fields(SearchRequestBuilder request) throws IllegalAccessException {
+    SearchSourceBuilder source = request.internalBuilder();
+    return (List<SortBuilder>) 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);
+  }
+}