Computation: propagate the same session object everywhere

  to avoid DB lock (avoid nested session)

7 files changed, 107 insertions, 59 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTask.java b/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTask.java
index a2e1fe842ec..f633414e2cd 100644
--- a/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTask.java
+++ b/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTask.java
@@ -27,6 +27,7 @@ import org.sonar.api.ServerComponent;
 import org.sonar.api.config.Settings;
 import org.sonar.api.utils.TimeUtils;
 import org.sonar.core.computation.dbcleaner.period.DefaultPeriodCleaner;
+import org.sonar.core.persistence.DbSession;
 import org.sonar.core.purge.PurgeConfiguration;
 import org.sonar.core.purge.PurgeDao;
 import org.sonar.core.purge.PurgeProfiler;
@@ -43,11 +44,11 @@ public class ProjectPurgeTask implements ServerComponent {
     this.profiler = profiler;
   }
 
-  public ProjectPurgeTask purge(PurgeConfiguration configuration, Settings settings) {
+  public ProjectPurgeTask purge(PurgeConfiguration configuration, Settings settings, DbSession session) {
     long start = System.currentTimeMillis();
     profiler.reset();
-    cleanHistoricalData(configuration.rootProjectId(), settings);
-    doPurge(configuration);
+    cleanHistoricalData(configuration.rootProjectId(), settings, session);
+    doPurge(configuration, session);
     if (settings.getBoolean(CoreProperties.PROFILING_LOG_PROPERTY)) {
       long duration = System.currentTimeMillis() - start;
       LOG.info("\n -------- Profiling for purge: " + TimeUtils.formatDuration(duration) + " --------\n");
@@ -57,18 +58,18 @@ public class ProjectPurgeTask implements ServerComponent {
     return this;
   }
 
-  private void cleanHistoricalData(long resourceId, Settings settings) {
+  private void cleanHistoricalData(long resourceId, Settings settings, DbSession session) {
     try {
-      periodCleaner.clean(resourceId, settings);
+      periodCleaner.clean(resourceId, settings, session);
     } catch (Exception e) {
       // purge errors must no fail the batch
       LOG.error("Fail to clean historical data [id=" + resourceId + "]", e);
     }
   }
 
-  private void doPurge(PurgeConfiguration configuration) {
+  private void doPurge(PurgeConfiguration configuration, DbSession session) {
     try {
-      purgeDao.purge(configuration);
+      purgeDao.purge(configuration, session);
     } catch (Exception e) {
       // purge errors must no fail the report analysis
       LOG.error("Fail to purge data [id=" + configuration.rootProjectId() + "]", e);
diff --git a/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleaner.java b/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleaner.java
index bbb6a222ba8..aedf34cf311 100644
--- a/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleaner.java
+++ b/sonar-core/src/main/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleaner.java
@@ -27,6 +27,8 @@ import org.sonar.api.ServerExtension;
 import org.sonar.api.config.Settings;
 import org.sonar.api.task.TaskExtension;
 import org.sonar.api.utils.DateUtils;
+import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.purge.PurgeDao;
 import org.sonar.core.purge.PurgeSnapshotQuery;
 import org.sonar.core.purge.PurgeableSnapshotDto;
@@ -38,10 +40,12 @@ public class DefaultPeriodCleaner implements TaskExtension, ServerExtension {
   private static final Logger LOG = LoggerFactory.getLogger(DefaultPeriodCleaner.class);
   private PurgeDao purgeDao;
   private Settings settings;
+  private MyBatis mybatis;
 
-  public DefaultPeriodCleaner(PurgeDao purgeDao, Settings settings) {
+  public DefaultPeriodCleaner(PurgeDao purgeDao, Settings settings, MyBatis mybatis) {
     this.purgeDao = purgeDao;
     this.settings = settings;
+    this.mybatis = mybatis;
   }
 
   public void clean(long projectId) {
@@ -49,27 +53,36 @@ public class DefaultPeriodCleaner implements TaskExtension, ServerExtension {
   }
 
   public void clean(long projectId, Settings settings) {
-    doClean(projectId, new Filters(settings).all());
+    DbSession session = mybatis.openSession(true);
+    try {
+      doClean(projectId, new Filters(settings).all(), session);
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
+
+  public void clean(long projectId, Settings settings, DbSession session) {
+    doClean(projectId, new Filters(settings).all(), session);
   }
 
   @VisibleForTesting
-  void doClean(long projectId, List<Filter> filters) {
-    List<PurgeableSnapshotDto> history = selectProjectSnapshots(projectId);
+  void doClean(long projectId, List<Filter> filters, DbSession session) {
+    List<PurgeableSnapshotDto> history = selectProjectSnapshots(projectId, session);
     for (Filter filter : filters) {
       filter.log();
-      delete(filter.filter(history));
+      delete(filter.filter(history), session);
     }
   }
 
-  private void delete(List<PurgeableSnapshotDto> snapshots) {
+  private void delete(List<PurgeableSnapshotDto> snapshots, DbSession session) {
     for (PurgeableSnapshotDto snapshot : snapshots) {
       LOG.info("<- Delete snapshot: " + DateUtils.formatDateTime(snapshot.getDate()) + " [" + snapshot.getSnapshotId() + "]");
-      purgeDao.deleteSnapshots(PurgeSnapshotQuery.create().setRootSnapshotId(snapshot.getSnapshotId()));
-      purgeDao.deleteSnapshots(PurgeSnapshotQuery.create().setId(snapshot.getSnapshotId()));
+      purgeDao.deleteSnapshots(PurgeSnapshotQuery.create().setRootSnapshotId(snapshot.getSnapshotId()), session);
+      purgeDao.deleteSnapshots(PurgeSnapshotQuery.create().setId(snapshot.getSnapshotId()), session);
     }
   }
 
-  private List<PurgeableSnapshotDto> selectProjectSnapshots(long resourceId) {
-    return purgeDao.selectPurgeableSnapshots(resourceId);
+  private List<PurgeableSnapshotDto> selectProjectSnapshots(long resourceId, DbSession session) {
+    return purgeDao.selectPurgeableSnapshots(resourceId, session);
   }
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java
index 6865434dfcd..9ba8f2468fa 100644
--- a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java
@@ -102,12 +102,16 @@ public class PropertiesDao implements BatchComponent, ServerComponent, DaoCompon
   public List<PropertyDto> selectProjectProperties(long resourceId) {
     SqlSession session = mybatis.openSession(false);
     try {
-      return session.getMapper(PropertiesMapper.class).selectProjectPropertiesByResourceId(resourceId);
+      return selectProjectProperties(resourceId, session);
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
+  public List<PropertyDto> selectProjectProperties(long resourceId, SqlSession session) {
+    return session.getMapper(PropertiesMapper.class).selectProjectPropertiesByResourceId(resourceId);
+  }
+
   public List<PropertyDto> selectProjectProperties(String resourceKey) {
     SqlSession session = mybatis.openSession(false);
     try {
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 08589a2d465..55b5981f1b5 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
@@ -56,19 +56,8 @@ public class PurgeDao {
 
   public PurgeDao purge(PurgeConfiguration conf) {
     DbSession session = mybatis.openSession(true);
-    PurgeMapper mapper = session.getMapper(PurgeMapper.class);
-    PurgeCommands commands = new PurgeCommands(session, mapper, profiler);
     try {
-      List<ResourceDto> projects = getProjects(conf.rootProjectId(), session);
-      for (ResourceDto project : projects) {
-        LOG.info("-> Clean " + project.getLongName() + " [id=" + project.getId() + "]");
-        deleteAbortedBuilds(project, commands);
-        purge(project, conf.scopesWithoutHistoricalData(), commands);
-      }
-      for (ResourceDto project : projects) {
-        disableOrphanResources(project, session, mapper);
-      }
-      deleteOldClosedIssues(conf, mapper);
+      purge(conf, session);
       session.commit();
     } finally {
       MyBatis.closeQuietly(session);
@@ -76,6 +65,21 @@ public class PurgeDao {
     return this;
   }
 
+  public void purge(PurgeConfiguration conf, DbSession session) {
+    PurgeMapper mapper = session.getMapper(PurgeMapper.class);
+    PurgeCommands commands = new PurgeCommands(session, mapper, profiler);
+    List<ResourceDto> projects = getProjects(conf.rootProjectId(), session);
+    for (ResourceDto project : projects) {
+      LOG.info("-> Clean " + project.getLongName() + " [id=" + project.getId() + "]");
+      deleteAbortedBuilds(project, commands);
+      purge(project, conf.scopesWithoutHistoricalData(), commands);
+    }
+    for (ResourceDto project : projects) {
+      disableOrphanResources(project, session, mapper);
+    }
+    deleteOldClosedIssues(conf, mapper);
+  }
+
   private void deleteOldClosedIssues(PurgeConfiguration conf, PurgeMapper mapper) {
     Date toDate = conf.maxLiveDateOfClosedIssues();
     mapper.deleteOldClosedIssueChanges(conf.rootProjectId(), toDate);
@@ -87,7 +91,7 @@ public class PurgeDao {
       LOG.info("<- Delete aborted builds");
       PurgeSnapshotQuery query = PurgeSnapshotQuery.create()
         .setIslast(false)
-        .setStatus(new String[]{"U"})
+        .setStatus(new String[] {"U"})
         .setRootProjectId(project.getId());
       commands.deleteSnapshots(query);
     }
@@ -96,7 +100,7 @@ public class PurgeDao {
   private boolean hasAbortedBuilds(Long projectId, PurgeCommands commands) {
     PurgeSnapshotQuery query = PurgeSnapshotQuery.create()
       .setIslast(false)
-      .setStatus(new String[]{"U"})
+      .setStatus(new String[] {"U"})
       .setResourceId(projectId);
     return !commands.selectSnapshotIds(query).isEmpty();
   }
@@ -107,7 +111,7 @@ public class PurgeDao {
         .setResourceId(project.getId())
         .setIslast(false)
         .setNotPurged(true)
-    );
+      );
     for (final Long projectSnapshotId : projectSnapshotIds) {
       LOG.info("<- Clean snapshot " + projectSnapshotId);
       if (!ArrayUtils.isEmpty(scopesWithoutHistoricalData)) {
@@ -142,18 +146,22 @@ public class PurgeDao {
   public List<PurgeableSnapshotDto> selectPurgeableSnapshots(long resourceId) {
     DbSession session = mybatis.openSession(true);
     try {
-      PurgeMapper mapper = session.getMapper(PurgeMapper.class);
-      List<PurgeableSnapshotDto> result = Lists.newArrayList();
-      result.addAll(mapper.selectPurgeableSnapshotsWithEvents(resourceId));
-      result.addAll(mapper.selectPurgeableSnapshotsWithoutEvents(resourceId));
-      // sort by date
-      Collections.sort(result);
-      return result;
+      return selectPurgeableSnapshots(resourceId, session);
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
+  public List<PurgeableSnapshotDto> selectPurgeableSnapshots(long resourceId, DbSession session) {
+    PurgeMapper mapper = session.getMapper(PurgeMapper.class);
+    List<PurgeableSnapshotDto> result = Lists.newArrayList();
+    result.addAll(mapper.selectPurgeableSnapshotsWithEvents(resourceId));
+    result.addAll(mapper.selectPurgeableSnapshotsWithoutEvents(resourceId));
+    // sort by date
+    Collections.sort(result);
+    return result;
+  }
+
   public PurgeDao deleteResourceTree(long rootProjectId) {
     final DbSession session = mybatis.openSession(true);
     final PurgeMapper mapper = session.getMapper(PurgeMapper.class);
@@ -185,14 +193,18 @@ public class PurgeDao {
   public PurgeDao deleteSnapshots(PurgeSnapshotQuery query) {
     final DbSession session = mybatis.openSession(true);
     try {
-      new PurgeCommands(session, profiler).deleteSnapshots(query);
-      return this;
+      return deleteSnapshots(query, session);
 
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
+  public PurgeDao deleteSnapshots(PurgeSnapshotQuery query, final DbSession session) {
+    new PurgeCommands(session, profiler).deleteSnapshots(query);
+    return this;
+  }
+
   /**
    * Load the whole tree of projects, including the project given in parameter.
    */
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 8e4be2cfce8..c2f648720bb 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
@@ -53,8 +53,7 @@ public class ResourceIndexerDao {
   public ResourceIndexerDao indexProject(final int rootProjectId) {
     DbSession session = mybatis.openSession(true);
     try {
-      ResourceIndexerMapper mapper = session.getMapper(ResourceIndexerMapper.class);
-      doIndexProject(rootProjectId, session, mapper);
+      indexProject(rootProjectId, session);
       session.commit();
       return this;
 
@@ -63,6 +62,11 @@ public class ResourceIndexerDao {
     }
   }
 
+  public void indexProject(final int rootProjectId, DbSession session) {
+    ResourceIndexerMapper mapper = session.getMapper(ResourceIndexerMapper.class);
+    doIndexProject(rootProjectId, session, mapper);
+  }
+
   /**
    * This method is reentrant. It can be executed even if some projects are already indexed.
    */
diff --git a/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTaskTest.java b/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTaskTest.java
index 5b96c00a150..b9e034193eb 100644
--- a/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTaskTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/ProjectPurgeTaskTest.java
@@ -26,11 +26,19 @@ import org.slf4j.Logger;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
 import org.sonar.core.computation.dbcleaner.period.DefaultPeriodCleaner;
+import org.sonar.core.persistence.DbSession;
 import org.sonar.core.purge.PurgeConfiguration;
 import org.sonar.core.purge.PurgeDao;
 import org.sonar.core.purge.PurgeProfiler;
 
-import static org.mockito.Mockito.*;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class ProjectPurgeTaskTest {
 
@@ -53,7 +61,7 @@ public class ProjectPurgeTaskTest {
     Settings settings = mock(Settings.class);
     when(settings.getBoolean(CoreProperties.PROFILING_LOG_PROPERTY)).thenReturn(false);
 
-    sut.purge(mock(PurgeConfiguration.class), settings);
+    sut.purge(mock(PurgeConfiguration.class), settings, mock(DbSession.class));
 
     verify(profiler, never()).dump(anyLong(), any(Logger.class));
   }
@@ -63,7 +71,7 @@ public class ProjectPurgeTaskTest {
     Settings settings = mock(Settings.class);
     when(settings.getBoolean(CoreProperties.PROFILING_LOG_PROPERTY)).thenReturn(true);
 
-    sut.purge(mock(PurgeConfiguration.class), settings);
+    sut.purge(mock(PurgeConfiguration.class), settings, mock(DbSession.class));
 
     verify(profiler, times(1)).dump(anyLong(), any(Logger.class));
   }
@@ -72,17 +80,17 @@ public class ProjectPurgeTaskTest {
   public void if_dao_purge_fails_it_should_not_interrupt_program_execution() throws Exception {
     when(dao.purge(any(PurgeConfiguration.class))).thenThrow(NullPointerException.class);
 
-    sut.purge(mock(PurgeConfiguration.class), mock(Settings.class));
+    sut.purge(mock(PurgeConfiguration.class), mock(Settings.class), mock(DbSession.class));
 
-    verify(dao, times(1)).purge(any(PurgeConfiguration.class));
+    verify(dao, times(1)).purge(any(PurgeConfiguration.class), any(DbSession.class));
   }
 
   @Test
   public void if_profiler_cleaning_fails_it_should_not_interrupt_program_execution() throws Exception {
     doThrow(NullPointerException.class).when(periodCleaner).clean(anyLong(), any(Settings.class));
 
-    sut.purge(mock(PurgeConfiguration.class), mock(Settings.class));
+    sut.purge(mock(PurgeConfiguration.class), mock(Settings.class), mock(DbSession.class));
 
-    verify(periodCleaner, times(1)).clean(anyLong(), any(Settings.class));
+    verify(periodCleaner, times(1)).clean(anyLong(), any(Settings.class), any(DbSession.class));
   }
 }
diff --git a/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleanerTest.java b/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleanerTest.java
index 0a5872041d2..759dac500af 100644
--- a/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleanerTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/computation/dbcleaner/period/DefaultPeriodCleanerTest.java
@@ -27,6 +27,8 @@ import org.mockito.ArgumentMatcher;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.sonar.api.config.Settings;
+import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.purge.PurgeDao;
 import org.sonar.core.purge.PurgeSnapshotQuery;
 import org.sonar.core.purge.PurgeableSnapshotDto;
@@ -36,26 +38,30 @@ import java.util.Date;
 
 import static org.mockito.Matchers.anyListOf;
 import static org.mockito.Matchers.argThat;
-import static org.mockito.Mockito.*;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class DefaultPeriodCleanerTest {
 
-
   @Test
   public void doClean() {
     PurgeDao dao = mock(PurgeDao.class);
-    when(dao.selectPurgeableSnapshots(123L)).thenReturn(Arrays.asList(
-        new PurgeableSnapshotDto().setSnapshotId(999L).setDate(new Date())));
+    DbSession session = mock(DbSession.class);
+    when(dao.selectPurgeableSnapshots(123L, session)).thenReturn(Arrays.asList(
+      new PurgeableSnapshotDto().setSnapshotId(999L).setDate(new Date())));
     Filter filter1 = newLazyFilter();
     Filter filter2 = newLazyFilter();
 
-    DefaultPeriodCleaner cleaner = new DefaultPeriodCleaner(dao, mock(Settings.class));
-    cleaner.doClean(123L, Arrays.asList(filter1, filter2));
+    DefaultPeriodCleaner cleaner = new DefaultPeriodCleaner(dao, mock(Settings.class), mock(MyBatis.class));
+    cleaner.doClean(123L, Arrays.asList(filter1, filter2), session);
 
     verify(filter1).log();
     verify(filter2).log();
-    verify(dao, times(2)).deleteSnapshots(argThat(newRootSnapshotQuery()));
-    verify(dao, times(2)).deleteSnapshots(argThat(newSnapshotIdQuery()));
+    verify(dao, times(2)).deleteSnapshots(argThat(newRootSnapshotQuery()), eq(session));
+    verify(dao, times(2)).deleteSnapshots(argThat(newSnapshotIdQuery()), eq(session));
   }
 
   private BaseMatcher<PurgeSnapshotQuery> newRootSnapshotQuery() {