fix: recording statistics randomly failing due to lack of ongoing DB …

…session to join fetch stats history
Roche · Mar 21, 2022 · dabfe5f · dabfe5f
1 parent 9f12ef9
commit dabfe5f
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 32 deletions.
diff --git a/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt b/ambassador-application/src/main/kotlin/com/roche/ambassador/analysis/AnalysisService.kt
@@ -11,20 +11,27 @@ import com.roche.ambassador.model.project.Project
 import com.roche.ambassador.model.source.ProjectSources
 import com.roche.ambassador.storage.project.ProjectEntity
 import com.roche.ambassador.storage.project.ProjectEntityRepository
+import com.roche.ambassador.storage.project.ProjectStatisticsHistory
+import com.roche.ambassador.storage.project.ProjectStatisticsHistoryRepository
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import org.springframework.stereotype.Service
+import org.springframework.transaction.PlatformTransactionManager
 import org.springframework.transaction.annotation.Propagation
 import org.springframework.transaction.annotation.Transactional
+import org.springframework.transaction.support.TransactionTemplate
 
 @Service
 internal class AnalysisService(
     private val scorecardConfiguration: ScorecardConfiguration,
     private val projectEntityRepository: ProjectEntityRepository,
+    private val projectStatisticsHistoryRepository: ProjectStatisticsHistoryRepository,
     private val projectSources: ProjectSources,
+    platformTransactionManager: PlatformTransactionManager,
     concurrencyProvider: ConcurrencyProvider
 ) {
 
+    private val transactionTemplate: TransactionTemplate = TransactionTemplate(platformTransactionManager)
     private val analysisScope: CoroutineScope = CoroutineScope(concurrencyProvider.getSupportingDispatcher())
     private val calculator: ScorecardCalculator = ScorecardCalculator(scorecardConfiguration)
 
@@ -45,7 +52,7 @@ internal class AnalysisService(
     }
 
     // TODO run async
-    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    @Transactional
     fun analyzeAll() {
         val total = projectEntityRepository.countAllBySubscribed(true)
         val progressMonitor: ProgressMonitor = LoggingProgressMonitor(total, 2, AnalysisService::class)
@@ -57,16 +64,19 @@ internal class AnalysisService(
     }
 
     private fun analyze(entity: ProjectEntity, progressMonitor: ProgressMonitor) {
-        analysisScope.launch {
-            try {
-                entity.project = analyze(entity.project)
-                entity.updateScore(entity.project)
-                entity.recordStatistics()
-                projectEntityRepository.save(entity)
-                progressMonitor.success()
-            } catch (e: Throwable) {
-                log.error("Failed to analyze project '{}' (id={}).", entity.project.fullName, entity.project.id, e)
-                progressMonitor.failure()
+        transactionTemplate.execute {
+            analysisScope.launch {
+                try {
+                    entity.project = analyze(entity.project)
+                    entity.updateScore(entity.project)
+                    val savedEntity = projectEntityRepository.save(entity)
+                    val historyEntry = ProjectStatisticsHistory.from(savedEntity)
+                    projectStatisticsHistoryRepository.save(historyEntry)
+                    progressMonitor.success()
+                } catch (e: Throwable) {
+                    log.error("Failed to analyze project '{}' (id={}).", entity.project.fullName, entity.project.id, e)
+                    progressMonitor.failure()
+                }
             }
         }
     }

diff --git a/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt b/ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt
@@ -64,12 +64,6 @@ class ProjectEntity(
 
     fun wasIndexedBefore(otherDate: LocalDate): Boolean = lastIndexedDate.isBefore(otherDate.atStartOfDay())
 
-    fun recordStatistics(): ProjectStatisticsHistory {
-        val historyEntry = ProjectStatisticsHistory.from(this)
-        statsHistory.add(historyEntry)
-        return historyEntry
-    }
-
     fun updateIndex(project: Project) {
         this.name = project.name
         this.project = project

diff --git a/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt b/ambassador-storage/src/test/kotlin/com/roche/ambassador/storage/project/ProjectEntityTest.kt
@@ -4,10 +4,7 @@ import com.roche.ambassador.model.Visibility
 import com.roche.ambassador.model.project.Permissions
 import com.roche.ambassador.model.project.Project
 import com.roche.ambassador.model.stats.Statistics
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.jupiter.api.Test
 import java.time.LocalDate
-import java.time.LocalDateTime
 
 class ProjectEntityTest {
 
@@ -21,16 +18,16 @@ class ProjectEntityTest {
         Permissions(true, true)
     )
 
-    @Test
-    fun `should create new stats history entry when recording stats`() {
-        val project = ProjectEntity(project = createProject(), lastIndexedDate = LocalDateTime.now(),)
-
-        val history = project.recordStatistics()
-
-        assertThat(project.statsHistory).hasSize(1)
-            .containsExactly(history)
-        assertThat(history)
-            .extracting(ProjectStatisticsHistory::date, ProjectStatisticsHistory::project)
-            .containsExactly(project.lastIndexedDate, project)
-    }
+//    @Test
+//    fun `should create new stats history entry when recording stats`() {
+//        val project = ProjectEntity(project = createProject(), lastIndexedDate = LocalDateTime.now(),)
+//
+//        val history = project.recordStatistics()
+//
+//        assertThat(project.statsHistory).hasSize(1)
+//            .containsExactly(history)
+//        assertThat(history)
+//            .extracting(ProjectStatisticsHistory::date, ProjectStatisticsHistory::project)
+//            .containsExactly(project.lastIndexedDate, project)
+//    }
 }