Merge pull request #101 from SonarSource/fix/file_sources_migration

SONAR-6174 Improve exception handling during migration

7 files changed, 195 insertions, 19 deletions

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 4c5b35143df..869c74e33c9 100644
--- 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
@@ -22,6 +22,7 @@ package org.sonar.server.db.migrations;
 import javax.annotation.CheckForNull;
 
 import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.sql.Timestamp;
 import java.util.Date;
@@ -105,6 +106,25 @@ public interface Select extends SqlStatement<Select> {
     public byte[] getBytes(int columnIndex) throws SQLException {
       return rs.getBytes(columnIndex);
     }
+
+    @Override
+    public String toString() {
+      try {
+        ResultSetMetaData rsMetaData = rs.getMetaData();
+        StringBuilder sb = new StringBuilder();
+        for (int i = 1; i <= rsMetaData.getColumnCount(); i++) {
+          if (i > 1) {
+            sb.append(",");
+          }
+          sb.append(rsMetaData.getColumnLabel(i).toLowerCase());
+          sb.append("=");
+          sb.append(rs.getObject(i));
+        }
+        return sb.toString();
+      } catch (Exception e) {
+        return "Unavailable: " + e.getMessage();
+      }
+    }
   }
 
   static interface RowReader<T> {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/SelectImpl.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/SelectImpl.java
index 81a8ac9103e..2c8e30821b8 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/SelectImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/SelectImpl.java
@@ -45,6 +45,8 @@ class SelectImpl extends BaseSqlStatement<Select> implements Select {
         rows.add(reader.read(row));
       }
       return rows;
+    } catch (Exception e) {
+      throw newExceptionWithRowDetails(row, e);
     } finally {
       DbUtils.closeQuietly(rs);
       close();
@@ -60,6 +62,8 @@ class SelectImpl extends BaseSqlStatement<Select> implements Select {
         return reader.read(row);
       }
       return null;
+    } catch (Exception e) {
+      throw newExceptionWithRowDetails(row, e);
     } finally {
       DbUtils.closeQuietly(rs);
       close();
@@ -74,12 +78,18 @@ class SelectImpl extends BaseSqlStatement<Select> implements Select {
       while (rs.next()) {
         handler.handle(row);
       }
+    } catch (Exception e) {
+      throw newExceptionWithRowDetails(row, e);
     } finally {
       DbUtils.closeQuietly(rs);
       close();
     }
   }
 
+  private IllegalStateException newExceptionWithRowDetails(Select.Row row, Exception e) {
+    return new IllegalStateException("Error during processing of row: [" + row + "]", e);
+  }
+
   static SelectImpl create(Database db, Connection connection, String sql) throws SQLException {
     // TODO use DbClient#newScrollingSelectStatement()
     PreparedStatement pstmt = connection.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/FileSourceDto.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/FileSourceDto.java
index f1bbcb00af0..3a4e6a7d53e 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/FileSourceDto.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/FileSourceDto.java
@@ -29,7 +29,6 @@ import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.utils.KeyValueFormat;
-import org.sonar.api.utils.SonarException;
 import org.sonar.api.utils.text.CsvWriter;
 
 import javax.xml.stream.XMLInputFactory;
@@ -161,8 +160,8 @@ class FileSourceDto {
    * Parses data of {@link CoreMetrics#DUPLICATIONS_DATA}.
    */
   private static List<List<Block>> parseDuplicationData(String data) {
+    ImmutableList.Builder<List<Block>> groups = ImmutableList.builder();
     try {
-      ImmutableList.Builder<List<Block>> groups = ImmutableList.builder();
       StringReader reader = new StringReader(data);
       SMInputFactory inputFactory = initStax();
       SMHierarchicCursor rootC = inputFactory.rootElementCursor(reader);
@@ -183,10 +182,11 @@ class FileSourceDto {
         }
         groups.add(group.build());
       }
-      return groups.build();
-    } catch (XMLStreamException e) {
-      throw new SonarException(e.getMessage(), e);
+    } catch (Exception e) {
+      // SONAR-6174 Ignore any issue while parsing duplication measure. There is nothing user can do and things will get solved after
+      // next analysis anyway
     }
+    return groups.build();
   }
 
   private static int getAttrIntValue(SMInputCursor cursor, String attrName) throws XMLStreamException {
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 cb156004ef6..1c975a4f7e5 100644
--- 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
@@ -21,11 +21,15 @@ package org.sonar.server.db.migrations;
 
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.rules.ExpectedException;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 import org.sonar.core.persistence.BatchSession;
 import org.sonar.core.persistence.DbTester;
+import org.sonar.server.db.migrations.Select.Row;
+import org.sonar.server.db.migrations.Select.RowReader;
 import org.sonar.test.DbTests;
 
 import java.sql.SQLException;
@@ -42,6 +46,9 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   @ClassRule
   public static DbTester db = new DbTester().schema(BaseDataChangeTest.class, "schema.sql");
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @Before
   public void setUp() throws Exception {
     db.executeUpdateSql("truncate table persons");
@@ -104,6 +111,48 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   }
 
   @Test
+  public void display_current_row_details_if_error_during_get() throws Exception {
+    db.prepareDbUnit(getClass(), "persons.xml");
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Error during processing of row: [id=2]");
+
+    new BaseDataChange(db.database()) {
+      @Override
+      public void execute(Context context) throws SQLException {
+        context.prepareSelect("select id from persons where id>=?").setLong(1, 2L).get(new RowReader<Long>() {
+          @Override
+          public Long read(Row row) throws SQLException {
+            throw new IllegalStateException("Unexpected error");
+          }
+        });
+      }
+    }.execute();
+
+  }
+
+  @Test
+  public void display_current_row_details_if_error_during_list() throws Exception {
+    db.prepareDbUnit(getClass(), "persons.xml");
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Error during processing of row: [id=2]");
+
+    new BaseDataChange(db.database()) {
+      @Override
+      public void execute(Context context) throws SQLException {
+        context.prepareSelect("select id from persons where id>=?").setLong(1, 2L).list(new RowReader<Long>() {
+          @Override
+          public Long read(Row row) throws SQLException {
+            throw new IllegalStateException("Unexpected error");
+          }
+        });
+      }
+    }.execute();
+
+  }
+
+  @Test
   public void bad_parameterized_query() throws Exception {
     db.prepareDbUnit(getClass(), "persons.xml");
 
@@ -115,12 +164,10 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
         ids.addAll(context.prepareSelect("select id from persons where id>=?").list(Select.LONG_READER));
       }
     };
-    try {
-      change.execute();
-      fail();
-    } catch (SQLException e) {
-      // ok
-    }
+
+    thrown.expect(SQLException.class);
+
+    change.execute();
   }
 
   @Test
@@ -265,6 +312,28 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   }
 
   @Test
+  public void display_current_row_details_if_error_during_scroll() throws Exception {
+    db.prepareDbUnit(getClass(), "persons.xml");
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Error during processing of row: [id=1]");
+
+    new BaseDataChange(db.database()) {
+      @Override
+      public void execute(Context context) throws SQLException {
+        final Upsert upsert = context.prepareUpsert("update persons set login=?, age=? where id=?");
+        context.prepareSelect("select id from persons").scroll(new Select.RowHandler() {
+          @Override
+          public void handle(Select.Row row) throws SQLException {
+            throw new IllegalStateException("Unexpected error");
+          }
+        });
+        upsert.commit().close();
+      }
+    }.execute();
+  }
+
+  @Test
   public void mass_update() throws Exception {
     db.prepareDbUnit(getClass(), "persons.xml");
 
@@ -292,6 +361,29 @@ public class BaseDataChangeTest extends AbstractDaoTestCase {
   }
 
   @Test
+  public void display_current_row_details_if_error_during_mass_update() throws Exception {
+    db.prepareDbUnit(getClass(), "persons.xml");
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Error during processing of row: [id=2]");
+
+    new BaseDataChange(db.database()) {
+      @Override
+      public void execute(Context context) throws SQLException {
+        MassUpdate massUpdate = context.prepareMassUpdate();
+        massUpdate.select("select id from persons where id>=?").setLong(1, 2L);
+        massUpdate.update("update persons set login=?, age=? where id=?");
+        massUpdate.execute(new MassUpdate.Handler() {
+          @Override
+          public boolean handle(Select.Row row, SqlStatement update) throws SQLException {
+            throw new IllegalStateException("Unexpected error");
+          }
+        });
+      }
+    }.execute();
+  }
+
+  @Test
   public void mass_update_nothing() throws Exception {
     db.prepareDbUnit(getClass(), "persons.xml");
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/FeedFileSourcesTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/FeedFileSourcesTest.java
index 90f12826a24..9607166b22d 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/FeedFileSourcesTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/FeedFileSourcesTest.java
@@ -214,4 +214,42 @@ public class FeedFileSourcesTest {
 
     db.assertDbUnit(getClass(), "after-with-scm.xml", "file_sources");
   }
+
+  @Test
+  public void migrate_sources_with_invalid_duplication() throws Exception {
+    db.prepareDbUnit(getClass(), "before.xml");
+
+    Connection connection = null;
+    try {
+      connection = db.openConnection();
+
+      connection.prepareStatement("insert into snapshot_sources " +
+        "(snapshot_id, data, updated_at) " +
+        "values " +
+        "(6, 'class Foo {\r\n  // Empty\r\n}\r\n', '2014-10-31 16:44:02.000')")
+        .executeUpdate();
+
+      db.executeUpdateSql("insert into snapshot_sources " +
+        "(snapshot_id, data, updated_at) " +
+        "values " +
+        "(7, '', '2014-10-31 16:44:02.000')");
+
+      PreparedStatement duplicationDataStmt = connection.prepareStatement("insert into project_measures " +
+        "(metric_id, snapshot_id, text_value) " +
+        "values " +
+        "(13, 6, ?)");
+      duplicationDataStmt
+        .setBytes(
+          1,
+          "<duplications><g><b s=\"1\" l=\"1\" r=\"MyProject:src/main/xoo/prj/MyFile.xoo\"/><b s=\"2\" l=\"1\" r=\"MyProject:src/main/xoo/prj/MyFile.xoo\"/><b s=\"3\" l=\"1\" r=\"MyProject:src/main/xoo/prj/AnotherFile.xoo\"/"
+            .getBytes(Charsets.UTF_8));
+      duplicationDataStmt.executeUpdate();
+    } finally {
+      DbUtils.commitAndCloseQuietly(connection);
+    }
+
+    migration.execute();
+
+    db.assertDbUnit(getClass(), "after-with-invalid-duplication.xml", "file_sources");
+  }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v51/FeedFileSourcesBinaryDataTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v51/FeedFileSourcesBinaryDataTest.java
index 80fc3c07db9..59736a306a2 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v51/FeedFileSourcesBinaryDataTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v51/FeedFileSourcesBinaryDataTest.java
@@ -21,7 +21,9 @@ package org.sonar.server.db.migrations.v51;
 
 import org.apache.commons.dbutils.DbUtils;
 import org.junit.ClassRule;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.core.persistence.DbTester;
 import org.sonar.core.source.db.FileSourceDto;
 import org.sonar.server.db.migrations.DatabaseMigration;
@@ -34,10 +36,12 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.fail;
 
 public class FeedFileSourcesBinaryDataTest {
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @ClassRule
   public static DbTester db = new DbTester().schema(FeedFileSourcesBinaryDataTest.class, "schema.sql");
 
@@ -51,7 +55,7 @@ public class FeedFileSourcesBinaryDataTest {
     int count = db.countSql("select count(*) from file_sources where binary_data is not null");
     assertThat(count).isEqualTo(3);
 
-    try(Connection connection = db.openConnection()) {
+    try (Connection connection = db.openConnection()) {
       FileSourceDb.Data data = selectData(connection, 1L);
       assertThat(data.getLinesCount()).isEqualTo(4);
       assertThat(data.getLines(0).getScmRevision()).isEqualTo("aef12a");
@@ -70,12 +74,11 @@ public class FeedFileSourcesBinaryDataTest {
     db.prepareDbUnit(getClass(), "bad_data.xml");
 
     DatabaseMigration migration = new FeedFileSourcesBinaryData(db.database());
-    try {
-      migration.execute();
-      fail();
-    } catch (IllegalStateException e) {
-      assertThat(e).hasMessageContaining("Invalid FILE_SOURCES.DATA on row with ID 1:");
-    }
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Error during processing of row: [id=1]");
+
+    migration.execute();
   }
 
   private FileSourceDb.Data selectData(Connection connection, long fileSourceId) throws SQLException {
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/FeedFileSourcesTest/after-with-invalid-duplication.xml b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/FeedFileSourcesTest/after-with-invalid-duplication.xml
new file mode 100644
index 00000000000..7762dcd311d
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/FeedFileSourcesTest/after-with-invalid-duplication.xml
@@ -0,0 +1,13 @@
+<dataset>
+
+  <file_sources id="1" project_uuid="uuid-MyProject" file_uuid="uuid-Migrated.xoo" created_at="1416238020000" updated_at="1414770242000"
+    data=""
+    line_hashes=""
+    data_hash="" />
+
+  <file_sources id="2" project_uuid="uuid-MyProject" file_uuid="uuid-MyFile.xoo" created_at="1416238020000" updated_at="1414770242000"
+    data=",,,,,,,,,,,,,,,class Foo {&#13;&#10;,,,,,,,,,,,,,,,  // Empty&#13;&#10;,,,,,,,,,,,,,,,}&#13;&#10;,,,,,,,,,,,,,,,&#13;&#10;"
+    line_hashes="6a19ce786467960a3a9b0d26383a464a&#10;aab2dbc5fdeaa80b050b1d049ede357c&#10;cbb184dd8e05c9709e5dcaedaa0495cf&#10;&#10;"
+    data_hash="" />
+
+</dataset>