fix: recording statistics randomly failing due to lack of ongoing DB session to join fetch stats history

Author: filipowm

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)
 
@@ -61,8 +68,13 @@ internal class AnalysisService(
             try {
                 entity.project = analyze(entity.project)
                 entity.updateScore(entity.project)
-                entity.recordStatistics()
-                projectEntityRepository.save(entity)
+                transactionTemplate.executeWithoutResult {
+                    val savedEntity = projectEntityRepository.save(entity)
+                    val historyEntry = ProjectStatisticsHistory.from(savedEntity)
+                    val entryDate = historyEntry.date.toLocalDate().atStartOfDay()
+                    projectStatisticsHistoryRepository.deleteByProjectIdAndDateBetween(historyEntry.projectId, entryDate, entryDate.plusDays(1))
+                    projectStatisticsHistoryRepository.save(historyEntry)
+                }
                 progressMonitor.success()
             } catch (e: Throwable) {
                 log.error("Failed to analyze project '{}' (id={}).", entity.project.fullName, entity.project.id, e)

ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntity.kt

@@ -2,7 +2,6 @@ package com.roche.ambassador.storage.project
 
 import com.roche.ambassador.model.project.Project
 import com.vladmihalcea.hibernate.type.json.JsonBinaryType
-import org.hibernate.annotations.BatchSize
 import org.hibernate.annotations.Type
 import org.hibernate.annotations.TypeDef
 import java.time.LocalDate
@@ -13,10 +12,6 @@ import javax.persistence.*
 @Entity
 @Table(name = "project")
 @TypeDef(name = "jsonb", typeClass = JsonBinaryType::class)
-@NamedEntityGraph(
-    name = "Project.statsHistory",
-    attributeNodes = [NamedAttributeNode("statsHistory")]
-)
 class ProjectEntity(
     @Id var id: Long? = null,
     var name: String? = null,
@@ -36,15 +31,6 @@ class ProjectEntity(
     var lastIndexedDate: LocalDateTime = LocalDateTime.now(),
     @Column(name = "last_analysis_date")
     var lastAnalysisDate: LocalDateTime? = null,
-    @OneToMany(
-        mappedBy = "project",
-        cascade = [CascadeType.ALL],
-        fetch = FetchType.LAZY,
-        orphanRemoval = true
-    )
-    @BatchSize(size = 25)
-    @OrderBy("date")
-    var statsHistory: MutableList<ProjectStatisticsHistory> = mutableListOf(),
     var source: String? = null,
     @Column(name = "last_indexing_id")
     var lastIndexingId: UUID? = null,  // mapping is not needed here yet, thus not adding it
@@ -64,12 +50,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

ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectEntityRepository.kt

@@ -2,7 +2,6 @@ package com.roche.ambassador.storage.project
 
 import com.roche.ambassador.storage.Lookup
 import org.hibernate.jpa.QueryHints.HINT_CACHEABLE
-import org.springframework.data.jpa.repository.EntityGraph
 import org.springframework.data.jpa.repository.Modifying
 import org.springframework.data.jpa.repository.Query
 import org.springframework.data.jpa.repository.QueryHints
@@ -23,7 +22,6 @@ interface ProjectEntityRepository : PagingAndSortingRepository<ProjectEntity, Lo
     @Modifying
     override fun deleteAll()
 
-    @EntityGraph(value = "Project.statsHistory")
     override fun findById(id: Long): Optional<ProjectEntity>
 
     fun countAllBySubscribed(subscribed: Boolean): Long

ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistory.kt

@@ -12,9 +12,8 @@ import javax.persistence.*
 @TypeDef(name = "jsonb", typeClass = JsonBinaryType::class)
 class ProjectStatisticsHistory(
     @Id @GeneratedValue var id: UUID? = null,
-    @ManyToOne
-    @JoinColumn(name = "project_id", nullable = false, updatable = false)
-    var project: ProjectEntity,
+    @Column(name = "project_id", nullable = false, updatable = false)
+    var projectId: Long,
     @Type(type = "jsonb")
     @Column(name = "stats", columnDefinition = "jsonb", updatable = false)
     @Basic(fetch = FetchType.EAGER)
@@ -27,7 +26,7 @@ class ProjectStatisticsHistory(
         fun from(projectEntity: ProjectEntity): ProjectStatisticsHistory {
             val stats = ProjectStatistics.from(projectEntity.project)
             return ProjectStatisticsHistory(
-                null, projectEntity, stats, projectEntity.lastIndexedDate
+                null, projectEntity.id!!, stats, projectEntity.lastIndexedDate
             )
         }
     }

ambassador-storage/src/main/kotlin/com/roche/ambassador/storage/project/ProjectStatisticsHistoryRepository.kt

@@ -1,5 +1,6 @@
 package com.roche.ambassador.storage.project
 
+import org.springframework.data.jpa.repository.Modifying
 import org.springframework.data.jpa.repository.Query
 import org.springframework.data.repository.CrudRepository
 import org.springframework.data.repository.query.Param
@@ -8,16 +9,19 @@ import java.util.*
 
 interface ProjectStatisticsHistoryRepository : CrudRepository<ProjectStatisticsHistory, UUID> {
 
-    @Query("FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id ORDER BY ph.date DESC")
+    @Query("FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id ORDER BY ph.date DESC")
     fun findByProjectId(@Param("id") id: Long): List<ProjectStatisticsHistory>
 
-    @Query("FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date >= :startDate ORDER BY ph.date DESC")
+    @Query("FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date >= :startDate ORDER BY ph.date DESC")
     fun findByProjectIdAndDateGreaterThanEqual(@Param("id") id: Long, @Param("startDate") startDate: LocalDateTime): List<ProjectStatisticsHistory>
 
-    @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date < :endDate ORDER BY ph.date DESC")
+    @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date < :endDate ORDER BY ph.date DESC")
     fun findByProjectIdAndDateLessThan(@Param("id") id: Long, @Param("endDate") endDate: LocalDateTime): List<ProjectStatisticsHistory>
 
-    @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.project.id = :id AND ph.date BETWEEN :startDate AND :endDate ORDER BY ph.date DESC")
+    @Query(value = "FROM ProjectStatisticsHistory ph WHERE ph.projectId = :id AND ph.date BETWEEN :startDate AND :endDate ORDER BY ph.date DESC")
     fun findByProjectIdAndDateBetween(@Param("id") id: Long, @Param("startDate") startDate: LocalDateTime, @Param("endDate") endDate: LocalDateTime): List<ProjectStatisticsHistory>
 
+    @Query("DELETE FROM ProjectStatisticsHistory ph WHERE ph.projectId = :projectId AND ph.date BETWEEN :startDate AND :endDate")
+    @Modifying
+    fun deleteByProjectIdAndDateBetween(@Param("projectId") projectId: Long, @Param("startDate") startDate: LocalDateTime, @Param("endDate") endDate: LocalDateTime)
 }

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)
+//    }
 }