Fix some quality flaws

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/BaseSqlStatement.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/BaseSqlStatement.java
index f04e2da2078dd70f4e8ebeb43c6138d6170e7084..7992219f65753ad256e9da0893c1b9460114a167 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/BaseSqlStatement.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/BaseSqlStatement.java
@@ -28,10 +28,10 @@ import java.sql.Timestamp;
 import java.sql.Types;
 import java.util.Date;
 
-abstract class BaseSqlStatement<CHILD extends SqlStatement> implements SqlStatement<CHILD> {
+class BaseSqlStatement<CHILD extends SqlStatement> implements SqlStatement<CHILD> {
   protected PreparedStatement pstmt;
 
-  BaseSqlStatement(PreparedStatement pstmt) {
+  protected BaseSqlStatement(PreparedStatement pstmt) {
     this.pstmt = pstmt;
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/MassUpdate.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/MassUpdate.java
index 46e00c7eb621c66ec12c1e3113c27e6edcf445fc..8013d9cff81a6a633ef57ebbec559351c3b7b8e9 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/MassUpdate.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/MassUpdate.java
@@ -34,6 +34,7 @@ public class MassUpdate {
   public static interface Handler {
     /**
      * Convert some column values of a given row.
+     *
      * @return true if the row must be updated, else false. If false, then the update parameter must not be touched.
      */
     boolean handle(Select.Row row, SqlStatement update) throws SQLException;
@@ -99,8 +100,8 @@ public class MassUpdate {
   }
 
   static class ProgressTask extends TimerTask {
-    static final long PERIOD_MS = 60000L;
-    private final Logger logger = LoggerFactory.getLogger("DbMigration");
+    private static final Logger LOGGER = LoggerFactory.getLogger("DbMigration");
+    private static final long PERIOD_MS = 60000L;
     private final AtomicLong counter;
     private String rowName = "rows";
 
@@ -118,7 +119,7 @@ public class MassUpdate {
     }
 
     void log() {
-      logger.info(String.format("%d %s processed", counter.get(), rowName));
+      LOGGER.info(String.format("%d %s processed", counter.get(), rowName));
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/Select.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/Select.java
index 57eb407d38aa0e7a3b7f0b21e16513054e086ff5..a2307474d5dc4eca8b7acee1604d6b5f6c092985 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/Select.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/Select.java
@@ -74,7 +74,6 @@ public interface Select extends SqlStatement<Select> {
   }
 
   static interface RowReader<T> {
-    RowReader<Long> LONG = new LongReader();
     T read(Row row) throws SQLException;
   }
 
@@ -88,6 +87,8 @@ public interface Select extends SqlStatement<Select> {
     }
   }
 
+  static final RowReader<Long> LONG_READER = new LongReader();
+
   static interface RowHandler<T> {
     void handle(Row row) throws SQLException;
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigration.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigration.java
index 5f419da982f19e47dbfcc6eca732f48ed1bfe76c..08746ac32c11f5f92b5dd1dde54f2b897e8566c3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigration.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigration.java
@@ -48,7 +48,7 @@ public class DevelopmentCostMeasuresMigration extends BaseDataChange {
   public void execute(Context context) throws SQLException {
     workDurationConvertor.init();
 
-    Long metricId = context.prepareSelect("select id from metrics where name='development_cost'").get(Select.RowReader.LONG);
+    Long metricId = context.prepareSelect("select id from metrics where name='development_cost'").get(Select.LONG_READER);
     if (metricId != null) {
       MassUpdate massUpdate = context.prepareMassUpdate();
       massUpdate.select("select id, value from project_measures where metric_id=? and value is not null").setLong(1, metricId);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/TechnicalDebtMeasuresMigration.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/TechnicalDebtMeasuresMigration.java
index 37a43d0ed2f5eaa97cc5414a46660914d34dc9a3..e5bf642069d923c67e0108d73bbce14e2f88d720 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/TechnicalDebtMeasuresMigration.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v43/TechnicalDebtMeasuresMigration.java
@@ -55,7 +55,7 @@ public class TechnicalDebtMeasuresMigration extends BaseDataChange {
       "where name='sqale_index' or name='new_technical_debt' " +
       "or name='sqale_effort_to_grade_a' or name='sqale_effort_to_grade_b' or name='sqale_effort_to_grade_c' " +
       "or name='sqale_effort_to_grade_d' or name='blocker_remediation_cost' or name='critical_remediation_cost' " +
-      "or name='major_remediation_cost' or name='minor_remediation_cost' or name='info_remediation_cost'").list(Select.RowReader.LONG);
+      "or name='major_remediation_cost' or name='minor_remediation_cost' or name='info_remediation_cost'").list(Select.LONG_READER);
 
     if (!metricIds.isEmpty()) {
       MassUpdate massUpdate = context.prepareMassUpdate();
@@ -71,35 +71,37 @@ public class TechnicalDebtMeasuresMigration extends BaseDataChange {
       }
       massUpdate.update("UPDATE project_measures SET value=?," +
         "variation_value_1=?,variation_value_2=?,variation_value_3=?,variation_value_4=?,variation_value_5=? WHERE id=?");
-      massUpdate.execute(new MassUpdate.Handler() {
-        @Override
-        public boolean handle(Select.Row row, SqlStatement update) throws SQLException {
-          Long id = row.getLong(1);
-          Double value = row.getDouble(2);
-          Double var1 = row.getDouble(3);
-          Double var2 = row.getDouble(4);
-          Double var3 = row.getDouble(5);
-          Double var4 = row.getDouble(6);
-          Double var5 = row.getDouble(7);
-
-          update.setLong(1, convertDebtForDays(value));
-          update.setLong(2, convertDebtForDays(var1));
-          update.setLong(3, convertDebtForDays(var2));
-          update.setLong(4, convertDebtForDays(var3));
-          update.setLong(5, convertDebtForDays(var4));
-          update.setLong(6, convertDebtForDays(var5));
-          update.setLong(7, id);
-          return true;
-        }
-      });
+      massUpdate.execute(new Converter());
     }
   }
 
-  @CheckForNull
-  private Long convertDebtForDays(@Nullable Double data) {
-    if (data != null) {
-      return workDurationConvertor.createFromDays(data);
+  private class Converter implements MassUpdate.Handler {
+    @Override
+    public boolean handle(Select.Row row, SqlStatement update) throws SQLException {
+      Long id = row.getLong(1);
+      Double value = row.getDouble(2);
+      Double var1 = row.getDouble(3);
+      Double var2 = row.getDouble(4);
+      Double var3 = row.getDouble(5);
+      Double var4 = row.getDouble(6);
+      Double var5 = row.getDouble(7);
+
+      update.setLong(1, convertDebtForDays(value));
+      update.setLong(2, convertDebtForDays(var1));
+      update.setLong(3, convertDebtForDays(var2));
+      update.setLong(4, convertDebtForDays(var3));
+      update.setLong(5, convertDebtForDays(var4));
+      update.setLong(6, convertDebtForDays(var5));
+      update.setLong(7, id);
+      return true;
+    }
+
+    @CheckForNull
+    private Long convertDebtForDays(@Nullable Double data) {
+      if (data != null) {
+        return workDurationConvertor.createFromDays(data);
+      }
+      return null;
     }
-    return null;
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/BaseDataChangeTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/BaseDataChangeTest.java
index c8190fe267f5e9a83138e24065c4f7067734dc7b..f34f3675c7618070171b438f69f39af5a1a81cc9 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/BaseDataChangeTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/BaseDataChangeTest.java
@@ -29,6 +29,7 @@ import org.sonar.core.persistence.TestDatabase;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
@@ -47,14 +48,20 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   public void query() throws Exception {
     db.prepareDbUnit(getClass(), "persons.xml");
 
-    final List<Long> ids = new ArrayList<Long>();
+    final AtomicBoolean executed = new AtomicBoolean(false);
     new BaseDataChange(db.database()) {
       @Override
       public void execute(Context context) throws SQLException {
-        ids.addAll(context.prepareSelect("select id from persons order by id desc").list(Select.RowReader.LONG));
+        assertThat(context.prepareSelect("select id from persons order by id desc").list(Select.LONG_READER))
+          .containsExactly(3L, 2L, 1L);
+        assertThat(context.prepareSelect("select id from persons where id=?").setLong(1, 2L).get(Select.LONG_READER))
+          .isEqualTo(2L);
+        assertThat(context.prepareSelect("select id from persons where id=?").setLong(1, 12345L).get(Select.LONG_READER))
+          .isNull();
+        executed.set(true);
       }
     }.execute();
-    assertThat(ids).containsExactly(3L, 2L, 1L);
+    assertThat(executed.get()).isTrue();
   }
 
   @Test
@@ -87,7 +94,7 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
     new BaseDataChange(db.database()) {
       @Override
       public void execute(Context context) throws SQLException {
-        ids.addAll(context.prepareSelect("select id from persons where id>=?").setLong(1, 2L).list(Select.RowReader.LONG));
+        ids.addAll(context.prepareSelect("select id from persons where id>=?").setLong(1, 2L).list(Select.LONG_READER));
       }
     }.execute();
     assertThat(ids).containsOnly(2L, 3L);
@@ -102,7 +109,7 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
       @Override
       public void execute(Context context) throws SQLException {
         // parameter value is not set
-        ids.addAll(context.prepareSelect("select id from persons where id>=?").list(Select.RowReader.LONG));
+        ids.addAll(context.prepareSelect("select id from persons where id>=?").list(Select.LONG_READER));
       }
     };
     try {
@@ -332,7 +339,7 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   static class UserReader implements Select.RowReader<Object[]> {
     @Override
     public Object[] read(Select.Row row) throws SQLException {
-      return new Object[] {
+      return new Object[]{
         // id, login, age, enabled
         row.getLong(1),
         row.getString(2),

diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest.java
index 5faeeba177f82561d16c4c9bb3f7963c89f0569e..2c4e061c3a31b71be9dc1c0b944485fbd1505c6c 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest.java
@@ -59,4 +59,12 @@ public class DevelopmentCostMeasuresMigrationTest {
     db.assertDbUnit(getClass(), "migrate_dev_cost_measures_result.xml", "project_measures");
   }
 
+  @Test
+  public void metric_does_not_exist() throws Exception {
+    db.prepareDbUnit(getClass(), "metric_does_not_exist.xml");
+
+    migration.execute();
+
+    db.assertDbUnit(getClass(), "metric_does_not_exist_result.xml", "project_measures");
+  }
 }

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist.xml b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist.xml
new file mode 100644 (file)
index 0000000..0fb812f
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist.xml
@@ -0,0 +1,12 @@
+<dataset>
+
+  <!-- ncloc -->
+  <metrics delete_historical_data="[null]" id="1" name="ncloc" VAL_TYPE="STRING" DESCRIPTION="[null]"  domain="[null]" short_name=""
+           enabled="true" worst_value="[null]" optimized_best_value="[null]" best_value="[null]" direction="0" hidden="false"/>
+
+  <project_measures id="1" VALUE="1.0" METRIC_ID="1" SNAPSHOT_ID="1000" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]" person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+</dataset>

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist_result.xml b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist_result.xml
new file mode 100644 (file)
index 0000000..0fb812f
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v43/DevelopmentCostMeasuresMigrationTest/metric_does_not_exist_result.xml
@@ -0,0 +1,12 @@
+<dataset>
+
+  <!-- ncloc -->
+  <metrics delete_historical_data="[null]" id="1" name="ncloc" VAL_TYPE="STRING" DESCRIPTION="[null]"  domain="[null]" short_name=""
+           enabled="true" worst_value="[null]" optimized_best_value="[null]" best_value="[null]" direction="0" hidden="false"/>
+
+  <project_measures id="1" VALUE="1.0" METRIC_ID="1" SNAPSHOT_ID="1000" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]" person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+</dataset>