DBCleaner : fix deletion of events + improve mybatis batch mode

11 files changed, 169 insertions, 71 deletions

diff --git a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DefaultPeriodCleaner.java b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DefaultPeriodCleaner.java
index ea50bac1d65..c313ecc4f8b 100644
--- a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DefaultPeriodCleaner.java
+++ b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DefaultPeriodCleaner.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.plugins.dbcleaner.period;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.config.Settings;
@@ -59,7 +58,7 @@ public final class DefaultPeriodCleaner implements PeriodCleaner {
 
   private void delete(List<PurgeableSnapshotDto> snapshots) {
     for (PurgeableSnapshotDto snapshot : snapshots) {
-      LOG.debug("<- Delete snapshot: " + DateUtils.formatDateTime(snapshot.getDate()) + " [" + snapshot.getSnapshotId() + "]");
+      LOG.info("<- Delete snapshot: " + DateUtils.formatDateTime(snapshot.getDate()) + " [" + snapshot.getSnapshotId() + "]");
       purgeDao.deleteSnapshots(PurgeSnapshotQuery.create().setRootSnapshotId(snapshot.getSnapshotId()));
     }
   }
diff --git a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DeleteAllFilter.java b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DeleteAllFilter.java
index 51fdd03ff05..78273c440c8 100644
--- a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DeleteAllFilter.java
+++ b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/DeleteAllFilter.java
@@ -47,6 +47,6 @@ class DeleteAllFilter extends Filter {
 
   @Override
   void log() {
-    LoggerFactory.getLogger(getClass()).debug("-> Delete data prior to: " + DateUtils.formatDate(before));
+    LoggerFactory.getLogger(getClass()).info("-> Delete data prior to: " + DateUtils.formatDate(before));
   }
 }
diff --git a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/KeepOneFilter.java b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/KeepOneFilter.java
index fa8c5b3255f..7775c9bece9 100644
--- a/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/KeepOneFilter.java
+++ b/plugins/sonar-dbcleaner-plugin/src/main/java/org/sonar/plugins/dbcleaner/period/KeepOneFilter.java
@@ -55,7 +55,7 @@ class KeepOneFilter extends Filter {
 
   @Override
   void log() {
-    LoggerFactory.getLogger(getClass()).debug("-> Keep one snapshot per " + label + " between " + DateUtils.formatDate(start) + " and " + DateUtils.formatDate(end));
+    LoggerFactory.getLogger(getClass()).info("-> Keep one snapshot per " + label + " between " + DateUtils.formatDate(start) + " and " + DateUtils.formatDate(end));
   }
 
   private void appendSnapshotsToDelete(Interval interval, List<PurgeableSnapshotDto> toDelete) {
diff --git a/sonar-core/src/main/java/org/sonar/core/duplication/DuplicationDao.java b/sonar-core/src/main/java/org/sonar/core/duplication/DuplicationDao.java
index fb6503ccb0a..d54c8761c6e 100644
--- a/sonar-core/src/main/java/org/sonar/core/duplication/DuplicationDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/duplication/DuplicationDao.java
@@ -51,7 +51,7 @@ public class DuplicationDao implements BatchComponent, ServerComponent {
    * Note that generated ids are not returned.
    */
   public void insert(Collection<DuplicationUnitDto> units) {
-    SqlSession session = mybatis.openSession(ExecutorType.BATCH);
+    SqlSession session = mybatis.openBatchSession();
     try {
       DuplicationMapper mapper = session.getMapper(DuplicationMapper.class);
       for (DuplicationUnitDto unit : units) {
diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java b/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java
index 7a333228a08..2a416fd11b3 100644
--- a/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java
+++ b/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java
@@ -19,10 +19,17 @@
  */
 package org.sonar.core.persistence;
 
+import org.apache.ibatis.executor.BatchResult;
+import org.apache.ibatis.session.Configuration;
 import org.apache.ibatis.session.ResultHandler;
+import org.apache.ibatis.session.RowBounds;
 import org.apache.ibatis.session.SqlSession;
 
-public final class BatchSession {
+import java.sql.Connection;
+import java.util.List;
+import java.util.Map;
+
+public final class BatchSession implements SqlSession {
 
   public static final int MAX_BATCH_SIZE = 1000;
 
@@ -44,32 +51,145 @@ public final class BatchSession {
    */
   public BatchSession increment(int nbSqlRequests) {
     count += nbSqlRequests;
-    if (count > batchSize) {
+    if (count >= batchSize) {
       commit();
     }
     return this;
   }
 
-  public BatchSession commit() {
+  private void reset() {
+    count=0;
+  }
+
+  public void select(String statement, Object parameter, ResultHandler handler) {
+    reset();
+    session.select(statement, parameter, handler);
+  }
+
+  public void select(String statement, ResultHandler handler) {
+    reset();
+    session.select(statement, handler);
+  }
+
+  public Object selectOne(String statement) {
+    reset();
+    return session.selectOne(statement);
+  }
+
+  public Object selectOne(String statement, Object parameter) {
+    reset();
+    return session.selectOne(statement, parameter);
+  }
+
+  public List selectList(String statement) {
+    reset();
+    return session.selectList(statement);
+  }
+
+  public List selectList(String statement, Object parameter) {
+    reset();
+    return session.selectList(statement, parameter);
+  }
+
+  public List selectList(String statement, Object parameter, RowBounds rowBounds) {
+    reset();
+    return session.selectList(statement, parameter, rowBounds);
+  }
+
+  public Map selectMap(String statement, String mapKey) {
+    reset();
+    return session.selectMap(statement, mapKey);
+  }
+
+  public Map selectMap(String statement, Object parameter, String mapKey) {
+    reset();
+    return session.selectMap(statement, parameter, mapKey);
+  }
+
+  public Map selectMap(String statement, Object parameter, String mapKey, RowBounds rowBounds) {
+    reset();
+    return session.selectMap(statement, parameter, mapKey, rowBounds);
+  }
+
+  public void select(String statement, Object parameter, RowBounds rowBounds, ResultHandler handler) {
+    reset();
+    session.select(statement, parameter, rowBounds, handler);
+  }
+
+  public int insert(String statement) {
+    increment(1);
+    return session.insert(statement);
+  }
+
+  public int insert(String statement, Object parameter) {
+    increment(1);
+    return session.insert(statement, parameter);
+  }
+
+  public int update(String statement) {
+    increment(1);
+    return session.update(statement);
+  }
+
+  public int update(String statement, Object parameter) {
+    increment(1);
+    return session.update(statement, parameter);
+  }
+
+  public int delete(String statement) {
+    increment(1);
+    return session.delete(statement);
+  }
+
+  public int delete(String statement, Object parameter) {
+    increment(1);
+    return session.delete(statement, parameter);
+  }
+
+  public void commit() {
     session.commit();
-    count = 0;
-    return this;
+    reset();
   }
 
-  public <T> T getMapper(Class<T> type) {
-    return session.getMapper(type);
+  public void commit(boolean force) {
+    session.commit(force);
+    reset();
   }
 
-  public SqlSession getSqlSession() {
-    return session;
+  public void rollback() {
+    session.rollback();
+    reset();
   }
 
-  public void select(String statement, Object parameter, ResultHandler handler) {
-    session.select(statement, parameter, handler);
+  public void rollback(boolean force) {
+    session.rollback(force);
+    reset();
   }
 
-  public void select(String statement, ResultHandler handler) {
-    session.select(statement, handler);
+  public List<BatchResult> flushStatements() {
+    List<BatchResult> batchResults = session.flushStatements();
+    reset();
+    return batchResults;
+  }
+
+  public void close() {
+    session.close();
+  }
+
+  public void clearCache() {
+    session.clearCache();
+  }
+
+  public Configuration getConfiguration() {
+    return session.getConfiguration();
+  }
+
+  public <T> T getMapper(Class<T> type) {
+    return session.getMapper(type);
+  }
+
+  public Connection getConnection() {
+    return session.getConnection();
   }
 
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java
index f74d34fed79..b42b27d54d5 100644
--- a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java
+++ b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java
@@ -97,15 +97,11 @@ public class MyBatis implements BatchComponent, ServerComponent {
   }
 
   public SqlSession openSession() {
-    return sessionFactory.openSession();
-  }
-
-  public SqlSession openSession(ExecutorType type) {
-    return sessionFactory.openSession(type);
+    return sessionFactory.openSession(ExecutorType.REUSE);
   }
 
   public BatchSession openBatchSession() {
-    SqlSession session = openSession(ExecutorType.BATCH);
+    SqlSession session = sessionFactory.openSession(ExecutorType.BATCH);
     return new BatchSession(session);
   }
 
@@ -120,12 +116,6 @@ public class MyBatis implements BatchComponent, ServerComponent {
     }
   }
 
-  public static void closeQuietly(BatchSession session) {
-    if (session != null) {
-      closeQuietly(session.getSqlSession());
-    }
-  }
-
   private void loadMapper(Configuration conf, Class mapperClass) throws IOException {
     // trick to use database-specific XML files for a single Mapper Java interface
     InputStream input = getPathToMapper(mapperClass);
diff --git a/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java b/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java
index cd38614b5e7..726f59dcb52 100644
--- a/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java
@@ -21,8 +21,6 @@ package org.sonar.core.purge;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
-import org.apache.ibatis.session.ExecutorType;
 import org.apache.ibatis.session.ResultContext;
 import org.apache.ibatis.session.ResultHandler;
 import org.apache.ibatis.session.SqlSession;
@@ -32,7 +30,6 @@ import org.sonar.core.resource.ResourceDao;
 
 import java.util.Collections;
 import java.util.List;
-import java.util.SortedSet;
 
 public class PurgeDao {
   private final MyBatis mybatis;
@@ -44,7 +41,7 @@ public class PurgeDao {
   }
 
   public PurgeDao purgeProject(long rootProjectId) {
-    SqlSession session = mybatis.openSession(ExecutorType.BATCH);
+    SqlSession session = mybatis.openBatchSession();
     PurgeMapper purgeMapper = session.getMapper(PurgeMapper.class);
     try {
       List<Long> projectIds = resourceDao.getDescendantProjectIdsAndSelf(rootProjectId, session);
@@ -89,7 +86,7 @@ public class PurgeDao {
   }
 
   public List<PurgeableSnapshotDto> selectPurgeableSnapshots(long resourceId) {
-    SqlSession session = mybatis.openSession(ExecutorType.REUSE);
+    SqlSession session = mybatis.openBatchSession();
     try {
       PurgeMapper mapper = session.getMapper(PurgeMapper.class);
       List<PurgeableSnapshotDto> result = Lists.newArrayList();
@@ -106,7 +103,7 @@ public class PurgeDao {
     final BatchSession session = mybatis.openBatchSession();
     try {
       final PurgeMapper mapper = session.getMapper(PurgeMapper.class);
-      List<Long> projectIds = resourceDao.getDescendantProjectIdsAndSelf(rootProjectId, session.getSqlSession());
+      List<Long> projectIds = resourceDao.getDescendantProjectIdsAndSelf(rootProjectId, session);
       for (Long projectId : projectIds) {
         session.select("org.sonar.core.purge.PurgeMapper.selectResourceIdsByRootId", projectId, new ResultHandler() {
           public void handleResult(ResultContext context) {
@@ -127,7 +124,7 @@ public class PurgeDao {
     session.select("org.sonar.core.purge.PurgeMapper.selectSnapshotIdsByResource", new ResultHandler() {
       public void handleResult(ResultContext context) {
         Long snapshotId = (Long) context.getResultObject();
-        session.increment(deleteSnapshot(snapshotId, mapper));
+        deleteSnapshot(snapshotId, mapper);
       }
     });
     // TODO optimization: filter requests according to resource scope
@@ -140,16 +137,14 @@ public class PurgeDao {
     mapper.deleteResourceReviews(resourceId);
     mapper.deleteResourceEvents(resourceId);
     mapper.deleteResource(resourceId);
-    session.increment(9);
   }
 
   @VisibleForTesting
-  int disableResource(long resourceId, PurgeMapper mapper) {
+  void disableResource(long resourceId, PurgeMapper mapper) {
     mapper.deleteResourceIndex(resourceId);
     mapper.setSnapshotIsLastToFalse(resourceId);
     mapper.disableResource(resourceId);
     mapper.closeResourceReviews(resourceId);
-    return 4; // nb of SQL requests
   }
 
 
@@ -160,7 +155,7 @@ public class PurgeDao {
       session.select("org.sonar.core.purge.PurgeMapper.selectSnapshotIds", query, new ResultHandler() {
         public void handleResult(ResultContext context) {
           Long snapshotId = (Long) context.getResultObject();
-          session.increment(deleteSnapshot(snapshotId, mapper));
+          deleteSnapshot(snapshotId, mapper);
         }
       });
       session.commit();
@@ -172,19 +167,18 @@ public class PurgeDao {
   }
 
   @VisibleForTesting
-  int purgeSnapshot(long snapshotId, PurgeMapper mapper) {
-    mapper.deleteSnapshotEvents(snapshotId);
+  void purgeSnapshot(long snapshotId, PurgeMapper mapper) {
+    // note that events are not deleted
     mapper.deleteSnapshotDependencies(snapshotId);
     mapper.deleteSnapshotDuplications(snapshotId);
     mapper.deleteSnapshotSource(snapshotId);
     mapper.deleteSnapshotViolations(snapshotId);
     mapper.deleteSnapshotWastedMeasures(snapshotId);
     mapper.updatePurgeStatusToOne(snapshotId);
-    return 7; // nb of SQL requests
   }
 
   @VisibleForTesting
-  int deleteSnapshot(Long snapshotId, PurgeMapper mapper) {
+  void deleteSnapshot(Long snapshotId, PurgeMapper mapper) {
     mapper.deleteSnapshotDependencies(snapshotId);
     mapper.deleteSnapshotDuplications(snapshotId);
     mapper.deleteSnapshotEvents(snapshotId);
@@ -193,6 +187,5 @@ public class PurgeDao {
     mapper.deleteSnapshotSource(snapshotId);
     mapper.deleteSnapshotViolations(snapshotId);
     mapper.deleteSnapshot(snapshotId);
-    return 8; // nb of SQL requests
   }
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/resource/ResourceIndexerDao.java b/sonar-core/src/main/java/org/sonar/core/resource/ResourceIndexerDao.java
index af07153a03a..24d6a04ddf1 100644
--- a/sonar-core/src/main/java/org/sonar/core/resource/ResourceIndexerDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/resource/ResourceIndexerDao.java
@@ -50,7 +50,7 @@ public class ResourceIndexerDao {
    * This method is reentrant. It can be executed even if the project is already indexed.
    */
   public ResourceIndexerDao indexProject(final int rootProjectId) {
-    SqlSession session = mybatis.openSession(ExecutorType.BATCH);
+    SqlSession session = mybatis.openBatchSession();
     try {
       ResourceIndexerMapper mapper = session.getMapper(ResourceIndexerMapper.class);
       doIndexProject(rootProjectId, session, mapper);
@@ -66,7 +66,7 @@ public class ResourceIndexerDao {
    * This method is reentrant. It can be executed even if some projects are already indexed.
    */
   public ResourceIndexerDao indexProjects() {
-    final SqlSession session = mybatis.openSession(ExecutorType.BATCH);
+    final SqlSession session = mybatis.openBatchSession();
     try {
       final ResourceIndexerMapper mapper = session.getMapper(ResourceIndexerMapper.class);
       session.select("selectRootProjectIds", /* workaround to get booleans */ResourceIndexerQuery.create(), new ResultHandler() {
diff --git a/sonar-core/src/test/java/org/sonar/core/persistence/MyBatisTest.java b/sonar-core/src/test/java/org/sonar/core/persistence/MyBatisTest.java
index 658454a76f8..254881f268e 100644
--- a/sonar-core/src/test/java/org/sonar/core/persistence/MyBatisTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/persistence/MyBatisTest.java
@@ -63,8 +63,8 @@ public class MyBatisTest {
   }
 
   @Test
-  public void shouldOpenSession() throws IOException {
-    SqlSession session = myBatis.openSession(ExecutorType.BATCH);
+  public void shouldOpenBatchSession() throws IOException {
+    SqlSession session = myBatis.openBatchSession();
     try {
       assertThat(session.getConnection(), notNullValue());
       assertThat(session.getMapper(RuleMapper.class), notNullValue());
diff --git a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteSnapshot.xml b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteSnapshot.xml
index 9a564593166..ba27c74731c 100644
--- a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteSnapshot.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteSnapshot.xml
@@ -1,17 +1,15 @@
 <dataset>
 
   <!-- snapshot to keep -->
-  <snapshots id="1" purge_status="[null]" period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+  <snapshots id="1" project_id="1" parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]"
+             status="P" islast="false" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
              period2_mode="[null]" period2_param="[null]" period2_date="[null]"
-             period3_mode="[null]" period3_param="[null]"
-             period3_date="[null]" period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
              period5_mode="[null]" period5_param="[null]" period5_date="[null]"
-             depth="[null]"
-             scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
-             version="[null]"
-             project_id="1"
-             parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]" status="P" islast="false"
-             path="[null]"/>
+             depth="[null]" scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path="[null]"/>
 
   <SNAPSHOT_SOURCES ID="1" SNAPSHOT_ID="1" DATA="foo"/>
   <rule_failures switched_off="[null]" permanent_id="[null]" ID="1" SNAPSHOT_ID="1" RULE_ID="1" FAILURE_LEVEL="2"
@@ -35,17 +33,15 @@
   <duplications_index project_snapshot_id="1" snapshot_id="1" hash="bb" index_in_file="0" start_line="0" end_line="0"/>
 
   <!-- snapshot to remove, id 5 on resource 5-->
-  <snapshots id="5" purge_status="[null]" period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+  <snapshots id="5" project_id="5" parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]"
+             status="P" islast="true" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
              period2_mode="[null]" period2_param="[null]" period2_date="[null]"
-             period3_mode="[null]" period3_param="[null]"
-             period3_date="[null]" period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
              period5_mode="[null]" period5_param="[null]" period5_date="[null]"
-             depth="[null]"
-             scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
-             version="[null]"
-             project_id="5"
-             parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]" status="P" islast="true"
-             path="[null]"/>
+             depth="[null]" scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path="[null]"/>
 
   <SNAPSHOT_SOURCES ID="2" SNAPSHOT_ID="5" DATA="foo"/>
   <rule_failures switched_off="[null]" permanent_id="[null]" ID="3" SNAPSHOT_ID="5" RULE_ID="1" FAILURE_LEVEL="2"
diff --git a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldPurgeSnapshot-result.xml b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldPurgeSnapshot-result.xml
index 0408788f8dc..ce1e62326e1 100644
--- a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldPurgeSnapshot-result.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldPurgeSnapshot-result.xml
@@ -4,7 +4,7 @@ Changes:
 * snapshot.purge_status=1
 * commented-out rows are deleted
 
-Note that measure and review are not deleted.
+Note that measures, events and reviews are not deleted.
 
 -->
 <dataset>
@@ -41,8 +41,8 @@ Note that measure and review are not deleted.
   <!--parent_dependency_id="[null]" project_snapshot_id="2"-->
   <!--dep_usage="USES" dep_weight="1" from_scope="LIB" to_scope="PRJ"/>-->
 
-  <!--<events id="1" resource_id="1" snapshot_id="1"-->
-  <!--category="VERSION" description="[null]" name="Version 1.0" event_date="2008-12-02 13:58:00.00" created_at="[null]"/>-->
+  <events id="1" resource_id="1" snapshot_id="1"
+          category="VERSION" description="[null]" name="Version 1.0" event_date="2008-12-02 13:58:00.00" created_at="[null]"/>
 
   <!--<duplications_index project_snapshot_id="1" snapshot_id="1"-->
   <!--hash="bb" index_in_file="0" start_line="0" end_line="0"/>-->