SONAR-3825 improve error handling + compatibility of sorting with oracle

7 files changed, 163 insertions, 36 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterEngine.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterEngine.java
index 69bc849667c..84d3211ecf2 100644
--- a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterEngine.java
+++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterEngine.java
@@ -22,7 +22,6 @@ package org.sonar.core.measure;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import org.apache.commons.lang.SystemUtils;
-import org.json.simple.parser.ParseException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.ServerComponent;
@@ -37,38 +36,49 @@ public class MeasureFilterEngine implements ServerComponent {
   private final MeasureFilterFactory factory;
   private final MeasureFilterExecutor executor;
 
+  private static final int MAX_ROWS = 5000;
+
   public MeasureFilterEngine(MeasureFilterFactory factory, MeasureFilterExecutor executor) {
     this.executor = executor;
     this.factory = factory;
   }
 
-  public List<MeasureFilterRow> execute(Map<String, Object> filterMap, @Nullable Long userId) throws ParseException {
+  public MeasureFilterResult execute(Map<String, Object> filterMap, @Nullable Long userId) {
     return execute(filterMap, userId, LoggerFactory.getLogger("org.sonar.MEASURE_FILTER"));
   }
 
   @VisibleForTesting
-  List<MeasureFilterRow> execute(Map<String, Object> filterMap, @Nullable Long userId, Logger logger) throws ParseException {
+  MeasureFilterResult execute(Map<String, Object> filterMap, @Nullable Long userId, Logger logger) {
+    long start = System.currentTimeMillis();
+    MeasureFilterResult result = new MeasureFilterResult();
     MeasureFilterContext context = new MeasureFilterContext();
     context.setUserId(userId);
     context.setData(String.format("{%s}", Joiner.on('|').withKeyValueSeparator("=").join(filterMap)));
     try {
-      long start = System.currentTimeMillis();
       MeasureFilter filter = factory.create(filterMap);
       List<MeasureFilterRow> rows = executor.execute(filter, context);
-      log(context, rows, (System.currentTimeMillis() - start), logger);
-      return rows;
+      if (rows.size() <= MAX_ROWS) {
+        result.setRows(rows);
+      } else {
+        result.setError(MeasureFilterResult.Error.TOO_MANY_RESULTS);
+      }
+      result.setDurationInMs(System.currentTimeMillis() - start);
+      log(context, result, logger);
+
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to execute filter: " + context, e);
+      result.setError(MeasureFilterResult.Error.UNKNOWN);
+      logger.error("Fail to execute measure filter: " + context, e);
     }
+    return result;
   }
 
-  private void log(MeasureFilterContext context, List<MeasureFilterRow> rows, long durationMs, Logger logger) {
+  private void log(MeasureFilterContext context, MeasureFilterResult result, Logger logger) {
     if (logger.isDebugEnabled()) {
       StringBuilder log = new StringBuilder();
       log.append(SystemUtils.LINE_SEPARATOR);
-      log.append(" filter: ").append(context.getData()).append(SystemUtils.LINE_SEPARATOR);
+      log.append("request: ").append(context.getData()).append(SystemUtils.LINE_SEPARATOR);
+      log.append(" result: ").append(result.toString()).append(SystemUtils.LINE_SEPARATOR);
       log.append("    sql: ").append(context.getSql()).append(SystemUtils.LINE_SEPARATOR);
-      log.append("results: ").append(rows.size()).append(" rows in ").append(durationMs).append("ms").append(SystemUtils.LINE_SEPARATOR);
       logger.debug(log.toString());
     }
   }
diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterResult.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterResult.java
new file mode 100644
index 00000000000..118733e9e59
--- /dev/null
+++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterResult.java
@@ -0,0 +1,82 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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.
+ *
+ * Sonar 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 Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.core.measure;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+
+public class MeasureFilterResult {
+
+  public static enum Error {
+    TOO_MANY_RESULTS, UNKNOWN
+  }
+
+  private List<MeasureFilterRow> rows = null;
+  private Error error = null;
+  private long durationInMs;
+
+  MeasureFilterResult() {
+  }
+
+  public List<MeasureFilterRow> getRows() {
+    return rows;
+  }
+
+  public Error getError() {
+    return error;
+  }
+
+  public long getDurationInMs() {
+    return durationInMs;
+  }
+
+  MeasureFilterResult setDurationInMs(long l) {
+    this.durationInMs = l;
+    return this;
+  }
+
+  MeasureFilterResult setRows(@Nullable List<MeasureFilterRow> rows) {
+    this.rows = rows;
+    return this;
+  }
+
+  MeasureFilterResult setError(@Nullable Error err) {
+    this.error = err;
+    return this;
+  }
+
+  public boolean isSuccess() {
+    return error == null;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder();
+    if (rows != null) {
+      sb.append(rows.size()).append(" rows, ");
+    }
+    if (error != null) {
+      sb.append("error=").append(error).append(", ");
+    }
+    sb.append(durationInMs).append("ms");
+    return sb.toString();
+  }
+}
diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterSql.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterSql.java
index 21a99ef5197..0da49a1f0f2 100644
--- a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterSql.java
+++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterSql.java
@@ -75,7 +75,7 @@ class MeasureFilterSql {
   }
 
   private void init() {
-    sql.append("SELECT block.id, max(block.rid) as rid, max(block.rootid) as rootid, max(sortval) as sortval");
+    sql.append("SELECT block.id, max(block.rid) AS rid, max(block.rootid) AS rootid, max(sortval) AS sortval1, CASE WHEN sortval IS NULL THEN 1 ELSE 0 END AS sortval2 ");
     for (int index = 0; index < filter.getMeasureConditions().size(); index++) {
       sql.append(", max(crit_").append(index).append(")");
     }
@@ -88,7 +88,7 @@ class MeasureFilterSql {
       appendConditionBlock(index, condition);
     }
 
-    sql.append(") block GROUP BY block.id");
+    sql.append(") block GROUP BY block.id, sortval2");
     if (!filter.getMeasureConditions().isEmpty()) {
       sql.append(" HAVING ");
       for (int index = 0; index < filter.getMeasureConditions().size(); index++) {
@@ -99,14 +99,13 @@ class MeasureFilterSql {
       }
     }
     if (filter.sort().isSortedByDatabase()) {
-      sql.append(" ORDER BY sortval ");
-      sql.append(filter.sort().isAsc() ? "ASC" : "DESC");
-      sql.append(" NULLS LAST");
+      sql.append(" ORDER BY sortval2 ASC, sortval1 ");
+      sql.append(filter.sort().isAsc() ? "ASC " : "DESC ");
     }
   }
 
   private void appendSortBlock() {
-    sql.append(" SELECT s.id, s.project_id AS rid, s.root_project_id AS rootid, ").append(filter.sort().column()).append(" AS sortval");
+    sql.append(" SELECT s.id, s.project_id AS rid, s.root_project_id AS rootid, ").append(filter.sort().column()).append(" AS sortval ");
     for (int index = 0; index < filter.getMeasureConditions().size(); index++) {
       MeasureFilterCondition condition = filter.getMeasureConditions().get(index);
       sql.append(", ").append(nullSelect(condition.metric())).append(" AS crit_").append(index);
@@ -125,7 +124,7 @@ class MeasureFilterSql {
   }
 
   private void appendConditionBlock(int conditionIndex, MeasureFilterCondition condition) {
-    sql.append(" SELECT s.id, s.project_id AS rid, s.root_project_id AS rootid, null AS sortval");
+    sql.append(" SELECT s.id, s.project_id AS rid, s.root_project_id AS rootid, null AS sortval ");
     for (int j = 0; j < filter.getMeasureConditions().size(); j++) {
       sql.append(", ");
       if (j == conditionIndex) {
diff --git a/sonar-core/src/main/java/org/sonar/core/measure/package-info.java b/sonar-core/src/main/java/org/sonar/core/measure/package-info.java
new file mode 100644
index 00000000000..6b2f8b948d7
--- /dev/null
+++ b/sonar-core/src/main/java/org/sonar/core/measure/package-info.java
@@ -0,0 +1,24 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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.
+ *
+ * Sonar 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 Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+@ParametersAreNonnullByDefault
+package org.sonar.core.measure;
+
+import javax.annotation.ParametersAreNonnullByDefault;
+
diff --git a/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterEngineTest.java b/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterEngineTest.java
index 06051a65d5d..95a4e8fc992 100644
--- a/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterEngineTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterEngineTest.java
@@ -20,29 +20,26 @@
 package org.sonar.core.measure;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
-import org.json.simple.parser.ParseException;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.slf4j.Logger;
 
-import java.util.HashMap;
 import java.util.Map;
 
+import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.argThat;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.refEq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class MeasureFilterEngineTest {
 
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
   @Test
   public void should_create_and_execute_filter() throws Exception {
-    Map<String,Object> filterMap = ImmutableMap.of("qualifiers", (Object) "TRK");
+    Map<String, Object> filterMap = ImmutableMap.of("qualifiers", (Object) "TRK");
     MeasureFilterFactory factory = mock(MeasureFilterFactory.class);
     MeasureFilter filter = new MeasureFilter();
     when(factory.create(filterMap)).thenReturn(filter);
@@ -67,16 +64,16 @@ public class MeasureFilterEngineTest {
   }
 
   @Test
-  public void throw_definition_of_filter_on_error() throws Exception {
-    thrown.expect(IllegalStateException.class);
-    thrown.expectMessage("filter={qualifiers=TRK}");
-
-    Map<String,Object> filterMap = ImmutableMap.of("qualifiers", (Object)"TRK");
+  public void keep_error_but_do_not_fail() throws Exception {
+    Map<String, Object> filterMap = ImmutableMap.of("qualifiers", (Object) "TRK");
     MeasureFilterFactory factory = mock(MeasureFilterFactory.class);
     when(factory.create(filterMap)).thenThrow(new IllegalArgumentException());
     MeasureFilterExecutor executor = mock(MeasureFilterExecutor.class);
 
     MeasureFilterEngine engine = new MeasureFilterEngine(factory, executor);
-    engine.execute(filterMap, 50L);
+    MeasureFilterResult result = engine.execute(filterMap, 50L);
+
+    assertThat(result.isSuccess()).isFalse();
+    assertThat(result.getError()).isEqualTo(MeasureFilterResult.Error.UNKNOWN);
   }
 }
diff --git a/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterExecutorTest.java b/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterExecutorTest.java
index 3416b62fa8e..4eeab0003ec 100644
--- a/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterExecutorTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/measure/MeasureFilterExecutorTest.java
@@ -46,6 +46,7 @@ public class MeasureFilterExecutorTest extends AbstractDaoTestCase {
   private static final Metric METRIC_LINES = new Metric.Builder("lines", "Lines", Metric.ValueType.INT).create().setId(1);
   private static final Metric METRIC_PROFILE = new Metric.Builder("profile", "Profile", Metric.ValueType.STRING).create().setId(2);
   private static final Metric METRIC_COVERAGE = new Metric.Builder("coverage", "Coverage", Metric.ValueType.FLOAT).create().setId(3);
+  private static final Metric METRIC_UNKNOWN = new Metric.Builder("unknown", "Unknown", Metric.ValueType.FLOAT).create().setId(4);
 
   private MeasureFilterExecutor executor;
 
@@ -224,7 +225,7 @@ public class MeasureFilterExecutorTest extends AbstractDaoTestCase {
       .setSortOnMetric(METRIC_COVERAGE).setSortAsc(false);
     List<MeasureFilterRow> rows = executor.execute(filter, new MeasureFilterContext());
 
-    // Java has coverage but not PHP
+    // Java project has coverage but not PHP
     assertThat(rows).hasSize(2);
     verifyJavaProject(rows.get(0));
     verifyPhpProject(rows.get(1));
@@ -236,7 +237,7 @@ public class MeasureFilterExecutorTest extends AbstractDaoTestCase {
         .setSortOnMetric(METRIC_COVERAGE).setSortAsc(true);
       List<MeasureFilterRow> rows = executor.execute(filter, new MeasureFilterContext());
 
-      // Java has coverage but not PHP
+      // Java project has coverage but not PHP
       assertThat(rows).hasSize(2);
       verifyJavaProject(rows.get(0));
       verifyPhpProject(rows.get(1));
@@ -245,7 +246,7 @@ public class MeasureFilterExecutorTest extends AbstractDaoTestCase {
   @Test
   public void sort_by_missing_numeric_measure() throws SQLException {
     // coverage measures are not computed
-    MeasureFilter filter = new MeasureFilter().setResourceQualifiers(Arrays.asList("CLA")).setSortOnMetric(METRIC_COVERAGE);
+    MeasureFilter filter = new MeasureFilter().setResourceQualifiers(Arrays.asList("CLA")).setSortOnMetric(METRIC_UNKNOWN);
     List<MeasureFilterRow> rows = executor.execute(filter, new MeasureFilterContext());
 
     // 2 files, random order
diff --git a/sonar-core/src/test/resources/org/sonar/core/measure/MeasureFilterExecutorTest/shared.xml b/sonar-core/src/test/resources/org/sonar/core/measure/MeasureFilterExecutorTest/shared.xml
index bd5a18fea6b..8b4a6bbc26e 100644
--- a/sonar-core/src/test/resources/org/sonar/core/measure/MeasureFilterExecutorTest/shared.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/measure/MeasureFilterExecutorTest/shared.xml
@@ -16,6 +16,12 @@
            optimized_best_value="[true]" best_value="100" direction="1" hidden="[false]"
            delete_historical_data="[null]"/>
 
+  <metrics id="4" name="unknown" val_type="FLOAT" description="Coverage" domain="Test"
+             short_name="Unknown" qualitative="[true]" user_managed="[false]" enabled="[true]" origin="JAV"
+             worst_value="[null]"
+             optimized_best_value="[true]" best_value="100" direction="1" hidden="[false]"
+             delete_historical_data="[null]"/>
+
   <!-- java project -->
   <projects kee="java_project" long_name="Java project" scope="PRJ" qualifier="TRK" name="Java project"
             id="1" root_id="[null]"
@@ -108,6 +114,14 @@
                     RULE_ID="[null]" text_value="Sonar way" tendency="[null]" measure_date="[null]" project_id="[null]"
                     alert_status="[null]" description="[null]" characteristic_id="[null]"/>
 
+  <!-- coverage of java project -->
+  <project_measures id="1006" metric_id="3" value="12.3" snapshot_id="101"
+                      url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]"
+                      variation_value_4="[null]" variation_value_5="[null]"
+                      rule_priority="[null]" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                      RULE_ID="[null]" text_value="Sonar way" tendency="[null]" measure_date="[null]" project_id="[null]"
+                      alert_status="[null]" description="[null]" characteristic_id="[null]"/>
+
   <!-- php project -->
   <projects kee="php_project" long_name="PHP project" scope="PRJ" qualifier="TRK" name="PHP project"
             id="10" root_id="[null]"