From dabfe5f7c42cde0c73c68cf1e06fe23a59061b2d Mon Sep 17 00:00:00 2001 From: Mateusz Filipowicz Date: Mon, 21 Mar 2022 10:25:06 +0100 Subject: [PATCH] fix: recording statistics randomly failing due to lack of ongoing DB session to join fetch stats history --- .../ambassador/analysis/AnalysisService.kt | 32 ++++++++++++------- .../storage/project/ProjectEntity.kt | 6 ---- .../storage/project/ProjectEntityTest.kt | 27 +++++++--------- 3 files changed, 33 insertions(+), 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 index 6f58a58f..24aa10aa 100644 --- 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 index 5755818e..9b3de6da 100644 --- 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 index 74797235..4da95141 100644 --- 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) +// } }