SONAR-2642 Use MyBatis instead of Hibernate for CPD

6 files changed, 210 insertions, 78 deletions

diff --git a/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/SonarEngine.java b/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/SonarEngine.java
index e8d75620072..30904911e38 100644
--- a/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/SonarEngine.java
+++ b/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/SonarEngine.java
@@ -31,7 +31,6 @@ import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.SensorContext;
-import org.sonar.api.database.DatabaseSession;
 import org.sonar.api.database.model.ResourceModel;
 import org.sonar.api.resources.*;
 import org.sonar.api.utils.Logs;
@@ -48,6 +47,7 @@ import org.sonar.duplications.java.JavaTokenProducer;
 import org.sonar.duplications.statement.Statement;
 import org.sonar.duplications.statement.StatementChunker;
 import org.sonar.duplications.token.TokenChunker;
+import org.sonar.persistence.dao.DuplicationDao;
 import org.sonar.plugins.cpd.index.DbDuplicationsIndex;
 import org.sonar.plugins.cpd.index.SonarDuplicationsIndex;
 
@@ -61,7 +61,7 @@ public class SonarEngine extends CpdEngine {
   private static final int TIMEOUT = 5 * 60;
 
   private final ResourcePersister resourcePersister;
-  private final DatabaseSession dbSession;
+  private final DuplicationDao dao;
 
   /**
    * For dry run, where is no access to database.
@@ -70,9 +70,9 @@ public class SonarEngine extends CpdEngine {
     this(null, null);
   }
 
-  public SonarEngine(ResourcePersister resourcePersister, DatabaseSession dbSession) {
+  public SonarEngine(ResourcePersister resourcePersister, DuplicationDao dao) {
     this.resourcePersister = resourcePersister;
-    this.dbSession = dbSession;
+    this.dao = dao;
   }
 
   @Override
@@ -85,7 +85,7 @@ public class SonarEngine extends CpdEngine {
    */
   private boolean isCrossProject(Project project) {
     return project.getConfiguration().getBoolean(CoreProperties.CPD_CROSS_RPOJECT, CoreProperties.CPD_CROSS_RPOJECT_DEFAULT_VALUE)
-        && resourcePersister != null && dbSession != null
+        && resourcePersister != null && dao != null
         && StringUtils.isBlank(project.getConfiguration().getString(CoreProperties.PROJECT_BRANCH_PROPERTY));
   }
 
@@ -108,7 +108,7 @@ public class SonarEngine extends CpdEngine {
     final SonarDuplicationsIndex index;
     if (isCrossProject(project)) {
       Logs.INFO.info("Cross-project analysis enabled");
-      index = new SonarDuplicationsIndex(new DbDuplicationsIndex(dbSession, resourcePersister, project));
+      index = new SonarDuplicationsIndex(new DbDuplicationsIndex(resourcePersister, project, dao));
     } else {
       Logs.INFO.info("Cross-project analysis disabled");
       index = new SonarDuplicationsIndex();
diff --git a/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/index/DbDuplicationsIndex.java b/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/index/DbDuplicationsIndex.java
index c63271e4807..831464be4e8 100644
--- a/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/index/DbDuplicationsIndex.java
+++ b/plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/index/DbDuplicationsIndex.java
@@ -24,18 +24,14 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
-import javax.persistence.Query;
-
-import org.hibernate.ejb.HibernateQuery;
-import org.hibernate.transform.Transformers;
-import org.sonar.api.database.DatabaseSession;
 import org.sonar.api.database.model.Snapshot;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
 import org.sonar.batch.index.ResourcePersister;
 import org.sonar.duplications.block.Block;
 import org.sonar.duplications.block.ByteArray;
-import org.sonar.jpa.entity.DuplicationBlock;
+import org.sonar.persistence.dao.DuplicationDao;
+import org.sonar.persistence.model.DuplicationUnit;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -44,13 +40,14 @@ public class DbDuplicationsIndex {
 
   private final Map<ByteArray, Collection<Block>> cache = Maps.newHashMap();
 
-  private final DatabaseSession session;
   private final ResourcePersister resourcePersister;
   private final int currentProjectSnapshotId;
   private final Integer lastSnapshotId;
 
-  public DbDuplicationsIndex(DatabaseSession session, ResourcePersister resourcePersister, Project currentProject) {
-    this.session = session;
+  private DuplicationDao dao;
+
+  public DbDuplicationsIndex(ResourcePersister resourcePersister, Project currentProject, DuplicationDao dao) {
+    this.dao = dao;
     this.resourcePersister = resourcePersister;
     Snapshot currentSnapshot = resourcePersister.getSnapshotOrFail(currentProject);
     Snapshot lastSnapshot = resourcePersister.getLastSnapshot(currentSnapshot, false);
@@ -61,8 +58,8 @@ public class DbDuplicationsIndex {
   /**
    * For tests.
    */
-  DbDuplicationsIndex(DatabaseSession session, ResourcePersister resourcePersister, Integer currentProjectSnapshotId, Integer prevSnapshotId) {
-    this.session = session;
+  DbDuplicationsIndex(DuplicationDao dao, ResourcePersister resourcePersister, Integer currentProjectSnapshotId, Integer prevSnapshotId) {
+    this.dao = dao;
     this.resourcePersister = resourcePersister;
     this.currentProjectSnapshotId = currentProjectSnapshotId;
     this.lastSnapshotId = prevSnapshotId;
@@ -74,37 +71,17 @@ public class DbDuplicationsIndex {
 
   public void prepareCache(Resource resource) {
     int resourceSnapshotId = getSnapshotIdFor(resource);
-
-    // Order of columns is important - see code below!
-    String sql = "SELECT DISTINCT to_blocks.hash, res.kee, to_blocks.index_in_file, to_blocks.start_line, to_blocks.end_line" +
-        " FROM duplications_index to_blocks, duplications_index from_blocks, snapshots snapshot, projects res" +
-        " WHERE from_blocks.snapshot_id = :resource_snapshot_id" +
-        " AND to_blocks.hash = from_blocks.hash" +
-        " AND to_blocks.snapshot_id = snapshot.id" +
-        " AND snapshot.islast = :is_last" +
-        " AND snapshot.project_id = res.id";
-    if (lastSnapshotId != null) {
-      // Filter for blocks from previous snapshot of current project
-      sql += " AND to_blocks.project_snapshot_id != :last_project_snapshot_id";
-    }
-    Query query = session.getEntityManager().createNativeQuery(sql)
-        .setParameter("resource_snapshot_id", resourceSnapshotId)
-        .setParameter("is_last", Boolean.TRUE);
-    if (lastSnapshotId != null) {
-      query.setParameter("last_project_snapshot_id", lastSnapshotId);
-    }
-    // Ugly hack for mapping results of custom SQL query into plain list (MyBatis is coming soon)
-    ((HibernateQuery) query).getHibernateQuery().setResultTransformer(Transformers.TO_LIST);
-    List<List<Object>> blocks = query.getResultList();
-
+    List<DuplicationUnit> units = dao.selectCandidates(resourceSnapshotId, lastSnapshotId);
     cache.clear();
-    for (List<Object> dbBlock : blocks) {
-      String hash = (String) dbBlock.get(0);
-      String resourceKey = (String) dbBlock.get(1);
-      int indexInFile = ((Number) dbBlock.get(2)).intValue();
-      int startLine = ((Number) dbBlock.get(3)).intValue();
-      int endLine = ((Number) dbBlock.get(4)).intValue();
-
+    // TODO Godin: maybe remove conversion of units to blocks?
+    for (DuplicationUnit unit : units) {
+      String hash = unit.getHash();
+      String resourceKey = unit.getResourceKey();
+      int indexInFile = unit.getIndexInFile();
+      int startLine = unit.getStartLine();
+      int endLine = unit.getEndLine();
+
+      // TODO Godin: in fact we could work directly with id instead of key - this will allow to decrease memory consumption
       Block block = new Block(resourceKey, new ByteArray(hash), indexInFile, startLine, endLine);
 
       // Group blocks by hash
@@ -128,17 +105,21 @@ public class DbDuplicationsIndex {
 
   public void insert(Resource resource, Collection<Block> blocks) {
     int resourceSnapshotId = getSnapshotIdFor(resource);
+
+    // TODO Godin: maybe remove conversion of blocks to units?
+    List<DuplicationUnit> units = Lists.newArrayList();
     for (Block block : blocks) {
-      DuplicationBlock dbBlock = new DuplicationBlock(
+      DuplicationUnit unit = new DuplicationUnit(
           currentProjectSnapshotId,
           resourceSnapshotId,
           block.getBlockHash().toString(),
           block.getIndexInFile(),
           block.getFirstLineNumber(),
           block.getLastLineNumber());
-      session.save(dbBlock);
+      units.add(unit);
     }
-    session.commit();
+
+    dao.insert(units);
   }
 
 }
diff --git a/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest.java b/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest.java
index fedf032033c..543adf4b821 100644
--- a/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest.java
+++ b/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest.java
@@ -21,28 +21,71 @@ package org.sonar.plugins.cpd.index;
 
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
 
+import java.io.InputStream;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 
+import org.apache.commons.io.IOUtils;
+import org.dbunit.Assertion;
+import org.dbunit.DataSourceDatabaseTester;
+import org.dbunit.DatabaseUnitException;
+import org.dbunit.IDatabaseTester;
+import org.dbunit.database.IDatabaseConnection;
+import org.dbunit.dataset.*;
+import org.dbunit.dataset.filter.DefaultColumnFilter;
+import org.dbunit.dataset.xml.FlatXmlDataSet;
+import org.dbunit.operation.DatabaseOperation;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.resources.JavaFile;
 import org.sonar.api.resources.Resource;
 import org.sonar.duplications.block.Block;
 import org.sonar.duplications.block.ByteArray;
-import org.sonar.jpa.test.AbstractDbUnitTestCase;
+import org.sonar.persistence.InMemoryDatabase;
+import org.sonar.persistence.MyBatis;
+import org.sonar.persistence.dao.DuplicationDao;
 
-public class DbDuplicationsIndexTest extends AbstractDbUnitTestCase {
+/**
+ * TODO Godin: would be better to split this test on two - one for DAO and one for DbDuplicationsIndex
+ */
+public class DbDuplicationsIndexTest {
+
+  private static IDatabaseTester databaseTester;
+  private static InMemoryDatabase database;
+  private static DuplicationDao dao;
 
   private DbDuplicationsIndex index;
 
+  @Before
+  public void startDatabase() throws Exception {
+    database = new InMemoryDatabase();
+    MyBatis myBatis = new MyBatis(database);
+
+    database.start();
+    myBatis.start();
+
+    dao = new DuplicationDao(myBatis);
+    databaseTester = new DataSourceDatabaseTester(database.getDataSource());
+  }
+
+  @After
+  public void stopDatabase() throws Exception {
+    if (databaseTester != null) {
+      databaseTester.onTearDown();
+    }
+    database.stop();
+  }
+
   @Test
-  public void shouldGetByHash() {
+  public void shouldGetByHash() throws Exception {
     Resource resource = new JavaFile("foo");
-    index = spy(new DbDuplicationsIndex(getSession(), null, 9, 7));
+    index = spy(new DbDuplicationsIndex(dao, null, 9, 7));
     doReturn(10).when(index).getSnapshotIdFor(resource);
     setupData("shouldGetByHash");
 
@@ -58,12 +101,18 @@ public class DbDuplicationsIndexTest extends AbstractDbUnitTestCase {
     assertThat("block index in file", block.getIndexInFile(), is(0));
     assertThat("block start line", block.getFirstLineNumber(), is(1));
     assertThat("block end line", block.getLastLineNumber(), is(2));
+
+    // check null for lastSnapshotId
+    index = spy(new DbDuplicationsIndex(dao, null, 9, null));
+    doReturn(10).when(index).getSnapshotIdFor(resource);
+
+    index.prepareCache(resource);
   }
 
   @Test
-  public void shouldInsert() {
+  public void shouldInsert() throws Exception {
     Resource resource = new JavaFile("foo");
-    index = spy(new DbDuplicationsIndex(getSession(), null, 1, null));
+    index = spy(new DbDuplicationsIndex(dao, null, 1, null));
     doReturn(2).when(index).getSnapshotIdFor(resource);
     setupData("shouldInsert");
 
@@ -72,4 +121,106 @@ public class DbDuplicationsIndexTest extends AbstractDbUnitTestCase {
     checkTables("shouldInsert", "duplications_index");
   }
 
+  // ============================================================
+  // TODO Godin: a kind of copy-paste from AbstractDbUnitTestCase
+
+  private final void setupData(String... testNames) {
+    InputStream[] streams = new InputStream[testNames.length];
+    try {
+      for (int i = 0; i < testNames.length; i++) {
+        String className = getClass().getName();
+        className = String.format("/%s/%s.xml", className.replace(".", "/"), testNames[i]);
+        streams[i] = getClass().getResourceAsStream(className);
+        if (streams[i] == null) {
+          throw new RuntimeException("Test not found :" + className);
+        }
+      }
+
+      setupData(streams);
+
+    } finally {
+      for (InputStream stream : streams) {
+        IOUtils.closeQuietly(stream);
+      }
+    }
+  }
+
+  private final void setupData(InputStream... dataSetStream) {
+    try {
+      IDataSet[] dataSets = new IDataSet[dataSetStream.length];
+      for (int i = 0; i < dataSetStream.length; i++) {
+        ReplacementDataSet dataSet = new ReplacementDataSet(new FlatXmlDataSet(dataSetStream[i]));
+        dataSet.addReplacementObject("[null]", null);
+        dataSets[i] = dataSet;
+      }
+      CompositeDataSet compositeDataSet = new CompositeDataSet(dataSets);
+
+      databaseTester.setDataSet(compositeDataSet);
+      IDatabaseConnection connection = databaseTester.getConnection();
+
+      DatabaseOperation.CLEAN_INSERT.execute(connection, databaseTester.getDataSet());
+
+      connection.getConnection().commit();
+      connection.close();
+    } catch (Exception e) {
+      throw translateException("Could not setup DBUnit data", e);
+    }
+  }
+
+  private final void checkTables(String testName, String... tables) {
+    checkTables(testName, new String[] {}, tables);
+  }
+
+  private final void checkTables(String testName, String[] excludedColumnNames, String... tables) {
+    // getSession().commit();
+    try {
+      IDataSet dataSet = getCurrentDataSet();
+      IDataSet expectedDataSet = getExpectedData(testName);
+      for (String table : tables) {
+        ITable filteredTable = DefaultColumnFilter.excludedColumnsTable(dataSet.getTable(table), excludedColumnNames);
+        Assertion.assertEquals(expectedDataSet.getTable(table), filteredTable);
+      }
+    } catch (DataSetException e) {
+      throw translateException("Error while checking results", e);
+    } catch (DatabaseUnitException e) {
+      fail(e.getMessage());
+    }
+  }
+
+  private final IDataSet getExpectedData(String testName) {
+    String className = getClass().getName();
+    className = String.format("/%s/%s-result.xml", className.replace(".", "/"), testName);
+
+    InputStream in = getClass().getResourceAsStream(className);
+    try {
+      return getData(in);
+    } finally {
+      IOUtils.closeQuietly(in);
+    }
+  }
+
+  private final IDataSet getData(InputStream stream) {
+    try {
+      ReplacementDataSet dataSet = new ReplacementDataSet(new FlatXmlDataSet(stream));
+      dataSet.addReplacementObject("[null]", null);
+      return dataSet;
+    } catch (Exception e) {
+      throw translateException("Could not read the dataset stream", e);
+    }
+  }
+
+  private final IDataSet getCurrentDataSet() {
+    try {
+      IDatabaseConnection connection = databaseTester.getConnection();
+      return connection.createDataSet();
+    } catch (Exception e) {
+      throw translateException("Could not create the current dataset", e);
+    }
+  }
+
+  private static RuntimeException translateException(String msg, Exception cause) {
+    RuntimeException runtimeException = new RuntimeException(String.format("%s: [%s] %s", msg, cause.getClass().getName(), cause.getMessage()));
+    runtimeException.setStackTrace(cause.getStackTrace());
+    return runtimeException;
+  }
 }
diff --git a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldGetByHash.xml b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldGetByHash.xml
index 9581b0be96c..8edbe99067b 100644
--- a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldGetByHash.xml
+++ b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldGetByHash.xml
@@ -1,24 +1,24 @@
 <dataset>
 
-  <snapshots id="1" status="P" islast="false" project_id="0" />
-  <snapshots id="2" status="P" islast="false" project_id="1" />
-  <projects id="1" kee="bar-old" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="1" status="P" islast="0" project_id="0" />
+  <snapshots id="2" status="P" islast="0" project_id="1" />
+  <projects id="1" kee="bar-old" enabled="1" scope="FIL" qualifier="CLA" />
 
-  <snapshots id="3" status="P" islast="true" />
-  <snapshots id="4" status="P" islast="true" project_id="2" />
-  <projects id="2" kee="bar-last" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="3" status="P" islast="1" project_id="2" />
+  <snapshots id="4" status="P" islast="1" project_id="2" />
+  <projects id="2" kee="bar-last" enabled="1" scope="FIL" qualifier="CLA" />
 
-  <snapshots id="5" status="P" islast="false" />
-  <snapshots id="6" status="P" islast="false" project_id="3" />
-  <projects id="3" kee="foo-old" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="5" status="P" islast="0" project_id="3" />
+  <snapshots id="6" status="P" islast="0" project_id="3" />
+  <projects id="3" kee="foo-old" enabled="1" scope="FIL" qualifier="CLA" />
 
-  <snapshots id="7" status="P" islast="true" />
-  <snapshots id="8" status="P" islast="true" project_id="4" />
-  <projects id="4" kee="foo-last" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="7" status="P" islast="1" project_id="4" />
+  <snapshots id="8" status="P" islast="1" project_id="4" />
+  <projects id="4" kee="foo-last" enabled="1" scope="FIL" qualifier="CLA" />
 
-  <snapshots id="9" status="U" islast="false" />
-  <snapshots id="10" status="U" islast="false" project_id="5" />
-  <projects id="5" kee="foo" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="9" status="U" islast="0" project_id="5" />
+  <snapshots id="10" status="U" islast="0" project_id="5" />
+  <projects id="5" kee="foo" enabled="1" scope="FIL" qualifier="CLA" />
 
   <!-- Old snapshot of another project -->
   <!-- bar-old -->
@@ -34,7 +34,7 @@
 
   <!-- Last snapshot of current project -->
   <!-- foo-last -->
-  <duplications_index id="4" project_snapshot_id="7" snapshot_id="8" hash="bb" index_in_file="0" start_line="0" end_line="0" />
+  <duplications_index id="4" project_snapshot_id="7" snapshot_id="8" hash="aa" index_in_file="0" start_line="0" end_line="0" />
 
   <!-- New snapshot of current project -->
   <!-- foo -->
diff --git a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert-result.xml b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert-result.xml
index 5848ecb5723..619646c1821 100644
--- a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert-result.xml
+++ b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert-result.xml
@@ -1,8 +1,8 @@
 <dataset>
 
-  <snapshots id="1" status="U" islast="false" project_id="0" />
-  <snapshots id="2" status="U" islast="false" project_id="1" />
-  <projects id="1" kee="foo" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="1" status="U" islast="0" project_id="0" />
+  <snapshots id="2" status="U" islast="0" project_id="1" />
+  <projects id="1" kee="foo" enabled="1" scope="FIL" qualifier="CLA" />
 
   <duplications_index id="1" project_snapshot_id="1" snapshot_id="2" hash="bb" index_in_file="0" start_line="1" end_line="2" />
 
diff --git a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert.xml b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert.xml
index 940281a0599..e0efcf156c4 100644
--- a/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert.xml
+++ b/plugins/sonar-cpd-plugin/src/test/resources/org/sonar/plugins/cpd/index/DbDuplicationsIndexTest/shouldInsert.xml
@@ -1,7 +1,7 @@
 <dataset>
 
-  <snapshots id="1" status="U" islast="false" project_id="0" />
-  <snapshots id="2" status="U" islast="false" project_id="1" />
-  <projects id="1" kee="foo" enabled="true" scope="FIL" qualifier="CLA" />
+  <snapshots id="1" status="U" islast="0" project_id="0" />
+  <snapshots id="2" status="U" islast="0" project_id="1" />
+  <projects id="1" kee="foo" enabled="1" scope="FIL" qualifier="CLA" />
 
 </dataset>